minor language fixes

one unittest fixed
settings for webserver improved
somme RFCs and TODOs added

pythonrewrite/bin2/CryptoBox.py

@@ -148,12 +148,12 @@ class CryptoBox:
 		try:
 			for key in ["MountParentDir", "NameDatabase", "TemplateDir", "LangDir", "DocDir"]:
 				value = self.cbxPrefs["Locations"][key]
-				#if not (os.path.exists(value) and os.access(value, os.R_OK)):
-				if not os.access(value, os.R_OK):
+				if not (os.path.exists(value) and os.access(value, os.R_OK)):
 					raise CBConfigInvalidValueError("Locations", key, value, "could not access")
 		except KeyError:
 			raise CBConfigUndefinedError("Locations", key)
 
+
 	def __runTestRootPriv(self):
 		"""try to run 'super' with 'CryptoBoxRootActions'"""
 		try:
@@ -287,6 +287,7 @@ class CryptoBoxProps(CryptoBox):
 		"the uuid was not found"
 		return None
 
+
 	def getAvailableLanguages(self):
 		'''reads all files in path LangDir and returns a list of
 		basenames from existing hdf files, that should are all available
@@ -298,6 +299,7 @@ class CryptoBoxProps(CryptoBox):
 			self.log.warn("No .hdf files found! The website won't render properly.")
 		return languages
 
+
 	def getAvailableDocLanguages(self):
 		'''reads all dirs in path DocDir and returns a list of
 		available documentation languages, this list might be empty.'''

pythonrewrite/bin2/CryptoBoxWebserver.py

@@ -17,17 +17,14 @@ class CryptoBoxWebserver:
 		#I currently have no idea how to cleanly extract the stylesheet path from 
 		#the config object without an extra CryptoBox.CryptoBoxProps instance.
 		#perhaps put config handling into a seperate class in CryptoBox.py?
-		# [l] why do we need to map the css manually? Shouldn't the whole
-		# www-data path be accessible anyway?
-		'''
-		cherrypy.config.configMap.update(
-			{ 
-			"/cryptobox.css": { 
+		#
+		# the following manual mapping is necessary, as we may not use relative
+		# paths in the config file
+		cherrypy.config.configMap.update({ 
+			"/cryptobox-misc": { 
 					"staticFilter.on" : True, 
-					"staticFilter.file": os.path.abspath("../www-data/cryptobox.css" )
-				}
-		})
-		'''
+					"staticFilter.dir": os.path.abspath("../www-data" )}
+			})
 		
 	def start(self):
 		# just use this config, when we're startet directly

pythonrewrite/bin2/CryptoBoxWebserverRender.py

@@ -25,6 +25,7 @@ class CryptoBoxWebserverRender:
 			return "Couldn't read cs file: %s" % cs_path
 
 		#hdf_path = website.cbxPrefs["Locations"]["LangDir"]+"/"+website.cbxPrefs["WebSettings"]["Language"]+".hdf"
+		# use the user selected language ('Settings.Language') instead of the configured
 		hdf_path = website.cbxPrefs["Locations"]["LangDir"]+"/"+website.settings["Settings.Language"]+".hdf"
 		if not os.access(hdf_path, os.R_OK):
 			website.log.error("Couldn't read hdf file: %s" % hdf_path)
@@ -32,7 +33,7 @@ class CryptoBoxWebserverRender:
 
 		hdf = neo_util.HDF()
 		hdf.readFile(hdf_path)
-		website.log.info(website.settings)
+		website.log.debug(website.settings)
 		for key in website.settings.keys():
 			hdf.setValue(key,str(website.settings[key]))
 		cs = neo_cs.CS(hdf)

pythonrewrite/bin2/CryptoBoxWebserverSettings.py

@@ -10,6 +10,8 @@ class CryptoBoxWebserverSettings:
 		website.settings={}
 		#TODO: this is nessessary since the last config split in different sections
 		## put all found WebSettings values in the dictionary
+		## RFC: arbitrarily importing all available keys does not sound very secure [l]
+		## RFC: settings with the same names in different sections (Log, Main, ...) will collide silently - right? [l]
 		for key in self.cbxPrefs["WebSettings"].keys():
 			website.settings["Settings."+key] = self.cbxPrefs["WebSettings"][key]
 		## also all Log values

pythonrewrite/bin2/CryptoBoxWebserverSites.py

@@ -1,10 +1,13 @@
 import CryptoBox
 import CryptoBoxWebserverSettings
 import CryptoBoxWebserverRender
+# RFC: is this global variable necessary? [l]
 website = CryptoBoxWebserverRender.CryptoBoxWebserverRender()
 
-# is it necessary to inherit these both classes?
-# for clarity they should be just instanciated - or not?
+# RFC: where should we put the information gathering? (available harddisks, current volume info, ...) - selectively for every "site" or always (as before)? [l]
+
+# RFC: is it necessary to inherit these both classes?
+#	for clarity they should be just instanciated - or not? [l]
 class CryptoBoxWebserverSites(CryptoBox.CryptoBoxProps, CryptoBoxWebserverSettings.CryptoBoxWebserverSettings):
 	'''
 	url2func = {'index':'show_status','doc':'show_doc','logs':'show_log'}
@@ -41,7 +44,9 @@ class CryptoBoxWebserverSites(CryptoBox.CryptoBoxProps, CryptoBoxWebserverSettin
 		checking.
 		# RFC: why shouldn't it be called in __init__? [l]
 			there is no such thing like __init__ in cherrypy sites [a]
+				what about the unnamed place, where 'exposed' attributes are set? [l]
 		'''
+		# RFC: this dictionary is not sufficient for arbitrary text inputs (e.g.: names) or numbers [l]
 		niceparams = { 
 				'weblang': ["Settings.Language", self.settings["Settings.AvailableLanguages"]],
 				'loglevel': ["Log.Level", ('','info', 'warn', 'debug', 'error')],
@@ -50,11 +55,13 @@ class CryptoBoxWebserverSites(CryptoBox.CryptoBoxProps, CryptoBoxWebserverSettin
 		## check all given evil parameters against the nice ones
 		## set them to self.settings if accepted, otherwise do nothing
 		for evilkey in evilparams.keys():
+			# TODO: should be easier without the following for-loop
 			for nicekey in niceparams.keys():
 				if evilkey == nicekey:
 					#self.log.warn(niceparams[nicekey][0])
 					#self.log.warn(niceparams[nicekey][1])
 					if evilparams[nicekey] and evilparams[nicekey] in niceparams[nicekey][1]: 
+						# RFC: isn't "self.settings" a non-obvious name for user input? [l]
 						self.settings[niceparams[nicekey][0]] = evilparams[nicekey]
 						#self.log.warn(niceparams[nicekey][0])
 						#self.log.warn(evilparams[nicekey])
@@ -75,16 +82,17 @@ class CryptoBoxWebserverSites(CryptoBox.CryptoBoxProps, CryptoBoxWebserverSettin
 		return
 
 	def __isHDAvailable(self):
-		return False
+		#TODO: implement this
+		return True
 
 	def __check_config(self):
-		#TODO
-		pass
+		#TODO: from now on a cryptobox is always configured
+		return True
 
 
 	def __check_init_running(self):
-		#TODO
-		pass
+		#TODO: implement this check (is mkfs still running?)
+		return False
 
 	######################################################################
 	## put real sites down here and don't forget to expose them at the end
@@ -93,17 +101,18 @@ class CryptoBoxWebserverSites(CryptoBox.CryptoBoxProps, CryptoBoxWebserverSettin
 		'''displays a HTML version of the logfile
 
 		The loglevel has to be set and nothing else, as we just log in english.
-		RFC: what does this mean? We still have to save the current language - or not?
+		RFC: what does this mean? We still have to save the current language - or not? [l]
 
 		Be aware not to name this method just "log" as it seems to be a
 		reserved word.
-		# RFC: maybe it conflicts with CryptoBoxProps.log - which we inherited?
+		# RFC: maybe it conflicts with CryptoBoxProps.log - which we inherited? [l]
 		'''
 		self.__prepare("show_log")
 		self.__sanitize_input({"loglevel":loglevel})
 		self.settings["Data.Log"] = "<br/>".join(self.getLogData(lines=30, maxSize=2000))
 		return website.render(self)
 
+
 	def status(self, weblang=""):
 		'''shows the current status of the box
 		'''
@@ -127,15 +136,16 @@ class CryptoBoxWebserverSites(CryptoBox.CryptoBoxProps, CryptoBoxWebserverSettin
 		pass
 
 
-	def doc(self,action="",page="",weblang=""):
+	# TODO: add "doclang" - it is selected by each link in the doc pages
+	def doc(self,page="",weblang=""):
 		'''prints the offline wikipage
 
-		TODO: "action" is unnessessary, remove it here and from all html
-		files in doc/html/[de|en]/*
+		TODO: "action" is unnessessary, remove it from all html files in doc/html/[de|en]/*
 		'''
 		self.__prepare("show_doc")
 		# TODO: single pagenames should be sanitized
 		self.__sanitize_input({"weblang":weblang}) 
+		# TODO: check the supplied page name for validity (is it text? pattern match?)
 		if page:
 			self.settings["Data.Doc.Page"] = page
 		else:
@@ -143,9 +153,12 @@ class CryptoBoxWebserverSites(CryptoBox.CryptoBoxProps, CryptoBoxWebserverSettin
 			self.settings["Data.Doc.Page"] ="CryptoBoxUser"
 		if len(self.settings["Settings.AvailableDocLanguages"]) < 1:
 			self.settings["Data.Error"] = "NoDocumentation"
+			# TODO: what should be done, if there is an error?
 		## set doclang to weblang, otherwise the default weblang from the config will be used for doclang
+		# TODO: hard coded languages?
 		elif self.settings["Settings.Language"] in ("en","de"):
 			self.settings["Settings.DocLang"] = self.settings["Settings.Language"]
+		# TODO: missing 'else'?
 
 		return website.render(self)
 

pythonrewrite/bin2/cryptobox.conf

@@ -53,7 +53,7 @@ Details	= ./cryptobox.log
 
 [WebSettings]
 # URL of default stylesheet
-Stylesheet = cryptobox.css
+Stylesheet = /cryptobox-misc/cryptobox.css
 
 # default language
 Language = de

pythonrewrite/bin2/cryptoboxwebserver.conf

@@ -9,9 +9,5 @@ server.logFile = "cryptoboxwebserver.log"
 
 [/favicon.ico]
 static_filter.on = True
-static_filter.file = "/var/www/cryptobox/favicon.ico"
-
-[/cryptobox.css]
-static_filter.on = True
-static_filter.file = "/var/www/cryptobox/cryptobox.css"
+static_filter.file = "/usr/share/doc/python-cherrypy/cherrypy/favicon.ico"
 

pythonrewrite/bin2/unittests.CryptoBox.py

@@ -71,11 +71,11 @@ CryptoBoxRootActions	= CryptoBoxRootActions
 	def tearDown(self):
 		'''remove the created tmpfiles'''
 		os = self.os
-		os.chdir(self.tmpdirname)
 		# remove temp files
 		for file in self.filenames.values():
-			if os.path.exists(file): 
-				os.remove(file)
+			compl_name = os.path.join(self.tmpdirname, file)
+			if os.path.exists(compl_name): 
+				os.remove(compl_name)
 		# remove temp dir
 		os.rmdir(self.tmpdirname)
 
@@ -85,12 +85,16 @@ CryptoBoxRootActions	= CryptoBoxRootActions
 		import os
 		self.assertRaises(CBConfigUnavailableError, self.CryptoBox.CryptoBoxProps,"/invalid/path/to/config/file")
 		self.assertRaises(CBConfigUnavailableError, self.CryptoBox.CryptoBoxProps,"/etc/shadow")
+		""" check one of the following things:
+		1) are we successfully using an existing config file?
+		2) do we break, if no config file is there?
+		depending on the existence of a config file, only one of these conditions
+		can be checked - hints for more comprehensive tests are appreciated :) """
 		for a in self.CryptoBox.CONF_LOCATIONS:
-			if os.path.exists(a): 
+			if os.path.exists(a):
 				self.CryptoBox.CryptoBoxProps()
-				self.CryptoBox.CryptoBoxProps(self.filenames["configFileOK"])
-			else: 
-				self.assertRaises(CBConfigUnavailableError, self.CryptoBox.CryptoBoxProps) 
+				break	# this skips the 'else' clause
+		else:	self.assertRaises(CBConfigUnavailableError, self.CryptoBox.CryptoBoxProps) 
 		self.assertRaises(CBConfigUnavailableError, self.CryptoBox.CryptoBoxProps,[])
 	
 	def testBrokenConfigs(self):

pythonrewrite/bin2/unittests.CryptoBoxWebserver.py

@@ -1,3 +1,5 @@
+#!/usr/bin/env python2.4
+
 import unittest
 import cherrypy, os
 import CryptoBox

pythonrewrite/lang/en.hdf

@@ -252,8 +252,8 @@ Lang {
 		}
 
 		NoDocumentation {
-			Title	= No Documentation
-			Text	= There is no Documentation available!
+			Title	= No documentation
+			Text	= There is no documentation available!
 		}
 	}
 

pythonrewrite/lang/si.hdf

@@ -213,8 +213,8 @@ Lang {
 		}
 
 		NoDocumentation {
-			Title	= No Documentation
-			Text	= There is no Documentation available!
+			Title	= No documentation
+			Text	= There is no documentation available!
 		}
 	}