From e339fec6976c44e3c277260959e480e3ce58237c Mon Sep 17 00:00:00 2001 From: marko-bekhta Date: Fri, 8 Sep 2023 17:35:49 +0200 Subject: [PATCH] Check that embedded property types are marked as @Embeddable --- .../orm/deployment/HibernateOrmProcessor.java | 3 +- .../orm/deployment/JpaJandexScavenger.java | 28 +++- ...hancerMissingEmbeddableAnnotationTest.java | 154 ++++++++++++++++++ 3 files changed, 176 insertions(+), 9 deletions(-) create mode 100644 extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/enhancer/HibernateEntityEnhancerMissingEmbeddableAnnotationTest.java diff --git a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java index cd4fa7da36329..55d6ebe006545 100644 --- a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java +++ b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java @@ -78,6 +78,7 @@ import io.quarkus.arc.deployment.ValidationPhaseBuildItem.ValidationErrorBuildItem; import io.quarkus.arc.deployment.staticmethods.InterceptedStaticMethodsTransformersRegisteredBuildItem; import io.quarkus.arc.processor.DotNames; +import io.quarkus.builder.BuildException; import io.quarkus.datasource.common.runtime.DataSourceUtil; import io.quarkus.datasource.common.runtime.DatabaseKind; import io.quarkus.deployment.Capabilities; @@ -393,7 +394,7 @@ public void defineJpaEntities( List ignorableNonIndexedClassesBuildItems, BuildProducer reflectiveClass, BuildProducer hotDeploymentWatchedFiles, - List jpaModelPuContributions) { + List jpaModelPuContributions) throws BuildException { Set ignorableNonIndexedClasses = Collections.emptySet(); if (!ignorableNonIndexedClassesBuildItems.isEmpty()) { diff --git a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/JpaJandexScavenger.java b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/JpaJandexScavenger.java index 84edc7d837e66..225b2a406491e 100644 --- a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/JpaJandexScavenger.java +++ b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/JpaJandexScavenger.java @@ -38,6 +38,7 @@ import org.jboss.jandex.IndexView; import org.jboss.jandex.Type; +import io.quarkus.builder.BuildException; import io.quarkus.deployment.annotations.BuildProducer; import io.quarkus.deployment.builditem.HotDeploymentWatchedFileBuildItem; import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem; @@ -82,7 +83,7 @@ public final class JpaJandexScavenger { this.ignorableNonIndexedClasses = ignorableNonIndexedClasses; } - public JpaModelBuildItem discoverModelAndRegisterForReflection() { + public JpaModelBuildItem discoverModelAndRegisterForReflection() throws BuildException { Collector collector = new Collector(); for (DotName packageAnnotation : HibernateOrmTypes.PACKAGE_ANNOTATIONS) { @@ -308,7 +309,7 @@ private void enlistExplicitClass(Collector collector, String className) { addClassHierarchyToReflectiveList(collector, dotName); } - private void enlistEmbeddedsAndElementCollections(Collector collector) { + private void enlistEmbeddedsAndElementCollections(Collector collector) throws BuildException { Set embeddedTypes = new HashSet<>(); for (DotName embeddedAnnotation : EMBEDDED_ANNOTATIONS) { @@ -319,10 +320,10 @@ private void enlistEmbeddedsAndElementCollections(Collector collector) { switch (target.kind()) { case FIELD: - collectEmbeddedTypes(embeddedTypes, target.asField().type()); + collectEmbeddedTypes(embeddedAnnotation, embeddedTypes, target.asField().type()); break; case METHOD: - collectEmbeddedTypes(embeddedTypes, target.asMethod().returnType()); + collectEmbeddedTypes(embeddedAnnotation, embeddedTypes, target.asMethod().returnType()); break; default: throw new IllegalStateException( @@ -480,19 +481,22 @@ private static void collectModelType(Collector collector, ClassInfo modelClass) } } - private static void collectEmbeddedTypes(Set embeddedTypes, Type indexType) { + private void collectEmbeddedTypes(DotName embeddedAnnotation, Set embeddedTypes, Type indexType) + throws BuildException { switch (indexType.kind()) { case CLASS: - embeddedTypes.add(indexType.asClassType().name()); + DotName className = indexType.asClassType().name(); + validateEmbeddable(embeddedAnnotation, className); + embeddedTypes.add(className); break; case PARAMETERIZED_TYPE: embeddedTypes.add(indexType.name()); for (Type typeArgument : indexType.asParameterizedType().arguments()) { - collectEmbeddedTypes(embeddedTypes, typeArgument); + collectEmbeddedTypes(embeddedAnnotation, embeddedTypes, typeArgument); } break; case ARRAY: - collectEmbeddedTypes(embeddedTypes, indexType.asArrayType().constituent()); + collectEmbeddedTypes(embeddedAnnotation, embeddedTypes, indexType.asArrayType().constituent()); break; default: // do nothing @@ -500,6 +504,14 @@ private static void collectEmbeddedTypes(Set embeddedTypes, Type indexT } } + private void validateEmbeddable(DotName embeddedAnnotation, DotName className) throws BuildException { + if ((ClassNames.EMBEDDED.equals(embeddedAnnotation) || ClassNames.EMBEDDED_ID.equals(embeddedAnnotation)) + && !index.getClassByName(className).hasAnnotation(ClassNames.EMBEDDABLE)) { + throw new BuildException( + className + " is used as an embeddable but does not have an @Embeddable annotation."); + } + } + private static boolean isIgnored(DotName classDotName) { String className = classDotName.toString(); if (className.startsWith("java.util.") || className.startsWith("java.lang.") diff --git a/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/enhancer/HibernateEntityEnhancerMissingEmbeddableAnnotationTest.java b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/enhancer/HibernateEntityEnhancerMissingEmbeddableAnnotationTest.java new file mode 100644 index 0000000000000..f880a2193949a --- /dev/null +++ b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/enhancer/HibernateEntityEnhancerMissingEmbeddableAnnotationTest.java @@ -0,0 +1,154 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package io.quarkus.hibernate.orm.enhancer; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.fail; + +import jakarta.persistence.Embeddable; +import jakarta.persistence.Embedded; +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.Id; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.hibernate.orm.TransactionTestUtils; +import io.quarkus.test.QuarkusUnitTest; + +/** + * Checks that a missing @Embeddable is reported as failure at the build stage rather than a cryptic error at the runtime. + * See https://github.com/quarkusio/quarkus/issues/35598 + */ +class HibernateEntityEnhancerMissingEmbeddableAnnotationTest { + + @RegisterExtension + static QuarkusUnitTest runner = new QuarkusUnitTest() + .withApplicationRoot((jar) -> jar + .addClass(TransactionTestUtils.class) + .addClasses( + EntityWithEmbedded.class, + EntityWithEmbedded.EmbeddableMissingAnnotation.class, + EntityWithEmbedded.EmbeddableWithAnnotation.class)) + .withConfigurationResource("application.properties") + .assertException(ex -> assertThat(ex) + .isNotNull() + .hasMessageContainingAll( + EntityWithEmbedded.EmbeddableMissingAnnotation.class.getName(), + "is used as an embeddable but does not have an @Embeddable annotation")); + + // Just test that the embedded non-ID works correctly over a persist/retrieve cycle + @Test + void test() throws Exception { + fail(); + } + + @Entity + public static class EntityWithEmbedded { + + @Id + @GeneratedValue + private Long id; + + private String name; + + @Embedded + private EmbeddableWithAnnotationExtended embedded; + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public EmbeddableWithAnnotationExtended getEmbedded() { + return embedded; + } + + public void setEmbedded(EmbeddableWithAnnotationExtended otherEmbedded) { + this.embedded = otherEmbedded; + } + + // @Embeddable // is missing on this class + public static class EmbeddableMissingAnnotation { + private String string; + + public EmbeddableMissingAnnotation() { + } + + public EmbeddableMissingAnnotation(String string) { + this.string = string; + } + + public String getString() { + return string; + } + + public void setString(String string) { + this.string = string; + } + } + + @Embeddable + public static class EmbeddableWithAnnotation { + private String text; + + @Embedded + private EmbeddableMissingAnnotation embeddableMissingAnnotation; + + protected EmbeddableWithAnnotation() { + // For Hibernate ORM only - it will change the property value through reflection + } + + public EmbeddableWithAnnotation(String text) { + this.text = text; + this.embeddableMissingAnnotation = new EmbeddableMissingAnnotation(text); + } + + public String getText() { + return text; + } + + public void setText(String mutableProperty) { + this.text = mutableProperty; + } + + public EmbeddableMissingAnnotation getEmbeddableMissingAnnotation() { + return embeddableMissingAnnotation; + } + + public void setEmbeddableMissingAnnotation(EmbeddableMissingAnnotation embeddableMissingAnnotation) { + this.embeddableMissingAnnotation = embeddableMissingAnnotation; + } + } + + @Embeddable + public static class EmbeddableWithAnnotationExtended extends EmbeddableWithAnnotation { + private Integer integer; + + public Integer getInteger() { + return integer; + } + + public void setInteger(Integer integer) { + this.integer = integer; + } + } + } + +}