From 6f594710ac4465c074f17347cfba5ef3c8afb094 Mon Sep 17 00:00:00 2001 From: Santiago Pericas-Geertsen Date: Fri, 5 Apr 2024 10:57:46 -0400 Subject: [PATCH] Validate @WithExecutor annotations at start up time, during execution of the FT CDI extension. --- microprofile/fault-tolerance/pom.xml | 5 ++ .../FaultToleranceExtension.java | 20 +++++++- .../faulttolerance/MethodAntn.java | 5 +- .../faulttolerance/MethodInvoker.java | 10 +--- .../faulttolerance/AsynchronousBean.java | 12 ----- .../faulttolerance/AsynchronousTest.java | 9 ---- .../ValidateWithExecutorTest.java | 51 +++++++++++++++++++ 7 files changed, 77 insertions(+), 35 deletions(-) create mode 100644 microprofile/fault-tolerance/src/test/java/io/helidon/microprofile/faulttolerance/ValidateWithExecutorTest.java diff --git a/microprofile/fault-tolerance/pom.xml b/microprofile/fault-tolerance/pom.xml index 797eba92d9a..17d3eae60ba 100644 --- a/microprofile/fault-tolerance/pom.xml +++ b/microprofile/fault-tolerance/pom.xml @@ -123,6 +123,11 @@ helidon-microprofile-testing-junit5 test + + org.mockito + mockito-core + test + diff --git a/microprofile/fault-tolerance/src/main/java/io/helidon/microprofile/faulttolerance/FaultToleranceExtension.java b/microprofile/fault-tolerance/src/main/java/io/helidon/microprofile/faulttolerance/FaultToleranceExtension.java index 80945de55b6..7c551c79e26 100644 --- a/microprofile/fault-tolerance/src/main/java/io/helidon/microprofile/faulttolerance/FaultToleranceExtension.java +++ b/microprofile/fault-tolerance/src/main/java/io/helidon/microprofile/faulttolerance/FaultToleranceExtension.java @@ -17,12 +17,14 @@ package io.helidon.microprofile.faulttolerance; import java.lang.annotation.Annotation; +import java.lang.reflect.Method; import java.lang.reflect.Type; import java.util.Arrays; import java.util.HashSet; import java.util.Optional; import java.util.Set; import java.util.concurrent.CopyOnWriteArraySet; +import java.util.concurrent.ExecutorService; import java.util.stream.Collectors; import jakarta.annotation.Priority; @@ -33,6 +35,7 @@ import jakarta.enterprise.inject.spi.AnnotatedField; import jakarta.enterprise.inject.spi.AnnotatedMethod; import jakarta.enterprise.inject.spi.AnnotatedType; +import jakarta.enterprise.inject.spi.Bean; import jakarta.enterprise.inject.spi.BeanManager; import jakarta.enterprise.inject.spi.BeforeBeanDiscovery; import jakarta.enterprise.inject.spi.BeforeShutdown; @@ -50,6 +53,7 @@ import org.eclipse.microprofile.faulttolerance.Fallback; import org.eclipse.microprofile.faulttolerance.Retry; import org.eclipse.microprofile.faulttolerance.Timeout; +import org.eclipse.microprofile.faulttolerance.exceptions.FaultToleranceDefinitionException; import org.glassfish.jersey.process.internal.RequestScope; import static jakarta.interceptor.Interceptor.Priority.LIBRARY_BEFORE; @@ -210,8 +214,6 @@ void validateAnnotations(BeanManager bm, @Initialized(ApplicationScoped.class) Object event) { if (FaultToleranceMetrics.enabled()) { getRegisteredMethods().forEach(annotatedMethod -> { - final AnnotatedType annotatedType = annotatedMethod.getDeclaringType(); - // Metrics depending on the annotationSet present if (MethodAntn.isAnnotationPresent(annotatedMethod, Retry.class, bm)) { new RetryAntn(annotatedMethod).validate(); @@ -230,11 +232,25 @@ void validateAnnotations(BeanManager bm, } if (MethodAntn.isAnnotationPresent(annotatedMethod, Asynchronous.class, bm)) { new AsynchronousAntn(annotatedMethod).validate(); + validateWithExecutor(bm, annotatedMethod.getJavaMember()); } }); } } + static void validateWithExecutor(BeanManager bm, Method method) { + WithExecutor withExecutor = method.getAnnotation(WithExecutor.class); + if (withExecutor != null) { + Set> beans = bm.getBeans(ExecutorService.class, withExecutor); + if (beans.isEmpty()) { + throw new FaultToleranceDefinitionException("Unable to resolved named executor service '" + + withExecutor.value() + "' at " + + method.getDeclaringClass().getName() + "::" + + method.getName()); + } + } + } + void close(@Observes BeforeShutdown shutdown) { FaultToleranceMetrics.close(); // we need to clear method cache, as the next start could use different config diff --git a/microprofile/fault-tolerance/src/main/java/io/helidon/microprofile/faulttolerance/MethodAntn.java b/microprofile/fault-tolerance/src/main/java/io/helidon/microprofile/faulttolerance/MethodAntn.java index eb564ef0618..67f5e0bf1ee 100644 --- a/microprofile/fault-tolerance/src/main/java/io/helidon/microprofile/faulttolerance/MethodAntn.java +++ b/microprofile/fault-tolerance/src/main/java/io/helidon/microprofile/faulttolerance/MethodAntn.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018, 2023 Oracle and/or its affiliates. + * Copyright (c) 2018, 2024 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -39,8 +39,6 @@ abstract class MethodAntn { private static final AnnotationFinder ANNOTATION_FINDER = AnnotationFinder.create(Retry.class.getPackage()); - private final AnnotatedType annotatedType; - private final AnnotatedMethod annotatedMethod; enum MatchingType { @@ -80,7 +78,6 @@ public A getAnnotation() { */ MethodAntn(AnnotatedMethod annotatedMethod) { this.annotatedMethod = annotatedMethod; - this.annotatedType = annotatedMethod.getDeclaringType(); } Method method() { diff --git a/microprofile/fault-tolerance/src/main/java/io/helidon/microprofile/faulttolerance/MethodInvoker.java b/microprofile/fault-tolerance/src/main/java/io/helidon/microprofile/faulttolerance/MethodInvoker.java index bc237ba8012..a6169a0bd1d 100644 --- a/microprofile/fault-tolerance/src/main/java/io/helidon/microprofile/faulttolerance/MethodInvoker.java +++ b/microprofile/fault-tolerance/src/main/java/io/helidon/microprofile/faulttolerance/MethodInvoker.java @@ -43,12 +43,10 @@ import io.helidon.faulttolerance.RetryTimeoutException; import io.helidon.faulttolerance.Timeout; -import jakarta.enterprise.inject.UnsatisfiedResolutionException; import jakarta.enterprise.inject.spi.CDI; import jakarta.interceptor.InvocationContext; import org.eclipse.microprofile.faulttolerance.exceptions.BulkheadException; import org.eclipse.microprofile.faulttolerance.exceptions.CircuitBreakerOpenException; -import org.eclipse.microprofile.faulttolerance.exceptions.FaultToleranceException; import org.eclipse.microprofile.metrics.Counter; import static io.helidon.faulttolerance.SupplierHelper.toRuntimeException; @@ -343,12 +341,8 @@ private CompletableFuture callSupplierNewThread(FtSupplier suppl // Handle a user-provided executor via @WithExecutor if (introspector.hasWithExecutor()) { WithExecutor withExecutor = introspector.withExecutor(); - try { - ExecutorService executorService = CDI.current().select(ExecutorService.class, withExecutor).get(); - asyncBuilder.executor(executorService); - } catch (UnsatisfiedResolutionException e) { - throw new FaultToleranceException(e); - } + ExecutorService executorService = CDI.current().select(ExecutorService.class, withExecutor).get(); + asyncBuilder.executor(executorService); } // Invoke async call diff --git a/microprofile/fault-tolerance/src/test/java/io/helidon/microprofile/faulttolerance/AsynchronousBean.java b/microprofile/fault-tolerance/src/test/java/io/helidon/microprofile/faulttolerance/AsynchronousBean.java index 3c33da480e0..233ee651496 100644 --- a/microprofile/fault-tolerance/src/test/java/io/helidon/microprofile/faulttolerance/AsynchronousBean.java +++ b/microprofile/fault-tolerance/src/test/java/io/helidon/microprofile/faulttolerance/AsynchronousBean.java @@ -27,7 +27,6 @@ import jakarta.enterprise.inject.Produces; import org.eclipse.microprofile.faulttolerance.Asynchronous; import org.eclipse.microprofile.faulttolerance.Fallback; -import org.eclipse.microprofile.faulttolerance.exceptions.FaultToleranceException; /** * A stateful bean that defines some async methods. @@ -188,15 +187,4 @@ CompletableFuture asyncWithExecutor() { ExecutorService executorService() { return Executors.newFixedThreadPool(10); } - - /** - * Bad executor name, should throw a {@link FaultToleranceException}. - * - * @return A future. - */ - @Asynchronous - @WithExecutor("does-not-exist") - CompletableFuture asyncWithBadExecutor() { - return CompletableFuture.completedFuture("failed"); - } } diff --git a/microprofile/fault-tolerance/src/test/java/io/helidon/microprofile/faulttolerance/AsynchronousTest.java b/microprofile/fault-tolerance/src/test/java/io/helidon/microprofile/faulttolerance/AsynchronousTest.java index d2e47dc72c3..a33077168f7 100644 --- a/microprofile/fault-tolerance/src/test/java/io/helidon/microprofile/faulttolerance/AsynchronousTest.java +++ b/microprofile/fault-tolerance/src/test/java/io/helidon/microprofile/faulttolerance/AsynchronousTest.java @@ -22,12 +22,10 @@ import io.helidon.microprofile.testing.junit5.AddBean; import jakarta.inject.Inject; -import org.eclipse.microprofile.faulttolerance.exceptions.FaultToleranceException; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; -import static org.junit.jupiter.api.Assertions.assertThrows; /** * Tests for asynchronous invocations using {@code Asynchronous}. @@ -132,11 +130,4 @@ void testAsyncWithExecutor() throws Exception { future.get(); assertThat(bean.wasCalled(), is(true)); } - - @Test - void testAsyncWithBadExecutor() { - assertThat(bean.wasCalled(), is(false)); - assertThrows(FaultToleranceException.class, () -> bean.asyncWithBadExecutor()); - assertThat(bean.wasCalled(), is(false)); - } } diff --git a/microprofile/fault-tolerance/src/test/java/io/helidon/microprofile/faulttolerance/ValidateWithExecutorTest.java b/microprofile/fault-tolerance/src/test/java/io/helidon/microprofile/faulttolerance/ValidateWithExecutorTest.java new file mode 100644 index 00000000000..d824762cfbc --- /dev/null +++ b/microprofile/fault-tolerance/src/test/java/io/helidon/microprofile/faulttolerance/ValidateWithExecutorTest.java @@ -0,0 +1,51 @@ +/* + * Copyright (c) 2024 Oracle and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.helidon.microprofile.faulttolerance; + +import java.lang.reflect.Method; +import java.util.Collections; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutorService; + +import jakarta.enterprise.inject.spi.BeanManager; +import org.eclipse.microprofile.faulttolerance.Asynchronous; +import org.eclipse.microprofile.faulttolerance.exceptions.FaultToleranceDefinitionException; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +class ValidateWithExecutorTest { + + static class BadWithExecutorBean { + + @Asynchronous + @WithExecutor("does-not-exist") + public CompletableFuture shouldNotBeCalled() { + return CompletableFuture.completedFuture(null); + } + } + + @Test + void badExecutorTest() throws Exception { + Method method = BadWithExecutorBean.class.getMethod("shouldNotBeCalled"); + WithExecutor withExecutor = method.getAnnotation(WithExecutor.class); + BeanManager bm = Mockito.mock(BeanManager.class); + Mockito.when(bm.getBeans(ExecutorService.class, withExecutor)).thenReturn(Collections.emptySet()); + assertThrows(FaultToleranceDefinitionException.class, + () -> FaultToleranceExtension.validateWithExecutor(bm, method)); + } +}