Skip to content

Commit

Permalink
Enforce event registration
Browse files Browse the repository at this point in the history
Make it mandatory to register an event before being fired, subscribed
, or unsubscribed.

It helps with two scenarios:

- It avoids typo errors, like subscribing to an event `ordr_finalized`
  instead of `order_finalized`.

- It avoids naming collisions between user-defined events and core ones.

Example:

```ruby
Spree::Event.register('foo')
Spree::Event.fire('foo')
```

We add this behavior only to the new, recommended adapter. However, we
add into the deprecation error for the legacy adapter the instructions
to update.

We also add a new `#unregister` method for the test interface.
  • Loading branch information
waiting-for-dev committed Dec 22, 2021
1 parent d467123 commit 7a8b9a6
Show file tree
Hide file tree
Showing 13 changed files with 539 additions and 8 deletions.
14 changes: 14 additions & 0 deletions core/lib/spree/core/engine.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# frozen_string_literal: true

require 'spree/config'
require 'spree/event'
require 'spree/event/adapters/deprecation_handler'

module Spree
module Core
Expand Down Expand Up @@ -44,6 +46,18 @@ class Engine < ::Rails::Engine
Migrations.new(config, engine_name).check
end

# Register core events
initializer 'spree.core.register_events' do
unless Spree::Event::Adapters::DeprecationHandler.legacy_adapter?
%w[
order_finalized
order_recalculated
reimbursement_reimbursed
reimbursement_errored
].each { |event_name| Spree::Event.register(event_name) }
end
end

# Setup Event Subscribers
initializer 'spree.core.initialize_subscribers' do |app|
app.reloader.to_prepare do
Expand Down
42 changes: 42 additions & 0 deletions core/lib/spree/event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
require_relative 'event/listener'
require_relative 'event/subscriber_registry'
require_relative 'event/subscriber'
require 'spree/deprecation'

module Spree
# Event bus for Solidus.
Expand All @@ -13,6 +14,12 @@ module Spree
# Solidus. You can use different underlying adapters to provide the core
# logic. It's recommended that you use {Spree::Event::Adapters::Default}.
#
# Before firing, subscribing, or unsubscribing an event, you need to
# {#register} it:
#
# @example
# Spree::Event.register 'order_finalized'
#
# You use the {#fire} method to trigger an event:
#
# @example
Expand Down Expand Up @@ -43,6 +50,34 @@ module Event

delegate :activate_autoloadable_subscribers, :activate_all_subscribers, :deactivate_all_subscribers, to: :subscriber_registry

# Registers an event
#
# This step is needed before firing, subscribing or unsubscribing an
# event. It helps to prevent typos and naming collision.
#
# This method is not available in the legacy adapter.
#
# @example
# Spree::Event.register('foo')
#
# @param [String, Symbol] event_name
# @param [Any] adapter the event bus adapter to use.
def register(event_name, adapter: default_adapter)
warn_registration_on_legacy_adapter if deprecation_handler.render_deprecation_message?(adapter)
return if deprecation_handler.legacy_adapter?(adapter)


adapter.register(normalize_name(event_name), caller_location: caller_locations(1)[0])
end

# @api private
def registry(adapter: default_adapter)
warn_registration_on_legacy_adapter if deprecation_handler.render_deprecation_message?(adapter)
return if deprecation_handler.legacy_adapter?(adapter)

adapter.registry
end

# Allows to trigger events that can be subscribed using {#subscribe}.
#
# The actual code implementation is delegated to the adapter.
Expand Down Expand Up @@ -253,6 +288,13 @@ def handle_block_on_fire(block, opts, adapter)
end
end

def warn_registration_on_legacy_adapter
Spree::Deprecation.warn <<~MSG
Event registration works only on the new adapter
`Spree::Event::Adapters::Default`. Please, update to it.
MSG
end

def deprecation_handler
Adapters::DeprecationHandler
end
Expand Down
20 changes: 17 additions & 3 deletions core/lib/spree/event/adapters/default.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
require 'spree/event/event'
require 'spree/event/listener'
require 'spree/event/firing'
require 'spree/event/registry'

module Spree
module Event
Expand All @@ -26,14 +27,21 @@ module Adapters
# be the default one.
class Default
# @api private
attr_reader :listeners
attr_reader :listeners, :registry

def initialize(listeners = [])
def initialize(listeners = [], registry = Registry.new)
@listeners = listeners
@registry = registry
end

