Changeset 475

Timestamp:

09/29/09 14:14:37 (4 years ago)

Author:

ged

Message:

- changed the M2M column format again (only differ from the 0.6.x naming for

selfref M2M) and use the inverse relationship name to produce more sensible
column names.

- added a test simulating a migration from the old column format to the new

one.

- added lots of comments to explain what the different variables you can use in

the M2M format strings actually mean.

- changed the migration_aid_m2m_column_formatter so that the method can be

reused to migrate from any format to any format.

Location:

elixir/trunk

Files:

• elixir/options.py (modified) (1 diff)
• elixir/relationships.py (modified) (5 diffs)
• tests/test_m2m.py (modified) (5 diffs)

elixir/trunk/elixir/options.py

@@ -183 +183 @@
 __doc_all__ = ['options_defaults']
 
+OLD_M2MCOL_NAMEFORMAT = "%(tablename)s_%(key)s%(numifself)s"
+ALTERNATE_M2MCOL_NAMEFORMAT = "%(inversename)s_%(key)s"
+
+def default_m2m_column_formatter(data):
+    if data['selfref']:
+        return ALTERNATE_M2MCOL_NAMEFORMAT % data
+    else:
+        return OLD_M2MCOL_NAMEFORMAT % data
+
+NEW_M2MCOL_NAMEFORMAT = default_m2m_column_formatter
+
 # format constants
 FKCOL_NAMEFORMAT = "%(relname)s_%(key)s"
-OLD_M2MCOL_NAMEFORMAT = "%(tablename)s_%(key)s%(numifself)s"
-NEW_M2MCOL_NAMEFORMAT = "%(relname)s_%(key)s"
-M2MCOL_NAMEFORMAT = OLD_M2MCOL_NAMEFORMAT
+M2MCOL_NAMEFORMAT = NEW_M2MCOL_NAMEFORMAT
 CONSTRAINT_NAMEFORMAT = "%(tablename)s_%(colnames)s_fk"
 MULTIINHERITANCECOL_NAMEFORMAT = "%(entity)s_%(key)s"

elixir/trunk/elixir/relationships.py

@@ -870 +870 @@
         if options.MIGRATION_TO_07_AID:
             self.column_format = \
-                migration_aid_m2m_column_formatter(self.column_format)
+                migration_aid_m2m_column_formatter(
+                    lambda data: options.OLD_M2MCOL_NAMEFORMAT % data,
+                    self.column_format)
 
         self.filter = filter
@@ -995 +997 @@
 
             joins = (self.primaryjoin_clauses, self.secondaryjoin_clauses)
-            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)):
+            for num, desc, fk_name, rel, inverse, colnames in (
+              (0, e1_desc, source_fk_name, self, self.inverse, self.local_colname),
+              (1, e2_desc, target_fk_name, self.inverse, self, self.remote_colname)):
 
                 fk_colnames = []
@@ -1005 +1007 @@
                     assert len(colnames) == len(desc.primary_keys)
                 else:
