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(); + } +}