-
Notifications
You must be signed in to change notification settings - Fork 190
Description
Bug Summary
When calling find_by multiple times on an ActiveHash::Relation obtained via Course.all, the original Relation's conditions gets mutated, causing subsequent find_by calls to return incorrect results.
Environment
- Ruby: 3.3.4
- Rails: 7.x
- active_hash: 4.0.0
Steps to Reproduce
# ActiveHash model
class Course < ActiveHash::Base
self.data = [
{ id: 1, name: "Course A" },
{ id: 2, name: "Course B" },
{ id: 3, name: "Course C" }
]
end
# Reproduction code
courses = Course.all
courses.conditions #=> Conditions(@conditions=[])
courses.find_by(id: 1) #=> #<Course id: 1, name: "Course A"> ✅ Works
courses.conditions #=> Conditions(@conditions=[Condition(id: 1)]) ← Mutated!
courses.find_by(id: 2) #=> nil ❌ Should return #<Course id: 2>
courses.conditions #=> Conditions(@conditions=[Condition(id: 1), Condition(id: 2)])
courses.find_by(id: 1) #=> nil ❌ Was working before!
courses.all #=> [] ❌ EmptyExpected Behavior
Calling find_by multiple times should not mutate the original Relation's conditions.
courses = Course.all
courses.find_by(id: 1) #=> #<Course id: 1>
courses.find_by(id: 2) #=> #<Course id: 2>
courses.find_by(id: 1) #=> #<Course id: 1>
courses.conditions #=> Conditions(@conditions=[]) ← Should remain emptyAnalysis
After investigating, it appears that the spawn method in lib/active_hash/relation.rb passes conditions by reference:
def spawn
self.class.new(klass, all_records, conditions, order_values)
endfind_by internally calls where(options).first, and where calls spawn.where!. Even though spawn creates a new Relation, the conditions (ActiveHash::Relation::Conditions object) still references the same object as the original Relation. When where! calls conditions << Condition.new(...), it also mutates the original Relation's @conditions array.
Current Workaround
In our project, we are using the following monkey patch as a workaround:
# config/initializers/active_hash_patch.rb
module ActiveHashConditionsPatch
def deep_dup
self.class.new(conditions.dup)
end
end
ActiveHash::Relation::Conditions.prepend(ActiveHashConditionsPatch)
module ActiveHashRelationPatch
def spawn
self.class.new(klass, all_records, conditions.deep_dup, order_values.dup)
end
end
ActiveHash::Relation.prepend(ActiveHashRelationPatch)We have confirmed that this patch resolves the issue and produces the expected behavior.
If this approach seems appropriate, I would be happy to see it incorporated into the gem. However, I'm not deeply familiar with the internal design of this gem, so please let me know if I'm missing something important or if there's a specific reason for the current implementation.