diff --git a/core/lib/spree/preferences/preferable.rb b/core/lib/spree/preferences/preferable.rb index 5753fdbc7f8..86da3aa2f41 100644 --- a/core/lib/spree/preferences/preferable.rb +++ b/core/lib/spree/preferences/preferable.rb @@ -10,6 +10,9 @@ module Preferences # # A class including Preferable must implement #preferences which should return # an object responding to .fetch(key), []=(key, val), and .delete(key). + # If #preferences is initialized with `default_preferences` and one of the + # preferences is another preference, it will cause a stack level too deep error. + # To avoid it do not memoize #preferences. # # It may also define a `#context_for_default` method. It should return an # array with the arguments to be provided to a proc used as the `default:` @@ -111,6 +114,8 @@ def defined_preferences end # @return [Hash{Symbol => Object}] Default for all preferences defined on this class + # This may raise an infinite loop error if any of the defaults are + # dependent on other preferences defaults. def default_preferences Hash[ defined_preferences.map do |preference| diff --git a/core/lib/spree/preferences/preferable_class_methods.rb b/core/lib/spree/preferences/preferable_class_methods.rb index 4da470c0c9a..25416197632 100644 --- a/core/lib/spree/preferences/preferable_class_methods.rb +++ b/core/lib/spree/preferences/preferable_class_methods.rb @@ -75,7 +75,7 @@ def preference(name, type, options = {}) # is a pending preference before going to default define_method preference_getter_method(name) do value = preferences.fetch(name) do - default.call(*context_for_default) + instance_exec(*context_for_default, &default) end value = preference_encryptor.decrypt(value) if preference_encryptor.present? value @@ -92,7 +92,7 @@ def preference(name, type, options = {}) end define_method preference_default_getter_method(name) do - default.call(*context_for_default) + instance_exec(*context_for_default, &default) end define_method preference_type_getter_method(name) do diff --git a/core/spec/models/spree/preferences/preferable_spec.rb b/core/spec/models/spree/preferences/preferable_spec.rb index 25addfa03ff..1064ae92462 100644 --- a/core/spec/models/spree/preferences/preferable_spec.rb +++ b/core/spec/models/spree/preferences/preferable_spec.rb @@ -3,105 +3,110 @@ require 'rails_helper' RSpec.describe Spree::Preferences::Preferable, type: :model do - before :all do - class A + let(:config_class_a) do + Class.new do include Spree::Preferences::Preferable attr_reader :id def initialize @id = rand(999) + initialize_preference_defaults end def preferences - @preferences ||= default_preferences + @preferences ||= {} + end + + def initialize_preference_defaults + @preferences = default_preferences end preference :color, :string, default: 'green' end + end - class B < A + let(:config_class_b) do + Class.new(config_class_a) do preference :flavor, :string end end - before :each do - @a = A.new - @b = B.new - end + let(:a) { config_class_a.new } + let(:b) { config_class_b.new } describe "preference definitions" do it "parent should not see child definitions" do - expect(@a.has_preference?(:color)).to be true - expect(@a.has_preference?(:flavor)).not_to be true + expect(a.has_preference?(:color)).to be true + expect(a.has_preference?(:flavor)).not_to be true end it "child should have parent and own definitions" do - expect(@b.has_preference?(:color)).to be true - expect(@b.has_preference?(:flavor)).to be true + expect(b.has_preference?(:color)).to be true + expect(b.has_preference?(:flavor)).to be true end it "instances have defaults" do - expect(@a.preferred_color).to eq 'green' - expect(@b.preferred_color).to eq 'green' - expect(@b.preferred_flavor).to be_nil + expect(a.preferred_color).to eq 'green' + expect(b.preferred_color).to eq 'green' + expect(b.preferred_flavor).to be_nil end it "can be asked if it has a preference definition" do - expect(@a.has_preference?(:color)).to be true - expect(@a.has_preference?(:bad)).to be false + expect(a.has_preference?(:color)).to be true + expect(a.has_preference?(:bad)).to be false end it "can be asked and raises" do expect { - @a.has_preference! :flavor + a.has_preference! :flavor }.to raise_error(NoMethodError, "flavor preference not defined") end it "has a type" do - expect(@a.preferred_color_type).to eq :string - expect(@a.preference_type(:color)).to eq :string + expect(a.preferred_color_type).to eq :string + expect(a.preference_type(:color)).to eq :string end it "has a default" do - expect(@a.preferred_color_default).to eq 'green' - expect(@a.preference_default(:color)).to eq 'green' + expect(a.preferred_color_default).to eq 'green' + expect(a.preference_default(:color)).to eq 'green' end it "raises if not defined" do expect { - @a.get_preference :flavor + a.get_preference :flavor }.to raise_error(NoMethodError, "flavor preference not defined") end end describe "preference access" do it "handles ghost methods for preferences" do - @a.preferred_color = 'blue' - expect(@a.preferred_color).to eq 'blue' + a.preferred_color = 'blue' + expect(a.preferred_color).to eq 'blue' end it "parent and child instances have their own prefs" do - @a.preferred_color = 'red' - @b.preferred_color = 'blue' + a.preferred_color = 'red' + b.preferred_color = 'blue' - expect(@a.preferred_color).to eq 'red' - expect(@b.preferred_color).to eq 'blue' + expect(a.preferred_color).to eq 'red' + expect(b.preferred_color).to eq 'blue' end it "raises when preference not defined" do expect { - @a.set_preference(:bad, :bone) + a.set_preference(:bad, :bone) }.to raise_exception(NoMethodError, "bad preference not defined") end it "builds a hash of preferences" do - @b.preferred_flavor = :strawberry - expect(@b.preferences[:flavor]).to eq 'strawberry' - expect(@b.preferences[:color]).to eq 'green' # default from A + b.preferred_flavor = :strawberry + expect(b.preferences[:flavor]).to eq 'strawberry' + expect(b.preferences[:color]).to eq 'green' # default from A end it "builds a hash of preference defaults" do - expect(@b.default_preferences).to eq({ + expect(b.default_preferences).to eq({ flavor: nil, color: 'green' }) @@ -151,158 +156,170 @@ def self.allowed_admin_form_preference_types end end + context "when a default is a proc" do + before do + config_class_a.preference(:context, :string, default: -> { preferred_color }) + end + + it "calls it from the instance" do + expect(a.preferred_context).to eq("green") + end + end + context "converts integer preferences to integer values" do before do - A.preference :is_integer, :integer + config_class_a.preference :is_integer, :integer end it "with strings" do - @a.set_preference(:is_integer, '3') - expect(@a.preferences[:is_integer]).to eq(3) + a.set_preference(:is_integer, '3') + expect(a.preferences[:is_integer]).to eq(3) - @a.set_preference(:is_integer, '') - expect(@a.preferences[:is_integer]).to eq(0) + a.set_preference(:is_integer, '') + expect(a.preferences[:is_integer]).to eq(0) end it 'does not convert if value is nil' do - @a.set_preference(:is_integer, nil) - expect(@a.preferences[:is_integer]).to be_nil + a.set_preference(:is_integer, nil) + expect(a.preferences[:is_integer]).to be_nil end end context "converts decimal preferences to BigDecimal values" do before do - A.preference :if_decimal, :decimal + config_class_a.preference :if_decimal, :decimal end it "returns a BigDecimal" do - @a.set_preference(:if_decimal, 3.3) - expect(@a.preferences[:if_decimal].class).to eq(BigDecimal) + a.set_preference(:if_decimal, 3.3) + expect(a.preferences[:if_decimal].class).to eq(BigDecimal) end it "with strings" do - @a.set_preference(:if_decimal, '3.3') - expect(@a.preferences[:if_decimal]).to eq(3.3) + a.set_preference(:if_decimal, '3.3') + expect(a.preferences[:if_decimal]).to eq(3.3) - @a.set_preference(:if_decimal, '') - expect(@a.preferences[:if_decimal]).to eq(0.0) + a.set_preference(:if_decimal, '') + expect(a.preferences[:if_decimal]).to eq(0.0) end end context "converts boolean preferences to boolean values" do before do - A.preference :is_boolean, :boolean, default: true + config_class_a.preference :is_boolean, :boolean, default: true end it "with strings" do - @a.set_preference(:is_boolean, '0') - expect(@a.preferences[:is_boolean]).to be false - @a.set_preference(:is_boolean, 'f') - expect(@a.preferences[:is_boolean]).to be false - @a.set_preference(:is_boolean, 't') - expect(@a.preferences[:is_boolean]).to be true + a.set_preference(:is_boolean, '0') + expect(a.preferences[:is_boolean]).to be false + a.set_preference(:is_boolean, 'f') + expect(a.preferences[:is_boolean]).to be false + a.set_preference(:is_boolean, 't') + expect(a.preferences[:is_boolean]).to be true end it "with integers" do - @a.set_preference(:is_boolean, 0) - expect(@a.preferences[:is_boolean]).to be false - @a.set_preference(:is_boolean, 1) - expect(@a.preferences[:is_boolean]).to be true + a.set_preference(:is_boolean, 0) + expect(a.preferences[:is_boolean]).to be false + a.set_preference(:is_boolean, 1) + expect(a.preferences[:is_boolean]).to be true end it "with an empty string" do - @a.set_preference(:is_boolean, '') - expect(@a.preferences[:is_boolean]).to be false + a.set_preference(:is_boolean, '') + expect(a.preferences[:is_boolean]).to be false end it "with an empty hash" do - @a.set_preference(:is_boolean, []) - expect(@a.preferences[:is_boolean]).to be false + a.set_preference(:is_boolean, []) + expect(a.preferences[:is_boolean]).to be false end end context "converts array preferences to array values" do before do - A.preference :is_array, :array, default: [] + config_class_a.preference :is_array, :array, default: [] end it "with arrays" do - @a.set_preference(:is_array, []) - expect(@a.preferences[:is_array]).to eq [] + a.set_preference(:is_array, []) + expect(a.preferences[:is_array]).to eq [] end end context "converts hash preferences to hash values" do before do - A.preference :is_hash, :hash, default: {} + config_class_a.preference :is_hash, :hash, default: {} end it "with hash" do - @a.set_preference(:is_hash, {}) - expect(@a.preferences[:is_hash]).to be_is_a(Hash) + a.set_preference(:is_hash, {}) + expect(a.preferences[:is_hash]).to be_is_a(Hash) end it "with hash and keys are integers" do - @a.set_preference(:is_hash, { 1 => 2, 3 => 4 }) - expect(@a.preferences[:is_hash]).to eql({ 1 => 2, 3 => 4 }) + a.set_preference(:is_hash, { 1 => 2, 3 => 4 }) + expect(a.preferences[:is_hash]).to eql({ 1 => 2, 3 => 4 }) end end context "converts any preferences to any values" do - before do - A.preference :product_ids, :any, default: [] - A.preference :product_attributes, :any, default: {} - end - it "with array" do - expect(@a.preferences[:product_ids]).to eq([]) - @a.set_preference(:product_ids, [1, 2]) - expect(@a.preferences[:product_ids]).to eq([1, 2]) + config_class_a.preference :product_ids, :any, default: [] + config_class_a.preference :product_attributes, :any, default: {} + + expect(a.preferences[:product_ids]).to eq([]) + a.set_preference(:product_ids, [1, 2]) + expect(a.preferences[:product_ids]).to eq([1, 2]) end it "with hash" do - expect(@a.preferences[:product_attributes]).to eq({}) - @a.set_preference(:product_attributes, { id: 1, name: 2 }) - expect(@a.preferences[:product_attributes]).to eq({ id: 1, name: 2 }) + config_class_a.preference :product_ids, :any, default: [] + config_class_a.preference :product_attributes, :any, default: {} + + expect(a.preferences[:product_attributes]).to eq({}) + a.set_preference(:product_attributes, { id: 1, name: 2 }) + expect(a.preferences[:product_attributes]).to eq({ id: 1, name: 2 }) end end context "converts encrypted_string preferences to encrypted values" do it "with string, encryption key provided as option" do - A.preference :secret, :encrypted_string, + config_class_a.preference :secret, :encrypted_string, encryption_key: 'VkYp3s6v9y$B?E(H+MbQeThWmZq4t7w!' - @a.set_preference(:secret, 'secret_client_id') - expect(@a.get_preference(:secret)).to eq('secret_client_id') - expect(@a.preferences[:secret]).not_to eq('secret_client_id') + a.set_preference(:secret, 'secret_client_id') + expect(a.get_preference(:secret)).to eq('secret_client_id') + expect(a.preferences[:secret]).not_to eq('secret_client_id') end it "with string, encryption key provided as env variable" do expect(ENV).to receive(:[]).with("SOLIDUS_PREFERENCES_MASTER_KEY").and_return("VkYp3s6v9y$B?E(H+MbQeThWmZq4t7w!") - A.preference :secret, :encrypted_string + config_class_a.preference :secret, :encrypted_string - @a.set_preference(:secret, 'secret_client_id') - expect(@a.get_preference(:secret)).to eq('secret_client_id') - expect(@a.preferences[:secret]).not_to eq('secret_client_id') + a.set_preference(:secret, 'secret_client_id') + expect(a.get_preference(:secret)).to eq('secret_client_id') + expect(a.preferences[:secret]).not_to eq('secret_client_id') end it "with string, encryption key provided as option, set using syntactic sugar method" do - A.preference :secret, :encrypted_string, + config_class_a.preference :secret, :encrypted_string, encryption_key: 'VkYp3s6v9y$B?E(H+MbQeThWmZq4t7w!' - @a.preferred_secret = 'secret_client_id' - expect(@a.preferred_secret).to eq('secret_client_id') - expect(@a.preferences[:secret]).not_to eq('secret_client_id') + a.preferred_secret = 'secret_client_id' + expect(a.preferred_secret).to eq('secret_client_id') + expect(a.preferences[:secret]).not_to eq('secret_client_id') end it "with string, default value" do - A.preference :secret, :encrypted_string, + config_class_a.preference :secret, :encrypted_string, default: 'my_default_secret', encryption_key: 'VkYp3s6v9y$B?E(H+MbQeThWmZq4t7w!' - expect(@a.get_preference(:secret)).to eq('my_default_secret') - expect(@a.preferences[:secret]).not_to eq('my_default_secret') + a = config_class_a.new + expect(a.get_preference(:secret)).to eq('my_default_secret') + expect(a.preferences[:secret]).not_to eq('my_default_secret') end end end