From 53d3886a17aba21f4f2d1940a72744b8046300f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20=C3=89pardaud?= Date: Mon, 16 Dec 2019 10:36:57 +0100 Subject: [PATCH 1/3] Fix #5885: generics break panache repo enhancer Do not enhance generic/abstract repos Use proper generics resolution when looking up the repository type arguments --- .../deployment/PanacheResourceProcessor.java | 13 +++++++++++++ .../deployment/PanacheRepositoryEnhancer.java | 16 +++++++--------- .../it/panache/Bug5885AbstractRepository.java | 7 +++++++ .../it/panache/Bug5885EntityRepository.java | 7 +++++++ .../java/io/quarkus/it/panache/TestEndpoint.java | 11 +++++++++++ .../it/panache/PanacheFunctionalityTest.java | 5 +++++ 6 files changed, 50 insertions(+), 9 deletions(-) create mode 100644 integration-tests/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/Bug5885AbstractRepository.java create mode 100644 integration-tests/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/Bug5885EntityRepository.java diff --git a/extensions/panache/hibernate-orm-panache/deployment/src/main/java/io/quarkus/hibernate/orm/panache/deployment/PanacheResourceProcessor.java b/extensions/panache/hibernate-orm-panache/deployment/src/main/java/io/quarkus/hibernate/orm/panache/deployment/PanacheResourceProcessor.java index bbfb8146778fd..1e4a9c2844cfb 100644 --- a/extensions/panache/hibernate-orm-panache/deployment/src/main/java/io/quarkus/hibernate/orm/panache/deployment/PanacheResourceProcessor.java +++ b/extensions/panache/hibernate-orm-panache/deployment/src/main/java/io/quarkus/hibernate/orm/panache/deployment/PanacheResourceProcessor.java @@ -1,5 +1,6 @@ package io.quarkus.hibernate.orm.panache.deployment; +import java.lang.reflect.Modifier; import java.util.Collections; import java.util.HashSet; import java.util.List; @@ -82,9 +83,13 @@ void build(CombinedIndexBuildItem index, // Skip PanacheRepository if (classInfo.name().equals(DOTNAME_PANACHE_REPOSITORY)) continue; + if (skipRepository(classInfo)) + continue; daoClasses.add(classInfo.name().toString()); } for (ClassInfo classInfo : index.getIndex().getAllKnownImplementors(DOTNAME_PANACHE_REPOSITORY)) { + if (skipRepository(classInfo)) + continue; daoClasses.add(classInfo.name().toString()); } for (String daoClass : daoClasses) { @@ -95,6 +100,7 @@ void build(CombinedIndexBuildItem index, Set modelClasses = new HashSet<>(); // Note that we do this in two passes because for some reason Jandex does not give us subtypes // of PanacheEntity if we ask for subtypes of PanacheEntityBase + // NOTE: we don't skip abstract/generic entities because they still need accessors for (ClassInfo classInfo : index.getIndex().getAllKnownSubclasses(DOTNAME_PANACHE_ENTITY_BASE)) { // FIXME: should we really skip PanacheEntity or all MappedSuperClass? if (classInfo.name().equals(DOTNAME_PANACHE_ENTITY)) @@ -122,4 +128,11 @@ void build(CombinedIndexBuildItem index, } } + private boolean skipRepository(ClassInfo classInfo) { + // we don't want to add methods to abstract/generic entities/repositories: they get added to bottom types + // which can't be either + return Modifier.isAbstract(classInfo.flags()) + || !classInfo.typeParameters().isEmpty(); + } + } diff --git a/extensions/panache/panache-common/deployment/src/main/java/io/quarkus/panache/common/deployment/PanacheRepositoryEnhancer.java b/extensions/panache/panache-common/deployment/src/main/java/io/quarkus/panache/common/deployment/PanacheRepositoryEnhancer.java index 4c8a54154a559..af1bc0dea32cd 100644 --- a/extensions/panache/panache-common/deployment/src/main/java/io/quarkus/panache/common/deployment/PanacheRepositoryEnhancer.java +++ b/extensions/panache/panache-common/deployment/src/main/java/io/quarkus/panache/common/deployment/PanacheRepositoryEnhancer.java @@ -90,15 +90,13 @@ private String recursivelyFindEntityTypeFromClass(DotName clazz, DotName reposit return null; } - final ClassInfo classByName = indexView.getClassByName(clazz); - for (org.jboss.jandex.Type type : classByName.interfaceTypes()) { - if (type.name().equals(repositoryDotName)) { - org.jboss.jandex.Type entityType = type.asParameterizedType().arguments().get(0); - return entityType.name().toString().replace('.', '/'); - } - } - - return recursivelyFindEntityTypeFromClass(classByName.superName(), repositoryDotName); + List typeParameters = io.quarkus.deployment.util.JandexUtil + .resolveTypeParameters(clazz, repositoryDotName, indexView); + if (typeParameters.isEmpty()) + throw new IllegalStateException( + "Failed to find supertype " + repositoryDotName + " from entity class " + clazz); + org.jboss.jandex.Type entityType = typeParameters.get(0); + return entityType.name().toString().replace('.', '/'); } @Override diff --git a/integration-tests/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/Bug5885AbstractRepository.java b/integration-tests/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/Bug5885AbstractRepository.java new file mode 100644 index 0000000000000..0bd118580bf55 --- /dev/null +++ b/integration-tests/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/Bug5885AbstractRepository.java @@ -0,0 +1,7 @@ +package io.quarkus.it.panache; + +import io.quarkus.hibernate.orm.panache.PanacheRepository; + +public abstract class Bug5885AbstractRepository implements PanacheRepository { + +} diff --git a/integration-tests/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/Bug5885EntityRepository.java b/integration-tests/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/Bug5885EntityRepository.java new file mode 100644 index 0000000000000..be9d686c6adc0 --- /dev/null +++ b/integration-tests/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/Bug5885EntityRepository.java @@ -0,0 +1,7 @@ +package io.quarkus.it.panache; + +import javax.enterprise.context.ApplicationScoped; + +@ApplicationScoped +public class Bug5885EntityRepository extends Bug5885AbstractRepository { +} diff --git a/integration-tests/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/TestEndpoint.java b/integration-tests/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/TestEndpoint.java index 9aa806f7266f0..0a4e1d2bf3ac2 100644 --- a/integration-tests/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/TestEndpoint.java +++ b/integration-tests/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/TestEndpoint.java @@ -987,4 +987,15 @@ public String testBug5274() { bug5274EntityRepository.count(); return "OK"; } + + @Inject + Bug5885EntityRepository bug5885EntityRepository; + + @GET + @Path("5885") + @Transactional + public String testBug5885() { + bug5885EntityRepository.findById(1L); + return "OK"; + } } diff --git a/integration-tests/hibernate-orm-panache/src/test/java/io/quarkus/it/panache/PanacheFunctionalityTest.java b/integration-tests/hibernate-orm-panache/src/test/java/io/quarkus/it/panache/PanacheFunctionalityTest.java index 4c5b29fdfadd7..8ee58859b2615 100644 --- a/integration-tests/hibernate-orm-panache/src/test/java/io/quarkus/it/panache/PanacheFunctionalityTest.java +++ b/integration-tests/hibernate-orm-panache/src/test/java/io/quarkus/it/panache/PanacheFunctionalityTest.java @@ -54,6 +54,11 @@ public void testBug5274() { RestAssured.when().get("/test/5274").then().body(is("OK")); } + @Test + public void testBug5885() { + RestAssured.when().get("/test/5885").then().body(is("OK")); + } + /** * This test is disabled in native mode as there is no interaction with the quarkus integration test endpoint. */ From e830a7914b454d1a85fdec7219294b9f0b446529 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20=C3=89pardaud?= Date: Mon, 16 Dec 2019 12:22:22 +0100 Subject: [PATCH 2/3] Moved skipRepository up --- .../deployment/PanacheResourceProcessor.java | 14 +++----------- .../deployment/PanacheResourceProcessor.java | 5 +++++ .../deployment/PanacheRepositoryEnhancer.java | 8 ++++++++ 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/extensions/panache/hibernate-orm-panache/deployment/src/main/java/io/quarkus/hibernate/orm/panache/deployment/PanacheResourceProcessor.java b/extensions/panache/hibernate-orm-panache/deployment/src/main/java/io/quarkus/hibernate/orm/panache/deployment/PanacheResourceProcessor.java index 1e4a9c2844cfb..82ce06315c2b5 100644 --- a/extensions/panache/hibernate-orm-panache/deployment/src/main/java/io/quarkus/hibernate/orm/panache/deployment/PanacheResourceProcessor.java +++ b/extensions/panache/hibernate-orm-panache/deployment/src/main/java/io/quarkus/hibernate/orm/panache/deployment/PanacheResourceProcessor.java @@ -1,6 +1,5 @@ package io.quarkus.hibernate.orm.panache.deployment; -import java.lang.reflect.Modifier; import java.util.Collections; import java.util.HashSet; import java.util.List; @@ -31,6 +30,7 @@ import io.quarkus.panache.common.deployment.EntityModel; import io.quarkus.panache.common.deployment.MetamodelInfo; import io.quarkus.panache.common.deployment.PanacheFieldAccessEnhancer; +import io.quarkus.panache.common.deployment.PanacheRepositoryEnhancer; public final class PanacheResourceProcessor { @@ -83,12 +83,12 @@ void build(CombinedIndexBuildItem index, // Skip PanacheRepository if (classInfo.name().equals(DOTNAME_PANACHE_REPOSITORY)) continue; - if (skipRepository(classInfo)) + if (PanacheRepositoryEnhancer.skipRepository(classInfo)) continue; daoClasses.add(classInfo.name().toString()); } for (ClassInfo classInfo : index.getIndex().getAllKnownImplementors(DOTNAME_PANACHE_REPOSITORY)) { - if (skipRepository(classInfo)) + if (PanacheRepositoryEnhancer.skipRepository(classInfo)) continue; daoClasses.add(classInfo.name().toString()); } @@ -127,12 +127,4 @@ void build(CombinedIndexBuildItem index, } } } - - private boolean skipRepository(ClassInfo classInfo) { - // we don't want to add methods to abstract/generic entities/repositories: they get added to bottom types - // which can't be either - return Modifier.isAbstract(classInfo.flags()) - || !classInfo.typeParameters().isEmpty(); - } - } diff --git a/extensions/panache/mongodb-panache/deployment/src/main/java/io/quarkus/mongodb/panache/deployment/PanacheResourceProcessor.java b/extensions/panache/mongodb-panache/deployment/src/main/java/io/quarkus/mongodb/panache/deployment/PanacheResourceProcessor.java index d7806dbfd25dc..616e81b9bc04e 100644 --- a/extensions/panache/mongodb-panache/deployment/src/main/java/io/quarkus/mongodb/panache/deployment/PanacheResourceProcessor.java +++ b/extensions/panache/mongodb-panache/deployment/src/main/java/io/quarkus/mongodb/panache/deployment/PanacheResourceProcessor.java @@ -28,6 +28,7 @@ import io.quarkus.mongodb.panache.PanacheMongoRepository; import io.quarkus.mongodb.panache.PanacheMongoRepositoryBase; import io.quarkus.panache.common.deployment.PanacheFieldAccessEnhancer; +import io.quarkus.panache.common.deployment.PanacheRepositoryEnhancer; public class PanacheResourceProcessor { static final DotName DOTNAME_PANACHE_REPOSITORY_BASE = DotName.createSimple(PanacheMongoRepositoryBase.class.getName()); @@ -88,9 +89,13 @@ void build(CombinedIndexBuildItem index, // Skip PanacheRepository if (classInfo.name().equals(DOTNAME_PANACHE_REPOSITORY)) continue; + if (PanacheRepositoryEnhancer.skipRepository(classInfo)) + continue; daoClasses.add(classInfo.name().toString()); } for (ClassInfo classInfo : index.getIndex().getAllKnownImplementors(DOTNAME_PANACHE_REPOSITORY)) { + if (PanacheRepositoryEnhancer.skipRepository(classInfo)) + continue; daoClasses.add(classInfo.name().toString()); } for (String daoClass : daoClasses) { diff --git a/extensions/panache/panache-common/deployment/src/main/java/io/quarkus/panache/common/deployment/PanacheRepositoryEnhancer.java b/extensions/panache/panache-common/deployment/src/main/java/io/quarkus/panache/common/deployment/PanacheRepositoryEnhancer.java index af1bc0dea32cd..4a5fb5f99b54a 100644 --- a/extensions/panache/panache-common/deployment/src/main/java/io/quarkus/panache/common/deployment/PanacheRepositoryEnhancer.java +++ b/extensions/panache/panache-common/deployment/src/main/java/io/quarkus/panache/common/deployment/PanacheRepositoryEnhancer.java @@ -1,5 +1,6 @@ package io.quarkus.panache.common.deployment; +import java.lang.reflect.Modifier; import java.util.List; import java.util.function.BiFunction; @@ -165,4 +166,11 @@ private void generateMethod(MethodInfo method, AnnotationValue targetReturnTypeE mv.visitEnd(); } } + + public static boolean skipRepository(ClassInfo classInfo) { + // we don't want to add methods to abstract/generic entities/repositories: they get added to bottom types + // which can't be either + return Modifier.isAbstract(classInfo.flags()) + || !classInfo.typeParameters().isEmpty(); + } } From 75489e29496bd114cc86407ea4261d76394d60eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20=C3=89pardaud?= Date: Mon, 16 Dec 2019 12:22:46 +0100 Subject: [PATCH 3/3] Test for mongo version of #5885 bug --- .../panache/bugs/Bug5885AbstractRepository.java | 7 +++++++ .../mongodb/panache/bugs/Bug5885EntityRepository.java | 9 +++++++++ .../quarkus/it/mongodb/panache/bugs/BugResource.java | 10 ++++++++++ .../it/mongodb/panache/MongodbPanacheResourceTest.java | 4 ++++ 4 files changed, 30 insertions(+) create mode 100644 integration-tests/mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/bugs/Bug5885AbstractRepository.java create mode 100644 integration-tests/mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/bugs/Bug5885EntityRepository.java diff --git a/integration-tests/mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/bugs/Bug5885AbstractRepository.java b/integration-tests/mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/bugs/Bug5885AbstractRepository.java new file mode 100644 index 0000000000000..4b4549792bac3 --- /dev/null +++ b/integration-tests/mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/bugs/Bug5885AbstractRepository.java @@ -0,0 +1,7 @@ +package io.quarkus.it.mongodb.panache.bugs; + +import io.quarkus.mongodb.panache.PanacheMongoRepositoryBase; + +public abstract class Bug5885AbstractRepository implements PanacheMongoRepositoryBase { + +} diff --git a/integration-tests/mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/bugs/Bug5885EntityRepository.java b/integration-tests/mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/bugs/Bug5885EntityRepository.java new file mode 100644 index 0000000000000..bb9642055fa40 --- /dev/null +++ b/integration-tests/mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/bugs/Bug5885EntityRepository.java @@ -0,0 +1,9 @@ +package io.quarkus.it.mongodb.panache.bugs; + +import javax.enterprise.context.ApplicationScoped; + +import io.quarkus.it.mongodb.panache.person.PersonEntity; + +@ApplicationScoped +public class Bug5885EntityRepository extends Bug5885AbstractRepository { +} diff --git a/integration-tests/mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/bugs/BugResource.java b/integration-tests/mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/bugs/BugResource.java index 788c811915a7d..bc39e8c657cdb 100644 --- a/integration-tests/mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/bugs/BugResource.java +++ b/integration-tests/mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/bugs/BugResource.java @@ -21,4 +21,14 @@ public String testBug5274() { bug5274EntityRepository.count(); return "OK"; } + + @Inject + Bug5885EntityRepository bug5885EntityRepository; + + @GET + @Path("5885") + public String testBug5885() { + bug5885EntityRepository.findById(1L); + return "OK"; + } } diff --git a/integration-tests/mongodb-panache/src/test/java/io/quarkus/it/mongodb/panache/MongodbPanacheResourceTest.java b/integration-tests/mongodb-panache/src/test/java/io/quarkus/it/mongodb/panache/MongodbPanacheResourceTest.java index ff899be5d54a2..2c75db6d87471 100644 --- a/integration-tests/mongodb-panache/src/test/java/io/quarkus/it/mongodb/panache/MongodbPanacheResourceTest.java +++ b/integration-tests/mongodb-panache/src/test/java/io/quarkus/it/mongodb/panache/MongodbPanacheResourceTest.java @@ -334,4 +334,8 @@ public void testBug5274() { get("/bugs/5274").then().body(is("OK")); } + @Test + public void testBug5885() { + get("/bugs/5885").then().body(is("OK")); + } }