MONGOID-5274: Issues with #touch method

[johnnyshields]

MONGOID-5274

This PR only contains specs which demonstrate the issues.

Please run spec/mongoid/touchable_spec.rb

1. On referenced relations with touch: false, calling #touch on document A will touch document B.

2. On embedded relations with touch: false, calling #touch on child document touches its parent (the root cause is different than #2)

3. On embedded relations with touch: true, there is strange behavior where in some cases calling #save! and #destroy on the child does not touch its parent. This works correctly on referenced relations.

I have raised a PR here #5045 that refactors the Touchable module. AFAIK my changes do not break/alter any behavior, but they eliminate unnecessary queries and make the code much easier to read, so I recommend you consider to try debugging with my #5045 version.

Note re: #2. When you investigate it, I believe you will find that on embedded childs, we need a way to know if the association with the parent has touch: true/false. Note that we have the following:

child._parent #=> returns parent object itself
child._association #=> returns the EmbedsMany/EmbedsOne association of the parent

# suggest we add the following:
child._association_to_parent #=> returns the EmbeddedIn association

(_association should probably be renamed to _association_from_parent)

In order to permanently fix the issue, we should add specs to cover all possible relation cases:

• embeds_one (calling #touch on parent)
• embeds_one -> embedded_in (calling #touch on child)
• embeds_many (calling #touch on parent)
• referenced has_one
• referenced has_many
• referenced habtm

[johnnyshields]

@Neilshweky this seems like the kind of investigation you are very good at.

[Neilshweky]

@Neilshweky this seems like the kind of investigation you are very good at.

Hey Johnny! Would have to see what's on the docket and how soon I can take a look at it! I definitely won't be looking at it until at least Monday though... Thanks for the PR, it's super helpful!

[Neilshweky]

Hey Johnny! I am currently investigating this. A few things...

• I have confirmed the first two bugs, I have not looked at the third one yet.
• This PR is a little bit complicated, so I hope you don't mind if I put up a PR that more succinctly illustrates the specific problem.
• With respect to your question about getting the embedded_in association, you can always do something like floor._association.inverse_association

Hope that helps! I will @ you when I get that other PR up. I'm also updating the JIRA ticket to give a little more info. Thanks for identifying this bug!

[johnnyshields]

@Neilshweky I'm pretty sure floor._association.inverse_association didn't work, but maybe I missed it...

You're certainly welcome to refactor as much as you want, it was just to demonstrate the issues.

[johnnyshields]

Be sure to look at #5045 by the way...

[Neilshweky]

@Neilshweky I'm pretty sure floor._association.inverse_association didn't work, but maybe I missed it...

You're certainly welcome to refactor as much as you want, it was just to demonstrate the issues.

Okay, so after a lot more investigation I think I have a grasp of the problem here.

1. Your first issue is not actually a bug. After fixing the comments on your PR, the tests for touch: false on referenced association do work.
2. The second issue does look like a bug, and I have confirmed it with a test.
3. This issue, and the reason why floor._association.inverse_association doesn't work are one and the same. Basically, the inverse association cannot be found on the embedded class, and I suspect it has to do with MONGOID-4882.

[Neilshweky]

Hey @johnnyshields check out this PR #5204

[johnnyshields]

@Neilshweky re:

Your first issue is not actually a bug. After fixing the comments on your PR, the tests for touch: false on referenced association do work.

Which comment(s) are you specifically referring to?

[Neilshweky]

@Neilshweky re:

Your first issue is not actually a bug. After fixing the comments on your PR, the tests for touch: false on referenced association do work.

Which comment(s) are you specifically referring to?

I put some comments on your PR where you had code like:

building_updated = entrance.updated_at

Which is clearly a bug. Fixing those resolved the first issue.

[p-mongo]

@johnnyshields now that #5204 is merged, can you please update this PR and ideally separate it into multiple PRs that each covers a single issue?

[Neilshweky]

@johnnyshields now that #5204 is merged, can you please update this PR and ideally separate it into multiple PRs that each covers a single issue?

Actually I think this one can be closed. #5204 was just a simplification of this PR

[p-mongo]

Okay, that sounds good.