Changeset 397

Timestamp:

08/21/08 17:32:31 (5 years ago)

Author:

ged

Message:

New features:
- The local_colname and remote_colname arguments on ManyToMany relationships

can now also be used to set custom names for the ManyToMany table columns.
This effectively replace the column_format on ManyToMany relationships which
is now deprecated. Change based on a patch from Diez B. Roggisch.

- Added (or rather fixed and documented) a "table" argument on ManyToMany

relationships to allow using a manually-defined Table (closes #44).

- Added a "schema" argument on ManyToMany relationship to be able to create the

ManyToMany table in a custom schema and not necessarily the same schema as
the table of the "source" entity (patch from Diez B. Roggisch).

Changes:
- Renamed remote_side and local_side ManyToMany arguments to remote_colname and

local_colname respectively to not collide with the remote_side argument
provided by SA (it doesn't make much sense on ManyToMany relationships but
still).

Bug fixes:
- Changed slightly the algorithm to generate the name of the table for

bidirectional self-referential ManyToMany relationships so that it doesn't
depend on the order of declaration of each side (closes #19). If you are
upgrading an application with existing data from an earlier version of
Elixir, you are STRONGLY ADVISED to read the upgrade notes!
=============================
TEMPORARY BUT IMPORTANT NOTE:
=============================
For now it is even worse: the table works but the relationship's meaning is
reversed. Will be fixed before 0.7 ships (see ticket #69).
=============================

- Added missing documentation for the "filter" argument on OneToMany

relationships

Not logged in CHANGES:
- bumped version
- added a few tests
- completed/fixed the API doc a bit
- added some comments in the code so that I remember why things are done this

way.

Location:

elixir/trunk

Files:

• CHANGES (modified) (1 diff)
• elixir/__init__.py (modified) (1 diff)
• elixir/options.py (modified) (1 diff)
• elixir/relationships.py (modified) (18 diffs)
• setup.cfg (modified) (1 diff)
• setup.py (modified) (1 diff)
• tests/test_autoload.py (modified) (1 diff)
• tests/test_m2m.py (modified) (2 diffs)

elixir/trunk/CHANGES

@@ +1 @@
+0.7.0
+
+Please see http://elixir.ematia.de/trac/wiki/Migrate06to07 for detailed
+upgrade notes.
+
+New features:
+- The local_colname and remote_colname arguments on ManyToMany relationships
+  can now also be used to set custom names for the ManyToMany table columns.
+  This effectively replace the column_format on ManyToMany relationships which
+  is now deprecated. Change based on a patch from Diez B. Roggisch.
+- Added (or rather fixed and documented) a "table" argument on ManyToMany
+  relationships to allow using a manually-defined Table (closes #44).
+- Added a "schema" argument on ManyToMany relationship to be able to create the
+  ManyToMany table in a custom schema and not necessarily the same schema as
+  the table of the "source" entity (patch from Diez B. Roggisch).
+
+Changes:
+- Renamed remote_side and local_side ManyToMany arguments to remote_colname and
+  local_colname respectively to not collide with the remote_side argument
+  provided by SA (it doesn't make much sense on ManyToMany relationships but
+  still).
+
+Bug fixes:
+- Changed slightly the algorithm to generate the name of the table for
+  bidirectional self-referential ManyToMany relationships so that it doesn't
+  depend on the order of declaration of each side (closes #19). If you are
+  upgrading an application with existing data from an earlier version of
+  Elixir, you are STRONGLY ADVISED to read the upgrade notes!
+  =================
+  TEMPORARY NOTICE:
+  =================
+  For now it is even worse: the table works but the relationship's meaning is
+  reversed. Will be fixed before 0.7 ships (see ticket #69).
+  =================
+- Added missing documentation for the "filter" argument on OneToMany
+  relationships
+
 0.6.1 - 2008-08-18
 

elixir/trunk/elixir/__init__.py

@@ -42 +42 @@
 
 
-__version__ = '0.6.1'
+__version__ = '0.7.0'
 
 __all__ = ['Entity', 'EntityMeta', 'EntityCollection', 'entities',

elixir/trunk/elixir/options.py

@@ -185 +185 @@
 POLYMORPHIC_COL_TYPE = String(POLYMORPHIC_COL_SIZE)
 
+# debugging/migration constants
+CHECK_TABLENAME_CHANGES = False
+
 #
 options_defaults = dict(

elixir/trunk/elixir/relationships.py

@@ -165 +165 @@
 |                    | by a minus (for descending order).                     |
 +--------------------+--------------------------------------------------------+
+| ``filter``         | Specify a filter criterion (as a clause element) for   |
+|                    | this relationship. This criterion will be and_'ed with |
+|                    | the normal join criterion (primaryjoin) generated by   |
+|                    | Elixir for the relationship. For example:              |
+|                    | boston_addresses = \                                   |
+|                    |  OneToMany('Address', filter=Address.city == 'Boston') |
++--------------------+--------------------------------------------------------+
 
 Additionally, Elixir supports an alternate, DSL-based, syntax to define
 OneToMany_ relationships, with the has_many_ statement.
-
-Also, as for standard SQLAlchemy relations, the ``order_by`` keyword argument
 
 
@@ -220 +225 @@
 
 If the entity containing your ``ManyToMany`` relationship is
-autoloaded, you **must** specify at least one of either the ``remote_side`` or
-``local_side`` argument.
+autoloaded, you **must** specify at least one of either the ``remote_colname``
+or ``local_colname`` argument.
 
 In addition to keyword arguments inherited from SQLAlchemy, ``ManyToMany``
@@ -234 +239 @@
 |                    | database.                                              |
 +--------------------+--------------------------------------------------------+
-| ``remote_side``    | A column name or list of column names specifying       |
-|                    | which column(s) in the intermediary table are used     |
-|                    | for the "remote" part of a self-referential            |
-|                    | relationship. This argument has an effect only when    |
-|                    | your entities are autoloaded.                          |
-+--------------------+--------------------------------------------------------+
-| ``local_side``     | A column name or list of column names specifying       |
-|                    | which column(s) in the intermediary table are used     |
-|                    | for the "local" part of a self-referential             |
-|                    | relationship. This argument has an effect only when    |
-|                    | your entities are autoloaded.                          |
+| ``schema``         | Specify a custom schema for the intermediate table.    |
+|                    | This can be used both when the tables needs to         |
+|                    | be created and when the table is autoloaded/reflected  |
+|                    | from the database.                                     |
++--------------------+--------------------------------------------------------+
+| ``remote_colname`` | A string or list of strings specifying the names of    |
+|                    | the column(s) in the intermediary table which          |
+|                    | reference the "remote"/target entity's table.          |
++--------------------+--------------------------------------------------------+
+| ``local_colname``  | A string or list of strings specifying the names of    |
+|                    | the column(s) in the intermediary table which          |
+|                    | reference the "local"/current entity's table.          |
++--------------------+--------------------------------------------------------+
+| ``table``          | Use a manually created table. If this argument is      |
+|                    | used, Elixir won't generate a table for this           |
+|                    | relationship, and use the one given instead.           |
 +--------------------+--------------------------------------------------------+
 | ``order_by``       | Specify which field(s) should be used to sort the      |
@@ -252 +262 @@
 |                    | entity. These field names can optionally be prefixed   |
 |                    | by a minus (for descending order).                     |
-+--------------------+--------------------------------------------------------+
-| ``column_format``  | Specify an alternate format string for naming the      |
++----------------------+------------------------------------------------------+
+| ``ondelete``       | Value for the foreign key constraint ondelete clause.  |
+|                    | May be one of: ``cascade``, ``restrict``,              |
+|                    | ``set null``, or ``set default``.                      |
++--------------------+--------------------------------------------------------+
+| ``onupdate``       | Value for the foreign key constraint onupdate clause.  |
+|                    | May be one of: ``cascade``, ``restrict``,              |
+|                    | ``set null``, or ``set default``.                      |
++--------------------+--------------------------------------------------------+
+| ``column_format``  | DEPRECATED. Specify an alternate format string for     |
+|                    | naming the                                             |
 |                    | columns in the mapping table.  The default value is    |
 |                    | defined in ``elixir.options.M2MCOL_NAMEFORMAT``.  You  |
@@ -356 +375 @@
 
 import sys
-
-from sqlalchemy         import ForeignKeyConstraint, Column, \
-                               Table, and_
-from sqlalchemy.orm     import relation, backref
+import warnings
+
+from sqlalchemy import ForeignKeyConstraint, Column, Table, and_
+from sqlalchemy.orm import relation, backref
 from sqlalchemy.ext.associationproxy import association_proxy
 
 import options
-from elixir.statements  import ClassMutator
-from elixir.fields      import Field
-from elixir.properties  import Property
-from elixir.entity      import EntityDescriptor, EntityMeta
+from elixir.statements import ClassMutator
+from elixir.fields import Field
+from elixir.properties import Property
+from elixir.entity import EntityDescriptor, EntityMeta
 
 
@@ -418 +437 @@
 
         # transform callable arguments
-        for arg in ('primaryjoin', 'secondaryjoin', 'remote_side'):
+        for arg in ('primaryjoin', 'secondaryjoin', 'remote_side', 'filter'):
             kwarg = kwargs.get(arg, None)
             if callable(kwarg):
@@ -669 +688 @@
 
         if self.entity.table is self.target.table:
+            # this is needed because otherwise SA has no way to know what is
+            # the direction of the relationship since both columns present in 
+            # the primaryjoin belong to the same table. In other words, it is 
+            # necessary to know if this particular relation 
+            # is the many-to-one side, or the one-to-xxx side. The foreignkey 
+            # doesn't help in this case.
             kwargs['remote_side'] = \
                 [col for col in self.target.table.primary_key.columns]
@@ -686 +711 @@
         self.filter = kwargs.pop('filter', None)
         if self.filter is not None:
-            kwargs['viewonly'] = True
+            # We set viewonly to True by default for filtered relationship,
+            # unless manually overridden.
+            # This is not strictly necessary, as SQLAlchemy allows non viewonly
+            # relationships with a custom join/filter. The example at:
+            # SADOCS/05/mappers.html#advdatamapping_relation_customjoin
+            # is not viewonly. Those relationships can be used as if the extra
+            # filter wasn't present when inserting. This can lead to a
+            # confusing behavior (if you insert data which doesn't match the
+            # extra criterion it'll get inserted anyway but you won't see it 
+            # when you query back the attribute after a round-trip to the 
+            # database).
+            if 'viewonly' not in kwargs:
+                kwargs['viewonly'] = True
         super(OneToOne, self).__init__(*args, **kwargs)
 
@@ -701 +738 @@
                       "inheritance you "
                       "might need to specify inverse relationships "
-                      "manually by using the inverse keyword."
+                      "manually by using the 'inverse' argument."
                       % (self.target.__name__, self.name,
                          self.entity.__name__))
@@ -711 +748 @@
         # So, we should either complete the selfref test to prove that they
         # are indeed useful, or remove them. It might be they are indeed
-        # useless because of the primaryjoin, and that the remote_side is
-        # already setup in the other way (belongs_to).
+        # useless because the remote_side is already setup in the other way 
+        # (ManyToOne).
         if self.entity.table is self.target.table:
             #FIXME: IF this code is of any use, it will probably break for
@@ -738 +775 @@
     def __init__(self, *args, **kwargs):
         self.user_tablename = kwargs.pop('tablename', None)
-        self.local_side = kwargs.pop('local_side', [])
-        if self.local_side and not isinstance(self.local_side, list):
-            self.local_side = [self.local_side]
-        self.remote_side = kwargs.pop('remote_side', [])
-        if self.remote_side and not isinstance(self.remote_side, list):
-            self.remote_side = [self.remote_side]
+        self.local_colname = kwargs.pop('local_colname', [])
+        if self.local_colname and not isinstance(self.local_colname, list):
+            self.local_colname = [self.local_colname]
+        self.remote_colname = kwargs.pop('remote_colname', [])
+        if self.remote_colname and not isinstance(self.remote_colname, list):
+            self.remote_colname = [self.remote_colname]
         self.ondelete = kwargs.pop('ondelete', None)
         self.onupdate = kwargs.pop('onupdate', None)
-        self.column_format = kwargs.pop('column_format', options.M2MCOL_NAMEFORMAT)
+        if 'column_format' in kwargs:
+            warnings.warn("The 'column_format' argument on ManyToMany "
+                "relationships is deprecated. Please use the 'local_colname' "
+                "and/or 'remote_colname' arguments if you want custom "
+                "column names for this table only, or modify "
+                "options.M2MCOL_NAMEFORMAT if you want a custom format for "
+                "all ManyToMany tables", DeprecationWarning, stacklevel=3)
+        self.column_format = kwargs.pop('column_format',
+                                        options.M2MCOL_NAMEFORMAT)
 
         self.secondary_table = kwargs.pop('table', None)
+        self.schema = kwargs.pop('schema', None)
+
         self.primaryjoin_clauses = list()
         self.secondaryjoin_clauses = list()
@@ -762 +809 @@
         # autoloaded entities
         if self.secondary_table:
+            self._reflect_table()
             return
 
@@ -776 +824 @@
         e1_schema = e1_desc.table_options.get('schema', None)
         e2_schema = e2_desc.table_options.get('schema', None)
-        assert e1_schema == e2_schema, \
-               "Schema %r for entity %s differs from schema %r of entity %s" \
+        schema = (self.schema is not None) and schema or e1_schema
+
+        assert e1_schema == e2_schema or self.schema, \
+               "Schema %r for entity %s differs from schema %r of entity %s." \
+               " Consider using the schema-parameter. "\
                % (e1_schema, self.entity.__name__,
                   e2_schema, self.target.__name__)
@@ -804 +855 @@
             # We need to keep the table name consistent (independant of
             # whether this relation or its inverse is setup first).
-            if self.inverse and e1_desc.tablename < e2_desc.tablename:
+            if self.inverse and source_part < target_part:
+                #TODO: if self-relf, only include source_part
                 tablename = "%s__%s" % (target_part, source_part)
+                if options.CHECK_TABLENAME_CHANGES and \
+                   e1_desc.tablename >= e2_desc.tablename:
+                    oldname = "%s__%s" % (source_part, target_part)
+
+                    raise Exception(
+                        "The generated table name for the '%s' relationship "
+                        "on the '%s' entity changed from '%s' (the name "
+                        "generated by Elixir 0.6.1 and earlier) to '%s'. "
+                        "You should either rename the table in the database "
+                        "to the new name or use the tablename argument on the "
+                        "relationship to force the old name: tablename='%s'!"
+                        % (self.name, self.entity.__name__, oldname,
+                           tablename, oldname))
             else:
                 tablename = "%s__%s" % (source_part, target_part)
 
         if e1_desc.autoload:
-            self._reflect_table(tablename)
+            if not e2_desc.autoload:
+                raise Exception(
+                    "Entity '%s' is autoloaded and its '%s' "
+                    "ManyToMany relationship points to "
+                    "the '%s' entity which is not autoloaded"
+                    % (self.entity.__name__, self.name,
+                       self.target.__name__))
+
+            self.secondary_table = Table(tablename, e1_desc.metadata,
+                                         autoload=True)
+            self._reflect_table()
         else:
             # We pre-compute the names of the foreign key constraints
@@ -828 +903 @@
 
             joins = (self.primaryjoin_clauses, self.secondaryjoin_clauses)
-            for num, desc, fk_name, rel in (
-                    (0, e1_desc, source_fk_name, self),
-                    (1, e2_desc, target_fk_name, self.inverse)):
+            for num, desc, fk_name, rel, colnames in (
+              (0, e1_desc, source_fk_name, self, self.local_colname),
+              (1, e2_desc, target_fk_name, self.inverse, self.remote_colname)):
+
                 fk_colnames = list()
                 fk_refcols = list()
 
-                for pk_col in desc.primary_keys:
-                    colname = self.column_format % \
-                              {'tablename': desc.tablename,
-                               'key': pk_col.key,
-                               'entity': desc.entity.__name__.lower()}
-
-                    # In case we have a many-to-many self-reference, we
-                    # need to tweak the names of the columns so that we
-                    # don't end up with twice the same column name.
-                    if self.entity is self.target:
-                        colname += str(num + 1)
-
+                if colnames:
+                    assert len(colnames) == len(desc.primary_keys)
+                else:
+                    for pk_col in desc.primary_keys:
+                        colname = self.column_format % \
+                                  {'tablename': desc.tablename,
+                                   'key': pk_col.key,
+                                   'entity': desc.entity.__name__.lower()}
+                        # In case we have a many-to-many self-reference, we
+                        # need to tweak the names of the columns so that we
+                        # don't end up with twice the same column name.
+                        if self.entity is self.target:
+                            colname += str(num + 1)
+                        colnames.append(colname)
+
+                for pk_col, colname in zip(desc.primary_keys, colnames):
                     col = Column(colname, pk_col.type, primary_key=True)
                     columns.append(col)
@@ -873 +953 @@
 
             self.secondary_table = Table(tablename, e1_desc.metadata,
-                                         schema=e1_schema, *args)
-
-    def _reflect_table(self, tablename):
-        if not self.target._descriptor.autoload:
-            raise Exception(
-                "Entity '%s' is autoloaded and its '%s' "
-                "ManyToMany relationship points to "
-                "the '%s' entity which is not autoloaded"
-                % (self.entity.__name__, self.name,
-                   self.target.__name__))
-
-        self.secondary_table = Table(tablename,
-                                     self.entity._descriptor.metadata,
-                                     autoload=True)
-
+                                         schema=schema, *args)
+
+    def _reflect_table(self):
         # In the case we have a self-reference, we need to build join clauses
         if self.entity is self.target:
             #CHECKME: maybe we should try even harder by checking if that
             # information was defined on the inverse relationship)
-            if not self.local_side and not self.remote_side:
+            if not self.local_colname and not self.remote_colname:
                 raise Exception(
                     "Self-referential ManyToMany "
                     "relationships in autoloaded entities need to have at "
-                    "least one of either 'local_side' or 'remote_side' "
+                    "least one of either 'local_colname' or 'remote_colname' "
                     "argument specified. The '%s' relationship in the '%s' "
                     "entity doesn't have either."
@@ -903 +971 @@
             self.primaryjoin_clauses, self.secondaryjoin_clauses = \
                 _get_join_clauses(self.secondary_table,
-                                  self.local_side, self.remote_side,
+                                  self.local_colname, self.remote_colname,
                                   self.entity.table)
 
@@ -946 +1014 @@
 
             # if all columns point to the correct table, we use the constraint
+            #TODO: check that it contains as many columns as the pk of the
+            #target entity, or even that it points to the actual pk columns
             for fk in constraint.elements:
                 if fk.references(target_table):

elixir/trunk/setup.cfg

@@ -9 +9 @@
 modules = elixir, elixir.ext.associable, elixir.ext.versioned,
           elixir.ext.encrypted, elixir.ext.list
-trac_browser_url = http://elixir.ematia.de/trac/browser/elixir/tags/0.6.1
+trac_browser_url = http://elixir.ematia.de/trac/browser/elixir/tags/0.7.0
 

elixir/trunk/setup.py

@@ -2 +2 @@
 
 setup(name="Elixir",
-      version="0.6.1",
+      version="0.7.0",
       description="Declarative Mapper for SQLAlchemy",
       long_description="""

elixir/trunk/tests/test_autoload.py

@@ -60 +60 @@
         appreciate = ManyToMany('Person',
                                 tablename='person_person',
-                                local_side='person_id1')
+                                local_colname='person_id1')
         isappreciatedby = ManyToMany('Person',
                                 tablename='person_person',
-                                local_side='person_id2')
+                                local_colname='person_id2')
 
     class Animal(Entity):

elixir/trunk/tests/test_m2m.py

@@ -121 +121 @@
     def test_selfref(self):
         class Person(Entity):
+            using_options(shortnames=True)
             name = Field(String(30))
 
             friends = ManyToMany('Person')
+
+        setup_all(True)
+
+        barney = Person(name="Barney")
+        homer = Person(name="Homer", friends=[barney])
+        barney.friends.append(homer)
+
+        session.commit()
+        session.clear()
+
+        homer = Person.get_by(name="Homer")
+        barney = Person.get_by(name="Barney")
+
+        assert homer in barney.friends
+        assert barney in homer.friends
+
+    def test_bidirectional_selfref(self):
+        class Person(Entity):
+            using_options(shortnames=True)
+            name = Field(String(30))
+
+            friends = ManyToMany('Person')
+            is_friend_of = ManyToMany('Person')
 
         setup_all(True)
@@ -170 +194 @@
         assert not a3.bs
 
+    def test_local_and_remote_colnames(self):
+        class A(Entity):
+            using_options(shortnames=True)
+            key1 = Field(Integer, primary_key=True, autoincrement=False)
+            key2 = Field(String(40), primary_key=True)
+
+            bs_ = ManyToMany('B', local_colname=['foo', 'bar'],
+                                  remote_colname="baz")
+
+        class B(Entity):
+            using_options(shortnames=True)
+            name = Field(String(60))
+            as_ = ManyToMany('A', remote_colname=['foo', 'bar'],
+                                  local_colname="baz")
+
+        setup_all(True)
+
+        b1 = B(name='b1', as_=[A(key1=10, key2='a1')])
+
+        session.commit()
+        session.clear()
+
+        a = A.query.one()
+        b = B.query.one()
+
+        assert a in b.as_
+        assert b in a.bs_
+
+    def test_manual_table_auto_joins(self):
+        from sqlalchemy import Table, Column, ForeignKey, ForeignKeyConstraint
+
+        a_b = Table('a_b', metadata,
+                    Column('a_key1', None),
+                    Column('a_key2', None),
+                    Column('b_id', None, ForeignKey('b.id')),
+                    ForeignKeyConstraint(['a_key1', 'a_key2'],
+                                         ['a.key1', 'a.key2']))
+
+        class A(Entity):
+            using_options(shortnames=True)
+            key1 = Field(Integer, primary_key=True, autoincrement=False)
+            key2 = Field(String(40), primary_key=True)
+
+            bs_ = ManyToMany('B', table=a_b)
+
+        class B(Entity):
+            using_options(shortnames=True)
+            name = Field(String(60))
+            as_ = ManyToMany('A', table=a_b)
+
+        setup_all(True)
+
+        b1 = B(name='b1', as_=[A(key1=10, key2='a1')])
+
+        session.commit()
+        session.clear()
+
+        a = A.query.one()
+        b = B.query.one()
+
+        assert a in b.as_
+        assert b in a.bs_
+
+    def test_manual_table_manual_joins(self):
+        from sqlalchemy import Table, Column, ForeignKey, \
+                               ForeignKeyConstraint, and_
+
+        a_b = Table('a_b', metadata,
+                    Column('a_key1', Integer),
+                    Column('a_key2', String(40)),
+                    Column('b_id', String(60)))
+
+        class A(Entity):
+            using_options(shortnames=True)
+            key1 = Field(Integer, primary_key=True, autoincrement=False)
+            key2 = Field(String(40), primary_key=True)
+
+            bs_ = ManyToMany('B', table=a_b,
+                             primaryjoin=lambda: and_(A.key1 == a_b.c.a_key1,
+                                                      A.key2 == a_b.c.a_key2),
+                             secondaryjoin=lambda: B.id == a_b.c.b_id,
+                             foreign_keys=[a_b.c.a_key1, a_b.c.a_key2,
+                                 a_b.c.b_id])
+
+        class B(Entity):
+            using_options(shortnames=True)
+            name = Field(String(60))
+
+        setup_all(True)
+
+        a1 = A(key1=10, key2='a1', bs_=[B(name='b1')])
+
+        session.commit()
+        session.clear()
+
+        a = A.query.one()
+        b = B.query.one()
+
+        assert b in a.bs_