rails_event_store icon indicating copy to clipboard operation
rails_event_store copied to clipboard

Decouple serialisation-related logic from AsyncHandler

Open milgner opened this issue 3 years ago • 13 comments

Trying to use the async handler in my ActiveJob class like so:

class MyEventProcessingJob < ApplicationJob
  prepend RailsEventStore::CorrelatedHandler
  prepend RailsEventStore::AsyncHandler.with(serializer: RubyEventStore::NULL)

  def perform(the_event)
  end
end

This works flawlessly in a regular setup: the perform method receives instances of the ActiveRecord-based event model.

But now it's not possible to use the job manually anymore: calling MyEventProcessingJob.perform_now(some_event) will raise undefined method symbolize_keys'` because that's not a method of the Rails model class.

The current implementation reads

    def self.with(event_store: Rails.configuration.event_store, serializer: YAML)
      Module.new do
        define_method :perform do |payload|
          super(event_store.deserialize(serializer: serializer, **payload.symbolize_keys))
        end
      end
    end

I propose moving the ** and symbolize_keys somewhere else, probably into the serializer.

That way, when using the NULL serializer and calling perform with an event instance, the same event instance will end up in the job class.

milgner avatar May 24 '22 12:05 milgner

Potentially related to #945

milgner avatar May 24 '22 13:05 milgner

I can totally feel your pain with regard to current async handlers implementation and various limitations coming from the framework it leans on: https://gist.github.com/pawelpacana/295337e92dd9edd2e443e6b31ac2683a

What works for me, which can be considered as a hack, is to:

class MyEventProcessingJob < ApplicationJob
   prepend RailsEventStore::AsyncHandler

   def perform(event)
     call(event)
   end

   def call(event)
     # ...
   end
 end

or — to see it clearly — inlined:

class EventHandler < ApplicationJob
  def perform(payload)
    event = event_store.read.event(payload.symbolize_keys.fetch(:event_id))
    call(event)
  end
end

class MyEventProcessingJob < EventHandler
  def call(event)
    # ...
  end
end

and then:

  • calling MyEventProcessingJob.new.call(event) when working with event objects at hand
  • leaving ActiveJob.perform(payload) shell for typical framework calls

I consider this a hack because of instantiating framework class (inheriting from ActiveJob after all) and its a bit of grey area. Like every hack, what is acceptable for me might not be acceptable by someone else.

mostlyobvious avatar Jun 21 '22 08:06 mostlyobvious

perform method receives instances of the ActiveRecord-based event model

Just to expand a bit:

  • ActiveRecord is only inside rails_event_store_active_record gem and its repository and it never leaves it — event store operates on RubyEventStore::Record/SerializedRecord instead which is common for any repository implementation: InMemory, ROM and others.

  • ActiveJob's perform receives a hash that looks more or less like this: https://github.com/RailsEventStore/rails_event_store/blob/e424c3e57b76db8544d5d9179dc0e5c4a0e7acef/rails_event_store/spec/active_job_scheduler_spec.rb#L66-L74 and via prepend RailsEventStore::AsyncHandler we prepend another perform method which consumes this hash, turning it into Event object and passing it to handler's defined perform(event) method

  • NULL serializer only affects how data and metadata keys in that hash are transformed: https://github.com/RailsEventStore/rails_event_store/blob/e424c3e57b76db8544d5d9179dc0e5c4a0e7acef/ruby_event_store/lib/ruby_event_store/record.rb#L47-L48 and https://github.com/RailsEventStore/rails_event_store/blob/master/ruby_event_store/lib/ruby_event_store/serialized_record.rb#L45-L46

mostlyobvious avatar Jun 21 '22 09:06 mostlyobvious

Also to keep in mind, a likely direction we'll take in the future: https://github.com/RailsEventStore/rails_event_store/issues/755#issuecomment-721059147

mostlyobvious avatar Jun 21 '22 09:06 mostlyobvious

Perhaps there's also some quick win with globalid and its support in ActiveJob: https://edgeguides.rubyonrails.org/active_job_basics.html#globalid

mostlyobvious avatar Jun 21 '22 09:06 mostlyobvious

That said, I'm aware that async handler experience can be improved here. I'm not convinced yet with proposed implementation. I'd also be very cautious when introducing changes here — we'll have to support them for a longer term in the future. I'll keep the issue open as a reminder.

mostlyobvious avatar Jun 21 '22 09:06 mostlyobvious

I don't know enough about the internals of this gem yet (and I'm happy to delete my comment if this is not helpful context), but I ran into an error I think is related to this issue. It seems like async handlers can't handle OpenStruct object types as we end up with this error:

NoMethodError (undefined method `fetch' for "OpenStruct":String):
  
