Skip to content

Commit f195e03

Browse files
jorgemanrubiabyroot
authored andcommitted
Freeze casted values instead of original values in database queries
This deals with a problem introduced in #7743ab95b8e15581f432206245c691434a3993d1a751b9d451170956d59457a9R8 that was preventing query `Class` serialized attributes. Duplicating the original `Class` argument generates an anonymous class that can't be serialized as YAML. This change makes query attributes hasheable based on their frozen casted values to prevent the problem. This solution is based on an idea by @matthewd from rails#47338 (comment).
1 parent 912096d commit f195e03

File tree

6 files changed

+43
-7
lines changed

6 files changed

+43
-7
lines changed

activemodel/lib/active_model/attribute.rb

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,13 +115,17 @@ def has_been_read?
115115
def ==(other)
116116
self.class == other.class &&
117117
name == other.name &&
118-
value_before_type_cast == other.value_before_type_cast &&
118+
identifier_value == other.identifier_value &&
119119
type == other.type
120120
end
121121
alias eql? ==
122122

123123
def hash
124-
[self.class, name, value_before_type_cast, type].hash
124+
[self.class, name, identifier_value, type].hash
125+
end
126+
127+
def identifier_value
128+
value_before_type_cast
125129
end
126130

127131
def init_with(coder)

activemodel/lib/active_model/type/helpers/mutable.rb

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,6 @@ module ActiveModel
44
module Type
55
module Helpers # :nodoc: all
66
module Mutable
7-
def immutable_value(value)
8-
value.deep_dup.freeze
9-
end
10-
117
def cast(value)
128
deserialize(serialize(value))
139
end

activerecord/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
* Fix error when querying serialized Class attributes.
2+
3+
*Jorge Manrubia*
4+
15
* Add support for `Array#intersect?` to `ActiveRecord::Relation`.
26

37
`Array#intersect?` is only available on Ruby 3.1 or later.

activerecord/lib/active_record/relation/predicate_builder.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ def build(attribute, value, operator = nil)
6666

6767
def build_bind_attribute(column_name, value)
6868
type = table.type(column_name)
69-
Relation::QueryAttribute.new(column_name, type.immutable_value(value), type)
69+
Relation::QueryAttribute.new(column_name, value, type)
7070
end
7171

7272
def resolve_arel_attribute(table_name, column_name, &block)

activerecord/lib/active_record/relation/query_attribute.rb

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,24 @@ def unboundable?
3636
@_unboundable
3737
end
3838

39+
def identifier_value
40+
if unboundable?
41+
super
42+
else
43+
# Override to define equality based on frozen casted values to prevent caching
44+
# issues with mutable database query values.
45+
value_for_database
46+
end
47+
end
48+
3949
private
4050
def infinity?(value)
4151
value.respond_to?(:infinite?) && value.infinite?
4252
end
53+
54+
def _value_for_database
55+
super.deep_dup.freeze
56+
end
4357
end
4458
end
4559
end

activerecord/test/cases/serialized_attribute_test.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@
99
class SerializedAttributeTest < ActiveRecord::TestCase
1010
def setup
1111
ActiveRecord.use_yaml_unsafe_load = true
12+
@yaml_column_permitted_classes_default = ActiveRecord.yaml_column_permitted_classes
13+
end
14+
15+
def teardown
16+
ActiveRecord.yaml_column_permitted_classes = @yaml_column_permitted_classes_default
1217
end
1318

1419
fixtures :topics, :posts
@@ -23,6 +28,10 @@ class ImportantTopic < Topic
2328
serialize :important, type: Hash
2429
end
2530

31+
class ClassifiedTopic < Topic
32+
serialize :important, type: Class
33+
end
34+
2635
teardown do
2736
Topic.serialize("content")
2837
end
@@ -166,6 +175,14 @@ def test_serialized_string_attribute
166175
assert_equal(myobj, topic.content)
167176
end
168177

178+
def test_serialized_class_attribute
179+
ActiveRecord.yaml_column_permitted_classes += [Class]
180+
181+
topic = ClassifiedTopic.create(important: Symbol).reload
182+
assert_equal(Symbol, topic.important)
183+
assert_not_empty ClassifiedTopic.where(important: Symbol)
184+
end
185+
169186
def test_nil_serialized_attribute_without_class_constraint
170187
topic = Topic.new
171188
assert_nil topic.content
@@ -551,6 +568,7 @@ def test_serialized_attribute_works_under_concurrent_initial_access
551568

552569
class SerializedAttributeTestWithYamlSafeLoad < SerializedAttributeTest
553570
def setup
571+
super
554572
ActiveRecord.use_yaml_unsafe_load = false
555573
end
556574

0 commit comments

Comments
 (0)