openrewrite / rewrite-hibernate

OpenRewrite recipes for Hibernate ORM.
Apache License 2.0
5 stars 7 forks source link

Boolean used to be @Type mappable, but now requires conversion (when migrating from hibernate 5 to 6) #18

Closed Laurens-W closed 6 months ago

Laurens-W commented 7 months ago

What version of OpenRewrite are you using?

I am using

How are you running OpenRewrite?

I am using the Maven plugin, and my project is a single module project.

mvn org.openrewrite.maven:rewrite-maven-plugin:5.22.0:run -Drewrite.activeRecipes=redacted -Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-spring:5.4.0,org.openrewrite.recipe:rewrite-static-analysis:1.3.0 -Drewrite.exclusions=**api**.yaml

What is the smallest, simplest way to reproduce the problem?

@Entity
class A {
  @Type(type = "boolean")
  private Boolean isAllowed;
}

What did you expect to see?

@Entity
class A {
  @Convert(converter = org.hibernate.type.NumericBooleanConverter.class)
  private Boolean isAllowed;
}

What did you see instead?

@Entity
class A {
  @Type(boolean.class)
  private Boolean isAllowed;
}

What is the full stack trace of any errors you encountered?

It's not necessarily an issue with openrewrite, more that the code left after the recipe ran is non compiling. I think the NumericConverter matches previous behaviour.

Are you interested in contributing a fix to OpenRewrite?

timtebeek commented 7 months ago

Thanks for logging this as an issue! Indeed it seems our TypeAnnotationParameter recipe right now converts this to something that does not compile. We can either add handling in that existing recipe, or write a new recipe that executes first to do this particular replacement. Here's a test to get that started.

@Test
void booleanParameter() {
    rewriteRun(
      //language=java
      java(
        """
          import org.hibernate.annotations.Type;

          public class TestApplication {
              @Type(type = "boolean")
              Object a;
          }
          """,
        """
          import org.hibernate.annotations.Converter;
          import org.hibernate.annotations.Type;

          public class TestApplication {
              @Convert(converter = org.hibernate.type.NumericBooleanConverter.class)
              Object a;
          }
          """
      )
    );
}
simonzn commented 7 months ago

I opened #20 for a similar issue - the recipe could easily be extended to fix this (if it is accepted)

simonzn commented 7 months ago

I extended #20 so the "boolean" mapping is migrated as well now. We have to somehow ensure that recipe runs before TypeAnnotationParameter, or TypeAnnotationParameter does nothing for type = "boolean and type = "true_false. Maybe it could check if the type-parameter is a valid FQN?

simonzn commented 7 months ago

Added MigrateBooleanMappings to the recipe list, ahead of TypeAnnotationParameter. See #20

simonzn commented 6 months ago

Unfortunately I found out that the "boolean" type uses a dialect-specific mapping, so replacing it with the NumericBooleanConverter might not always be the right thing to do.

See https://github.com/hibernate/hibernate-orm/blob/5.6.15/hibernate-core/src/main/java/org/hibernate/type/BooleanType.java#L57