New :fallback option to handle nil for singular associations#6149
New :fallback option to handle nil for singular associations#6149cperezabo wants to merge 7 commits into
Conversation
Introduces a :fallback option on belongs_to, has_one, and embeds_one that takes a Proc invoked whenever the association resolves to nil, returning a null object stand-in. Direct assignment of an instance that does not match the association's class is treated as nil so the null object never reaches the database.
|
Wow! Nice feature! 🚀 |
jamis
left a comment
There was a problem hiding this comment.
@cperezabo thank you for this PR -- it looks like a convenient feature.
However, there are some subtle potential side-effects that I'm a bit worried about:
- Associations with
dependent:set may cause problems. If you look at any of the_dependent_*helper methods (inassociation/depending.rb, you'll see they callsend(association.name)with nowithout_autobuildwrapper. This leaves them vulnerable to returning the fallback object, if one is defined---which could result inNoMethodErrorbeing raised whendestroyornullify(etc.) is called on that fallback. - Relying on
association.relation_classis a bit fraught. If you look at the comment inRelatable#relation_class, you'll seecalling this method on a polymorphic association will generally fail with a NameError or produce misleading results. Polymorphic associations may need to be treated specially in the guard you added indefine_setter!. - Nested attributes (specifically in
association/nested/one.rb) callparent.send(association.name), which may return the fallback object. Looking in thebuildmethod in that file, there are multiple opportunities forNoMethodErrorexceptions to be raised when processing a nested attribute update.
| end | ||
|
|
||
| if @options.key?(:autobuild) | ||
| raise Errors::InvalidRelationOption.new(@owner_class, name, :fallback, self.class::VALID_OPTIONS) |
There was a problem hiding this comment.
InvalidRelationOption is mostly intended for reporting an option that is not recognized. It might be better to simply raise ArgumentError here? Because the message with this exception won't say anything about :autobuild being invalid when :fallback is specified, which is the actual issue here.
The _dependent_*! helpers read the association with a bare send(name), so an association with a :fallback returned the null object instead of nil when no real document existed. Destroying the owner then raised NoMethodError (destroy/delete_all/nullify) or a false DeleteRestriction and aborted the destroy (restrict_with_*). Read through without_autobuild in all five helpers so the cascade sees the real value (nil when absent), matching how the rest of Mongoid's internals read associations.
The association setter guarded the :fallback null object with object.is_a?(relation_class). On a polymorphic belongs_to, relation_class cannot resolve a target class from the association name and raised NameError on assignment. Check against Mongoid::Document for polymorphic associations (any document is a valid target) and keep the relation_class check for the rest. A non-document assignment such as the null object is treated as nil so it never reaches the database.
Association::Nested::One#build reads the existing association with parent.send(name) to decide whether to update, replace, or delete the nested document. With a :fallback that returned the null object instead of nil, so build never took the replace branch and called document methods (_id, using_object_ids?, ...) on the null object, raising NoMethodError. When the association has a :fallback, read the real value through without_autobuild so build sees nil and builds the nested document. Autobuilding associations are left untouched.
Combining :fallback with :autobuild previously raised InvalidRelationOption, whose message lists the valid options and does not explain that the real problem is the combination. Raise ArgumentError with a message naming both options instead, matching the ArgumentError already raised for a non-callable :fallback.
serializable_hash serializes included associations through serialize_relations, which read the association with a bare send(name) outside the without_autobuild block that already wraps field serialization. With a :fallback that returned the null object, and serializable_hash(include: ...) then called serializable_hash on it, raising NoMethodError. When the association has a :fallback, read the value through without_autobuild so a nil association is omitted from the output. Autobuilding associations are left untouched.
The counter cache after_create/after_update/before_destroy callbacks read the parent with __send__(name) to adjust its counter. With a :fallback that returned the null object when no parent was set, and the callbacks then indexed into it (record[cache_column]), raising NoMethodError on create, update, or destroy. Read through without_autobuild so the callbacks operate on the real parent and no-op when there is none, matching how the cascade helpers in Depending read associations.
|
Hi @jamis, I've addressed each of your points. I also scanned the repository with Claude to look for any other spots with the same problem, and it found two — I've added tests for those as well. There's a commit per point so they're easy to review. You can squash the branch or leave it as is; I don't mind either way! Just to clarify, I always follow the TDD approach, so the tests were added one by one, along with the production code. |
Hi folks, this PR adds a
:fallbackoption tobelongs_to,has_one, andembeds_one.You hand it a Proc, and whenever the association would be
nil, Mongoid returns whatever the Proc gives back (a null object).Works the same on
has_oneandembeds_one.Things worth knowing:
nil— whether via setter or constructor.:autobuild(they want opposite things). RaisesInvalidRelationOption.