envato / event_sourcery

A library for building event sourced applications in Ruby
MIT License
84 stars 10 forks source link

Upcasters #96

Closed stevehodgkiss closed 6 years ago

stevehodgkiss commented 7 years ago

Upcasters are a function applied to event bodies of a given type. Multiple can be defined for a given type and they're executed in the order they're defined. Useful when the schema changes and in order to fill in default values to avoid spreading the default knowledge across the event consumers.

vonconrad commented 7 years ago

Love to see a usage example. 😄

stevehodgkiss commented 7 years ago

https://github.com/envato/event_sourcery/pull/96/files#diff-2f6feae8e2eeacf3bef7946ac34b2740R6

Usage in an app in it's current state would be EventSourcery.config.upcaster_chain.define('ItemAdded') { |body| ... }

stevehodgkiss commented 7 years ago

I'm wondering if upcasters should instead be defined as classes themselves, then added to an upcaster chain. Perhaps like this:

class Upcasters::DefaultCurrencyToUSD
  include EventSourcery::Upcaster
  event_type ItemAdded

  def upcast(body)
    body['currency'] ||= 'USD'
  end
end

EventSourcery.config.upcaster_chain.add(
  Upcasters::DefaultCurrencyToUSD,
  Upcasters::SomeOtherUpcaster
)

Thoughts?

stevehodgkiss commented 7 years ago

Also pondering whether to not mutate the body and instead return a new body in each upcaster... the logic would be the same body['currency'] ||= 'USD' just on a .dup'd body (we'd .dup the body before supplying to each upcaster function).

orien commented 7 years ago

@stevehodgkiss and co: you've thought about this a lot longer than I have, but a much simpler implementation would be to add it as a attribute reader on a custom event class. This has the advantage of encapsulating all event specific knowledge in one class.

class ItemAdded < EventSourcery::Event
  def currency
    body['currency'] || 'USD'
  end
end

Perhaps with some syntactic sugar:

class ItemAdded < EventSourcery::Event
  event_attribute :currency, default: 'USD'
end
orien commented 7 years ago

+1 to not mutating the body too

mjward commented 7 years ago

Sorry for coming late to the party, head in another space recently. I think I prefer your 2nd suggestion @stevehodgkiss, that upcasters should be their own class. Keeps the config clean and these things become more of a "thing".

That being said, @orien suggestion makes more sense to me. Move the logic onto the event, rather than going in via the backdoor to config. Makes things much easier to reason about.

stevehodgkiss commented 7 years ago

Since methods on event classes to access body attributes is optional and usually not the way of accessing the event body. My preference would be not use them to upcast bodies, but that's definitely an option if you prefer that approach. Most apps access the body directly so I'd rather have the upcasting behaviour be applied before the event is built.

In terms of testing upcasters you really want to test the whole chain vs individual upcasters since the behaviour of another upcaster for the same event type could just override what you do with the first one. I'll have a think about how that could be modelled as a class rather than a config style thing.

One option:


class MyUpcasterChain < EventSourcery::UpcasterChain
  define ItemAdded do |body|
    body['currency'] ||= 'USD'
  end
end
twe4ked commented 6 years ago

A simple dup on the hash won't work with nested hashes.

twe4ked commented 6 years ago

We've been using upcasting with Event Sourcery for a while by overriding our EventBuilder:

class EventBuilder
  def initialize(event_type_serializer:)
    @event_type_serializer = event_type_serializer
  end

  def build(event_data)
    type = event_data.fetch(:type)
    klass = event_type_serializer.deserialize(type)
    upcast(klass.new(event_data))
  end

  private

  attr_reader :event_type_serializer

  def upcast(event)
    if event.class.respond_to?(:upcast)
      event.class.upcast(event)
    else
      event
    end
  end
end

Example usage:

# event definition
TodoCreated = Class.new(EventSourcery::Event) do
  def self.upcast(event)
    body['content_type'] ||= 'photo'
    event.with(body: body)
  end
end
stevehodgkiss commented 6 years ago

@twe4ked nice. Perhaps a default implementation of .upcast could return self to avoid having to use respond_to?. Do you want to make a PR with that approach?

twe4ked commented 6 years ago

Perhaps a default implementation of .upcast could return self to avoid having to use respond_to?. Do you want to make a PR with that approach?

Yeah I did consider that, sounds good. Feel free to PR it or I'm happy to. Let me know :)

stevehodgkiss commented 6 years ago

@twe4ked I'm happy for you to make the PR when you have time 😜