Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deserialisation of child models should not validate ParentalKey #127

Open
gasman opened this issue Jul 13, 2020 · 0 comments
Open

Deserialisation of child models should not validate ParentalKey #127

gasman opened this issue Jul 13, 2020 · 0 comments

Comments

@gasman
Copy link
Contributor

gasman commented Jul 13, 2020

On a fresh bakerydemo install:

from bakerydemo.blog.models import BlogPage

BlogPage.from_json('''{
  "pk": 62,
  "blog_person_relationship": [
    {
      "pk": 10,
      "sort_order": 0,
      "page": 62,
      "people": 3
    }
  ]
}''').blog_person_relationship.all()

# returns: [<BlogPeopleRelationship: BlogPeopleRelationship object (10)>]

However:

BlogPage.from_json('''{
  "pk": 999,
  "blog_person_relationship": [
    {
      "pk": 10,
      "sort_order": 0,
      "page": 999,
      "people": 3
    }
  ]
}''').blog_person_relationship.all()

# returns: []

This is because, when the blog_person_relationship child object is deserialised, foreign keys including the ParentalKey to Page are integrity-checked according to the on_delete property. Since there is no existing Page in the database with ID 999, and the ParentalKey has on_delete=CASCADE, the deserialisation logic decides that the child object should not exist, and drops it from the relation. Clearly, this is wrong - it should be able to recognise from the fact that it's a ParentalKey that the parent object is being created at the same time as the child object, and thus skip over the check for it existing in the database.

For a ParentalKey with on_delete=PROTECT, this has a different effect: since the dangling-foreign-key check does not implement a case for PROTECT, deserialisation fails with "can't currently handle on_delete types other than CASCADE, SET_NULL and DO_NOTHING". (For a legitimate dangling foreign key, this would not be entirely a bad thing, since the logical outcome would be an IntegrityError anyway; however, in this case we ought to be skipping that check entirely.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant