Merge pull request payara#20 from pdudits/mprc-hotfix

PAYARA-3859: Fix MP Rest client return type handling
pdudits · May 24, 2019 · 0cd9b34 · 0cd9b34
2 parents 86cb030 + f7906d0
commit 0cd9b34
Show file tree

Hide file tree

Showing 10 changed files with 168 additions and 52 deletions.
diff --git a/bom/pom.xml b/bom/pom.xml
@@ -250,7 +250,7 @@
                 <version>${project.version}</version>
             </dependency>
             <dependency>
-                <groupId>org.glassfish.jersey.ext</groupId>
+                <groupId>org.glassfish.jersey.ext.microprofile</groupId>
                 <artifactId>jersey-mp-rest-client</artifactId>
                 <version>${project.version}</version>
             </dependency>

diff --git a/ext/microprofile/mp-rest-client/pom.xml b/ext/microprofile/mp-rest-client/pom.xml
@@ -20,8 +20,8 @@
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
     <parent>
-        <artifactId>jersey-microprofile</artifactId>
-        <groupId>org.glassfish.jersey.ext</groupId>
+        <artifactId>project</artifactId>
+        <groupId>org.glassfish.jersey.ext.microprofile</groupId>
         <version>2.29.payara-p3-SNAPSHOT</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
@@ -146,16 +146,26 @@
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-surefire-plugin</artifactId>
-                <configuration>
-                    <!--<parallel>classes</parallel>
-                    <perCoreThreadCount>true</perCoreThreadCount>
-                    <threadCount>1</threadCount>
-                    <forkCount>1C</forkCount>
-                    <reuseForks>true</reuseForks>-->
-                    <suiteXmlFiles>
-                        <suiteXmlFile>tck-suite.xml</suiteXmlFile>
-                    </suiteXmlFiles>
-                </configuration>
+                <executions>
+                    <!-- run junit tests in default-test, tck in extra execution -->
+                    <execution>
+                        <id>tck</id>
+                        <goals>
+                            <goal>test</goal>
+                        </goals>
+                        <phase>test</phase>
+                        <configuration>
+                            <!--<parallel>classes</parallel>
+                            <perCoreThreadCount>true</perCoreThreadCount>
+                            <threadCount>1</threadCount>
+                            <forkCount>1C</forkCount>
+                            <reuseForks>true</reuseForks>-->
+                            <suiteXmlFiles>
+                                <suiteXmlFile>tck-suite.xml</suiteXmlFile>
+                            </suiteXmlFiles>
+                        </configuration>
+                    </execution>
+                </executions>
             </plugin>
             <plugin>
                 <groupId>uk.co.deliverymind</groupId>

diff --git a/...rest-client/src/main/java/org/glassfish/jersey/microprofile/restclient/ConfigWrapper.java b/...rest-client/src/main/java/org/glassfish/jersey/microprofile/restclient/ConfigWrapper.java
@@ -1,3 +1,19 @@
+/*
+ * Copyright (c) 2019 Oracle and/or its affiliates. All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0, which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the
+ * Eclipse Public License v. 2.0 are satisfied: GNU General Public License,
+ * version 2 with the GNU Classpath Exception, which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+ */
+
 package org.glassfish.jersey.microprofile.restclient;
 
 import java.util.Collection;

diff --git a/...nt/src/main/java/org/glassfish/jersey/microprofile/restclient/ExecutorServiceWrapper.java b/...nt/src/main/java/org/glassfish/jersey/microprofile/restclient/ExecutorServiceWrapper.java
@@ -1,3 +1,19 @@
+/*
+ * Copyright (c) 2019 Oracle and/or its affiliates. All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0, which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the
+ * Eclipse Public License v. 2.0 are satisfied: GNU General Public License,
+ * version 2 with the GNU Classpath Exception, which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+ */
+
 package org.glassfish.jersey.microprofile.restclient;
 
 import java.util.Collection;

