Merge pull request quarkusio#18558 from mkouba/issue-18538

ArC - fix ordering of multiple decorators with the same priority

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

@@ -383,12 +383,15 @@ List<DecoratorInfo> getBoundDecorators() {
                 }
             }
         }
-        // Sort by priority (highest goes first) and by bean class
+        // Sort by priority (highest goes first) and by bean class (reversed lexicographic-order)
         // Highest priority first because the decorators are instantiated in the reverse order, 
         // i.e. when the subclass constructor is generated the delegate subclass of the first decorator 
         // (lower priority) needs a reference to the next decorator in the chain (higher priority)
+        // Note that this set must be always reversed compared to the result coming from the BeanInfo#getNextDecorators(DecoratorInfo)
         Collections.sort(bound,
-                Comparator.comparing(DecoratorInfo::getPriority).reversed().thenComparing(DecoratorInfo::getBeanClass));
+                Comparator.comparing(DecoratorInfo::getPriority)
+                        .thenComparing(DecoratorInfo::getBeanClass)
+                        .reversed());
         return bound;
     }
 

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

@@ -421,7 +421,8 @@ private void processDecorator(DecoratorInfo decorator, BeanInfo bean, Type provi
         ClassCreator delegateSubclass = delegateSubclassBuilder.build();
 
         Map<MethodDescriptor, DecoratorInfo> nextDecorators = bean.getNextDecorators(decorator);
-        List<DecoratorInfo> decoratorParameters = new ArrayList<>(nextDecorators.values());
+        Collection<DecoratorInfo> nextDecoratorsValues = nextDecorators.values();
+        List<DecoratorInfo> decoratorParameters = new ArrayList<>(new HashSet<>(nextDecoratorsValues));
         Collections.sort(decoratorParameters);
         Set<MethodInfo> decoratedMethods = bean.getDecoratedMethods(decorator);
         Set<MethodDescriptor> decoratedMethodDescriptors = new HashSet<>(decoratedMethods.size());
@@ -432,7 +433,7 @@ private void processDecorator(DecoratorInfo decorator, BeanInfo bean, Type provi
         List<String> constructorParameterTypes = new ArrayList<>();
         // Fields and constructor
         FieldCreator subclassField = null;
-        if (decoratedMethods.size() != decoratorParameters.size()) {
+        if (decoratedMethods.size() != nextDecoratorsValues.size()) {
             subclassField = delegateSubclass.getFieldCreator("subclass", subclass.getClassName())
                     .setModifiers(ACC_PRIVATE | ACC_FINAL);
             constructorParameterTypes.add(subclass.getClassName());

independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/priority/AlphaConverterDecorator.java

@@ -1,6 +1,5 @@
 package io.quarkus.arc.test.decorators.priority;
 
-import io.quarkus.arc.test.decorators.priority.MultipleDecoratorsTest.Converter;
 import javax.annotation.Priority;
 import javax.decorator.Decorator;
 import javax.decorator.Delegate;
@@ -17,7 +16,7 @@ class AlphaConverterDecorator implements Converter<String> {
     @Override
     public String convert(String value) {
         // skip first char
-        return value.substring(1);
+        return delegate.convert(value).substring(1);
     }
 
 }

independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/priority/AlsoAlphaConverterDecorator.java

@@ -0,0 +1,22 @@
+package io.quarkus.arc.test.decorators.priority;
+
+import javax.annotation.Priority;
+import javax.decorator.Decorator;
+import javax.decorator.Delegate;
+import javax.inject.Inject;
+
+@Priority(20)
+@Decorator
+class AlsoAlphaConverterDecorator implements Converter<String> {
+
+    @Inject
+    @Delegate
+    Converter<String> delegate;
+
+    @Override
+    public String convert(String value) {
+        // skip first char
+        return delegate.convert(value).substring(1);
+    }
+
+}

independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/priority/BravoConverterDecorator.java

@@ -1,6 +1,5 @@
 package io.quarkus.arc.test.decorators.priority;
 
-import io.quarkus.arc.test.decorators.priority.MultipleDecoratorsTest.Converter;
 import javax.annotation.Priority;
 import javax.decorator.Decorator;
 import javax.decorator.Delegate;

independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/priority/Converter.java

@@ -0,0 +1,7 @@
+package io.quarkus.arc.test.decorators.priority;
+
+interface Converter<T> {
+
+    T convert(T value);
+
+}

independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/priority/MultipleDecoratorsSamePriorityTest.java

@@ -0,0 +1,33 @@
+package io.quarkus.arc.test.decorators.priority;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import io.quarkus.arc.Arc;
+import io.quarkus.arc.test.ArcTestContainer;
+import javax.enterprise.context.ApplicationScoped;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+public class MultipleDecoratorsSamePriorityTest {
+
+    @RegisterExtension
+    public ArcTestContainer container = new ArcTestContainer(Converter.class, ToUpperCaseConverter.class,
+            AlphaConverterDecorator.class, AlsoAlphaConverterDecorator.class);
+
+    @Test
+    public void testDecoration() {
+        ToUpperCaseConverter converter = Arc.container().instance(ToUpperCaseConverter.class).get();
+        assertEquals("J", converter.convert("hej"));
+    }
+
+    @ApplicationScoped
+    static class ToUpperCaseConverter implements Converter<String> {
+
+        @Override
+        public String convert(String value) {
+            return value.toUpperCase();
+        }
+
+    }
+
+}

independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/priority/MultipleDecoratorsTest.java

@@ -17,13 +17,7 @@ public class MultipleDecoratorsTest {
     @Test
     public void testDecoration() {
         ToUpperCaseConverter converter = Arc.container().instance(ToUpperCaseConverter.class).get();
-        assertEquals("je", converter.convert("hej"));
-    }
-
-    interface Converter<T> {
-
-        T convert(T value);
-
+        assertEquals("JE", converter.convert("hej"));
     }
 
     @ApplicationScoped