- some of the refinements from the sa_synonyms branch which will

allow the hybrid extension to work, but doesn't re-implement synonym,
comparable_property,concreteinheritedproperty
- mapper.get_property() and _entity_descriptor use plain getattr()
to get at descriptors in all cases, thereby placing more
trust in the ultimate class-bound attribute to provide mapped
properties

5 files changed, 119 insertions, 142 deletions

diff --git a/lib/sqlalchemy/orm/interfaces.py b/lib/sqlalchemy/orm/interfaces.py
index 91c2ae403..9874e644f 100644
--- a/lib/sqlalchemy/orm/interfaces.py
+++ b/lib/sqlalchemy/orm/interfaces.py
@@ -474,7 +474,7 @@ class MapperProperty(object):
 
         return iter(())
 
-    def set_parent(self, parent):
+    def set_parent(self, parent, init):
         self.parent = parent
 
     def instrument_class(self, mapper):
diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 9cb358151..74853dbba 100644
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -57,8 +57,6 @@ _COMPILE_MUTEX = util.threading.RLock()
 
 # initialize these lazily
 ColumnProperty = None
-SynonymProperty = None
-ComparableProperty = None
 RelationshipProperty = None
 ConcreteInheritedProperty = None
 _expire_state = None
@@ -709,35 +707,11 @@ class Mapper(object):
                 for col in col.proxy_set:
                     self._columntoproperty[col] = prop
 
-        elif isinstance(prop, (ComparableProperty, SynonymProperty)) and \
-                            setparent:
-            if prop.descriptor is None:
-                desc = getattr(self.class_, key, None)
-                if self._is_userland_descriptor(desc):
-                    prop.descriptor = desc
-            if getattr(prop, 'map_column', False):
-                if key not in self.mapped_table.c:
-                    raise sa_exc.ArgumentError(
-                        "Can't compile synonym '%s': no column on table "
-                        "'%s' named '%s'" 
-                         % (prop.name, self.mapped_table.description, key))
-                elif self.mapped_table.c[key] in self._columntoproperty and \
-                        self._columntoproperty[
-                                                self.mapped_table.c[key]
-                                            ].key == prop.name:
-                    raise sa_exc.ArgumentError(
-                        "Can't call map_column=True for synonym %r=%r, "
-                        "a ColumnProperty already exists keyed to the name "
-                        "%r for column %r" % 
-                        (key, prop.name, prop.name, key)
-                    )
-                p = ColumnProperty(self.mapped_table.c[key])
-                self._configure_property(
-                                        prop.name, p, 
-                                        init=init, 
-                                        setparent=setparent)
-                p._mapped_by_synonym = key
-        
+        prop.key = key
+
+        if setparent:
+            prop.set_parent(self, init)
+
         if key in self._props and \
                 getattr(self._props[key], '_mapped_by_synonym', False):
             syn = self._props[key]._mapped_by_synonym
@@ -748,10 +722,6 @@ class Mapper(object):
                     )
             
         self._props[key] = prop
-        prop.key = key
-
-        if setparent:
-            prop.set_parent(self)
 
         if not self.non_primary:
             prop.instrument_class(self)
@@ -905,26 +875,36 @@ class Mapper(object):
     def has_property(self, key):
         return key in self._props
 
-    def get_property(self, key, resolve_synonyms=False, raiseerr=True):
-        """return a MapperProperty associated with the given key."""
+    def get_property(self, key, 
+                            resolve_synonyms=False, 
+                            raiseerr=True, _compile_mappers=True):
+                            
+        """return a :class:`.MapperProperty` associated with the given key.
+        
+        resolve_synonyms=False and raiseerr=False are deprecated.
+        
+        """
 
-        if not self.compiled:
+        if _compile_mappers and not self.compiled:
             self.compile()
