Skip to content

Commit

Permalink
Merge pull request #35822 from marko-bekhta/feat/i35598-check-embedda…
Browse files Browse the repository at this point in the history
…ble-is-not-missing

Check that embedded property types are marked as @embeddable
  • Loading branch information
Sanne authored Sep 10, 2023
2 parents 482501b + e339fec commit 21ec9a7
Show file tree
Hide file tree
Showing 3 changed files with 176 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -393,7 +394,7 @@ public void defineJpaEntities(
List<IgnorableNonIndexedClasses> ignorableNonIndexedClassesBuildItems,
BuildProducer<ReflectiveClassBuildItem> reflectiveClass,
BuildProducer<HotDeploymentWatchedFileBuildItem> hotDeploymentWatchedFiles,
List<JpaModelPersistenceUnitContributionBuildItem> jpaModelPuContributions) {
List<JpaModelPersistenceUnitContributionBuildItem> jpaModelPuContributions) throws BuildException {

Set<String> ignorableNonIndexedClasses = Collections.emptySet();
if (!ignorableNonIndexedClassesBuildItems.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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<DotName> embeddedTypes = new HashSet<>();

for (DotName embeddedAnnotation : EMBEDDED_ANNOTATIONS) {
Expand All @@ -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(
Expand Down Expand Up @@ -480,26 +481,37 @@ private static void collectModelType(Collector collector, ClassInfo modelClass)
}
}

private static void collectEmbeddedTypes(Set<DotName> embeddedTypes, Type indexType) {
private void collectEmbeddedTypes(DotName embeddedAnnotation, Set<DotName> 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
break;
}
}

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.")
Expand Down
Original file line number Diff line number Diff line change
@@ -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 <http://www.gnu.org/licenses/lgpl-2.1.html>.
*/
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;
}
}
}

}

0 comments on commit 21ec9a7

Please sign in to comment.