Merge pull request #2 from mik3y/mikey/fixups

Various modest improvements

CHANGELOG.md

@@ -1,5 +1,13 @@
 # Changelog
 
+## 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.
+* Symbols are now exported from the top-level `django_spicy_id` module.
+
 ## v0.2.2 (2022-12-14)
 
 * First official release.

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.MalformedSpicyIdError`](#django_spicy_idmalformedspicyiderror)
 - [Tips and tricks](#tips-and-tricks)
   - [Don't change field configuration](#dont-change-field-configuration)
 - [Changelog](#changelog)
@@ -86,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')
@@ -122,25 +127,49 @@ 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 `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.
-- **`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),
+
+### 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.MalformedSpicyIdError`
+
+A subclass of `ValueError`, raised by `.validate_string(strval)` when the provided string is invalid for the field's configuration.
 
 ## Tips and tricks
 

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,
+]

django_spicy_id/fields.py

@@ -1,29 +1,33 @@
 import math
-import random
 import re
+import secrets
 
 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.
@@ -67,6 +71,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
@@ -90,22 +96,50 @@ 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 _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 = random.randrange(1, self.max_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:
@@ -114,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:
@@ -123,8 +157,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):

django_spicy_id/tests/fields_test.py

@@ -1,10 +1,10 @@
 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
-from django_spicy_id.fields import SpicyAutoField
+from django_spicy_id import MalformedSpicyIdError, SpicyAutoField
 from django_spicy_id.tests import models
 
 
@@ -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
 
@@ -52,7 +57,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)
@@ -75,7 +80,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)
@@ -114,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)
 
@@ -146,7 +151,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)
 
@@ -160,22 +165,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):

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):