Remove magic behaviour in UuidMixin. Fixes #197 (#198)

* Rename UuidMixin.url_id to huuid
* Drop fix for missing suuid in url_name_suuid
* BaseIdNameMixin should not use uuid column

CHANGES.rst

@@ -24,6 +24,7 @@
 * New: ``coaster.utils.classmethodproperty``: like a property, but for class
   methods
 * New: ``coaster.views.endpoint_for``: discover an endpoint given a URL
+* Changed: ``UuidMixin`` no longer provides ``url_id``
 
 
 0.6.0

coaster/sqlalchemy/mixins.py

@@ -102,31 +102,8 @@ class UuidMixin(object):
     """
     Provides a ``uuid`` attribute that is either a SQL UUID column or an alias
     to the existing ``id`` column if the class uses UUID primary keys. Also
-    provides hybrid properties ``url_id``, ``buid`` and ``suuid`` that provide
-    hex, BUID and ShortUUID representations of the ``uuid`` column.
-
-    :class:`UuidMixin` must appear before other classes in the base class order::
-
-        class MyDocument(UuidMixin, BaseMixin, db.Model):
-            pass
-
-    Compatibility table:
-
-    +-----------------------+-------------+-----------------------------------------+
-    | Base class            | Compatible? | Notes                                   |
-    +=======================+=============+=========================================+
-    | BaseMixin             | Yes         |                                         |
-    +-----------------------+-------------+-----------------------------------------+
-    | BaseIdNameMixin       | Yes         |                                         |
-    +-----------------------+-------------+-----------------------------------------+
-    | BaseNameMixin         | N/A         | ``name`` is secondary key, not ``uuid`` |
-    +-----------------------+-------------+-----------------------------------------+
-    | BaseScopedNameMixin   | N/A         | ``name`` is secondary key, not ``uuid`` |
-    +-----------------------+-------------+-----------------------------------------+
-    | BaseScopedIdMixin     | No          | Conflicting :attr:`url_id` attribute    |
-    +-----------------------+-------------+-----------------------------------------+
-    | BaseScopedIdNameMixin | No          | Conflicting :attr:`url_id` attribute    |
-    +-----------------------+-------------+-----------------------------------------+
+    provides hybrid properties ``huuid``, ``buid`` and ``suuid`` that provide
+    hex, URL-safe Base64 and ShortUUID representations of the ``uuid`` column.
     """
     @with_roles(read={'all'})
     @declared_attr
@@ -138,20 +115,20 @@ def uuid(cls):
             return immutable(Column(UUIDType(binary=False), default=uuid_.uuid4, unique=True, nullable=False))
 
     @hybrid_property
-    def url_id(self):
+    def huuid(self):
         """URL-friendly UUID representation as a hex string"""
         return self.uuid.hex
 
-    @url_id.comparator
-    def url_id(cls):
+    @huuid.comparator
+    def huuid(cls):
         # For some reason the test fails if we use `cls.uuid` here
         # but works fine in the `buid` and `suuid` comparators below
         if hasattr(cls, '__uuid_primary_key__') and cls.__uuid_primary_key__:
             return SqlHexUuidComparator(cls.id)
         else:
             return SqlHexUuidComparator(cls.uuid)
 
-    url_id = with_roles(url_id, read={'all'})
+    huuid = with_roles(huuid, read={'all'})
 
     @hybrid_property
     def buid(self):
@@ -564,18 +541,16 @@ def make_name(self):
     @hybrid_property
     def url_id_name(self):
         """
-        Returns a URL name combining :attr:`url_id` and :attr:`name` in id-name
-        syntax. This property is also available as :attr:`url_name` for legacy
-        reasons.
+        Returns a URL name combining :attr:`url_id` and :attr:`name` in
+        id-name syntax. This property is also available as :attr:`url_name`
+        for legacy reasons.
         """
         return '%s-%s' % (self.url_id, self.name)
 
     @url_id_name.comparator
     def url_id_name(cls):
         if cls.__uuid_primary_key__:
             return SqlHexUuidComparator(cls.id, splitindex=0)
-        elif issubclass(cls, UuidMixin):
-            return SqlHexUuidComparator(cls.uuid, splitindex=0)
         else:
             return SqlSplitIdComparator(cls.id, splitindex=0)
 
@@ -588,19 +563,11 @@ def url_name_suuid(self):
         Returns a URL name combining :attr:`name` and :attr:`suuid` in name-suuid syntax.
         To use this, the class must derive from :class:`UuidMixin`.
         """
-        if isinstance(self, UuidMixin):
-            return '%s-%s' % (self.name, self.suuid)
-        else:
-            return '%s-%s' % (self.name, self.url_id)
+        return '%s-%s' % (self.name, self.suuid)
 
     @url_name_suuid.comparator
     def url_name_suuid(cls):
-        if issubclass(cls, UuidMixin):
-            return SqlSuuidComparator(cls.uuid, splitindex=-1)
-        elif cls.__uuid_primary_key__:
-            return SqlHexUuidComparator(cls.id, splitindex=-1)
-        else:
-            return SqlSplitIdComparator(cls.id, splitindex=-1)
+        return SqlSuuidComparator(cls.uuid, splitindex=-1)
 
 
 class BaseScopedIdMixin(BaseMixin):

tests/test_models.py

@@ -623,9 +623,15 @@ def test_uuid_key(self):
 
     def test_uuid_url_id(self):
         """
