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

Extended generics for Identifier interface #58

Open
odrotbohm opened this issue Aug 31, 2021 · 0 comments
Open

Extended generics for Identifier interface #58

odrotbohm opened this issue Aug 31, 2021 · 0 comments
Labels
lifecycle: for-team-discussion module: ddd Domain-Driven Design related support type: enhancement New feature or request

Comments

@odrotbohm
Copy link
Member

Summary

Does it make sense to enrich the Identifier interface with generic parameters so that the model implementation can be inspected for valid relationships more easily and does the effect of that move outweigh the cost (breaking change, a slight bit more verbosity / boilerplate)?

Context

Identifier is currently a marker interface that doesn't use any generics to establish a relationship to an Identifiable. This causes ambiguities when inspecting a type using multiple Identifier implementations:

class Order {
  OrderIdentifier id;
  CustomerIdentifier customerId;
}

While a human can probably guess that id contains the identifier of Order (due to the field name and type name pattern), without further assumptions there's no way for tooling to differentiate between the identifier of the owning type and an identifier that refers to some other identifiable

Current ways to deal with this situation

We already have Association that allows to explicitly declare relationships to other Aggregate instances. One can argue that instead of using an Identifier as field type directly, one should rather use Association to express the relationship explicitly. That said, there are a couple of constraints this imposes which it would be nice to get input for whether they're a good thing because they're guiding developers to do the right thing or rather perceived as invasive:

  • Relationships can only point towards an AggregateRoot. While that can be considered appropriate as we must not point towards an entity contained in another aggregate root. That said, the relationships to entities contained in the owning aggregate root always have to be modeled as entity instances. Mapping only the identifiers results in the problem mentioned above. Does it actually make sense to only map the identifier of a contained entity in the first place? If so, how would you materialize it? Are there modeling cases in which using an ID is just sufficient, and you wouldn't necessarily need to materialize it?
  • Mapping an Association to a persistence model can be a bit tedious. Currently, integration into JPA exists in jMolecules Integrations, which works around some JPA limitations by a bit of code generation. It's obvious that you could just argue that this is not an argument, as a separate persistence model is preferred anyway.

Potential solution

If we come to the conclusion that we would like to support multiple Identifier implementations as field types in a model class, one way to tighten the model would be to introduce generic parameters to Identifier. This would need quite a few changes to existing generic type arrangement that would clearly break existing applications. Here's what the move would look like:

Before After Description
Identifiable<ID> Identifiable<T extends Identifiable<T, ID>, ID extends Identifier<T, ID>> New generic parameter
Identifier Identifier<T extends Identifiable<T, ID>, ID extends Identifier<T, ID>> Two new generic parameters, previously none
Entity<T extends AggregateRoot<T, ?>, ID> extends Identifiable<ID> Entity<T extends AggregateRoot<T, ?>, S extends Entity<T, S, ID>, ID extends Identifier<S, ID>> extends Identifiable<S, ID> Three generic parameters instead of two -> breaking
AggregateRoot<T extends AggregateRoot<T, ID>, ID extends Identifier> extends Entity<T, ID> AggregateRoot<T extends AggregateRoot<T, ID>, ID extends Identifier<T, ID>> extends Entity<T, T, ID> Identifier tightened, breaks compilation for unadapted Identifier implementations
Association<T extends AggregateRoot<T, ID>, ID extends Identifier> extends Identifiable<ID> Association<T extends AggregateRoot<T, ID>, ID extends Identifier<T, ID>> extends Identifiable<T, ID> Identifier tightened, breaks compilation for unadapted Identifier implementations
Repository<T extends AggregateRoot<T, ID>, ID extends Identifier> Repository<T extends AggregateRoot<T, ID>, ID extends Identifier<T, ID>> Needs adaption of Identifier

Examples of necessary changes from the Spring RESTBucks example:

Before After
OrderIdentifier implements Identifier OrderIdentifier implements Identifier<Order, OrderIdentifier>
LineItem implements Entity<Order, LineItemIdentifier> LineItem implements Entity<Order, LineItem, LineItemIdentifier>
PaymentIdentifier implements Identifier PaymentIdentifier<T extends AggregateRoot<T, PaymentIdentifier<T>>> implements Identifier<T, PaymentIdentifier<T>>

Here are the major effects of that move:

  • We now need Identifier implementations per Identifiable. Previously, it was possible to share Identifier implementations. Adding the generics primarily affect type hierarchies (like Payment in the example above).
  • There's a degree of boilerplate introduced as both Identifier and Entity now need to be self-referencing. Entity would end up with three generic parameters, which can understandably perceived as too noisy.
  • Identifiable's getId() method now returns a parameter that is bound to a generic type parameter containing a bound. The code generation integration for Spring Data's Persistable that previously was able to use the induced getId() method from the domain type is now not properly bound to that method anymore. The test cases contained in the adapted jMolecules Integrations prototype branch fail to invoke the correct method. I assume, there's some bridge method missing being created during code generation. I'd investigate this further if we decided to pursue this in the first place.

WIP Prototype

There are branches that explore the feasibility of this:

@odrotbohm odrotbohm added type: enhancement New feature or request module: ddd Domain-Driven Design related support lifecycle: for-team-discussion labels Aug 31, 2021
odrotbohm added a commit that referenced this issue Aug 31, 2021
Fixed versions of build plugins.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle: for-team-discussion module: ddd Domain-Driven Design related support type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant