From 32d3949108bad641999029078a880175c3ebbdfb Mon Sep 17 00:00:00 2001 From: Andriy Redko Date: Sun, 29 Aug 2021 13:47:59 -0400 Subject: [PATCH] CXF-8578: Bridge methods for covariant return types cannot be invoked on client proxies --- .../apache/cxf/jaxrs/utils/ResourceUtils.java | 22 +++- .../cxf/jaxrs/utils/ResourceUtilsTest.java | 104 ++++++++++++++++++ .../client/JAXRSClientFactoryBeanTest.java | 16 +++ .../apache/cxf/jaxrs/resources/SuperBook.java | 44 ++++++++ .../cxf/jaxrs/resources/SuperBookStore.java | 74 +++++++++++++ 5 files changed, 255 insertions(+), 5 deletions(-) create mode 100644 rt/rs/client/src/test/java/org/apache/cxf/jaxrs/resources/SuperBook.java create mode 100644 rt/rs/client/src/test/java/org/apache/cxf/jaxrs/resources/SuperBookStore.java diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/ResourceUtils.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/ResourceUtils.java index fb53bd6f6fd..775683b104f 100644 --- a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/ResourceUtils.java +++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/ResourceUtils.java @@ -322,15 +322,15 @@ private static void evaluateResourceClass(ClassResourceInfo cri, boolean enableS MethodDispatcher md = new MethodDispatcher(); Class serviceClass = cri.getServiceClass(); - final Set annotatedMethods = new HashSet<>(); + final Map annotatedMethods = new HashMap<>(); for (Method m : serviceClass.getMethods()) { if (!m.isBridge() && !m.isSynthetic()) { //do real methods first Method annotatedMethod = AnnotationUtils.getAnnotatedMethod(serviceClass, m); - if (!annotatedMethods.contains(annotatedMethod)) { + if (!annotatedMethods.containsKey(annotatedMethod)) { evaluateResourceMethod(cri, enableStatic, md, m, annotatedMethod); - annotatedMethods.add(annotatedMethod); + annotatedMethods.put(annotatedMethod, m); } } } @@ -338,14 +338,26 @@ private static void evaluateResourceClass(ClassResourceInfo cri, boolean enableS if (m.isBridge() || m.isSynthetic()) { //if a bridge/synthetic method isn't already mapped to something, go ahead and do it Method annotatedMethod = AnnotationUtils.getAnnotatedMethod(serviceClass, m); - if (!annotatedMethods.contains(annotatedMethod)) { + if (!annotatedMethods.containsKey(annotatedMethod)) { evaluateResourceMethod(cri, enableStatic, md, m, annotatedMethod); - annotatedMethods.add(annotatedMethod); + annotatedMethods.put(annotatedMethod, m); + } else { + // Certain synthetic / bridge methods could be quite useful to handle + // methods with co-variant return types, especially when used with client proxies, + // see please: https://blogs.oracle.com/sundararajan/covariant-return-types-in-java + bindResourceMethod(md, m, annotatedMethods.get(annotatedMethod)); } } } cri.setMethodDispatcher(md); } + + private static void bindResourceMethod(MethodDispatcher md, Method m, Method bound) { + final OperationResourceInfo ori = md.getOperationResourceInfo(bound); + if (ori != null && !ori.getMethodToInvoke().equals(m)) { + md.bind(ori, bound, m); + } + } private static void evaluateResourceMethod(ClassResourceInfo cri, boolean enableStatic, MethodDispatcher md, Method m, Method annotatedMethod) { diff --git a/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/utils/ResourceUtilsTest.java b/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/utils/ResourceUtilsTest.java index e470f44af88..f3e89b3f9a6 100644 --- a/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/utils/ResourceUtilsTest.java +++ b/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/utils/ResourceUtilsTest.java @@ -21,6 +21,7 @@ import java.lang.reflect.Constructor; import java.lang.reflect.Method; import java.lang.reflect.Type; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -48,12 +49,14 @@ import org.apache.cxf.jaxrs.resources.Book; import org.apache.cxf.jaxrs.resources.BookInterface; import org.apache.cxf.jaxrs.resources.Chapter; +import org.apache.cxf.jaxrs.resources.SuperBook; import org.junit.Test; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; public class ResourceUtilsTest { @@ -167,6 +170,76 @@ public void testClassResourceInfoWithOverride() throws Exception { assertNotNull(ori); assertEquals("GET", ori.getHttpMethod()); } + + @Test + public void testClassResourceInfoWithBridgeMethod() throws Exception { + ClassResourceInfo cri = + ResourceUtils.createClassResourceInfo(ExampleBridgeImpl.class, ExampleBridgeImpl.class, true, true); + assertNotNull(cri); + assertEquals(1, cri.getMethodDispatcher().getOperationResourceInfos().size()); + + Method m = ExampleBridgeImpl.class.getMethod("get"); + OperationResourceInfo ori = cri.getMethodDispatcher().getOperationResourceInfo(m); + assertNotNull(ori); + assertEquals("GET", ori.getHttpMethod()); + + m = Arrays + .stream(ExampleBridgeImpl.class.getMethods()) + .filter(method -> method.getName().equals("get")) + .filter(Method::isBridge) + .findAny() + .orElse(null); + + ori = cri.getMethodDispatcher().getOperationResourceInfo(m); + assertNotNull(ori); + assertEquals("GET", ori.getHttpMethod()); + } + + @Test + public void testGenericClassResourceInfoWithBridgeMethod() throws Exception { + ClassResourceInfo cri = ResourceUtils.createClassResourceInfo(GenericExampleBridgeImpl.class, + GenericExampleBridgeImpl.class, true, true); + assertNotNull(cri); + assertEquals(1, cri.getMethodDispatcher().getOperationResourceInfos().size()); + + Method m = GenericExampleBridgeImpl.class.getMethod("get"); + OperationResourceInfo ori = cri.getMethodDispatcher().getOperationResourceInfo(m); + assertNotNull(ori); + assertEquals("GET", ori.getHttpMethod()); + + m = Arrays + .stream(GenericExampleBridgeImpl.class.getMethods()) + .filter(method -> method.getName().equals("get")) + .filter(Method::isBridge) + .findAny() + .orElse(null); + + ori = cri.getMethodDispatcher().getOperationResourceInfo(m); + assertNotNull(ori); + assertEquals("GET", ori.getHttpMethod()); + } + + @Test + public void testGenericClassResourceInfo() throws Exception { + ClassResourceInfo cri = ResourceUtils.createClassResourceInfo(GenericExampleImpl.class, + GenericExampleImpl.class, true, true); + assertNotNull(cri); + assertEquals(1, cri.getMethodDispatcher().getOperationResourceInfos().size()); + + Method m = GenericExampleImpl.class.getMethod("get"); + OperationResourceInfo ori = cri.getMethodDispatcher().getOperationResourceInfo(m); + assertNotNull(ori); + assertEquals("GET", ori.getHttpMethod()); + + m = Arrays + .stream(GenericExampleImpl.class.getMethods()) + .filter(method -> method.getName().equals("get")) + .filter(Method::isBridge) + .findAny() + .orElse(null); + + assertNull(m); + } @Path("/synth-hello") protected interface SyntheticHelloInterface { @@ -301,6 +374,13 @@ public interface Example { @GET Book get(); } + + @Path("example") + public interface GenericExample { + + @GET + T get(); + } public static class ExampleImpl implements Example { @@ -309,6 +389,30 @@ public Book get() { return null; } } + + public static class ExampleBridgeImpl implements Example { + + @Override + public SuperBook get() { + return null; + } + } + + public static class GenericExampleImpl implements GenericExample { + + @Override + public Book get() { + return null; + } + } + + public static class GenericExampleBridgeImpl implements GenericExample { + + @Override + public SuperBook get() { + return null; + } + } @XmlRootElement public static class OrderItem { diff --git a/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/client/JAXRSClientFactoryBeanTest.java b/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/client/JAXRSClientFactoryBeanTest.java index 6089d5b4cb1..c705fcb4b50 100644 --- a/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/client/JAXRSClientFactoryBeanTest.java +++ b/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/client/JAXRSClientFactoryBeanTest.java @@ -44,6 +44,9 @@ import org.apache.cxf.jaxrs.resources.BookInterface; import org.apache.cxf.jaxrs.resources.BookStore; import org.apache.cxf.jaxrs.resources.BookStoreSubresourcesOnly; +import org.apache.cxf.jaxrs.resources.BookSuperClass; +import org.apache.cxf.jaxrs.resources.SuperBook; +import org.apache.cxf.jaxrs.resources.SuperBookStore; import org.apache.cxf.message.Message; import org.apache.cxf.phase.AbstractPhaseInterceptor; import org.apache.cxf.phase.Phase; @@ -203,6 +206,19 @@ public void testComplexProxy() throws Exception { IProductResource productResourceElement = parts.elementAt("1"); assertNotNull(productResourceElement); } + + @Test + public void testBookAndBridgeMethods() throws Exception { + SuperBookStore superBookResource = JAXRSClientFactory.create("http://localhost:9000", + SuperBookStore.class); + assertNotNull(superBookResource); + + Book book = ((BookSuperClass)superBookResource).getNewBook("id4", true); + assertNotNull(book); + + SuperBook superBook = (SuperBook)superBookResource.getNewBook("id4", true); + assertNotNull(superBook); + } @Test public void testVoidResponseAcceptWildcard() throws Exception { diff --git a/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/resources/SuperBook.java b/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/resources/SuperBook.java new file mode 100644 index 00000000000..79120aa9b70 --- /dev/null +++ b/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/resources/SuperBook.java @@ -0,0 +1,44 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.cxf.jaxrs.resources; + +import javax.xml.bind.annotation.XmlRootElement; + + +@XmlRootElement(name = "SuperBook") +public class SuperBook extends Book { + private long superId; + public SuperBook() { + } + + public SuperBook(String name, long id, long superId) { + super(name, id); + this.superId = superId; + } + + public void setSuperId(long i) { + superId = i; + } + + public long getSuperId() { + return superId; + } + +} diff --git a/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/resources/SuperBookStore.java b/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/resources/SuperBookStore.java new file mode 100644 index 00000000000..ffdd51cfbf2 --- /dev/null +++ b/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/resources/SuperBookStore.java @@ -0,0 +1,74 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.cxf.jaxrs.resources; + +import javax.ws.rs.Consumes; +import javax.ws.rs.DELETE; +import javax.ws.rs.POST; +import javax.ws.rs.PUT; +import javax.ws.rs.Path; +import javax.ws.rs.PathParam; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; + +@Path("/bookstore/") +public class SuperBookStore extends BookSuperClass implements BookInterface { + public SuperBookStore() { + } + + public SuperBook getBook(String id) { + return null; + } + + @Override + public SuperBook getNewBook(String id, Boolean isNew) { + return null; + } + + @POST + @Path("/books") + @Consumes(MediaType.APPLICATION_XML) + public void addBook(Book book) { + } + + @PUT + @Path("/books/") + public Response updateBook(SuperBook book) { + return null; + } + + @DELETE + @Path("/books/{bookId}/") + public Response deleteBook(@PathParam("bookId") String id) { + return null; + } + + @Override + public String getDescription() { + return null; + } + + public String getAuthor() { + return null; + } +} + +