Merge pull request #6191 from FroMage/5885

Fix #5885: generics break panache repo enhancer
quarkusio · Dec 16, 2019 · d00260f · d00260f
2 parents 535e033 + 75489e2
commit d00260f
Show file tree

Hide file tree

Showing 11 changed files with 86 additions and 10 deletions.
diff --git a/...t/src/main/java/io/quarkus/hibernate/orm/panache/deployment/PanacheResourceProcessor.java b/...t/src/main/java/io/quarkus/hibernate/orm/panache/deployment/PanacheResourceProcessor.java
@@ -30,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 {
 
@@ -82,9 +83,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) {
@@ -95,6 +100,7 @@ void build(CombinedIndexBuildItem index,
         Set<String> 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))
@@ -121,5 +127,4 @@ void build(CombinedIndexBuildItem index,
             }
         }
     }
-
 }
diff --git a/...loyment/src/main/java/io/quarkus/mongodb/panache/deployment/PanacheResourceProcessor.java b/...loyment/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/...loyment/src/main/java/io/quarkus/panache/common/deployment/PanacheRepositoryEnhancer.java b/...loyment/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;
 
@@ -90,15 +91,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<org.jboss.jandex.Type> 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
@@ -167,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();
+    }
 }
diff --git a/.../hibernate-orm-panache/src/main/java/io/quarkus/it/panache/Bug5885AbstractRepository.java b/.../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<T> implements PanacheRepository<T> {
+
+}
diff --git a/...ts/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/Bug5885EntityRepository.java b/...ts/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<Person> {
+}
diff --git a/...gration-tests/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/TestEndpoint.java b/...gration-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/...s/hibernate-orm-panache/src/test/java/io/quarkus/it/panache/PanacheFunctionalityTest.java b/...s/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.
      */

diff --git a/...b-panache/src/main/java/io/quarkus/it/mongodb/panache/bugs/Bug5885AbstractRepository.java b/...b-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<T> implements PanacheMongoRepositoryBase<T, Long> {
+
+}
diff --git a/...odb-panache/src/main/java/io/quarkus/it/mongodb/panache/bugs/Bug5885EntityRepository.java b/...odb-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<PersonEntity> {
+}
diff --git a/...n-tests/mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/bugs/BugResource.java b/...n-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/...ngodb-panache/src/test/java/io/quarkus/it/mongodb/panache/MongodbPanacheResourceTest.java b/...ngodb-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"));
+    }
 }