- Register a ``repoze.bfg.resource.OverrideProvider`` as a

  pkg_resources provider only for modules which are known to have
  overrides, instead of globally, when a <resource> directive is used
  (performance).

5 files changed, 101 insertions, 47 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index cae21995d..7ff778ec4 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,7 +1,10 @@
 Next release
 ============
 
-- ...
+- Register a ``repoze.bfg.resource.OverrideProvider`` as a
+  pkg_resources provider only for modules which are known to have
+  overrides, instead of globally, when a <resource> directive is used
+  (performance).
 
 1.0a6 (2009-06-29)
 ==================
diff --git a/repoze/bfg/resource.py b/repoze/bfg/resource.py
index d11993e5b..4112925b4 100644
--- a/repoze/bfg/resource.py
+++ b/repoze/bfg/resource.py
@@ -48,9 +48,27 @@ class OverrideProvider(pkg_resources.DefaultProvider):
 
 class PackageOverrides:
     implements(IPackageOverrides)
-    def __init__(self, overridden_package):
+    # pkg_resources arg in kw args below for testing
+    def __init__(self, package, pkg_resources=pkg_resources):
+        if hasattr(package, '__loader__'):
+            raise TypeError('Package %s already has a __loader__ '
+                            '(probably a module in a zipped egg)' % package)
+        # We register ourselves as a __loader__ *only* to support the
+        # setuptools _find_adapter adapter lookup; this class doesn't
+        # actually support the PEP 302 loader "API".  This is
+        # excusable due to the following statement in the spec:
+        # ... Loader objects are not
+        # required to offer any useful functionality (any such functionality,
+        # such as the zipimport get_data() method mentioned above, is
+        # optional)...
+        # A __loader__ attribute is basically metadata, and setuptools
+        # uses it as such.
+        package.__loader__ = self 
+        # we call register_loader_type for every instantiation of this
+        # class; that's OK, it's idempotent to do it more than once.
+        pkg_resources.register_loader_type(self.__class__, OverrideProvider)
         self.overrides = []
-        self.overridden_package = overridden_package
+        self.overridden_package_name = package.__name__
 
     def insert(self, path, package, prefix):
         if not path or path.endswith('/'):
diff --git a/repoze/bfg/tests/test_resource.py b/repoze/bfg/tests/test_resource.py
index 176a07049..9f466b002 100644
--- a/repoze/bfg/tests/test_resource.py
+++ b/repoze/bfg/tests/test_resource.py
@@ -117,13 +117,40 @@ class TestPackageOverrides(unittest.TestCase):
         from repoze.bfg.resource import PackageOverrides
         return PackageOverrides
 
-    def _makeOne(self, package):
+    def _makeOne(self, package, pkg_resources=None):
         klass = self._getTargetClass()
-        return klass(package)
+        if pkg_resources is None:
+            pkg_resources = DummyPkgResources()
+        return klass(package, pkg_resources=pkg_resources)
+
+    def test_ctor_package_already_has_loader(self):
+        package = DummyPackage('package')
+        package.__loader__ = True
+        self.assertRaises(TypeError, self._makeOne, package)
+
+    def test_ctor_sets_loader(self):
+        package = DummyPackage('package')
+        po = self._makeOne(package)
+        self.assertEqual(package.__loader__, po)
+
+    def test_ctor_registers_loader_type(self):
+        from repoze.bfg.resource import OverrideProvider
+        dummy_pkg_resources = DummyPkgResources()
+        package = DummyPackage('package')
+        po = self._makeOne(package, dummy_pkg_resources)
+        self.assertEqual(dummy_pkg_resources.registered, [(po.__class__,
+                         OverrideProvider)])
+
+    def test_ctor_sets_local_state(self):
+        package = DummyPackage('package')
+        po = self._makeOne(package)
+        self.assertEqual(po.overrides, [])
+        self.assertEqual(po.overridden_package_name, 'package')
 
     def test_insert_directory(self):
         from repoze.bfg.resource import DirectoryOverride
-        po = self._makeOne('package')
+        package = DummyPackage('package')
+        po = self._makeOne(package)
         po.overrides= [None]
         po.insert('foo/', 'package', 'bar/')
         self.assertEqual(len(po.overrides), 2)
@@ -132,7 +159,8 @@ class TestPackageOverrides(unittest.TestCase):
 
     def test_insert_file(self):
         from repoze.bfg.resource import FileOverride
-        po = self._makeOne('package')
+        package = DummyPackage('package')
+        po = self._makeOne(package)
         po.overrides= [None]
         po.insert('foo.pt', 'package', 'bar.pt')
         self.assertEqual(len(po.overrides), 2)
