Default DomainEventPayload

[Sandreu]

Hello,

I found while using that domain events payload definition is often copy in this of the event argument :

var domainEvents = 
  create: (attributes) ->
    @[key] = val for key, val of attributes
    @
  remove: ->
    @
  #....

context.addDomainEvents domainEvents

I figured out that almost every domainEvent definition is quite dumb. The real brain is the aggregate.

What do you think about put the payload into the domainEvent from the context::emitDomainEvent method if the domainEventClass is not defined ?

To be more restrictive it could be only if the payload is an object... or put it straight in payload if the domainEventClass is not defined.

I talk about what happens here

I will pull request if you agree.

[Sandreu]

I just figured out that this doesn't matter.
My example could be

var domainEvents = 
  create: (attributes) ->
    attributes
  remove: ->
    @
  #....

This makes the solution even more simple !

emitDomainEvent: (domainEventName, domainEventPayload) =>
    DomainEventClass = @getDomainEvent domainEventName
    if !DomainEventClass
      DomainEventClass = (payload = {}) -> payload

    domainEvent = @_createDomainEvent domainEventName, DomainEventClass, domainEventPayload
    @saveAndPublishDomainEvent domainEvent
    .then =>
      @_eventric.log.debug "Created and Handled DomainEvent in Context", domainEvent

[Sandreu]

Do you want me to PR this with tests ?

[Deleted user]

Hi @Sandreu - happy new year =)

We did talk internally some time ago if it would make sense to automagically create DomainEvent Classes. Back then the decision was made, that we do not want this mainly because it might be prone to errors - especially if you have typos in the DomainEvent Name.

Consider two emitDomainEvents, one emits 'SomethingHappened', the other 'SomethingHapened' - both will get emitted with no hint about the typo. If you define the DomainEvent explicitly, the one with the typo will fail. Same goes for DomainEvent attributes - event though its (currently) not as easy to spot typos there.

Maybe this might be a separate mode to activate, but currently I'm not sure about this. Can you see the point? What do you think?

Best regards,
Johannes

[Sandreu]

Thank you Happy new year to you too ! With a great 0.2 release of eventric ! ...And a good health too... ;) !

Yes I thought about this. An optionnal mode can be a solution.
I also imagined to define domainEvents just with a string. addDomainEvent could have the domainEventFn optionnal, and for addDomainEvents, the argument can be an object of functions as today or an array of string... ?

What do you think ?

I'm currently working on a module to connect eventric and React (without eventric on client side but with a kind of shared projections to avoid playing all past domainEvents at init)... I think that will be nice ! And I think I will soon pull request projections like sagas as we evoqued on #12

I'll start working on it (projection sagas) tomorrow.

Regards.

[Deleted user]

Thanks :)

Defining DomainEvents with optional Class sounds good for me. @alexlawrence @medialwerk Do you guys see any issue with this?

Cool, the React stuff sounds interesting. We actually wanted to try that out too, because it might be a good match.

Global Projections is still a thing too, yep. I'll open a ticket for that.

[alexlawrence]

I wouldn´t recommend to introduce this default mode and even if I wouldn´t reccomend to use it. I would like to have a more restrictive mode instead. Domain events are the most valuable things in an event sourced system and we should do everything to ensure their structure is correct.

Example:
You develop a newsletter signup feature. The resulting event is "SubscribedToNewsletter". The payload attributes are "email", "firstName" and "lastName". Now you make a typo and write "fristName". You go into production and save domain events. Later on you want to do reporting and create a projection. Now you realize there is a typo and you correct it. Since events should "never" be altered your projection has to handle events that have a "fristName" and some with a "firstName" attribute. If you would have ensured that the payload attributes are provided correctly you would have recognized the error before going into production.

I completely agree with that the domain event definition functions are a bit verbose because it is always just assigning properties from the params object. However the intention was to make clear what properties and event should have.

So why not thinking about how we could make the syntax more expressive and gain the possibility to check for valid params?

Suggestion:

context.defineDomainEvent('SubscribedToNewsletter', ['email', 'firstName', 'lastName']);

