changed plugin specification: use generic "handle_event" instead of "setup" and "cleanup"

fixed bug in reinitializing of plugins (Closes: #111)
fixed glitch that delayed the results of the plugin_manager to go into effect by one request

plugins/plugin-interface.txt

@@ -31,10 +31,9 @@ Python code interface:
 	- function "get_status":
 		- returns a string, that describes a state connected to this plugin (e.g. the current date and
 		  time (for the "date" plugin))
-	- function "setup":
-		- may be overridden to specify bootup behaviour
-	- function "cleanup":
-		- may be overridden to specify shutdown behaviour
+	- function "handle_event(event, event_info)":
+		- may be overridden to specify event handling (e.g. "bootup", "shutdown")
+		- see src/cryptobox/plugins/base.py for details
 	- the class variable "plugin_capabilities" must be an array of strings (supported: "system" and
 	  "volume")
 	- the class variable "plugin_visibility" may contain one or more of the following items:

plugins/volume_automount/volume_automount.py

@@ -62,15 +62,16 @@ class volume_automount(cryptobox.plugins.base.CryptoBoxPlugin):
 		return "volume_automount"
 	
 
-	def setup(self):
+	def handle_event(self, event, event_info=None):
 		"""Override bootup behaviour.
 		
 		Mount all volumes marked as 'automount'.
 		"""
-		cryptobox.plugins.base.CryptoBoxPlugin.setup(self)
-		for cont in self.cbox.get_container_list():
-			if self.__is_auto_mount(cont) and not cont.is_mounted():
-				cont.mount()
+		cryptobox.plugins.base.CryptoBoxPlugin.handle_event(self, event, event_info)
+		if event == "bootup":
+			for cont in self.cbox.get_container_list():
+				if self.__is_auto_mount(cont) and not cont.is_mounted():
+					cont.mount()
 	
 
 	def is_useful(self, device):

src/cryptobox/plugins/base.py

@@ -90,14 +90,12 @@ class CryptoBoxPlugin:
 		return self.__module__
 	
 
-	def setup(self):
-		"""Any plugin that wants to define bootup actions may override this.
-		"""
-		pass
+	def handle_event(self, event_name, event_info=None):
+		"""Any plugin that wants to define event actions may override this.
 
-
-	def cleanup(self):
-		"""Any plugin that wants to define shutdown actions may override this.
+		currently only the following events are defined:
+			- "bootup" (the cryptobox server is starting)
+			- "shutdown" (the cryptobox server is stopping)
 		"""
 		pass
 

src/cryptobox/web/sites.py

@@ -43,7 +43,10 @@ GETTEXT_DOMAIN = 'cryptobox-server'
 
 
 class PluginIconHandler:
-	"""deliver the icons of available plugins via cherrypy"""
+	"""deliver the icons of available plugins via cherrypy
+	
+	the state (enabled/disabled) and the require-auth setting is ignored to
+	avoid repetitive reloading"""
 
 	def __init__(self, plugins):
 		for plugin in plugins.get_plugins():
@@ -76,13 +79,19 @@ class WebInterfaceSites:
 		self.cbox = cryptobox.core.main.CryptoBox(conf_file)
 		self.__cached_language_data = None
 		self.__dataset = None
-		self.icons = None
-		self.__plugin_manager = None
+		## load the plugin manager - we will not try to detect new plugins on
+		## the fly ...
+		self.__plugin_manager = cryptobox.plugins.manage.PluginManager(
+				self.cbox, self.cbox.prefs["Locations"]["PluginDir"], self)
 		self.__reset_dataset()
 		## store the original http error handler
 		self._cp_on_http_error = self.new_http_error_handler
 		## set initial language order
 		self.lang_order = self.cbox.prefs["WebSettings"]["Languages"][:]
+		## publish plugin icons
+		self.icons = PluginIconHandler(self.__plugin_manager)
+		self.icons.exposed = True
+		## announce that the server started up
 		self.setup()
 
 
@@ -92,7 +101,7 @@ class WebInterfaceSites:
 		self.cbox.setup()
 		for plugin in self.__plugin_manager.get_plugins():
 			if plugin:
-				plugin.setup()
+				plugin.handle_event("bootup")
 
 
 	def cleanup(self):
@@ -102,7 +111,7 @@ class WebInterfaceSites:
 		for plugin in self.__plugin_manager.get_plugins():
 			if plugin:
 				self.cbox.log.info("Cleaning up plugin '%s' ..." % plugin.get_name())
-				plugin.cleanup()
+				plugin.handle_event("shutdown")
 		self.cbox.cleanup()
 
 
@@ -114,10 +123,7 @@ class WebInterfaceSites:
 		"""
 		self.__load_plugins()
 		self.__dataset = cryptobox.web.dataset.WebInterfaceDataset(
-				self.cbox, self.cbox.prefs, self.__plugin_manager.get_plugins())
-		## publish plugin icons
-		self.icons = PluginIconHandler(self.__plugin_manager)
-		self.icons.exposed = True
+				self.cbox, self.cbox.prefs, self.__plugin_manager)
 		## check, if a configuration partition has become available
 		self.cbox.prefs.prepare_partition()
 
