As a patch in case my example was unclear -- this seems to also work, would
this solve the issue simpler? Or could this be a performance issue/gap in
the specs?
diff --git a/lib/sequel/model/associations.rb
b/lib/sequel/model/associations.rb
index bf713d988..ac500ea2a 100644
--- a/lib/sequel/model/associations.rb
+++ b/lib/sequel/model/associations.rb
@@ -2600,11 +2600,11 @@ module Sequel
no_set_assoc = !set_associated_object_if_same?
return if a && a == o && no_set_assoc
run_association_callbacks(opts, :before_set, o)
- remove_reciprocal_object(opts, a) if a && no_set_assoc
+ remove_reciprocal_object(opts, a) if a
+ add_reciprocal_object(opts, o) if o
# Allow calling private _setter method
send(opts[:_setter_method], o)
associations[opts[:name]] = o
- add_reciprocal_object(opts, o) if o
run_association_callbacks(opts, :after_set, o)
o
end
On Thursday, December 13, 2018 at 1:34:00 PM UTC-8, Ben Alavi wrote:
>
> Actually... do you know why the reciprocal object isn't just replaced
> *before* calling the setter?
>
> This works for my example and passes specs:
>
> remove_reciprocal_object(opts, a) if a
> add_reciprocal_object(opts, o) if o
> # Allow calling private _setter method
> send(opts[:_setter_method], o)
>
>
> On Thursday, December 13, 2018 at 12:43:42 PM UTC-8, Ben Alavi wrote:
>>
>> Oh yep just got there!
>>
>> So I think I see what you mean -- if a and o already had the same
>> reciprocal association set you could avoid the same issue by checking for
>> that as well...something like:
>>
>> remove_reciprocal_object(opts, a) if a && a.associations[opts.reciprocal]
>> != self
>>
>> ...and then you could still check no_set_assoc if that is indeed any
>> faster as a performance improvement for the case where a == o -- I'm not
>> sure if it would be any faster to justify the extra logic
>>
>> remove_reciprocal_object(opts, a) if a && (no_set_assoc ||
>> a.associations[opts.reciprocal] != self)
>>
>> ...and then I guess it might make sense to do the same thing when adding
>> the reciprocal object (don't add it if it's already set)
>>
>> add_reciprocal_object(opts, o) if o && o.associations[opts.reciprocal] !=
>> self
>>
>> ...again I'm not sure what the performance gain (if any) would be to skip
>> the extra add
>>
>> On Thursday, December 13, 2018 at 12:21:50 PM UTC-8, Jeremy Evans wrote:
>>>
>>> On Thursday, December 13, 2018 at 12:14:22 PM UTC-8, Ben Alavi wrote:
>>>>
>>>> That makes sense to me, although it looks like what should happen
>>>> before your change is that
>>>>
>>>>
>>>> https://github.com/jeremyevans/sequel/commit/de183ed7ac6ad8df5353f54be9579350977d1c16#diff-0cd793d48726eacb1a6cb8af71e125a4L2602
>>>>
>>>> removes the reciprocal object and then
>>>>
>>>> https://github.com/jeremyevans/sequel/commit/de183ed7ac6ad8df5353f54be9579350977d1c16#diff-0cd793d48726eacb1a6cb8af71e125a4L2606
>>>>
>>>> adds the same object back
>>>>
>>>> ...so I'm trying to figure out why that didn't happen.
>>>>
>>>
>>> It did happen, it's just the saving is in between those two lines of
>>> code.
>>>
>>> A more general approach would be to actually check the reciprocal
>>> association, and make no changes to the reciprocal association if the
>>> current object is the same as the reciprocal association. I'll look into
>>> that. It's probably slower, but should be more correct for other cases
>>> where the associations cache is manipulated directly.
>>>
>>> Thanks,
>>> Jeremy
>>>
>>
--
You received this message because you are subscribed to the Google Groups
"sequel-talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at https://groups.google.com/group/sequel-talk.
For more options, visit https://groups.google.com/d/optout.