-        return self._get_property(key, 
-                            resolve_synonyms=resolve_synonyms,
-                            raiseerr=raiseerr)
-
-    def _get_property(self, key, resolve_synonyms=False, raiseerr=True):
-        prop = self._props.get(key, None)
-        if resolve_synonyms:
-            while isinstance(prop, SynonymProperty):
-                prop = self._props.get(prop.name, None)
-        if prop is None and raiseerr:
-            raise sa_exc.InvalidRequestError(
-                    "Mapper '%s' has no property '%s'" % 
-                    (self, key))
-        return prop
-    
+        
+        if not resolve_synonyms:
+            prop = self._props.get(key, None)
+            if prop is None and raiseerr:
+                raise sa_exc.InvalidRequestError(
+                        "Mapper '%s' has no property '%s'" % 
+                        (self, key))
+            return prop
+        else:
+            try:
+                return getattr(self.class_, key).property
+            except AttributeError:
+                if raiseerr:
+                    raise sa_exc.InvalidRequestError(
+                        "Mapper '%s' has no property '%s'" % (self, key))
+                else:
+                    return None
+
     @property
     def iterate_properties(self):
         """return an iterator of all MapperProperty objects."""
diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py
index cf5f31162..cbfba91f3 100644
--- a/lib/sqlalchemy/orm/properties.py
+++ b/lib/sqlalchemy/orm/properties.py
@@ -237,7 +237,22 @@ class CompositeProperty(ColumnProperty):
     def __str__(self):
         return str(self.parent.class_.__name__) + "." + self.key
 
-class ConcreteInheritedProperty(MapperProperty):
+class DescriptorProperty(MapperProperty):
+    """:class:`MapperProperty` which proxies access to a 
+        plain descriptor."""
+
+    def setup(self, context, entity, path, adapter, **kwargs):
+        pass
+
+    def create_row_processor(self, selectcontext, path, mapper, row, adapter):
+        return (None, None)
+
+    def merge(self, session, source_state, source_dict, 
+                dest_state, dest_dict, load, _recursive):
+        pass
+
+
+class ConcreteInheritedProperty(DescriptorProperty):
     """A 'do nothing' :class:`MapperProperty` that disables 
     an attribute on a concrete subclass that is only present
     on the inherited mapper, not the concrete classes' mapper.
@@ -254,18 +269,6 @@ class ConcreteInheritedProperty(MapperProperty):
     
     """
     
-    extension = None
-
-    def setup(self, context, entity, path, adapter, **kwargs):
-        pass
-
-    def create_row_processor(self, selectcontext, path, mapper, row, adapter):
-        return (None, None)
-
-    def merge(self, session, source_state, source_dict, dest_state,
-                dest_dict, load, _recursive):
-        pass
-        
     def instrument_class(self, mapper):
         def warn():
             raise AttributeError("Concrete %s does not implement "
@@ -284,7 +287,7 @@ class ConcreteInheritedProperty(MapperProperty):
         comparator_callable = None
         # TODO: put this process into a deferred callable?
         for m in self.parent.iterate_to_root():
-            p = m._get_property(self.key)
+            p = m.get_property(self.key, _compile_mappers=False)
             if not isinstance(p, ConcreteInheritedProperty):
                 comparator_callable = p.comparator_factory
                 break
@@ -299,9 +302,7 @@ class ConcreteInheritedProperty(MapperProperty):
             )
 
 
-class SynonymProperty(MapperProperty):
-
-    extension = None
+class SynonymProperty(DescriptorProperty):
 
     def __init__(self, name, map_column=None, 
                             descriptor=None, comparator_factory=None,
@@ -313,14 +314,40 @@ class SynonymProperty(MapperProperty):
         self.doc = doc or (descriptor and descriptor.__doc__) or None
         util.set_creation_order(self)
 
-    def setup(self, context, entity, path, adapter, **kwargs):
-        pass
+    def set_parent(self, parent, init):
+        if self.map_column:
+            # implement the 'map_column' option.
+            if self.key not in parent.mapped_table.c:
+                raise sa_exc.ArgumentError(
+                    "Can't compile synonym '%s': no column on table "
+                    "'%s' named '%s'" 
+                     % (self.name, parent.mapped_table.description, self.key))
+            elif parent.mapped_table.c[self.key] in \
+                    parent._columntoproperty and \
+                    parent._columntoproperty[
+                                            parent.mapped_table.c[self.key]
+                                        ].key == self.name:
+                raise sa_exc.ArgumentError(
+                    "Can't call map_column=True for synonym %r=%r, "
+                    "a ColumnProperty already exists keyed to the name "
+                    "%r for column %r" % 
+                    (self.key, self.name, self.name, self.key)
+                )
+            p = ColumnProperty(parent.mapped_table.c[self.key])
+            parent._configure_property(
+                                    self.name, p, 
+                                    init=init, 
+                                    setparent=True)
+            p._mapped_by_synonym = self.key
 
-    def create_row_processor(self, selectcontext, path, mapper, row, adapter):
-        return (None, None)
+        self.parent = parent
 
     def instrument_class(self, mapper):
-        class_ = self.parent.class_
+
+        if self.descriptor is None:
+            desc = getattr(mapper.class_, self.key, None)
+            if mapper._is_userland_descriptor(desc):
+                self.descriptor = desc
 
         if self.descriptor is None:
             class SynonymProp(object):
@@ -337,8 +364,9 @@ class SynonymProperty(MapperProperty):
 
         def comparator_callable(prop, mapper):
             def comparator():
-                prop = self.parent._get_property(
-                                        self.key, resolve_synonyms=True)
+                prop = mapper.get_property(
+                                        self.name, resolve_synonyms=True, 
+                                        _compile_mappers=False)
                 if self.comparator_factory:
                     return self.comparator_factory(prop, mapper)
                 else:
@@ -355,17 +383,10 @@ class SynonymProperty(MapperProperty):
             doc=self.doc
             )
 
