Fix potential scope leakage in ThroughAssociationPatch

[oehlschl]

Fixes #166

Problem

This PR fixes bug where scopes from one has_many :through association leak into queries for other through associations when the owner model has a default_scope with an ORDER BY clause. This bug was introduced in version 4.2.1 with the ThroughAssociationPatch and affects all versions through the current (5.4.0).

The ThroughAssociationPatch module enables auto-including through associations after the through association has been loaded. However, when the owner model has a default_scope, Rails' association preloader incorrectly reuses scoped queries across different through associations, causing scope leakage. This is arguably a Rails bug in the association preloader, but we need to work around it in Goldiloader by detecting when this pattern exists and disabling the optimization in those cases.

Fix

This PR adds a has_scope_leakage_risk? check to the ThroughAssociationPatch that:

• Checks if owner model has default_scope with ORDER BY
• Determines if multiple scoped through associations exist via the same join table
• Returns false from auto_include? when that case exists, preventing scope leakage

The fix uses instance-level caching for performance, ensuring the detection logic only runs once per association.

And again, this may be more of a "workaround" then a fix, as the function of this PR is to disable the optimzation added in 4.2.1 when this buggy case exists.

Changes

• Added has_scope_leakage_risk? detection method to ThroughAssociationPatch
• Added helper methods: resolve_check_class, class_has_order_default_scope?, has_other_scoped_through_associations?
• Added instance-level caching for performance (@order_scope_cache, @scoped_through_cache)
• Added tests covering:
  • Scope leakage prevention (primary bug fix verification)
  • Auto-include behavior when buggy case exists
  • Correct results from scoped through associations
  • Correct results when accessing multiple associations in various orders
  • Behavior across multiple owner instances

Testing

Tests cases show that Goldiloader:

1. Doesn't leak scopes between through associations - Verifies the core bug is fixed
2. Doesn't auto-include through associations when scope leakage risk exists - Verifies optimization is disabled
3. Returns correct results for scoped through associations - Verifies scoped associations still work
4. Returns correct results when accessing multiple scoped associations - Tests various access patterns
5. Returns correct results when accessing unscoped before scoped associations - Tests reverse order
6. Returns correct results for both sections - Verifies fix works across multiple instances

Conclusion

The fix does add some complexity but should be performant and backwards compatible.The fix ensures these apps with these cases get correct query results while maintaining Goldiloader's performance benefits where safe to do so.

[jturkel]

Thanks for the PR @oehlschl. Apologies for the delay getting around to a review.

This sounds like a pretty nasty bug! I spent some time trying to narrow down the Rails bug to ensure we can workaround it properly in goldiloader and file a Rails bug. I was able to trigger it using this gist with a filtered and unfiltered through association. Is this a more general issue of Rails eager loading ignoring any through association scope customizations if there's a similar through association without any scope customizations that's loaded first?

[oehlschl]

Hi @jturkel; apologies for the delayed response as well.

Is this a more general issue of Rails eager loading ignoring any through association scope customizations if there's a similar through association without any scope customizations that's loaded first?

I would believe that. I didn't go so far as to file the Rails bug yet, as it took a while just to get this far, so if you feel like you have a better handle on it, please feel free, or let me know what you need.

[jturkel]

I filed rails/rails#56110. Let's see what the Rails maintainers say and then we can workaround the issue as needed.