Use the global site manager as a cache for legacy search modules.

- Sort modules when building the cache, ensuring we have some
  determinism. (Although autoPackageIO doesn't set a __name__ yet.)
- Log when a conflict is discovered.
- Issue a warning at runtime when an old factory is used so we know if
  it's happening.

src/nti/externalization/internalization.py

@@ -46,10 +46,12 @@
 from nti.externalization.interfaces import StandardExternalFields
 
 # pylint: disable=protected-access,ungrouped-imports,too-many-branches
-# pylint: disable=redefined-outer-name
+# pylint: disable=redefined-outer-name,inherit-non-class
 
 logger = __import__('logging').getLogger(__name__)
 
+_EMPTY_DICT = {}
+
 #: .. deprecated:: 1.0
 #: This is legacy functionality, please do not access directly.
 #: The public interface is through :func:`register_legacy_search_module`
@@ -92,19 +94,62 @@ def register_legacy_search_module(module_name):
            "https://github.com/NextThought/nti.externalization/issues/35",
            cls=FutureWarning)
 
-_EMPTY_DICT = {}
 
+## Implementation of legacy search modules.
+
+# We go through the global component registry, using a local
+# interface. We treat the registry as a cache and we will only
+# look at any given module object one time. We can detect duplicates
+# in this fashion.
+
+class _ILegacySearchModuleFactory(interface.Interface):
+
+    def __call__(*args, **kwargs): # pylint:disable=no-method-argument,arguments-differ
+        """
+        Create and return the object.
+        """
 
-def _find_class_in_dict(className, mod_dict):
-    clazz = mod_dict.get(className)
-    if not clazz and className.lower() == className:
-        # case-insensitive search of loaded modules if it was lower case.
-        for k in mod_dict:
-            if k.lower() == className:
-                clazz = mod_dict[k]
-                break
-    return clazz if getattr(clazz, '__external_can_create__', False) else None
+def __register_legacy_if_not(gsm, name, factory):
+    registered = gsm.queryUtility(_ILegacySearchModuleFactory, name)
+    if registered is not None:
+        logger.info("Found duplicate registration for legacy search path."
+                    "Factory %r will be used for class name %r overriding %r",
+                    registered, name, factory)
+        return
 
