Merge pull request quarkusio#18743 from gsmet/split-package-work

Fix split packages issue in Hibernate ORM with Panache
Postremus · Jul 16, 2021 · dc60271 · dc60271
2 parents 16dd69e + 4284f51
commit dc60271
Show file tree

Hide file tree

Showing 9 changed files with 99 additions and 23 deletions.
diff --git a/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/SplitPackageProcessor.java b/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/SplitPackageProcessor.java
@@ -6,6 +6,9 @@
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Set;
+import java.util.TreeSet;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
 
 import org.jboss.jandex.ClassInfo;
 import org.jboss.logging.Logger;
@@ -29,6 +32,17 @@ public class SplitPackageProcessor {
 
     private static final Logger LOGGER = Logger.getLogger(SplitPackageProcessor.class);
 
+    private static final Predicate<String> IGNORE_PACKAGE = new Predicate<>() {
+
+        @Override
+        public boolean test(String packageName) {
+            // Remove the elements from this list when the original issue is fixed
+            // so that we can detect further issues.
+            return packageName.startsWith("io.fabric8.kubernetes")
+                    || packageName.equals("io.quarkus.hibernate.orm.panache");
+        }
+    };
+
     @BuildStep
     void splitPackageDetection(ApplicationArchivesBuildItem archivesBuildItem,
             // Dummy producer to make sure the build step executes
@@ -48,46 +62,46 @@ void splitPackageDetection(ApplicationArchivesBuildItem archivesBuildItem,
         }
         // for each split package, create something like
         // - "com.me.app.sub" found in [archiveA, archiveB]
-        StringBuilder splitPackages = new StringBuilder();
+        StringBuilder splitPackagesWarning = new StringBuilder();
         for (String packageName : packageToArchiveMap.keySet()) {
+            if (IGNORE_PACKAGE.test(packageName)) {
+                continue;
+            }
+
             Set<ApplicationArchive> applicationArchives = packageToArchiveMap.get(packageName);
             if (applicationArchives.size() > 1) {
-                splitPackages.append("\n- \"" + packageName + "\" found in " + "[");
+                splitPackagesWarning.append("\n- \"" + packageName + "\" found in ");
                 Iterator<ApplicationArchive> iterator = applicationArchives.iterator();
+                Set<String> splitPackages = new TreeSet<>();
                 while (iterator.hasNext()) {
                     ApplicationArchive next = iterator.next();
                     AppArtifactKey a = next.getArtifactKey();
                     // can be null for instance in test mode where all application classes go under target/classes
                     if (a == null) {
                         if (archivesBuildItem.getRootArchive().equals(next)) {
                             // the archive we found is a root archive, e.g. application classes
-                            splitPackages.append("application classes");
+                            splitPackages.add("application classes");
                         } else {
                             // as next best effort, we try to take first path from archive paths collection
                             Iterator<Path> pathIterator = next.getPaths().iterator();
                             if (pathIterator.hasNext()) {
-                                splitPackages.append(pathIterator.next().toString());
+                                splitPackages.add(pathIterator.next().toString());
                             } else {
                                 // if all else fails, we mark it as unknown archive
-                                splitPackages.append("unknown archive");
+                                splitPackages.add("unknown archive");
                             }
                         }
-
-                    } else {
-                        splitPackages.append(a.getGroupId() + ":" + a.getArtifactId());
-                    }
-                    if (iterator.hasNext()) {
-                        splitPackages.append(", ");
                     } else {
-                        splitPackages.append("]");
+                        splitPackages.add(a.getGroupId() + ":" + a.getArtifactId());
                     }
                 }
+                splitPackagesWarning.append(splitPackages.stream().collect(Collectors.joining(", ", "[", "]")));
             }
         }
         // perform logging if needed
-        if (splitPackages.length() > 0) {
+        if (splitPackagesWarning.length() > 0) {
             LOGGER.warnf("Detected a split package usage which is considered a bad practice and should be avoided. " +
-                    "Following packages were detected in multiple archives: %s", splitPackages.toString());
+                    "Following packages were detected in multiple archives: %s", splitPackagesWarning.toString());
         }
     }
 }
diff --git a/...nt/PanacheJpaCommonResourceProcessor.java → ...nt/PanacheJpaCommonResourceProcessor.java b/...nt/PanacheJpaCommonResourceProcessor.java → ...nt/PanacheJpaCommonResourceProcessor.java
@@ -1,4 +1,4 @@
-package io.quarkus.hibernate.orm.panache.deployment;
+package io.quarkus.hibernate.orm.panache.common.deployment;
 
 import java.util.HashMap;
 import java.util.HashSet;

diff --git a/...anacheNamedQueryEntityClassBuildStep.java → ...anacheNamedQueryEntityClassBuildStep.java b/...anacheNamedQueryEntityClassBuildStep.java → ...anacheNamedQueryEntityClassBuildStep.java
@@ -1,4 +1,4 @@
-package io.quarkus.hibernate.orm.panache.deployment;
+package io.quarkus.hibernate.orm.panache.common.deployment;
 
 import java.util.Set;
 

diff --git a/...che-common/runtime/src/main/java/io/quarkus/hibernate/orm/panache/ProjectedFieldName.java b/...che-common/runtime/src/main/java/io/quarkus/hibernate/orm/panache/ProjectedFieldName.java
@@ -9,9 +9,12 @@
  * Define a field's path for the SELECT statement when using a projection DTO.
  * It supports the "dot" notation for fields in referenced entities:
  * the name is composed of the property name for the relationship, followed by a dot ("."), followed by the name of the field.
+ *
+ * @deprecated use {@link io.quarkus.hibernate.orm.panache.common.ProjectedFieldName} instead.
  */
 @Retention(RetentionPolicy.RUNTIME)
 @Target(ElementType.PARAMETER)
