From 6c58e915233fb57f5e7083878d693ff8ad267920 Mon Sep 17 00:00:00 2001 From: Fabricio Silva Lima Date: Sun, 7 Dec 2025 10:03:33 -0400 Subject: [PATCH] Fix decorator abstract methods validation to match CDI specification The check for abstract methods in decorators was too restrictive. According to the CDI specification, decorators may declare abstract methods that belong to decorated types. The check should only reject abstract methods that do not belong to any decorated type. Changes: - Modified Decorators.createDecorator() to check if abstract methods belong to decorated types before rejecting them - Added methodExistsInHierarchy() to verify if a method exists in the decorated type hierarchy (including superinterfaces) - Added allParamsMatch() helper to compare method signatures - Now checks abstract methods from decorator class and inherited from superclasses (as mentioned in the TODO comment) - Added comprehensive test cases for the fix This fixes the issue where decorators with abstract methods that belong to the decorated interface were incorrectly rejected. Note: This fix does not handle generic interfaces with type parameters (e.g., GenericInterface with T resolved to String). This is a known limitation that could be addressed in a follow-up PR. Fixes: #51196 --- .../io/quarkus/arc/processor/Decorators.java | 84 +++++++++++++++--- ...tractMethodBelongsToDecoratedTypeTest.java | 79 +++++++++++++++++ ...rAbstractMethodFromSuperinterfaceTest.java | 81 ++++++++++++++++++ ...ractMethodInheritedFromSuperclassTest.java | 85 +++++++++++++++++++ ...MethodNotBelongingToDecoratedTypeTest.java | 72 ++++++++++++++++ 5 files changed, 389 insertions(+), 12 deletions(-) create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/abstractimpl/DecoratorAbstractMethodBelongsToDecoratedTypeTest.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/abstractimpl/DecoratorAbstractMethodFromSuperinterfaceTest.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/abstractimpl/DecoratorAbstractMethodInheritedFromSuperclassTest.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/abstractimpl/DecoratorAbstractMethodNotBelongingToDecoratedTypeTest.java diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Decorators.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Decorators.java index 5dd691f766ad5..092cae09966be 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Decorators.java +++ b/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 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 invalidAbstractMethods = new ArrayList<>(); + ClassInfo currentClass = decoratorClass; + IndexView index = beanDeployment.getBeanArchiveIndex(); + Set 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 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 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 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 } } -} +} \ No newline at end of file diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/abstractimpl/DecoratorAbstractMethodBelongsToDecoratedTypeTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/abstractimpl/DecoratorAbstractMethodBelongsToDecoratedTypeTest.java new file mode 100644 index 0000000000000..30a5968385f39 --- /dev/null +++ b/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 Issue #51196 + */ +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(); + } +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/abstractimpl/DecoratorAbstractMethodFromSuperinterfaceTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/abstractimpl/DecoratorAbstractMethodFromSuperinterfaceTest.java new file mode 100644 index 0000000000000..dc0b1bb3e60aa --- /dev/null +++ b/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 Issue #51196 + */ +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); + } +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/abstractimpl/DecoratorAbstractMethodInheritedFromSuperclassTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/abstractimpl/DecoratorAbstractMethodInheritedFromSuperclassTest.java new file mode 100644 index 0000000000000..ff7d7ccd294e9 --- /dev/null +++ b/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 Issue #51196 + */ +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 + } +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/abstractimpl/DecoratorAbstractMethodNotBelongingToDecoratedTypeTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/abstractimpl/DecoratorAbstractMethodNotBelongingToDecoratedTypeTest.java new file mode 100644 index 0000000000000..450cddb6459a0 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/abstractimpl/DecoratorAbstractMethodNotBelongingToDecoratedTypeTest.java @@ -0,0 +1,72 @@ +package io.quarkus.arc.test.decorators.abstractimpl; + +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import jakarta.annotation.Priority; +import jakarta.decorator.Decorator; +import jakarta.decorator.Delegate; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.enterprise.inject.spi.DefinitionException; +import jakarta.inject.Inject; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.test.ArcTestContainer; + +/** + * Test that a decorator cannot declare abstract methods that do not belong + * to any decorated type. This should result in a DefinitionException. + * + * @see Issue #51196 + */ +public class DecoratorAbstractMethodNotBelongingToDecoratedTypeTest { + + @RegisterExtension + public ArcTestContainer container = ArcTestContainer.builder() + .beanClasses(Service.class, ServiceImpl.class, InvalidDecorator.class) + .shouldFail() + .build(); + + @Test + public void testDecoratorWithAbstractMethodNotBelongingToDecoratedType() { + Throwable error = container.getFailure(); + assertNotNull(error); + assertInstanceOf(DefinitionException.class, error); + assertTrue(error.getMessage().contains("abstract methods that do not belong to a decorated type")); + assertTrue(error.getMessage().contains("unrelatedMethod")); + } + + interface Service { + String process(String value); + } + + @ApplicationScoped + static class ServiceImpl implements Service { + + @Override + public String process(String value) { + return value.toUpperCase(); + } + } + + @Priority(1) + @Decorator + static abstract class InvalidDecorator implements Service { + + @Inject + @Delegate + Service delegate; + + @Override + public String process(String value) { + return "decorated: " + delegate.process(value); + } + + // This abstract method does NOT belong to Service (decorated type) + // and should cause a DefinitionException + public abstract void unrelatedMethod(); + } +}