- at long last have gotten the "proxy_property" keyword

arg of register_descriptor to not be needed.   synonym, comparable, concreteinherited
props now supply a descriptor directly in the class dict, whose
__get__(None, cls) supplies a QueryableAttribute.   The basic idea is that
the hybrid prop can be used for this.   Payoff here is arguable, except that
hybrid can be at the base of future synonym/comparable operations.

8 files changed, 190 insertions, 170 deletions

diff --git a/doc/build/mappers.rst b/doc/build/mappers.rst
index 97677aa08..98dd3997a 100644
--- a/doc/build/mappers.rst
+++ b/doc/build/mappers.rst
@@ -295,28 +295,39 @@ through the ``email`` descriptor and into the ``_email``
 mapped attribute, the class level ``EmailAddress.email``
 attribute does not have the usual expression semantics
 usable with :class:`.Query`. To provide
-these, we instead use the :func:`.synonym`
-function as follows::
+these, we instead use the :func:`.hybrid.property`
+decorator as follows::
 
-    mapper(EmailAddress, addresses_table, properties={
-        'email': synonym('_email', map_column=True)
-    })
+    from sqlalchemy.ext import hybrid
+
+    class EmailAddress(object):
+        
+        @hybrid.property
+        def email(self):
+            return self._email
+            
+        @email.setter
+        def email(self, email):
+            self._email = email
+    
+        @email.expression
+        def email(cls):
+            return cls._email
 
 The ``email`` attribute is now usable in the same way as any
 other mapped attribute, including filter expressions,
 get/set operations, etc.::
 
-    address = session.query(EmailAddress).filter(EmailAddress.email == 'some address').one()
+    address = session.query(EmailAddress).\\
+                filter(EmailAddress.email == 'some address').\\
+                one()
 
     address.email = 'some other address'
-    session.flush()
-
-    q = session.query(EmailAddress).filter_by(email='some other address')
+    session.commit()
 
-If the mapped class does not provide a property, the :func:`.synonym` construct will create a default getter/setter object automatically.
+    q = session.query(EmailAddress).\
+                filter_by(email='some other address')
 
-To use synonyms with :mod:`~sqlalchemy.ext.declarative`, see the section 
-:ref:`declarative_synonyms`.
 
 .. _custom_comparators:
 
diff --git a/lib/sqlalchemy/ext/hybrid.py b/lib/sqlalchemy/ext/hybrid.py
index 2432ec9d7..3134db12d 100644
--- a/lib/sqlalchemy/ext/hybrid.py
+++ b/lib/sqlalchemy/ext/hybrid.py
@@ -62,11 +62,12 @@ or as the class itself::
     ### TODO ADD EXAMPLES HERE AND STUFF THIS ISN'T FINISHED ###
     
 """
+from sqlalchemy import util
 
 class method(object):
     def __init__(self, func, expr=None):
         self.func = func
-        self.expr = expr or fund
+        self.expr = expr or func
         
     def __get__(self, instance, owner):
         if instance is None:
@@ -78,16 +79,17 @@ class method(object):
         self.expr = expr
         return self
 
-class property(object):
+class property_(object):
     def __init__(self, fget, fset=None, fdel=None, expr=None):
         self.fget = fget
         self.fset = fset
         self.fdel = fdel
         self.expr = expr or fget
-        
+        util.update_wrapper(self, fget)
+
     def __get__(self, instance, owner):
         if instance is None:
-            return self.expr(owner)
+            return util.update_wrapper(self.expr(owner), self)
         else:
             return self.fget(instance)
             
@@ -109,3 +111,4 @@ class property(object):
         self.expr = expr
         return self
     
+
diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index f91ff51f2..c21585cea 100644
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -176,23 +176,30 @@ def proxied_attribute_factory(descriptor):
     behavior and getattr() to the given descriptor.
     """
 
-    class Proxy(InstrumentedAttribute):
+    class Proxy(QueryableAttribute):
         """A combination of InsturmentedAttribute and a regular descriptor."""
 
