From 6564fd1328688254a1d554dba19e6c5c35b0a3b2 Mon Sep 17 00:00:00 2001 From: Kiran Jonnalagadda Date: Mon, 5 Nov 2018 20:38:37 +0530 Subject: [PATCH] Rename UuidMixin.url_id to huuid. Fixes #197 Also drop missing suuid fix for url_name_suuid --- CHANGES.rst | 1 + coaster/sqlalchemy/mixins.py | 59 ++++++++++-------------------------- tests/test_models.py | 39 ++++++++++++++---------- tests/test_roles.py | 2 +- 4 files changed, 41 insertions(+), 60 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 601d3d4b..f29b756a 100644 --- a/CHANGES.rst +++ b/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 diff --git a/coaster/sqlalchemy/mixins.py b/coaster/sqlalchemy/mixins.py index 47454f80..da89c4ca 100644 --- a/coaster/sqlalchemy/mixins.py +++ b/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,12 +115,12 @@ 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__: @@ -151,7 +128,7 @@ def url_id(cls): 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,11 +541,15 @@ 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:`huuid` or :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) + # Use UUID instead of id when available + if isinstance(self, UuidMixin): + return '%s-%s' % (self.huuid, self.name) + else: + return '%s-%s' % (self.url_id, self.name) @url_id_name.comparator def url_id_name(cls): @@ -588,19 +569,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): diff --git a/tests/test_models.py b/tests/test_models.py index cc788e45..c4082ed1 100644 --- a/tests/test_models.py +++ b/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,12 +849,13 @@ 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.huuid, '74d588574a7611e78c27c38403d0935c') self.assertEqual(u3.url_id_name, '74d588574a7611e78c27c38403d0935c-test') self.assertEqual(u3.url_name_suuid, 'test-vVoaZTeXGiD4qrMtYNosnN') diff --git a/tests/test_roles.py b/tests/test_roles.py index e7cefb1a..765bc3a8 100644 --- a/tests/test_roles.py +++ b/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"""