Feedback and discussion on this POC

[hohwille]

Hi @trivesc

thanks for your contribution to make devon4j better. I appreciate this a lot and will try to support you.

Do you have customer demands that define requirements on the technical data model and force you to do so? (do not tell details and customer names here in OSS space)

This is adding quite some complexity that can be avoided by having a technical primary key and maintaining the business key as additional attributes instead. However, in some cases I can see your need and am willing to extend devonfw with your feature. But if we know your use-case and demands more detailed, we can better design and plan changes.

public class NameEntity extends ApplicationPersistenceEntity<Long> implements Name<Long>

I do not understand why NameEntity is already bound to Long while the entity interface Name still carries the generic type. IMHO this does not make sense and as generics are sometimes nasty for developers (esp. beginners) we should avoid them where possible. So IMHO it should be just Name here and that interface can then have the in its extends declaration.

Further we need to think about compatibility. ApplicationPersistenceEntity already exists in hunders of devonfw projects. Chaning this and the cobigen templates in an incompatible way is IMHO a very bad idea. Instead you can introduce a new super type ApplicationGenericPersistenceEntity<ID> and existing projects can modify ApplicationPersistenceEntity to extends ApplicationGenericPersistenceEntity<Long> and their code will still work and they can continue to use CobiGen with the same templates. Do you agree and keeping compatibility for existing projects even if it leads to somewhat more artifical names?

[trivesc]

Hello @hohwille, Yes, this is something that has been demanded in Yammer https://www.yammer.com/capgemini.com/#/threads/show?threadId=39821155885056 and in some projects. In addition, in this PoC i left the modificationCounter, as i think its a great idea to keep it and its something that might be easier to convice the customer about.

I agree with you that leaving a technical primary key and using a bussiness key would be the best idea, as keeping the technical primary key is better for faster indexing.

In regards, to the NameEntity being bound and Name being generic, this is something that had to be done to allow the use of a ComposedKey Object. An ComposedKey object needs the annotation @Embeddable. The library that has this annotation is not being imported in the API side, i thought this was done in purpose so people dont start using annotations on the API side. So, a solution was to have two objects, one CompositeKey and a CompositeKeyImpl. Having a generic Name interface, allowed for Name allowing to use ComposedKeyImpl in the CORE and ComposedKey in the API, finally letting us have annotations. But i do agree that this case should be only used in the ComposedKey case and not in the Long case.

I do agree with you that, keeping the compability for existing projects and letting this be a separated possibilty is a much better idea since this is a small scope, even if it leads to more artificial names.

Am going to modify the PoC and using your suggestion am going to keep the default devon4j structure classes and create separated ones for this case.

Thanks and regards

[hohwille]

Hi @trivesc thanks for your explanation. I see your point but would still question if we need to separate CompositeKey and CompositeKeyImpl. Otherwise you can still override return types and cast arguments in setters to avoid the generic. When I started coding with generics I used to spam my code with generic and then developers were complaining about the complexity. Now I always try to question them and only use them where really required.

BTW: Will the user have the freedom of choice to define his own CompositeKey[Impl] and can he have multiple of them? How would CobiGen be able to detect this?

[trivesc]

Hello @hohwille When i say CompositeKey[Impl] i mean serializable objects that have to be created manually for entities that have a composite key.

Regarding CobiGen, when i use CobiGen from an entity that has an specified id with a Type thats different of Long, it generates the proper Type in the interface,eto,cto. But, some manual changes are going to be needed since the generator uses ApplicationPersistenceEntity as default for example.

Regarding about removing as much as generic typing as possible to simplify a question rises, should we create ApplicationGenericPersistenceEntity or should we create ApplicationComposedKeyPersistenceEntity ?

In the second case, i was able to make it work with only CompositeKey, without having to create an implementation to add annotations. This is due to being able to put @EmbeddedId in the id of ApplicationComposedKeyPersistenceEntity<>. I dont really know why, but this way, hibernate is detecting the embedded class without the need of @Embeddable. This change, also avoids having to cast classes.

The bad thing about this is that, it doesnt support ApplicationComposedKeyPersistenceEntity<Long/String....>, but the default ApplicationPersistenceEntity can be used for normal entities and ApplicationComposedKeyPersistenceEntity<> for the CompositeKey entities.

You can see this last change in this commit .

Thanks and regards

[hohwille]

Maybe ApplicationPersistenceEntity can remain untouched. Users are free to create their own mapped superclasses implementing PersistenceEntity<...>. Then only CobiGen would need to be extended if you want full generation support such that CobiGen is only triggered by PersistenceEntity (rather than ApplicationPersistenceEntity) and auto-detects the ID type. Then it could derive Eto and BO-Interface from «Prefix»Eto and «Prefix»Entity. So the default «Prefix» would be Application what implies the defaults (Long as primary key, modificationCounter, etc.) but everybody is free to create itws own hierarchies like:

• CustomerNumberEntity
• CustomerNumberPersistenceEntity
• CustomerNumberEto

Then actually nothing has to be changed except a little extension of CobiGen templates. WDYT?

[hohwille]

IMHO the only things left to do in order to complete the feature would be:

• add and link some new guide to the documentation of devon4j describing the pattern
• extend cobigen templates in order to detect the parent entity type and not only support the fixed ApplicationPersistenceEntity but also your custom super entity type (no matter if it has a composite key or it was introduced for other customizations such as e.g. using private field JPA annotation style instead of public getter method annotations).

It would be great if you could raise according issues in devon4j and tools-cobigen repo.

[sjimenez77]

Could we recover this PoC, update it and integrate finally in devon4j @hohwille, @vapadwal?

[sujith-mn]

Cobigen changes IMHO is (eclipse)

Prerequisite:

• Entity class should have an "id" instance of composite key.
• The composite Key class must be defined in the api module.

Cobigen changes:

• Cobegen generate Select Dialog should have an option to select "Composite key based ID" (default will be long)
• Cobgen should make entity class to the extend "ApplicationComposedKeyPersistenceEntity", replace the actual class with ID_class (this can be identified from the entity class)
• The below classes are new to cobigen ApplicationComposedKeyPersistenceEntity interface AbstractComposedKeyEto class ApplicationComposedKeyEntity class AbstractComposedKeyEto class
• Other user classes which are dealing with Entity needs to accommodate Composite key.