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

Register for reflection the hierarchy of the entity in MongoDB with Panache #15503

Merged

Conversation

loicmathieu
Copy link
Contributor

Fixes #13301

I register for reflection the hierarchy of the entity and not juste the entity itself.

I didn't add any test yet as I wanted to have some feedback on the usage of the ReflectiveHierarchyBuildItem as it's the first time I used it. Is my usage correct ?
I notice that Hibernate ORM didn't use it and instead use a rather complex way of climbing the hierarchy of types: https://github.com/quarkusio/quarkus/blob/master/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/JpaJandexScavenger.java#L228

@quarkus-bot
Copy link

quarkus-bot bot commented Mar 5, 2021

Thanks for your pull request!

The title of your pull request does not follow our editorial rules. Could you have a look?

  • title should preferably start with an uppercase character (if it makes sense!)

This message is automatically generated by a bot.

@loicmathieu loicmathieu changed the title fix: register for reflection the hierarchy of the entity Register for reflection the hierarchy of the entity in MongoDB with Panache Mar 5, 2021
@loicmathieu
Copy link
Contributor Author

@geoand can I have your advice on this one ?

@geoand
Copy link
Contributor

geoand commented Mar 9, 2021

That JPAScavenger thing is pretty old... ReflectiveHierarchyBuildItem is definitely the way to go. Just make sure you aren't registering too much for reflection depending on your use case.

What is the exact problem you are trying to solve?

@loicmathieu
Copy link
Contributor Author

The issue is exaplained in #13301
Today we register the mongo discovered entity class (from the parameter type of the repository), but if this class extends a base class we didn't register the base class as we didn't climb up the hierarchy of classes.
So basically I wanted to register the hierarchy of the class instead of the class.

@geoand
Copy link
Contributor

geoand commented Mar 9, 2021

ReflectiveHierarchyBuildItem sounds perfectly reasonable in this case

@loicmathieu loicmathieu force-pushed the fix/mongodb-panache-register-hierarchy branch from 24741d5 to bd5a6c5 Compare March 11, 2021 18:52
@loicmathieu loicmathieu marked this pull request as ready for review March 12, 2021 08:05
@loicmathieu loicmathieu requested a review from geoand March 12, 2021 08:06
@loicmathieu
Copy link
Contributor Author

@geoand I added a test, this PR is ready to go.

Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

LGTM!

@geoand geoand added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Mar 12, 2021
@geoand geoand merged commit a4597d9 into quarkusio:master Mar 12, 2021
@quarkus-bot quarkus-bot bot added this to the 1.13 - master milestone Mar 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/mongodb area/panache triage/waiting-for-ci Ready to merge when CI successfully finishes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Native vs jvm command line mode with panache mongodb
2 participants