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

Add the AggregateMessageHandler interface, and associated types. #1

Merged
merged 6 commits into from
Dec 6, 2018

Conversation

jmalloc
Copy link
Member

@jmalloc jmalloc commented Dec 5, 2018

I've brought the Aggregate interface over from the old repo, and made some terminology changes to bring the naming more into line with official DDD terminology.

The Aggregate interface is now named AggregateMessageHandler, to make it clear that the interface is to an object that handles messages, not "the aggregate" itself, which more correctly refers to a "collection of domain objects".

The AggregateState interface is now named AggregateRoot, to reflect that fact that it is just the interface to the root object within the aggregate.

@jmalloc
Copy link
Member Author

jmalloc commented Dec 5, 2018

I am still not very happy with AggregateScope, so I'm open to more suggestions.

Edit: referring to its name, specifically.

@danilvpetrov
Copy link
Member

When looking at the closest synonyms of scope: context and span, they both are meaningful in their own areas while scope is not used anywhere that much. TBH, I don’t know why scope is so unattractive. To me it’s quite a fit.

@jmalloc
Copy link
Member Author

jmalloc commented Dec 5, 2018

I don't think it's so much that the word "scope" is bad, but that the name AggregateScope is not really conveying any information. I'll leave it for now, I just can't shake the feeling there's a better name lurking somewhere :)

@jmalloc
Copy link
Member Author

jmalloc commented Dec 6, 2018

I've pushed some fairly substantial changes to the documentation after working through it with @koden-km.

There is one other issue I feel that should be raised in this PR, which is whether or not AggregateScope should have CreateInstance() and DestroyInstance() methods, akin to the workflow's BeginWorkflow() and EndWorkflow().

In Ax, aggregates are created whenever they receive a command. We do have the ability to "complete" a saga of any type, including aggregates. The exact meaning of complete depends on whether the saga is using CRUD or ES persistence. "deletion" proper doesn't make much sense when doing ES, but as it stands, there would be no way to delete a CRUD aggregate with the Dogma interfaces.

@jmalloc jmalloc self-assigned this Dec 6, 2018
Copy link
Member

@koden-km koden-km left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs are much clearer now. Just the suggestions/questions.

aggregate.go Show resolved Hide resolved
aggregate.go Show resolved Hide resolved
aggregate.go Outdated Show resolved Hide resolved
@koden-km
Copy link
Member

koden-km commented Dec 6, 2018

Docs are good now.

As for AggregateScope having CreateInstance() and DestroyInstance():

  • Who is expected to be calling those, the engine or the application?
  • They do make sense for CRUD. Create works for ES, but Destroy is more like archive and/or make read only? There is also the GDPR thing, which might mean Destroy for ES is at least partially valid (destroy content but keep ids for idempotency)?

@jmalloc
Copy link
Member Author

jmalloc commented Dec 6, 2018

  • Who is expected to be calling those, the engine or the application?

The application. The way the equivalent methods work for workflows is that Begin() must be called before any other operation on WorkflowScope can be performed. Conversely, after End() is called, no future operations can be performed. Logging is the exception to this, it is allowed at any time.

  • They do make sense for CRUD. Create works for ES, but Destroy is more like archive and/or make read only? There is also the GDPR thing, which might mean Destroy for ES is at least partially valid (destroy content but keep ids for idempotency)?

These are all valid points and approaches. I would expect these decisions to be considered "implementation defined", that is, engine specific. You could even have an ES implementation which stores events to represent create and destroy operations then only loads state from the most recent create event.


It sounds like we agree that these are a necessity for any CRUD-based engine, anyway. Create() and Destroy() might be good names, as Destroy() feels a little more flexible than Delete(), for the reasons you described.

On a slightly different note, I believe these operations might be a good fit from an "expressing intent" point of view as well. Some aggregates merely need to track their existence, without further state. These methods would provide a way to do that without having to define an AggregateRoot implementation with an Exists flag, as is the case for the bank.Transaction aggregate in the bank example.

@jmalloc
Copy link
Member Author

jmalloc commented Dec 6, 2018

I will merge this PR, and create a separate one for further discussion and implementation of the create/destroy operations.

@jmalloc jmalloc merged commit ac8c8a4 into master Dec 6, 2018
@jmalloc jmalloc deleted the aggregate branch December 6, 2018 23:59
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

Successfully merging this pull request may close these issues.

3 participants