@@ -141,7 +169,8 @@ class TestPackageOverrides(unittest.TestCase):
 
     def test_search_path(self):
         overrides = [ DummyOverride(None), DummyOverride(('package', 'name'))]
-        po = self._makeOne('package')
+        package = DummyPackage('package')
+        po = self._makeOne(package)
         po.overrides= overrides
         self.assertEqual(list(po.search_path('whatever')),
                          [('package', 'name')])
@@ -150,7 +179,8 @@ class TestPackageOverrides(unittest.TestCase):
         import os
         overrides = [ DummyOverride(None), DummyOverride(
             ('repoze.bfg.tests', 'test_resource.py'))]
-        po = self._makeOne('package')
+        package = DummyPackage('package')
+        po = self._makeOne(package)
         po.overrides= overrides
         here = os.path.dirname(os.path.abspath(__file__))
         expected = os.path.join(here, 'test_resource.py')
@@ -160,7 +190,8 @@ class TestPackageOverrides(unittest.TestCase):
         import os
         overrides = [ DummyOverride(None), DummyOverride(
             ('repoze.bfg.tests', 'test_resource.py'))]
-        po = self._makeOne('package')
+        package = DummyPackage('package')
+        po = self._makeOne(package)
         po.overrides= overrides
         here = os.path.dirname(os.path.abspath(__file__))
         expected = open(os.path.join(here, 'test_resource.py')).read()
@@ -170,7 +201,8 @@ class TestPackageOverrides(unittest.TestCase):
         import os
         overrides = [ DummyOverride(None), DummyOverride(
             ('repoze.bfg.tests', 'test_resource.py'))]
-        po = self._makeOne('package')
+        package = DummyPackage('package')
+        po = self._makeOne(package)
         po.overrides= overrides
         here = os.path.dirname(os.path.abspath(__file__))
         expected = open(os.path.join(here, 'test_resource.py')).read()
@@ -216,15 +248,12 @@ class TestFileOverride(unittest.TestCase):
         result = o('notfound.pt')
         self.assertEqual(result, None)
         
-    
-
 class DummyOverride:
     def __init__(self, result):
         self.result = result
 
     def __call__(self, resource_name):
         return self.result
-    
 
 class DummyOverrides:
     def __init__(self, result):
@@ -235,3 +264,14 @@ class DummyOverrides:
 
     get_stream = get_string = get_filename
     
+class DummyPkgResources:
+    def __init__(self):
+        self.registered = []
+
+    def register_loader_type(self, typ, inst):
+        self.registered.append((typ, inst))
+        
+class DummyPackage:
+    def __init__(self, name):
+        self.__name__ = name
+    
diff --git a/repoze/bfg/tests/test_zcml.py b/repoze/bfg/tests/test_zcml.py
index cba460a0a..cb841013d 100644
--- a/repoze/bfg/tests/test_zcml.py
+++ b/repoze/bfg/tests/test_zcml.py
@@ -883,7 +883,7 @@ class TestResourceDirective(unittest.TestCase):
         self.assertEqual(action['callable'], _override)
         self.assertEqual(action['discriminator'], None)
         self.assertEqual(action['args'],
-                         ('IDummy', '', 'IDummy', ''))
+                         (IDummy, '', IDummy, ''))
 
     def test_with_colons(self):
         from repoze.bfg.zcml import _override
@@ -895,7 +895,7 @@ class TestResourceDirective(unittest.TestCase):
         self.assertEqual(action['callable'], _override)
         self.assertEqual(action['discriminator'], None)
         self.assertEqual(action['args'],
-                         ('IDummy', 'foo.pt', 'IDummy', 'foo.pt'))
+                         (IDummy, 'foo.pt', IDummy, 'foo.pt'))
 
     def test_override_module_with_directory(self):
         from repoze.bfg.zcml import _override
@@ -907,7 +907,7 @@ class TestResourceDirective(unittest.TestCase):
         self.assertEqual(action['callable'], _override)
         self.assertEqual(action['discriminator'], None)
         self.assertEqual(action['args'],
-                         ('IDummy', '', 'IDummy', 'foo/'))
+                         (IDummy, '', IDummy, 'foo/'))
 
     def test_override_directory_with_module(self):
         from repoze.bfg.zcml import _override
@@ -919,7 +919,7 @@ class TestResourceDirective(unittest.TestCase):
         self.assertEqual(action['callable'], _override)
         self.assertEqual(action['discriminator'], None)
         self.assertEqual(action['args'],
-                         ('IDummy', 'foo/', 'IDummy', ''))
+                         (IDummy, 'foo/', IDummy, ''))
 
     def test_override_module_with_module(self):
         from repoze.bfg.zcml import _override
