Migrate UserType implementations to hibernate 6
#40
+456
−4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
src/test/java/org/openrewrite/hibernate/MigrateUserTypeTest.java
Outdated
Show resolved
Hide resolved
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
timtebeek
reviewed
Oct 6, 2024
| void shouldMigrateUserType() { | ||
| //language=java | ||
| rewriteRun( | ||
| spec -> spec.expectedCyclesThatMakeChanges(2), |
There was a problem hiding this comment.
Ideally we fix the recipe such that we don't need this escape hatch, as we'd love to drop that in a next major version. Did you explore why this was needed in the first place?
Suggested change
| spec -> spec.expectedCyclesThatMakeChanges(2), |
| import org.openrewrite.java.JavaVisitor; | ||
| import org.openrewrite.java.MethodMatcher; | ||
| import org.openrewrite.java.search.UsesType; | ||
| import org.openrewrite.java.tree.*; |
There was a problem hiding this comment.
Suggested change
| import org.openrewrite.java.tree.*; | |
| import org.openrewrite.java.tree.Expression; | |
| import org.openrewrite.java.tree.J; | |
| import org.openrewrite.java.tree.ControlParentheses; | |
| import org.openrewrite.java.tree.JRightPadded; | |
| import org.openrewrite.java.tree.JavaType; | |
| import org.openrewrite.java.tree.Space; | |
| import org.openrewrite.java.tree.Statement; | |
| import org.openrewrite.java.tree.TypeTree; | |
| import org.openrewrite.java.tree.TypeUtils; |
timtebeek
reviewed
Oct 6, 2024
Comment on lines
+65
to
+66
| return "With hibernate 6 the UserType interface received a type parameter making it more strictly typed" + | ||
| "This recipe applies the changes required to adhere to this change."; |
There was a problem hiding this comment.
Be careful when you join strings; this would look odd in the docs without . in between.
Suggested change
| return "With hibernate 6 the UserType interface received a type parameter making it more strictly typed" + | |
| "This recipe applies the changes required to adhere to this change."; | |
| return "With Hibernate 6 the `UserType` interface received a type parameter making it more strictly typed. " + | |
| "This recipe applies the changes required to adhere to this change."; |
timtebeek
reviewed
Oct 6, 2024
Comment on lines
+42
to
+53
| private static final MethodMatcher SQL_TYPES = new MethodMatcher("* sqlTypes()"); | ||
| private static final MethodMatcher RETURNED_CLASS = new MethodMatcher("* returnedClass()"); | ||
| private static final MethodMatcher EQUALS = new MethodMatcher("* equals(java.lang.Object, java.lang.Object)"); | ||
| private static final MethodMatcher HASHCODE = new MethodMatcher("* hashCode(java.lang.Object)"); | ||
| private static final MethodMatcher NULL_SAFE_GET_STRING_ARRAY = new MethodMatcher("* nullSafeGet(java.sql.ResultSet, java.lang.String[], org.hibernate.engine.spi.SharedSessionContractImplementor, java.lang.Object)"); | ||
| private static final MethodMatcher NULL_SAFE_GET_INT = new MethodMatcher("* nullSafeGet(java.sql.ResultSet, int, org.hibernate.engine.spi.SharedSessionContractImplementor, java.lang.Object)"); | ||
| private static final MethodMatcher RESULT_SET_STRING_PARAM = new MethodMatcher("java.sql.ResultSet *(java.lang.String)"); | ||
| private static final MethodMatcher NULL_SAFE_SET = new MethodMatcher("* nullSafeSet(java.sql.PreparedStatement, java.lang.Object, int, org.hibernate.engine.spi.SharedSessionContractImplementor)"); | ||
| private static final MethodMatcher DEEP_COPY = new MethodMatcher("* deepCopy(java.lang.Object)"); | ||
| private static final MethodMatcher DISASSEMBLE = new MethodMatcher("* disassemble(java.lang.Object)"); | ||
| private static final MethodMatcher ASSEMBLE = new MethodMatcher("* assemble(java.io.Serializable, java.lang.Object)"); | ||
| private static final MethodMatcher REPLACE = new MethodMatcher("* replace(java.lang.Object, java.lang.Object, java.lang.Object)"); |
There was a problem hiding this comment.
The order of these fields feels random; could you clean that up such that it's easier to understand & extend?
timtebeek
reviewed
Oct 6, 2024
Comment on lines
+55
to
+56
| @Nullable | ||
| private static J.FieldAccess parameterizedType = null; |
There was a problem hiding this comment.
Mutable static fields in a recipe are not ideal; can we move this field into the visitor instead? As each call the getVisitor returns a new isolated instance, whereas the Recipe instance can be shared.
There was a problem hiding this comment.
Suggested change
| @Nullable | |
| private static J.FieldAccess parameterizedType = null; |
There was a problem hiding this comment.
Maybe we should check if we have any legitimate cases of this. If not, we can add a best practice recipe, maybe?
timtebeek
reviewed
Oct 6, 2024
|
|
||
| @Override | ||
| public String getDisplayName() { | ||
| return "Migrate UserType to hibernate 6"; |
There was a problem hiding this comment.
Lets pay closer attention to how this would look in the docs.
Suggested change
| return "Migrate UserType to hibernate 6"; | |
| return "Migrate `UserType` to Hibernate 6"; |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Anything in particular you'd like reviewers to focus on?
Test coverage
Anyone you would like to review specifically?
@timtebeek
Any additional context
https://docs.jboss.org/hibernate/orm/5.6/javadocs/org/hibernate/usertype/UserType.html
https://docs.jboss.org/hibernate/orm/6.0/javadocs/org/hibernate/usertype/UserType.html
Checklist