You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by GitBox <gi...@apache.org> on 2017/12/20 12:07:41 UTC
[GitHub] johnament closed pull request #360: MP Rest Client interface validation and tests
johnament closed pull request #360: MP Rest Client interface validation and tests
URL: https://github.com/apache/cxf/pull/360
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/CxfTypeSafeClientBuilder.java b/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/CxfTypeSafeClientBuilder.java
index d8955867f53..2daf94479e4 100644
--- a/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/CxfTypeSafeClientBuilder.java
+++ b/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/CxfTypeSafeClientBuilder.java
@@ -42,6 +42,7 @@ public RestClientBuilder baseUrl(URL url) {
if (baseUri == null) {
throw new IllegalStateException("baseUrl not set");
}
+ Validator.checkValid(aClass);
RegisterProvider[] providers = aClass.getAnnotationsByType(RegisterProvider.class);
Configuration config = configImpl.getConfiguration();
if (providers != null) {
diff --git a/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/Messages.properties b/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/Messages.properties
new file mode 100644
index 00000000000..d2d53631d6d
--- /dev/null
+++ b/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/Messages.properties
@@ -0,0 +1,24 @@
+#
+#
+# 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.
+#
+#
+
+VALIDATION_NOT_AN_INTERFACE=The type, {0}, passed in to the build method is not an interface.
+VALIDATION_METHOD_WITH_MULTIPLE_VERBS=The client interface contains a method, {0} annotated with more than one HTTP method: {1}
+VALIDATION_UNRESOLVED_PATH_PARAMS=The client interface, {0} has one or more methods with unresolved path template variables: {1}
diff --git a/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/MicroProfileClientConfigurableImpl.java b/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/MicroProfileClientConfigurableImpl.java
index 3df42460770..97e8c7d81b3 100644
--- a/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/MicroProfileClientConfigurableImpl.java
+++ b/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/MicroProfileClientConfigurableImpl.java
@@ -27,10 +27,10 @@
import javax.ws.rs.ext.MessageBodyWriter;
import javax.ws.rs.ext.ReaderInterceptor;
import javax.ws.rs.ext.WriterInterceptor;
+
import org.apache.cxf.jaxrs.impl.ConfigurableImpl;
import org.apache.cxf.jaxrs.impl.ConfigurationImpl;
-import org.eclipse.microprofile.config.Config;
-import org.eclipse.microprofile.config.ConfigProvider;
+import org.apache.cxf.microprofile.client.config.ConfigFacade;
import org.eclipse.microprofile.rest.client.ext.ResponseExceptionMapper;
public class MicroProfileClientConfigurableImpl<C extends Configurable<C>>
@@ -55,10 +55,8 @@ boolean isDefaultExceptionMapperDisabled() {
Object prop = getConfiguration().getProperty(CONFIG_KEY_DISABLE_MAPPER);
if (prop instanceof Boolean) {
return (Boolean)prop;
- } else {
- Config config = ConfigProvider.getConfig();
- return config.getOptionalValue(CONFIG_KEY_DISABLE_MAPPER,
- Boolean.class).orElse(false);
}
+ return ConfigFacade.getOptionalValue(CONFIG_KEY_DISABLE_MAPPER,
+ Boolean.class).orElse(false);
}
}
diff --git a/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/Validator.java b/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/Validator.java
new file mode 100644
index 00000000000..8d77e0225c2
--- /dev/null
+++ b/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/Validator.java
@@ -0,0 +1,124 @@
+/**
+ * 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.microprofile.client;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Method;
+import java.lang.reflect.Parameter;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.ResourceBundle;
+import java.util.Set;
+
+import javax.ws.rs.HttpMethod;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+
+import org.apache.cxf.common.i18n.BundleUtils;
+import org.apache.cxf.common.i18n.Message;
+import org.apache.cxf.jaxrs.model.URITemplate;
+import org.eclipse.microprofile.rest.client.RestClientDefinitionException;
+
+final class Validator {
+ private static final ResourceBundle BUNDLE = BundleUtils.getBundle(Validator.class);
+
+ private Validator() {
+ }
+
+
+ public static void checkValid(Class<?> userType) throws RestClientDefinitionException {
+ if (!userType.isInterface()) {
+ throwException("VALIDATION_NOT_AN_INTERFACE", userType);
+ }
+ Method[] methods = userType.getMethods();
+ checkMethodsForMultipleHTTPMethodAnnotations(methods);
+ checkMethodsForInvalidURITemplates(userType, methods);
+ }
+
+ private static void checkMethodsForMultipleHTTPMethodAnnotations(Method[] clientMethods)
+ throws RestClientDefinitionException {
+
+ Map<String, Class<? extends Annotation>> httpMethods = new HashMap<>();
+ for (Method method : clientMethods) {
+
+ for (Annotation anno : method.getAnnotations()) {
+ Class<? extends Annotation> annoClass = anno.annotationType();
+ HttpMethod verb = annoClass.getAnnotation(HttpMethod.class);
+ if (verb != null) {
+ httpMethods.put(verb.value(), annoClass);
+ }
+ }
+ if (httpMethods.size() > 1) {
+ throwException("VALIDATION_METHOD_WITH_MULTIPLE_VERBS", method, httpMethods.values());
+ }
+ httpMethods.clear();
+ }
+
+ }
+
+ private static void checkMethodsForInvalidURITemplates(Class<?> userType, Method[] methods)
+ throws RestClientDefinitionException {
+
+ Path classPathAnno = userType.getAnnotation(Path.class);
+
+ final Set<String> classLevelVariables = new HashSet<>();
+ URITemplate classTemplate = null;
+ if (classPathAnno != null) {
+ classTemplate = new URITemplate(classPathAnno.value());
+ classLevelVariables.addAll(classTemplate.getVariables());
+ }
+ URITemplate template;
+ for (Method method : methods) {
+
+ Path methodPathAnno = method.getAnnotation(Path.class);
+ if (methodPathAnno != null) {
+ template = classPathAnno == null ? new URITemplate(methodPathAnno.value())
+ : new URITemplate(classPathAnno.value() + "/" + methodPathAnno.value());
+ } else {
+ template = classTemplate;
+ }
+ if (template == null) {
+ continue;
+ }
+ Set<String> allVariables = new HashSet<>(template.getVariables());
+ if (!allVariables.isEmpty()) {
+ Map<String, String> paramMap = new HashMap<>();
+ for (Parameter p : method.getParameters()) {
+ PathParam pathParam = p.getAnnotation(PathParam.class);
+ if (pathParam != null) {
+ paramMap.put(pathParam.value(), "x");
+ }
+ }
+ try {
+ template.substitute(paramMap, Collections.<String>emptySet(), false);
+ } catch (IllegalArgumentException ex) {
+ throwException("VALIDATION_UNRESOLVED_PATH_PARAMS", userType, method);
+ }
+ }
+ }
+ }
+
+ private static void throwException(String msgKey, Object...msgParams) throws RestClientDefinitionException {
+ Message msg = new Message(msgKey, BUNDLE, msgParams);
+ throw new RestClientDefinitionException(msg.toString());
+ }
+
+}
diff --git a/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/cdi/RestClientBean.java b/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/cdi/RestClientBean.java
index 7baec1c89f3..7524f0cec17 100644
--- a/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/cdi/RestClientBean.java
+++ b/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/cdi/RestClientBean.java
@@ -28,6 +28,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Set;
+
import javax.enterprise.context.Dependent;
import javax.enterprise.context.spi.CreationalContext;
import javax.enterprise.inject.Default;
@@ -36,9 +37,9 @@
import javax.enterprise.inject.spi.InjectionPoint;
import javax.enterprise.inject.spi.PassivationCapable;
import javax.enterprise.util.AnnotationLiteral;
+
import org.apache.cxf.microprofile.client.CxfTypeSafeClientBuilder;
-import org.eclipse.microprofile.config.Config;
-import org.eclipse.microprofile.config.ConfigProvider;
+import org.apache.cxf.microprofile.client.config.ConfigFacade;
import org.eclipse.microprofile.rest.client.inject.RestClient;
public class RestClientBean implements Bean<Object>, PassivationCapable {
@@ -48,12 +49,10 @@
private final Class<?> clientInterface;
private final Class<? extends Annotation> scope;
private final BeanManager beanManager;
- private final Config config;
RestClientBean(Class<?> clientInterface, BeanManager beanManager) {
this.clientInterface = clientInterface;
this.beanManager = beanManager;
- this.config = ConfigProvider.getConfig();
this.scope = this.readScope();
}
@Override
@@ -124,16 +123,20 @@ public boolean isAlternative() {
private String getBaseUrl() {
String property = String.format(REST_URL_FORMAT, clientInterface.getName());
- return config.getValue(property, String.class);
+ String baseURL = ConfigFacade.getValue(property, String.class);
+ if (baseURL == null) {
+ throw new IllegalStateException("Unable to determine base URL from configuration");
+ }
+ return baseURL;
}
private Class<? extends Annotation> readScope() {
// first check to see if the value is set
String property = String.format(REST_SCOPE_FORMAT, clientInterface.getName());
- String configuredScope = config.getOptionalValue(property, String.class).orElse(null);
+ String configuredScope = ConfigFacade.getOptionalValue(property, String.class).orElse(null);
if (configuredScope != null) {
try {
- return (Class<? extends Annotation>)Class.forName(configuredScope);
+ return (Class<? extends Annotation>) Class.forName(configuredScope);
} catch (Exception e) {
throw new IllegalArgumentException("The scope " + configuredScope + " is invalid", e);
}
diff --git a/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/config/ConfigFacade.java b/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/config/ConfigFacade.java
new file mode 100644
index 00000000000..e543d7c0cfb
--- /dev/null
+++ b/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/config/ConfigFacade.java
@@ -0,0 +1,53 @@
+/**
+ * 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.microprofile.client.config;
+
+import java.util.Optional;
+
+import org.eclipse.microprofile.config.Config;
+import org.eclipse.microprofile.config.ConfigProvider;
+
+public final class ConfigFacade {
+
+ private ConfigFacade() {
+ }
+
+ private static Optional<Config> config() {
+ Config c;
+ try {
+ c = ConfigProvider.getConfig();
+ } catch (ExceptionInInitializerError | NoClassDefFoundError | IllegalStateException ex) {
+ // expected if no MP Config implementation is available
+ c = null;
+ }
+ return Optional.ofNullable(c);
+ }
+
+ public static <T> Optional<T> getOptionalValue(String propertyName, Class<T> clazz) {
+ Optional<Config> c = config();
+ return c.isPresent() ? c.get().getOptionalValue(propertyName, clazz) : Optional.empty();
+ }
+
+ public static <T> T getValue(String propertyName, Class<T> clazz) {
+ Optional<Config> c = config();
+ return c.isPresent() ? c.get().getValue(propertyName, clazz) : null;
+ }
+
+}
diff --git a/rt/rs/microprofile-client/src/test/java/org/apache/cxf/microprofile/client/ValidatorTest.java b/rt/rs/microprofile-client/src/test/java/org/apache/cxf/microprofile/client/ValidatorTest.java
new file mode 100644
index 00000000000..7877574a436
--- /dev/null
+++ b/rt/rs/microprofile-client/src/test/java/org/apache/cxf/microprofile/client/ValidatorTest.java
@@ -0,0 +1,134 @@
+/**
+ * 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.microprofile.client;
+
+import java.net.MalformedURLException;
+import java.net.URL;
+
+import javax.ws.rs.DELETE;
+import javax.ws.rs.GET;
+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.Response;
+
+import org.eclipse.microprofile.rest.client.RestClientBuilder;
+import org.eclipse.microprofile.rest.client.RestClientDefinitionException;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class ValidatorTest extends Assert {
+
+ public abstract static class NotAnInterface {
+ @GET
+ public abstract Response get();
+ }
+
+ public interface MultiVerbMethod {
+ @GET
+ Response get();
+ @PUT
+ Response put(String x);
+ @POST
+ @DELETE
+ Response postAndDelete();
+ }
+
+ @Path("/rest/{class}")
+ public interface UnresolvedClassUriTemplate {
+ @GET
+ Response myUnresolvedMethod();
+ }
+
+ @Path("/rest")
+ public interface UnresolvedMethodUriTemplate {
+ @Path("/{method}")
+ @GET
+ Response myOtherUnresolvedMethod();
+ }
+
+ @Path("/rest/{class}")
+ public interface PartiallyResolvedUriTemplate {
+ @GET
+ Response get(@PathParam("class")String className);
+
+ @PUT
+ @Path("/{method}")
+ Response put(@PathParam("method")String methodName);
+ }
+
+ @Path("/rest/{class}")
+ public interface PartiallyResolvedUriTemplate2 {
+ @DELETE
+ Response delete(@PathParam("class")String className);
+
+ @POST
+ @Path("/{method}")
+ Response post(@PathParam("class")String className);
+ }
+
+ private static RestClientBuilder newBuilder() {
+ RestClientBuilder builder = RestClientBuilder.newBuilder();
+ try {
+ builder = builder.baseUrl(new URL("http://localhost:8080/test"));
+ } catch (MalformedURLException e) {
+ fail("MalformedURL - bad testcase");
+ }
+ return builder;
+ }
+
+ @Test
+ public void testNotAnInterface() {
+ test(NotAnInterface.class, "is not an interface", "NotAnInterface");
+ }
+
+ @Test
+ public void testMethodWithMultipleVerbs() {
+ test(MultiVerbMethod.class, "more than one HTTP method", "postAndDelete", "javax.ws.rs.POST",
+ "javax.ws.rs.DELETE");
+ }
+
+ @Test
+ public void testUnresolvedUriTemplates() {
+ test(UnresolvedClassUriTemplate.class, "unresolved path template variables", "UnresolvedClassUriTemplate",
+ "myUnresolvedMethod");
+ test(UnresolvedMethodUriTemplate.class, "unresolved path template variables", "UnresolvedMethodUriTemplate",
+ "myOtherUnresolvedMethod");
+ test(PartiallyResolvedUriTemplate.class, "unresolved path template variables", "PartiallyResolvedUriTemplate",
+ "put");
+ test(PartiallyResolvedUriTemplate2.class, "unresolved path template variables", "PartiallyResolvedUriTemplate2",
+ "post");
+ }
+
+ private void test(Class<?> clientInterface, String...expectedMessageTexts) {
+ try {
+ newBuilder().build(clientInterface);
+ fail("Expected RestClientDefinitionException");
+ } catch (RestClientDefinitionException ex) {
+ String msgText = ex.getMessage();
+ assertNotNull("No message text in RestClientDefinitionException", msgText);
+ for (String expectedMessageText : expectedMessageTexts) {
+ assertTrue("Exception text does not contain expected message: " + expectedMessageText,
+ msgText.contains(expectedMessageText));
+ }
+ }
+ }
+}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services