Fix for 5274 (quarkusio#5276)

* Panache: do not create findById bridge for abstract entity repositories If the entity type is not fixed in the hierarchy yet, don't generate the bridge Fix quarkusio#5274 * Test for quarkusio#5274: duplicate findById for abstract repositories * Fix quarkusio#5274 for Mongo too * Test for quarkusio#5274 for Mongo
dmlloyd · Nov 19, 2019 · 7372929 · 7372929
1 parent cdadeb5
commit 7372929
Show file tree

Hide file tree

Showing 10 changed files with 114 additions and 35 deletions.
diff --git a/...c/main/java/io/quarkus/hibernate/orm/panache/deployment/PanacheJpaRepositoryEnhancer.java b/...c/main/java/io/quarkus/hibernate/orm/panache/deployment/PanacheJpaRepositoryEnhancer.java
@@ -51,24 +51,26 @@ protected String getPanacheOperationsBinaryName() {
 
         @Override
         public void visitEnd() {
-            // Bridge for findById
-            MethodVisitor mv = super.visitMethod(Opcodes.ACC_PUBLIC | Opcodes.ACC_SYNTHETIC | Opcodes.ACC_BRIDGE,
-                    "findById",
-                    "(Ljava/lang/Object;)Ljava/lang/Object;",
-                    null,
-                    null);
-            mv.visitParameter("id", 0);
-            mv.visitCode();
-            mv.visitIntInsn(Opcodes.ALOAD, 0);
-            mv.visitIntInsn(Opcodes.ALOAD, 1);
-            mv.visitMethodInsn(Opcodes.INVOKEVIRTUAL,
-                    daoBinaryName,
-                    "findById",
-                    "(Ljava/lang/Object;)" + entitySignature, false);
-            mv.visitInsn(Opcodes.ARETURN);
-            mv.visitMaxs(0, 0);
-            mv.visitEnd();
-
+            // Bridge for findById, but only if we actually know the end entity (which we don't for intermediate
+            // abstract repositories that haven't fixed their entity type yet
+            if (!"Ljava/lang/Object;".equals(entitySignature)) {
+                MethodVisitor mv = super.visitMethod(Opcodes.ACC_PUBLIC | Opcodes.ACC_SYNTHETIC | Opcodes.ACC_BRIDGE,
+                        "findById",
+                        "(Ljava/lang/Object;)Ljava/lang/Object;",
+                        null,
+                        null);
+                mv.visitParameter("id", 0);
+                mv.visitCode();
+                mv.visitIntInsn(Opcodes.ALOAD, 0);
+                mv.visitIntInsn(Opcodes.ALOAD, 1);
+                mv.visitMethodInsn(Opcodes.INVOKEVIRTUAL,
+                        daoBinaryName,
+                        "findById",
+                        "(Ljava/lang/Object;)" + entitySignature, false);
+                mv.visitInsn(Opcodes.ARETURN);
+                mv.visitMaxs(0, 0);
+                mv.visitEnd();
+            }
             super.visitEnd();
         }
 

diff --git a/...t/src/main/java/io/quarkus/mongodb/panache/deployment/PanacheMongoRepositoryEnhancer.java b/...t/src/main/java/io/quarkus/mongodb/panache/deployment/PanacheMongoRepositoryEnhancer.java
@@ -50,23 +50,26 @@ protected String getPanacheOperationsBinaryName() {
 
         @Override
         public void visitEnd() {
-            // Bridge for findById
-            MethodVisitor mv = super.visitMethod(Opcodes.ACC_PUBLIC | Opcodes.ACC_SYNTHETIC | Opcodes.ACC_BRIDGE,
-                    "findById",
-                    "(Ljava/lang/Object;)Ljava/lang/Object;",
-                    null,
-                    null);
-            mv.visitParameter("id", 0);
-            mv.visitCode();
-            mv.visitIntInsn(Opcodes.ALOAD, 0);
-            mv.visitIntInsn(Opcodes.ALOAD, 1);
-            mv.visitMethodInsn(Opcodes.INVOKEVIRTUAL,
-                    daoBinaryName,
-                    "findById",
-                    "(Ljava/lang/Object;)" + entitySignature, false);
-            mv.visitInsn(Opcodes.ARETURN);
-            mv.visitMaxs(0, 0);
-            mv.visitEnd();
+            // Bridge for findById, but only if we actually know the end entity (which we don't for intermediate
+            // abstract repositories that haven't fixed their entity type yet
+            if (!entitySignature.equals("Ljava/lang/Object;")) {
+                MethodVisitor mv = super.visitMethod(Opcodes.ACC_PUBLIC | Opcodes.ACC_SYNTHETIC | Opcodes.ACC_BRIDGE,
+                        "findById",
+                        "(Ljava/lang/Object;)Ljava/lang/Object;",
+                        null,
+                        null);
+                mv.visitParameter("id", 0);
+                mv.visitCode();
+                mv.visitIntInsn(Opcodes.ALOAD, 0);
+                mv.visitIntInsn(Opcodes.ALOAD, 1);
+                mv.visitMethodInsn(Opcodes.INVOKEVIRTUAL,
+                        daoBinaryName,
+                        "findById",
+                        "(Ljava/lang/Object;)" + entitySignature, false);
+                mv.visitInsn(Opcodes.ARETURN);
+                mv.visitMaxs(0, 0);
+                mv.visitEnd();
+            }
 
             super.visitEnd();
         }

diff --git a/...n-tests/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/AbstractRepository.java b/...n-tests/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/AbstractRepository.java
@@ -0,0 +1,6 @@
+package io.quarkus.it.panache;
+
+import io.quarkus.hibernate.orm.panache.PanacheRepositoryBase;
+
+public class AbstractRepository<T> implements PanacheRepositoryBase<T, String> {
+}
diff --git a/...ts/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/Bug5274EntityRepository.java b/...ts/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/Bug5274EntityRepository.java
@@ -0,0 +1,7 @@
+package io.quarkus.it.panache;
+
+import javax.enterprise.context.ApplicationScoped;
+
+@ApplicationScoped
+public class Bug5274EntityRepository extends AbstractRepository<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
@@ -789,4 +789,15 @@ public Person ignoredProperties() throws NoSuchMethodException, SecurityExceptio
         person.status = Status.DECEASED;
         return person;
     }
+
+    @Inject
+    Bug5274EntityRepository bug5274EntityRepository;
+
+    @GET
+    @Path("5274")
+    @Transactional
+    public String testBug5274() {
+        bug5274EntityRepository.count();
+        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
@@ -44,4 +44,9 @@ public void testPanacheSerialisation() {
     public void testPanacheInTest() {
         Assertions.assertEquals(0, Person.count());
     }
+
+    @Test
+    public void testBug5274() {
+        RestAssured.when().get("/test/5274").then().body(is("OK"));
+    }
 }
diff --git a/.../mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/bugs/AbstractRepository.java b/.../mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/bugs/AbstractRepository.java
@@ -0,0 +1,6 @@
+package io.quarkus.it.mongodb.panache.bugs;
+
+import io.quarkus.mongodb.panache.PanacheMongoRepositoryBase;
+
+public class AbstractRepository<T> implements PanacheMongoRepositoryBase<T, String> {
+}
diff --git a/...odb-panache/src/main/java/io/quarkus/it/mongodb/panache/bugs/Bug5274EntityRepository.java b/...odb-panache/src/main/java/io/quarkus/it/mongodb/panache/bugs/Bug5274EntityRepository.java
@@ -0,0 +1,9 @@
+package io.quarkus.it.mongodb.panache.bugs;
+
+import javax.enterprise.context.ApplicationScoped;
+
+import io.quarkus.it.mongodb.panache.book.Book;
+
+@ApplicationScoped
+public class Bug5274EntityRepository extends AbstractRepository<Book> {
+}
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
@@ -0,0 +1,24 @@
+package io.quarkus.it.mongodb.panache.bugs;
+
+import javax.inject.Inject;
+import javax.ws.rs.Consumes;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.Produces;
+import javax.ws.rs.core.MediaType;
+
+@Path("/bugs")
+@Produces(MediaType.TEXT_PLAIN)
+@Consumes(MediaType.TEXT_PLAIN)
+public class BugResource {
+
+    @Inject
+    Bug5274EntityRepository bug5274EntityRepository;
+
+    @GET
+    @Path("5274")
+    public String testBug5274() {
+        bug5274EntityRepository.count();
+        return "OK";
+    }
+}
diff --git a/...n-tests/mongodb-panache/src/test/java/io/quarkus/it/mongodb/panache/BookResourceTest.java b/...n-tests/mongodb-panache/src/test/java/io/quarkus/it/mongodb/panache/BookResourceTest.java
@@ -1,6 +1,7 @@
 package io.quarkus.it.mongodb.panache;
 
 import static io.restassured.RestAssured.get;
+import static org.hamcrest.Matchers.is;
 
 import java.io.IOException;
 import java.time.LocalDate;
@@ -324,4 +325,9 @@ private Date fromYear(int year) {
         return Date.from(LocalDate.of(year, 1, 1).atStartOfDay().toInstant(ZoneOffset.UTC));
     }
 
+    @Test
+    public void testBug5274() {
+        get("/bugs/5274").then().body(is("OK"));
+    }
+
 }