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

Support Spring Data JpaRepository#getReferenceById(ID) #41993

Merged
merged 1 commit into from
Jul 19, 2024

Conversation

aureamunoz
Copy link
Member

Fixes #41987

@geoand
Copy link
Contributor

geoand commented Jul 19, 2024

Won't this change remove JpaRepository#getOne(ID)?
We should not remove it, but instead have the new method be an additional one

@geoand
Copy link
Contributor

geoand commented Jul 19, 2024

Furthermore, we should have tests using both the new and the deprecated method

@aureamunoz
Copy link
Member Author

Initially, I thought about keeping the StockMethodsAdder#getOne method and adding a new one. However, since getOne was private and only used in one place, I decided to refactor it instead. I renamed the method and its variables to getReferenceById. As RepositorySupport#getOne is public, I didn't delete it. Instead, I marked it as @deprecated and added a new method, RepositorySupport@getReferenceById.

Regarding the tests I'm not sure to understand, I added a few tests using both. What do you exactly mean?

@geoand
Copy link
Contributor

geoand commented Jul 19, 2024

Regarding the tests I'm not sure to understand, I added a few tests using both. What do you exactly mean

I had missed those I guess. If these pass, we are fine

@quarkus-bot

This comment has been minimized.

@aureamunoz
Copy link
Member Author

Looking at this

@geoand
Copy link
Contributor

geoand commented Jul 19, 2024

The problem is likely what I mentione earlier: The change seems to have removed the implementation of the getOne method

@aureamunoz
Copy link
Member Author

Yes, it seems to be a good reason to keep adding the deprecated methods implems 😆
I refactored and added a few more that were lacking.

Let's see what the CI says

@geoand geoand added triage/waiting-for-ci Ready to merge when CI successfully finishes triage/backport-3.13 labels Jul 19, 2024
@quarkus-bot
Copy link

quarkus-bot bot commented Jul 19, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 4e06bfb.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@geoand geoand merged commit 8246009 into quarkusio:main Jul 19, 2024
20 checks passed
@quarkus-bot quarkus-bot bot added kind/enhancement New feature or request and removed triage/waiting-for-ci Ready to merge when CI successfully finishes labels Jul 19, 2024
@quarkus-bot quarkus-bot bot added this to the 3.14 - main milestone Jul 19, 2024
@famod
Copy link
Member

famod commented Jul 19, 2024

Can this also be backported to 3.12? My reasoning would be that the Spring API update was also done in 3.12.

@geoand
Copy link
Contributor

geoand commented Jul 19, 2024

Sure yeah

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/spring Issues relating to the Spring integration kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Spring Data JpaRepository#getReferenceById(ID) (replacement for deprecated getOne(ID))
4 participants