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

Upgrade to Spring Boot 3.2.3 #25317

Merged
merged 10 commits into from
Mar 1, 2024
Merged

Upgrade to Spring Boot 3.2.3 #25317

merged 10 commits into from
Mar 1, 2024

Conversation

mraible
Copy link
Contributor

@mraible mraible commented Feb 25, 2024

Depends on jhipster/jhipster-bom#1526.


Please make sure the below checklist is followed for Pull Requests.

@mshima
Copy link
Member

mshima commented Feb 29, 2024

@fbiville any advice to fix No bean named 'neo4jTemplate' available caused by spring-boot 3.2.3 update?

@meistermeier
Copy link

meistermeier commented Feb 29, 2024

At the moment I don't have time, to have a deeper look at this project, maybe something for tomorrow.
Spring Data Neo4j (me to be precise) changed the optional existence of a transaction manager to a requirement with this PR spring-projects/spring-data-neo4j#2859 targeting the latest release and milestone version.
This was beside some nice side-effects a needed bugfix and we would have never guessed before that it will break other people code, sorry. It might also be a consequence of a change in Spring Boot adapting this new behaviour and changing the auto configuration for Spring Data Neo4j (spring-projects/spring-boot@b6467ed), so I am not sure what exactly leads to this problem. As you can see it should create a (default) Neo4j transaction manager.
This are the hints, I can give from the top of my head right now. For more I would need to remember how JHipster works, my last involvement was ~3-4 years ago.

Edit: Also I have on my desk to review the changes done in Spring Boot in detail and see if we can find a middle-ground that will not introduce such an impact on the user/application side.

@atomfrede
Copy link
Member

Thanks @meistermeier for the details. I will try to help here. At work the update 3.2.3 had also some strange effects (not related to neo4j) which I would have not expected

@meistermeier
Copy link

Cannot bring myself into the state from a few years ago to understand how to build a (test) project against this patch (incl. the patched bom reference).
But I have a wild guess: the template file in generators/spring-data-neo4j/templates/src/main/java/package/config/DatabaseConfiguration.java_neo4j.ejs does create either one or the other.
At the current state of libs, both transaction managers are required (details at the end). If this file would not switch between reactive or imperative (without the prefix) but instantiate both, you would be good to go.
For this to happen, the example/test has to be a reactive project. Is this assumption right?

Background: The change within SDN would require a Neo4jTransactionManager for the imperative part and a ReactiveNeo4jTransactionManager for the reactive part.
Before the change, if people want to use a transaction manager for the reactive part, they would have to define it manually. There was just no support in Spring Boot (this was a few years requested by the Spring Boot team to remove it from the auto-config, IIRC). The situation was that we created the imperative transaction manager, if it did not exist, but left the reactive one for the user to define, if wanted.
Now the auto config in Spring Boot changed and it would still create "a" transaction manager but -and this is where I need to speak with the Spring Boot team after I confirmed this for myself- if the reactive transaction manager gets instantiated first no further (imperative) transaction manager will get created.
With the given config above it is even easier to recreate:

  1. the app has a reactive transaction manager
  2. Spring Boot's auto config discovers it and does not do anything here: https://github.com/spring-projects/spring-boot/blob/b6467ed826818db5d18061496768db3f6758c916/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/neo4j/Neo4jTransactionManagerConfiguration.java#L40 The condition is already met because the ReactiveTransactionManager is (unfortunately in this case) also a TransactionManager.
  3. Our change in SDN that requires the transaction manager to be present for the imperative part (yes, I know in your case dead code) will yell an exception because the dependency does not exist.

Conclusion from my side: It is a really unfortunate situation right now. Spring Boot has no chance on the dependencies to decide reliable if the user wants to go the reactive or imperative way. In contrast to e.g. "layered" mongo db's driver, the Neo4j driver comes with all dependencies incl. project-reactor. We are currently investigating if we might switch the driver dependency to the bundled version. As such, the reactor dependency and other will be shaded in a different package and Spring Boot could finally look for the (default) full qualified class name of Flux or similar and avoid the instantiation of the whole imperative chain, if not needed.

This was longer than I expected and I hope I made things a little bit clear. If you have any further questions, feel free to ping me.

@DanielFran DanielFran enabled auto-merge March 1, 2024 14:28
@DanielFran DanielFran merged commit 1e638ae into main Mar 1, 2024
55 checks passed
@DanielFran DanielFran deleted the spring-boot-3.2.3 branch March 1, 2024 15:42
@deepu105 deepu105 added this to the 8.2.0 milestone Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants