Merge pull request quarkusio#31846 from mkouba/hr-panache-classlevel-…

…binding-validation

Hibernate reactive panache - validation of interceptor bindings

extensions/panache/hibernate-reactive-panache-common/deployment/src/main/java/io/quarkus/hibernate/reactive/panache/common/deployment/PanacheJpaCommonResourceProcessor.java

@@ -8,7 +8,6 @@
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
-import java.util.stream.Collectors;
 
 import jakarta.annotation.Priority;
 import jakarta.interceptor.Interceptor;
@@ -22,6 +21,7 @@
 import org.jboss.jandex.DotName;
 import org.jboss.jandex.MethodInfo;
 import org.jboss.jandex.Type;
+import org.jboss.logging.Logger;
 
 import io.quarkus.arc.deployment.AdditionalBeanBuildItem;
 import io.quarkus.arc.deployment.AnnotationsTransformerBuildItem;
@@ -59,6 +59,8 @@
 @BuildSteps(onlyIf = HibernateOrmEnabled.class)
 public final class PanacheJpaCommonResourceProcessor {
 
+    private static final Logger LOG = Logger.getLogger(PanacheJpaCommonResourceProcessor.class);
+
     private static final DotName DOTNAME_NAMED_QUERY = DotName.createSimple(NamedQuery.class.getName());
     private static final DotName DOTNAME_NAMED_QUERIES = DotName.createSimple(NamedQueries.class.getName());
     private static final String TEST_REACTIVE_TRANSACTION = "io.quarkus.test.TestReactiveTransaction";
@@ -114,15 +116,7 @@ void validateInterceptedMethods(ValidationPhaseBuildItem validationPhase,
                     // Method returns Uni - no need to iterate over the bindings
                     continue;
                 }
-                if (Annotations.containsAny(e.getValue(), bindings)) {
-                    errors.produce(new ValidationErrorBuildItem(
-                            new IllegalStateException(
-                                    "A method annotated with "
-                                            + bindings.stream().map(b -> "@" + b.withoutPackagePrefix())
-                                                    .collect(Collectors.toList())
-                                            + " must return Uni: "
-                                            + e.getKey() + " declared on " + e.getKey().declaringClass())));
-                }
+                validateBindings(bindings, e, errors);
             }
         }
     }
@@ -247,4 +241,26 @@ private void lookupNamedQueries(CombinedIndexBuildItem index, DotName name, Set<
             }
         }
     }
+
+    private void validateBindings(List<DotName> bindings, Entry<MethodInfo, Set<AnnotationInstance>> entry,
+            BuildProducer<ValidationErrorBuildItem> errors) {
+        for (DotName binding : bindings) {
+            for (AnnotationInstance annotation : entry.getValue()) {
+                if (annotation.name().equals(binding)) {
+                    if (annotation.target().kind() == Kind.METHOD) {
+                        errors.produce(new ValidationErrorBuildItem(
+                                new IllegalStateException(
+                                        "A method annotated with @"
+                                                + binding.withoutPackagePrefix()
+                                                + " must return io.smallrye.mutiny.Uni: "
+                                                + entry.getKey() + " declared on " + entry.getKey().declaringClass())));
+                    } else {
+                        LOG.debugf("Class-level binding %s will be ignored for method %s() declared on %s", binding,
+                                entry.getKey().name(), entry.getKey().declaringClass());
+                    }
+                    return;
+                }
+            }
+        }
+    }
 }

extensions/panache/hibernate-reactive-panache-common/runtime/src/main/java/io/quarkus/hibernate/reactive/panache/common/WithSession.java

@@ -16,8 +16,8 @@
  * triggered, then this session is reused. Otherwise, a new session is opened and eventually closed when the
  * {@link io.smallrye.mutiny.Uni} completes.
  * <p>
- * A method annotated with this annotation must return either {@link io.smallrye.mutiny.Uni}. If declared on a class then all
- * methods that are intercepted must return {@link io.smallrye.mutiny.Uni}.
+ * A method annotated with this annotation must return {@link io.smallrye.mutiny.Uni}. If declared on a class then all methods
+ * that return {@link io.smallrye.mutiny.Uni} are considered; all other methods are ignored.
  */
 @Inherited
 @InterceptorBinding

extensions/panache/hibernate-reactive-panache-common/runtime/src/main/java/io/quarkus/hibernate/reactive/panache/common/WithSessionOnDemand.java

@@ -10,13 +10,14 @@
 
 /**
  * Instructs Panache to trigger the {@link io.smallrye.mutiny.Uni} returned from the intercepted method within a scope of a
- * reactive {@link org.hibernate.reactive.mutiny.Mutiny.Session} (if needed). If a reactive session exists when the
- * {@link io.smallrye.mutiny.Uni} returned from the annotated method is triggered, then this session is reused. Otherwise, a new
- * session is opened <b>when needed</b> and eventually closed when the
+ * reactive {@link org.hibernate.reactive.mutiny.Mutiny.Session} (if needed).
+ * <p>
+ * If a reactive session exists when the {@link io.smallrye.mutiny.Uni} returned from the annotated method is triggered, then
+ * this session is reused. Otherwise, a new session is opened <b>when needed</b> and eventually closed when the
  * {@link io.smallrye.mutiny.Uni} completes.
  * <p>
- * A method annotated with this annotation must return {@link io.smallrye.mutiny.Uni}. If declared on a class then all methods
- * that are intercepted must return {@link io.smallrye.mutiny.Uni}.
+ * * A method annotated with this annotation must return {@link io.smallrye.mutiny.Uni}. If declared on a class then all methods
+ * that return {@link io.smallrye.mutiny.Uni} are considered; all other methods are ignored.
  */
 @Inherited
 @InterceptorBinding

extensions/panache/hibernate-reactive-panache-common/runtime/src/main/java/io/quarkus/hibernate/reactive/panache/common/WithTransaction.java

@@ -10,13 +10,14 @@
 
 /**
  * Instructs Panache to trigger the {@link io.smallrye.mutiny.Uni} returned from the intercepted method within a scope of a
- * reactive {@link org.hibernate.reactive.mutiny.Mutiny.Transaction}. If a reactive session exists when the
- * {@link io.smallrye.mutiny.Uni} returned from the annotated method is triggered, then this session is reused. Otherwise, a new
- * session is opened and eventually closed when the
- * {@link io.smallrye.mutiny.Uni} completes.
+ * reactive {@link org.hibernate.reactive.mutiny.Mutiny.Transaction}.
+ * <p>
+ * If a reactive session exists when the {@link io.smallrye.mutiny.Uni} returned from the annotated method is triggered, then
+ * this session is reused. Otherwise, a new
+ * session is opened and eventually closed when the {@link io.smallrye.mutiny.Uni} completes.
  * <p>
  * A method annotated with this annotation must return {@link io.smallrye.mutiny.Uni}. If declared on a class then all methods
- * that are intercepted must return {@link io.smallrye.mutiny.Uni}.
+ * that return {@link io.smallrye.mutiny.Uni} are considered; all other methods are ignored.
  *
  * @see org.hibernate.reactive.mutiny.Mutiny.SessionFactory#withTransaction(java.util.function.Function)
  */

extensions/panache/hibernate-reactive-panache-common/runtime/src/main/java/io/quarkus/hibernate/reactive/panache/common/runtime/AbstractUniInterceptor.java

@@ -15,4 +15,8 @@ protected <T> Uni<T> proceedUni(InvocationContext context) {
         }
     }
 
+    protected boolean isUniReturnType(InvocationContext context) {
+        return context.getMethod().getReturnType().equals(Uni.class);
+    }
+
 }

extensions/panache/hibernate-reactive-panache-common/runtime/src/main/java/io/quarkus/hibernate/reactive/panache/common/runtime/WithSessionInterceptor.java

@@ -14,9 +14,12 @@ public class WithSessionInterceptor extends AbstractUniInterceptor {
 
     @AroundInvoke
     public Object intercept(InvocationContext context) throws Exception {
-        // Note that intercepted methods annotated with @WithSession are validated at build time
-        // The build fails if a method does not return Uni
-        return SessionOperations.withSession(s -> proceedUni(context));
+        // Bindings are validated at build time - method-level binding declared on a method that does not return Uni results in a build failure
+        // However, a class-level binding implies that methods that do not return Uni are just a no-op
+        if (isUniReturnType(context)) {
+            return SessionOperations.withSession(s -> proceedUni(context));
+        }
+        return context.proceed();
     }
 
 }

