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.4.0 #4488

Open
wants to merge 48 commits into
base: main
Choose a base branch
from

Conversation

achimber-moj
Copy link
Contributor

No description provided.

dependabot bot and others added 30 commits November 29, 2024 15:08
Bumps the minor group with 5 updates:

| Package | From | To |
| --- | --- | --- |
| [org.springframework.boot](https://github.com/spring-projects/spring-boot) | `3.3.5` | `3.4.0` |
| software.amazon.awssdk:aws-query-protocol | `2.29.15` | `2.29.20` |
| software.amazon.awssdk:sts | `2.29.15` | `2.29.20` |
| [com.microsoft.graph:microsoft-graph](https://github.com/microsoftgraph/msgraph-sdk-java) | `6.20.0` | `6.21.0` |
| [org.springdoc:springdoc-openapi-starter-webmvc-ui](https://github.com/springdoc/springdoc-openapi) | `2.6.0` | `2.7.0` |


Updates `org.springframework.boot` from 3.3.5 to 3.4.0
- [Release notes](https://github.com/spring-projects/spring-boot/releases)
- [Commits](spring-projects/spring-boot@v3.3.5...v3.4.0)

Updates `software.amazon.awssdk:aws-query-protocol` from 2.29.15 to 2.29.20

Updates `software.amazon.awssdk:sts` from 2.29.15 to 2.29.20

Updates `com.microsoft.graph:microsoft-graph` from 6.20.0 to 6.21.0
- [Release notes](https://github.com/microsoftgraph/msgraph-sdk-java/releases)
- [Changelog](https://github.com/microsoftgraph/msgraph-sdk-java/blob/main/CHANGELOG.md)
- [Commits](microsoftgraph/msgraph-sdk-java@v6.20.0...v6.21.0)

Updates `org.springdoc:springdoc-openapi-starter-webmvc-ui` from 2.6.0 to 2.7.0
- [Release notes](https://github.com/springdoc/springdoc-openapi/releases)
- [Changelog](https://github.com/springdoc/springdoc-openapi/blob/main/CHANGELOG.md)
- [Commits](springdoc/springdoc-openapi@v2.6.0...v2.7.0)

---
updated-dependencies:
- dependency-name: org.springframework.boot
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: minor
- dependency-name: software.amazon.awssdk:aws-query-protocol
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: minor
- dependency-name: software.amazon.awssdk:sts
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: minor
- dependency-name: com.microsoft.graph:microsoft-graph
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: minor
- dependency-name: org.springdoc:springdoc-openapi-starter-webmvc-ui
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: minor
...

Signed-off-by: dependabot[bot] <[email protected]>
…en tests are run as whole, rather than individually.
…ce.nextval. Also, move some test data creation to fix optimistic lock error when tests run in a suite.
…ce.nextval. Also, move some test data creation to fix optimistic lock error when tests run in a suite.
…luded, generated sql is escape ''. This does not return required data against h2 database.
…luded, generated sql is escape ''. This does not return required data against h2 database.
…tryofjustice/hmpps-probation-integration-services into dependabot/gradle/minor-4e1fa6339b
… of LOW - LOW is the default one, NULL values are considered as smaller than other values during sorting.
@achimber-moj achimber-moj requested a review from a team as a code owner December 11, 2024 18:13
@marcus-bcl marcus-bcl changed the title Dependabot/gradle/minor 4e1fa6339b Upgrade to Spring Boot 3.4.0 Dec 16, 2024
@anthony-britton-moj
Copy link
Contributor

anthony-britton-moj commented Jan 7, 2025

Is there a reason all the run files (to run the services in inteliij) have changed?
Also - looks like sonar has failed due to some entity changes, meaning it thinks there is now more duplicated code. Moving a field to a different place on some of the duplicates would get around that.

Copy link
Contributor

@anthony-britton-moj anthony-britton-moj left a comment

Choose a reason for hiding this comment

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

Mostly looks good - couple of recurring themes - please see the comments. Will continue to review later as there are a lot of file changes.

build.gradle.kts Outdated Show resolved Hide resolved
@PersistenceContext
private lateinit var entityManager: EntityManager

var personAddressId: Long? = null
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a reason we have fields on this that are then shared rather than just having the entities in the generators?

Copy link
Contributor Author

@achimber-moj achimber-moj Jan 8, 2025

Choose a reason for hiding this comment

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

Following the upgrade to springboot 3.4, a few tests were failing due to an optimistic locking error when tests are run as whole, rather than individually. I added a new class EntityManagerDataLoader to handle this error. I put the new fields in the generators initially, then moved it to the class.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think having the data loader separate is fine - the reason we use the generators for storing the entities though (and thus why I think it should remain) is because then you can use them using
SomeGenerator.SOME_ENTITY (ie statically) ...
with your way we have to inject the data loader (which means you need to have the spring context inject it etc)

Personally I think I prefer it in the generators - but maybe something to run by the rest of the team if you think there's a good reason to not use static (or equivalent) references.

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

Successfully merging this pull request may close these issues.

3 participants