-    def merge(self, session, source_state, source_dict, dest_state,
-                dest_dict, load, _recursive):
-        pass
         
-log.class_logger(SynonymProperty)
-
-class ComparableProperty(MapperProperty):
+class ComparableProperty(DescriptorProperty):
     """Instruments a Python property for use in query expressions."""
 
-    extension = None
-    
     def __init__(self, comparator_factory, descriptor=None, doc=None):
         self.descriptor = descriptor
         self.comparator_factory = comparator_factory
@@ -374,6 +395,11 @@ class ComparableProperty(MapperProperty):
 
     def instrument_class(self, mapper):
         """Set up a proxy to the unmanaged descriptor."""
+        
+        if self.descriptor is None:
+            desc = getattr(mapper.class_, self.key, None)
+            if mapper._is_userland_descriptor(desc):
+                self.descriptor = desc
 
         attributes.register_descriptor(
             mapper.class_, 
@@ -385,16 +411,6 @@ class ComparableProperty(MapperProperty):
             doc=self.doc,
             )
 
-    def setup(self, context, entity, path, adapter, **kwargs):
-        pass
-
-    def create_row_processor(self, selectcontext, path, mapper, row, adapter):
-        return (None, None)
-
-    def merge(self, session, source_state, source_dict, 
-                dest_state, dest_dict, load, _recursive):
-        pass
-
 
 class RelationshipProperty(StrategizedProperty):
     """Describes an object property that holds a single item or list
@@ -837,7 +853,7 @@ class RelationshipProperty(StrategizedProperty):
                     yield (c, instance_mapper, instance_state)
 
     def _add_reverse_property(self, key):
-        other = self.mapper._get_property(key)
+        other = self.mapper.get_property(key, _compile_mappers=False)
         self._reverse_property.add(other)
         other._reverse_property.add(self)
         
@@ -924,8 +940,7 @@ class RelationshipProperty(StrategizedProperty):
         if not self.parent.concrete:
             for inheriting in self.parent.iterate_to_root():
                 if inheriting is not self.parent \
-                    and inheriting._get_property(self.key,
-                        raiseerr=False):
+                    and inheriting.has_property(self.key):
                     util.warn("Warning: relationship '%s' on mapper "
                               "'%s' supercedes the same relationship "
                               "on inherited mapper '%s'; this can "
@@ -1216,7 +1231,7 @@ class RelationshipProperty(StrategizedProperty):
     def _assert_is_primary(self):
         if not self.is_primary() \
             and not mapper.class_mapper(self.parent.class_,
-                compile=False)._get_property(self.key, raiseerr=False):
+                compile=False).has_property(self.key):
             raise sa_exc.ArgumentError("Attempting to assign a new "
                     "relationship '%s' to a non-primary mapper on "
                     "class '%s'.  New relationships can only be added "
@@ -1234,8 +1249,7 @@ class RelationshipProperty(StrategizedProperty):
             else:
                 backref_key, kwargs = self.backref
             mapper = self.mapper.primary_mapper()
-            if mapper._get_property(backref_key, raiseerr=False) \
-                is not None:
+            if mapper.has_property(backref_key):
                 raise sa_exc.ArgumentError("Error creating backref "
                         "'%s' on relationship '%s': property of that "
                         "name exists on mapper '%s'" % (backref_key,
@@ -1416,7 +1430,5 @@ PropertyLoader = RelationProperty = RelationshipProperty
 log.class_logger(RelationshipProperty)
 
 mapper.ColumnProperty = ColumnProperty
-mapper.SynonymProperty = SynonymProperty
-mapper.ComparableProperty = ComparableProperty
 mapper.RelationshipProperty = RelationshipProperty
 mapper.ConcreteInheritedProperty = ConcreteInheritedProperty
diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py
index cc6d15a74..c269c8bb9 100644
--- a/lib/sqlalchemy/orm/query.py
+++ b/lib/sqlalchemy/orm/query.py
@@ -864,7 +864,7 @@ class Query(object):
         """apply the given filtering criterion to the query and return 
         the newly resulting ``Query``."""
 
-        clauses = [_entity_descriptor(self._joinpoint_zero(), key)[0] == value
+        clauses = [_entity_descriptor(self._joinpoint_zero(), key) == value
             for key, value in kwargs.iteritems()]
 
         return self.filter(sql.and_(*clauses))
@@ -1158,7 +1158,7 @@ class Query(object):
             if isinstance(onclause, basestring):
                 left_entity = self._joinpoint_zero()
 
-                descriptor, prop = _entity_descriptor(left_entity, onclause)
+                descriptor = _entity_descriptor(left_entity, onclause)
                 onclause = descriptor
             
             # check for q.join(Class.propname, from_joinpoint=True)
@@ -1171,7 +1171,7 @@ class Query(object):
                                     _entity_info(self._joinpoint_zero())
                 if left_mapper is left_entity:
                     left_entity = self._joinpoint_zero()
-                    descriptor, prop = _entity_descriptor(left_entity,
+                    descriptor = _entity_descriptor(left_entity,
                                                             onclause.key)
                     onclause = descriptor
 
diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py
index ef5413724..0f4adec00 100644
--- a/lib/sqlalchemy/orm/util.py
+++ b/lib/sqlalchemy/orm/util.py
@@ -347,9 +347,12 @@ class AliasedClass(object):
         return queryattr
 
     def __getattr__(self, key):
-        prop = self.__mapper._get_property(key, raiseerr=False)
-        if prop:
-            return self.__adapt_prop(prop)
+        if self.__mapper.has_property(key):
+            return self.__adapt_prop(
+                        self.__mapper.get_property(
+                            key, _compile_mappers=False
+                        )
+                    )
 
         for base in self.__target.__mro__:
             try:
@@ -537,40 +540,22 @@ def _entity_info(entity, compile=True):
     return mapper, mapper._with_polymorphic_selectable, False
 
 def _entity_descriptor(entity, key):
-    """Return attribute/property information given an entity and string name.
-
-    Returns a 2-tuple representing InstrumentedAttribute/MapperProperty.
+    """Return a class attribute given an entity and string name.
+    
+    May return :class:`.InstrumentedAttribute` or user-defined
+    attribute.
 
     """
-    if isinstance(entity, AliasedClass):
-        try:
-            desc = getattr(entity, key)
-            return desc, desc.property
-        except AttributeError:
-            raise sa_exc.InvalidRequestError(
-                        "Entity '%s' has no property '%s'" % 
-                        (entity, key)
-                    )
-            
-    elif isinstance(entity, type):
-        try:
-            desc = attributes.manager_of_class(entity)[key]
-            return desc, desc.property
-        except KeyError:
-            raise sa_exc.InvalidRequestError(
-                        "Entity '%s' has no property '%s'" % 
-                        (entity, key)
-                    )
-            
-    else:
-        try:
-            desc = entity.class_manager[key]
-            return desc, desc.property
-        except KeyError:
-            raise sa_exc.InvalidRequestError(
-                        "Entity '%s' has no property '%s'" % 
-                        (entity, key)
-                    )
+    if not isinstance(entity, (AliasedClass, type)):
+        entity = entity.class_
+    
+    try:
+        return getattr(entity, key)
+    except AttributeError:
+        raise sa_exc.InvalidRequestError(
+                    "Entity '%s' has no property '%s'" % 
+                    (entity, key)
+                )
 
 def _orm_columns(entity):
     mapper, selectable, is_aliased_class = _entity_info(entity)