diff --git a/.circleci/config.yml b/.circleci/config.yml index 21728ea5eaf..782b3fd7f22 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -130,6 +130,15 @@ jobs: - setup - test + event_bus_legacy_adapter: + executor: postgres + parallelism: *parallelism + environment: + CI_LEGACY_EVENT_BUS_ADAPTER: '1' + steps: + - setup + - test + workflows: build: jobs: @@ -141,6 +150,7 @@ workflows: - mysql - postgres_rails60 - postgres_rails52 + - event_bus_legacy_adapter - stoplight/push: context: "Solidus Core Team" project: solidus/solidus-api diff --git a/core/lib/spree/event.rb b/core/lib/spree/event.rb index d18da828c86..5cf3aa0ca6c 100644 --- a/core/lib/spree/event.rb +++ b/core/lib/spree/event.rb @@ -1,8 +1,9 @@ # frozen_string_literal: true -require_relative 'event/adapters/active_support_notifications' -require_relative 'event/subscriber_registry' +require_relative 'event/adapters/deprecation_handler' require_relative 'event/configuration' +require_relative 'event/listener' +require_relative 'event/subscriber_registry' require_relative 'event/subscriber' module Spree @@ -99,7 +100,7 @@ def fire(event_name, opts = {}, &block) def subscribe(event_name, adapter: default_adapter, &block) event_name = normalize_name(event_name) adapter.subscribe(event_name, &block).tap do - if legacy_adapter?(adapter) + if deprecation_handler.legacy_adapter?(adapter) listener_names << adapter.normalize_name(event_name) end end @@ -154,7 +155,7 @@ def unsubscribe(subscriber_or_event_name, adapter: default_adapter) # # => {"order_finalized"=> [#], # "reimbursement_reimbursed"=> [#]} def listeners(adapter: default_adapter) - if legacy_adapter?(adapter) + if deprecation_handler.legacy_adapter?(adapter) adapter.listeners_for(listener_names) else init = Hash.new { |h, k| h[k] = [] } @@ -224,8 +225,8 @@ def handle_block_on_fire(block, opts, adapter) order.do_something Spree::Event.fire 'event_name', order: order MSG - if legacy_adapter?(adapter) - Spree::Deprecation.warn <<~MSG + if deprecation_handler.legacy_adapter?(adapter) + Spree::Deprecation.warn <<~MSG if deprecation_handler.render_deprecation_message?(adapter) Blocks on `Spree::Event.fire` are ignored in the new adapter `Spree::Event::Adapters::Default`, and your current adapter (`Spree::Event::Adapters::ActiveSupportNotifications`) is deprecated. @@ -247,8 +248,8 @@ def handle_block_on_fire(block, opts, adapter) end end - def legacy_adapter?(adapter) - adapter == Adapters::ActiveSupportNotifications + def deprecation_handler + Adapters::DeprecationHandler end end end diff --git a/core/lib/spree/event/adapters/deprecation_handler.rb b/core/lib/spree/event/adapters/deprecation_handler.rb new file mode 100644 index 00000000000..28f35433dbf --- /dev/null +++ b/core/lib/spree/event/adapters/deprecation_handler.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spree/event/adapters/active_support_notifications' +require 'spree/deprecation' + +module Spree + module Event + module Adapters + # @api private + module DeprecationHandler + LEGACY_ADAPTER = ActiveSupportNotifications + + CI_LEGACY_ADAPTER_ENV_KEY = 'CI_LEGACY_EVENT_BUS_ADAPTER' + + def self.legacy_adapter?(adapter) + adapter == LEGACY_ADAPTER + end + + def self.legacy_adapter_set_by_env + return LEGACY_ADAPTER if ENV[CI_LEGACY_ADAPTER_ENV_KEY].present? + end + + def self.render_deprecation_message?(adapter) + legacy_adapter?(adapter) && legacy_adapter_set_by_env.nil? + end + end + end + end +end diff --git a/core/lib/spree/event/configuration.rb b/core/lib/spree/event/configuration.rb index 63f3dae3019..7d54bb76ffd 100644 --- a/core/lib/spree/event/configuration.rb +++ b/core/lib/spree/event/configuration.rb @@ -1,8 +1,7 @@ # frozen_string_literal: true -require 'spree/core/versioned_value' +require 'spree/event/adapters/deprecation_handler' require 'spree/event/adapters/active_support_notifications' -require 'spree/event/adapters/default' module Spree module Event @@ -17,64 +16,56 @@ def autoload_subscribers @autoload_subscribers.nil? ? true : !!@autoload_subscribers end + # TODO: Update to Adapters::Default.new on Solidus 4 def adapter - @adapter ||= Spree::Core::VersionedValue.new( - Spree::Event::Adapters::ActiveSupportNotifications, - "4.0.0.alpha" => Spree::Event::Adapters::Default.new - ).call.tap do |value| - deprecate_if_legacy_adapter(value) - end - end - - # Only used by {Spree::Event::Adapters::ActiveSupportNotifications}. - def suffix - @suffix ||= '.spree' - end - - private - - def deprecate_if_legacy_adapter(adapter) - Spree::Deprecation.warn <<~MSG if adapter == Spree::Event::Adapters::ActiveSupportNotifications - `Spree::Event::Adapters::ActiveSupportNotifications` adapter is - deprecated. Please, take your time to update it to an instance of - `Spree::Event::Adapters::Default`. I.e., in your `spree.rb`: + @adapter ||= Adapters::ActiveSupportNotifications.tap do |adapter| + Spree::Deprecation.warn <<~MSG if Adapters::DeprecationHandler.render_deprecation_message?(adapter) + `Spree::Event::Adapters::ActiveSupportNotifications` adapter is + deprecated. Please, take your time to update it to an instance of + `Spree::Event::Adapters::Default`. I.e., in your `spree.rb`: + + require 'spree/event/adapters/default' + + Spree.config do |config| + # ... + config.events.adapter = Spree::Event::Adapters.Default.new + # ... + end - require 'spree/event/adapters/default' + That will be the new default on Solidus 4. - Spree.config do |config| - # ... - config.events.adapter = Spree::Event::Adapters.Default.new - # ... - end + Take into account there're two critical changes in behavior in the new adapter: - That will be the new default on Solidus 4. + - Event names are no longer automatically suffixed with `.spree`, as + they're no longer in the same bucket that Rails's ones. So, for + instance, if you were relying on a global subscription to all Solidus + events with: - Take into account there're two critical changes in behavior in the new adapter: + Spree::Event.subscribe /.*\.spree$/ - - Event names are no longer automatically suffixed with `.spree`, as - they're no longer in the same bucket that Rails's ones. So, for - instance, if you were relying on a global subscription to all Solidus - events with: + You should change it to: - Spree::Event.subscribe /.*\.spree$/ + Spree::Event.subscribe /.*/ - You should change it to: + - Providing a block to `Spree::Event.fire` is no longer supported. If + you're doing something like: - Spree::Event.subscribe /.*/ + Spree::Event.fire 'event_name', order: order do + order.do_something + end - - Providing a block to `Spree::Event.fire` is no longer supported. If - you're doing something like: + You now need to change it to: - Spree::Event.fire 'event_name', order: order do order.do_something - end + Spree::Event.fire 'event_name', order: order - You now need to change it to: - - order.do_something - Spree::Event.fire 'event_name', order: order + MSG + end + end - MSG + # Only used by {Spree::Event::Adapters::ActiveSupportNotifications}. + def suffix + @suffix ||= '.spree' end end end diff --git a/core/lib/spree/testing_support/dummy_app.rb b/core/lib/spree/testing_support/dummy_app.rb index 1f31fcf2b5a..8bccfa63ac8 100644 --- a/core/lib/spree/testing_support/dummy_app.rb +++ b/core/lib/spree/testing_support/dummy_app.rb @@ -135,8 +135,11 @@ class Application < ::Rails::Application Spree.config do |config| config.mails_from = "store@example.com" # TODO: Remove on Solidus 4.0 as it'll be the default - require 'spree/event/adapters/default' - config.events.adapter = Spree::Event::Adapters::Default.new + require 'spree/event/adapters/deprecation_handler' + if Spree::Event::Adapters::DeprecationHandler.legacy_adapter_set_by_env.nil? + require 'spree/event/adapters/default' + config.events.adapter = Spree::Event::Adapters::Default.new + end if ENV['DISABLE_ACTIVE_STORAGE'] config.image_attachment_module = 'Spree::Image::PaperclipAttachment' diff --git a/core/spec/lib/spree/app_configuration_spec.rb b/core/spec/lib/spree/app_configuration_spec.rb index 2c89da73558..58a9360d6e7 100644 --- a/core/spec/lib/spree/app_configuration_spec.rb +++ b/core/spec/lib/spree/app_configuration_spec.rb @@ -135,7 +135,7 @@ class DummyClass; end; expect(prefs.admin_vat_location.country_id).to eq(nil) end - it 'can access event adapter' do - expect(prefs.events.adapter).to be_an_instance_of(Spree::Event::Adapters::Default) + it 'can access event configuration' do + expect(prefs.events).to be_an_instance_of(Spree::Event::Configuration) end end diff --git a/core/spec/lib/spree/event/adapters/deprecation_handler_spec.rb b/core/spec/lib/spree/event/adapters/deprecation_handler_spec.rb new file mode 100644 index 00000000000..2fc6df1ebdc --- /dev/null +++ b/core/spec/lib/spree/event/adapters/deprecation_handler_spec.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'spree/event/adapters/deprecation_handler' +require 'spree/event/adapters/default' + +module Spree + module Event + module Adapters + RSpec.describe DeprecationHandler do + let(:env_key) { described_class::CI_LEGACY_ADAPTER_ENV_KEY } + + let(:reset_env_key) do + lambda do |example| + old_value = ENV[env_key] + example.run + ENV[env_key] = old_value + end + end + + describe '#legacy_adapter?' do + context 'when the events adapter is the legacy' do + it 'returns true' do + expect(described_class.legacy_adapter?(ActiveSupportNotifications)).to be(true) + end + end + + context 'when the events adapter is not the legacy' do + it 'returns false' do + expect(described_class.legacy_adapter?(Default.new)).to be(false) + end + end + end + + describe '#legacy_adapter_set_by_env' do + around { |example| reset_env_key.(example) } + + context 'when env var is set' do + it 'returns the legacy adapter' do + ENV[env_key] = '1' + + expect(described_class.legacy_adapter_set_by_env).to be(ActiveSupportNotifications) + end + end + + context 'when env var is not set' do + it 'returns nil' do + ENV[env_key] = nil + + expect(described_class.legacy_adapter_set_by_env).to be(nil) + end + end + end + + describe '#render_deprecation_message?' do + context 'when adapter is legacy and is not set by env' do + around { |example| reset_env_key.(example) } + + it 'returns true' do + ENV[env_key] = nil + + expect(described_class.render_deprecation_message?(ActiveSupportNotifications)).to be(true) + end + end + + context 'when adapter is legacy and is set by env' do + around { |example| reset_env_key.(example) } + + it 'returns true' do + ENV[env_key] = '1' + + expect(described_class.render_deprecation_message?(ActiveSupportNotifications)).to be(false) + end + end + + context 'when adapter is not legacy' do + it 'returns false' do + expect(described_class.render_deprecation_message?(Default.new)).to be(false) + end + end + end + end + end + end +end diff --git a/core/spec/lib/spree/event/configuration_spec.rb b/core/spec/lib/spree/event/configuration_spec.rb new file mode 100644 index 00000000000..d8042b69f2d --- /dev/null +++ b/core/spec/lib/spree/event/configuration_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'spree/event/configuration' + +RSpec.describe Spree::Event::Configuration do + describe '#adapter' do + context "when it hasn't been explicitly set" do + let(:supress_warning_env_key) { Spree::Event::Adapters::DeprecationHandler::CI_LEGACY_ADAPTER_ENV_KEY } + + it 'is ActiveSupportNotifications adapter' do + allow(Spree::Deprecation).to receive(:warn) + + expect(described_class.new.adapter).to be(Spree::Event::Adapters::ActiveSupportNotifications) + end + + it 'renders a deprecation message when no supressed via env' do + old_value = ENV[supress_warning_env_key] + ENV[supress_warning_env_key] = nil + expect(Spree::Deprecation).to receive(:warn).with(/adapter is.*deprecated/m) + + described_class.new.adapter + ensure + ENV[supress_warning_env_key] = old_value + end + + it "doesn't render a deprecation message when supressed via env" do + old_value = ENV[supress_warning_env_key] + ENV[supress_warning_env_key] = '1' + expect(Spree::Deprecation).not_to receive(:warn).with(/adapter is.*deprecated/m) + + described_class.new.adapter + ensure + ENV[supress_warning_env_key] = old_value + end + end + + context "when it has been explicitly set" do + it "doesn't render a deprecation message" do + expect(Spree::Deprecation).not_to receive(:warn) + + config = described_class.new + config.adapter = Spree::Event::Adapters::ActiveSupportNotifications + + config.adapter + end + end + end +end diff --git a/core/spec/lib/spree/event_spec.rb b/core/spec/lib/spree/event_spec.rb index 04ab8515956..728256a8324 100644 --- a/core/spec/lib/spree/event_spec.rb +++ b/core/spec/lib/spree/event_spec.rb @@ -2,6 +2,7 @@ require 'rails_helper' require 'spree/event/adapters/default' +require 'spree/event/adapters/deprecation_handler' require 'spree/event/adapters/active_support_notifications' RSpec.describe Spree::Event do @@ -13,7 +14,7 @@ def build_bus describe '.default_adapter' do it 'returns configured adapter' do - expect(subject.default_adapter).to be_an_instance_of Spree::Event::Adapters::Default + expect(subject.default_adapter).to be(Spree::Config.events.adapter) end end @@ -74,7 +75,7 @@ def toggle end.to raise_error(ArgumentError, /Blocks.*are ignored/) end - it 'renders a deprecation warning if a block is given and the adapter is ActiveSupportNotifications but still executes it' do + it 'executes a block when given and the adapter is ActiveSupportNotifications' do dummy = Class.new do attr_reader :run @@ -87,7 +88,11 @@ def toggle end end.new - expect(Spree::Deprecation).to receive(:warn).with(/Blocks.*are ignored/) + if Spree::Event::Adapters::DeprecationHandler.legacy_adapter_set_by_env + allow(Spree::Deprecation).to receive(:warn).with(/Blocks.*are ignored/) + else + expect(Spree::Deprecation).to receive(:warn).with(/Blocks.*are ignored/) + end subject.fire(:foo, adapter: Spree::Event::Adapters::ActiveSupportNotifications) { dummy.toggle }