Changeset 458

Timestamp:

09/17/09 13:54:19 (4 years ago)

Author:

ged

Message:

- Changed the pattern used by default to generate column names for (all)

ManyToMany relationships so that the meaning of bidirectional
self-referential relationships does not depend on the order of declaration
of each side (closes #69). See upgrade notes for details.

- Made M2MCOL_NAMEFORMAT accept a callable, so that more complex naming

generation can be used if so desired.

Location:

elixir/trunk

Files:

• CHANGES (modified) (4 diffs)
• elixir/options.py (modified) (2 diffs)
• elixir/relationships.py (modified) (5 diffs)
• tests/test_m2m.py (modified) (6 diffs)

elixir/trunk/CHANGES

@@ -2 +2 @@
 
 Please see http://elixir.ematia.de/trac/wiki/Migrate06to07 for detailed
-upgrade notes.
+upgrade notes. If you are upgrading an application with existing data from an
+earlier version of Elixir, you are VERY STRONGLY ADVISED to read them!
 
 New features:
@@ -20 +21 @@
   ManyToMany table in a custom schema and not necessarily the same schema as
   the table of the "source" entity (patch from Diez B. Roggisch).
-- Added a new "target_column" argument on ManyToOne relationships (ticket #26).
-  TODOC
+- Added a new "target_column" argument on ManyToOne relationships so that you
+  can target unique but non-primary key columns. At the moment, this only works
+  if the target column is declared before the ManyToOne (see ticket #26).
 - Added new column_names argument to the acts_as_versioned extension, allowing
   to specify custom column names (inspired by a patch by Alex Bodnaru).
@@ -49 +51 @@
 
 Bug fixes:
+- Changed the pattern used to generate column names for all ManyToMany
+  relationships so that the meaning of bidirectional
+  self-referential relationships does not depend on the order of declaration
+  of each side (closes #69). See upgrade notes for details.
+- Changed slightly the pattern used 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).
+  See upgrade notes for details.
 - default EntityCollection raise an exception instead of returning None when
   trying to resolve an inexisting Entity from outside of another entity (for
   example through EntityCollection.__getattr__
-- 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).
-  =================
 - Fixed the case where you specify both "primaryjoin" and "colname" arguments
   (useless in this case, but harmless) on a ManyToOne relationship of an
@@ -72 +71 @@
   secondaryjoin as not been specified manually.
 - Added missing documentation for the "filter" argument on OneToMany
-  relationships
+  relationships.
 - Fixed the act_as_list extension's move_to_bottom method to work on MySQL
   (closes #34).
 - Fixed event methods not being called when they are defined on a parent class.
-  (introduced in r262).
+  (introduced in release 0.5.0).
 - Added workaround for an odd mod_python behavior (class.__module__ returns a
   weird name which is not in sys.modules).

elixir/trunk/elixir/options.py

@@ -184 +184 @@
 # format constants
 FKCOL_NAMEFORMAT = "%(relname)s_%(key)s"
-M2MCOL_NAMEFORMAT = "%(tablename)s_%(key)s"
+OLD_M2MCOL_NAMEFORMAT = "%(tablename)s_%(key)s%(numifself)s"
+NEW_M2MCOL_NAMEFORMAT = "%(relname)s_%(key)s"
+M2MCOL_NAMEFORMAT = NEW_M2MCOL_NAMEFORMAT
 CONSTRAINT_NAMEFORMAT = "%(tablename)s_%(colnames)s_fk"
 MULTIINHERITANCECOL_NAMEFORMAT = "%(entity)s_%(key)s"
@@ -197 +199 @@
 
 # debugging/migration help
-CHECK_TABLENAME_CHANGES = False
+MIGRATION_TO_07_AID = False
 
 #

elixir/trunk/elixir/relationships.py

@@ -417 +417 @@
 __doc_all__ = []
 
+
 class Relationship(Property):
     '''
@@ -862 +863 @@
                 "all ManyToMany tables", DeprecationWarning, stacklevel=3)
         self.column_format = column_format or options.M2MCOL_NAMEFORMAT
+        if not callable(self.column_format):
+            # we need to store the format in a variable so that the
+            # closure of the lambda is correct
+            format = self.column_format
+            self.column_format = lambda data: format % data
+        if options.MIGRATION_TO_07_AID:
+            self.column_format = \
+                migration_aid_column_formatter(self.column_format)
 
         self.filter = filter
@@ -938 +947 @@
             # whether this relation or its inverse is setup first).
             if self.inverse and source_part < target_part:
-                #TODO: if self-relf, only include source_part
+                #XXX: use a different scheme for selfref (to not include the
+                #     table name twice)?
                 tablename = "%s__%s" % (target_part, source_part)
-                if options.CHECK_TABLENAME_CHANGES and \
+                if options.MIGRATION_TO_07_AID and \
                    e1_desc.tablename >= e2_desc.tablename:
                     oldname = "%s__%s" % (source_part, target_part)
-
-                    raise Exception(
+                    warnings.warn(
                         "The generated table name for the '%s' relationship "
                         "on the '%s' entity changed from '%s' (the name "
@@ -996 +1005 @@
                     assert len(colnames) == len(desc.primary_keys)
                 else:
+                    data = {# relationship info
+                            'relname': rel and rel.name or 'inverse',
+                            'selfref': e1_desc is e2_desc,
+                            'num': num,
+                            'numifself': e1_desc is e2_desc and str(num + 1)
+                                                            or '',
+                            # target info
+                            'target': desc.entity,
+                            'entity': desc.entity.__name__.lower(),
+                            'tablename': desc.tablename
+                           }
+                    colnames = []
                     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)
+                        data.update(key=pk_col.key)
+                        colnames.append(self.column_format(data))
 
                 for pk_col, colname in zip(desc.primary_keys, colnames):
@@ -1087 +1100 @@
 
 
+def alternate_m2m_column_formatter(data):
+    if data['selfref']:
+        return options.NEW_M2MCOL_NAMEFORMAT % data
+    else:
+        return options.OLD_M2MCOL_NAMEFORMAT % data
+
+
+def migration_aid_m2m_column_formatter(formatter):
+    def debug_formatter(data):
+        new_name = formatter(data)
+        old_name = options.OLD_M2MCOL_NAMEFORMAT % data
+        if new_name != old_name:
+            complete_data = data.copy()
+            #TODO: use explicit num and selfref variables
+            complete_data.update(old_name=old_name,
+                                 new_name=new_name,
+                                 dir=data['num'] is 0 and 'local' or 'remote')
+            # Specifying a stacklevel is useless in this case as the name
+            # generation is triggered by setup_all(), not by the declaration
+            # of the offending relationship.
+            warnings.warn("The generated column name for the '%(relname)s' "
+                          "relationship on the '%(entity)s' entity changed "
+                          "from '%(old_name)s' to '%(new_name)s'. "
+                          % complete_data)
+        return new_name
+    return debug_formatter
+
+
 def _get_join_clauses(local_table, local_cols1, local_cols2, target_table):
     primary_join, secondary_join = [], []

elixir/trunk/tests/test_m2m.py

@@ -4 +4 @@
 
 from elixir import *
+import elixir
 
 #-----------
@@ -24 +25 @@
 
         setup_all(True)
-
+        A.mapper.compile()
+
+        # check m2m table was generated correctly
+        m2m_table = A.bs_.property.secondary
+        assert m2m_table.name in metadata.tables
+
+        # check column names
+        m2m_cols = m2m_table.columns
+        assert 'bs__id' in m2m_cols
+        assert 'as__id' in m2m_cols
+
+        # check the relationships work as expected
         b1 = B(name='b1', as_=[A(name='a1')])
 
@@ -36 +48 @@
         assert b in a.bs_
 
-    def test_column_format(self):
+    def test_custom_global_column_nameformat(self):
+        # this needs to be done before declaring the classes
+        elixir.options.M2MCOL_NAMEFORMAT = elixir.options.OLD_M2MCOL_NAMEFORMAT
+
+        class A(Entity):
+            bs_ = ManyToMany('B')
+
+        class B(Entity):
+            as_ = ManyToMany('A')
+
+        setup_all(True)
+
+        # revert to original format
+        elixir.options.M2MCOL_NAMEFORMAT = elixir.options.NEW_M2MCOL_NAMEFORMAT
+
+        # check m2m table was generated correctly
+        A.mapper.compile()
+        m2m_table = A.bs_.property.secondary
+        assert m2m_table.name in metadata.tables
+
+        # check column names
+        m2m_cols = m2m_table.columns
+        assert '%s_id' % A.table.name in m2m_cols
+        assert '%s_id' % B.table.name in m2m_cols
+
+    def test_alternate_column_formatter(self):
+        # this needs to be done before declaring the classes
+        elixir.options.M2MCOL_NAMEFORMAT = \
+            elixir.relationships.alternate_m2m_column_formatter
+
+        class A(Entity):
+            as_ = ManyToMany('A')
+            bs_ = ManyToMany('B')
+
+        class B(Entity):
+            as_ = ManyToMany('A')
+
+        setup_all(True)
+        A.mapper.compile()
+
+        # revert to original format
+        elixir.options.M2MCOL_NAMEFORMAT = elixir.options.NEW_M2MCOL_NAMEFORMAT
+
+        # check m2m table column names were generated correctly
+        m2m_cols = A.bs_.property.secondary.columns
+        assert '%s_id' % A.table.name in m2m_cols
+        assert '%s_id' % B.table.name in m2m_cols
+
+        # check selfref m2m table column names were generated correctly
+        m2m_cols = A.as_.property.secondary.columns
+        assert 'as__id' in m2m_cols
+        assert 'inverse_id' in m2m_cols
+
+    def test_manual_column_format(self):
         class A(Entity):
             using_options(tablename='aye')
@@ -49 +114 @@
         setup_all(True)
 
-        b1 = B(name='b1', as_=[A(name='a1')])
-
-        session.commit()
-        session.clear()
-
-        a = A.query.one()
-        b = B.query.one()
-
-        assert a in b.as_
-        assert b in a.bs_
-
+        # check column names were generated correctly
+        A.mapper.compile()
         m2m_cols = A.bs_.property.secondary.columns
         assert 'a_id' in m2m_cols
         assert 'b_id' in m2m_cols
 
+        # check the relationships work as expected
+        b1 = B(name='b1', as_=[A(name='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_multi_pk_in_target(self):
         class A(Entity):
@@ -137 +205 @@
         assert barney in homer.friends
 
+        m2m_cols = Person.friends.property.secondary.columns
+        assert 'friends_id' in m2m_cols
+        assert 'inverse_id' in m2m_cols
+
     def test_bidirectional_selfref(self):
         class Person(Entity):
@@ -159 +231 @@
         assert homer in barney.friends
         assert barney in homer.friends
+
+        m2m_cols = Person.friends.property.secondary.columns
+        assert 'friends_id' in m2m_cols
+        assert 'is_friend_of_id' in m2m_cols
 
     def test_has_and_belongs_to_many(self):