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 28, 2021
1 parent d467123 commit f420cba
Show file tree
Hide file tree
Showing 13 changed files with 541 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
41 changes: 41 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,33 @@ 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 +287,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
11 changes: 10 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,15 @@ 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 (not necessary for events provided by Solidus or other
extensions). It should be done at the end of the `spree.rb`
initializer. Example:
Spree::Event.register('foo')
Spree::Event.fire('foo')
MSG
end
end
Expand Down
94 changes: 94 additions & 0 deletions core/lib/spree/event/registry.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
# 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 events at the end of the `spree.rb`
initializer:
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 f420cba

Please sign in to comment.