Skip to content

Commit

Permalink
Make Omnes the default for pub/sub events
Browse files Browse the repository at this point in the history
We add an Omnes subscriber class for order mailing work, equivalent to
the legacy subscriber module.

From now on, new applications will use Omnes by default. We also run our
test suite with it, but we still allow running with the legacy adapter
through a `USE_LEGACY_EVENTS` environment variable (and use it in a new
CI job).
  • Loading branch information
waiting-for-dev committed May 20, 2022
1 parent 17bbcc1 commit ae2b7d3
Show file tree
Hide file tree
Showing 11 changed files with 166 additions and 44 deletions.
11 changes: 11 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,16 @@ jobs:
- libvips
- test

legacy_events:
executor: postgres
parallelism: &parallelism
environment:
USE_LEGACY_EVENTS: '1'
steps:
- setup
- libvips
- test

mysql:
executor: mysql
parallelism: *parallelism
Expand Down Expand Up @@ -157,3 +167,4 @@ workflows:
- postgres_rails61
- postgres_rails60
- postgres_rails52
- legacy_events
4 changes: 4 additions & 0 deletions core/app/subscribers/spree/mailer_subscriber.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
require 'spree/event/subscriber'

module Spree
# Legacy subscriber
#
# This subscriber module is used by the legacy pub/sub system (see
# {Spree::Event}).
module MailerSubscriber
include Spree::Event::Subscriber

Expand Down
35 changes: 35 additions & 0 deletions core/app/subscribers/spree/order_mailer_subscriber.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# frozen_string_literal: true

module Spree
# Mailing after events on a {Spree::Order}
class OrderMailerSubscriber
include Omnes::Subscriber

handle :order_finalized,
with: :send_confirmation_email,
id: :spree_order_mailer_send_confirmation_email

handle :reimbursement_reimbursed,
with: :send_reimbursement_email,
id: :spree_order_mailer_send_reimbursement_email

# Sends confirmation email to the user
#
# @param event [Omnes::UnstructuredEvent]
def send_confirmation_email(event)
order = event[:order]
unless order.confirmation_delivered?
Spree::Config.order_mailer_class.confirm_email(order).deliver_later
order.update_column(:confirmation_delivered, true)
end
end

# Sends reimbursement email to the user
#
# @param event [Omnes::UnstructuredEvent]
def send_reimbursement_email(event)
reimbursement = event[:reimbursement]
Spree::Config.reimbursement_mailer_class.reimbursement_email(reimbursement.id).deliver_later
end
end
end
6 changes: 5 additions & 1 deletion core/lib/spree/app_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -269,8 +269,12 @@ class AppConfiguration < Preferences::Configuration
preference :track_inventory_levels, :boolean, default: true

# @!attribute [rw] use_legacy_events
# Before v3.2, Solidus used a custom pub/sub implementation based on
# ActiveSupport::Notifications. Now, we internally use and recommend
# [Omnes](https://github.com/nebulab/omnes). This preference allows falling
# back to the old system.
# @return [Boolean]
preference :use_legacy_events, :boolean, default: true
versioned_preference :use_legacy_events, :boolean, initial_value: true, boundaries: { "3.2.0.alpha" => false }

# Other configurations

Expand Down
1 change: 1 addition & 0 deletions core/lib/spree/bus.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ module Spree
if Spree::Config.use_legacy_events
Spree::Event.fire(*args, **kwargs, &block)
else
# Override caller_location to point to the actual event publisher
super(*args, **kwargs, caller_location: caller_locations(1)[0], &block)
end
end
Expand Down
29 changes: 22 additions & 7 deletions core/lib/spree/core/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,29 @@ class Engine < ::Rails::Engine
Migrations.new(config, engine_name).check
end

# Setup Event Subscribers
initializer 'spree.core.initialize_subscribers' do |app|
app.reloader.to_prepare do
Spree::Event.activate_autoloadable_subscribers
end
# Setup pub/sub
initializer 'spree.core.pub_sub' do |app|
if Spree::Config.use_legacy_events
app.reloader.to_prepare do
Spree::Event.activate_autoloadable_subscribers
end

app.reloader.before_class_unload do
Spree::Event.deactivate_all_subscribers
end
else
app.reloader.to_prepare do
Spree::Bus.clear

%i[
order_finalized
order_recalculated
reimbursement_reimbursed
reimbursement_errored
].each { |event_name| Spree::Bus.register(event_name) }

app.reloader.before_class_unload do
Spree::Event.deactivate_all_subscribers
Spree::OrderMailerSubscriber.new.subscribe_to(Spree::Bus)
end
end
end

Expand Down
10 changes: 4 additions & 6 deletions core/lib/spree/event/subscriber_registry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,11 @@ def deactivate_subscriber(subscriber, event_action_name = nil)
# Loading the files has the side effect of adding their module to the
# list in Spree::Event.subscribers.
def require_subscriber_files
pattern = "app/subscribers/**/*_subscriber.rb"
require_dependency(
Spree::Core::Engine.root.join('app', 'subscribers', 'spree', 'mailer_subscriber.rb')
)