-        IdMixin and UuidMixin provide a url_id that renders as a string of
-        either the integer primary key or the UUID primary key
+        IdMixin provides a url_id that renders as a string of either the
+        integer primary key or the UUID primary key. In addition, UuidMixin
+        provides a huuid that always renders a UUID against either the
+        id or uuid columns.
         """
+        # TODO: This test is a little muddled because UuidMixin renamed
+        # its url_id property (which overrode IdMixin's url_id) to huuid.
+        # This test needs to be broken down into separate tests for each of
+        # these properties.
         u1 = NonUuidKey()
         u2 = UuidKey()
         u3 = NonUuidMixinKey()
@@ -648,14 +654,14 @@ def test_uuid_url_id(self):
         self.assertFalse('-' in u2.url_id)  # Without dashes
 
         self.assertIsInstance(i3, uuid.UUID)
-        self.assertEqual(u3.url_id, i3.hex)
-        self.assertEqual(len(u3.url_id), 32)  # This is a 32-byte hex representation
-        self.assertFalse('-' in u3.url_id)  # Without dashes
+        self.assertEqual(u3.huuid, i3.hex)
+        self.assertEqual(len(u3.huuid), 32)  # This is a 32-byte hex representation
+        self.assertFalse('-' in u3.huuid)  # Without dashes
 
         self.assertIsInstance(i4, uuid.UUID)
-        self.assertEqual(u4.url_id, i4.hex)
-        self.assertEqual(len(u4.url_id), 32)  # This is a 32-byte hex representation
-        self.assertFalse('-' in u4.url_id)  # Without dashes
+        self.assertEqual(u4.huuid, i4.hex)
+        self.assertEqual(len(u4.huuid), 32)  # This is a 32-byte hex representation
+        self.assertFalse('-' in u4.huuid)  # Without dashes
 
         # Querying against `url_id` redirects the query to
         # `id` (IdMixin) or `uuid` (UuidMixin).
@@ -672,7 +678,7 @@ def test_uuid_url_id(self):
             "non_uuid_key.id = '1'")
 
         # With UUID primary keys, `url_id` casts the value into a UUID
-        # and then queries against `id` or ``uuid``
+        # and then queries against `id`
 
         # Note that `literal_binds` here doesn't know how to render UUIDs if
         # no engine is specified, and so casts them into a string. We test this
@@ -715,7 +721,7 @@ def test_uuid_url_id(self):
                 ).compile(compile_kwargs={'literal_binds': True})),
             "non_uuid_key.id IS NULL")
         self.assertEqual(
-            six.text_type((NonUuidMixinKey.url_id == None  # NOQA
+            six.text_type((NonUuidMixinKey.huuid == None  # NOQA
                 ).compile(compile_kwargs={'literal_binds': True})),
             "non_uuid_mixin_key.uuid IS NULL")
 
@@ -729,11 +735,11 @@ def test_uuid_url_id(self):
 
         # Repeat against UuidMixin classes (with only hex keys for brevity)
         self.assertEqual(
-            six.text_type((NonUuidMixinKey.url_id == '74d588574a7611e78c27c38403d0935c'
+            six.text_type((NonUuidMixinKey.huuid == '74d588574a7611e78c27c38403d0935c'
                 ).compile(compile_kwargs={'literal_binds': True})),
             "non_uuid_mixin_key.uuid = '74d588574a7611e78c27c38403d0935c'")
         self.assertEqual(
-            six.text_type((UuidMixinKey.url_id == '74d588574a7611e78c27c38403d0935c'
+            six.text_type((UuidMixinKey.huuid == '74d588574a7611e78c27c38403d0935c'
                 ).compile(compile_kwargs={'literal_binds': True})),
             "uuid_mixin_key.id = '74d588574a7611e78c27c38403d0935c'")
 
@@ -843,13 +849,15 @@ def test_uuid_url_id_name_suuid(self):
 
         self.assertEqual(u1.url_id, '74d588574a7611e78c27c38403d0935c')
         self.assertEqual(u1.url_id_name, '74d588574a7611e78c27c38403d0935c-test')
-        # No UuidMixin means no suuid, so fallback to hex UUID
-        self.assertEqual(u1.url_name_suuid, 'test-74d588574a7611e78c27c38403d0935c')
-        self.assertEqual(u2.url_id, '74d588574a7611e78c27c38403d0935c')
+        # No suuid without UuidMixin
+        with self.assertRaises(AttributeError):
+            self.assertEqual(u1.url_name_suuid, 'test-vVoaZTeXGiD4qrMtYNosnN')
+        self.assertEqual(u2.huuid, '74d588574a7611e78c27c38403d0935c')
         self.assertEqual(u2.url_id_name, '74d588574a7611e78c27c38403d0935c-test')
         self.assertEqual(u2.url_name_suuid, 'test-vVoaZTeXGiD4qrMtYNosnN')
-        self.assertEqual(u3.url_id, '74d588574a7611e78c27c38403d0935c')
-        self.assertEqual(u3.url_id_name, '74d588574a7611e78c27c38403d0935c-test')
+        self.assertEqual(u3.huuid, '74d588574a7611e78c27c38403d0935c')
+        # url_id_name in BaseIdNameMixin uses the id column, not the uuid column
+        self.assertEqual(u3.url_id_name, '1-test')
         self.assertEqual(u3.url_name_suuid, 'test-vVoaZTeXGiD4qrMtYNosnN')
 
         # url_name is legacy

tests/test_roles.py

@@ -155,7 +155,7 @@ def test_basemixin_roles(self):
 
     def test_uuidmixin_roles(self):
         """A model with UuidMixin provides 'all' read access to uuid, url_id, buid and suuid"""
-        self.assertLessEqual({'uuid', 'url_id', 'buid', 'suuid'}, UuidModel.__roles__['all']['read'])
+        self.assertLessEqual({'uuid', 'huuid', 'buid', 'suuid'}, UuidModel.__roles__['all']['read'])
 
     def test_roles_for_anon(self):
         """An anonymous actor should have 'all' and 'anon' roles"""