Merge 79e82e5 into e419401

CHANGES.rst

@@ -14,6 +14,11 @@
 
 - Fix tests with Persistent 4.4.3 and above.
 
+- Support zope.interface 4.7, which lets tagged values on interfaces
+  be inherited, when using ``<registerAutoPackageIO>`` on a module
+  that had multiple objects implementing a derived interface. See `issue 97
+  <https://github.com/NextThought/nti.externalization/issues/97>`_.
+
 1.0.0a13 (2018-09-20)
 =====================
 

src/nti/externalization/autopackage.py

@@ -13,6 +13,7 @@
 from zope import interface
 
 from zope.dottedname import resolve as dottedname
+from zope.interface.interface import Element
 from zope.mimetype.interfaces import IContentTypeAware
 
 from nti.schema.interfaces import find_most_derived_interface
@@ -50,6 +51,20 @@ class AutoPackageSearchingScopedInterfaceObjectIO(ModuleScopedInterfaceObjectIO)
     You can still customize the behaviour by providing the ``iobase`` argument.
     """
 
+    @staticmethod
+    def _ap_iface_queryTaggedValue(iface, name):
+        # zope.interface 4.7.0 caused tagged values to become
+        # inherited. If we happened to have two items that implement a
+        # derived interface in a module, then we could get duplicate
+        # registartions (issue #97). So we make sure to only look
+        # at exactly the class we're interested in to make sure we
+        # return the same set of registrations as we did under 4.6.0
+        # and before.
+        #
+        # _ap_find_potential_factories_in_module() solves a related problem
+        # when class aliases were being used.
+        return Element.queryTaggedValue(iface, name)
+
     @classmethod
     def _ap_compute_external_class_name_from_interface_and_instance(cls, unused_iface, impl):
         """
@@ -112,6 +127,26 @@ def _ap_enumerate_module_names(cls):
         """
         raise NotImplementedError()
 
+    @classmethod
+    def _ap_find_potential_factories_in_module(cls, module):
+        """
+        Given a module that we're supposed to examine, iterate over
+        the types that could be factories.
+
+        This includes only types defined in that module. Any given
+        type will only be returned once.
+        """
+        seen = set()
+        for v in vars(module).values():
+            # ignore imports and non-concrete classes
+            # NOTE: using issubclass to properly support metaclasses
+            if getattr(v, '__module__', None) != module.__name__ \
+               or not issubclass(type(v), type) \
+               or v in seen:
+                continue
+            seen.add(v)
+            yield v
+
     @classmethod
     def _ap_find_factories(cls, package_name):
         """
@@ -147,13 +182,8 @@ def _ap_find_factories(cls, package_name):
 
         for mod_name in cls._ap_enumerate_module_names():
             mod = dottedname.resolve(package_name + '.' + mod_name)
-            for _, v in mod.__dict__.items():
-                # ignore imports and non-concrete classes
-                # NOTE: using issubclass to properly support metaclasses
-                if getattr(v, '__module__', None) != mod.__name__ \
-                    or not issubclass(type(v), type):
-                    continue
-                cls._ap_handle_one_potential_factory_class(registry, package_name, v)
+            for potential_factory in cls._ap_find_potential_factories_in_module(mod):
+                cls._ap_handle_one_potential_factory_class(registry, package_name, potential_factory)
         return registry
 
     @classmethod
