Skip to content
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

Aggregate.replay cannot work with required attributes #3

Closed
diegodurs opened this issue May 7, 2015 · 6 comments
Closed

Aggregate.replay cannot work with required attributes #3

diegodurs opened this issue May 7, 2015 · 6 comments

Comments

@diegodurs
Copy link
Contributor

In the documentation, you're using Hashie::Dash to build an aggregate and require the id property.
Something that I want too!

But this cannot work with the Aggregate.replay method since it initialize a new instance without any attributes: https://github.com/gregory/jouba/blob/master/lib/jouba/aggregate.rb#L59
Thus Hashie::Dash raises an exception ...

Aggregate.find has the same problem since it uses the replay method.

I'm wondering how you would solve this problem.

One option could be: we build the object with the first event in the stream and then replay all events: somehting like SomeAgg.new(SomeAgg.stream(uuid).first).replay(SomeAgg.stream(uuid))

But I'm wondering if an issue can arise if some procesing is done somewhere in created instance/class method or on_created...

Otherwise, if a cache is present, we can easily build the object from the last cache which has the required attributes.

My suggestion is to create a class method called rebuild(uuid) and start from there.

@gregory
Copy link
Owner

gregory commented May 7, 2015

Thanks for input!
Could you post a Dummy use case to reproduce?
Something like:

class A
 include Jouba::Aggregate
end
class B; end
 A.foo # => Error

@diegodurs
Copy link
Contributor Author

require 'jouba/aggregate'
class Customer < Hashie::Dash
  include Jouba::Aggregate.new(prefix: :on)

  property :uuid, required: true
  property :name

  def self.create(attributes)
    attributes[:uuid] = SecureRandom.uuid 
    Customer.new(attributes).tap do |customer|
      customer.create(attributes)
    end
  end

  def create(attributes)
    emit(:created, attributes)
  end

private

  def on_created(attributes)
    update_attributes!(attributes)
  end
end

And you'll never be able to call Customer.find(uuid) nor Customer.replay(Customer.stream(uuid)) since it will call Customer.new and Hashie::Dash will raise an exception for not having uuid attribute set.

@gregory
Copy link
Owner

gregory commented May 8, 2015

thx! i ll look into that in +9h, but the quick fix sounds like we could add hashie extension to ignore undefined attr.

thx for your input

Have a nice day! :)


Grégory Horion
Ruby Developer

On Fri, May 8, 2015 at 10:08 AM, diegodurs notifications@github.com
wrote:

require 'jouba/aggregate'
class Customer < Hashie::Dash
  include Jouba::Aggregate.new(prefix: :on)
  property :uuid, required: true
  property :name
  def self.create(attributes)
    attributes[:uuid] = SecureRandom.uuid 
    Customer.new(attributes).tap do |customer|
      customer.create(attributes)
    end
  end
  def create(attributes)
    emit(:created, attributes)
  end
private
  def on_created(attributes)
    update_attributes!(attributes)
  end
end

And you'll never be able to call Customer.find(uuid) nor Customer.replay(Customer.stream(uuid)) since it will call Customer.new and Dashie::Dash will raise an exception for not having uuid attribute set.

Reply to this email directly or view it on GitHub:
#3 (comment)

@gregory
Copy link
Owner

gregory commented May 9, 2015

Had a second look and understood the issue now.
Specs passed cause i haven't require the field :)

Actually think that this is a bug in the Readme and that this kind of domain validation should happen outside the domain, ie in a form object or a command object.

Indeed, there is no way we could have data "integrity" enforcement when you base your data on events, like we could have in sql systems where you can add db constraints.
We could add some data coercion but input validation should be moved to Form objects or whatever you use to get the input of your data.

Something to keep in mind too though, is that your data input validation might change with the context.
ie: you might want to validate some stuffs at user creation and not validate those stuffs on user update: term of services and other fancy attributes.

This enforce the theory that it's a bug in the readme, not to say, that the specs are green :D

what do you think?

@gregory
Copy link
Owner

gregory commented May 9, 2015

Here would be the fix:

class Customer::Create
  def self.call(form)
   Customer.create(form.attributes)if form.valid?
  end
end
class Customer::Create::Form < Reform::Form
  property :name
  validates :name, presence: true
end
require 'jouba/aggregate'
class Customer < Hashie::Dash
  include Jouba::Aggregate.new(prefix: :on)

  property :uuid
  property :name

  def self.create(attributes)
    attributes[:uuid] = SecureRandom.uuid 
    Customer.new(attributes).tap do |customer|
      customer.create(attributes)
    end
  end

  def create(attributes)
    emit(:created, attributes)
  end

private

  def on_created(attributes)
    update_attributes!(attributes)
  end
end

Faire question would be: how can i enforce the fact that any user creation in my system will have those UUID?

Since this is event based, the only way to "create" a user is by emitting a create event from your Customer domain. So if you write a spec for that to make sure that that even has uuid, you should be safe.

@diegodurs
Copy link
Contributor Author

Jouba allready requires the presence of an uuid :) https://github.com/gregory/jouba/blob/master/lib/jouba/aggregate.rb#L42

So I guess it's fair to not be able to requires attributes on initialization and decouple this by action as you shown.

I'm also wondering if it would make sense to validate some invariants after playing events.
Otherwise, someone could explicitely require mandatory entity/value-object/attribute in the initializer as this suggest: https://lostechies.com/jimmybogard/2010/02/24/strengthening-your-domain-aggregate-construction/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants