Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Multiple fixes for cascade=True save issues #2615

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

Open
nickfrev wants to merge 9 commits into MongoEngine:master
base: master
Choose a base branch
Loading
from nickfrev:save-with-cascade-include-nested

Conversation

Copy link

@nickfrev nickfrev commented Jan 7, 2022
edited
Loading

Fixes multiple issues when cascade=True in the Document save method:

  • When a ReferenceField is holding an unsaved object you no longer get a ValidationError. The unsaved object is just saved as one would expect.
  • When a ReferenceField is embedded in a ComplexBaseField or any number of nested ComplexBaseField it will be saved.

Modifications:

  • The save method has been edited to be more modular and future friendly.
    • A new BaseField class has been added, SaveableBaseField, which adds a save method to the baseField class.
    • ComplexBaseField now inherits from SaveableBaseField.
    • During a cascade save any SaveableBaseField will run it's coresponding save method. This removes the need to hardcode the saveable field classes as done currently: mongoengine/document.py line 565
    • This allows for developers to easily create their own "saveable" fields.
  • Moved the save logic for ComplexBaseFields out of the Document class and into the ComplexBaseField class to align with the new SaveableBaseField design.
  • Added _save_place_holder method to the Document class which is a helper method for saving that inserts a totally empty (except id) placeholder object into the db. This is what allows that object to get an id before its parent validates.

Below is some code (based on new pytest test_cascade_save_with_cycles) to demonstrate how cascade saves can now work:

class Object1(Document):
 name = StringField()
 oject2_reference = ReferenceField('Object2')
 oject2_list = ListField(ReferenceField('Object2'))
class Object2(Document):
 name = StringField()
 oject1_reference = ReferenceField(Object1)
 oject1_list = ListField(ReferenceField(Object1))
obj_1 = Object1(name="Test Object 1")
obj_2 = Object2(name="Test Object 2")
# Create a cyclic reference nightmare
obj_2.oject1_reference = obj_1
obj_2.oject1_list = [obj_1]
obj_1.oject2_reference = obj_2
obj_1.oject2_list = [obj_2]
# Save only once
obj_1.save(cascade=True)

Personal Notes:

It is my personal opinion that these changes make the code look cleaner as you don't have .save() calls all over the place and you can also avoid the habit of over-saving documents (which I have been guilty of due to a lack of trust in cascade saving). This also removes the burden from the developer to keep track of what has and has not been saved.

I also believe this makes the code look less like ORM code as you only require one save at the end of all data creation/manipulation for complex objects.

I feel a bit shaky about how I implimented the _is_saving flag that is now used during a cascade save. This flag must be set to false when the save method has completed (either regularly or if there was an error thrown). I acomplished this with a wrapper try-catch and the use of a finally clause.

Save the children documents first to avoid the issue where a parent cannot save due to having new children documents.
- Created a new base field (SaveableBaseField) which allows a field to marked as savable during a cascade save.
- Each SaveableBaseField defines a save method which describes how it will deal with a cascade save call this allows lists, dicts, and maps to be effected during a cascade save.
- Added an _is_saving flag during Document save to avoid saving a document that is already in the process of being saved. (Caused if there is a circular reference.)
- Allows for users to cascade save unsaved documents even if a cycle exists.
Copy link

can we get an update, if this will be merged and if so, when?

ivke-99 reacted with thumbs up emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

AltStyle によって変換されたページ (->オリジナル) /