@@ -164,14 +194,16 @@ def _ap_handle_one_potential_factory_class(cls, namespace, package_name, impleme
         # identified by ``_ap_enumerate_externalizable_root_interfaces()`` in ``__class_init__``
 
         interfaces_implemented = list(interface.implementedBy(implementation_class))
-        check_ext = any(iface.queryTaggedValue('__external_class_name__')
+        check_ext = any(cls._ap_iface_queryTaggedValue(iface, '__external_class_name__')
                         for iface in interfaces_implemented)
         if not check_ext:
             return
 
-        most_derived = find_most_derived_interface(None, interface.Interface, interfaces_implemented)
+        most_derived = find_most_derived_interface(None, interface.Interface,
+                                                   interfaces_implemented)
         if (most_derived is not interface.Interface
-            and not most_derived.queryTaggedValue('__external_default_implementation__')):
+            and not cls._ap_iface_queryTaggedValue(most_derived,
+                                                   '__external_default_implementation__')):
             logger.log(TRACE,
                        "Autopackage setting %s as __external_default_implementation__ for %s "
                        "which is the most derived interface out of %r.",

src/nti/externalization/tests/test_zcml.py

@@ -166,17 +166,21 @@ def _ap_find_package_interface_module(cls):
         import sys
         return sys.modules[__name__]
 
+
 class TestAutoPackageZCML(PlacelessSetup,
                           RegistrationMixin,
                           unittest.TestCase):
-    SCAN_THIS_MODULE = """
+    SCAN_THIS_MODULE_TMPL = """
         <configure xmlns:ext="http://nextthought.com/ntp/ext">
            <include package="nti.externalization" file="meta.zcml" />
-           <ext:registerAutoPackageIO root_interfaces="%s.IExtRoot" modules="%s"
-              iobase="%s.IOBase"
+           <ext:registerAutoPackageIO root_interfaces="%(mod)s.%(root)s"
+              modules="%(mod)s"
+              iobase="%(mod)s.IOBase"
               register_legacy_search_module="yes" />
         </configure>
-        """ % (__name__, __name__, __name__)
+        """
+
+    SCAN_THIS_MODULE = SCAN_THIS_MODULE_TMPL % {'mod': __name__, 'root': IExtRoot.__name__}
 
     def test_scan_package_empty(self):
         from nti.externalization import internalization as INT
@@ -192,6 +196,56 @@ def test_scan_package_empty(self):
         assert_that(INT.LEGACY_FACTORY_SEARCH_MODULES,
                     is_empty())
 
+    def test_scan_package_inherited(self):
+        # Issue #97: When interface tags are inherited,
+        # we don't double register.
+        import sys
+        from zope.interface.interfaces import IInterface
+        from ..interfaces import IInternalObjectIOFinder
+
+        class IPublic(interface.Interface):
+            pass
+
+        class IPrivate(IPublic):
+            pass
+
+        @interface.implementer(IPublic)
+        class Public(object):
+            pass
+
+        @interface.implementer(IPrivate)
+        class Private(object):
+            pass
+
+
+        to_reg = (IPublic, IPrivate, Public, Private, )
+        for r in to_reg:
+            setattr(sys.modules[__name__], r.__name__, r)
+        sys.modules[__name__].Alias = Private # The alias triggers issue #97
+        try:
+            xmlconfig.string(self.SCAN_THIS_MODULE_TMPL % {
+                'mod': __name__,
+                'root': IPublic.__name__,
+            })
+            gsm = component.getGlobalSiteManager()
+
+            ifaces = list(gsm.getAllUtilitiesRegisteredFor(IInterface))
+            assert_that(set(ifaces), is_({
+                _ILegacySearchModuleFactory,
+                IInternalObjectIOFinder,
+                IPublic,
+                IMimeObjectFactory,
+            }))
+            mime_factories = list(gsm.getAllUtilitiesRegisteredFor(IMimeObjectFactory))
+            assert_that(mime_factories, has_length(1))
+            # The root interface was registered
+            adapters = list(gsm.registeredAdapters())
+            assert_that(adapters, has_length(1))
+            assert_that(adapters[0], has_property('required', (IPublic,)))
+        finally:
+            for r in to_reg:
+                delattr(sys.modules[__name__], r.__name__)
+            del sys.modules[__name__].Alias
 
     def test_scan_package_legacy_utility(self):
         @interface.implementer(IExtRoot)
@@ -204,7 +258,6 @@ class O(object):
         gsm = component.getGlobalSiteManager()
         # The interfaces IExtRoot and IInternalObjectIO were registered,
         # as well as an IMimeObjectFactory and that interface,
-        # two variants on ILegacySearchModuleFactory and its interface.
         assert_that(list(gsm.registeredUtilities()), has_length(7))
 
         factory = gsm.getUtility(IMimeObjectFactory, 'application/foo')