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

Explicitly document the different Panache ORM approaches #6135

Closed
sdaschner opened this issue Dec 12, 2019 · 4 comments · Fixed by #6923
Closed

Explicitly document the different Panache ORM approaches #6135

sdaschner opened this issue Dec 12, 2019 · 4 comments · Fixed by #6923

Comments

@sdaschner
Copy link
Contributor

sdaschner commented Dec 12, 2019

The ORM with Panache guide describes how to create repositories that access the entities.
However, we might be more explicit in the different approaches, that for
repositories, you don't need to define PanacheEntity, but it already
works with plain JPA entities. If you think about it, it makes sense,
however the sections and the code examples could be more explicit:

  • More explicit "take either option 1 or option 2", e.g. in the headlines
  • The PersonRepository reuses the Person entity in the code, which is only defined as extends PanacheEntity. This might not be obvious that both works (but only the plain JPA entity approach makes sense for the latter, IMO).

Additionally, we might want to add a word on when to chose which approach.
From my point of view:

  • Active PanacheEntity makes sense for simpler entities that don't represent a complex DDD domain entity, aggregate or hierarchical type.
  • PanacheRepository makes sense for "cleaner" entities (only domain-specific methods), complex examples, hierarchical types, entities that extend other classes, and complex database operations.

For the latter reasons, I'd prefer the repository approach, however, we should just be more explicit that both is perfectly possible.

Thoughts? Btw, happy to provide some doc suggestions.

@gsmet
Copy link
Member

gsmet commented Dec 12, 2019

/cc @FroMage

@FroMage
Copy link
Member

FroMage commented Dec 12, 2019

So I don't think it's correct that you should pick between the two approaches, because enhancing the entity gives you benefits even if you use the DAO approach:

  • ease of writing (no need for accessors)
  • default ID strategy
  • DB methods for all the times you don't need a DAO for DDD operations

We can make it more explicit that extending EntityBase is not required, but still recommended.

I also disagree that you cannot use enhanced entities for complex examples, hierarchical types or entities that extend other classes [unless those classes are outside of your control] and complex db operations. I also strongly disagree that PanacheRepository makes sense for "cleaner" entities.

As for DDD, I do agree that if you have operations that work on a mix of entity types if can be clearer to put those operations in a dedicated repository, but that's rarely the general case: you will have mostly per-entity operations (or at least operations that strongly related to a main entity) and some mixed cases. We should not discourage people from simplicity for all the easy cases just for a few rarer cases.

We can certainly add some sentence saying that while you should always start by enhancing your entity, if you have some operations that really bother you to place in a single entity type and pick a side, you can resort to a repository for those.

@loicmathieu
Copy link
Contributor

A note on testability can also be added, repository are easier to mock than entities that relies on static methods.

@sdaschner
Copy link
Contributor Author

Hi @FroMage

I see your point. I think the important additional information to give to the users is that the repositories also work with plain JPA entities. This is also an interesting point for migrating existent code. Just letting the user know they have this power and that they're free to choose which mix&match makes sense for them.

loicmathieu added a commit to loicmathieu/quarkus that referenced this issue Jan 31, 2020
loicmathieu added a commit to loicmathieu/quarkus that referenced this issue Feb 3, 2020
loicmathieu added a commit to loicmathieu/quarkus that referenced this issue Feb 4, 2020
loicmathieu added a commit to loicmathieu/quarkus that referenced this issue Feb 10, 2020
loicmathieu added a commit to loicmathieu/quarkus that referenced this issue Feb 26, 2020
Fixes quarkusio#6135 and quarkusio#6945
WIP Apply suggestions from code review

will be squashed later

Co-Authored-By: Georgios Andrianakis <geoand@gmail.com>
@gsmet gsmet added this to the 1.3.0 milestone Feb 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants