From 5b675bb7961901d6d5df99540406914a4cd2e1cb Mon Sep 17 00:00:00 2001 From: Auri Munoz Date: Tue, 3 Dec 2024 16:30:51 +0100 Subject: [PATCH] Refactor according to code review --- .../JaxrsClientReactiveProcessor.java | 7 +++- ...EndpointValidationPredicatesBuildItem.java | 20 +++++++++++ .../deployment/ResteasyReactiveProcessor.java | 19 ++++++++++- ...SpringRestControllerAnnotationHandler.java | 17 ---------- .../web/deployment/SpringWebProcessor.java | 14 ++++++++ .../DefaultEndpointAnnotationHandler.java | 18 ---------- .../common/processor/EndpointIndexer.java | 33 +++++++++++-------- 7 files changed, 77 insertions(+), 51 deletions(-) create mode 100644 extensions/resteasy-reactive/rest-common/deployment/src/main/java/io/quarkus/resteasy/reactive/common/deployment/EndpointValidationPredicatesBuildItem.java delete mode 100644 extensions/spring-web/core/deployment/src/main/java/io/quarkus/spring/web/deployment/SpringRestControllerAnnotationHandler.java delete mode 100644 independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/DefaultEndpointAnnotationHandler.java diff --git a/extensions/resteasy-reactive/rest-client-jaxrs/deployment/src/main/java/io/quarkus/jaxrs/client/reactive/deployment/JaxrsClientReactiveProcessor.java b/extensions/resteasy-reactive/rest-client-jaxrs/deployment/src/main/java/io/quarkus/jaxrs/client/reactive/deployment/JaxrsClientReactiveProcessor.java index 2f4ac2e80302f..33f74574a49dc 100644 --- a/extensions/resteasy-reactive/rest-client-jaxrs/deployment/src/main/java/io/quarkus/jaxrs/client/reactive/deployment/JaxrsClientReactiveProcessor.java +++ b/extensions/resteasy-reactive/rest-client-jaxrs/deployment/src/main/java/io/quarkus/jaxrs/client/reactive/deployment/JaxrsClientReactiveProcessor.java @@ -50,6 +50,7 @@ import java.util.function.Predicate; import java.util.function.Supplier; import java.util.regex.Pattern; +import java.util.stream.Collectors; import jakarta.ws.rs.ProcessingException; import jakarta.ws.rs.RuntimeType; @@ -171,6 +172,7 @@ import io.quarkus.jaxrs.client.reactive.runtime.ToObjectArray; import io.quarkus.jaxrs.client.reactive.runtime.impl.MultipartResponseDataBase; import io.quarkus.resteasy.reactive.common.deployment.ApplicationResultBuildItem; +import io.quarkus.resteasy.reactive.common.deployment.EndpointValidationPredicatesBuildItem; import io.quarkus.resteasy.reactive.common.deployment.ParameterContainersBuildItem; import io.quarkus.resteasy.reactive.common.deployment.QuarkusFactoryCreator; import io.quarkus.resteasy.reactive.common.deployment.QuarkusResteasyReactiveDotNames; @@ -289,7 +291,8 @@ void setupClientProxies(JaxrsClientReactiveRecorder recorder, List defaultProduces, List disableSmartDefaultProduces, List disableRemovalTrailingSlashProduces, - List parameterContainersBuildItems) { + List parameterContainersBuildItems, + List validationPredicatesBuildItems) { String defaultConsumesType = defaultMediaType(defaultConsumes, MediaType.APPLICATION_OCTET_STREAM); String defaultProducesType = defaultMediaType(defaultProduces, MediaType.TEXT_PLAIN); @@ -343,6 +346,8 @@ public boolean test(Map anns) { return anns.containsKey(NOT_BODY) || anns.containsKey(URL); } }) + .setValidateEndpoint(validationPredicatesBuildItems.stream().map(item -> item.getPredicate()) + .collect(Collectors.toUnmodifiableList())) .setResourceMethodCallback(new Consumer<>() { @Override public void accept(EndpointIndexer.ResourceMethodCallbackEntry entry) { diff --git a/extensions/resteasy-reactive/rest-common/deployment/src/main/java/io/quarkus/resteasy/reactive/common/deployment/EndpointValidationPredicatesBuildItem.java b/extensions/resteasy-reactive/rest-common/deployment/src/main/java/io/quarkus/resteasy/reactive/common/deployment/EndpointValidationPredicatesBuildItem.java new file mode 100644 index 0000000000000..427b5642b4f09 --- /dev/null +++ b/extensions/resteasy-reactive/rest-common/deployment/src/main/java/io/quarkus/resteasy/reactive/common/deployment/EndpointValidationPredicatesBuildItem.java @@ -0,0 +1,20 @@ +package io.quarkus.resteasy.reactive.common.deployment; + +import java.util.function.Predicate; + +import org.jboss.jandex.ClassInfo; + +import io.quarkus.builder.item.MultiBuildItem; + +public final class EndpointValidationPredicatesBuildItem extends MultiBuildItem { + + private final Predicate predicate; + + public EndpointValidationPredicatesBuildItem(Predicate predicate) { + this.predicate = predicate; + } + + public Predicate getPredicate() { + return predicate; + } +} diff --git a/extensions/resteasy-reactive/rest/deployment/src/main/java/io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveProcessor.java b/extensions/resteasy-reactive/rest/deployment/src/main/java/io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveProcessor.java index 35ce4ab24bdde..d07af25448a44 100644 --- a/extensions/resteasy-reactive/rest/deployment/src/main/java/io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveProcessor.java +++ b/extensions/resteasy-reactive/rest/deployment/src/main/java/io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveProcessor.java @@ -14,6 +14,7 @@ import java.io.File; import java.io.IOException; +import java.lang.reflect.Modifier; import java.nio.file.Path; import java.util.ArrayDeque; import java.util.ArrayList; @@ -161,6 +162,7 @@ import io.quarkus.netty.deployment.MinNettyAllocatorMaxOrderBuildItem; import io.quarkus.resteasy.reactive.common.deployment.AggregatedParameterContainersBuildItem; import io.quarkus.resteasy.reactive.common.deployment.ApplicationResultBuildItem; +import io.quarkus.resteasy.reactive.common.deployment.EndpointValidationPredicatesBuildItem; import io.quarkus.resteasy.reactive.common.deployment.FactoryUtils; import io.quarkus.resteasy.reactive.common.deployment.ParameterContainersBuildItem; import io.quarkus.resteasy.reactive.common.deployment.QuarkusFactoryCreator; @@ -468,7 +470,8 @@ public void setupEndpoints(ApplicationIndexBuildItem applicationIndexBuildItem, CompiledJavaVersionBuildItem compiledJavaVersionBuildItem, ResourceInterceptorsBuildItem resourceInterceptorsBuildItem, Capabilities capabilities, - Optional allowNotRestParametersBuildItem) { + Optional allowNotRestParametersBuildItem, + List validationPredicatesBuildItems) { if (!resourceScanningResultBuildItem.isPresent()) { // no detected @Path, bail out @@ -640,6 +643,8 @@ private boolean hasAnnotation(MethodInfo method, short paramPosition, DotName an }) .setResteasyReactiveRecorder(recorder) .setApplicationClassPredicate(applicationClassPredicate) + .setValidateEndpoint(validationPredicatesBuildItems.stream().map(item -> item.getPredicate()) + .collect(Collectors.toUnmodifiableList())) .setTargetJavaVersion(new TargetJavaVersion() { private final Status result; @@ -849,6 +854,18 @@ private FilterClassIntrospector createFilterClassIntrospector() { return ab.get(); } + @BuildStep + EndpointValidationPredicatesBuildItem createSpringRestControllerPredicate() { + Predicate predicate = new Predicate() { + @Override + public boolean test(ClassInfo classInfo) { + return Modifier.isInterface(classInfo.flags()) + || Modifier.isAbstract(classInfo.flags()); + } + }; + return new EndpointValidationPredicatesBuildItem(predicate); + } + // We want to add @Typed to resources, beanparams and providers so that they can be resolved as CDI bean using purely their // class as a bean type. This removes any ambiguity that potential subclasses may have. @BuildStep diff --git a/extensions/spring-web/core/deployment/src/main/java/io/quarkus/spring/web/deployment/SpringRestControllerAnnotationHandler.java b/extensions/spring-web/core/deployment/src/main/java/io/quarkus/spring/web/deployment/SpringRestControllerAnnotationHandler.java deleted file mode 100644 index 0c2ae5b70d400..0000000000000 --- a/extensions/spring-web/core/deployment/src/main/java/io/quarkus/spring/web/deployment/SpringRestControllerAnnotationHandler.java +++ /dev/null @@ -1,17 +0,0 @@ -package io.quarkus.spring.web.deployment; - -import org.jboss.jandex.ClassInfo; -import org.jboss.jandex.DotName; -import org.jboss.resteasy.reactive.common.processor.EndpointAnnotationHandler; - -public class SpringRestControllerAnnotationHandler implements EndpointAnnotationHandler { - public static final DotName SPRING_REST_CONTROLLER = DotName - .createSimple("org.springframework.web.bind.annotation.RestController"); - - @Override - public boolean isEndpointAnnotationValid(ClassInfo classInfo) { - return classInfo - .declaredAnnotation(SPRING_REST_CONTROLLER) != null; - } - -} diff --git a/extensions/spring-web/core/deployment/src/main/java/io/quarkus/spring/web/deployment/SpringWebProcessor.java b/extensions/spring-web/core/deployment/src/main/java/io/quarkus/spring/web/deployment/SpringWebProcessor.java index 4699cc9593141..752cb73baac71 100644 --- a/extensions/spring-web/core/deployment/src/main/java/io/quarkus/spring/web/deployment/SpringWebProcessor.java +++ b/extensions/spring-web/core/deployment/src/main/java/io/quarkus/spring/web/deployment/SpringWebProcessor.java @@ -6,6 +6,7 @@ import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.function.Predicate; import jakarta.ws.rs.Priorities; import jakarta.ws.rs.core.HttpHeaders; @@ -35,6 +36,7 @@ import io.quarkus.gizmo.ClassOutput; import io.quarkus.jaxrs.spi.deployment.AdditionalJaxRsResourceMethodAnnotationsBuildItem; import io.quarkus.resteasy.common.spi.ResteasyJaxrsProviderBuildItem; +import io.quarkus.resteasy.reactive.common.deployment.EndpointValidationPredicatesBuildItem; import io.quarkus.resteasy.reactive.spi.ExceptionMapperBuildItem; public class SpringWebProcessor { @@ -86,6 +88,18 @@ public AdditionalJaxRsResourceMethodAnnotationsBuildItem additionalJaxRsResource return new AdditionalJaxRsResourceMethodAnnotationsBuildItem(MAPPING_ANNOTATIONS); } + @BuildStep + EndpointValidationPredicatesBuildItem createSpringRestControllerPredicate() { + Predicate predicate = new Predicate<>() { + @Override + public boolean test(ClassInfo classInfo) { + return classInfo + .declaredAnnotation(REST_CONTROLLER_ANNOTATION) == null; + } + }; + return new EndpointValidationPredicatesBuildItem(predicate); + } + @BuildStep public void ignoreReflectionHierarchy(BuildProducer ignore) { ignore.produce(new ReflectiveHierarchyIgnoreWarningBuildItem( diff --git a/independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/DefaultEndpointAnnotationHandler.java b/independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/DefaultEndpointAnnotationHandler.java deleted file mode 100644 index 188530e49e8ef..0000000000000 --- a/independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/DefaultEndpointAnnotationHandler.java +++ /dev/null @@ -1,18 +0,0 @@ -package org.jboss.resteasy.reactive.common.processor; - -import java.lang.reflect.Modifier; - -import org.jboss.jandex.ClassInfo; - -public class DefaultEndpointAnnotationHandler implements EndpointAnnotationHandler { - public DefaultEndpointAnnotationHandler() { - } - - @Override - public boolean isEndpointAnnotationValid(ClassInfo classInfo) { - if (Modifier.isInterface(classInfo.flags()) || Modifier.isAbstract(classInfo.flags())) { - return true; - } - return false; - } -} diff --git a/independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/EndpointIndexer.java b/independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/EndpointIndexer.java index dda303afc2f4e..07a1e26da4172 100644 --- a/independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/EndpointIndexer.java +++ b/independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/EndpointIndexer.java @@ -97,7 +97,6 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; -import java.util.ServiceLoader; import java.util.Set; import java.util.function.Consumer; import java.util.function.Function; @@ -240,6 +239,7 @@ public abstract class EndpointIndexer> isDisabledCreator; private final Predicate> skipMethodParameter; + private final List> validateEndpoint; private final boolean skipNotRestParameters; private SerializerScanningResult serializerScanningResult; @@ -268,6 +268,7 @@ protected EndpointIndexer(Builder builder) { this.isDisabledCreator = builder.isDisabledCreator; this.skipMethodParameter = builder.skipMethodParameter; this.skipNotRestParameters = builder.skipNotRestParameters; + this.validateEndpoint = builder.validateEndpoint; } public Optional createEndpoints(ClassInfo classInfo, boolean considerApplication) { @@ -325,21 +326,19 @@ public Optional createEndpoints(ClassInfo classInfo, boolean cons return Optional.of(clazz); } catch (Exception e) { - if (isEndpointValid(classInfo)) { - //kinda bogus, but we just ignore failed interfaces for now - //they can have methods that are not valid until they are actually extended by a concrete type - log.debug("Ignoring interface " + classInfo.name(), e); - return Optional.empty(); + for (Predicate predicate : validateEndpoint) { + if (predicate.test(classInfo)) { + //kinda bogus, but we just ignore failed interfaces for now + //they can have methods that are not valid until they are actually extended by a concrete type + log.debug("Ignoring interface " + classInfo.name(), e); + return Optional.empty(); + } else { + throw new RuntimeException(e); + } + } - throw new RuntimeException(e); } - } - - private boolean isEndpointValid(ClassInfo classInfo) { - EndpointAnnotationHandler defaultEndpointAnnotationHandler = new DefaultEndpointAnnotationHandler(); - return ServiceLoader.load(EndpointAnnotationHandler.class).findFirst() - .orElse(defaultEndpointAnnotationHandler).isEndpointAnnotationValid(classInfo); - + return Optional.empty(); } private String sanitizePath(String path) { @@ -1716,6 +1715,7 @@ public boolean handleMultipartForReturnType(AdditionalWriters additionalWriters, private Function> isDisabledCreator = null; private Predicate> skipMethodParameter = null; + private List> validateEndpoint = null; private boolean skipNotRestParameters = false; @@ -1852,6 +1852,11 @@ public B setSkipMethodParameter( return (B) this; } + public B setValidateEndpoint(List> validateEndpoint) { + this.validateEndpoint = validateEndpoint; + return (B) this; + } + public B skipNotRestParameters(boolean skipNotRestParameters) { this.skipNotRestParameters = skipNotRestParameters; return (B) this;