Skip to content

Commit

Permalink
Add workaround for model sometimes returning a numeric .id on create
Browse files Browse the repository at this point in the history
See issue #6.

This works around the following upstream Django issue, which affects
versions pre-4.0: https://code.djangoproject.com/ticket/32442

The fix is a bit ugly (installs a signal handler; which runs in non-deterministic
order alongside any others that have been installed), but I don't see a better one.
  • Loading branch information
mik3y committed Aug 23, 2023
1 parent 6024ecb commit 574df80
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 4 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ This package supports and is tested against the latest patch versions of:

All database backends are tested with the latest versions of their drivers. SQLite is also tested on GitHub Actions' latest macOS virtual environment.

**Note:** Django 4.x is recommended due to an [upstream bug](https://code.djangoproject.com/ticket/32442) that is present in 3.x. See [#6](https://github.com/mik3y/django-spicy-id/issues/6) for further details.

### Instructions

```
Expand Down
33 changes: 33 additions & 0 deletions django_spicy_id/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
import re
import secrets

import django
from django.core.exceptions import ImproperlyConfigured
from django.db import models
from django.db.models.signals import post_save
from django.db.utils import ProgrammingError

from django_spicy_id.errors import MalformedSpicyIdError
Expand Down Expand Up @@ -182,6 +184,37 @@ def deconstruct(self):
del kwargs["default"]
return name, path, args, kwargs

def contribute_to_class(self, cls, name, **kwargs):
super().contribute_to_class(cls, name, **kwargs)

# Special case: Register a signal handler when this row is created.
# Workaround for issue #6 / Django issue 32442.
#
# When the DB connection does not support `can_return_columns_from_insert`,
# the raw row value is returned by Django on insert. This causes the caller
# to receive a numeric value when the row is inserted, due to the upstream
# bug (value does flow through `Field.from_db_value()`).
#
# https://code.djangoproject.com/ticket/32442

# Not needed in Django >= 4.
if django.VERSION >= (4, 0):
return

# Only affects primary keys.
if not self.primary_key:
return

def spicy_id_create_handler(sender, instance, created, raw, **kwargs):
if not created or raw:
return
nonlocal name
val = getattr(instance, name)
if isinstance(val, int):
setattr(instance, name, self.to_python(val))

post_save.connect(spicy_id_create_handler, sender=cls, weak=False)


class SpicyBigAutoField(BaseSpicyAutoField, models.BigAutoField):
"""A Spicy ID field that is backed by a standard 64-bit Django BigAutoField."""
Expand Down
6 changes: 3 additions & 3 deletions django_spicy_id/tests/fields_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,12 +209,12 @@ def test_base62_model_create_by_integer(self):
@mock.patch("secrets.randbelow")
def test_randomize_sets_pk_to_a_string(self, mock_secrets_randbelow):
"""Ensures that when `randomize` is used, the value set is a string not a number."""
model = models.Base62Model_WithRandomize
model = models.SpicyAutoFieldModel_WithRandomize

mock_secrets_randbelow.return_value = 1
o = model()
self.assertEqual("ex_2", o.pk)
self.assertEqual("ex_2", o.id)
self.assertTrue(o._state.adding)
o.save()
self.assertEqual("ex_2", o.pk)
self.assertEqual("ex_2", o.id)
self.assertFalse(o._state.adding)
6 changes: 5 additions & 1 deletion django_spicy_id/tests/models.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from django.db import models

from django_spicy_id import SpicyBigAutoField
from django_spicy_id import SpicyAutoField, SpicyBigAutoField


class Model_WithDefaults(models.Model):
Expand Down Expand Up @@ -31,3 +31,7 @@ class Base62Model_WithRandomize(models.Model):

class HexModel_WithRandomize(models.Model):
id = SpicyBigAutoField("ex", primary_key=True, encoding="hex", randomize=True)


class SpicyAutoFieldModel_WithRandomize(models.Model):
id = SpicyAutoField("ex", primary_key=True, encoding="hex", randomize=True)

0 comments on commit 574df80

Please sign in to comment.