@@ -129,38 +135,49 @@ class WebInterfaceSites:
 		  - reload all plugins and check their state (disabled or not)
 		  - reinitilize the datasets of all plugins
 		"""
-		self.__plugin_manager = cryptobox.plugins.manage.PluginManager(
-				self.cbox, self.cbox.prefs["Locations"]["PluginDir"], self)
+		#TODO: in the long-term we should create a separate object that only
+		# contains the plugin handlers - this avoids some hassle of namespace
+		# conflicts - this object will be the cherrypy.server.root
+		# finish this for v0.4
 		for plugin in self.__plugin_manager.get_plugins():
 			if not plugin:
 				continue
 			plname = plugin.get_name()
-			## check if there are name conflicts: e.g. a local variable has the
-			## same name as a plugin to be loaded -> skip these plugins
-			## if we do not check this here, nasty side effects may occour ...
+			## remove the old plugin handler and attach a new one
 			try:
+				## check if there are name conflicts: e.g. a local variable has
+				## the same name as a plugin to be loaded -> skip these plugins
+				## if we would not check this here, nasty effects could occour
 				prev_obj = getattr(self, plname)
-				if not callable(prev_obj) \
-						or not prev_obj.exposed:
-					self.cbox.log.error("Skipped feature (%s) as its name"
-						    + " conflicts with a local variable - see"
-						    + " module cryptobox.web.sites" % plname)
-					## skip this plugin
-					continue
-			except (NameError, AttributeError):
-				## an attribute with the same name does not exist -> ok
-				if plugin.is_enabled():
-					self.cbox.log.info("Plugin '%s' loaded" % plname)
-					## expose all features as URLs
-					setattr(self, plname, self.return_plugin_action(plugin))
-					getattr(self, plname).exposed = True
-					#TODO: check, if this really works
-					#for now the "stream_response" feature seems to be broken
-					#setattr(getattr(self, plname), "stream_respones", True)
-				else:
-					self.cbox.log.info("Plugin '%s' is disabled" % plname)
-					## remove the plugin, if it was active before
-					setattr(self, plname, None)
+				if not callable(prev_obj) or not prev_obj.exposed:
+					## name conflict - see below
+					raise NameError
+				## remove the plugin handler
+				delattr(self, plname)
+			except AttributeError:
+				## "self" does not contain the given "plname" element
+				## this is ok, as we are just cleaning up
+				pass
+			except NameError:
+				## the attribute "exposed" of the element self."plname" does 
+				## not exist - it seems, that we have a name conflict
+				self.cbox.log.error("Skipping feature (%s) as its" % plname
+						+ " name conflicts with a local variable - see"
+						+ " module cryptobox.web.sites")
+				## skip this plugin
+				continue
+			## the old attribute was cleaned up - we can reinitialize it now
+			if plugin.is_enabled():
+				self.cbox.log.info("Plugin '%s' loaded" % plname)
+				## expose all features as URLs
+				setattr(self, plname, self.return_plugin_action(plugin))
+				getattr(self, plname).exposed = True
+				#TODO: check, if the stream_response feature really works
+				#for now the "stream_response" feature seems to be broken
+				#setattr(getattr(self, plname), "stream_respones", True)
+			else:
+				self.cbox.log.info("Plugin '%s' is disabled" % plname)
+				## nothing else has to be done
 
 
 	## sub pages requiring authentication may not be defined above
@@ -273,7 +290,8 @@ class WebInterfaceSites:
 		""" returns a function that is suitable for handling a cherrypy
 		page request
 		"""
-		def handler(self, weblang="", device=None, help="0", redirect=None, message_keep=None, **args):
+		def handler(self, weblang="", device=None, help="0", redirect=None,
+				message_keep=None, **args):
 			"""this function handles a cherrypy page request
 			"""
 			plugin.reset()
@@ -323,10 +341,17 @@ class WebInterfaceSites:
 			else:
 				## some non-volume plugins change the internal state of other
 				## plugins - e.g.: plugin_manager
+				## if we do not call __load_plugins now, then it is possible
+				## to call a plugin directly after disabling it (only once)
+				self.__load_plugins()
 				self.__dataset.set_plugin_data()
 				## default page for non-volume plugins is the disk selection
 				if not next_template:
 					next_template = { "plugin":"disks", "values":{} }
+			#TODO: there is a lot of piece-by-piece updating around here
+			# for v0.4 we should just call __reset_dataset - but this would
+			# require to store the currently changed dataset values (e.g.i
+			# weblang) somewhere else to not override it
 			## some non-volume plugins may change the state of containers
 			## the mount plugin may change the number of active disks - for the logo
 			self.__dataset.set_containers_state()
@@ -412,8 +437,7 @@ class WebInterfaceSites:
 		## check an environment setting - this is quite common behind proxies
 		if os.environ.has_key("HTTPS"):
 			return True
-		## this arbitrarily chosen header must be documented in README.proxy
-		#TODO: check http://jamesthornton.com/writing/openacs-pound.html for this
+		## this arbitrarily chosen header is documented in README.proxy
 		if cherrypy.request.headers.has_key("X-SSL-Request") \
 				and (cherrypy.request.headers["X-SSL-Request"] == "1"):
 			return True