-        def __init__(self, key, descriptor, comparator, parententity):
+        def __init__(self, key, descriptor, comparator, adapter=None):
             self.key = key
             # maintain ProxiedAttribute.user_prop compatability.
             self.descriptor = self.user_prop = descriptor
             self._comparator = comparator
-            self._parententity = parententity
             self.impl = _ProxyImpl(key)
-
+            self.adapter = adapter
+            
         @util.memoized_property
         def comparator(self):
             if util.callable(self._comparator):
                 self._comparator = self._comparator()
+            if self.adapter:
+                self._comparator = self._comparator.adapted(self.adapter)
             return self._comparator
-
+        
+        def adapted(self, adapter):
+            return self.__class__(self.key, self.descriptor,
+                                       self._comparator,
+                                       adapter)
+        
         def __get__(self, instance, owner):
             """Delegate __get__ to the original descriptor."""
             if instance is None:
@@ -216,7 +223,7 @@ def proxied_attribute_factory(descriptor):
                 return getattr(descriptor, attribute)
             except AttributeError:
                 try:
-                    return getattr(self._comparator, attribute)
+                    return getattr(self.comparator, attribute)
                 except AttributeError:
                     raise AttributeError(
                     'Neither %r object nor %r object has an attribute %r' % (
@@ -1462,6 +1469,7 @@ def register_descriptor(class_, key, proxy_property=None, comparator=None,
     manager = manager_of_class(class_)
 
     if proxy_property:
+        raise NotImplementedError()
         proxy_type = proxied_attribute_factory(proxy_property)
         descriptor = proxy_type(key, proxy_property, comparator, parententity)
     else:
diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 54674a7cc..4e7702698 100644
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -1074,10 +1074,16 @@ class Mapper(object):
         return result
 
     def _is_userland_descriptor(self, obj):
+#        return not isinstance(obj, 
+#                    (MapperProperty, attributes.InstrumentedAttribute)) and \
+#                    hasattr(obj, '__get__')
+                    
         return not isinstance(obj, 
                     (MapperProperty, attributes.InstrumentedAttribute)) and \
-                    hasattr(obj, '__get__')
-
+                    hasattr(obj, '__get__') and not \
+                     isinstance(obj.__get__(None, obj),
+                                    attributes.QueryableAttribute)
+        
     def _should_exclude(self, name, assigned_name, local):
         """determine whether a particular property should be implicitly
         present on the class.
diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py
index 9cc03833a..50d95b777 100644
--- a/lib/sqlalchemy/orm/properties.py
+++ b/lib/sqlalchemy/orm/properties.py
@@ -237,13 +237,87 @@ 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 
+        user-defined descriptor."""
+
+    def set_parent(self, parent, init):
+        if self.descriptor is None:
+            desc = getattr(parent.class_, self.key, None)
+            if parent._is_userland_descriptor(desc):
+                self.descriptor = desc
+        self.parent = parent
+    
+    def instrument_class(self, mapper):
+        class_ = self.parent.class_
+        
+        from sqlalchemy.ext import hybrid
+
+        # hackety hack hack
+        class _ProxyImpl(object):
+            accepts_scalar_loader = False
+            expire_missing = True
+
+            def __init__(self, key):
+                self.key = key
+
+        if self.descriptor is None:
+            def fset(obj, value):
+                setattr(obj, self.name, value)
+            def fdel(obj):
+                delattr(obj, self.name)
+            def fget(obj):
+                return getattr(obj, self.name)
+            fget.__doc__ = self.doc
+
+            descriptor = hybrid.property_(
+                fget=fget,
+                fset=fset,
+                fdel=fdel,
+            )
+        elif isinstance(self.descriptor, property):
+            descriptor = hybrid.property_(
+                fget=self.descriptor.fget,
+                fset=self.descriptor.fset,
+                fdel=self.descriptor.fdel,
+            )
+        else:
+            descriptor = hybrid.property_(
+                fget=self.descriptor.__get__,
+                fset=self.descriptor.__set__,
+                fdel=self.descriptor.__delete__,
+            )
+
+        proxy_attr = attributes.\
+                        proxied_attribute_factory(self.descriptor 
+                                                    or descriptor)\
+                    (self.key, self.descriptor or descriptor,
+                        lambda: self._comparator_factory(mapper))
+        def get_comparator(owner):
+            return proxy_attr
+        descriptor.expr = get_comparator
+        
+        descriptor.impl = _ProxyImpl(self.key)
+        mapper.class_manager.instrument_attribute(self.key, 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.
-    
+
     Cases where this occurs include:
-    
+
     * When the superclass mapper is mapped against a 
       "polymorphic union", which includes all attributes from 
       all subclasses.
@@ -251,22 +325,22 @@ class ConcreteInheritedProperty(MapperProperty):
       but not on the subclass mapper.  Concrete mappers require
       that relationship() is configured explicitly on each 
       subclass. 
-    
-    """
-    
-    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
+    extension = None
+    
+    def _comparator_factory(self, mapper):
+        comparator_callable = None
         
-    def instrument_class(self, mapper):
+        for m in self.parent.iterate_to_root():
+            p = m._props[self.key]
+            if not isinstance(p, ConcreteInheritedProperty):
+                comparator_callable = p.comparator_factory
+                break
+        return comparator_callable
+    
+    def __init__(self):
         def warn():
             raise AttributeError("Concrete %s does not implement "
                 "attribute %r at the instance level.  Add this "
@@ -279,27 +353,13 @@ class ConcreteInheritedProperty(MapperProperty):
             def __delete__(s, obj):
                 warn()
             def __get__(s, obj, owner):
+                if obj is None:
+                    return self.descriptor
                 warn()
-
-        comparator_callable = None
-        # TODO: put this process into a deferred callable?
-        for m in self.parent.iterate_to_root():
-            p = m._props[self.key]
-            if not isinstance(p, ConcreteInheritedProperty):
-                comparator_callable = p.comparator_factory
-                break
-
-        attributes.register_descriptor(
-            mapper.class_, 
-            self.key, 
-            comparator=comparator_callable(self, mapper), 
-            parententity=mapper,
-            property_=self,
-            proxy_property=NoninheritedConcreteProp()
-            )
-
-
-class SynonymProperty(MapperProperty):
+        self.descriptor = NoninheritedConcreteProp()
+        
+        
+class SynonymProperty(DescriptorProperty):
 
     extension = None
 
@@ -313,12 +373,16 @@ 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 _comparator_factory(self, mapper):
+        class_ = self.parent.class_
+        prop = getattr(class_, self.name).property
+
+        if self.comparator_factory:
+            comp = self.comparator_factory(prop, mapper)
+        else:
+            comp = prop.comparator_factory(prop, mapper)
+        return comp
 
-    def create_row_processor(self, selectcontext, path, mapper, row, adapter):
-        return (None, None)
-    
     def set_parent(self, parent, init):
         if self.descriptor is None:
             desc = getattr(parent.class_, self.key, None)
@@ -349,50 +413,8 @@ class SynonymProperty(MapperProperty):
             p._mapped_by_synonym = self.key
     
         self.parent = parent
-    
-    def instrument_class(self, mapper):
-        class_ = self.parent.class_
-
-        if self.descriptor is None:
-            class SynonymProp(object):
-                def __set__(s, obj, value):
-                    setattr(obj, self.name, value)
-                def __delete__(s, obj):
-                    delattr(obj, self.name)
-                def __get__(s, obj, owner):
-                    if obj is None:
-                        return s
-                    return getattr(obj, self.name)
-
-            self.descriptor = SynonymProp()
-
-        def comparator_callable(prop, mapper):
-            def comparator():
-                prop = getattr(self.parent.class_,
-                                        self.name).property
-                if self.comparator_factory:
-                    return self.comparator_factory(prop, mapper)
-                else:
-                    return prop.comparator_factory(prop, mapper)
-            return comparator
-
-        attributes.register_descriptor(
-            mapper.class_, 
-            self.key, 
-            comparator=comparator_callable(self, mapper), 
-            parententity=mapper,
-            property_=self,
-            proxy_property=self.descriptor,
-            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
@@ -410,28 +432,10 @@ class ComparableProperty(MapperProperty):
                 self.descriptor = desc
         self.parent = parent
 
-    def instrument_class(self, mapper):
-        """Set up a proxy to the unmanaged descriptor."""
+    def _comparator_factory(self, mapper):
+        return self.comparator_factory(self, mapper)
 
-        attributes.register_descriptor(
-            mapper.class_, 
-            self.key, 
-            comparator=self.comparator_factory(self, mapper), 
-            parententity=mapper,
-            property_=self,
-            proxy_property=self.descriptor,
-            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):
diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py
index 8d468babe..c05dfe1e4 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
 
@@ -2579,7 +2579,10 @@ class _ColumnEntity(_QueryEntity):
         if isinstance(column, basestring):
             column = sql.literal_column(column)
             self._label_name = column.name
-        elif isinstance(column, attributes.QueryableAttribute):
+        elif isinstance(column, (
+                                    attributes.QueryableAttribute,
+                                    interfaces.PropComparator
+                                )):
             self._label_name = column.key
             column = column.__clause_element__()
         else:
diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py
index c9004990a..49f572572 100644
--- a/lib/sqlalchemy/orm/util.py
+++ b/lib/sqlalchemy/orm/util.py
@@ -359,15 +359,17 @@ class AliasedClass(object):
         
         if isinstance(attr, attributes.QueryableAttribute):
             return self.__adapt_prop(attr.property)
-            
-        if hasattr(attr, 'func_code'):
+        elif hasattr(attr, 'func_code'):
             is_method = getattr(self.__target, key, None)
             if is_method and is_method.im_self is not None:
                 return util.types.MethodType(attr.im_func, self, self)
             else:
                 return None
         elif hasattr(attr, '__get__'):
-            return attr.__get__(None, self)
+            ret = attr.__get__(None, self)
+            if isinstance(ret, PropComparator):
+                return ret.adapted(self.__adapt_element)
+            return ret
         else:
             return attr
 
@@ -536,40 +538,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)
diff --git a/test/orm/test_mapper.py b/test/orm/test_mapper.py
index e301b8d07..c9b86e8ee 100644
--- a/test/orm/test_mapper.py
+++ b/test/orm/test_mapper.py
@@ -899,9 +899,10 @@ class MapperTest(_fixtures.FixtureTest):
                 args = (UCComparator, User.uc_name)
             else:
                 args = (UCComparator,)
-
             mapper(User, users, properties=dict(
                     uc_name = sa.orm.comparable_property(*args)))
+#            import pdb
+#            pdb.set_trace()
             return User
 
         for User in (map_(True), map_(False)):
@@ -1169,7 +1170,7 @@ class DocumentTest(testing.TestBase):
                                     backref=backref('foo',doc='foo relationship')
                                 ),
             'foober':column_property(t1.c.col3, doc='alternate data col'),
-            'hoho':synonym(t1.c.col4, doc="syn of col4")
+            'hoho':synonym("col4", doc="syn of col4")
         })
         mapper(Bar, t2)
         compile_mappers()
@@ -1554,7 +1555,7 @@ class ComparatorFactoryTest(_fixtures.FixtureTest, AssertsCompiledSQL):
                     User.name == 'ed', 
                     "foobar(users.name) = foobar(:foobar_1)",
                     dialect=default.DefaultDialect())
-
+        
         self.assert_compile(
                     aliased(User).name == 'ed', 
                     "foobar(users_1.name) = foobar(:foobar_1)",