openrewrite / rewrite-spring

OpenRewrite recipes for Spring projects.
Apache License 2.0
237 stars 64 forks source link

Spring 2.7: Recipe to replace getById with getReferenceById #515

Closed lucashan closed 1 month ago

lucashan commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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

timtebeek commented 2 months ago

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: https://github.com/openrewrite/rewrite-spring/blob/5b5c130733fcbad3127144f9abe1133bb12f81ef/build.gradle.kts#L196-L207

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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? https://github.com/openrewrite/rewrite-spring/pull/518

timtebeek commented 2 months ago

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

timtebeek commented 1 month ago

Fixed in: https://github.com/openrewrite/rewrite-spring/pull/518

Thanks again!