@@ -931,7 +931,7 @@ class TestResourceDirective(unittest.TestCase):
         self.assertEqual(action['callable'], _override)
         self.assertEqual(action['discriminator'], None)
         self.assertEqual(action['args'],
-                         ('IDummy', '', 'IDummy', ''))
+                         (IDummy, '', IDummy, ''))
 
 class Test_OverrideFunction(unittest.TestCase):
     def setUp(self):
@@ -951,24 +951,22 @@ class Test_OverrideFunction(unittest.TestCase):
         sm.registerUtility(overrides, IPackageOverrides, name=package_name)
 
     def test_overrides_not_yet_registered(self):
-        from repoze.bfg.resource import OverrideProvider
         from zope.component import queryUtility
         from repoze.bfg.interfaces import IPackageOverrides
-        resources = DummyPackageResources()
-        self._callFUT('package', 'path', 'opackage', 'oprefix',
-                      PackageOverrides=DummyOverrides, pkg_resources=resources)
+        package = DummyPackage('package')
+        opackage = DummyPackage('opackage')
+        self._callFUT(package, 'path', opackage, 'oprefix',
+                      PackageOverrides=DummyOverrides)
         overrides = queryUtility(IPackageOverrides, name='package')
-        self.assertEqual(overrides.package, 'package')
+        self.assertEqual(overrides.package, package)
         self.assertEqual(overrides.inserted, [('path', 'opackage', 'oprefix')])
-        self.assertEqual(len(resources.registered), 1)
-        resource = resources.registered[0]
-        self.assertEqual(resource[0], type(None))
-        self.assertEqual(resource[1], OverrideProvider)
 
     def test_overrides_already_registered(self):
-        overrides = DummyOverrides('package')
+        package = DummyPackage('package')
+        opackage = DummyPackage('opackage')
+        overrides = DummyOverrides(package)
         self._registerOverrides(overrides, 'package')
-        self._callFUT('package', 'path', 'opackage', 'oprefix')
+        self._callFUT(package, 'path', opackage, 'oprefix')
         self.assertEqual(overrides.inserted, [('path', 'opackage', 'oprefix')])
 
 class TestZCMLConfigure(unittest.TestCase):
@@ -1374,10 +1372,7 @@ class DummyOverrides:
     def insert(self, path, package, prefix):
         self.inserted.append((path, package, prefix))
         
-class DummyPackageResources:
-    def __init__(self):
-        self.registered  = []
-
-    def register_loader_type(self, typ, provider):
-        self.registered.append((typ, provider))
+class DummyPackage:
+    def __init__(self, name):
+        self.__name__ = name
         
diff --git a/repoze/bfg/zcml.py b/repoze/bfg/zcml.py
index 1aedd88e4..cf9a3d880 100644
--- a/repoze/bfg/zcml.py
+++ b/repoze/bfg/zcml.py
@@ -175,18 +175,16 @@ class IResourceDirective(Interface):
         required=True)
 
 def _override(package, path, override_package, override_prefix,
-              PackageOverrides=PackageOverrides, pkg_resources=pkg_resources):
-    # PackageOverrides and pkg_resources kw args for tests
+              PackageOverrides=PackageOverrides):
+    # PackageOverrides kw arg for tests
     sm = getSiteManager()
-    override = queryUtility(IPackageOverrides, name=package)
+    package_name = package.__name__
+    override_package_name = override_package.__name__
+    override = queryUtility(IPackageOverrides, name=package_name)
     if override is None:
         override = PackageOverrides(package)
-        sm.registerUtility(override, IPackageOverrides, name=package)
-        # register_loader_type will be called too many times if there
-        # is more than one overridden package; that's OK, as our
-        # mutation is idempotent
-        pkg_resources.register_loader_type(type(None), OverrideProvider)
-    override.insert(path, override_package, override_prefix)
+        sm.registerUtility(override, IPackageOverrides, name=package_name)
+    override.insert(path, override_package_name, override_prefix)
 
 def resource(context, to_override, override_with):
     if to_override == override_with:
@@ -214,8 +212,8 @@ def resource(context, to_override, override_with):
                 'A file cannot be overridden with a directory (put a slash '
                 'at the end of to_override if necessary)')
 
-    package = context.resolve(package).__name__
-    override_package = context.resolve(package).__name__
+    package = context.resolve(package)
+    override_package = context.resolve(override_package)
 
     context.action(
         discriminator = None,