This way we don´t have to write a function and we could check upon emitDomainEvent() that email, firstName and lastName were given.

Alternative:

context.defineDomainEvent('SubscribedToNewsletter', {
  email: String, 
  firstName: String, 
  lastName: String
});

Here we could even check the types if we wanted to.

[Deleted user]

I really like the idea to make the definition of a DomainEvent more like a schema! And I do absolutely agree that DomainEvents (and thus their definitions) are important.

[Sandreu]

Yes I think that type definitions is a good idea ! Still, a joker should be possible.
Sometimes your payload could be anything !

React props validation is nice to use.

But this become a kind of validator... isn't it the aggregate responsibility ?
What about a type validator in aggregate context instead...?

Still... I like the idea too. Making it more restrictive as you said make events declarations more valuables. As it is today, it's verbose with no value.

[alexlawrence]

Still, a joker should be possible. Sometimes your payload could be anything !

Could you give me an example? I can´t really think of a scenario and dislike the idea of having "anything" in the domain event payload.

But this become a kind of validator... isn't it the aggregate responsibility ?

I guess it depends. There are always two kinds of validation. One is simple type and existence validation (is field filled, is field an e-mail, etc.) and the other is the validation of business rules (is the e-mail blacklisted). Clearly business rules should be validated in the aggregate.

However I´m unsure about using schemas with strict types. I like JavaScript for not having strict types and I´m unsure if this could have negative effects. Aren´t there other possibilities? Are all fields always required or could we have optional fields?

context.defineDomainEvent('SubscribedToNewsletter', {
  email: 'required', 
  firstName: 'required', 
  lastName: 'required',
  middleName: 'optional'
});

[Deleted user]

Personally I like the idea of having an array. This way we could say: If an entry is a string, we only check the presence. If its an object it could have more validation options (like required, type, etc).

context.defineDomainEvent('SomethingHappened', [
  'email',
  {name: 'firstName', required: true}
]);

This way we could start to implement the easier one (only array with strings). Completely omitting the array could accept any payload (even though we discourage this kind of usage in the docs then).

[Sandreu]

We talked about validation rules in #6...
What I retained form this, is that the aggregate doesn't know outside world.
And a uniqueness test should be done in command handler.

If you have to validate upon the state of the command itself, that should happen in the command handler

For example I'm building an environment where every attributes of an entity can be personalized. the I have events to create/update those attributes... and this object of keys/values is my payload.
I know that there is workaround... I could use an array of objects with key and value [{key:'foo', val:bar}, {key......} but this adds useless verbosity...

I think that the real value is in domain event names not in payload... If you take your todo example... Every payloads set in domain_events are not valuable at all ! Projection has to set those value but @removed = true is already said in TodoRemoved ! Same thing with TodoCompleted. the only one really using payload is Title handling... In this case event payload could be a string instead of an object with a title key...

For me payload is a poor value, and this just has to be checked in aggregate...

Take a look (around 1 minute) to React types validation rules this is lighweight and does exactly what you want !

But still ! ;)
I think that your way is a good way to validate event attributes. This is just a chat about the best place to handle typo issues... :)

[Deleted user]

I agree, sometimes there's no need for DomainEvent payloads - the TodoMVC is a good example. Also I wouldn't go with the full-fledged validation solution just yet. Would the proposed array/string solution fit your needs?

[Sandreu]

But still ! ;)
I think that your way is a good way to validate event attributes. This is just a chat about the best place to handle typo issues... :)

I agree that array in domainEvent declaration is a good solution. I just asked if payload validation isn't an aggregate responsibility. And we are just thinking about the best place to do it... talk before code avoid many useless commits ! :)

This is to be clear about who you (as product owners :) ) want to orient those validation rules... The way it is today already works fine ! this is just very verbose... but there isn't any kind of emergency in those concerns ! I will work on global projections before ! :)

[Sandreu]

This solution is the easiest to implement... I just ask if it is the good one... :)

[Deleted user]

You stated correctly

As it is today, it's verbose with no value.

