From 0e3add0675548ea6e73f77e20ab4df2408b14b26 Mon Sep 17 00:00:00 2001 From: Martin Panzer Date: Tue, 18 Feb 2025 07:30:40 +0100 Subject: [PATCH] Check reachability of sub resources before indexing them This makes sure, that the absent and present sub resources can actually be reached from a root resource class in the server endpoint indexer. Otherwise, e.g. client sub resource interface could get indexed as server endpoints --- .../deployment/ResteasyReactiveProcessor.java | 132 +++++++++--- ...sourceInterfaceAndClientInterfaceTest.java | 198 ++++++++++++++++++ 2 files changed, 304 insertions(+), 26 deletions(-) create mode 100644 extensions/resteasy-reactive/rest/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/resource/basic/SubResourceInterfaceAndClientInterfaceTest.java 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 b255194d2925fb..7a90653308737e 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 @@ -730,14 +730,29 @@ public Supplier apply(ClassInfo classInfo) { checkForDuplicateEndpoint(config, allServerMethods); + Function typeToReturnName = new Function() { + @Override + public DotName apply(Type type) { + DotName typeName = type.name(); + if (type.kind() == Type.Kind.CLASS) { + return typeName; + } else if (type.kind() == Type.Kind.PARAMETERIZED_TYPE + && typeName.equals(DotName.createSimple(Class.class))) { + return type.asParameterizedType().arguments().get(0).name(); + } + return null; + } + }; + + Map> declaringClassToReturnNames = new HashMap<>(); //now index possible sub resources. These are all classes that have method annotations //that are not annotated @Path - Deque toScan = new ArrayDeque<>(); for (DotName methodAnnotation : result.getHttpAnnotationToMethod().keySet()) { for (AnnotationInstance instance : index.getAnnotations(methodAnnotation)) { MethodInfo method = instance.target().asMethod(); ClassInfo classInfo = method.declaringClass(); - toScan.add(classInfo); + declaringClassToReturnNames.computeIfAbsent(classInfo.name(), ignored -> new HashSet<>()) + .add(typeToReturnName.apply(method.returnType())); } } //sub resources can also have just a path annotation @@ -746,11 +761,98 @@ public Supplier apply(ClassInfo classInfo) { if (instance.target().kind() == AnnotationTarget.Kind.METHOD) { MethodInfo method = instance.target().asMethod(); ClassInfo classInfo = method.declaringClass(); - toScan.add(classInfo); + declaringClassToReturnNames.computeIfAbsent(classInfo.name(), ignored -> new HashSet<>()) + .add(typeToReturnName.apply(method.returnType())); + } + } + + Map> subClassesBySubResources = new HashMap<>(); + for (DotName dotName : declaringClassToReturnNames.keySet()) { + Set all = new HashSet<>(); + all.add(dotName); + index.getAllKnownSubclasses(dotName).forEach(c2 -> all.add(c2.name())); + index.getAllKnownSubinterfaces(dotName).forEach(c2 -> all.add(c2.name())); + index.getAllKnownImplementors(dotName).forEach(c2 -> all.add(c2.name())); + + subClassesBySubResources.put(dotName, all); + } + + Map> childs = new HashMap<>(); + Set resourceClassNames = new HashSet<>(); + for (ResourceClass resourceClass : resourceClasses) { + resourceClassNames.add(DotName.createSimple(resourceClass.getClassName())); + } + + Deque declQueue = new ArrayDeque<>(resourceClassNames); + Set seen = new HashSet<>(); + Deque toScan = new ArrayDeque<>(); + while (!declQueue.isEmpty()) { + DotName poll = declQueue.poll(); + if (!seen.add(poll)) { + continue; + } + + Set foundParentSubResources = new HashSet<>(); + if (resourceClassNames.contains(poll)) { + foundParentSubResources.add(poll); + } + subClassesBySubResources.forEach((subResource, childClasses) -> { + if (childClasses.contains(poll)) { + foundParentSubResources.add(subResource); + } + }); + + if (!foundParentSubResources.isEmpty()) { + toScan.add(index.getClassByName(poll)); + } + + if (!foundParentSubResources.contains(poll)) { + // might be an extending interface, which itself is not a subresource locator + // It will get indexed, but it does not contain any further links to other subresources + continue; + } + + Set methodReturnTypes = new HashSet<>(); + for (DotName dotName : foundParentSubResources) { + if (declaringClassToReturnNames.containsKey(dotName)) { + methodReturnTypes.addAll(declaringClassToReturnNames.get(dotName)); + } + } + + for (DotName methodReturnType : methodReturnTypes) { + Set decls = childs.computeIfAbsent(methodReturnType, dotName -> { + if (dotName == null) { + return Collections.emptySet(); + } + + Set all = new HashSet<>(); + if (dotName.equals(DotName.createSimple(Object.class.getName()))) { + all.addAll(declaringClassToReturnNames.keySet()); + for (DotName name : declaringClassToReturnNames.keySet()) { + //we need to also look for all subclasses and interfaces + //they may have type variables that need to be handled + index.getAllKnownSubclasses(name).forEach(c2 -> all.add(c2.name())); + index.getAllKnownSubinterfaces(name).forEach(c2 -> all.add(c2.name())); + index.getAllKnownImplementors(name).forEach(c2 -> all.add(c2.name())); + } + } else { + // index the returntype, might already be a sub resource locator + all.add(dotName); + + //we need to also look for all subclasses and interfaces + //they may have type variables that need to be handled + index.getAllKnownSubclasses(dotName).forEach(c2 -> all.add(c2.name())); + index.getAllKnownSubinterfaces(dotName).forEach(c2 -> all.add(c2.name())); + index.getAllKnownImplementors(dotName).forEach(c2 -> all.add(c2.name())); + } + + return all; + }); + declQueue.addAll(decls); } } + Map possibleSubResources = new HashMap<>(); - Set resourceClassNames = null; while (!toScan.isEmpty()) { ClassInfo classInfo = toScan.poll(); if (scannedResources.containsKey(classInfo.name()) || @@ -760,32 +862,10 @@ public Supplier apply(ClassInfo classInfo) { } possibleSubResources.put(classInfo.name(), classInfo); - if (classInfo.isInterface()) { - int resourceClassImplCount = 0; - if (resourceClassNames == null) { - resourceClassNames = resourceClasses.stream().map(ResourceClass::getClassName) - .collect(Collectors.toSet()); - } - for (ClassInfo impl : index.getAllKnownImplementors(classInfo.name())) { - if (resourceClassNames.contains(impl.name().toString())) { - resourceClassImplCount++; - } - } - if (resourceClassImplCount > 1) { - // this is the case were an interface doesn't denote a subresource, but it's simply used - // to share method and annotations between Resource classes - continue; - } - } - Optional endpoints = serverEndpointIndexer.createEndpoints(classInfo, false); if (endpoints.isPresent()) { subResourceClasses.add(endpoints.get()); } - //we need to also look for all subclasses and interfaces - //they may have type variables that need to be handled - toScan.addAll(index.getKnownDirectImplementors(classInfo.name())); - toScan.addAll(index.getKnownDirectSubclasses(classInfo.name())); } setupEndpointsResultProducer.produce(new SetupEndpointsResultBuildItem(resourceClasses, subResourceClasses, diff --git a/extensions/resteasy-reactive/rest/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/resource/basic/SubResourceInterfaceAndClientInterfaceTest.java b/extensions/resteasy-reactive/rest/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/resource/basic/SubResourceInterfaceAndClientInterfaceTest.java new file mode 100644 index 00000000000000..b805d7ff63449e --- /dev/null +++ b/extensions/resteasy-reactive/rest/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/resource/basic/SubResourceInterfaceAndClientInterfaceTest.java @@ -0,0 +1,198 @@ +package io.quarkus.resteasy.reactive.server.test.resource.basic; + +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; +import java.util.List; +import java.util.function.Consumer; +import java.util.function.Supplier; + +import jakarta.ws.rs.GET; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.client.Client; +import jakarta.ws.rs.client.ClientBuilder; +import jakarta.ws.rs.core.Response; + +import org.jboss.resteasy.reactive.RestPath; +import org.jboss.resteasy.reactive.common.model.ResourceClass; +import org.jboss.shrinkwrap.api.ShrinkWrap; +import org.jboss.shrinkwrap.api.spec.JavaArchive; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.builder.BuildChainBuilder; +import io.quarkus.builder.BuildContext; +import io.quarkus.builder.BuildStep; +import io.quarkus.deployment.builditem.FeatureBuildItem; +import io.quarkus.resteasy.reactive.server.deployment.SetupEndpointsResultBuildItem; +import io.quarkus.resteasy.reactive.server.test.simple.PortProviderUtil; +import io.quarkus.test.QuarkusUnitTest; + +public class SubResourceInterfaceAndClientInterfaceTest { + + @RegisterExtension + static QuarkusUnitTest testExtension = new QuarkusUnitTest() + .setArchiveProducer(new Supplier<>() { + @Override + public JavaArchive get() { + JavaArchive war = ShrinkWrap.create(JavaArchive.class); + war.addClasses(PortProviderUtil.class); + war.addClasses(StoreResource.class); + war.addClasses(OrderResource.class); + war.addClasses(PositionResource.class); + war.addClasses(PositionResourceImpl.class); + war.addClasses(UndangerousGoodsResource.class); + war.addClasses(DangerousGoodsResource.class); + war.addClasses(VeryDangerousGoodsResource.class); + war.addClasses(SubResourceRestClientInterface.class); + war.addClasses(ContactResource.class); + war.addClasses(ContactResourceImpl.class); + return war; + } + }) + .addBuildChainCustomizer(new Consumer() { + @Override + public void accept(BuildChainBuilder buildChainBuilder) { + buildChainBuilder.addBuildStep(new BuildStep() { + @Override + public void execute(BuildContext context) { + SetupEndpointsResultBuildItem consumed = context.consume(SetupEndpointsResultBuildItem.class); + context.produce(new FeatureBuildItem("just-here-to-invoke-buildstep")); + + for (ResourceClass subResourceClass : consumed.getSubResourceClasses()) { + if (subResourceClass.getClassName().contains("SubResourceRestClientInterface")) { + throw new IllegalStateException( + "Client Interface SubResourceRestClientInterface got endpoint indexed."); + } + } + } + }).consumes(SetupEndpointsResultBuildItem.class).produces(FeatureBuildItem.class).build(); + } + }); + + @Test + public void basicTest() { + { + Client client = ClientBuilder.newClient(); + Response response = client.target( + PortProviderUtil.generateURL( + "/store/orders/orderId/positions/positionId/dangerousgoods/dangerousgoodId/some-field", + SubResourceInterfaceAndClientInterfaceTest.class.getSimpleName())) + .request().get(); + Assertions.assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); + Assertions.assertEquals("someFielddangerousgoodId", response.readEntity(String.class), "Wrong content of response"); + response.close(); + client.close(); + } + + { + Client client = ClientBuilder.newClient(); + Response response = client.target( + PortProviderUtil.generateURL( + "/store/orders/orderId/contacts", + SubResourceInterfaceAndClientInterfaceTest.class.getSimpleName())) + .request().get(); + Assertions.assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); + Assertions.assertEquals("[name1, name2]", response.readEntity(String.class), "Wrong content of response"); + response.close(); + client.close(); + } + } + + @Path("store") + public static class StoreResource { + @Path("orders/{id}") + public OrderResource get(@RestPath String id) { + + return new OrderResource() { + + @Override + public PositionResource get(String id) { + return new PositionResourceImpl(id); + } + + @Override + public Class contacts() { + return (Class) (Object) ContactResourceImpl.class; + } + }; + } + + @Path("user-count") + public Long getUserCount() { + return 4L; + } + } + + public interface OrderResource { + @Path("positions/{id}") + PositionResource get(@RestPath String id); + + @Path("contacts") + Class contacts(); + } + + public interface ContactResource { + @GET + List getContactNames(); + } + + public static class ContactResourceImpl implements ContactResource { + + @Override + public List getContactNames() { + return List.of("name1", "name2"); + } + } + + public interface PositionResource { + @Path("dangerousgoods/{id}") + UndangerousGoodsResource get(@RestPath String id); + } + + public static class PositionResourceImpl implements PositionResource { + + private final String id; + + public PositionResourceImpl(String id) { + this.id = id; + } + + @Override + public UndangerousGoodsResource get(String id) { + InvocationHandler handler = new InvocationHandler() { + @Override + public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { + if (method.getName().equals("getSomeField")) { + return "someField" + id; + } + return null; + } + }; + Class[] intfs = { VeryDangerousGoodsResource.class }; + return (VeryDangerousGoodsResource) Proxy.newProxyInstance(PositionResourceImpl.class.getClassLoader(), intfs, + handler); + } + } + + public interface UndangerousGoodsResource { + // not even dangerous enough to get a resource method + } + + public interface DangerousGoodsResource extends UndangerousGoodsResource { + @GET + String get(); + } + + public interface VeryDangerousGoodsResource extends DangerousGoodsResource { + @GET + @Path("some-field") + String getSomeField(); + } + + public interface SubResourceRestClientInterface { + @GET + String getSomeField(); + } +}