Skip to content

Commit 7145697

Browse files
committed
Updated preference init to skip redundant write
Persistable stores Preferable preferences in a YAML-serialized column. The initialize_preference_defaults callback ran on every load and reassigned the preferences attribute even when the merged defaults already matched the stored value. That assignment switches the attribute to ActiveModel's "from user" state, whose reads round-trip the hash through YAML on every access, while "from database" reads only load it. Hot paths that read calculator preferences per record paid this on every request. Shipping-rate estimation, for example, reads each shipping method's calculator preferences and re-dumped YAML once per method per estimation. Guard the assignment so it only runs when the merge actually changes the stored preferences. Unchanged records stay on the cheaper read path and defaults are still backfilled when missing.
1 parent 8d781ac commit 7145697

2 files changed

Lines changed: 15 additions & 3 deletions

File tree

core/app/models/concerns/spree/preferences/persistable.rb

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,10 @@ module Persistable
2020
private
2121

2222
def initialize_preference_defaults
23-
if has_attribute?(:preferences)
24-
self.preferences = default_preferences.merge(preferences)
25-
end
23+
return unless has_attribute?(:preferences)
24+
25+
merged = default_preferences.merge(preferences)
26+
self.preferences = merged unless merged == preferences
2627
end
2728
end
2829
end

core/spec/models/spree/preferences/preferable_spec.rb

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,17 @@ class ComplexPreferableClass
387387
end
388388
end
389389

390+
it "does not re-serialize unchanged preferences when read after load" do
391+
loaded = PrefTest.find(@pt.id)
392+
393+
# NOTE: assigning preferences in after_initialize (even an identical value) flips
394+
# the attribute to "from user" state, so every later read re-dumps YAML. Skipping
395+
# the redundant assignment keeps reads on the cheaper "from database" path.
396+
expect(Psych).not_to receive(:safe_dump)
397+
398+
loaded.preferences
399+
end
400+
390401
it "clear preferences when record is deleted" do
391402
@pt.save!
392403
@pt.preferred_pref_test_pref = "lmn"

0 commit comments

Comments
 (0)