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

Update Spring APIs to Spring Boot 3 #40344

Merged
merged 1 commit into from
Jun 10, 2024
Merged

Conversation

aureamunoz
Copy link
Member

@aureamunoz aureamunoz commented Apr 29, 2024

Highlight overview

This pull request primarily includes cosmetic changes, such as renaming methods and reorganizing packages. However, it also features significant refactoring of the quarkus-spring-data-rest-extension to align with the latest spring-data-jpa 3.x updates.

Key Changes in spring data jpa latest version:

Introduction of New Interfaces:

  • With spring-data-jpa 3.x, two new interfaces, ListCrudRepository and ListPagingAndSortingRepository, were introduced. These interfaces return List<T> types, unlike the existing PagingAndSortingRepository and CrudRepository, which return Iterable<T>.
  • The class hierarchy has been modified as follows:
Captura de pantalla 2024-06-13 a las 14 42 32
  • PagingAndSortingRepository no longer extends CrudRepository.
  • ListCrudRepository extends CrudRepository.
  • ListPagingAndSortingRepository extends PagingAndSortingRepository.
  • JpaRepository extends both ListCrudRepositoryand ListPagingAndSortingRepository.

Refactoring of spring-data-rest Extension:

  • To accommodate these changes, the extension's hierarchy was restructured. Rather than maintaining different implementors, a single implementor now checks the repository type and implements the corresponding methods.
  • All logic is consolidated in the RepositoryMethodsImplementor, which implements all repository methods based on the repository type.
  • The SpringDataRestProcessor has been updated to include the new interfaces and now performs registration in a unified method (registerRepositories) instead of using two separate methods.
  • Unification of PropertiesProvider Classes: these classes have been unified to incorporate the new methods from the List***Repository interfaces.
  • Updates to EntityClassHelper: the EntityClassHelper has been relocated and now includes new methods used by RepositoryMethodsImplementor to detect the repository type for implementation.
  • Modifications to Tests: the Paged***Test no longer checks methods inherited from Crud**Repos. A new test case, CrudAndPagedResourceTest, has been added for a repository extending both Crud and Paged repositories.

Note:
The new List**** methods are not specifically exposed via REST, as this is consistent with Spring's behavior.

Additionally, I plan to clean up some comments in the code. I will address these and any other changes based on your feedback during the review.

@quarkus-bot quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/spring Issues relating to the Spring integration labels Apr 29, 2024
@aureamunoz aureamunoz marked this pull request as ready for review April 29, 2024 13:23
@aureamunoz
Copy link
Member Author

cc @geoand

This comment has been minimized.

@geoand
Copy link
Contributor

geoand commented Apr 30, 2024

Thanks for this.

The CI failures definitely seem related to the PR

@aureamunoz
Copy link
Member Author

Thanks for this.

The CI failures definitely seem related to the PR

Oh, absolutely. I will take a look

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@aureamunoz aureamunoz force-pushed the to-spring-3 branch 2 times, most recently from a4b0b45 to 3059274 Compare June 6, 2024 10:02
@gsmet gsmet changed the title To spring 3 Update Spring APIs to Spring Boot 3 Jun 6, 2024
@aureamunoz
Copy link
Member Author

This is off to a good start, so could you, @geoand take a look as soon as you have some time available?

@geoand
Copy link
Contributor

geoand commented Jun 6, 2024

Sure, I'll try take and take a look tomorrow

@geoand
Copy link
Contributor

geoand commented Jun 6, 2024

A high level overview of the changes in the description of the PR would be nice for future reference.

This comment has been minimized.

This comment has been minimized.

@geoand
Copy link
Contributor

geoand commented Jun 7, 2024

A high level overview of the changes in the description of the PR would be nice for future reference.

I would also like to know why there are new implementors. Please add this information to the description of the PR

@aureamunoz
Copy link
Member Author

A high level overview of the changes in the description of the PR would be nice for future reference.

I would also like to know why there are new implementors. Please add this information to the description of the PR

Yes, sure. It's in progress.

@michalvavrik
Copy link
Member

@geoand I added breaking-change, not sure why you removed it but this is super breaking. for example it breaks compilation due to The class hierarchy has been modified: PagingAndSortingRepository no longer extends CrudRepository as written in description. it already broke QE project and if compilation is broken, it must be documented. Please add note to the migration guide.

@gsmet
Copy link
Member

gsmet commented Jun 11, 2024

@michalvavrik I added the description to the migration guide for 3.12.

@gsmet
Copy link
Member

gsmet commented Jun 11, 2024

Thanks for your vigilance BTW!

@michalvavrik
Copy link
Member

thank you @gsmet . BTW I have 4 failures after this PR got merged. I didn't use Spring for 3 years, so my knowledge is limited, so it could be false alarm. Anyway, it feels like a bug so I'll open issues within one hours.

@gsmet
Copy link
Member

gsmet commented Jun 11, 2024

@michalvavrik you can create them tomorrow. We still have a week before the Core Final.

@geoand
Copy link
Contributor

geoand commented Jun 11, 2024

Thanks.

I removed it by mistake

@jedla97
Copy link
Contributor

jedla97 commented Jun 12, 2024

The class hierarchy has been modified: PagingAndSortingRepository no longer extends CrudRepository. Instead, JpaRepository now extends both CrudRepository and PagingAndSortingRepository.

Reading this I'm not sure if it's correct. JpaRepository is now extended by ListCrudRepository and ListPagingAndSortingRepository. Yes ListCrudRepository is extended by CrudRepository but JpaRepository is not directly extend CrudRepository.

Not sure if this should be updated in migration guide.

@aureamunoz
Copy link
Member Author

aureamunoz commented Jun 14, 2024

The class hierarchy has been modified: PagingAndSortingRepository no longer extends CrudRepository. Instead, JpaRepository now extends both CrudRepository and PagingAndSortingRepository.

Reading this I'm not sure if it's correct. JpaRepository is now extended by ListCrudRepository and ListPagingAndSortingRepository. Yes ListCrudRepository is extended by CrudRepository but JpaRepository is not directly extend CrudRepository.

Not sure if this should be updated in migration guide.

It's the opposite, I improved the description here. Take a look and let me know if it's clearer @jedla97

@aureamunoz
Copy link
Member Author

aureamunoz commented Jun 14, 2024

A new PR fixing the #41134 , #41135 and #41136 is coming but it needs a quarkus-spring-data-api release to be done (PR#14)
cc @michalvavrik @jedla97

@michalvavrik
Copy link
Member

Cool, thanks!

@jedla97
Copy link
Contributor

jedla97 commented Jun 14, 2024

@aureamunoz Thanks for working on this and thanks for update.

It's much more clearer. I know what's happend after reading it and don't need to deep dive how it changed.

Probably one thing is it should be also updated in https://github.com/quarkusio/quarkus/wiki/Migration-Guide-3.12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dependencies Pull requests that update a dependency file area/spring Issues relating to the Spring integration release/breaking-change release/noteworthy-feature triage/flaky-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants