From fee6b2b2297e251b1bb8ef5df50083b88d05baa1 Mon Sep 17 00:00:00 2001 From: Martin Kouba Date: Thu, 4 Mar 2021 13:34:24 +0100 Subject: [PATCH] Dev UI - fix comparator for CDI observers --- .../deployment/devconsole/DevBeanInfo.java | 73 +++++++++------- .../devconsole/DevConsoleProcessor.java | 6 +- .../devconsole/DevObserverInfo.java | 84 ++++++++++++------- .../test/devconsole/DevObserverInfoTest.java | 49 +++++++++++ 4 files changed, 148 insertions(+), 64 deletions(-) create mode 100644 extensions/arc/deployment/src/test/java/io/quarkus/arc/test/devconsole/DevObserverInfoTest.java diff --git a/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/devconsole/DevBeanInfo.java b/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/devconsole/DevBeanInfo.java index 9650437659288..85809f19c0bbd 100644 --- a/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/devconsole/DevBeanInfo.java +++ b/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/devconsole/DevBeanInfo.java @@ -17,60 +17,73 @@ public class DevBeanInfo implements Comparable { - private final DevBeanKind kind; - private final boolean isApplicationBean; - private final Name providerType; - private final String memberName; - private final Set types; - private final Set qualifiers; - private final Name scope; - private final Name declaringClass; - - public DevBeanInfo(BeanInfo bean, CompletedApplicationClassPredicateBuildItem predicate) { - qualifiers = new HashSet<>(); + public static DevBeanInfo from(BeanInfo bean, CompletedApplicationClassPredicateBuildItem predicate) { + Set qualifiers = new HashSet<>(); for (AnnotationInstance qualifier : bean.getQualifiers()) { qualifiers.add(Name.from(qualifier)); } - scope = Name.from(bean.getScope().getDotName()); - types = new HashSet<>(); + Set types = new HashSet<>(); for (Type beanType : bean.getTypes()) { types.add(Name.from(beanType)); } - - providerType = Name.from(bean.getProviderType()); + Name scope = Name.from(bean.getScope().getDotName()); + Name providerType = Name.from(bean.getProviderType()); if (bean.getTarget().isPresent()) { AnnotationTarget target = bean.getTarget().get(); + DevBeanKind kind; + String memberName; + boolean isApplicationBean; + Name declaringClass; if (target.kind() == Kind.METHOD) { MethodInfo method = target.asMethod(); memberName = method.name(); - this.kind = DevBeanKind.METHOD; - this.isApplicationBean = predicate.test(bean.getDeclaringBean().getBeanClass()); - this.declaringClass = Name.from(bean.getDeclaringBean().getBeanClass()); + kind = DevBeanKind.METHOD; + isApplicationBean = predicate.test(bean.getDeclaringBean().getBeanClass()); + declaringClass = Name.from(bean.getDeclaringBean().getBeanClass()); } else if (target.kind() == Kind.FIELD) { FieldInfo field = target.asField(); - this.memberName = field.name(); - this.kind = DevBeanKind.FIELD; - this.isApplicationBean = predicate.test(bean.getDeclaringBean().getBeanClass()); - this.declaringClass = Name.from(bean.getDeclaringBean().getBeanClass()); + memberName = field.name(); + kind = DevBeanKind.FIELD; + isApplicationBean = predicate.test(bean.getDeclaringBean().getBeanClass()); + declaringClass = Name.from(bean.getDeclaringBean().getBeanClass()); } else if (target.kind() == Kind.CLASS) { ClassInfo clazz = target.asClass(); - this.kind = DevBeanKind.CLASS; - this.memberName = null; - this.isApplicationBean = predicate.test(clazz.name()); - this.declaringClass = null; + kind = DevBeanKind.CLASS; + memberName = null; + isApplicationBean = predicate.test(clazz.name()); + declaringClass = null; } else { throw new IllegalArgumentException("Invalid annotation target: " + target); } + return new DevBeanInfo(kind, isApplicationBean, providerType, memberName, types, qualifiers, scope, declaringClass); } else { // Synthetic bean - this.kind = DevBeanKind.SYNTHETIC; - this.isApplicationBean = false; - this.declaringClass = null; - this.memberName = null; + return new DevBeanInfo(DevBeanKind.SYNTHETIC, false, providerType, null, types, qualifiers, scope, null); } } + public DevBeanInfo(DevBeanKind kind, boolean isApplicationBean, Name providerType, String memberName, Set types, + Set qualifiers, Name scope, Name declaringClass) { + this.kind = kind; + this.isApplicationBean = isApplicationBean; + this.providerType = providerType; + this.memberName = memberName; + this.types = types; + this.qualifiers = qualifiers; + this.scope = scope; + this.declaringClass = declaringClass; + } + + private final DevBeanKind kind; + private final boolean isApplicationBean; + private final Name providerType; + private final String memberName; + private final Set types; + private final Set qualifiers; + private final Name scope; + private final Name declaringClass; + public DevBeanKind getKind() { return kind; } diff --git a/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/devconsole/DevConsoleProcessor.java b/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/devconsole/DevConsoleProcessor.java index 3801d05cbc3ee..f3235148f9b98 100644 --- a/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/devconsole/DevConsoleProcessor.java +++ b/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/devconsole/DevConsoleProcessor.java @@ -93,13 +93,13 @@ public DevConsoleTemplateInfoBuildItem collectBeanInfo(ValidationPhaseBuildItem BeanDeploymentValidator.ValidationContext validationContext = validationPhaseBuildItem.getContext(); DevBeanInfos beanInfos = new DevBeanInfos(); for (BeanInfo beanInfo : validationContext.beans()) { - beanInfos.addBean(new DevBeanInfo(beanInfo, predicate)); + beanInfos.addBean(DevBeanInfo.from(beanInfo, predicate)); } for (BeanInfo beanInfo : validationContext.removedBeans()) { - beanInfos.addRemovedBean(new DevBeanInfo(beanInfo, predicate)); + beanInfos.addRemovedBean(DevBeanInfo.from(beanInfo, predicate)); } for (ObserverInfo observerInfo : validationContext.get(BuildExtension.Key.OBSERVERS)) { - beanInfos.addObserver(new DevObserverInfo(observerInfo, predicate)); + beanInfos.addObserver(DevObserverInfo.from(observerInfo, predicate)); } beanInfos.sort(); return new DevConsoleTemplateInfoBuildItem("devBeanInfos", beanInfos); diff --git a/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/devconsole/DevObserverInfo.java b/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/devconsole/DevObserverInfo.java index ea1fe066055b6..d09374728e023 100644 --- a/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/devconsole/DevObserverInfo.java +++ b/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/devconsole/DevObserverInfo.java @@ -12,6 +12,24 @@ public class DevObserverInfo implements Comparable { + public static DevObserverInfo from(ObserverInfo observer, CompletedApplicationClassPredicateBuildItem predicate) { + List qualifiers; + if (observer.getQualifiers().isEmpty()) { + qualifiers = Collections.emptyList(); + } else { + qualifiers = observer.getQualifiers().stream().map(Name::from).collect(Collectors.toList()); + } + if (observer.getDeclaringBean() != null) { + return new DevObserverInfo(predicate.test(observer.getObserverMethod().declaringClass().name()), + Name.from(observer.getObserverMethod().declaringClass().name()), observer.getObserverMethod().name(), + Name.from(observer.getObservedType()), qualifiers, observer.getPriority(), observer.isAsync(), + observer.getReception(), observer.getTransactionPhase()); + } else { + return new DevObserverInfo(false, null, null, Name.from(observer.getObservedType()), qualifiers, + observer.getPriority(), observer.isAsync(), observer.getReception(), observer.getTransactionPhase()); + } + } + private final boolean isApplicationObserver; private final Name declaringClass; private final String methodName; @@ -22,27 +40,17 @@ public class DevObserverInfo implements Comparable { private final Reception reception; private final TransactionPhase transactionPhase; - public DevObserverInfo(ObserverInfo observer, CompletedApplicationClassPredicateBuildItem predicate) { - priority = observer.getPriority(); - reception = observer.getReception(); - transactionPhase = observer.getTransactionPhase(); - isAsync = observer.isAsync(); - observedType = Name.from(observer.getObservedType()); - - if (observer.getQualifiers().isEmpty()) { - qualifiers = Collections.emptyList(); - } else { - qualifiers = observer.getQualifiers().stream().map(Name::from).collect(Collectors.toList()); - } - if (observer.getDeclaringBean() != null) { - declaringClass = Name.from(observer.getObserverMethod().declaringClass().name()); - methodName = observer.getObserverMethod().name(); - isApplicationObserver = predicate.test(observer.getObserverMethod().declaringClass().name()); - } else { - declaringClass = null; - methodName = null; - isApplicationObserver = false; - } + public DevObserverInfo(boolean isApplicationObserver, Name declaringClass, String methodName, Name observedType, + List qualifiers, int priority, boolean isAsync, Reception reception, TransactionPhase transactionPhase) { + this.isApplicationObserver = isApplicationObserver; + this.declaringClass = declaringClass; + this.methodName = methodName; + this.observedType = observedType; + this.qualifiers = qualifiers; + this.priority = priority; + this.isAsync = isAsync; + this.reception = reception; + this.transactionPhase = transactionPhase; } public Name getDeclaringClass() { @@ -82,17 +90,31 @@ public boolean isApplicationObserver() { } @Override - public int compareTo(DevObserverInfo o) { - // Application beans should go first - if (isApplicationObserver == o.isApplicationObserver) { - if (declaringClass == null && o.declaringClass != null) { - return -1; - } else if (declaringClass != null && o.declaringClass == null) { - return 1; + public int compareTo(DevObserverInfo other) { + // Application observers should go first + int ret = 0; + if (isApplicationObserver != other.isApplicationObserver) { + ret = isApplicationObserver ? -1 : 1; + } else { + // Note that declaringClass and methodName are null for synthetic observers + if (declaringClass == null && other.declaringClass == null) { + // Synthetic observers + ret = observedType.compareTo(other.observedType); + } else { + // At this point we can be sure that at least one of the observers is not synthetic + if (declaringClass == null && other.declaringClass != null) { + ret = 1; + } else if (declaringClass != null && other.declaringClass == null) { + ret = -1; + } else { + ret = declaringClass.compareTo(other.declaringClass); + } + if (ret == 0) { + // Observers are not synthetic - method name must be present + ret = methodName.compareTo(other.methodName); + } } - int ret = declaringClass.compareTo(o.declaringClass); - return ret == 0 ? methodName.compareTo(o.methodName) : ret; } - return isApplicationObserver ? -1 : 1; + return ret; } } diff --git a/extensions/arc/deployment/src/test/java/io/quarkus/arc/test/devconsole/DevObserverInfoTest.java b/extensions/arc/deployment/src/test/java/io/quarkus/arc/test/devconsole/DevObserverInfoTest.java new file mode 100644 index 0000000000000..ca364db576e2f --- /dev/null +++ b/extensions/arc/deployment/src/test/java/io/quarkus/arc/test/devconsole/DevObserverInfoTest.java @@ -0,0 +1,49 @@ +package io.quarkus.arc.test.devconsole; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import javax.enterprise.event.Reception; +import javax.enterprise.event.TransactionPhase; + +import org.junit.jupiter.api.Test; + +import io.quarkus.arc.deployment.devconsole.DevObserverInfo; +import io.quarkus.arc.deployment.devconsole.Name; + +public class DevObserverInfoTest { + + @Test + public void testCompare() { + List observers = new ArrayList<>(); + // Synthetic non-app - should be last + observers.add(new DevObserverInfo(false, null, null, new Name("Delta"), + Collections.emptyList(), 0, false, Reception.ALWAYS, TransactionPhase.IN_PROGRESS)); + // App observers + observers.add(new DevObserverInfo(true, new Name("Alpha"), "fooish", new Name("java.lang.String"), + Collections.emptyList(), 0, false, Reception.ALWAYS, TransactionPhase.IN_PROGRESS)); + observers.add(new DevObserverInfo(true, new Name("Alpha"), "blabla", new Name("java.lang.String"), + Collections.emptyList(), 1, false, Reception.ALWAYS, TransactionPhase.IN_PROGRESS)); + observers.add(new DevObserverInfo(true, null, null, new Name("Charlie"), + Collections.emptyList(), 0, false, Reception.ALWAYS, TransactionPhase.IN_PROGRESS)); + observers.add(new DevObserverInfo(true, new Name("Bravo"), "hop", new Name("java.lang.String"), + Collections.emptyList(), 0, false, Reception.IF_EXISTS, TransactionPhase.IN_PROGRESS)); + + Collections.sort(observers); + assertEquals("blabla", observers.get(0).getMethodName()); + assertEquals("Alpha", observers.get(0).getDeclaringClass().toString()); + assertEquals("fooish", observers.get(1).getMethodName()); + assertEquals("Alpha", observers.get(1).getDeclaringClass().toString()); + assertEquals("hop", observers.get(2).getMethodName()); + assertEquals("Bravo", observers.get(2).getDeclaringClass().toString()); + assertNull(observers.get(3).getMethodName()); + assertEquals("Charlie", observers.get(3).getObservedType().toString()); + assertEquals("Delta", observers.get(observers.size() - 1).getObservedType().toString()); + assertNull(observers.get(observers.size() - 1).getDeclaringClass()); + } + +}