+@Deprecated(forRemoval = true, since = "2.1.0.Final")
 public @interface ProjectedFieldName {
     String value();
 }
diff --git a/...mon/runtime/src/main/java/io/quarkus/hibernate/orm/panache/common/ProjectedFieldName.java b/...mon/runtime/src/main/java/io/quarkus/hibernate/orm/panache/common/ProjectedFieldName.java
@@ -0,0 +1,17 @@
+package io.quarkus.hibernate.orm.panache.common;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Define a field's path for the SELECT statement when using a projection DTO.
+ * It supports the "dot" notation for fields in referenced entities:
+ * the name is composed of the property name for the relationship, followed by a dot ("."), followed by the name of the field.
+ */
+@Retention(RetentionPolicy.RUNTIME)
+@Target(ElementType.PARAMETER)
+public @interface ProjectedFieldName {
+    String value();
+}
diff --git a/...src/main/java/io/quarkus/hibernate/orm/panache/common/runtime/CommonPanacheQueryImpl.java b/...src/main/java/io/quarkus/hibernate/orm/panache/common/runtime/CommonPanacheQueryImpl.java
@@ -19,7 +19,7 @@
 import org.hibernate.Session;
 import org.hibernate.engine.spi.RowSelection;
 
-import io.quarkus.hibernate.orm.panache.ProjectedFieldName;
+import io.quarkus.hibernate.orm.panache.common.ProjectedFieldName;
 import io.quarkus.panache.common.Page;
 import io.quarkus.panache.common.Range;
 import io.quarkus.panache.common.exception.PanacheQueryException;
@@ -95,13 +95,20 @@ public <T> CommonPanacheQueryImpl<T> project(Class<T> type) {
             String parameterName;
             if (parameter.isAnnotationPresent(ProjectedFieldName.class)) {
                 final String name = parameter.getAnnotation(ProjectedFieldName.class).value();
-                if (name.isEmpty())
+                if (name.isEmpty()) {
                     throw new PanacheQueryException("The annotation ProjectedFieldName must have a non-empty value.");
+                }
+                parameterName = name;
+            } else if (parameter.isAnnotationPresent(io.quarkus.hibernate.orm.panache.ProjectedFieldName.class)) {
+                final String name = parameter.getAnnotation(io.quarkus.hibernate.orm.panache.ProjectedFieldName.class).value();
+                if (name.isEmpty()) {
+                    throw new PanacheQueryException("The annotation ProjectedFieldName must have a non-empty value.");
+                }
                 parameterName = name;
             } else if (!parameter.isNamePresent()) {
                 throw new PanacheQueryException(
                         "Your application must be built with parameter names, this should be the default if" +
-                                " using Quarkus artifacts. Check the maven or gradle compiler configuration to include '-parameters'.");
+                                " using Quarkus project generation. Check the Maven or Gradle compiler configuration to include '-parameters'.");
             } else {
                 parameterName = parameter.getName();
             }

diff --git a/integration-tests/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/Cat.java b/integration-tests/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/Cat.java
@@ -8,10 +8,13 @@
 @Entity
 public class Cat extends PanacheEntity {
 
+    String name;
+
     @ManyToOne
     CatOwner owner;
 
-    public Cat(CatOwner owner) {
+    public Cat(String name, CatOwner owner) {
+        this.name = name;
         this.owner = owner;
     }
 

diff --git a/integration-tests/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/CatDto.java b/integration-tests/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/CatDto.java
@@ -0,0 +1,18 @@
+package io.quarkus.it.panache;
+
+import io.quarkus.hibernate.orm.panache.common.ProjectedFieldName;
+import io.quarkus.runtime.annotations.RegisterForReflection;
+
+@RegisterForReflection
+public class CatDto {
+
+    public String name;
+
+    public String ownerName;
+
+    public CatDto(String name, @ProjectedFieldName("owner.name") String ownerName) {
+        this.name = name;
+        this.ownerName = ownerName;
+    }
+
+}
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
@@ -1147,6 +1147,17 @@ public String testProjection() {
         Assertions.assertEquals("stef", dogDto.ownerName);
         owner.delete();
 
+        CatOwner catOwner = new CatOwner("Julie");
+        catOwner.persist();
+        Cat bubulle = new Cat("Bubulle", catOwner);
+        bubulle.persist();
+
+        CatDto catDto = Cat.findAll().project(CatDto.class).firstResult();
+        Assertions.assertEquals("Julie", catDto.ownerName);
+
+        Cat.deleteAll();
+        CatOwner.deleteAll();
+
         return "OK";
     }
 
@@ -1317,9 +1328,9 @@ public String testBug7721() {
     public String testBug8254() {
         CatOwner owner = new CatOwner("8254");
         owner.persist();
-        new Cat(owner).persist();
-        new Cat(owner).persist();
-        new Cat(owner).persist();
+        new Cat("Cat 1", owner).persist();
+        new Cat("Cat 2", owner).persist();
+        new Cat("Cat 3", owner).persist();
 
         // This used to fail with an invalid query "SELECT COUNT(*) SELECT DISTINCT cat.owner FROM Cat cat WHERE cat.owner = ?1"
         // Should now result in a valid query "SELECT COUNT(DISTINCT cat.owner) FROM Cat cat WHERE cat.owner = ?1"
@@ -1338,6 +1349,9 @@ public String testBug8254() {
         assertEquals(3L, Cat.find("owner", owner).count());
         assertEquals(1L, CatOwner.find("name = ?1", "8254").count());
 
+        Cat.deleteAll();
+        CatOwner.deleteAll();
+
         return "OK";
     }