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

@QuarkusTest does not handle entity updates #7102

Closed
zeljkot opened this issue Feb 9, 2020 · 21 comments
Closed

@QuarkusTest does not handle entity updates #7102

zeljkot opened this issue Feb 9, 2020 · 21 comments
Labels
area/hibernate-orm Hibernate ORM area/testing kind/bug Something isn't working
Milestone

Comments

@zeljkot
Copy link

zeljkot commented Feb 9, 2020

Describe the bug
A test creates, updates and reads the entity in separate transactions. The last transaction, which reads the entity, can find it, but the entity still has the same value as when it was created.

Expected behavior
After an entity is updated in one transaction, the change should be visible in the next transaction.

Actual behavior
After an entity is updated in one transaction, it has the same value as when it was created.

To Reproduce
Steps to reproduce the behavior:

  1. Make a test that calls three methods: persist, update, and read. Test method is not @Transactional, while called methods are.
  2. Run the test.

Configuration

quarkus.datasource.driver=org.h2.Driver
quarkus.datasource.url=jdbc:h2:file:./persons
quarkus.datasource.username=sa
quarkus.datasource.password=sa

quarkus.hibernate-orm.database.generation=drop-and-create

Screenshots
(If applicable, add screenshots to help explain your problem.)

Environment (please complete the following information):

  • Output of uname -a or ver: Microsoft Windows [Version 10.0.19041.21]
  • Output of java -version: 1.8
  • GraalVM version (if different from Java):
  • Quarkus version or git rev: 1.3.0.Alpha1

Additional context
The reproducer is attached:
test-update2.zip

@zeljkot zeljkot added the kind/bug Something isn't working label Feb 9, 2020
@stuartwdouglas
Copy link
Member

In your test you have @transactional on a method that is called by other methods on the same class. CDI interceptors are not applied for self invocation, only for external invocation (e.g. when you inject the bean and invoke on it). As no transaction is active this is why the change is not persisted.

@stuartwdouglas
Copy link
Member

@mkouba can you confirm that there are no plans to add support for self-interception? I know this is a topic that comes up every now and then but I can't remember what the spec has to say about it.

@zeljkot
Copy link
Author

zeljkot commented Feb 10, 2020

Things get a little weird here. If I remove @transactional and put transaction.begin(); and transaction.commit();, then everything work. If I put @transactional back on one of the called methods (not the test method, which is called by framework!), I get ARJUNA016051: thread is already associated with a transaction!. A transaction is created even for the self invocation.

transaction.getStatus() = 0, but the results are not as expected. As there are other issues regarding tests and transactions, it would be great if Quarkus team can document transaction strategies for testing in the testing guide.

@stuartwdouglas
Copy link
Member

ok, this is weird, I will investigate

@stuartwdouglas
Copy link
Member

Turns out I was wrong, we do support self invocation for interceptors, so now I have no idea what is going on here, I will investigate further.

@stuartwdouglas
Copy link
Member

@FroMage the test does not seem to be transformed to use the getters/setters, so the dirty checker is not picking up that the entity is dirty.

@FroMage
Copy link
Member

FroMage commented Feb 11, 2020

Well we really need to make sure all the classes that use the entity are transformed as well, otherwise even user-defined accessors won't get called and it will be all confusing.

@zeljkot
Copy link
Author

zeljkot commented Feb 12, 2020

Today I tried to run my regular tests, which run perfectly under Java 8, under Java 11. The result was a hot mess: half of the tests failed, probably due to some transaction issues. It only happens on Windows, even in WSL2 tests run fine. Should I open a new issue for that or you think that may be related to this issue? To be honest, I do not have a clue about what is going on. Do you test on Windows Java 11?

@zeljkot
Copy link
Author

zeljkot commented Feb 12, 2020

Here is more: if a class implementing the PanacheRepository is marked as @Transactional, methods injected by Quarkus are not transactional. @FroMage would you consider this as the same issue or different one?

@FroMage
Copy link
Member

FroMage commented Feb 13, 2020

This looks like a different issue.