extensions/panache/hibernate-reactive-panache-common/runtime/src/main/java/io/quarkus/hibernate/reactive/panache/common/runtime/WithSessionOnDemandInterceptor.java

@@ -14,9 +14,12 @@ public class WithSessionOnDemandInterceptor extends AbstractUniInterceptor {
 
     @AroundInvoke
     public Object intercept(InvocationContext context) throws Exception {
-        // Note that intercepted methods annotated with @WithSessionOnDemand are validated at build time
-        // The build fails if a method does not return Uni
-        return SessionOperations.withSessionOnDemand(() -> proceedUni(context));
+        // Bindings are validated at build time - method-level binding declared on a method that does not return Uni results in a build failure
+        // However, a class-level binding implies that methods that do not return Uni are just a no-op
+        if (isUniReturnType(context)) {
+            return SessionOperations.withSessionOnDemand(() -> proceedUni(context));
+        }
+        return context.proceed();
     }
 
 }

extensions/panache/hibernate-reactive-panache-common/runtime/src/main/java/io/quarkus/hibernate/reactive/panache/common/runtime/WithTransactionInterceptor.java

@@ -14,9 +14,12 @@ public class WithTransactionInterceptor extends AbstractUniInterceptor {
 
     @AroundInvoke
     public Object intercept(InvocationContext context) throws Exception {
-        // Note that intercepted methods annotated with @WithTransaction are validated at build time
-        // The build fails if the method does not return Uni
-        return SessionOperations.withTransaction(() -> proceedUni(context));
+        // Bindings are validated at build time - method-level binding declared on a method that does not return Uni results in a build failure
+        // However, a class-level binding implies that methods that do not return Uni are just a no-op
+        if (isUniReturnType(context)) {
+            return SessionOperations.withTransaction(() -> proceedUni(context));
+        }
+        return context.proceed();
     }
 
 }

extensions/panache/hibernate-reactive-panache/deployment/src/test/java/io/quarkus/hibernate/reactive/panache/test/WithTransactionClassLevelValidationTest.java

@@ -0,0 +1,41 @@
+package io.quarkus.hibernate.reactive.panache.test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.inject.Inject;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.arc.Unremovable;
+import io.quarkus.hibernate.reactive.panache.common.WithTransaction;
+import io.quarkus.test.QuarkusUnitTest;
+
+public class WithTransactionClassLevelValidationTest {
+
+    @RegisterExtension
+    static QuarkusUnitTest runner = new QuarkusUnitTest()
+            .withApplicationRoot(root -> root
+                    .addClasses(Bean.class));
+
+    @Inject
+    Bean bean;
+
+    @Test
+    public void testBindingIgnored() {
+        assertEquals("ok", bean.ping());
+    }
+
+    @WithTransaction
+    @Unremovable
+    @ApplicationScoped
+    static class Bean {
+
+        // this method is just ignored by the interceptor
+        String ping() {
+            return "ok";
+        }
+
+    }
+}

extensions/panache/hibernate-reactive-panache/deployment/src/test/java/io/quarkus/hibernate/reactive/panache/test/WithTransactionMethodLevelValidationTest.java

@@ -0,0 +1,36 @@
+package io.quarkus.hibernate.reactive.panache.test;
+
+import static org.junit.jupiter.api.Assertions.fail;
+
+import jakarta.enterprise.context.ApplicationScoped;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.arc.Unremovable;
+import io.quarkus.hibernate.reactive.panache.common.WithTransaction;
+import io.quarkus.test.QuarkusUnitTest;
+
+public class WithTransactionMethodLevelValidationTest {
+
+    @RegisterExtension
+    static QuarkusUnitTest runner = new QuarkusUnitTest()
+            .setExpectedException(IllegalStateException.class)
+            .withApplicationRoot(root -> root
+                    .addClasses(Bean.class));
+
+    @Test
+    public void testValidationFailed() {
+        fail();
+    }
+
+    @Unremovable
+    @ApplicationScoped
+    static class Bean {
+
+        @WithTransaction
+        void ping() {
+        }
+
+    }
+}