fixing bug with mixins

The Table metaclass now copies columns, otherwise when attributes were set on the columns, it would effect other users of that mixin.

Author: dantownsend

piccolo/apps/migrations/auto/migration_manager.py

@@ -609,7 +609,11 @@ async def _run_add_columns(self, backwards=False):
                 _Table._meta.tablename = add_columns[0].tablename
 
                 for add_column in add_columns:
-                    column = add_column.column
+                    # We fetch the column from the Table, as the metaclass
+                    # copies and sets it up properly.
+                    column = _Table._meta.get_column_by_name(
+                        add_column.column._meta.name
+                    )
                     await _Table.alter().add_column(
                         name=column._meta.name, column=column
                     ).run()

piccolo/table.py

@@ -172,7 +172,12 @@ def __init_subclass__(
 
             attribute = getattr(cls, attribute_name)
             if isinstance(attribute, Column):
-                column = attribute
+                # We have to copy, then override the existing column
+                # definition, in case this column is inheritted from a mixin.
+                # Otherwise, when we set attributes on that column, it will
+                # effect all other users of that mixin.
+                column = attribute.copy()
+                setattr(cls, attribute_name, column)
 
                 if isinstance(column, PrimaryKey):
                     # We want it at the start.
@@ -185,8 +190,8 @@ def __init_subclass__(
                 column._meta._name = attribute_name
                 column._meta._table = cls
 
-            if isinstance(column, ForeignKey):
-                foreign_key_columns.append(column)
+                if isinstance(column, ForeignKey):
+                    foreign_key_columns.append(column)
 
         cls._meta = TableMeta(
             tablename=tablename,

tests/table/test_inheritance.py

@@ -25,6 +25,14 @@ class Manager(StartedOnMixin, FavouriteMixin, Table):
     name = Varchar()
 
 
+class ManagerA(StartedOnMixin, Table):
+    name = Varchar()
+
+
+class ManagerB(StartedOnMixin, Table):
+    name = Varchar()
+
+
 class TestInheritance(TestCase):
     """
     Make sure columns can be inheritted from parent classes.
@@ -56,3 +64,43 @@ def test_inheritance(self):
         self.assertEqual(response["started_on"], started_on)
         self.assertEqual(response["name"], name)
         self.assertEqual(response["favourite"], favourite)
+
+
+class TestRepeatedMixin(TestCase):
+    """
+    Make sure that if a mixin is used multiple times (i.e. across several
+    Table classes), that it still works as expected.
+    """
+
+    def setUp(self):
+        ManagerA.create_table().run_sync()
+        ManagerB.create_table().run_sync()
+
+    def tearDown(self):
+        ManagerA.alter().drop_table().run_sync()
+        ManagerB.alter().drop_table().run_sync()
+
+    def test_inheritance(self):
+        """
+        Make sure both tables work as expected (they both inherit from the
+        same mixin).
+        """
+        # Make sure the columns can be retrieved from both tables:
+        for column_name in ("started_on", "name"):
+            for Table_ in (ManagerA, ManagerB):
+                Table_._meta.get_column_by_name(column_name)
+
+        # Test saving and retrieving data:
+        started_on = datetime.datetime(year=1989, month=12, day=1)
+        name = "Guido"
+
+        for _Table in (ManagerA, ManagerB):
+            _Table(name=name, started_on=started_on).save().run_sync()
+
+            response = _Table.select().first().run_sync()
+            self.assertEqual(response["started_on"], started_on)
+            self.assertEqual(response["name"], name)
+
+        # Make sure the tables have the correct tablenames still.
+        self.assertEqual(ManagerA._meta.tablename, "manager_a")
+        self.assertEqual(ManagerB._meta.tablename, "manager_b")