From b897eccb6cdd8235d782275e9df069d95474cd6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc=20Busqu=C3=A9?= Date: Mon, 17 May 2021 06:44:39 +0200 Subject: [PATCH] Allow using different preference defaults depending on a Solidus version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This feature is similar to [Rail's `load_defaults`](https://guides.rubyonrails.org/configuring.html#results-of-config-load-defaults). However, instead of loading imperatively, the defaults for a given version keep the history of the value in the preference declaration. Both systems are upgrade-friendly, as users need to adjust the new version defaults manually. The main advantage of this commit implementation is communication (as the code tells about its current state and history). However, it's architecturally more complex. `Spree::Core::VersionedValue` implements the core behavior. This class accepts a specification of how a value has changed in time and returns the result for a given solidus version. The specification consists of an initial value and a series of boundaries when it changed: ```ruby value = Spree::Core::VersionedValue.new(false, "3.0" => true) value.call("2.0") # => false value.call("3.0") # => true ``` `Spree::Preferences::Configuration` bundles the behavior into the preferences system. Its `.by_version` method builds a `Proc` that accepts a solidus version and returns the corresponding value. It's meant to be used in the `default:` keyword argument: ```ruby preference :foo, :boolean, default: by_version(false, "3.0" => false) ``` Accordingly, `Spree::Preferences::Preferable` has been modified to provide arguments to the `default:` `Proc`. As this module is not only included in `Spree::Preferences::Configuration` classes, it defaults to supply no arguments to it, but `Spree::Preferences::Configuration` overrides it to add the `#loaded_defaults` attribute. Users can specify the version defaults they want to use through the `#load_defaults` method in `Spree::AppConfiguration`, `Spree::BackendConfiguration`, `Spree::FrontendConfiguration` and `Spree::ApiConfiguration`. For instance, for `Spree::AppConfiguration', which gets bound in the `Spree.config` method used in the `spree` initializer: ```ruby Spree.config do |config| config.load_defaults '3.1' end ``` We have modified the `spree` generator template to add that line of code defaulting to the current Solidus version. As the sibling step, we should create an update task creating another initializer à la `new_framework_defaults` in Rails. Related to discussion https://github.com/solidusio/solidus/discussions/4045 --- .../templates/config/initializers/spree.rb.tt | 6 + core/lib/spree/core/versioned_value.rb | 75 +++++++++++ core/lib/spree/preferences/configuration.rb | 44 +++++++ core/lib/spree/preferences/preferable.rb | 8 ++ .../preferences/preferable_class_methods.rb | 8 +- .../lib/spree/core/versioned_value_spec.rb | 124 ++++++++++++++++++ .../spree/preferences/configuration_spec.rb | 27 ++++ 7 files changed, 289 insertions(+), 3 deletions(-) create mode 100644 core/lib/spree/core/versioned_value.rb create mode 100644 core/spec/lib/spree/core/versioned_value_spec.rb diff --git a/core/lib/generators/solidus/install/templates/config/initializers/spree.rb.tt b/core/lib/generators/solidus/install/templates/config/initializers/spree.rb.tt index 8c3051d432..8b1723b5db 100644 --- a/core/lib/generators/solidus/install/templates/config/initializers/spree.rb.tt +++ b/core/lib/generators/solidus/install/templates/config/initializers/spree.rb.tt @@ -4,6 +4,9 @@ Spree.config do |config| # Core: + # Solidus version defaults for preferences that are not overridden + config.load_defaults '<%= Spree.solidus_version %>' + # Default currency for new sites config.currency = "USD" @@ -62,12 +65,14 @@ end <% if defined?(Spree::Frontend::Engine) -%> Spree::Frontend::Config.configure do |config| + config.load_defaults '<%= Spree.solidus_version %>' config.locale = 'en' end <% end -%> <% if defined?(Spree::Backend::Engine) -%> Spree::Backend::Config.configure do |config| + config.load_defaults '<%= Spree.solidus_version %>' config.locale = 'en' # Uncomment and change the following configuration if you want to add @@ -83,6 +88,7 @@ end <% if defined?(Spree::Api::Engine) -%> Spree::Api::Config.configure do |config| + config.load_defaults '<%= Spree.solidus_version %>' config.requires_authentication = true end <% end -%> diff --git a/core/lib/spree/core/versioned_value.rb b/core/lib/spree/core/versioned_value.rb new file mode 100644 index 0000000000..bb3989ee29 --- /dev/null +++ b/core/lib/spree/core/versioned_value.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +module Spree + module Core + # Wrapper for a value that can be different depending on the Solidus version + # + # Some configuration defaults can be added or changed when a new Solidus + # version is released. This class encapsulates getting the correct value for a + # given Solidus version. + # + # The way it works is you provide an initial value in time, plus the version + # boundary where it got changed. Then you can fetch the value providing the + # desired Solidus version: + # + # @example + # value = VersionedValue.new(true, "3.0.0" => false) + # value.call("2.7.0") # => true + # value.call("3.0.0") # => false + # value.call("3.1.0") # => false + # + # Remember that you must provide the exact boundary when a value got changed, + # which could easily be during a pre-release: + # + # @example + # value = VersionedValue.new(true, "3.0.0" => false) + # value.call("3.0.0.alpha") # => true + # + # value = VersionedValue.new(true, "3.0.0.alpha" => false) + # value.call("3.0.0.alpha") # => false + # + # Multiple boundaries can also be provided: + # + # @example + # value = VersionedValue.new(0, "2.0.0" => 1, "3.0.0" => 2) + # value.call("1.0.0") # => 0 + # value.call("2.1.0") # => 1 + # value.call("3.0.0") # => 2 + class VersionedValue + attr_reader :boundaries + + # @param initial_value [Any] + # @param boundary [Hash] Map from version number to new value + def initialize(initial_value, boundaries = {}) + @boundaries = Hash[ + { '0' => initial_value } + .merge(boundaries) + .transform_keys { |version| to_gem_version(version) } + .sort + ] + end + + # @param solidus_version [String] + def call(solidus_version = Spree.solidus_version) + solidus_version = to_gem_version(solidus_version) + boundaries.fetch( + boundaries + .keys + .reduce do |target, following| + if target <= solidus_version && solidus_version < following + target + else + following + end + end + ) + end + + private + + def to_gem_version(string) + Gem::Version.new(string) + end + end + end +end diff --git a/core/lib/spree/preferences/configuration.rb b/core/lib/spree/preferences/configuration.rb index d430abe9c1..cc6cf2f58a 100644 --- a/core/lib/spree/preferences/configuration.rb +++ b/core/lib/spree/preferences/configuration.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require 'spree/core/versioned_value' require 'spree/preferences/preferable' module Spree::Preferences @@ -29,6 +30,26 @@ module Spree::Preferences class Configuration include Spree::Preferences::Preferable + # @!attribute [r] loaded_defaults + # @return [String] + # Some configuration defaults can be added or changed when a new Solidus + # version is released. Setting this to an older Solidus version allows keeping + # backward compatibility until the application code is updated to the new + # defaults. Set via {#load_defaults} + attr_reader :loaded_defaults + + def initialize + @loaded_defaults = Spree.solidus_version + end + + # @param [String] Solidus version from which take defaults when not + # overriden. + # @see #load_defaults + def load_defaults(version) + @loaded_defaults = version + reset + end + # @yield [config] Yields this configuration object to a block def configure yield(self) @@ -79,6 +100,23 @@ def set(preferences) end end + # Generates a different preference default depending on {#version_defaults} + # + # This method is meant to be used in the `default:` keyword argument for + # {.preference}. For instance, in the example, `foo`'s default was `true` + # until version 3.0.0.alpha, when it became `false`: + # + # @example + # preference :foo, :boolean, default: by_version(true, "3.0.0.alpha" => false) + # + # @see #loaded_defaults + # @see Spree::Core::VersionedValue + def self.by_version(*args) + proc do |loaded_defaults| + Spree::Core::VersionedValue.new(*args).call(loaded_defaults) + end + end + def self.preference(name, type, options = {}) super alias_method name.to_s, "preferred_#{name}" @@ -103,5 +141,11 @@ def self.class_name_attribute(name, default:) class_name end end + + private + + def context_for_default + [loaded_defaults] + end end end diff --git a/core/lib/spree/preferences/preferable.rb b/core/lib/spree/preferences/preferable.rb index ef1fa3a794..5753fdbc7f 100644 --- a/core/lib/spree/preferences/preferable.rb +++ b/core/lib/spree/preferences/preferable.rb @@ -11,6 +11,10 @@ module Preferences # A class including Preferable must implement #preferences which should return # an object responding to .fetch(key), []=(key, val), and .delete(key). # + # 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:` + # keyword for a preference. + # # The generated writer method performs typecasting before assignment into the # preferences object. # @@ -176,6 +180,10 @@ def convert_preference_value(value, type, preference_encryptor = nil) value end end + + def context_for_default + [].freeze + end end end end diff --git a/core/lib/spree/preferences/preferable_class_methods.rb b/core/lib/spree/preferences/preferable_class_methods.rb index 0b919e0246..65010b8cff 100644 --- a/core/lib/spree/preferences/preferable_class_methods.rb +++ b/core/lib/spree/preferences/preferable_class_methods.rb @@ -27,7 +27,7 @@ def preference(name, type, options = {}) end default = options[:default] - default = ->{ options[:default] } unless default.is_a?(Proc) + default = proc { options[:default] } unless default.is_a?(Proc) # The defined preferences on a class are all those defined directly on # that class as well as those defined on ancestors. @@ -44,7 +44,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 + default.call(*context_for_default) end value = preference_encryptor.decrypt(value) if preference_encryptor.present? value @@ -60,7 +60,9 @@ def preference(name, type, options = {}) preferences_will_change! if respond_to?(:preferences_will_change!) end - define_method preference_default_getter_method(name), &default + define_method preference_default_getter_method(name) do + default.call(*context_for_default) + end define_method preference_type_getter_method(name) do type diff --git a/core/spec/lib/spree/core/versioned_value_spec.rb b/core/spec/lib/spree/core/versioned_value_spec.rb new file mode 100644 index 0000000000..cf440425de --- /dev/null +++ b/core/spec/lib/spree/core/versioned_value_spec.rb @@ -0,0 +1,124 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'spree/core/versioned_value' + +RSpec.describe Spree::Core::VersionedValue do + context 'with no boundaries' do + it 'takes the initial value' do + expect( + described_class + .new(false) + .call("2.1.0") + ).to be(false) + end + end + + context 'with a single boundary' do + it 'takes the initial value when the version preceeds' do + expect( + described_class + .new(false, '3.0.0' => true) + .call("2.9.0") + ).to be(false) + end + + it 'takes the new value when the version matches' do + expect( + described_class + .new(false, '3.0.0' => true) + .call("3.0.0") + ).to be(true) + end + + it 'takes the new value when the version follows' do + expect( + described_class + .new(false, '3.0.0' => true) + .call("3.1.0") + ).to be(true) + end + + it 'compares as version numbers' do + expect( + described_class + .new(false, '2.10.0' => true) + .call("2.7.0") + ).to be(false) + end + + it 'sorts pre-releases before releases' do + expect( + described_class + .new(false, '3.1.0' => true) + .call("3.1.0.alpha") + ).to be(false) + end + end + + context 'with two boundaries' do + it 'takes the initial value when the version preceeds the first boundary' do + expect( + described_class + .new(0, '2.0.0' => 1, '3.0.0' => 2) + .call('1.0.0') + ).to be(0) + end + + it 'takes the new value after the first boundary when the version matches the first boundary' do + expect( + described_class + .new(0, '2.0.0' => 1, '3.0.0' => 2) + .call('2.0.0') + ).to be(1) + end + + it 'takes the new value after the first boundary when the version follows the first boundary but preceeds the second one' do + expect( + described_class + .new(0, '2.0.0' => 1, '3.0.0' => 2) + .call('2.5.0') + ).to be(1) + end + + it 'takes the new value after the second boundary when the version matches the second boundary' do + expect( + described_class + .new(0, '2.0.0' => 1, '3.0.0' => 2) + .call('3.0.0') + ).to be(2) + end + + it 'takes the new value after the second boundary when the version follows the second boundary' do + expect( + described_class + .new(0, '2.0.0' => 1, '3.0.0' => 2) + .call('4.0.0') + ).to be(2) + end + + it 'works regardless of the order given to the boundaries' do + expect( + described_class + .new(0, '3.0.0' => 2, '2.0.0' => 1) + .call('4.0.0') + ).to be(2) + end + + it 'compares as version numbers' do + expect( + described_class + .new(0, '2.0.0' => 1, '2.10.0' => 2) + .call("2.7.0") + ).to be(1) + end + + it 'sorts pre-releases before releases' do + expect( + described_class + .new(0, '3.1.0.alpha' => 1, '3.1.0' => 2) + .call("3.2.0") + ).to be(2) + end + end +end diff --git a/core/spec/models/spree/preferences/configuration_spec.rb b/core/spec/models/spree/preferences/configuration_spec.rb index b9b6056005..82c9ffd8e8 100644 --- a/core/spec/models/spree/preferences/configuration_spec.rb +++ b/core/spec/models/spree/preferences/configuration_spec.rb @@ -6,6 +6,7 @@ before :all do class AppConfig < Spree::Preferences::Configuration preference :color, :string, default: :blue + preference :foo, :boolean, default: by_version(true, "3.0" => false) end @config = AppConfig.new end @@ -24,4 +25,30 @@ class AppConfig < Spree::Preferences::Configuration @config.set(color: 'green') expect(@config.get(:color)).to eq 'green' end + + it "allows defining different defaults depending on the Solidus version" do + @config.load_defaults 2.1 + + expect(@config.get(:foo)).to be(true) + + @config.load_defaults 3.1 + + expect(@config.get(:foo)).to be(false) + end + + describe '#load_defaults' do + it 'changes loaded_defaults' do + @config.load_defaults '2.1' + + expect(@config.loaded_defaults).to eq('2.1') + + @config.load_defaults '3.1' + + expect(@config.loaded_defaults).to eq('3.1') + end + + it 'returns updated preferences' do + expect(@config.load_defaults('2.1')).to eq(foo: true, color: :blue) + end + end end