# Load Solidus subscribers
# rubocop:disable Rails/DynamicFindBy
solidus_core_dir = Gem::Specification.find_by_name('solidus_core').gem_dir
# rubocop:enable Rails/DynamicFindBy
Dir.glob(File.join(solidus_core_dir, pattern)) { |c| require_dependency(c.to_s) }
pattern = "app/subscribers/**/*_subscriber.rb"

# Load application subscribers, only when the flag is set to true:
if Spree::Config.events.autoload_subscribers
Expand Down
2 changes: 1 addition & 1 deletion core/lib/spree/testing_support/dummy_app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ class Application < ::Rails::Application
Spree.load_defaults(Spree.solidus_version)
Spree.config do |config|
config.mails_from = "[email protected]"
config.use_legacy_events = true
config.use_legacy_events = ENV['USE_LEGACY_EVENTS'].present?

if ENV['DISABLE_ACTIVE_STORAGE']
config.image_attachment_module = 'Spree::Image::PaperclipAttachment'
Expand Down
54 changes: 28 additions & 26 deletions core/spec/models/spree/order/finalizing_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,37 +87,39 @@
order.complete!
end

# These specs show how notifications can be removed, one at a time or
# all the ones set by MailerSubscriber module
context 'when removing the default email notification subscription' do
before do
Spree::MailerSubscriber.deactivate(:order_finalized)
end

after do
Spree::MailerSubscriber.activate
end

it 'does not send the email' do
expect(Spree::Config.order_mailer_class).not_to receive(:confirm_email)

order.complete!
if Spree::Config.use_legacy_events
# These specs show how notifications can be removed, one at a time or
# all the ones set by MailerSubscriber module
context 'when removing the default email notification subscription' do
before do
Spree::MailerSubscriber.deactivate(:order_finalized)
end

after do
Spree::MailerSubscriber.activate
end

it 'does not send the email' do
expect(Spree::Config.order_mailer_class).not_to receive(:confirm_email)

order.complete!
end
end
end

context 'when removing all the email notification subscriptions' do
before do
Spree::MailerSubscriber.deactivate
end
context 'when removing all the email notification subscriptions' do
before do
Spree::MailerSubscriber.deactivate
end

after do
Spree::MailerSubscriber.activate
end
after do
Spree::MailerSubscriber.activate
end

it 'does not send the email' do
expect(Spree::Config.order_mailer_class).not_to receive(:confirm_email)
it 'does not send the email' do
expect(Spree::Config.order_mailer_class).not_to receive(:confirm_email)

order.complete!
order.complete!
end
end
end
end
Expand Down
7 changes: 4 additions & 3 deletions core/spec/models/spree/order_updater_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -555,14 +555,15 @@ def create_adjustment(label, amount)

context "with 'order_recalculated' event subscription" do
let(:item) { spy('object') }
let(:bus) { Spree::Config.use_legacy_events ? Spree::Event : Spree::Bus }

let!(:event) do
Spree::Event.subscribe :order_recalculated do
let!(:subscription) do
bus.subscribe :order_recalculated do
item.do_something
end
end

after { Spree::Event.unsubscribe event }
after { bus.unsubscribe subscription }

it "fires the 'order_recalculated' event" do
order.recalculate
Expand Down
51 changes: 51 additions & 0 deletions core/spec/subscribers/spree/order_mailer_subscriber_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# frozen_string_literal: true

require 'rails_helper'
require 'action_mailer'

RSpec.describe Spree::OrderMailerSubscriber do
let(:bus) { Omnes::Bus.new }

before do
bus.register(:order_finalized)
bus.register(:reimbursement_reimbursed)

described_class.new.subscribe_to(bus)
end

describe 'on :on_order_finalized' do
it 'sends confirmation email' do
order = create(:order, confirmation_delivered: false)

expect(Spree::OrderMailer).to receive(:confirm_email).and_call_original

bus.publish(:order_finalized, order: order)
end

it 'marks the order as having the confirmation email delivered' do
order = create(:order, confirmation_delivered: false)

bus.publish(:order_finalized, order: order)

expect(order.confirmation_delivered).to be(true)
end

it "doesn't send confirmation email if already sent" do
order = build(:order, confirmation_delivered: true)

expect(Spree::OrderMailer).not_to receive(:confirm_email)

bus.publish(:order_finalized, order: order)
end
end

describe 'on :reimbursement_reimbursed' do
it 'sends reimbursement email' do
reimbursement = build(:reimbursement)

expect(Spree::ReimbursementMailer).to receive(:reimbursement_email).and_call_original

bus.publish(:reimbursement_reimbursed, reimbursement: reimbursement)
end
end
end

0 comments on commit ae2b7d3

Please sign in to comment.