Fix plugins enablement code

Signed-off-by: Simo Sorce <simo@redhat.com>
Reviewed-by: Patrick Uiterwijk <puiterwijk@redhat.com>

4 files changed, 43 insertions, 44 deletions

diff --git a/ipsilon/admin/common.py b/ipsilon/admin/common.py
index b68e53e..b9dfbf4 100755
--- a/ipsilon/admin/common.py
+++ b/ipsilon/admin/common.py
@@ -127,11 +127,18 @@ class AdminPluginsOrder(AdminPage):
     def GET(self, *args, **kwargs):
         return self.parent.root_with_msg()
 
+    def _get_enabled_by_name(self):
+        by_name = dict()
+        for p in self._site[self.facility]['available'].values():
+            if p.is_enabled:
+                by_name[p.name] = p
+        return by_name
+
     @admin_protect
     def POST(self, *args, **kwargs):
         message = "Nothing was modified."
         message_type = "info"
-        by_name = {p.name: p for p in self._site[self.facility]['enabled']}
+        by_name = self._get_enabled_by_name()
 
         if 'order' in kwargs:
             order = kwargs['order'].split(',')
@@ -214,7 +221,8 @@ class AdminPlugins(AdminPage):
         enabled = []
         if self.order:
             for plugin in plugins['enabled']:
-                enabled.append(plugin.name)
+                if plugin.is_enabled:
+                    enabled.append(plugin.name)
         else:
             for _, plugin in plugins['available'].iteritems():
                 if plugin.is_enabled:
diff --git a/ipsilon/info/common.py b/ipsilon/info/common.py
index 62f80ef..03de66a 100755
--- a/ipsilon/info/common.py
+++ b/ipsilon/info/common.py
@@ -14,36 +14,34 @@ class InfoProviderBase(PluginObject, Log):
     def __init__(self):
         super(InfoProviderBase, self).__init__()
         self._site = None
+        self.is_enabled = False
 
     def get_user_attrs(self, user):
         raise NotImplementedError
 
-    @property
-    def is_enabled(self):
-        if self._site:
-            return self in self._site[FACILITY]['enabled']
-        return False
-
     def enable(self, site):
-        self._site = site
-        plugins = site[FACILITY]
-        if self in plugins['enabled']:
+        if self.is_enabled:
             return
 
+        if not self._site:
+            self._site = site
+        plugins = self._site[FACILITY]
+
         # configure self
         if self.name in plugins['config']:
             self.set_config(plugins['config'][self.name])
 
         plugins['enabled'].append(self)
+        self.is_enabled = True
         self.debug('Info plugin enabled: %s' % self.name)
 
     def disable(self, site):
-        self._site = site
-        plugins = site[FACILITY]
-        if self not in plugins['enabled']:
+        if not self.is_enabled:
             return
 
+        plugins = self._site[FACILITY]
         plugins['enabled'].remove(self)
+        self.is_enabled = False
         self.debug('Info plugin disabled: %s' % self.name)
 
 
diff --git a/ipsilon/login/common.py b/ipsilon/login/common.py
index 73422ae..028b754 100755
--- a/ipsilon/login/common.py
+++ b/ipsilon/login/common.py
@@ -17,7 +17,6 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-from ipsilon.util.log import Log
 from ipsilon.util.page import Page
 from ipsilon.util.user import UserSession
 from ipsilon.util.plugin import PluginLoader, PluginObject
@@ -30,7 +29,7 @@ import cherrypy
 USERNAME_COOKIE = 'ipsilon_default_username'
 
 
-class LoginManagerBase(PluginObject, Log):
+class LoginManagerBase(PluginObject):
 
     def __init__(self):
         super(LoginManagerBase, self).__init__()
@@ -38,6 +37,7 @@ class LoginManagerBase(PluginObject, Log):
         self.path = '/'
         self.next_login = None
         self.info = None
+        self.is_enabled = False
 
     def redirect_to_path(self, path):
         base = cherrypy.config.get('base.mount', "")
@@ -116,18 +116,14 @@ class LoginManagerBase(PluginObject, Log):
     def get_tree(self, site):
         raise NotImplementedError
 
-    @property
-    def is_enabled(self):
-        if self._site:
-            return self in self._site[FACILITY]['enabled']
-        return False
-
     def enable(self, site):
-        self._site = site
-        plugins = site[FACILITY]
-        if self in plugins['enabled']:
+        if self.is_enabled:
             return
 
+        if not self._site:
+            self._site = site
+        plugins = self._site[FACILITY]
+
         # configure self
         if self.name in plugins['config']:
             self.set_config(plugins['config'][self.name])
@@ -149,17 +145,18 @@ class LoginManagerBase(PluginObject, Log):
             root.first_login = self
 
         plugins['enabled'].append(self)
+        self.is_enabled = True
         self._debug('Login plugin enabled: %s' % self.name)
 
         # Get handle of the info plugin
         self.info = root.info
 
     def disable(self, site):
-        self._site = site
-        plugins = site[FACILITY]
-        if self not in plugins['enabled']:
+        if not self.is_enabled:
             return
 
+        plugins = self._site[FACILITY]
+
         # remove self from chain
         root = plugins['root']
         if root.first_login == self:
@@ -173,6 +170,7 @@ class LoginManagerBase(PluginObject, Log):
         self.next_login = None
 
         plugins['enabled'].remove(self)
+        self.is_enabled = False
         self._debug('Login plugin disabled: %s' % self.name)
 
 
diff --git a/ipsilon/providers/common.py b/ipsilon/providers/common.py
index 6d454b5..d882b40 100755
--- a/ipsilon/providers/common.py
+++ b/ipsilon/providers/common.py
@@ -56,6 +56,7 @@ class ProviderBase(PluginObject):
         self.name = name
         self.path = path
         self.tree = None
+        self.is_enabled = False
 
     def on_enable(self):
         # this one does nothing
@@ -81,31 +82,24 @@ class ProviderBase(PluginObject):
         self._debug('IdP Provider registered: %s' % self.name)
 
         if self.get_config_value('enabled') == '1':
-            # and add self to the root
-            root = site[FACILITY]['root']
-            root.add_subtree(self.name, self.tree)
-            self._debug('IdP Provider enabled: %s' % self.name)
+            # and enable self
+            self._enable(site)
 
-    @property
-    def is_enabled(self):
-        if self.get_config_value('enabled') == '1':
-            return True
-        return False
+    def _enable(self, site):
+        root = site[FACILITY]['root']
+        root.add_subtree(self.name, self.tree)
+        self._debug('IdP Provider enabled: %s' % self.name)
+        self.is_enabled = True
+        self.on_enable()
 
     def enable(self, site):
         if self.is_enabled:
             return
 
-        # and add self to the root
-        root = site[FACILITY]['root']
-        root.add_subtree(self.name, self.tree)
-
+        self._enable(site)
         self.set_config_value('enabled', '1')
         self.save_plugin_config(FACILITY)
 
-        self.on_enable()
-        self._debug('IdP Provider enabled: %s' % self.name)
-
     def disable(self, site):
         if not self.is_enabled:
             return
@@ -114,6 +108,7 @@ class ProviderBase(PluginObject):
         root = site[FACILITY]['root']
         root.del_subtree(self.name)
 
+        self.is_enabled = False
         self.set_config_value('enabled', '0')
         self.save_plugin_config(FACILITY)
         self._debug('IdP Provider disabled: %s' % self.name)