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

Provided showcase for planned removal of @DocStore and @Inheritance #3210

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

rPraml
Copy link
Contributor

@rPraml rPraml commented Aug 30, 2023

Hello @rbygrave

in this PR I tried to provide some code, what we are actually doing with @DocStore and @Inheritance

We use a lot of @DbJson fields in our application, where we often store polymorph classes in these fields

e.g. we have an abstract "Report" in our ReportGeneratior that exists as different concrete instances,
or we have a "RepeatDefinition" when the report should be generated, here we have "RepeatDefinitionDaily/Weekly/Monthly" that can be configured to 1..n days/weeks/months or even a RepeatDefinitionWorkdays, that can be configured to 1..n workdays.

OK, there are always ways how to implement this without inheritance, e.g by an enum "RepeatDefinitionType" - but what to do, if there is the use case to add a repeat for "every holiday"

Ebean does a great job in classpath scan here, as I just need to add a RepeatDefinitionHoliday extends RepeatDefinition and overwrite the abstrat method computeNextExecution - I do not to extend enums or add @JsonSubType to base classes.

We also use the feature to reference other entities from JSONs, that was provided by me in #1158

To get @DbJson working properly with @DocStore we needed some glue logic, to redirect Jackson-serialization back to Ebean

I could imagine a possible migration plan like this

  • @DocStore annotation will be replaced by something called @JsonEntity
  • For @Inheritance, @DiscriminatorColumn and @DiscriminatorValue we would need similar annotations. e.g. @JsonBaseType(property="kind") as replacement for @Inheritance, @DiscriminatorColumn and optional @JsonTypeName for @DiscriminatorValue
  • It would be great, if ebean detects JsonEntities on @DbJson fields and uses the built in serialization, maybe we can use @JsonEntity(useBuiltinSerialization = true) or @DbJson(useBuiltinSerialization = true)

Note: This PR is for discussion in #3112 and I do not expect, that it gets merged in this form

@rbygrave
Copy link
Member

rbygrave commented Aug 30, 2023

Why do we want Report to be a ebean bean and not a pojo (plain ordinary java object) that is serialised by Jackson (or avaje-jsonb)?

What are the features of a ebean enhanced bean that this uses versus a plain java object? Edit: I see that the pojo beans reference entity beans (so the json doc contains pojo and entity beans).

@rPraml
Copy link
Contributor Author

rPraml commented Aug 30, 2023

Why do we want Report to be a ebean bean and not a pojo

  1. We ofen use the Ebean-Properties to manipulate values. Our application supports "custom expressions", so the end user (admins only) can configure workflow actions where values in ebean beans are set or retrieved. This is done via ebeans BeanDescriptor und BeanProperty (yes, there are other ways to do this with reflection etc. as it is done with DtoBeans)
  2. We have the change-detection and the old value. We have a custom PersistListener that recursively inspect beans on save and write something similar as @ChangeLog would do. To be honest, I'm unsure if this is really needed for JsonBeans.

that is serialised by Jackson (or avaje-jsonb)?

  1. We want to reference other beans from a JsonBean. This are in most cases references to Users. Technically only the UserId is stored. When ebean serialize the User-entity, it will write only the ID (or at least it is controllable) Jackson might serialize the whole object graph (to be honest: I did not check if jackson supports something like pathProperties)

  2. When ebean deserializes this, it does a nice job on such references, like deduplication and so on

What are the features of a ebean enhanced bean that this uses versus a plain java object? Edit: I see that the pojo beans reference entity beans (so the json doc contains pojo and entity beans).

Yes and the ability to access it with BeanProperty (respectively Property/ExpressionPath)

@rob-bygrave
Copy link
Contributor

Ok, so I think we outline the options available:

  1. Keep all inheritance (so DB inheritance & JSON inheritance)
  2. Remove the DB inheritance support but keep JSON inheritance (So support @DocStore + @Discriminator type beans)
  3. Change to use Jackson or avaje-jsonb and support using entity beans inside those (via say https://github.com/ebean-orm/ebean-jackson)

Any other options?

nb: If we go with option 2, my thought is that ideally that that would work without needing the @OneToMany @ManyToOne mapping annotations.

@rPraml
Copy link
Contributor Author

rPraml commented Oct 17, 2023

PR updated, resolved merge conflicts

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.

None yet

3 participants