307: Split Plugin.finalize() into two steps 1) Plugin.set_api() and 2) Plugin.finalize(); updated unit tests

4 files changed, 29 insertions, 49 deletions

diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index 19eae5043..725833cd2 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -340,17 +340,10 @@ class Plugin(ReadOnly):
                 return False
         return True
 
-    def finalize(self, api):
+    def finalize(self):
         """
-        After all the plugins are instantiated, `API` calls this method,
-        passing itself as the only argument. This is where plugins should
-        check that other plugins they depend upon have actually been loaded.
-
-        :param api: An `API` instance.
         """
-        assert self.__api is None, 'finalize() can only be called once'
-        assert api is not None, 'finalize() argument cannot be None'
-        self.__api = api
+        lock(self)
 
     def set_api(self, api):
         """
@@ -730,7 +723,9 @@ class API(DictProxy):
             object.__setattr__(self, name, namespace)
 
         for plugin in instances.itervalues():
-            plugin.finalize(self)
-            lock(plugin)
+            plugin.set_api(self)
             assert plugin.api is self
+
+        for plugin in instances.itervalues():
+            plugin.finalize()
         object.__setattr__(self, '_API__finalized', True)
diff --git a/ipalib/public.py b/ipalib/public.py
index 78fa09834..c48507471 100644
--- a/ipalib/public.py
+++ b/ipalib/public.py
@@ -237,8 +237,7 @@ class Command(plugable.Plugin):
     options = None
     params = None
 
-    def finalize(self, api):
-        super(Command, self).finalize(api)
+    def finalize(self):
         self.args = plugable.NameSpace(self.__check_args(), sort=False)
         if len(self.args) == 0 or not self.args[-1].multivalue:
             self.max_args = len(self.args)
@@ -248,6 +247,7 @@ class Command(plugable.Plugin):
         self.params = plugable.NameSpace(
             tuple(self.args()) + tuple(self.options()), sort=False
         )
+        super(Command, self).finalize()
 
     def get_args(self):
         return self.takes_args
@@ -395,11 +395,12 @@ class Object(plugable.Plugin):
         return self.__Property
     Property = property(__get_Property)
 
-    def finalize(self, api):
-        super(Object, self).finalize(api)
+    def set_api(self, api):
+        super(Object, self).set_api(api)
         self.__Method = self.__create_namespace('Method')
         self.__Property = self.__create_namespace('Property')
 
+
     def __create_namespace(self, name):
         return plugable.NameSpace(self.__filter_members(name))
 
@@ -443,9 +444,9 @@ class Attribute(plugable.Plugin):
         return self.__obj
     obj = property(__get_obj)
 
-    def finalize(self, api):
-        super(Attribute, self).finalize(api)
+    def set_api(self, api):
         self.__obj = api.Object[self.obj_name]
+        super(Attribute, self).set_api(api)
 
 
 class Method(Attribute, Command):
diff --git a/ipalib/tests/test_plugable.py b/ipalib/tests/test_plugable.py
index 3972cfa93..6c796f9e8 100644
--- a/ipalib/tests/test_plugable.py
+++ b/ipalib/tests/test_plugable.py
@@ -405,26 +405,10 @@ class test_Plugin(ClassChecker):
         """
         Tests the `plugable.Plugin.finalize` method.
         """
-        api = 'the api instance'
         o = self.cls()
-        assert read_only(o, 'name') == 'Plugin'
-        assert repr(o) == '%s.Plugin()' % plugable.__name__
-        assert read_only(o, 'api') is None
-        raises(AssertionError, o.finalize, None)
-        o.finalize(api)
-        assert read_only(o, 'api') is api
-        raises(AssertionError, o.finalize, api)
-
-        class some_plugin(self.cls):
-            pass
-        sub = some_plugin()
-        assert read_only(sub, 'name') == 'some_plugin'
-        assert repr(sub) == '%s.some_plugin()' % __name__
-        assert read_only(sub, 'api') is None
-        raises(AssertionError, sub.finalize, None)
-        sub.finalize(api)
-        assert read_only(sub, 'api') is api
-        raises(AssertionError, sub.finalize, api)
+        assert not o.__islocked__()
+        o.finalize()
+        assert o.__islocked__()
 
 
 class test_PluginProxy(ClassChecker):
diff --git a/ipalib/tests/test_public.py b/ipalib/tests/test_public.py
index 865a18fd3..d963233bb 100644
--- a/ipalib/tests/test_public.py
+++ b/ipalib/tests/test_public.py
@@ -379,7 +379,7 @@ class test_Command(ClassChecker):
             takes_args = args
             takes_options = options
         o = example()
-        o.finalize(object)
+        o.finalize()
         return o
 
     def test_class(self):
@@ -412,7 +412,7 @@ class test_Command(ClassChecker):
         assert 'args' in self.cls.__public__ # Public
         assert self.cls().args is None
         o = self.cls()
-        o.finalize(object)
+        o.finalize()
         assert type(o.args) is plugable.NameSpace
         assert len(o.args) == 0
         args = ('destination', 'source?')
@@ -462,7 +462,7 @@ class test_Command(ClassChecker):
         assert 'options' in self.cls.__public__ # Public
         assert self.cls().options is None
         o = self.cls()
-        o.finalize(object)
+        o.finalize()
         assert type(o.options) is plugable.NameSpace
         assert len(o.options) == 0
         options = ('target', 'files*')
@@ -491,7 +491,7 @@ class test_Command(ClassChecker):
         expected = dict(kw)
         expected.update(dict(option0=u'option0', option1=u'option1'))
         o = self.subcls()
-        o.finalize(object)
+        o.finalize()
         for (key, value) in o.convert(**kw).iteritems():
             v = expected[key]
             assert value == v
@@ -509,7 +509,7 @@ class test_Command(ClassChecker):
         )
         norm = dict((k, v.lower()) for (k, v) in kw.items())
         sub = self.subcls()
-        sub.finalize(object)
+        sub.finalize()
         assert sub.normalize(**kw) == norm
 
     def test_get_default(self):
@@ -530,7 +530,7 @@ class test_Command(ClassChecker):
             option1='the default',
         )
         sub = self.subcls()
-        sub.finalize(object)
+        sub.finalize()
         assert sub.get_default(**no_fill) == {}
         assert sub.get_default(**fill) == default
 
@@ -541,7 +541,7 @@ class test_Command(ClassChecker):
         assert 'validate' in self.cls.__public__ # Public
 
         sub = self.subcls()
-        sub.finalize(object)
+        sub.finalize()
 
         # Check with valid args
         okay = dict(
@@ -639,9 +639,9 @@ class test_Object(ClassChecker):
         assert read_only(o, 'Method') is None
         assert read_only(o, 'Property') is None
 
-    def test_finalize(self):
+    def test_set_api(self):
         """
-        Tests the `public.Object.finalize` method.
+        Tests the `public.Object.set_api` method.
         """
         # Setup for test:
         class DummyAttribute(object):
@@ -685,7 +685,7 @@ class test_Object(ClassChecker):
 
         # Actually perform test:
         o = user()
-        o.finalize(api)
+        o.set_api(api)
         assert read_only(o, 'api') is api
         for name in ['Method', 'Property']:
             namespace = getattr(o, name)
@@ -725,9 +725,9 @@ class test_Attribute(ClassChecker):
         assert read_only(o, 'obj_name') == 'user'
         assert read_only(o, 'attr_name') == 'add'
 
-    def test_finalize(self):
+    def test_set_api(self):
         """
-        Tests the `public.Attribute.finalize` method.
+        Tests the `public.Attribute.set_api` method.
         """
         user_obj = 'The user public.Object instance'
         class api(object):
@@ -737,7 +737,7 @@ class test_Attribute(ClassChecker):
         o = user_add()
         assert read_only(o, 'api') is None
         assert read_only(o, 'obj') is None
-        o.finalize(api)
+        o.set_api(api)
         assert read_only(o, 'api') is api
         assert read_only(o, 'obj') is user_obj