-                    #FIXME: desc is not the target desc. Do I need to fix to
-                    # code or the doc? in fact the relname corresponds to the
-                    # relationship going from the entity to the M2M, so the new
-                    # naming scheme might not really make sense
-                    data = {# relationship info
+                    # The data generated here will be fed to the M2M column
+                    # formatter to generate the name of the columns of the
+                    # intermediate table for *one* side of the relationship,
+                    # that is, from the intermediate table to the current
+                    # entity, as stored in the "desc" variable.
+                    data = {# A) relationships info
+
+                            # the name of the rel going *from* the entity
+                            # we are currently generating a column pointing
+                            # *to*. This is generally *not* what you want to
+                            # use. eg in a "Post" and "Tag" example, with
+                            # relationships named 'tags' and 'posts', when
+                            # creating the columns from the intermediate
+                            # table to the "Post" entity, 'relname' will
+                            # contain 'tags'.
                             'relname': rel and rel.name or 'inverse',
+
+                            # the name of the inverse relationship. In the
+                            # above example, 'inversename' will contain
+                            # 'posts'.
+                            'inversename': inverse and inverse.name
+                                                   or 'inverse',
+                            # is A == B?
                             'selfref': e1_desc is e2_desc,
+                            # provided for backward compatibility, DO NOT USE!
                             'num': num,
+                            # provided for backward compatibility, DO NOT USE!
                             'numifself': e1_desc is e2_desc and str(num + 1)
                                                             or '',
-                            # source (not target!) info
-                            'source': desc.entity,
+                            # B) target information (from the perspective of
+                            #    the intermediate table)
+                            'target': desc.entity,
                             'entity': desc.entity.__name__.lower(),
-                            'tablename': desc.tablename
+                            'tablename': desc.tablename,
+
+                            # C) current (intermediate) table name
+                            'current_table': tablename
                            }
                     colnames = []
@@ -1045 +1070 @@
                 ondelete = rel and rel.ondelete
 
+                #FIXME: fk_name is misleading
                 constraints.append(
                     ForeignKeyConstraint(fk_colnames, fk_refcols,
@@ -1104 +1130 @@
 
 
-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 migration_aid_m2m_column_formatter(oldformatter, newformatter):
     def debug_formatter(data):
-        new_name = formatter(data)
-        old_name = options.OLD_M2MCOL_NAMEFORMAT % data
+        old_name = oldformatter(data)
+        new_name = newformatter(data)
         if new_name != old_name:
             complete_data = data.copy()
             complete_data.update(old_name=old_name,
                                  new_name=new_name,
-                                 dir=data['num'] is 0 and 'local' or 'remote')
+                                 targetname=data['target'].__name__)
             # 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.
-            #FIXME: entity is probably wrong here since it refers to the target
-            #entity.
-            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'. "
+            warnings.warn("The '%(old_name)s' column in the "
+                          "'%(current_table)s' table, used as the "
+                          "intermediate table for the '%(relname)s' "
+                          "relationship on the '%(targetname)s' entity "
+                          "was renamed to '%(new_name)s'."
                           % complete_data)
         return new_name

elixir/trunk/tests/test_m2m.py

@@ -17 +17 @@
     def test_simple(self):
         class A(Entity):
-            name = Field(String(60))
-            bs_ = ManyToMany('B')
-
-        class B(Entity):
+            using_options(shortnames=True)
+            name = Field(String(60))
+            as_ = ManyToMany('A')
+            bs_ = ManyToMany('B')
+
+        class B(Entity):
+            using_options(shortnames=True)
             name = Field(String(60))
             as_ = ManyToMany('A')
@@ -33 +36 @@
         # check column names
         m2m_cols = m2m_table.columns
-        assert 'bs__id' in m2m_cols
+        assert 'a_id' in m2m_cols
+        assert 'b_id' 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
 
         # check the relationships work as expected
@@ -76 +84 @@
         # this needs to be done before declaring the classes
         elixir.options.M2MCOL_NAMEFORMAT = \
-            elixir.relationships.alternate_m2m_column_formatter
+            elixir.options.ALTERNATE_M2MCOL_NAMEFORMAT
 
         class A(Entity):
@@ -93 +101 @@
         # 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
+        assert 'as__id' in m2m_cols
+        assert 'bs__id' in m2m_cols
 
         # check selfref m2m table column names were generated correctly
@@ -101 +109 @@
         assert 'inverse_id' in m2m_cols
 
-    #TODO: add an upgrade test
-    #def test_upgrade(self):
-#        elixir.options.M2MCOL_NAMEFORMAT = elixir.options.OLD_M2MCOL_NAMEFORMAT
+    def test_upgrade(self):
+        elixir.options.M2MCOL_NAMEFORMAT = elixir.options.OLD_M2MCOL_NAMEFORMAT
+
+        class A(Entity):
+            using_options(shortnames=True)
+            name = Field(String(20))
+            links_to = ManyToMany('A')
+            is_linked_from = ManyToMany('A')
+            bs_ = ManyToMany('B')
+
+        class B(Entity):
+            using_options(shortnames=True)
+            name = Field(String(20))
+            as_ = ManyToMany('A')
+
+        setup_all(True)
+
+        a = A(name='a1', links_to=[A(name='a2')])
+
+        session.commit()
+        session.clear()
+
+        del A
+        del B
+
+        # do not drop the tables, that's the whole point!
+        cleanup_all()
+
+        # simulate a renaming of columns (as given by the migration aid)
+        # 'a_id1' to 'is_linked_from_id'.
+        # 'a_id2' to 'links_to_id'.
+        conn = metadata.bind.connect()
+        conn.execute("ALTER TABLE a_links_to__a_is_linked_from RENAME TO temp")
+        conn.execute("CREATE TABLE a_links_to__a_is_linked_from ("
+                        "is_linked_from_id INTEGER NOT NULL, "
+                        "links_to_id INTEGER NOT NULL, "
+                     "PRIMARY KEY (is_linked_from_id, links_to_id), "
+                     "CONSTRAINT a_fk1 FOREIGN KEY(is_linked_from_id) "
+                                      "REFERENCES a (id), "
+                     "CONSTRAINT a_fk2 FOREIGN KEY(links_to_id) "
+                                      "REFERENCES a (id))")
+        conn.execute("INSERT INTO a_links_to__a_is_linked_from "
+                     "(is_linked_from_id, links_to_id) "
+                     "SELECT a_id1, a_id2 FROM temp")
+        conn.close()
+
+        # ...
+        elixir.options.M2MCOL_NAMEFORMAT = elixir.options.NEW_M2MCOL_NAMEFORMAT
 #        elixir.options.MIGRATION_TO_07_AID = True
-#        metadata.bind = 'sqlite://'
-#        elixir.options.M2MCOL_NAMEFORMAT = elixir.options.NEW_M2MCOL_NAMEFORMAT    #    assert False
+
+        class A(Entity):
+            using_options(shortnames=True)
+            name = Field(String(20))
+            links_to = ManyToMany('A')
+            is_linked_from = ManyToMany('A')
+            bs_ = ManyToMany('B')
+
+        class B(Entity):
+            using_options(shortnames=True)
+            name = Field(String(20))
+            as_ = ManyToMany('A')
+
+        setup_all()
+
+        a1 = A.get_by(name='a1')
+        assert len(a1.links_to) == 1
+        assert not a1.is_linked_from
+
+        a2 = a1.links_to[0]
+        assert a2.name == 'a2'
+        assert not a2.links_to
+        assert a2.is_linked_from == [a1]
 
     def test_manual_column_format(self):