Fix decorator abstract methods validation to match CDI specification

independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Decorators.java

@@ -14,6 +14,7 @@
 import org.jboss.jandex.ClassInfo;
 import org.jboss.jandex.DotName;
 import org.jboss.jandex.FieldInfo;
+import org.jboss.jandex.IndexView;
 import org.jboss.jandex.MethodInfo;
 import org.jboss.jandex.Type;
 import org.jboss.logging.Logger;
@@ -117,19 +118,36 @@ static DecoratorInfo createDecorator(ClassInfo decoratorClass, BeanDeployment be
         }
 
         if (Modifier.isAbstract(decoratorClass.flags())) {
-            // TODO this check is not precise: we check that decorators do not declare _any_ abstract methods,
-            //  but the spec says that decorators may not declare abstract methods that do not belong
-            //  to a decorated type
-            //  also, we only check methods declared on the decorator class itself, not inherited methods
-            List<MethodInfo> abstractMethods = new ArrayList<>();
-            for (MethodInfo method : decoratorClass.methods()) {
-                if (Modifier.isAbstract(method.flags())) {
-                    abstractMethods.add(method);
+            // Check all abstract methods in the hierarchy; reject those not belonging to a decorated type
+            List<MethodInfo> invalidAbstractMethods = new ArrayList<>();
+            ClassInfo currentClass = decoratorClass;
+            IndexView index = beanDeployment.getBeanArchiveIndex();
+            Set<DotName> visitedClasses = new HashSet<>();
+            while (currentClass != null && !visitedClasses.contains(currentClass.name())) {
+                visitedClasses.add(currentClass.name());
+                for (MethodInfo method : currentClass.methods()) {
+                    if (Modifier.isAbstract(method.flags())) {
+                        boolean belongs = false;
+                        Set<DotName> visitedTypes = new HashSet<>();
+                        for (Type decoratedType : decoratedTypes) {
+                            if (methodExistsInHierarchy(method, decoratedType, index, visitedTypes)) {
+                                belongs = true;
+                                break;
+                            }
+                        }
+                        if (!belongs) {
+                            invalidAbstractMethods.add(method);
+                        }
+                    }
                 }
+                DotName superClass = currentClass.superName();
+                currentClass = superClass != null && !superClass.equals(DotNames.OBJECT)
+                        ? getClassByName(index, superClass)
+                        : null;
             }
-            if (!abstractMethods.isEmpty()) {
+            if (!invalidAbstractMethods.isEmpty()) {
                 throw new DefinitionException("An abstract decorator " + decoratorClass
-                        + " declares abstract methods: " + abstractMethods);
+                        + " declares abstract methods that do not belong to a decorated type: " + invalidAbstractMethods);
             }
         }
 
@@ -139,9 +157,51 @@ static DecoratorInfo createDecorator(ClassInfo decoratorClass, BeanDeployment be
                 decoratedTypes, injections, priority);
     }
 
+    private static boolean methodExistsInHierarchy(MethodInfo method, Type type, IndexView index, Set<DotName> visited) {
+        DotName typeName = type.name();
+        if (visited.contains(typeName)) {
+            return false;
+        }
+        visited.add(typeName);
+
+        ClassInfo typeClass = index.getClassByName(typeName);
+        if (typeClass == null) {
+            return false;
+        }
+
+        // Check direct methods
+        for (MethodInfo typeMethod : typeClass.methods()) {
+            if (method.name().equals(typeMethod.name())
+                    && method.parametersCount() == typeMethod.parametersCount()
+                    && allParamsMatch(method, typeMethod)) {
+                return true;
+            }
+        }
+
+        // Recurse on superinterfaces
+        for (Type superInterface : typeClass.interfaceTypes()) {
+            if (methodExistsInHierarchy(method, superInterface, index, visited)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    private static boolean allParamsMatch(MethodInfo m1, MethodInfo m2) {
+        for (int cont = 0; cont < m1.parametersCount(); cont++) {
+            if (!m1.parameterType(cont).name().equals(m2.parameterType(cont).name())) {
+                return false;
+            }
+        }
+        return true;
+    }
+
     private static void checkDecoratorFieldsAndMethods(ClassInfo decoratorClass, BeanDeployment beanDeployment) {
         ClassInfo aClass = decoratorClass;
-        while (aClass != null) {
+        Set<DotName> visited = new HashSet<>();
+        while (aClass != null && !visited.contains(aClass.name())) {
+            visited.add(aClass.name());
             for (MethodInfo method : aClass.methods()) {
                 if (beanDeployment.hasAnnotation(method, DotNames.PRODUCES)) {
                     throw new DefinitionException("Decorator declares a producer method: " + decoratorClass);
@@ -171,4 +231,4 @@ private static void checkDecoratorFieldsAndMethods(ClassInfo decoratorClass, Bea
         }
     }
 
-}
+}

independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/abstractimpl/DecoratorAbstractMethodBelongsToDecoratedTypeTest.java

@@ -0,0 +1,79 @@
+package io.quarkus.arc.test.decorators.abstractimpl;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import jakarta.annotation.Priority;
+import jakarta.decorator.Decorator;
+import jakarta.decorator.Delegate;
+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.Arc;
+import io.quarkus.arc.test.ArcTestContainer;
+
+/**
+ * Test that a decorator can declare abstract methods that belong to a decorated type.
+ * This is allowed by the CDI specification. Abstract methods in a decorator
+ * are simply not decorated - they delegate directly to the underlying bean.
+ *
+ * @see <a href="https://github.com/quarkusio/quarkus/issues/51196">Issue #51196</a>
+ */
+public class DecoratorAbstractMethodBelongsToDecoratedTypeTest {
+
+    @RegisterExtension
+    public ArcTestContainer container = new ArcTestContainer(
+            Service.class,
+            ServiceImpl.class,
+            ServiceDecorator.class);
+
+    @Test
+    public void testDecoratorWithAbstractMethodBelongingToDecoratedType() {
+        ServiceImpl service = Arc.container().instance(ServiceImpl.class).get();
+        // process() is decorated
+        assertEquals("decorated: HELLO", service.process("hello"));
+        // getId() is abstract in decorator, so it delegates directly (not decorated)
+        assertEquals("test", service.getId());
+    }
+
+    interface Service {
+        String process(String value);
+
+        String getId();
+    }
+
+    @ApplicationScoped
+    static class ServiceImpl implements Service {
+
+        @Override
+        public String process(String value) {
+            return value.toUpperCase();
+        }
+
+        @Override
+        public String getId() {
+            return "test";
+        }
+    }
+
+    @Priority(1)
+    @Decorator
+    static abstract class ServiceDecorator implements Service {
+
+        @Inject
+        @Delegate
+        Service delegate;
+
+        @Override
+        public String process(String value) {
+            return "decorated: " + delegate.process(value);
+        }
+
+        // This abstract method belongs to Service (decorated type)
+        // and should be allowed - it will delegate directly without decoration
+        @Override
+        public abstract String getId();
+    }
+}

independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/abstractimpl/DecoratorAbstractMethodFromSuperinterfaceTest.java

@@ -0,0 +1,81 @@
+package io.quarkus.arc.test.decorators.abstractimpl;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import jakarta.annotation.Priority;
+import jakarta.decorator.Decorator;
+import jakarta.decorator.Delegate;
+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.Arc;
+import io.quarkus.arc.test.ArcTestContainer;
+
+/**
+ * Test that a decorator can declare abstract methods that belong to a superinterface
+ * of a decorated type. The validation should check the entire interface hierarchy.
+ *
+ * @see <a href="https://github.com/quarkusio/quarkus/issues/51196">Issue #51196</a>
+ */
+public class DecoratorAbstractMethodFromSuperinterfaceTest {
+
+    @RegisterExtension
+    public ArcTestContainer container = new ArcTestContainer(
+            BaseService.class,
+            ExtendedService.class,
+            ExtendedServiceImpl.class,
+            ExtendedServiceDecorator.class);
+
+    @Test
+    public void testDecoratorWithAbstractMethodFromSuperinterface() {
+        ExtendedServiceImpl service = Arc.container().instance(ExtendedServiceImpl.class).get();
+        // extendedProcess() is decorated
+        assertEquals("decorated extended: HELLO", service.extendedProcess("hello"));
+        // baseProcess() is abstract in decorator (from superinterface), delegates directly
+        assertEquals("test", service.baseProcess("test"));
+    }
+
+    interface BaseService {
+        String baseProcess(String value);
+    }
+
+    interface ExtendedService extends BaseService {
+        String extendedProcess(String value);
+    }
+
+    @ApplicationScoped
+    static class ExtendedServiceImpl implements ExtendedService {
+
+        @Override
+        public String baseProcess(String value) {
+            return value;
+        }
+
+        @Override
+        public String extendedProcess(String value) {
+            return value.toUpperCase();
+        }
+    }
+
+    @Priority(1)
+    @Decorator
+    static abstract class ExtendedServiceDecorator implements ExtendedService {
+
+        @Inject
+        @Delegate
+        ExtendedService delegate;
+
+        @Override
+        public String extendedProcess(String value) {
+            return "decorated extended: " + delegate.extendedProcess(value);
+        }
+
+        // This abstract method belongs to BaseService (superinterface of ExtendedService)
+        // and should be allowed - it will delegate directly
+        @Override
+        public abstract String baseProcess(String value);
+    }
+}

independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/abstractimpl/DecoratorAbstractMethodInheritedFromSuperclassTest.java

@@ -0,0 +1,85 @@
+package io.quarkus.arc.test.decorators.abstractimpl;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import jakarta.annotation.Priority;
+import jakarta.decorator.Decorator;
+import jakarta.decorator.Delegate;
+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.Arc;
+import io.quarkus.arc.test.ArcTestContainer;
+
+/**
+ * Test that a decorator can inherit abstract methods from a superclass,
+ * as long as those methods belong to a decorated type.
+ *
+ * @see <a href="https://github.com/quarkusio/quarkus/issues/51196">Issue #51196</a>
+ */
+public class DecoratorAbstractMethodInheritedFromSuperclassTest {
+
+    @RegisterExtension
+    public ArcTestContainer container = new ArcTestContainer(
+            Service.class,
+            ServiceImpl.class,
+            AbstractBaseDecorator.class,
+            ConcreteDecorator.class);
+
+    @Test
+    public void testDecoratorWithAbstractMethodInheritedFromSuperclass() {
+        ServiceImpl service = Arc.container().instance(ServiceImpl.class).get();
+        // process() is decorated
+        assertEquals("decorated: HELLO", service.process("hello"));
+        // getId() is abstract (inherited from superclass), delegates directly
+        assertEquals("test", service.getId());
+    }
+
+    interface Service {
+        String process(String value);
+
+        String getId();
+    }
+
+    @ApplicationScoped
+    static class ServiceImpl implements Service {
+
+        @Override
+        public String process(String value) {
+            return value.toUpperCase();
+        }
+
+        @Override
+        public String getId() {
+            return "test";
+        }
+    }
+
+    // Abstract base class for decorators - not a decorator itself
+    static abstract class AbstractBaseDecorator implements Service {
+        // This abstract method will be inherited by the actual decorator
+        // It belongs to Service, so it should be allowed
+        @Override
+        public abstract String getId();
+    }
+
+    @Priority(1)
+    @Decorator
+    static abstract class ConcreteDecorator extends AbstractBaseDecorator {
+
+        @Inject
+        @Delegate
+        Service delegate;
+
+        @Override
+        public String process(String value) {
+            return "decorated: " + delegate.process(value);
+        }
+
+        // getId() is inherited from AbstractBaseDecorator as abstract
+        // and should be allowed since it belongs to Service
+    }
+}