Standardize way to map fields on DDD elements

[qejk]

Currently, there are two ways to map(describe) fields in DDD elements - via methods and objects:

• Entity: require to map fields as a method
• ValueObject: require to map fields as a method or object
• Aggregate/Process: require objects
• Command, Event - both store fields as objects
• Command Handlers, Event Handlers - methods (on aggregate, processor)
• API endpoints - methods

We should have standardized way to map fields, so there is no confusion what type of mapping we need to use on each DDD element and how to access them everywhere(this is very important). Another option is to allow both mappings (by doing in every place where its needed - a check for fn/object and acting accordingly) however by doing so - this can be confusing on long run.

By allowing objects were supporting extensibility, but the cost of confusion that you can extend anything from anywhere easily like that can let to bad design choices in developers code.

[rhyslbw]

I have a feeling we need to use a method for objects that are extended from Space.Struct including Entity and ValueObject. @DominikGuzei is that right?

[rhyslbw]

Also need to consider if this backwards compatibility class should be removed now, freeing up the mixin to be named `Space.messaging.Serializable'

[DominikGuzei]

We just have to mixin Space.messaging.SerializableMixin into Space.eventSourcing.Aggregate and update the internal code so that it uses the fields method instead. The switch to remove the compatibilitty class is a bit bigger and needs careful consideration.

[rhyslbw]

Consideration for upgrade path, or internal? Right now it's a little disjointed as we have an extended object that's really suited to being a trait that's added via a mixin, so can't use the obvious name in the current API.

[DominikGuzei]

We just have to check where it is used and update all places 😉

[rhyslbw]

If that's the only issue I'd suggest we proceed with that. I can do it this evening so we can release messaging without a delay. Now is the time for breaking things if there's a good reason 😄

@qejk Thoughts?

[DominikGuzei]

Haha, yeah it seems like it's never a good time to break things 😉 but yes, let's do that.

[qejk]

Yes! Like we spoke @DominikGuzei Aggregates/Processes also should be serializable Space.messaging.SerializableMixin. This will bring very niffty concept later for interacting with app layer if will start adding Repositories to the mix. Long story short:

We store on MongoDB our Aggregate via Repository (not the one from ES, related to storing our domain part via projections) and there use toData().

Then afterwards, when we need to talk with persistence (via find/findOne etc) and need whole/partial document - we recreate it with toData() and again we can interact with it normally like with EJSON's one (and still preserve sanity in doc structure yey).

IMPORTANT: will it work for partial data that have mappings in fields as non Match.Optional(so required)?

[rhyslbw]

FYI the mixin is now named Space.messaging.Ejsonable since it's a hard implementation for EJSON.

IMPORTANT: will it work for partial data that have mappings in fields as non Match.Optional(so required)?

If I'm understanding correctly, yes since it's all based on your match statement per field.

[darko-mijic]

Are we sure we want to refactor Space.messaging.Command and Space.messaging.Event also?

[DominikGuzei]

It's not so much about refactoring the names of the classes but splitting up the domain related parts (sourceId, timestamp, targetId etc.) into a separate Space.domain.Event / Command class.

[rhyslbw]

Ok so I agree here that switching the fields prop on Aggregate to a method makes sense, but we should retain the ability in the style to use an object for fields in a define static method.

[DominikGuzei]

define static method? you mean like Space.messaging.define?

[rhyslbw]

@DominikGuzei Yep, but as you pointed out in Slack, we could support object field props, which is better

[DominikGuzei]

Yep, that's definitely an option. This would be an issue for people using ES6 / CS class though.

[ghost]

Yes, please make Aggregate.fields a method for consistency with the rest of space:base and space:domain.