Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 32 additions & 10 deletions lib/coaster/serialized_properties.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,26 @@ def _write_sprop_value(key, value, apply_setter: true)
setting = self.class.serialized_property_setting(key)
return unless setting
col = setting[:column]
send("#{col}_will_change!")
hsh = send(col)
if value.nil?
old_value = hsh[key.to_s]

new_value = if value.nil?
nil
else
apply_setter && setting[:setter] ? setting[:setter].call(value) : value
end

# Skip dirty marking if value hasn't changed
return old_value if old_value == new_value

send("#{col}_will_change!")

if new_value.nil?
hsh.delete(key.to_s)
else
value = setting[:setter].call(value) if apply_setter && setting[:setter]
hsh[key.to_s] = value
hsh[key.to_s] = new_value
end
value
new_value
Comment on lines 30 to +49

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

이 로직은 _define_serialized_property 메서드 내에 정의된 setter (326-345행)의 로직과 거의 동일하여 코드 중복이 발생합니다. 유지보수성을 높이기 위해 이 공통 로직을 별도의 private 헬퍼 메서드로 추출하는 것을 고려해 보세요.

예를 들어, 다음과 같은 헬퍼 메서드를 if defined?(ActiveRecord::Base) && base < ActiveRecord::Base 블록 내에 정의할 수 있습니다.

private

def _update_sprop_if_changed(column, key, hsh, old_value, new_value)
  return old_value if old_value == new_value

  send("#{column}_will_change!")

  if new_value.nil?
    hsh.delete(key.to_s)
  else
    hsh[key.to_s] = new_value
  end
  new_value
end

그런 다음 _write_sprop_value_define_serialized_property의 setter에서 이 헬퍼 메서드를 호출하여 중복을 제거할 수 있습니다. 이렇게 하면 코드가 더 간결해지고 향후 수정이 필요할 때 한 곳만 변경하면 됩니다.

end

def write_attribute(attr_name, value)
Expand Down Expand Up @@ -312,15 +323,26 @@ def _define_serialized_property(serialize_column, key, getter: nil, setter: nil,
if is_active_record
define_method "#{key}_without_callback=".to_sym do |val|
col = serialize_column
send("#{col}_will_change!")
hsh = send(col)
if val.nil?
old_value = hsh[key.to_s]

new_value = if val.nil?
nil
else
setter ? setter.call(val) : val
end

# Skip dirty marking if value hasn't changed
return old_value if old_value == new_value

send("#{col}_will_change!")

if new_value.nil?
hsh.delete(key.to_s)
else
val = setter.call(val) if setter
hsh[key.to_s] = val
hsh[key.to_s] = new_value
end
val
new_value
end
else
define_method "#{key}_without_callback=".to_sym do |val|
Expand Down
108 changes: 108 additions & 0 deletions test/test_serialized_property.rb
Original file line number Diff line number Diff line change
Expand Up @@ -372,5 +372,113 @@ def test_nil_value_with_getter_setter
assert_nil student.encrypted_value
assert_nil student.data['encrypted_value']
end

# Tests for dirty tracking optimization (skip dirty marking when value unchanged)
def test_same_value_setter_does_not_mark_dirty
user = User.create(name: 'test_same_value')
user.simple = 'initial_value'
user.save!

# Reload to clear dirty state
user.reload
assert_equal false, user.changed?

# Set same value - should NOT mark dirty
user.simple = 'initial_value'
assert_equal false, user.simple_changed?, 'Setting same value should not mark property as changed'
assert_equal false, user.changed?, 'Setting same value should not mark record as changed'
end

def test_different_value_setter_marks_dirty
user = User.create(name: 'test_different_value')
user.simple = 'initial_value'
user.save!
user.reload

# Set different value - should mark dirty
user.simple = 'new_value'
assert_equal true, user.simple_changed?, 'Setting different value should mark property as changed'
assert_equal true, user.changed?, 'Setting different value should mark record as changed'
assert_equal ['initial_value', 'new_value'], user.simple_change
end

def test_nil_to_value_marks_dirty
user = User.create(name: 'test_nil_to_value')
user.save!
user.reload

assert_nil user.simple
user.simple = 'some_value'
assert_equal true, user.simple_changed?
assert_equal [nil, 'some_value'], user.simple_change
end

def test_value_to_nil_marks_dirty
user = User.create(name: 'test_value_to_nil')
user.simple = 'some_value'
user.save!
user.reload

user.simple = nil
assert_equal true, user.simple_changed?
assert_equal ['some_value', nil], user.simple_change
end

def test_nil_to_nil_does_not_mark_dirty
user = User.create(name: 'test_nil_to_nil')
user.save!
user.reload

assert_nil user.simple
user.simple = nil
assert_equal false, user.simple_changed?, 'Setting nil to nil should not mark as changed'
assert_equal false, user.changed?
end

def test_same_value_via_write_attribute_does_not_mark_dirty
user = User.create(name: 'test_write_attr_same')
user.write_attribute(:simple, 'initial')
user.save!
user.reload

user.write_attribute(:simple, 'initial')
assert_equal false, user.simple_changed?
assert_equal false, user.changed?
end

def test_same_value_via_bracket_accessor_does_not_mark_dirty
user = User.create(name: 'test_bracket_same')
user[:simple] = 'initial'
user.save!
user.reload

user[:simple] = 'initial'
assert_equal false, user.simple_changed?
assert_equal false, user.changed?
end

def test_same_value_with_setter_proc_does_not_mark_dirty
student = Student.create(name: 'test_setter_same')
student.uppercase_name = 'hello' # Stored as 'HELLO'
student.save!
student.reload

# Setting same raw input - setter transforms to same value
student.uppercase_name = 'hello'
assert_equal false, student.uppercase_name_changed?, 'Same value after setter transform should not mark dirty'
assert_equal false, student.changed?
end

def test_different_case_with_setter_proc_marks_dirty
student = Student.create(name: 'test_setter_diff')
student.uppercase_name = 'hello' # Stored as 'HELLO'
student.save!
student.reload

# Setting different value
student.uppercase_name = 'world' # Stored as 'WORLD'
assert_equal true, student.uppercase_name_changed?
assert_equal ['HELLO', 'WORLD'], student.uppercase_name_change
end
end
end