-
Type: Improvement
-
Resolution: Unresolved
-
Priority: Minor - P4
-
None
-
Affects Version/s: None
-
Component/s: Associations
-
None
- See MONGOID-5297 for companion ticket re: embedded associations
-
- Originally a comment on https://github.com/mongodb/mongoid/pull/5225 (
MONGOID-4484) where Oleg documented the current (counter-intuitive) behavior extensively.
- Originally a comment on https://github.com/mongodb/mongoid/pull/5225 (
First principles
As an app implementer, I want to perform DB persistence as few times as possible in my code (e.g. for any controller action, etc.) I want to have full control over when objects persist (i.e. by calling save!), and catch errors/rollback etc. as well.
Current state
Assigning an association (=, <<, push, etc.) triggers an automatic immediate save of both the document being assigned and the base document. This save happens irrespective of / independently from the `autosave` option (and should not be confused with `autosave`)
This behavior is problematic for many reasons:
- It is non-obvious, i.e. violates the Principle of Least Surprise. We require lengthy documentation to explain it; users have filed bugs because they didn't understand/expect it.
- It violates the Single Responsibility Principle; assignment methods are expected to assign in-Ruby-memory only, and save! should be used for persistence.
- Because such assignment is typically performed in the middle the app controller flow, it causes objects to be persisted in an half-completed, intermediate state.
- Because one usually calls save! at the end of the control flow, any persistence in the middle of the flow is an unnecessary performance hit, i.e. a synchronous roundtrip to the database.
- AFAIK the save of the assigned object triggers callbacks, while the save of the base object is done as an atomic operation, which further compounds the inconsistency/confusion. (I may be wrong on this point; need to double-check.)
As an illustrative example:
# in BandsController#create action 1 band = Band.new(some_attributes) # possibly saves here? 2 band.name = "Pink Floyd" 3 album = Album.new("Dark Side of the Moon") 4 band.albums << album # saves both band and album here 5 band.rating = 10 6 album.tracks = 13 7 album.save! # saves abum here 8 band.save! # saves band here
Any saving done on lines 1 or 4 is both an unnecessary performance hit. Moreover, suppose something fails before line 7/8, my band/album are in an indeterminate "partially persisted" state.
While first-time Mongoid devs might think autosaving is a convenience, as one's app grows in complexity, it quickly becomes an anti-pattern one wishes one had never (unwittingly) adopted.
Further Investigation
To help refine spec we should first test exactly which relations in which scenarios persist to DB. i.e.
- has_one, has_many, HABTM etc.
- accepts_nested_attributes
- autosave (not believed to have any effect, but should double-test)
Proposed Spec
- Assignment =, <<, push as well as clear, delete_if, reject, etc. should only mutate the state of Ruby's memory, and should not persist anything to the database.
- Calling save! on the base object should trigger the association assignment to be persisted to the database. The :autosave option on the association should determine whether the assigned object is also saved at this time.
- The change should be done with a feature flag ("persist_referenced_associations_on_assignment")
- has_one / has_many may be a special case where immediately persisting the foreign key on the inverse document is justified. Need to think more on this.
- is related to
-
MONGOID-5297 Do not immediately save embedded associations upon assignment
- Backlog
- related to
-
MONGOID-5364 assign_attributes to embedded child & deleting in the same association which itself is embedded produces bogus results
- Backlog