`@Transactional` annotation misbehavior

Nahuel92 commented 4 months ago

Expected Behavior

When @Transactional is applied at a class level, all non-private methods should be automatically wrapped in a transaction and have a connection available.

When @Transactional is applied at a method level (non-private), it should be wrapped in a transaction and have a connection available.

When using MockBean, TransactionalInterceptor shouldn't be applied (as it expects a DataSource to be created and the idea of mocking a repository is not having to have one).

It doesn't matter if it is:

@jakarta.transaction.Transactional
@io.micronaut.transaction.annotation.Transactional

The bug manifests itself with both annotations.

Actual Behaviour

When @Transactional is applied at a class level, all non-private methods are not automatically wrapped in a transaction and/or don't have a connection available. The following exception is thrown: io.micronaut.data.connection.exceptions.NoConnectionException: Expected an existing connection, but none was found.

When @Transactional is applied at a method level, tests involving those methods fail (see this Micronaut test bug I created and duplicated here in Micronaut-core repo).

It doesn't matter if it is @jakarta.transaction.Transactional or @io.micronaut.transaction.annotation.Transactional.

Bug test cases

Annotation / Annotated on	Repository	Repository method	Tests execution result [^2]	Production execution result
`@Transactional` [^1]	Yes	No	Pass	Fail
`@Transactional` [^1]	No	Yes	Fail	Works well
`@Transactional` [^1]	Yes	Yes	Fail	Works well

[^1]: It doesn't matter whether it's @jakarta.transaction.Transactional or @io.micronaut.transaction.annotation.Transactional.

[^2]: Tests mock repositories with @MockBean(RepositoryClass.class).

Steps To Reproduce

For your convenience, you can use the provided app example. But here are the steps anyway:

