Opened 8 years ago
Closed 7 years ago
#28385 closed Bug (fixed)
deserializers ignore natural keys when primary key has a default value
| Reported by: | Daniel Knell | Owned by: | Hasan Ramezani |
|---|---|---|---|
| Component: | Core (Serialization) | Version: | 1.11 |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Accepted | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | yes | UI/UX: | no |
Description (last modified by )
def build_instance(Model, data, db):
"""
Build a model instance.
If the model instance doesn't have a primary key and the model supports
natural keys, try to retrieve it from the database.
"""
obj = Model(**data)
if (obj.pk is None and hasattr(Model, 'natural_key') and
hasattr(Model._default_manager, 'get_by_natural_key')):
natural_key = obj.natural_key()
try:
obj.pk = Model._default_manager.db_manager(db).get_by_natural_key(*natural_key).pk
except Model.DoesNotExist:
pass
return obj
this causes loaddata to fail on second attempt when using natural keys and uuid primary keys as pk is set.
class FooManager(models.Manager):
def get_by_natural_key(self, name):
return self.get(name=name)
class Foo(DateTimeMixin, models.Model):
id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False)
name = models.CharField(max_length=100, unique=True)
objects = FooManager()
def natural_key(self):
return (self.name,)
checking the primary key was not actually set seems to fix things:
def build_instance(Model, data, db):
obj = Model(**data)
pk = data.get(Model._meta.pk.name, None)
if (pk is None and hasattr(Model, 'natural_key') and
hasattr(Model._default_manager, 'get_by_natural_key')):
natural_key = obj.natural_key()
try:
obj.pk = Model._default_manager.db_manager(db).get_by_natural_key(*natural_key).pk
except Model.DoesNotExist:
pass
return obj
import django.core.serializers.base
django.core.serializers.base.build_instance = build_instance
Change History (11)
comment:1 by , 8 years ago
| Description: | modified (diff) |
|---|---|
| Summary: | deserialisers ignore natural keys when primary key has a default value → deserializers ignore natural keys when primary key has a default value |
| Triage Stage: | Unreviewed → Accepted |
comment:2 by , 8 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:3 by , 8 years ago
comment:4 by , 8 years ago
When a models primary key has default set, and a fixture is loaded that does not have a pk attribute, then the serialisation code will refuse to update the existing instance and instead try to create a new record in the db.
When the primary key attribute has a default value - such as models using UUID primary keys - the model will fill the pk attribute with that at instantiation time, and the check in build_instance will see that default value and refuse to goto the database to fetch the existing instance.
This is against the expectations set in the docs, that when the model/manager implement the natural keys interfaces and the primary key is omitted, the framework will update the existing instance when deserialising.
comment:6 by , 8 years ago
Can you please list the correct steps to reproduce this bug ? or please tell us how you did?
comment:7 by , 8 years ago
define the above models then try and import the following fixture using loaddata.
- model: myapp.Foo
fields:
name: foobar
if you try and import a second time it will create a duplicate instance instead of using the existing one.
comment:8 by , 8 years ago
| Has patch: | set |
|---|
comment:9 by , 8 years ago
| Patch needs improvement: | set |
|---|
Hello,
I cannot reproduce this bug, i tried few ways, but got no error during
loaddata.May my understanding is not clear, but how do we want to get
natural_keyifFoomodel'sprimary keyexists?From
build_instancedocstring:Clarify bug's space for me, please. I wanna resolve it. Thanks.