Serialization (and deserialization) of MTI models doesn't work with natural keys

[Denys Duchier]

I am using Django1.8. The exact error message is:

ValueError: Problem installing fixture '/home/denys/src/GSM/tmp/bug/person.json': "<Person: >" needs to have a value for field "user" before this many-to-many relationship can be used.

I have created a very small example to reproduce the problem; it is available on github at:

https://github.com/denys-duchier/django-loaddata-bug.git

to create the database, migrate, populate the database, then dumpdata, execute:

step1.bash

to recreate the database, migrate, and attempt the loaddata, execute:

step2.bash

that is when the error occurs.

[Tim Graham]

The issue seems to be caused by the fact that --natural-primary causes the Person's 'id' to be omitted in the fixture dump (since AbstractBaseUser defines natural_key()), but without it, there's no way to connect Person instances to the corresponding User.

[Denys Duchier]

ah... I see, the issue is in get_dump_object

[Denys Duchier]

I am thinking that when we have use_natural_foreign_keys and the pk is a relation, we want to recurse.

[Denys Duchier]

I have created a PR that takes a first stab at this problem:

​https://github.com/django/django/pull/4473

[Denys Duchier]

new PR against master:

​https://github.com/django/django/pull/4477

[Tim Graham]

Waiting for a response from the reporter on the pull request, but it seems to me the issue is that use_natural_primary_keys is not compatible with serializing multi-table inheritance models since the child models require the primary key to be linked to their parent. I'm not sure that adding a lot of complexity to essential "undo" the option for the affected models is worth it. Could we simply document the limitation?

[Levi Cameron]

As it stands right now, use_natural_primary_keys doesn't work with multi-table inheritance with or without use_natural_foreign_keys -- but there's no fundamental reason stopping this and the most recent patch at least allows it to work if both use_natural_primary_keys and use_natural_foreign_keys are enabled.

The patch changes serialization requires use_natural_primary_keys and use_natural_foreign_keys to both be set in the case of multi-table inheritance. Possible options where use_natural_primary_keys is set but use_natural_foreign_keys isn't:

1. use_natural_primary_keys implies use_natural_foreign_keys
   1. Always (this is what the patch did)
   2. If there are any multi-table inheritance models defined that have a natural key
   3. Only for references to models that are involved in any multi-table inheritance
      • May be a little surprising if you add an inherited model and it changes the dump for the parent model changes
2. use_natural_primary_keys does not imply use_natural_foreign_keys
   • dumps may break if load/dump order is not identical as the real PKs may not match (ie is likely to break)
   • If there are any multi-table inheritance models with natural keys then a warning can be emitted
3. use_natural_primary_keys is ignored for models that are involved in any multi-table inheritance and have a natural_key defined
   • May be a little surprising if you add an inherited model and it changes the dump for the parent model changes

Option 2 involves the least change and with the warning follows the principle of least surprise. It is an improvement over the current situation and is as backwards-compatible as possible.

If the patch was bought up to date with the latest django version, and it was changed to use option 2 (so use_natural_primary_keys did not turn on use_natural_foreign_keys but gave a warning if a likely problem was detected) then would this be acceptable?

(As an aside: if use_natural_primary_keys is set but use_natural_foreign_keys isn't then even for tables without multi-table inheritance it would seem that there is a sensitivity to the load/dump order in the referenced/referencing tables. The documentation at ​https://docs.djangoproject.com/en/dev/topics/serialization/ doesn't mention this anywhere)

[João Paulo Melo de Sampaio]

New PR on this:

​https://github.com/django/django/pull/7231

Context:

​https://groups.google.com/d/msg/django-developers/HdfvARojZeo/e37rpYI_FAAJ

[Tim Graham <timograham@…>]

In 74a575eb:

Fixed #24607 -- Serialized natural keys in multi-table inheritance models.

[Tim Graham <timograham@…>]

In 0595bca:

Fixed #27742 -- Reverted "Fixed #24607 -- Serialized natural keys in multi-table inheritance models."

This reverts commit 74a575eb7296fb04e1fc2bd4e3f68dee3c66ee0a as it causes
unexpected migrations and doesn't seem to be the best solution.

[Tim Graham <timograham@…>]

In eeb28f47:

[1.11.x] Fixed #27742 -- Reverted "Fixed #24607 -- Serialized natural keys in multi-table inheritance models."

This reverts commit 74a575eb7296fb04e1fc2bd4e3f68dee3c66ee0a as it causes
unexpected migrations and doesn't seem to be the best solution.

Backport of 0595bca221825c0c6bd572a32f3bf9eff7069328 from master

[Tim Graham]

Reopening per discussion on #27742.

[Denys Duchier]

new PR: ​https://github.com/django/django/pull/8370

I slightly reworked @jpmelos earlier PR #7231 to hopefully avoid the problem with spurious migrations. When a relation with parent_link=True is used as a pk then, when serializing with natural primary keys, the field must be serialized, even when marked with serialize=False.

[Wai Yi Anthony Leung]

I'm reviewing the latest PR on ​https://github.com/django/django/pull/8370

[Anthony King]

posting into here for convenience:

For people that need something working now, I've worked this PR into a standalone serializer. It works for our use case, however I can't guarantee it will work in all circumstances.

​https://gist.github.com/cybojenix/2476434c513170d2f267cb588a8d645b

[Tim Graham <timograham@…>]

In cb7860cc:

Fixed #24607 -- Serialized natural keys in multi-table inheritance models.

Thanks João Paulo Melo de Sampaio for the test.