-
Notifications
You must be signed in to change notification settings - Fork 57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add queue-based event pub to make sure events are published in order #97
Changes from all commits
0b0225b
b2e0441
1de7799
331dc5e
959fe1f
c3d4d6d
4b3c5cb
4e2453e
ed7d206
1d55cc4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
module Sequent | ||
module Core | ||
# | ||
# EventPublisher ensures that, for every thread, events will be published in the order in which they are queued for publishing. | ||
# | ||
# This potentially introduces a wrinkle into your plans: You therefore should not split a "unit of work" across multiple threads. | ||
# | ||
# If you want other behaviour, you are free to implement your own version of EventPublisher and configure Sequent to use it. | ||
# | ||
class EventPublisher | ||
class PublishEventError < RuntimeError | ||
attr_reader :event_handler_class, :event | ||
|
||
def initialize(event_handler_class, event) | ||
@event_handler_class = event_handler_class | ||
@event = event | ||
end | ||
|
||
def message | ||
"Event Handler: #{@event_handler_class.inspect}\nEvent: #{@event.inspect}\nCause: #{cause.inspect}" | ||
end | ||
end | ||
|
||
def publish_events(events) | ||
return if configuration.disable_event_handlers | ||
events.each { |event| events_queue.push(event) } | ||
process_events | ||
end | ||
|
||
private | ||
|
||
def events_queue | ||
Thread.current[:events_queue] ||= Queue.new | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Queue only supports FIFO I think. We can safely use Array here to support both FIFO and LIFO. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should hold off on implementing something like this until someone asks for it / needs it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are changing the current behavior so I think it is good to support both cases and the current one as default, with maybe a warning message. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Neither of those scenarios is the current one. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes you are right... Okay, let's communicate this in the changelog when we release. |
||
end | ||
|
||
def process_events | ||
Sequent::Util.skip_if_already_processing(:events_queue_lock) do | ||
begin | ||
while(!events_queue.empty?) do | ||
process_event(events_queue.pop) | ||
end | ||
ensure | ||
events_queue.clear | ||
end | ||
end | ||
end | ||
|
||
def process_event(event) | ||
configuration.event_handlers.each do |handler| | ||
begin | ||
handler.handle_message event | ||
rescue | ||
raise PublishEventError.new(handler.class, event) | ||
end | ||
end | ||
end | ||
|
||
def configuration | ||
Sequent.configuration | ||
end | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
module Sequent | ||
module Util | ||
def self.skip_if_already_processing(already_processing_key, &block) | ||
return if Thread.current[already_processing_key] | ||
|
||
begin | ||
Thread.current[already_processing_key] = true | ||
|
||
block.yield | ||
ensure | ||
Thread.current[already_processing_key] = nil | ||
end | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
require_relative 'skip_if_already_processing' |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
require 'spec_helper' | ||
|
||
describe Sequent::Core::EventPublisher do | ||
class OtherAggregateTriggered < Sequent::Core::Event | ||
attrs other_aggregate_id: String | ||
end | ||
class EventAdded < Sequent::Core::Event; end | ||
class TriggerTestCase < Sequent::Core::Command; end | ||
class TriggerOtherAggregate < Sequent::Core::Command; end | ||
|
||
class TestAggregate < Sequent::Core::AggregateRoot | ||
def trigger_other_aggregate(aggregate_id) | ||
apply OtherAggregateTriggered, other_aggregate_id: aggregate_id | ||
end | ||
|
||
def add_event | ||
apply EventAdded | ||
end | ||
end | ||
|
||
class TestCommandHandler < Sequent::Core::BaseCommandHandler | ||
on TriggerTestCase do |command| | ||
agg1 = TestAggregate.new(aggregate_id: Sequent.new_uuid) | ||
agg2 = TestAggregate.new(aggregate_id: Sequent.new_uuid) | ||
|
||
agg1.trigger_other_aggregate(agg2.id) | ||
agg2.add_event | ||
|
||
repository.add_aggregate(agg1) | ||
repository.add_aggregate(agg2) | ||
end | ||
|
||
on TriggerOtherAggregate do |command| | ||
agg = repository.load_aggregate(command.aggregate_id) | ||
agg.add_event | ||
end | ||
end | ||
|
||
class TestWorkflow < Sequent::Core::Workflow | ||
on OtherAggregateTriggered do |event| | ||
execute_commands TriggerOtherAggregate.new(aggregate_id: event.other_aggregate_id) | ||
end | ||
end | ||
|
||
class TestEventHandler < Sequent::Core::BaseEventHandler | ||
def initialize(*args) | ||
@sequence_numbers = [] | ||
super(*args) | ||
end | ||
|
||
attr_reader :sequence_numbers | ||
|
||
on EventAdded do |event| | ||
@sequence_numbers << event.sequence_number | ||
end | ||
end | ||
|
||
it 'handles events in the proper order' do | ||
Sequent::Configuration.reset | ||
test_event_handler = TestEventHandler.new | ||
Sequent.configuration.event_handlers << TestWorkflow.new | ||
Sequent.configuration.event_handlers << test_event_handler | ||
Sequent.configuration.command_handlers << TestCommandHandler.new | ||
|
||
Sequent.command_service.execute_commands TriggerTestCase.new(aggregate_id: Sequent.new_uuid) | ||
|
||
expect(test_event_handler.sequence_numbers).to eq [1,2] | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
require 'spec_helper' | ||
require 'tmpdir' | ||
|
||
require 'sequent/support' | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we extract this to a separate class like the EventPublisher. Both are quite similar in intent and behavior. Same suggestion made for the EventPublisher also apply here then :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CommandService
is a separate class likeEventPublisher
already.