# @api private
def register(event_name, caller_location: caller_locations(1)[0])
registry.register(event_name, caller_location: caller_location)
end

# @api private
def fire(event_name, caller_location: caller_locations(1)[0], **payload)
registry.check_event_name_registered(event_name)
event = Event.new(payload: payload, caller_location: caller_location)
executions = listeners_for_event(event_name).map do |listener|
listener.call(event)
Expand All @@ -43,6 +51,7 @@ def fire(event_name, caller_location: caller_locations(1)[0], **payload)

# @api private
def subscribe(event_name_or_regexp, &block)
registry.check_event_name_registered(event_name_or_regexp) if event_name?(event_name_or_regexp)
Listener.new(pattern: event_name_or_regexp, block: block).tap do |listener|
@listeners << listener
end
Expand All @@ -53,13 +62,14 @@ def unsubscribe(subscriber_or_event_name)
if subscriber_or_event_name.is_a?(Listener)
unsubscribe_listener(subscriber_or_event_name)
else
registry.check_event_name_registered(subscriber_or_event_name) if event_name?(subscriber_or_event_name)
unsubscribe_event(subscriber_or_event_name)
end
end

# @api private
def with_listeners(listeners)
self.class.new(listeners)
self.class.new(listeners, registry)
end

private
Expand All @@ -79,6 +89,10 @@ def unsubscribe_event(event_name)
listener.unsubscribe(event_name)
end
end

def event_name?(candidate)
candidate.is_a?(String)
end
end
end
end
Expand Down
9 changes: 8 additions & 1 deletion core/lib/spree/event/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def adapter
That will be the new default on Solidus 4.
Take into account there're two critical changes in behavior in the new adapter:
Take into account there're three critical changes in behavior in the new adapter:
- Event names are no longer automatically suffixed with `.spree`, as
they're no longer in the same bucket that Rails's ones. So, for
Expand All @@ -59,6 +59,13 @@ def adapter
order.do_something
Spree::Event.fire 'event_name', order: order
- You need to register your custom events before firing or subscribing
to them (no need for Solidus' custom ones). Example:
Spree::Event.register('foo')
Spree::Event.fire('foo')
MSG
end
end
Expand Down
93 changes: 93 additions & 0 deletions core/lib/spree/event/registry.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
# frozen_string_literal: true

module Spree
module Event
# Registry of known events
#
# @api privte
class Registry
class Registration
attr_reader :event_name, :caller_location

def initialize(event_name:, caller_location:)
@event_name = event_name
@caller_location = caller_location
end
end

attr_reader :registrations

def initialize(registrations: [])
@registrations = registrations
end

def register(event_name, caller_location: caller_locations(1)[0])
registration = registration(event_name)
if registration
raise <<~MSG
Can't register #{event_name} event as it's already registered.
The registration happened at:
#{registration.caller_location}
MSG
else
@registrations << Registration.new(event_name: event_name, caller_location: caller_location)
end
end

def unregister(event_name)
raise <<~MSG unless registered?(event_name)
#{event_name} is not registered.
Known events are:
'#{event_names.join("' '")}'
MSG

@registrations.delete_if { |regs| regs.event_name == event_name }
end

def registration(event_name)
registrations.find { |reg| reg.event_name == event_name }
end

def registered?(event_name)
!registration(event_name).nil?
end

def event_names
registrations.map(&:event_name)
end

def check_event_name_registered(event_name)
return true if registered?(event_name)

raise <<~MSG
'#{event_name}' is not registered as a valid event name.
#{suggestions_message(event_name)}
All known events are:
'#{event_names.join(" ")}'
You can register the new event with:
Spree::Event.register('#{event_name}')
MSG
end

private

def suggestions(event_name)
dictionary = DidYouMean::SpellChecker.new(dictionary: event_names)

dictionary.correct(event_name)
end

def suggestions_message(event_name)
DidYouMean::PlainFormatter.new.message_for(suggestions(event_name))
end
end
end
end
7 changes: 7 additions & 0 deletions core/lib/spree/event/test_interface.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,13 @@ def performing_only(*listeners_and_subscribers)
def performing_nothing(&block)
performing_only(&block)
end

# Unregisters a previously registered event
#
# @param [String, Symbol] event_name
def unregister(event_name)
registry.unregister(normalize_name(event_name))
end
end

extend Methods
Expand Down
Loading

0 comments on commit 7a8b9a6

Please sign in to comment.