From fde846060221f9d390c02d18f781d936c27a1896 Mon Sep 17 00:00:00 2001 From: Elia Schito Date: Mon, 16 Jan 2023 19:05:19 +0100 Subject: [PATCH] Allow storing static preferences using string class names This will make unnecessary to wrap these definitions under `.to_prepare` blocks. Additionally the error message for unexpected keys has improved and now lists both all the unexpected keys along with the expected keys, mentioning the name associated to the definition. Backports https://github.com/solidusio/solidus/pull/4858 to v3.1 --- core/lib/spree/core/engine.rb | 1 + .../preferences/static_model_preferences.rb | 35 +++++++++++++------ .../static_model_preferences_spec.rb | 23 +++++++++--- 3 files changed, 45 insertions(+), 14 deletions(-) diff --git a/core/lib/spree/core/engine.rb b/core/lib/spree/core/engine.rb index 022463d1a25..ea19825640f 100644 --- a/core/lib/spree/core/engine.rb +++ b/core/lib/spree/core/engine.rb @@ -72,6 +72,7 @@ class Engine < ::Rails::Engine config.after_initialize do Spree::Config.check_load_defaults_called('Spree::Config') + Spree::Config.static_model_preferences.validate! end config.after_initialize do diff --git a/core/lib/spree/preferences/static_model_preferences.rb b/core/lib/spree/preferences/static_model_preferences.rb index 1d14688be1c..a3b05b70ab9 100644 --- a/core/lib/spree/preferences/static_model_preferences.rb +++ b/core/lib/spree/preferences/static_model_preferences.rb @@ -7,13 +7,8 @@ class Definition attr_reader :preferences def initialize(klass, hash) - hash = hash.symbolize_keys - hash.keys.each do |key| - if !klass.defined_preferences.include?(key) - raise "Preference #{key.inspect} is not defined on #{klass}" - end - end - @preferences = hash + @klass = klass + @preferences = hash.symbolize_keys end def fetch(key, &block) @@ -27,6 +22,8 @@ def []=(key, value) def to_hash @preferences.deep_dup end + + delegate :keys, to: :@preferences end def initialize @@ -36,14 +33,32 @@ def initialize end def add(klass, name, preferences) - # We use class name instead of class to allow reloading in dev - raise "Static model preference '#{name}' on #{klass} is already defined" if @store[klass.to_s][name] - @store[klass.to_s][name] = Definition.new(klass, preferences) + @store[klass.to_s][name] = Definition.new(klass.to_s, preferences) end def for_class(klass) @store[klass.to_s] end + + def validate! + @store.keys.map(&:constantize).each do |klass| + validate_for_class!(klass) + end + end + + private + + def validate_for_class!(klass) + for_class(klass).each do |name, preferences| + klass_keys = klass.defined_preferences.map(&:to_s) + extra_keys = preferences.keys.map(&:to_s) - klass_keys + next if extra_keys.empty? + + raise \ + "Unexpected keys found for #{klass} under #{name}: #{extra_keys.sort.join(', ')} " \ + "(expected keys: #{klass_keys.sort.join(', ')})" + end + end end end end diff --git a/core/spec/models/spree/preferences/static_model_preferences_spec.rb b/core/spec/models/spree/preferences/static_model_preferences_spec.rb index 4db4cc45727..a74443a7fe0 100644 --- a/core/spec/models/spree/preferences/static_model_preferences_spec.rb +++ b/core/spec/models/spree/preferences/static_model_preferences_spec.rb @@ -23,10 +23,12 @@ module Spree expect(definitions).to have_key('my_definition') end - it "errors assigning invalid preferences" do - expect { - subject.add(preference_class, 'my_definition', { ice_cream: 'chocolate' }) - }.to raise_error(/\APreference :ice_cream is not defined/) + it "can replace preferences" do + subject.add(preference_class, 'my_definition', { color: "red" }) + + subject.add(preference_class, 'my_definition', { color: "blue" }) + + expect(definitions['my_definition'].fetch(:color)).to eq("blue") end context "with stored definitions" do @@ -75,5 +77,18 @@ module Spree expect(subject.for_class(other_class)).to be_empty end end + + describe '.validate!' do + it "errors assigning invalid preferences" do + stub_const("SomeClass", preference_class) + subject.add(preference_class, 'my_definition', { ice_cream: 'chocolate', spoon: true }) + + expect { + subject.validate! + }.to raise_error( + /\AUnexpected keys found for SomeClass under my_definition: ice_cream, spoon \(expected keys: color\)/ + ) + end + end end end