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

Fix #5885: generics break panache repo enhancer #6191

Merged
merged 3 commits into from
Dec 16, 2019
Merged

Conversation

FroMage
Copy link
Member

@FroMage FroMage commented Dec 16, 2019

Do not enhance generic/abstract repos
Use proper generics resolution when looking up the repository type
arguments

Fixes: #5885

Do not enhance generic/abstract repos
Use proper generics resolution when looking up the repository type
arguments
@FroMage
Copy link
Member Author

FroMage commented Dec 16, 2019

@loicmathieu it's very possible you have the same bug in mongo-panache, and will need the same fix (do not enhance abstract/generic repos).

It's also possible that you have the same issue in your reactive-mongo-panache repo IF your entity base type is generic.

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.

👍

Copy link
Contributor

@loicmathieu loicmathieu left a comment

Choose a reason for hiding this comment

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

mongodb-panache and the future mongodb-panache-reactive suffers from the same bug.
If you don't have time to fix it in mongodb-panacheinside the same PR, can you open an issue for it ?

@FroMage
Copy link
Member Author

FroMage commented Dec 16, 2019

If you don't have time to fix it in mongodb-panache inside the same PR, can you open an issue for it ?

I can do it in this PR, should be simple.

@FroMage
Copy link
Member Author

FroMage commented Dec 16, 2019

Added test/fix for mongo too.

Copy link
Contributor

@loicmathieu loicmathieu left a comment

Choose a reason for hiding this comment

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

LGTM

@FroMage
Copy link
Member Author

FroMage commented Dec 16, 2019

Huh, so native test fails, due to who knows what, but I am already rebased on master.

@FroMage FroMage merged commit d00260f into quarkusio:master Dec 16, 2019
@FroMage
Copy link
Member Author

FroMage commented Dec 16, 2019

It passed after restart

@gsmet gsmet added this to the 1.2.0 milestone Dec 16, 2019
@MartinX3
Copy link

MartinX3 commented Dec 18, 2019

@FroMage Here we have the problem, that a Repository listAll() call gives us every DataBase entry in every table.
As example we made a trainer and the call for courses gives us all trainer, too.

Kotlin

abstract class BaseRepository<T>(@Inject val entityManager: EntityManager) : PanacheRepositoryBase<T, UUID> {

@Dependent class TrainerRepository(entityManager: EntityManager) : BaseRepository<Trainer>(entityManager) {

@Dependent class CourseRepository(entityManager: EntityManager) : BaseRepository<Course>(entityManager) {

Is it a different bug which needs to get reported in a new issue ticket?

@FroMage
Copy link
Member Author

FroMage commented Dec 18, 2019

@MartinX3 is this with the current master which has the fix? I think it's the same bug and it should be fixed.

@MartinX3
Copy link

@FroMage Thank you for your fast response.
We use the maven 1.1.0.final release in our company.
Nice to hear that it will get fixed :)
Will it be released as 1.1.1 or do we need to wait for 1.2.0 (which has no ETA?)

Thank you all for the fix. :)

@FroMage
Copy link
Member Author

FroMage commented Dec 18, 2019

That's a question for @gsmet but IIRC 1.2 is planned for January.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Abstraction layer over PanacheRepositoryBase
5 participants