From 9d6fcd1eeb030be9e0a24399493924cf5d963b28 Mon Sep 17 00:00:00 2001 From: Chris McDonough Date: Tue, 30 Jun 2009 13:10:44 +0000 Subject: - 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 directive is used (performance). --- CHANGES.txt | 5 +++- repoze/bfg/resource.py | 22 ++++++++++++-- repoze/bfg/tests/test_resource.py | 62 ++++++++++++++++++++++++++++++++------- repoze/bfg/tests/test_zcml.py | 39 +++++++++++------------- repoze/bfg/zcml.py | 20 ++++++------- 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 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, -- cgit v1.2.3