Improves compatibility with Resteasy though ContextResolvers #89
Conversation
| Client client = builder | ||
| .register(provider) | ||
| if (!builder.getConfiguration().getClasses().stream() | ||
| .filter(c -> JacksonJaxbJsonProvider.class.isAssignableFrom(c)).findAny().isPresent()) { |
There was a problem hiding this comment.
I'm a little new to Gradle, couldn't figure out if you were using Java 8 or not. Guessing you probably aren't. Could rewrite this in Java 6/7 form if needed
There was a problem hiding this comment.
Yeah, we're using 7 because some of our applications at Orbitz are stuck on 7.
|
Once you make it Java 7 compatible, I will merge. Thanks! We're going to need Vault support soon, so I'll likely start implementing the Vault client as well. :) |
jhovell
force-pushed
the
resteasy-compatibility
branch
from
5e4726f
to
eb93574
Compare
|
@rickfast Java 7 now (using Guava functional interfaces). Cool, I am also in the process of investigating vault. If you're looking for any help, let me know! |
Improves compatibility with Resteasy though ContextResolvers
|
Did you run the tests locally? Build failed. Just run a Consul Agent on localhost and the tests should work |
|
@rickfast I did. Even before my change 4 tests were failing so I wasn't sure what to make of it. I'll take another look at it. I wonder if using a package like wire mock could eliminate the need for a running consul server. |
|
Is it possible to hook up your codeship integration for pull requests as well? Might give us a good common point of reference. Something like this http://www.codeaffine.com/2014/10/06/codeship-continuous-integration/ |
|
@rickfast locally 4 tests fail for me before and after the change. I am not sure what is different about the codeship CI environment. Clicking on the codeship failed build link (https://codeship.com/projects/88244) just forwards to https://codeship.com/projects/ so many be I don't have permission to see the build? Local test failures (identical before and after my change) consul info: java info: gradle info: |
|
I'll check it out this evening. |
|
Can you try with Java 7? |
|
@rickfast how about this? I set up a Travis CI build for consul-client without my changes. It sets up this version of Consul before the build by running it as such: https://travis-ci.org/jhovell/consul-client/builds/99338912 tl;dr: So I am guessing my Consul assumptions are different than yours? |
|
I start Consul w/ Here's the failure I get on most tests when I run from Gradle. Oddly enough, they all pass in IDEA. This must be a classpath issue. <testcase name="shouldGetChecks" classname="com.orbitz.consul.AgentTests" time="0.023">
<failure message="javax.ws.rs.ProcessingException: RESTEASY004655: Unable to invoke request" type="javax.ws.rs.ProcessingException">javax.ws.rs.ProcessingException: RESTEASY004655: Unable to invoke request
at org.jboss.resteasy.client.jaxrs.engines.ApacheHttpClient4Engine.invoke(ApacheHttpClient4Engine.java:287)
at org.jboss.resteasy.client.jaxrs.internal.ClientInvocation.invoke(ClientInvocation.java:436)
at org.jboss.resteasy.client.jaxrs.internal.ClientInvocation.invoke(ClientInvocation.java:479)
at org.jboss.resteasy.client.jaxrs.internal.ClientInvocationBuilder.get(ClientInvocationBuilder.java:171)
at com.orbitz.consul.AgentClient.getChecks(AgentClient.java:347)
at com.orbitz.consul.AgentTests.shouldGetChecks(AgentTests.java:177)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:606)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecuter.runTestClass(JUnitTestClassExecuter.java:86)
at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecuter.execute(JUnitTestClassExecuter.java:49)
at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassProcessor.processTestClass(JUnitTestClassProcessor.java:64)
at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.processTestClass(SuiteTestClassProcessor.java:50)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:606)
at org.gradle.messaging.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:35)
at org.gradle.messaging.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
at org.gradle.messaging.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:32)
at org.gradle.messaging.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:93)
at com.sun.proxy.$Proxy2.processTestClass(Unknown Source)
at org.gradle.api.internal.tasks.testing.worker.TestWorker.processTestClass(TestWorker.java:106)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:606)
at org.gradle.messaging.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:35)
at org.gradle.messaging.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
at org.gradle.messaging.remote.internal.hub.MessageHub$Handler.run(MessageHub.java:360)
at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:54)
at org.gradle.internal.concurrent.StoppableExecutorImpl$1.run(StoppableExecutorImpl.java:40)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.IllegalStateException: Invalid use of BasicClientConnManager: connection still allocated.
Make sure to release the connection before allocating another one.
at org.apache.http.util.Asserts.check(Asserts.java:34)
at org.apache.http.impl.conn.BasicClientConnectionManager.getConnection(BasicClientConnectionManager.java:160)
at org.apache.http.impl.conn.BasicClientConnectionManager$1.getConnection(BasicClientConnectionManager.java:142)
at org.apache.http.impl.client.DefaultRequestDirector.execute(DefaultRequestDirector.java:423)
at org.apache.http.impl.client.AbstractHttpClient.doExecute(AbstractHttpClient.java:863)
at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:82)
at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:57)
at org.jboss.resteasy.client.jaxrs.engines.ApacheHttpClient4Engine.invoke(ApacheHttpClient4Engine.java:283)
... 48 more
</failure>
</testcase> |
|
@rickfast I adjusted my Travis consul build settings to match yours. Now, for both JDK8 and JDK7 I get 5 test failures (listed previously) This is for your consul-client 0.9.15 without any RESTEasy work. As for the RESTEasy issue, Intellij/Eclipse dependency management can be a bit inconsistent so I would take that with a grain of salt. I ran But maybe it would be helpful to put aside the RESTEasy issue for a moment & understand why this build won't pass? https://travis-ci.org/jhovell/consul-client |
@rickfast
Semi-related to here where I brought up a related issue: #76
JBoss/Wildfly includes an implementation of
JacksonJaxbJsonProvideron the classpath (e.g. resteasy-jackson2-provider) which is picked up by the JAX-RS client runtime (e.g. resteasy-client). So, the result is theJacksonJaxbJsonProviderdeclared by Consul-client is effectively ignored or may be ignored (I don't know if there is a way to define an order or priority among providers, it might be effectively whichever gets registered first wins), and as a result the Guava module is never loaded, and many interesting deserialization problems ensue, for example:So, instead of setting the mapper on the provider, I declared a
ContextResolver<ObjectMapper>and registered that, so it will be picked up by any message body reader/writer implementation already registered. Additionally I check to see if we already have aJacksonJaxbJsonProviderregistered and skip adding a new one if there is one there already. Seems safe since you don't seem to be doing anything special with it, but if you were, it might be very difficult to get this to play nicely in JEE Wildfly land 😞 .For testing, I just switched the jax.ws.rs runtime to Resteasy, but both Resteasy and Apache CXF seem to like this new setup. I might need something fancier like Shrinkwrap to play around with different classpath configurations if you wanted to be able to test against both during your build, and I am not sure if you have an interest in adding Resteasy as a test dependency, but let me know.
Anyway, cool library, planning to add vault support anytime soon? 😄