diff --git a/...p-rest-client/src/main/java/org/glassfish/jersey/microprofile/restclient/MethodModel.java b/...p-rest-client/src/main/java/org/glassfish/jersey/microprofile/restclient/MethodModel.java
@@ -83,7 +83,6 @@ class MethodModel {
     private final InterfaceModel interfaceModel;
 
     private final Method method;
-    private final Class<?> returnType;
     private final String httpMethod;
     private final String path;
     private final String[] produces;
@@ -102,7 +101,6 @@ class MethodModel {
      */
     static MethodModel from(InterfaceModel interfaceModel, Method method) {
         return new Builder(interfaceModel, method)
-                .returnType(method.getGenericReturnType())
                 .httpMethod(parseHttpMethod(interfaceModel, method))
                 .pathValue(method.getAnnotation(Path.class))
                 .produces(method.getAnnotation(Produces.class))
@@ -115,7 +113,6 @@ static MethodModel from(InterfaceModel interfaceModel, Method method) {
     private MethodModel(Builder builder) {
         this.method = builder.method;
         this.interfaceModel = builder.interfaceModel;
-        this.returnType = builder.returnType;
         this.httpMethod = builder.httpMethod;
         this.path = builder.pathValue;
         this.produces = builder.produces;
@@ -124,12 +121,18 @@ private MethodModel(Builder builder) {
         this.clientHeaders = builder.clientHeaders;
         this.invocationInterceptors = builder.invocationInterceptors;
         if (httpMethod.isEmpty()) {
-            subResourceModel = RestClientModel.from(returnType,
-                                                    interfaceModel.getResponseExceptionMappers(),
-                                                    interfaceModel.getParamConverterProviders(),
-                                                    interfaceModel.getAsyncInterceptors(),
-                                                    interfaceModel.getInjectionManager(),
-                                                    interfaceModel.getBeanManager());
+            Type returnType = method.getGenericReturnType();
+            if (returnType instanceof Class) {
+                subResourceModel = RestClientModel.from((Class<?>) returnType,
+                        interfaceModel.getResponseExceptionMappers(),
+                        interfaceModel.getParamConverterProviders(),
+                        interfaceModel.getAsyncInterceptors(),
+                        interfaceModel.getInjectionManager(),
+                        interfaceModel.getBeanManager());
+            } else {
+                throw new IllegalArgumentException("Subresource represented by method " + method.getName()
+                        + " must be non-generic class");
+            }
         } else {
             subResourceModel = null;
         }
@@ -177,7 +180,7 @@ Object invokeMethod(WebTarget classLevelTarget, Method method, Object[] args) {
         WebTarget webTarget = webTargetAtomicReference.get();
         if (httpMethod.isEmpty()) {
             //sub resource method
-            return subResourceProxy(webTarget, returnType);
+            return subResourceProxy(webTarget);
         }
         webTarget = addQueryParams(webTarget, args);
         webTarget = addMatrixParams(webTarget, args);
@@ -211,12 +214,12 @@ private Object synchronousCall(Invocation.Builder builder, Object entity, Method
 
         evaluateResponse(response, method);
 
-        if (returnType.equals(Void.class)) {
+        if (method.getReturnType().equals(Void.class)) {
             return null;
-        } else if (returnType.equals(Response.class)) {
+        } else if (method.getReturnType().equals(Response.class)) {
             return response;
         }
-        return response.readEntity(returnType);
+        return response.readEntity(new GenericType<>(method.getGenericReturnType()));
     }
 
     private CompletableFuture asynchronousCall(Invocation.Builder builder, Object entity, Method method) {
@@ -237,9 +240,9 @@ private CompletableFuture asynchronousCall(Invocation.Builder builder, Object en
             interfaceModel.getAsyncInterceptors().forEach(AsyncInvocationInterceptor::removeContext);
             try {
                 evaluateResponse(response, method);
-                if (returnType.equals(Void.class)) {
+                if (actualTypeArgument.equals(Void.class)) {
                     result.complete(null);
-                } else if (returnType.equals(Response.class)) {
+                } else if (actualTypeArgument.equals(Response.class)) {
                     result.complete(response);
                 } else {
                     result.complete(response.readEntity(new GenericType<>(actualTypeArgument)));
@@ -257,9 +260,10 @@ private CompletableFuture asynchronousCall(Invocation.Builder builder, Object en
     }
 
     @SuppressWarnings("unchecked")
-    private <T> T subResourceProxy(WebTarget webTarget, Class<T> subResourceType) {
-        return (T) Proxy.newProxyInstance(subResourceType.getClassLoader(),
-                                          new Class[] {subResourceType},
+    private <T> T subResourceProxy(WebTarget webTarget) {
+        Class<?> subResourceClass = subResourceModel.getRestClientClass();
+        return (T) Proxy.newProxyInstance(subResourceClass.getClassLoader(),
+                                          new Class[] {subResourceClass},
                                           new ProxyInvocationHandler(webTarget, subResourceModel)
         );
     }
@@ -460,7 +464,6 @@ private static class Builder {
         private final InterfaceModel interfaceModel;
         private final Method method;
 
-        private Class<?> returnType;
         private String httpMethod;
         private String pathValue;
         private String[] produces;
@@ -505,21 +508,6 @@ private void filterAllInterceptorAnnotations() {
             }
         }
 
-        /**
-         * Return type of the method.
-         *
-         * @param returnType Method return type
-         * @return updated Builder instance
-         */
-        Builder returnType(Type returnType) {
-            if (returnType instanceof ParameterizedType) {
-                this.returnType = (Class<?>) ((ParameterizedType) returnType).getActualTypeArguments()[0];
-            } else {
-                this.returnType = (Class<?>) returnType;
-            }
-            return this;
-        }
-
         /**
          * HTTP method of the method.
          *
@@ -603,7 +591,7 @@ MethodModel build() {
             Optional<ParamModel> entity = parameterModels.stream()
                     .filter(ParamModel::isEntity)
                     .findFirst();
-            if (JsonValue.class.isAssignableFrom(returnType)
+            if (JsonValue.class.isAssignableFrom(method.getReturnType())
                     || (
                     entity.isPresent() && entity.get().getType() instanceof Class
                             && JsonValue.class.isAssignableFrom((Class<?>) entity.get().getType()))) {

diff --git a/...st-client/src/main/java/org/glassfish/jersey/microprofile/restclient/RestClientModel.java b/...st-client/src/main/java/org/glassfish/jersey/microprofile/restclient/RestClientModel.java
@@ -102,6 +102,10 @@ <T> Object invokeMethod(WebTarget baseWebTarget, Method method, Object[] args) {
         }
     }
 
+    Class<?> getRestClientClass() {
+        return interfaceModel.getRestClientClass();
+    }
+
     private static Map<Method, MethodModel> parseMethodModels(InterfaceModel classModel) {
         Map<Method, MethodModel> methodMap = new HashMap<>();
         for (Method method : classModel.getRestClientClass().getMethods()) {

diff --git a/...lient/src/test/java/org/glassfish/jersey/microprofile/restclient/ApplicationResource.java b/...lient/src/test/java/org/glassfish/jersey/microprofile/restclient/ApplicationResource.java
@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2019 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019 Payara Foundation and/or its affiliates. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v. 2.0, which is available at
@@ -16,12 +17,19 @@
 
 package org.glassfish.jersey.microprofile.restclient;
 
+import javax.ws.rs.Consumes;
 import javax.ws.rs.GET;
 import javax.ws.rs.POST;
 import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.core.MediaType;
+import java.util.List;
+import java.util.Map;
 
 /**
- * Created by David Kral.
+ * @author David Kral
+ * @author Patrik Dudits
  */
 
 @Path("resource")
@@ -33,6 +41,20 @@ public interface ApplicationResource {
     @POST
     String postAppendValue(String value);
 
+    @Path("list/{size}")
+    @GET
+    List<String> list(@PathParam("size") int size);
+
+    @Path("intmap/{size}")
+    @GET
+    Map<String, Integer> map(@PathParam("size") int size);
+
+    @Path("list")
+    @POST
+    @Consumes(MediaType.APPLICATION_JSON)
+    @Produces(MediaType.APPLICATION_JSON)
+    Map<String, Integer> acceptList(List<Integer> numbers);
+
     default String sayHi() {
         return "Hi";
     }

diff --git a/...t/src/test/java/org/glassfish/jersey/microprofile/restclient/ApplicationResourceImpl.java b/...t/src/test/java/org/glassfish/jersey/microprofile/restclient/ApplicationResourceImpl.java
@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2019 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019 Payara Foundation and/or its affiliates. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v. 2.0, which is available at
@@ -16,8 +17,16 @@
 
 package org.glassfish.jersey.microprofile.restclient;
 
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+
 /**
- * Created by David Kral.
+ * @author David Kral
+ * @author Patrik Dudits
  */
 public class ApplicationResourceImpl implements ApplicationResource {
     @Override
@@ -29,4 +38,21 @@ public String getValue() {
     public String postAppendValue(String value) {
         return null;
     }
+
+    @Override
+    public List<String> list(int size) {
+        return IntStream.rangeClosed(1, size).mapToObj(i -> "int" + i).collect(Collectors.toList());
+    }
+
+    @Override
+    public Map<String, Integer> map(int size) {
+        return IntStream.rangeClosed(1, size).mapToObj(i -> i).collect(Collectors.toMap(String::valueOf, Function.identity()));
+    }
+
+    @Override
+    public Map<String, Integer> acceptList(List<Integer> numbers) {
+        Map<String, Integer> result = new HashMap<>();
+        result.put("size", numbers != null ? numbers.size() : -1);
+        return result;
+    }
 }