ruby_event_store (2.9.1) lib/ruby_event_store/mappers/transformation/preserve_types.rb:147:in `block in restore_types'

I can work around it of course, but it would be great if I was able to poke that through the un-modified OpenStruct somehow. An external API I'm consuming is returning OpenStructs and I am hoping to keep the integration layer there as slim as possible.

msencenb avatar Jun 06 '23 23:06 msencenb

@msencenb Can you please share some more bits of the code — i.e. RailsEvenStore instance configuration and the sample event that is causing this? I'm happy to help, executable code to reproduce the issue you're experiencing would make it much easier.

mostlyobvious avatar Jun 17 '23 07:06 mostlyobvious

@msencenb I've managed to recreate the issue you're experiencing: https://gist.github.com/pawelpacana/f39026514185e8f72e4daaca2d99b85a

I'll look into it more in the upcoming days. It seems that PreserveTypes transformation used by default in JSONClient is assuming a bit too much about the shape of the data.

I'm still guessing what is your exact configuration (RailsEventStore, database engine, etc.). But if you're not tied to JSON columns in the database for event_store_events table, perhaps switching to YAML serialization would give you the most of "keeping the integration layer there as slim as possible".

mostlyobvious avatar Jun 17 '23 10:06 mostlyobvious

@msencenb There's a workaround which might work without introducing too much changes:

preserve_types_as_seen_in_json_client.register(OpenStruct, serializer: ->(v) { YAML.dump(v) }, deserializer: ->(v) { YAML.unsafe_load(v) }),

That is using YAML only for particular type for storage. It works because it fits into current shape assumptions of PreserveTypes, as opposed to this:

preserve_types_as_seen_in_json_client.register(OpenStruct, serializer: ->(v) { v.to_h }, deserializer: ->(v) { OpenStruct.new(v) }),

mostlyobvious avatar Jun 17 '23 11:06 mostlyobvious

@pawelpacana Thank you and apologies for the slow response. You reproduced what I have, here is my config, which is very minimal and using the JSONClient.

require "rails_event_store"

Rails.configuration.to_prepare do
  Rails.configuration.event_store = RailsEventStore::JSONClient.new

  # Subscribe event handlers below
  Rails.configuration.event_store.tap do |store|
    store.subscribe(WiqListeners::Justifi::SubAccountEventHandler, to: [WiqEvents::Justifi::SubAccountUpdated])

    store.subscribe_to_all_events(RailsEventStore::LinkByEventType.new)
    store.subscribe_to_all_events(RailsEventStore::LinkByCorrelationId.new)
    store.subscribe_to_all_events(RailsEventStore::LinkByCausationId.new)
  end
end

I'm using Postgres as my data store. In my case I was able to just poke through an id in my event and re-pull the external resource from my partner's API.

msencenb avatar Jun 24 '23 00:06 msencenb

@msencenb we've just released v2.11.0 which should address the issue you experienced 🙂

fidel avatar Jul 20 '23 08:07 fidel

wow that was fast—thank you for the patch, I appreciate it 😃

msencenb avatar Jul 20 '23 16:07 msencenb