From: Mike Bayer Date: Sun, 28 Oct 2012 18:15:56 +0000 (-0400) Subject: - store only MultipleClassMarkers inside of ModuleMarker, then X-Git-Tag: rel_0_8_0b1~8 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=63a9d80f18ad22071742c8aa282ccc94ad94677c;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - store only MultipleClassMarkers inside of ModuleMarker, then store ModuleMarkers for multiple roots, one for each token in a module path. this allows partial path resolution. - docs to this effect --- diff --git a/lib/sqlalchemy/ext/declarative/__init__.py b/lib/sqlalchemy/ext/declarative/__init__.py index b3cecf6b07..bf6e6786e5 100644 --- a/lib/sqlalchemy/ext/declarative/__init__.py +++ b/lib/sqlalchemy/ext/declarative/__init__.py @@ -146,7 +146,7 @@ expression functions like :func:`~sqlalchemy.sql.expression.desc` and primaryjoin="Address.user_id==User.id") For the case where more than one module contains a class of the same name, -string class names can also be specified as fully module-qualified paths +string class names can also be specified as module-qualified paths within any of these string expressions:: class User(Base): @@ -156,9 +156,21 @@ within any of these string expressions:: primaryjoin="myapp.model.address.Address.user_id==" "myapp.model.user.User.id") +The qualified path can be any partial path that removes ambiguity between +the names. For example, to disambiguate between +``myapp.model.address.Address`` and ``myapp.model.lookup.Address``, +we can specify ``address.Address`` or ``lookup.Address``:: + + class User(Base): + # .... + addresses = relationship("address.Address", + order_by="desc(address.Address.email)", + primaryjoin="address.Address.user_id==" + "User.id") + .. versionadded:: 0.8 - Fully module-qualified paths can be used when specifying string arguments - with Declarative. + module-qualified paths can be used when specifying string arguments + with Declarative, in order to specify specific modules. Two alternatives also exist to using string-based attributes. A lambda can also be used, which will be evaluated after all mappers have been diff --git a/lib/sqlalchemy/ext/declarative/clsregistry.py b/lib/sqlalchemy/ext/declarative/clsregistry.py index 08b487db35..47450c5b74 100644 --- a/lib/sqlalchemy/ext/declarative/clsregistry.py +++ b/lib/sqlalchemy/ext/declarative/clsregistry.py @@ -38,14 +38,28 @@ def add_class(classname, cls): cls._decl_class_registry[classname] = cls try: - module = cls._decl_class_registry['_sa_module_registry'] + root_module = cls._decl_class_registry['_sa_module_registry'] except KeyError: cls._decl_class_registry['_sa_module_registry'] = \ - module = _ModuleMarker('_sa_module_registry', None) - for token in cls.__module__.split("."): - module = module.get_module(token) + root_module = _ModuleMarker('_sa_module_registry', None) + + tokens = cls.__module__.split(".") + + # build up a tree like this: + # modulename: myapp.snacks.nuts + # + # myapp->snack->nuts->(classes) + # snack->nuts->(classes) + # nuts->(classes) + # + # this allows partial token paths to be used. + while tokens: + token = tokens.pop(0) + module = root_module.get_module(token) + for token in tokens: + module = module.get_module(token) + module.add_class(classname, cls) - module.add_class(classname, cls) class _MultipleClassMarker(object): """refers to multiple classes of the same name @@ -53,7 +67,8 @@ class _MultipleClassMarker(object): """ - def __init__(self, classes): + def __init__(self, classes, on_remove=None): + self.on_remove = on_remove self.contents = set([ weakref.ref(item, self._remove_item) for item in classes]) _registries.add(self) @@ -61,12 +76,14 @@ class _MultipleClassMarker(object): def __iter__(self): return (ref() for ref in self.contents) - def attempt_get(self, key): + def attempt_get(self, path, key): if len(self.contents) > 1: raise exc.InvalidRequestError( - "Multiple classes with the classname " - "%r are in the registry of this declarative " - "base. Please use a fully module-qualified path." % key) + "Multiple classes found for path \"%s\" " + "in the registry of this declarative " + "base. Please use a fully module-qualified path." % + (".".join(path + [key])) + ) else: ref = list(self.contents)[0] cls = ref() @@ -78,8 +95,20 @@ class _MultipleClassMarker(object): self.contents.remove(ref) if not self.contents: _registries.discard(self) + if self.on_remove: + self.on_remove() - def add_item(self, item, base): + def add_item(self, item): + modules = set([cls().__module__ for cls in self.contents]) + if item.__module__ in modules: + util.warn( + "This declarative base already contains a class with the " + "same class name and module name as %s.%s, and will " + "be replaced in the string-lookup table." % ( + item.__module__, + item.__name__ + ) + ) self.contents.add(weakref.ref(item, self._remove_item)) class _ModuleMarker(object): @@ -92,13 +121,17 @@ class _ModuleMarker(object): self.name = name self.contents = {} self.mod_ns = _ModNS(self) + if self.parent: + self.path = self.parent.path + [self.name] + else: + self.path = [] _registries.add(self) def __contains__(self, name): return name in self.contents def __getitem__(self, name): - return self.contents[name]() + return self.contents[name] def _remove_item(self, name): self.contents.pop(name, None) @@ -112,20 +145,20 @@ class _ModuleMarker(object): def get_module(self, name): if name not in self.contents: marker = _ModuleMarker(name, self) - self.contents[name] = lambda: marker + self.contents[name] = marker else: - marker = self.contents[name]() + marker = self.contents[name] return marker def add_class(self, name, cls): if name in self.contents: - util.warn( - "This declarative base already contains a class with the " - "same class name and module name as %r, and will be replaced " - "in the string-lookup table." % cls) + existing = self.contents[name] + existing.add_item(cls) + else: + existing = self.contents[name] = \ + _MultipleClassMarker([cls], + on_remove=lambda: self._remove_item(name)) - self.contents[name] = weakref.ref(cls, - lambda ref: self._remove_item(name)) class _ModNS(object): @@ -138,12 +171,12 @@ class _ModNS(object): except KeyError: pass else: - value = value() if value is not None: if isinstance(value, _ModuleMarker): return value.mod_ns else: - return value + assert isinstance(value, _MultipleClassMarker) + return value.attempt_get(self.__parent.path, key) raise AttributeError("Module %r has no mapped classes " "registered under the name %r" % (self.__parent.name, key)) @@ -179,7 +212,7 @@ class _GetTable(object): def _determine_container(key, value): if isinstance(value, _MultipleClassMarker): - value = value.attempt_get(key) + value = value.attempt_get([], key) return _GetColumns(value) def _resolver(cls, prop): diff --git a/test/ext/declarative/test_clsregistry.py b/test/ext/declarative/test_clsregistry.py index db4b01470b..1d09f158e0 100644 --- a/test/ext/declarative/test_clsregistry.py +++ b/test/ext/declarative/test_clsregistry.py @@ -10,7 +10,7 @@ class MockClass(object): self._decl_class_registry = base tokens = name.split(".") self.__module__ = ".".join(tokens[0:-1]) - self.name = tokens[-1] + self.name = self.__name__ = tokens[-1] self.metadata = MetaData() @@ -30,7 +30,9 @@ class ClsRegistryTest(fixtures.TestBase): assert_raises_message( exc.SAWarning, - "This declarative base already contains a class ", + "This declarative base already contains a class with the " + "same class name and module name as foo.bar.Foo, and " + "will be replaced in the string-lookup table.", clsregistry.add_class, "Foo", f2 ) @@ -47,6 +49,41 @@ class ClsRegistryTest(fixtures.TestBase): is_(resolver("foo.bar.Foo")(), f1) is_(resolver("foo.alt.Foo")(), f2) + def test_fragment_resolve(self): + base = weakref.WeakValueDictionary() + f1 = MockClass(base, "foo.bar.Foo") + f2 = MockClass(base, "foo.alt.Foo") + f3 = MockClass(base, "bat.alt.Hoho") + clsregistry.add_class("Foo", f1) + clsregistry.add_class("Foo", f2) + clsregistry.add_class("HoHo", f3) + resolver = clsregistry._resolver(f1, MockProp()) + + gc_collect() + + is_(resolver("bar.Foo")(), f1) + is_(resolver("alt.Foo")(), f2) + + def test_fragment_ambiguous(self): + base = weakref.WeakValueDictionary() + f1 = MockClass(base, "foo.bar.Foo") + f2 = MockClass(base, "foo.alt.Foo") + f3 = MockClass(base, "bat.alt.Foo") + clsregistry.add_class("Foo", f1) + clsregistry.add_class("Foo", f2) + clsregistry.add_class("Foo", f3) + resolver = clsregistry._resolver(f1, MockProp()) + + gc_collect() + + assert_raises_message( + exc.InvalidRequestError, + 'Multiple classes found for path "alt.Foo" in the registry ' + 'of this declarative base. Please use a fully ' + 'module-qualified path.', + resolver("alt.Foo") + ) + def test_resolve_dupe_by_name(self): base = weakref.WeakValueDictionary() f1 = MockClass(base, "foo.bar.Foo") @@ -60,9 +97,9 @@ class ClsRegistryTest(fixtures.TestBase): resolver = resolver("Foo") assert_raises_message( exc.InvalidRequestError, - "Multiple classes with the classname 'Foo' " - "are in the registry of this declarative " - "base. Please use a fully module-qualified path.", + 'Multiple classes found for path "Foo" in the ' + 'registry of this declarative base. Please use a ' + 'fully module-qualified path.', resolver ) @@ -93,7 +130,7 @@ class ClsRegistryTest(fixtures.TestBase): clsregistry.add_class("Foo", f1) clsregistry.add_class("Foo", f2) - eq_(len(clsregistry._registries), 5) + eq_(len(clsregistry._registries), 11) del f1 del f2 @@ -133,7 +170,7 @@ class ClsRegistryTest(fixtures.TestBase): mod_entry = reg['foo']['bar'] resolver = clsregistry._resolver(f1, MockProp()) resolver = resolver("foo") - mod_entry.contents.update({"Foo": lambda: None}) + del mod_entry.contents["Foo"] assert_raises_message( AttributeError, "Module 'bar' has no mapped classes registered "