From a043ac4112723f361dfc694f5c74d0a3f148ac4b Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Fri, 22 Sep 2023 18:19:41 +0200 Subject: [PATCH 1/2] Fault Tolerance: fix duplicate circuit breaker name detection Previously, duplicate circuit breaker names were detected in a crude way: find all occurences of `@CircuitBreakerName` in the Jandex index, collect the values, and fail of some value is present more than once. With this commit, circuit breaker names are collected in a more precise way: only from beans, and only from methods that are actually detected as methods with fault tolerance annotations. This even correctly includes transformed annotations. --- .../deployment/FaultToleranceScanner.java | 2 +- .../SmallRyeFaultToleranceProcessor.java | 18 +++++------- .../noduplicate/CircuitBreakerService1.java | 16 ++++++++++ .../noduplicate/CircuitBreakerService2.java | 14 +++++++++ .../NoDuplicateCircuitBreakerNameTest.java | 29 +++++++++++++++++++ 5 files changed, 67 insertions(+), 12 deletions(-) create mode 100644 extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/noduplicate/CircuitBreakerService1.java create mode 100644 extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/noduplicate/CircuitBreakerService2.java create mode 100644 extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/noduplicate/NoDuplicateCircuitBreakerNameTest.java diff --git a/extensions/smallrye-fault-tolerance/deployment/src/main/java/io/quarkus/smallrye/faulttolerance/deployment/FaultToleranceScanner.java b/extensions/smallrye-fault-tolerance/deployment/src/main/java/io/quarkus/smallrye/faulttolerance/deployment/FaultToleranceScanner.java index 943d1725f9028..bca9ef3b1e576 100644 --- a/extensions/smallrye-fault-tolerance/deployment/src/main/java/io/quarkus/smallrye/faulttolerance/deployment/FaultToleranceScanner.java +++ b/extensions/smallrye-fault-tolerance/deployment/src/main/java/io/quarkus/smallrye/faulttolerance/deployment/FaultToleranceScanner.java @@ -80,7 +80,7 @@ boolean hasFTAnnotations(ClassInfo clazz) { void forEachMethod(ClassInfo clazz, Consumer action) { for (MethodInfo method : clazz.methods()) { if (method.name().startsWith("<")) { - // constructors (or static init blocks) can't be intercepted + // constructors and static inititalizers can't be intercepted continue; } if (method.isSynthetic()) { diff --git a/extensions/smallrye-fault-tolerance/deployment/src/main/java/io/quarkus/smallrye/faulttolerance/deployment/SmallRyeFaultToleranceProcessor.java b/extensions/smallrye-fault-tolerance/deployment/src/main/java/io/quarkus/smallrye/faulttolerance/deployment/SmallRyeFaultToleranceProcessor.java index 805878809ebce..345c8808dc85f 100644 --- a/extensions/smallrye-fault-tolerance/deployment/src/main/java/io/quarkus/smallrye/faulttolerance/deployment/SmallRyeFaultToleranceProcessor.java +++ b/extensions/smallrye-fault-tolerance/deployment/src/main/java/io/quarkus/smallrye/faulttolerance/deployment/SmallRyeFaultToleranceProcessor.java @@ -24,7 +24,6 @@ import org.jboss.jandex.ClassInfo; import org.jboss.jandex.DotName; import org.jboss.jandex.IndexView; -import org.jboss.jandex.MethodInfo; import io.quarkus.arc.deployment.AdditionalBeanBuildItem; import io.quarkus.arc.deployment.AnnotationsTransformerBuildItem; @@ -279,6 +278,7 @@ void processFaultToleranceAnnotations(SmallRyeFaultToleranceRecorder recorder, List ftMethods = new ArrayList<>(); List exceptions = new ArrayList<>(); + Map> existingCircuitBreakerNames = new HashMap<>(); for (BeanInfo info : validationPhase.getContext().beans()) { ClassInfo beanClass = info.getImplClazz(); @@ -319,6 +319,12 @@ void processFaultToleranceAnnotations(SmallRyeFaultToleranceRecorder recorder, reflectiveClass.produce(ReflectiveClassBuildItem.builder(exceptionNames.get()).build()); } } + + if (annotationStore.hasAnnotation(method, DotNames.CIRCUIT_BREAKER_NAME)) { + AnnotationInstance ann = annotationStore.getAnnotation(method, DotNames.CIRCUIT_BREAKER_NAME); + existingCircuitBreakerNames.computeIfAbsent(ann.value().asString(), ignored -> new HashSet<>()) + .add(method + " @ " + method.declaringClass()); + } } }); @@ -337,16 +343,6 @@ void processFaultToleranceAnnotations(SmallRyeFaultToleranceRecorder recorder, recorder.createFaultToleranceOperation(ftMethods); - // since annotation transformations are applied lazily, we can't know - // all transformed `@CircuitBreakerName`s and have to rely on Jandex here - Map> existingCircuitBreakerNames = new HashMap<>(); - for (AnnotationInstance it : index.getAnnotations(DotNames.CIRCUIT_BREAKER_NAME)) { - if (it.target().kind() == Kind.METHOD) { - MethodInfo method = it.target().asMethod(); - existingCircuitBreakerNames.computeIfAbsent(it.value().asString(), ignored -> new HashSet<>()) - .add(method + " @ " + method.declaringClass()); - } - } for (Map.Entry> entry : existingCircuitBreakerNames.entrySet()) { if (entry.getValue().size() > 1) { exceptions.add(new DefinitionException("Multiple circuit breakers have the same name '" diff --git a/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/noduplicate/CircuitBreakerService1.java b/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/noduplicate/CircuitBreakerService1.java new file mode 100644 index 0000000000000..78431399ba5ca --- /dev/null +++ b/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/noduplicate/CircuitBreakerService1.java @@ -0,0 +1,16 @@ +package io.quarkus.smallrye.faulttolerance.test.circuitbreaker.maintenance.noduplicate; + +import jakarta.inject.Singleton; + +import org.eclipse.microprofile.faulttolerance.CircuitBreaker; + +import io.smallrye.faulttolerance.api.CircuitBreakerName; + +@Singleton +public class CircuitBreakerService1 { + @CircuitBreaker + @CircuitBreakerName("hello") + public String hello() { + return "1"; + } +} diff --git a/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/noduplicate/CircuitBreakerService2.java b/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/noduplicate/CircuitBreakerService2.java new file mode 100644 index 0000000000000..5a0bdd28ffdc6 --- /dev/null +++ b/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/noduplicate/CircuitBreakerService2.java @@ -0,0 +1,14 @@ +package io.quarkus.smallrye.faulttolerance.test.circuitbreaker.maintenance.noduplicate; + +import org.eclipse.microprofile.faulttolerance.CircuitBreaker; + +import io.smallrye.faulttolerance.api.CircuitBreakerName; + +public class CircuitBreakerService2 { + // this class is not a bean, so there's no circuit breaker and hence no duplicate circuit breaker name + @CircuitBreaker + @CircuitBreakerName("hello") + public String hello() { + return "2"; + } +} diff --git a/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/noduplicate/NoDuplicateCircuitBreakerNameTest.java b/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/noduplicate/NoDuplicateCircuitBreakerNameTest.java new file mode 100644 index 0000000000000..f513b7cbdbd0c --- /dev/null +++ b/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/noduplicate/NoDuplicateCircuitBreakerNameTest.java @@ -0,0 +1,29 @@ +package io.quarkus.smallrye.faulttolerance.test.circuitbreaker.maintenance.noduplicate; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +import jakarta.inject.Inject; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.test.QuarkusUnitTest; +import io.smallrye.faulttolerance.api.CircuitBreakerMaintenance; +import io.smallrye.faulttolerance.api.CircuitBreakerState; + +public class NoDuplicateCircuitBreakerNameTest { + @RegisterExtension + static final QuarkusUnitTest config = new QuarkusUnitTest() + .withApplicationRoot((jar) -> jar + .addClasses(CircuitBreakerService1.class, CircuitBreakerService2.class)); + + @Inject + CircuitBreakerMaintenance cb; + + @Test + public void deploysWithoutError() { + assertNotNull(cb); + assertEquals(CircuitBreakerState.CLOSED, cb.currentState("hello")); + } +} From 5db67ac9f1eb491d4f63f66eeaeb44f0863a25a8 Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Fri, 22 Sep 2023 18:31:59 +0200 Subject: [PATCH 2/2] Fault Tolerance: fix misleading package names in the extension tests --- .../{inheritance => duplicate}/CircuitBreakerService1.java | 2 +- .../{inheritance => duplicate}/CircuitBreakerService2.java | 2 +- .../DuplicateCircuitBreakerNameTest.java | 2 +- .../CircuitBreakerNameInheritanceTest.java | 2 +- .../{duplicate => inheritance}/SubCircuitBreakerService.java | 2 +- .../{duplicate => inheritance}/SuperCircuitBreakerService.java | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) rename extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/{inheritance => duplicate}/CircuitBreakerService1.java (94%) rename extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/{inheritance => duplicate}/CircuitBreakerService2.java (94%) rename extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/{inheritance => duplicate}/DuplicateCircuitBreakerNameTest.java (97%) rename extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/{duplicate => inheritance}/CircuitBreakerNameInheritanceTest.java (96%) rename extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/{duplicate => inheritance}/SubCircuitBreakerService.java (93%) rename extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/{duplicate => inheritance}/SuperCircuitBreakerService.java (94%) diff --git a/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/inheritance/CircuitBreakerService1.java b/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/duplicate/CircuitBreakerService1.java similarity index 94% rename from extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/inheritance/CircuitBreakerService1.java rename to extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/duplicate/CircuitBreakerService1.java index 687f50da09b0b..884c337c7ef26 100644 --- a/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/inheritance/CircuitBreakerService1.java +++ b/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/duplicate/CircuitBreakerService1.java @@ -1,4 +1,4 @@ -package io.quarkus.smallrye.faulttolerance.test.circuitbreaker.maintenance.inheritance; +package io.quarkus.smallrye.faulttolerance.test.circuitbreaker.maintenance.duplicate; import jakarta.inject.Singleton; diff --git a/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/inheritance/CircuitBreakerService2.java b/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/duplicate/CircuitBreakerService2.java similarity index 94% rename from extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/inheritance/CircuitBreakerService2.java rename to extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/duplicate/CircuitBreakerService2.java index 94d7be6f34af0..6aa742b9b324d 100644 --- a/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/inheritance/CircuitBreakerService2.java +++ b/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/duplicate/CircuitBreakerService2.java @@ -1,4 +1,4 @@ -package io.quarkus.smallrye.faulttolerance.test.circuitbreaker.maintenance.inheritance; +package io.quarkus.smallrye.faulttolerance.test.circuitbreaker.maintenance.duplicate; import jakarta.inject.Singleton; diff --git a/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/inheritance/DuplicateCircuitBreakerNameTest.java b/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/duplicate/DuplicateCircuitBreakerNameTest.java similarity index 97% rename from extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/inheritance/DuplicateCircuitBreakerNameTest.java rename to extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/duplicate/DuplicateCircuitBreakerNameTest.java index c5bb50252ed96..bbc57be5a8726 100644 --- a/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/inheritance/DuplicateCircuitBreakerNameTest.java +++ b/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/duplicate/DuplicateCircuitBreakerNameTest.java @@ -1,4 +1,4 @@ -package io.quarkus.smallrye.faulttolerance.test.circuitbreaker.maintenance.inheritance; +package io.quarkus.smallrye.faulttolerance.test.circuitbreaker.maintenance.duplicate; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; diff --git a/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/duplicate/CircuitBreakerNameInheritanceTest.java b/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/inheritance/CircuitBreakerNameInheritanceTest.java similarity index 96% rename from extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/duplicate/CircuitBreakerNameInheritanceTest.java rename to extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/inheritance/CircuitBreakerNameInheritanceTest.java index 160e0d396ba18..d5fdf1a4a724e 100644 --- a/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/duplicate/CircuitBreakerNameInheritanceTest.java +++ b/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/inheritance/CircuitBreakerNameInheritanceTest.java @@ -1,4 +1,4 @@ -package io.quarkus.smallrye.faulttolerance.test.circuitbreaker.maintenance.duplicate; +package io.quarkus.smallrye.faulttolerance.test.circuitbreaker.maintenance.inheritance; import static org.junit.jupiter.api.Assertions.assertNotNull; diff --git a/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/duplicate/SubCircuitBreakerService.java b/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/inheritance/SubCircuitBreakerService.java similarity index 93% rename from extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/duplicate/SubCircuitBreakerService.java rename to extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/inheritance/SubCircuitBreakerService.java index b2f26bbc454a1..49e1c9c9bc742 100644 --- a/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/duplicate/SubCircuitBreakerService.java +++ b/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/inheritance/SubCircuitBreakerService.java @@ -1,4 +1,4 @@ -package io.quarkus.smallrye.faulttolerance.test.circuitbreaker.maintenance.duplicate; +package io.quarkus.smallrye.faulttolerance.test.circuitbreaker.maintenance.inheritance; import jakarta.inject.Singleton; diff --git a/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/duplicate/SuperCircuitBreakerService.java b/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/inheritance/SuperCircuitBreakerService.java similarity index 94% rename from extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/duplicate/SuperCircuitBreakerService.java rename to extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/inheritance/SuperCircuitBreakerService.java index 0a87d76f87b20..685b09931e0aa 100644 --- a/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/duplicate/SuperCircuitBreakerService.java +++ b/extensions/smallrye-fault-tolerance/deployment/src/test/java/io/quarkus/smallrye/faulttolerance/test/circuitbreaker/maintenance/inheritance/SuperCircuitBreakerService.java @@ -1,4 +1,4 @@ -package io.quarkus.smallrye.faulttolerance.test.circuitbreaker.maintenance.duplicate; +package io.quarkus.smallrye.faulttolerance.test.circuitbreaker.maintenance.inheritance; import jakarta.inject.Singleton;