+    gsm.registerUtility(factory, name=name, provided=_ILegacySearchModuleFactory)
+
+def _register_factories_from_module_dict(mod_dict):
+    gsm = component.getGlobalSiteManager()
+
+    for name, value in sorted(mod_dict.items()):
+        if callable(value) and getattr(value, '__external_can_create__', False):
+            # Found one. Register it if not already registered.
+            # For legacy reasons, we also register the lower-case version.
+            __register_legacy_if_not(gsm, name, value)
+            if name.lower() != name:
+                __register_legacy_if_not(gsm, name.lower(), value)
+
+def _register_factories_from_search_set():
+    search_modules = set(LEGACY_FACTORY_SEARCH_MODULES)
+    LEGACY_FACTORY_SEARCH_MODULES.clear()
+
+    # Make sure we have objects, not strs.
+    modules = []
+    for module in search_modules:
+        if not hasattr(module, '__dict__'):
+            # Let this throw ImportError, it's a programming bug
+            name = module
+            module = resolve(name)
+        modules.append(module)
+
+    # Sort them as best we can.
+    modules.sort(key=lambda s: getattr(s, '__name__', ''))
+    for module in modules:
+        _register_factories_from_module_dict(module.__dict__)
+
+# Explicit for use of the warnings module.
+__warningregistry__ = {}
 
 def _search_for_external_factory(typeName):
     """
@@ -117,31 +162,34 @@ def _search_for_external_factory(typeName):
     if not typeName:
         return None
 
-    search_set = LEGACY_FACTORY_SEARCH_MODULES
-    className = typeName[0:-1] if typeName.endswith('s') else typeName
-    result = None
+    # First, register anything that needs to be registered still.
+    # Note that there are potential race conditions here.
+    if LEGACY_FACTORY_SEARCH_MODULES:
+        _register_factories_from_search_set()
 
-    updates = None
-    for module in search_set:
-        # Support registering both names and actual module objects
-        if not hasattr(module, '__dict__'):
-            # Let this throw ImportError, it's a programming bug
-            name = module
-            module = resolve(module)
-            if updates is None:
-                updates = []
-            updates.append((name, module))
 
-        result = _find_class_in_dict(className, module.__dict__)
-        if result is not None:
-            break
+    # Now look for a factory, using both the given name and its
+    # lower case version.
+    className = typeName[0:-1] if typeName.endswith('s') else typeName
 
-    if updates:
-        for old_name, new_module in updates:
-            search_set.remove(old_name)
-            search_set.add(new_module)
+    gsm = component.getGlobalSiteManager()
+    factory = gsm.queryUtility(_ILegacySearchModuleFactory, name=className)
+    if factory is None and className.lower() != className:
+        factory = gsm.queryUtility(_ILegacySearchModuleFactory, name=className.lower())
+
+    if factory is not None:
+        # This will produce a new warnings cache entry for each new
+        # class type we get (because the whole string is part of the
+        # key). Since this is external input, that could theoretically
+        # be used to DDOS us. We keep an eye on that and don't let the cache
+        # get too big, but that does mean we could ultimately produce
+        # duplicate warnings.
+        warnings.warn("Using legacy class finder for %r" % (typeName),
+                      FutureWarning)
+        if len(__warningregistry__) > 1024:
+            __warningregistry__.clear() # pragma: no cover
 
-    return result
+    return factory
 
 
 @interface.implementer(IFactory)

src/nti/externalization/tests/test_internalization.py

@@ -6,7 +6,6 @@
 from __future__ import print_function
 
 # stdlib imports
-import sys
 import unittest
 
 import fudge
@@ -25,7 +24,9 @@
 from hamcrest import assert_that
 from hamcrest import calling
 from hamcrest import contains
+from hamcrest import contains_string
 from hamcrest import equal_to
+from hamcrest import greater_than_or_equal_to
 from hamcrest import has_entry
 from hamcrest import has_length
 from hamcrest import has_property
@@ -102,17 +103,54 @@ def test_search_for_factory_no_type(self):
         assert_that(INT.find_factory_for_class_name(''), is_(none()))
 
     def test_search_for_factory_updates_search_set(self):
-        INT.register_legacy_search_module(__name__)
-        assert_that(INT.find_factory_for_class_name('testfunctions'), is_(none()))
-
-        assert_that(sys.modules[__name__], is_in(INT.LEGACY_FACTORY_SEARCH_MODULES))
-        assert_that(__name__, is_not(is_in(INT.LEGACY_FACTORY_SEARCH_MODULES)))
-
-        TestFunctions.__external_can_create__ = True
-        try:
-            assert_that(INT.find_factory_for_class_name('testfunctions'), equal_to(TestFunctions))
-        finally:
-            del TestFunctions.__external_can_create__
+        from zope.deprecation import Suppressor
+        from zope.testing.loggingsupport import InstalledHandler
+
+        with Suppressor():
+            INT.register_legacy_search_module(__name__)
+            # The cache is initialized lazily
+            assert_that(__name__, is_in(INT.LEGACY_FACTORY_SEARCH_MODULES))
+
+            assert_that(INT.find_factory_for_class_name('testfunctions'), is_(none()))
+
+            # And we have been examined and removed
+            assert_that(__name__, is_not(is_in(INT.LEGACY_FACTORY_SEARCH_MODULES)))
+            assert_that(INT.LEGACY_FACTORY_SEARCH_MODULES, is_(set()))
+
+            # Now we're going to fiddle with our public classes and try again.
+            # This will force re-registration to occur. Note we do this before
+            # we make ourself public, so that we can assert it's lazy
+            INT.register_legacy_search_module(__name__)
+
+            TestFunctions.__external_can_create__ = True
+            handler = InstalledHandler(INT.__name__)
+            try:
+                assert_that(INT.find_factory_for_class_name('testfunctions'),
+                            equal_to(TestFunctions))
+
+                # Now lets register ourself again, to trigger the logged warnings.
+                assert_that(__name__, is_not(is_in(INT.LEGACY_FACTORY_SEARCH_MODULES)))
+                assert_that(INT.LEGACY_FACTORY_SEARCH_MODULES, is_(set()))
+
+                # Now we're going to fiddle with our public classes and try again.
+                # This will force re-registration to occur. Note we do this before
+                # we make ourself public, so that we can assert it's lazy
+                INT.register_legacy_search_module(__name__)
+
+                assert_that(INT.find_factory_for_class_name('testfunctions'),
+                            equal_to(TestFunctions))
+                # case doesn't matter
+                assert_that(INT.find_factory_for_class_name('TeStfUnctIons'),
+                            equal_to(TestFunctions))
+
+                assert_that(str(handler),
+                            contains_string("Found duplicate registration for legacy search path."))
+
+                assert_that(INT.__warningregistry__, has_length(greater_than_or_equal_to(2)))
+
+            finally:
+                del TestFunctions.__external_can_create__
+                handler.uninstall()
 
 
 class TestDefaultExternalizedObjectFactory(CleanUp,