A simple presence-check would give it value and make it less verbose. So yeah, that should definitely be a concern of the DomainEvent itself - at least from my perspective. Of course you will need extra validation in CommandHandlers and Aggregates too - but this is another story.

[Deleted user]

I will work on global projections before ! :)

Great. Could you please open a issue beforehand so that we can discuss the details?

[Sandreu]

Ok :)

[alexlawrence]

We talked about validation rules in #6...
What I retained form this, is that the aggregate doesn't know outside world.
And a uniqueness test should be done in command handler.

Yeah, that wasn´t the best example. Let´s say if you want to check whether an e-mail was blacklisted or not. You would do this in the aggregate (probably via using a domain servcie to make some request). However checking that an e-mail is a string is just type validation and does not belong in the aggregate.

For example I'm building an environment where every attributes of an entity can be personalized.

I don´t really understand what scenario this would be but I would recommend to question such a payload. Let´s say you have a registry of persons and details of persons can be changed. You could go along and introduce the event "PersonChanged" and just give some arbitrary payload. However this wouldn´t really conform to the idea of event based/sourced systems. Your events should describe what happened in a specific way (behaviour, intent). So you should have events like "PersonNameChanged" and "PersonAddressChanged". Otherwise you risk to implement a CRUD like system.

For me payload is a poor value, and this just has to be checked in aggregate...

I´m not sure what you mean by poor value. The payload has actually a lot of value. :-)

[alexlawrence]

Personally I like the idea of having an array. This way we could say: If an entry is a string, we only check the presence.

👍

[Sandreu]

So you should have events like "PersonNameChanged" and "PersonAddressChanged". Otherwise you risk to implement a CRUD like system.

I totally agree ! The value (for the dev) is in "PersonNameChanged", in this case the payload should be a string. If your payload is an object, that means (as it is in my case) that keys and values have no interest... The intend is the domainEvent name not in the payload... Payload is just user provided details about the intend.

With the PersonAddressChanges example the payload can be an object, but it's the aggregate duty to check that the zip code is a valid one or to be shure that the street exists... Your aggregate has to check the integrity of your payload. And with your kind of definitions you'll have to describe your payload 3 times : 1 in the form, 2 in the aggregate and 3 in domainEvents definitions.

In my case keys and values of the payload are user defined. I don't want to check anything.

Again... I don't say that arrays in domainEvents isn't a good idea ! This is a clean way to handle domainEvents... but I just ask if it is the good place to do this kind of validations.
This probably the good way, I'm just thinking with you about this while we are talking. :)

[alexlawrence]

And with your kind of definitions you'll have to describe your payload 3 times : 1 in the form, 2 in the aggregate and 3 in domainEvents definitions. In my case keys and values of the payload are user defined. I don't want to check anything.

If even the keys of your payload are user defined then I can understand that you don´t want to have a strict domain event definition. However I still do not understand the use case. In general I would prefer to explicitely define the command params, event params and even the aggregate params. They
make look similar but they reside in three different application layers. And explicit is always better than implicit.

While this discussion is interesting, it´s getting slightly offtopic regarding the ticket. So all in all I would choose not to implement a default payload and I like the concise array like definition.

[Sandreu]

You're the boss ! :) And certainly have more experience in ES than I do !
I am 100% fine with array event definitions :)

And explicit is always better than implicit.

And you chose coffee !!! ;) Just a joke !

[alexlawrence]

I don´t know whether one has more or less experience in ES. I just wanted to bring up my suggestion so we can make a decision. @JohannesBecker @medialwerk What is your opinion? Default payload yes/no? Array like payload definition yes/no?

And you chose coffee

It´s not that I´m completely satisfied with CS but it has its nice parts. ;)

[Deleted user]

Yes, lets take the array definition: Checks presence and warns if parameters are given which are not defined in the array.

@Sandreu About having user-defined payloads: Why not define a special DomainEvent Parameter like whatever which holds the user-defined stuff?

[Sandreu]

Yes I told that I can deal with it. I'm 100% all right with this solution !