Skip to content

Commit

Permalink
Rename UuidMixin.url_id to huuid. Fixes #197
Browse files Browse the repository at this point in the history
Also drop missing suuid fix for url_name_suuid
  • Loading branch information
jace committed Nov 5, 2018
1 parent 837b984 commit 6564fd1
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 60 deletions.
1 change: 1 addition & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
59 changes: 16 additions & 43 deletions coaster/sqlalchemy/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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):
Expand All @@ -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):
Expand Down
39 changes: 23 additions & 16 deletions tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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).
Expand All @@ -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
Expand Down Expand Up @@ -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")

Expand All @@ -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'")

Expand Down Expand Up @@ -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')

Expand Down
2 changes: 1 addition & 1 deletion tests/test_roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"""
Expand Down

0 comments on commit 6564fd1

Please sign in to comment.