From a4401100f32af022218a3bf335676ad2fa6944d0 Mon Sep 17 00:00:00 2001 From: mike wakerly Date: Wed, 14 Dec 2022 13:45:51 -0500 Subject: [PATCH 1/4] breaking change: illegal to provide both `randomize` and `default` The `randomize` option/feature is essentially a built-in shorthand for a special kind of `default`. --- CHANGELOG.md | 4 ++++ django_spicy_id/fields.py | 2 ++ django_spicy_id/tests/fields_test.py | 5 +++++ 3 files changed, 11 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 32508c0..052a087 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Current version (in development) + +* Breaking change: Providing both `default` and `randomize` is not alowed. + ## v0.2.2 (2022-12-14) * First official release. diff --git a/django_spicy_id/fields.py b/django_spicy_id/fields.py index e941919..57cdd8c 100644 --- a/django_spicy_id/fields.py +++ b/django_spicy_id/fields.py @@ -67,6 +67,8 @@ def __init__( raise ImproperlyConfigured( "prefix: only ascii numbers and letters allowed, must start with a letter" ) + if randomize and kwargs.get("default"): + raise ImproperlyConfigured("cannot provide both `randomize` and `default`") self.prefix = prefix self.sep = sep diff --git a/django_spicy_id/tests/fields_test.py b/django_spicy_id/tests/fields_test.py index c72ce61..5170f86 100644 --- a/django_spicy_id/tests/fields_test.py +++ b/django_spicy_id/tests/fields_test.py @@ -35,6 +35,11 @@ def test_field_configuration(self): with self.assertRaisesMessage(ImproperlyConfigured, "sep must be ascii"): SpicyAutoField(prefix="ex", sep="frozen🍌") + with self.assertRaisesMessage( + ImproperlyConfigured, "cannot provide both `randomize` and `default`" + ): + SpicyAutoField(prefix="ex", default=123, randomize=True) + def test_model_with_defaults(self): model = models.Model_WithDefaults From 3dd1e38ea1ee01f6e82222df88214d5474a4316b Mon Sep 17 00:00:00 2001 From: mike wakerly Date: Wed, 14 Dec 2022 13:56:03 -0500 Subject: [PATCH 2/4] improvement: use `secrets` for the `randomize` feature; note hazards --- CHANGELOG.md | 1 + README.md | 8 ++++++-- django_spicy_id/fields.py | 11 +++++++---- django_spicy_id/tests/fields_test.py | 16 ++++++++-------- 4 files changed, 22 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 052a087..01caea4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Current version (in development) * Breaking change: Providing both `default` and `randomize` is not alowed. +* The `randomize` feature now uses the `secrets` module. ## v0.2.2 (2022-12-14) diff --git a/README.md b/README.md index e607af3..10d853b 100644 --- a/README.md +++ b/README.md @@ -122,12 +122,16 @@ The following parameters are required at declaration: In addition to all parameters you can provide a normal `AutoField`, each of the field types above supports the following additional optional paramters: +- **`encoding`**: What numeric encoding scheme to use. One of `fields.ENCODING_BASE_62` (default), `fields.ENCODING_BASE_58`, or `fields.ENCODING_HEX`. - **`sep`**: The separator character. Defaults to `_`. Can be any string. -- **`encoding`**: What numeric encoding scheme to use. One of `fields.ENCODING_BASE_62`, `fields.ENCODING_BASE_58`, or `fields.ENCODING_HEX`. - **`pad`**: Whether the encoded portion of the id should be zero-padded so that all values are the same string length. Either `False` (default) or `True`. - Example without padding: `user_8M0kX` - Example with padding: `user_0000008M0kX` -- **`randomize`**: If `True`, the default value for creates will be chosen from `random.randrange()`. If `False` (the default), works just like a normal `AutoField` i.e. the default value comes from the database upon `INSERT`. +- **`randomize`**: If `True`, the default value of a new record will be generated randomly using `secrets.randbelow()`. If `False` (the default), works just like a normal `AutoField` i.e. the default value comes from the database upon `INSERT`. + - When `randomize` is set, an error will be thrown if `default` is also set, since `randomize` is essentially a special and built-in `default` function. + - If you use this feature, be aware of its hazards: + - The generated ID may conflict with an existing row, with probability [determined by the birthday problem](https://en.wikipedia.org/wiki/Birthday_problem#Probability_table) (i.e. the column size and the size of the existing dataset). + - A conflict can also arise if two processes generate the same value for `secrets.randbelow()` (i.e. if system entropy is identical or misconfigured for some reason), ### Errors diff --git a/django_spicy_id/fields.py b/django_spicy_id/fields.py index 57cdd8c..d9d6168 100644 --- a/django_spicy_id/fields.py +++ b/django_spicy_id/fields.py @@ -1,6 +1,6 @@ import math -import random import re +import secrets from django.core.exceptions import ImproperlyConfigured from django.db import models @@ -92,6 +92,10 @@ def _to_string(self, intvalue): return f"{self.prefix}{self.sep}{encoded}" + def _generate_random_default_value(self): + """Generates a random value on the range [1, self.max_value).""" + return 1 + secrets.randbelow(self.max_value - 1) + def from_db_value(self, value, expression, connection): if value is None: return None @@ -99,7 +103,7 @@ def from_db_value(self, value, expression, connection): def get_prep_value(self, value): if value is None and self.randomize: - value = random.randrange(1, self.max_value) + value = self._generate_random_default_value() return value if value is None or isinstance(value, int): return super().get_prep_value(value) @@ -125,8 +129,7 @@ def has_default(self): def get_default(self): if self.randomize: - value = random.randrange(1, self.max_value) - return value + return self._generate_random_default_value() return super().get_default() def deconstruct(self): diff --git a/django_spicy_id/tests/fields_test.py b/django_spicy_id/tests/fields_test.py index 5170f86..4c483ae 100644 --- a/django_spicy_id/tests/fields_test.py +++ b/django_spicy_id/tests/fields_test.py @@ -119,25 +119,25 @@ def test_hex_model_with_padding(self): boundary = model.objects.create(id=2**63 - 1) self.assertEqual("ex_7fffffffffffffff", boundary.id) - @mock.patch("random.randrange") - def test_base62_model_with_randomize(self, mock_randrange): + @mock.patch("secrets.randbelow") + def test_base62_model_with_randomize(self, mock_secrets_randbelow): model = models.Base62Model_WithRandomize - mock_randrange.return_value = 123456789 + mock_secrets_randbelow.return_value = 123456788 o = model.objects.create() self.assertEqual("ex_8M0kX", o.id) - mock_randrange.assert_called_with(1, 2**63 - 1) + mock_secrets_randbelow.assert_called_with(2**63 - 2) o = model.objects.create(id=7) self.assertEqual("ex_7", o.id) - @mock.patch("random.randrange") - def test_hex_model_with_randomize(self, mock_randrange): + @mock.patch("secrets.randbelow") + def test_hex_model_with_randomize(self, mock_secrets_randbelow): model = models.HexModel_WithRandomize - mock_randrange.return_value = 123456789 + mock_secrets_randbelow.return_value = 123456788 o = model.objects.create() self.assertEqual("ex_75bcd15", o.id) - mock_randrange.assert_called_with(1, 2**63 - 1) + mock_secrets_randbelow.assert_called_with(2**63 - 2) o = model.objects.create(id=7) self.assertEqual("ex_7", o.id) From 6c9d1b59390b48b653362e8eb338317c453fbd0c Mon Sep 17 00:00:00 2001 From: mike wakerly Date: Wed, 14 Dec 2022 14:57:48 -0500 Subject: [PATCH 3/4] breaking change: throw `ProgrammingError` on illegal value Also exposes and documents `.re` and `.validate_string(strval)` --- CHANGELOG.md | 2 ++ README.md | 35 ++++++++++++++++--- django_spicy_id/fields.py | 50 ++++++++++++++++++++++------ django_spicy_id/tests/fields_test.py | 15 +++++---- 4 files changed, 79 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01caea4..63d5e67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,9 @@ ## Current version (in development) * Breaking change: Providing both `default` and `randomize` is not alowed. +* Breaking change: Illegal values now throw `django.db.utils.ProgrammingError` * The `randomize` feature now uses the `secrets` module. +* Fields now expose `.re` and `.validate_string(strval)` to assist with validation. ## v0.2.2 (2022-12-14) diff --git a/README.md b/README.md index 10d853b..881566b 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,12 @@ A drop-in replacement for Django's `AutoField` that gives you "Stripe-style" sel - [Field types](#field-types) - [Required Parameters](#required-parameters) - [Optional Parameters](#optional-parameters) + - [Field Attributes](#field-attributes) + - [`.validate_string(strval)`](#validate_stringstrval) + - [`.re`](#re) - [Errors](#errors) + - [`django.db.utils.ProgrammingError`](#djangodbutilsprogrammingerror) + - [`django_spicy_id.errors.MalformedSpicyIdError`](#django_spicy_iderrorsmalformedspicyiderror) - [Tips and tricks](#tips-and-tricks) - [Don't change field configuration](#dont-change-field-configuration) - [Changelog](#changelog) @@ -133,18 +138,38 @@ In addition to all parameters you can provide a normal `AutoField`, each of the - The generated ID may conflict with an existing row, with probability [determined by the birthday problem](https://en.wikipedia.org/wiki/Birthday_problem#Probability_table) (i.e. the column size and the size of the existing dataset). - A conflict can also arise if two processes generate the same value for `secrets.randbelow()` (i.e. if system entropy is identical or misconfigured for some reason), +### Field Attributes + +The following attributes are available on the field once constructed + +#### `.validate_string(strval)` + +Checks whether `strval` is a legal value for the field, throwing `django_spicy_id.errors.MalformedSpicyIdError` if not. + +#### `.re` + +A compiled regex which can be used to validate a string, e.g. in Django `urlpatterns`. + ### Errors -The field will throw `django_spicy_id.errors.MalformedSpicyIdError`, a subclass of `ValueError`, when an "illegal" string is provided. Note that this error can happen at runtime. +#### `django.db.utils.ProgrammingError` -Some examples of situations that will throw this error: +Thrown when attempting to access or query this field using an illegal value. Some examples of this situation: -* Querying a spicy id with the wrong prefix or separator (e.g `id="acct_1234"` where `id="invoice_1234"` is expected). -* Using illegal characters in the string. +* Providing a spicy id with the wrong prefix or separator (e.g `id="acct_1234"` where `id="invoice_1234"` is expected). +* Providing a string with illegal characters in it (i.e. where the encoded part isn't decodable) * Providing an unpadded value when padding is enabled. * Providing a padded value when padded is disabled. -Take special note of the last two errors: Regardless of field configuration, the string value of a spicy id must **always** be treated as an _exact value_. Just like you would never modify a `UUID4`, a spicy id string should never be translated, re-interpreted, or changed by a client. +You can consider these situations analogous to providing a wrongly-typed object to any other field type, for example `SomeModel.objects.filter(id=object())`. + +You can avoid this situation by validating inputs first. See _Field Attributes_. + +**🚨 Warning:** Regardless of field configuration, the string value of a spicy id must **always** be treated as an _exact value_. Just like you would never modify the contents of a `UUID4`, a spicy id string must never be translated, re-interpreted, or changed by a client. + +#### `django_spicy_id.errors.MalformedSpicyIdError` + +A subclass of `ValueError`, raised by `.validate_string(strval)` when the provided string is invalid for the field's configuration. ## Tips and tricks diff --git a/django_spicy_id/fields.py b/django_spicy_id/fields.py index d9d6168..c46d603 100644 --- a/django_spicy_id/fields.py +++ b/django_spicy_id/fields.py @@ -4,26 +4,30 @@ from django.core.exceptions import ImproperlyConfigured from django.db import models +from django.db.utils import ProgrammingError from django_spicy_id.errors import MalformedSpicyIdError from . import baseconv +# Encoding strategies which may be selected with the `encoding=` field parameter. ENCODING_HEX = "hex" ENCODING_BASE_58 = "b58" ENCODING_BASE_62 = "b62" +# Maps encoding strategy to its encoder/decoder. CODECS_BY_ENCODING = { ENCODING_HEX: baseconv.base16, ENCODING_BASE_58: baseconv.base58, ENCODING_BASE_62: baseconv.base62, } +# Validates acceptable values for the `prefix=` field parameter. LEGAL_PREFIX_RE = re.compile("^[a-zA-Z][0-9a-z-A-Z]?$") def get_regex(preamble, codec, pad, char_len): - """Builder function + """Returns a regex that validates a spicy id with with given parameters. If `pad` is True, the regex allows leading padding characters (a zero in most codecs). Else, these are not allowed. @@ -96,22 +100,46 @@ def _generate_random_default_value(self): """Generates a random value on the range [1, self.max_value).""" return 1 + secrets.randbelow(self.max_value - 1) + def _validate_string_internal(self, s): + if not isinstance(s, str): + raise MalformedSpicyIdError("value must be a string") + if not s: + raise MalformedSpicyIdError("value must be non-empty") + m = self.re.match(s) + if not self.re.match(s): + raise MalformedSpicyIdError( + f"value does not match expected regex {repr(self.re.pattern)}" + ) + _, encoded = m.groups() + return encoded + + def validate_string(self, strval): + """Utility function to validate any string against this field's config. + + Raises `MalformedSpicyIdError` on any error. Returns + """ + # Implemented by wrapping `_validate_string_internal` and stripping away the + # return value, because we need access to the retval internally (but don't + # want public clients to depend on it). + self._validate_string_internal(strval) + def from_db_value(self, value, expression, connection): if value is None: return None return self._to_string(value) def get_prep_value(self, value): - if value is None and self.randomize: - value = self._generate_random_default_value() - return value - if value is None or isinstance(value, int): + if not value: + if self.randomize: + return self._generate_random_default_value() return super().get_prep_value(value) - m = self.re.match(value) - if not m: - raise MalformedSpicyIdError(f'Value "{value}" does not match {self.re}') - _, encoded = m.groups() - return self.codec.decode(encoded) + elif isinstance(value, int): + return super().get_prep_value(value) + try: + encoded = self._validate_string_internal(value) + return self.codec.decode(encoded) + except MalformedSpicyIdError as e: + raise ProgrammingError(f"the value {repr(value)} is not valid: {e}") def to_python(self, value): if not value: @@ -120,7 +148,7 @@ def to_python(self, value): return value elif isinstance(value, int): return self._to_string(value) - raise MalformedSpicyIdError(f"Bad value: ${value}") + raise ProgrammingError(f"The value {repr(value)} is not valid for this field") def has_default(self): if self.randomize: diff --git a/django_spicy_id/tests/fields_test.py b/django_spicy_id/tests/fields_test.py index 4c483ae..6e22d5f 100644 --- a/django_spicy_id/tests/fields_test.py +++ b/django_spicy_id/tests/fields_test.py @@ -1,6 +1,7 @@ from unittest import mock from django.core.exceptions import ImproperlyConfigured +from django.db.utils import ProgrammingError from django.test import TestCase from django_spicy_id.errors import MalformedSpicyIdError @@ -57,7 +58,7 @@ def test_model_with_defaults(self): # When padding is disabled, it's an error to use padding characters. self.assertTrue(model.objects.filter(id="ex_8M0kX").first()) - with self.assertRaises(MalformedSpicyIdError): + with self.assertRaises(ProgrammingError): model.objects.filter(id="ex_0008M0kX").first() boundary = model.objects.create(id=2**63 - 1) @@ -80,7 +81,7 @@ def test_hex_model_with_defaults(self): # Using uppercase hex characters (i.e. supporting multiple legal # representations of the same value) is not allowed. - with self.assertRaises(MalformedSpicyIdError): + with self.assertRaises(ProgrammingError): model.objects.filter(id="ex_75BCD15").first() boundary = model.objects.create(id=2**63 - 1) @@ -151,7 +152,7 @@ def test_base62_model_fetch_by_string(self): self.assertEqual(retrieved, o) # Exact padding characters are mandatory when configured on the field. - with self.assertRaises(MalformedSpicyIdError): + with self.assertRaises(ProgrammingError): model.objects.filter(pk="ex_0008M0kX").first() self.assertEqual(retrieved, o) @@ -165,22 +166,22 @@ def test_hex_model_fetch_by_string(self): self.assertEqual(retrieved, o) # Exact padding characters are mandatory when configured on the field. - with self.assertRaises(MalformedSpicyIdError): + with self.assertRaises(ProgrammingError): model.objects.filter(pk="ex_0075bcd15").first() self.assertEqual(retrieved, o) def test_base62_model_create_by_string(self): model = models.Base62Model_WithPadding - o = model.objects.create(id="ex_7j") + o = model.objects.create(id="ex_0000000007j") self.assertEqual("ex_0000000007j", o.id) - def test_base62_model_create_by_string(self): + def test_hex_model_create_by_string(self): model = models.HexModel_WithPadding o = model.objects.create(id="ex_0000000000000123") self.assertEqual("ex_0000000000000123", o.id) # Exact padding characters are mandatory when configured on the field. - with self.assertRaises(MalformedSpicyIdError): + with self.assertRaises(ProgrammingError): model.objects.create(id="ex_000124") def test_base62_model_create_by_integer(self): From 4c535dd2fa9d41ac66f6f6a373305d5e975450ed Mon Sep 17 00:00:00 2001 From: mike wakerly Date: Wed, 14 Dec 2022 15:01:59 -0500 Subject: [PATCH 4/4] export symbols from the top-level `django_spicy_id` module --- CHANGELOG.md | 1 + README.md | 8 ++++---- django_spicy_id/__init__.py | 20 ++++++++++++++++++++ django_spicy_id/tests/fields_test.py | 3 +-- django_spicy_id/tests/models.py | 2 +- 5 files changed, 27 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 63d5e67..1589de0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ * Breaking change: Illegal values now throw `django.db.utils.ProgrammingError` * The `randomize` feature now uses the `secrets` module. * Fields now expose `.re` and `.validate_string(strval)` to assist with validation. +* Symbols are now exported from the top-level `django_spicy_id` module. ## v0.2.2 (2022-12-14) diff --git a/README.md b/README.md index 881566b..238e4e7 100644 --- a/README.md +++ b/README.md @@ -26,7 +26,7 @@ A drop-in replacement for Django's `AutoField` that gives you "Stripe-style" sel - [`.re`](#re) - [Errors](#errors) - [`django.db.utils.ProgrammingError`](#djangodbutilsprogrammingerror) - - [`django_spicy_id.errors.MalformedSpicyIdError`](#django_spicy_iderrorsmalformedspicyiderror) + - [`django_spicy_id.MalformedSpicyIdError`](#django_spicy_idmalformedspicyiderror) - [Tips and tricks](#tips-and-tricks) - [Don't change field configuration](#dont-change-field-configuration) - [Changelog](#changelog) @@ -91,7 +91,7 @@ Given the following example model: ```py from django.db import models -from django_spicy_id.fields import SpicyBigAutoField +from django_spicy_id import SpicyBigAutoField class User(models.model): id = SpicyBigAutoField(primary_key=True, prefix='usr') @@ -127,7 +127,7 @@ The following parameters are required at declaration: In addition to all parameters you can provide a normal `AutoField`, each of the field types above supports the following additional optional paramters: -- **`encoding`**: What numeric encoding scheme to use. One of `fields.ENCODING_BASE_62` (default), `fields.ENCODING_BASE_58`, or `fields.ENCODING_HEX`. +- **`encoding`**: What numeric encoding scheme to use. One of `django_spicy_id.ENCODING_BASE_62` (default), `django_spicy_id.ENCODING_BASE_58`, or `django_spicy_id.ENCODING_HEX`. - **`sep`**: The separator character. Defaults to `_`. Can be any string. - **`pad`**: Whether the encoded portion of the id should be zero-padded so that all values are the same string length. Either `False` (default) or `True`. - Example without padding: `user_8M0kX` @@ -167,7 +167,7 @@ You can avoid this situation by validating inputs first. See _Field Attributes_. **🚨 Warning:** Regardless of field configuration, the string value of a spicy id must **always** be treated as an _exact value_. Just like you would never modify the contents of a `UUID4`, a spicy id string must never be translated, re-interpreted, or changed by a client. -#### `django_spicy_id.errors.MalformedSpicyIdError` +#### `django_spicy_id.MalformedSpicyIdError` A subclass of `ValueError`, raised by `.validate_string(strval)` when the provided string is invalid for the field's configuration. diff --git a/django_spicy_id/__init__.py b/django_spicy_id/__init__.py index e69de29..0186158 100644 --- a/django_spicy_id/__init__.py +++ b/django_spicy_id/__init__.py @@ -0,0 +1,20 @@ +from .errors import MalformedSpicyIdError, SpicyIdError +from .fields import ( + ENCODING_BASE_58, + ENCODING_BASE_62, + ENCODING_HEX, + SpicyAutoField, + SpicyBigAutoField, + SpicySmallAutoField, +) + +__all__ = [ + SpicySmallAutoField, + SpicyAutoField, + SpicyBigAutoField, + ENCODING_BASE_58, + ENCODING_HEX, + ENCODING_BASE_62, + SpicyIdError, + MalformedSpicyIdError, +] diff --git a/django_spicy_id/tests/fields_test.py b/django_spicy_id/tests/fields_test.py index 6e22d5f..9e90618 100644 --- a/django_spicy_id/tests/fields_test.py +++ b/django_spicy_id/tests/fields_test.py @@ -4,8 +4,7 @@ from django.db.utils import ProgrammingError from django.test import TestCase -from django_spicy_id.errors import MalformedSpicyIdError -from django_spicy_id.fields import SpicyAutoField +from django_spicy_id import MalformedSpicyIdError, SpicyAutoField from django_spicy_id.tests import models diff --git a/django_spicy_id/tests/models.py b/django_spicy_id/tests/models.py index 220c159..9003850 100644 --- a/django_spicy_id/tests/models.py +++ b/django_spicy_id/tests/models.py @@ -1,6 +1,6 @@ from django.db import models -from django_spicy_id.fields import SpicyBigAutoField +from django_spicy_id import SpicyBigAutoField class Model_WithDefaults(models.Model):