Panache: give a flush() method that pinpoints the right PU

[lamemind-ud]

Description

I'm using multiple named PUs.

There are two ways to flush() from a Panache repository

• persistAndFlush() which is fine IF I want to store an entity itself... But If I run several update() and then I want to flush, persistAndFlush is not the correct answer.
• then there is flush(): it works only on the default PU, which is not my case since the repo's entity is configured within a named PU

Any advice?

Implementation ideas

Maybe the flush() method could get an argument that pinpoints the right PU (an entity Class instance?)

[quarkus-bot]

/cc @FroMage, @loicmathieu

[loicmathieu]

@lamemind-ud the flush method of the repository should use the right PU, if not, it's a bug.
Can you validate that it didn't work as expected and create a reproducer ?

[loicmathieu]

I check the code and yes, it flushes on the default entity manager, see https://github.com/quarkusio/quarkus/blob/main/extensions/panache/hibernate-orm-panache/runtime/src/main/java/io/quarkus/hibernate/orm/panache/PanacheRepositoryBase.java#L114

I'll check if we can flush on the right PU instead.

[lamemind-ud]

Thanks @loicmathieu
I'll wait for updates!

[lamemind-ud]

@loicmathieu maybe this could help

You can get the class instance from the generic type, and then you can get che EntityManager for that class

Class entityClass = ((Class) ((ParameterizedType) getClass()
                .getGenericSuperclass()).getActualTypeArguments()[0]);
getEntityManager(entityClass).flush();

[loicmathieu]

@lamemind-ud thanks for the tip, you can use it as a workaround on your code. On Panache, we have already the infrastructure to get the correct entity manager, it's used in a lot of places (via bytecode enhancement so not visible on the Quarkus source code) but not on the flush method.

[lamemind-ud]

@loicmathieu
Actually I was suggesting a fix for PanacheRepositoryBase::flush()

PanacheRepositoryBase relies on JpaOperations to get the right EntityManager.
JpaOperations has map entityToPersistenceUnit map indexed with the entities' classes.

So you can refactor the flush method as follow

    default void flush() {
        Class entityClass = ((Class) ((ParameterizedType) getClass().getGenericSuperclass()).getActualTypeArguments()[0]);
        INSTANCE.getEntityManager(entityClass).flush();
    }

[loicmathieu]

@lamemind-ud as I said, we have a better alternative for this based on our bytecode enhancement mechanism and some internal classes that already contains this piece of code.

[loicmathieu]

@lamemind-ud I was wrong and you was right ;)
I was confusely think that our bytecode generation infrastructure was able to do what I wanted but it only works for repository methods that took the entity as first parameter.

So I use your trick :)
This is a bug so I'll change the issue type.

[lamemind-ud]

@loicmathieu I'm glad I have been helpful

Cheers

[loicmathieu]

@lamemind-ud I checked and in fact the current code seems OK, it uses the getEntityManager() method that should select the right PU. However the documentation was wronf.

So, reading the code it seems a JavaDoc issue but maybe I'm wrong.

Can you confirm that your have a real issue with the wrong PU being selected or do you open an issue because the JavaDoc tell you the default EntityManager will be used which is not really the case ?

[lamemind-ud]

@lamemind-ud I checked and in fact the current code seems OK, it uses the getEntityManager() method that should select the right PU. However the documentation was wronf.

So, reading the code it seems a JavaDoc issue but maybe I'm wrong.

Can you confirm that your have a real issue with the wrong PU being selected or do you open an issue because the JavaDoc tell you the default EntityManager will be used which is not really the case ?

Hello @loicmathieu ,
I didn't check the doc. I did check the code directly, then I filed an enhancement, not a bug.

As of my starting comment, the issue here is to have a flush() method that points to a named PU, w/o Injecting an EntityManger and w/o having an entity... I have a PanacheRepositoryBase and I'd like to call flush on it.

IMHO, I think PanacheRepositoryBase::flush() should not flush the default PU, but it should flush the Repo's PU: it's the most intuitive behavior to me.

Thanks for assistance

[loicmathieu]

So, this is a mis-understanding of the code: PanacheRepositoryBase::flush() delegates to PanacheRepositoryBase::getEntityManager() if you look at it's code it's empty (in fact, it throws an exception) as this method is generated at runtime using bytecode enhancement to get the right entity manager.

So, it should works (or nothing works as we use the same gentEntityManager() method in a lot of places).

Each method on Panache that is annotated with @GenerateBridgeMethod is generated via bytecode enhancement so be careful when you read it ;)

[loicmathieu]

To be crystal clear.

As of my starting comment, the issue here is to have a flush() method that points to a named PU, w/o Injecting an EntityManger and w/o having an entity... I have a PanacheRepositoryBase and I'd like to call flush on it.

The flush() do use the EntityManager of the repository entity class, the code is hidden inside the bytecode enhancement done at build time.

IMHO, I think PanacheRepositoryBase::flush() should not flush the default PU, but it should flush the Repo's PU: it's the most intuitive behavior to me.

This is already the case.

[lamemind-ud]

I'm gonna check my code before reply... I'll be back :)

[lamemind-ud]

Hello @loicmathieu , I'm back.

In my code, with a named PU, if I call PanacheRepositoryBase::flush(), I get the following exception:
java.lang.IllegalStateException: The default datasource has not been properly configured

Of course, I have no default datasource, cause I have no entities within the default datasource.
This error comes from the PanacheRepositoryBase::flush(), and the related entity is attached to a named PU.
Didn't check the doc because I took for granted it was the intended behavior.

Can you reproduce?
Do you need my code?

[loicmathieu]

Yes, I'll need a reproducer please

[lamemind-ud]

panache-flush-reproducer.zip

@loicmathieu I cleaned my project.
there are 3 PUs, but only using 2 right now.
there are 4 tests, all failing at flush()

[lamemind-ud]

Hello @loicmathieu ... I don't know if it can be useful... but I found a way to flush from RepositoryBase

Actually it's just as simple as RepositoryBase::getEntityManager::flush()
getEntityManager works with the right PU, but flush doesn't.
The question arise: what's the difference?

    @GenerateBridge
    default EntityManager getEntityManager() {
        throw implementationInjectionMissing();
    }

    default void flush() {
        INSTANCE.getEntityManager().flush();
    }

I am a huge noob and I could be wrong... but it seems that getEntityManager relies on the bytecode enhancement mechanism you talked about.... but flush doesn't. flush is implemented as is.

Cheers

[loicmathieu]

Oh yes, I read the code twice but didn't notice it :(

default void flush() {
        INSTANCE.getEntityManager().flush();
    }

Should be replaced by

default void flush() {
        getEntityManager().flush();
    }

So it uses the gentEntityManager() method that is implemented via bytecode enhancement so it will be translated to getEntityManager(EntityClass).

[loicmathieu]

Thanks for spotting the bug.

[loicmathieu]

And it fact it's what I did three days ago, didn't remember if I did it on purpose and forget that I spotted the bug myself or did it without thinking while modifying the comment ...

Anyway I have a PR that fixes the issue openned already ready

[lamemind-ud]

Actually you found the bug. I just pasted some random code