Create an app with Micronaut Data JDBC + H2 driver (for instance).
Create a Repository. Annotate it with both @Repository and @Transactional (at class level).
Inject a JdbcOperations object and run a query against H2, which should have been populated beforehand (using Liquibase, for instance). Query can be a simple SELECT * FROM mytable.
Run the app normally (you'll have to have some entrypoint such as a @EventListener or any other place where you inject the repository and call that method).

Important: Please notice that to expose the misbehavior, you'll need to run the app in Prod mode (that is, run the Application.java class).

Interestingly, another similar error happens during test execution (depending on what you annotate with @Transactional). Please take a look at:

A workaround I tried (hoping to pass tests and not receiving exceptions at runtime) but without any luck is to annotate both the class and non-private methods with @Transactional.

Thanks.

Environment Information

Windows/ Linux (it doesn't really matter).
JDK 21 (I don't think it matters).
jUnit 5 (I don't think it matters).
Mockito (I don't think it matters).

Example Application

https://github.com/Nahuel92/Micronaut-test-bug

I added my findings to the README.md file.

Please set MICRONAUT_ENVIRONMENTS=prod before starting the app.

Version

4.4.2

Nahuel92 commented 4 months ago

@sdelamo Hi Sergio, hope you're doing well :)

Sorry for bothering you, but I would like to know if you could give me a hand with these bugs. I can offer to help fixing them if you guys give me some pointers on where should I take a look at. I tried debugging and I found a couple of things I put in the README.md of the example application, but I'm not sure how Micronaut Data is weaving things together.

Thanks again!!

dstepanov commented 4 months ago

Looks like you need:

@Sql("classpath:sqls/init.sql")
@Property(name = "datasources.default.driver-class-name", value = "org.h2.Driver")
@Property(name = "datasources.default.url", value = "jdbc:h2:mem:h2")
@Property(name = "datasources.default.username", value = "sa")
@MicronautTest
class MyServiceTest

Otherwise, the TX manager is not created

Nahuel92 commented 4 months ago

Looks like you need:

@Sql("classpath:sqls/init.sql")
@Property(name = "datasources.default.driver-class-name", value = "org.h2.Driver")
@Property(name = "datasources.default.url", value = "jdbc:h2:mem:h2")
@Property(name = "datasources.default.username", value = "sa")
@MicronautTest
class MyServiceTest

Otherwise, the TX manager is not created

Hi dstepanov thanks for your answer :)

Regarding your suggestion, I'm mocking the repositories in that test. Why is the TX manager still required in this case?

Also, the @Transactional behavior based on where it is applied (class or method level) seems odd to me. I would love to hear your thoughts on that to understand if I'm misusing it or if this is effectively a bug.

Please let me know if you need more info, I'll be more than happy to expand on my explanation.

dstepanov commented 4 months ago

Regarding your suggestion, I'm mocking the repositories in that test. Why is the TX manager still required in this case?

The interceptor is applied around your mock and triggers a new TX.

Also, the @Transactional behavior based on where it is applied (class or method level) seems odd to me. I would love to hear your thoughts on that to understand if I'm misusing it or if this is effectively a bug.

It works in the same way, your test was incorrect.

Nahuel92 commented 4 months ago

Regarding your suggestion, I'm mocking the repositories in that test. Why is the TX manager still required in this case?

The interceptor is applied around your mock and triggers a new TX.

Also, the @transactional behavior based on where it is applied (class or method level) seems odd to me. I would love to hear your thoughts on that to understand if I'm misusing it or if this is effectively a bug.

It works in the same way, your test was incorrect.

Thanks for explaining that.

It's weird for me to still need a DB to run a test with mocked repositories, but adding those properties made the test to pass.

Regarding the other misbehavior (at least for me) I just pushed a change. Can you give it a try please and share your thoughts with me?

Apologies for bothering with this, but it kind of seems like a bug to me.

dstepanov commented 4 months ago

Regarding the other misbehavior (at least for me) I just pushed a change. Can you give it a try please and share your thoughts with me?

I tried and it works. What's wrong?

Nahuel92 commented 4 months ago

Regarding the other misbehavior (at least for me) I just pushed a change. Can you give it a try please and share your thoughts with me?

I tried and it works. What's wrong?

If you run the app normally executing Application.java passing the property MICRONAUT_ENVIRONMENTS=prod as follows:

An exception is thrown:

ERROR io.micronaut.runtime.Micronaut - Error starting Micronaut server: Expected an existing connection, but none was found.
io.micronaut.data.connection.exceptions.NoConnectionException: Expected an existing connection, but none was found.
    at java.base/java.util.Optional.orElseThrow(Optional.java:403)
    at io.micronaut.data.connection.ConnectionOperations.getConnectionStatus(ConnectionOperations.java:42)
    at io.micronaut.data.jdbc.operations.DefaultJdbcRepositoryOperations.getConnectionCtx(DefaultJdbcRepositoryOperations.java:819)
    at io.micronaut.data.jdbc.operations.DefaultJdbcRepositoryOperations.prepareStatement(DefaultJdbcRepositoryOperations.java:846)
    at org.nahuelrodriguez.WorkingMicronautRepository.buggedMethod(WorkingMicronautRepository.java:24)
    at org.nahuelrodriguez.EntryPoint.init(EntryPoint.java:28)
    at org.nahuelrodriguez.$EntryPoint$Definition$Exec.dispatch(Unknown Source)
    at io.micronaut.context.AbstractExecutableMethodsDefinition$DispatchedExecutableMethod.invoke(AbstractExecutableMethodsDefinition.java:456)
    at io.micronaut.context.DefaultBeanContext$BeanExecutionHandle.invoke(DefaultBeanContext.java:3856)
    at io.micronaut.aop.chain.AdapterIntroduction.intercept(AdapterIntroduction.java:84)
    at io.micronaut.aop.chain.MethodInterceptorChain.proceed(MethodInterceptorChain.java:138)
    at org.nahuelrodriguez.EntryPoint$ApplicationEventListener$init1$Intercepted.onApplicationEvent(Unknown Source)
    at io.micronaut.context.event.ApplicationEventPublisherFactory.notifyEventListeners(ApplicationEventPublisherFactory.java:266)
    at io.micronaut.context.event.ApplicationEventPublisherFactory$2.publishEvent(ApplicationEventPublisherFactory.java:226)
    at io.micronaut.context.DefaultBeanContext.publishEvent(DefaultBeanContext.java:1817)
    at io.micronaut.context.DefaultBeanContext.start(DefaultBeanContext.java:357)
    at io.micronaut.context.DefaultApplicationContext.start(DefaultApplicationContext.java:202)
    at io.micronaut.runtime.Micronaut.start(Micronaut.java:74)
    at io.micronaut.runtime.Micronaut.run(Micronaut.java:328)
    at io.micronaut.runtime.Micronaut.run(Micronaut.java:314)
    at org.nahuelrodriguez.Application.main(Application.java:7)

This only happens when @Transactional is annotated at a repository class level. If you see the last commit changes, you could see that I just moved that annotation from a method level to a class level.

In the following picture you can find the changes. When the code is like the one on the left, the app works well. But, if the code is like the one on the right, the app throws that exception I posted above.

Not sure if I'm doing something wrong, please let me know if more information is needed. I'm eager to help. Thank you dstepanov!!

micronaut-projects / micronaut-data