@bolodecenouracomcafe
Copy link

I believe I am in the same situation, but there is something strange when it is not in test mode too.

I did a test via POSTMAN and with MSSQL database. I followed the request in the database, the entity is updated as expected, but in the application I use the command MyEntity.findAll().list().get(0) and the entity remains not updated.

More informations: #7163

@bolodecenouracomcafe
Copy link

@zeljkot The quarkus-narayana-jta dependency is missing in pom.xml. I added and did tests with TransactionManager and UserTransaction and it didn't solve it. I'm importing from the javax.transaction package, is that correct?

I don't know why the lack of dependency does not generate an error.

In the tests with TransactionManager and UserTransaction the following message started to be displayed when the commit line is executed:
"WARN: Datasource '<default>': Closing open connection prior to commit" #6770

Project with my changes:
https://send.firefox.com/download/318867de387d3f58/#GyyXcEArjnuNlpblvOMqyA

@zeljkot
Copy link
Author

zeljkot commented Feb 13, 2020

Narayana is dependency of Hibernate, there is no need to include it per se.

@bolodecenouracomcafe
Copy link

My quarkus logs with log trace enabled:

https://send.firefox.com/download/1017dc8221806b4b/#HN3XWg9TzL4hc_db5reZ3Q

@mkouba
Copy link
Contributor

mkouba commented Feb 17, 2020

Turns out I was wrong, we do support self invocation for interceptors...

@stuartwdouglas Yes, we do. The behavior is undefined from the spec POV.

Here is more: if a class implementing the PanacheRepository is marked as @transactional, methods injected by Quarkus are not transactional.

@zeljkot Yes, this is a known issue - repository class is transformed/methods are generated and so the CDI container does not see the methods at all. And for entities there is a problem with static methods - these are not intercepted by design (as defined by the spec).

@bolodecenouracomcafe
Copy link

bolodecenouracomcafe commented Apr 6, 2020

I find a way around this problem, in the test files I did not do operations with the database directly, I did all through API. It worked for me.

FroMage added a commit to FroMage/quarkus that referenced this issue May 25, 2020
@FroMage
Copy link
Member

FroMage commented May 25, 2020

Added your test at #9577 and it passes. We must have fixed this already in master.

@FroMage
Copy link
Member

FroMage commented May 25, 2020

Please reopen if you experience this again with 1.5.0.Final due very soon.

@stuartwdouglas
Copy link
Member

I think this was fixed by #7188

FroMage added a commit to FroMage/quarkus that referenced this issue May 26, 2020
FroMage added a commit that referenced this issue May 26, 2020
j-be pushed a commit to j-be/quarkus-7102 that referenced this issue Jun 12, 2020
Signed-off-by: Juri Berlanda <juri.berlanda@tuwien.ac.at>
@j-be
Copy link
Contributor

j-be commented Jun 12, 2020

I am still able to reproduce the issue (or at least a very similar one) with Quarkus 1.5.1.Final. The issue on my side seems to hinge on findAll().list() being executed or not, which triggers some kind of TestMethodScoped caching.

See https://github.com/j-be/quarkus-7102 for a minimal project reproducing the issue.

EDIT: I extended the TestProject to better show what I mean with TestMethodScoped, i.e. if the change is checked in a second @Test the entities are indeed updated. I also ran the test against a build of current master (d8ec141d4 with patched dependencyManagement because of the Unsupported api 524288 bug reported in #9832) - still fails.

@stuartwdouglas
Copy link
Member

This is a different issue, and the behaviour you are seeing is expected. The EntityManager will always use the same object for a given ID. When you call findAll that object is loaded from the database. When you edit it in a new transaction the new transaction creates a new EntityManager, however the original EM still has the old entity loaded. When you then call findById the EM just returns the original object loaded by listAll(), which has not been modified yet.

If you want to see the updated object you need to call EntityManager.clear().

@gsmet gsmet added this to the 1.5.0.Final milestone Jun 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/hibernate-orm Hibernate ORM area/testing kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

8 participants