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

Spring 2.7: Recipe to replace getById with getReferenceById #515

Closed
lucashan opened this issue Apr 19, 2024 · 9 comments
Closed

Spring 2.7: Recipe to replace getById with getReferenceById #515

lucashan opened this issue Apr 19, 2024 · 9 comments
Labels

Comments

@lucashan
Copy link
Contributor

lucashan commented Apr 19, 2024

What problem are you trying to solve?

As of Springboot v2.7, both getOne() and getById() methods have been deprecated for JpaRepository. The documentation states that these deprecated methods should be replaced with getReferenceById(ID).

We should include this recipe for Spring 2.7, for example:

type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.spring.data.UseJpaRepositoryGetReferenceById
displayName: Use `JpaRepository#getReferenceById(ID id)`
description: '`JpaRepository#getById(ID)` was deprecated in 2.7.'
recipeList:
  - org.openrewrite.java.ChangeMethodName:
      methodPattern: org.springframework.data.jpa.repository.JpaRepository getById(..)
      newMethodName: getReferenceById

Describe the situation before applying the recipe

import org.springframework.data.jpa.repository.JpaRepository;

public interface TestRepository extends JpaRepository<Test, Long> {
}
    public Test getTestById(final long id) {
        return testRepository.getOne(id);
    }

Describe the situation after applying the recipe

import org.springframework.data.jpa.repository.JpaRepository;

public interface TestRepository extends JpaRepository<Test, Long> {
}
    public Test getTestById(final long id) {
        return testRepository.getReferenceById(id);
    }

Have you considered any alternatives or workarounds?

N/A

Are you interested in contributing this recipe to OpenRewrite?

@timtebeek timtebeek moved this to Recipes Wanted in OpenRewrite Apr 20, 2024
@timtebeek timtebeek added good first issue Good for newcomers boot-2.7 labels Apr 20, 2024
@timtebeek
Copy link
Contributor

Thanks for the report @lucashan ! Is this something you'd want to contribute given that you've already provided a good start with the ChangeMethodName yaml snippet above? We'd just need one or two unit tests and then we can ship this out with next week's release.

@lucashan
Copy link
Contributor Author

lucashan commented Apr 22, 2024

Sounds good, I can make the contribution! Which test class should I be including the unit tests to @timtebeek ?

@timtebeek
Copy link
Contributor

Thanks a lot! I think we can start a new data package here with a new test class for this recipe:
https://github.com/openrewrite/rewrite-spring/tree/main/src/testWithSpringBoot_2_7/java/org/openrewrite/java/spring

We likely need to add the spring-data dependency to this block of dependencies:

"testWithSpringBoot_2_7RuntimeOnly"("org.springframework:spring-context:5.3.+")
"testWithSpringBoot_2_7RuntimeOnly"("org.springframework.boot:spring-boot-starter:2.7.+")
"testWithSpringBoot_2_7RuntimeOnly"("org.springframework.boot:spring-boot:2.7.+")
"testWithSpringBoot_2_7RuntimeOnly"("org.springframework.boot:spring-boot-starter-test:2.7.+")
"testWithSpringBoot_2_7RuntimeOnly"("org.springframework:spring-web:5.3.+")
"testWithSpringBoot_2_7RuntimeOnly"("org.springframework:spring-webmvc:5.3.+")
"testWithSpringBoot_2_7RuntimeOnly"("org.springframework:spring-webflux:5.3.+")
"testWithSpringBoot_2_7RuntimeOnly"("org.springframework.security:spring-security-core:5.7.+")
"testWithSpringBoot_2_7RuntimeOnly"("org.springframework.security:spring-security-config:5.7.+")
"testWithSpringBoot_2_7RuntimeOnly"("org.springframework.security:spring-security-web:5.7.+")
"testWithSpringBoot_2_7RuntimeOnly"("org.springframework.security:spring-security-ldap:5.7.+")
"testWithSpringBoot_2_7RuntimeOnly"("org.apache.tomcat.embed:tomcat-embed-core:9.0.+")

Then the method pattern should match the getById method as defined here.
https://docs.spring.io/spring-data/jpa/docs/current/api/org/springframework/data/jpa/repository/JpaRepository.html#getById(ID)

Let me know if you'd like any help with setting up that test; a draft PR would let me help you most easily there.

@lucashan
Copy link
Contributor Author

lucashan commented Apr 24, 2024

Sounds good! Do you mind creating a branch that I can use for the PR? I don't have write access for this issue.

@timtebeek
Copy link
Contributor

Feel free to create a fork of this project and a branch there. Then you can open a draft PR that I can edit. I don't think that works the other way around when I first create a branch, as there's no way for me to them easily give you limited access.

@lucashan
Copy link
Contributor Author

lucashan commented Apr 25, 2024

Thank you, I had reduced bandwidth today but will work on this tomorrow! I've also created another recipe issue, apologies in advanced if it was created in the wrong OpenRewrite repository.

@lucashan
Copy link
Contributor Author

Here's the draft PR! unfortunately I have not been able to figure out how to properly write the unit test/update the build.gradle.kts. Would it be possible to take a look at the PR? #518

@timtebeek
Copy link
Contributor

Awesome thanks! I'll try to fit in writing tests for this case on your PR! :)

@timtebeek
Copy link
Contributor

@github-project-automation github-project-automation bot moved this from Recipes Wanted to Done in OpenRewrite Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

No branches or pull requests

2 participants