Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update MP REST Client to 4.0 #43959

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Update MP REST Client to 4.0 #43959

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions bom/application/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
<javax.annotation-api.version>1.3.2</javax.annotation-api.version>
<javax.inject.version>1</javax.inject.version>
<parsson.version>1.1.7</parsson.version>
<resteasy-microprofile.version>2.1.5.Final</resteasy-microprofile.version>
<resteasy-microprofile.version>3.0.0.Final</resteasy-microprofile.version>
<resteasy-spring-web.version>3.1.3.Final</resteasy-spring-web.version>
<resteasy.version>6.2.10.Final</resteasy.version>
<opentracing.version>0.33.0</opentracing.version>
Expand All @@ -48,7 +48,7 @@
<microprofile-opentracing-api.version>3.0</microprofile-opentracing-api.version>
<microprofile-fault-tolerance-api.version>4.1.1</microprofile-fault-tolerance-api.version>
<microprofile-reactive-streams-operators.version>3.0.1</microprofile-reactive-streams-operators.version>
<microprofile-rest-client.version>3.0.1</microprofile-rest-client.version>
<microprofile-rest-client.version>4.0</microprofile-rest-client.version>
<microprofile-jwt.version>2.1</microprofile-jwt.version>
<microprofile-lra.version>2.0</microprofile-lra.version>
<microprofile-openapi.version>3.1.2</microprofile-openapi.version>
Expand Down
4 changes: 4 additions & 0 deletions extensions/resteasy-classic/resteasy-client/runtime/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.jboss.resteasy</groupId>
<artifactId>resteasy-multipart-provider</artifactId>
</dependency>
Comment on lines +61 to +64
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that I mind, but I would like to know why this is now necessary

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also if it's necessary, we might have to depend on the quarkus-resteasy-multipart extension instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the dependency from RESTEasy that provides the EntityPart.Builder implementation. Note that for the REST Client Classic, I've tried to keep it as the original implementation: resteasy/resteasy-microprofile@bb8b433

<dependency>
<groupId>jakarta.interceptor</groupId>
<artifactId>jakarta.interceptor-api</artifactId>
Expand Down
42 changes: 38 additions & 4 deletions ...-client/runtime/src/main/java/io/quarkus/restclient/runtime/QuarkusRestClientBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import static org.jboss.resteasy.client.jaxrs.ResteasyClientBuilder.PROPERTY_PROXY_PORT;
import static org.jboss.resteasy.client.jaxrs.ResteasyClientBuilder.PROPERTY_PROXY_SCHEME;

import java.io.Closeable;
import java.lang.annotation.Annotation;
import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.Field;
Expand All @@ -22,14 +23,17 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.TimeUnit;
import java.util.stream.Stream;

import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.SSLContext;
Expand All @@ -44,6 +48,7 @@
import jakarta.ws.rs.Priorities;
import jakarta.ws.rs.core.Configuration;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.MultivaluedMap;
import jakarta.ws.rs.ext.ParamConverterProvider;

import org.eclipse.microprofile.config.Config;
Expand All @@ -64,6 +69,7 @@
import org.jboss.resteasy.client.jaxrs.internal.LocalResteasyProviderFactory;
import org.jboss.resteasy.concurrent.ContextualExecutorService;
import org.jboss.resteasy.concurrent.ContextualExecutors;
import org.jboss.resteasy.core.Headers;
import org.jboss.resteasy.microprofile.client.ConfigurationWrapper;
import org.jboss.resteasy.microprofile.client.DefaultMediaTypeFilter;
import org.jboss.resteasy.microprofile.client.DefaultResponseExceptionMapper;
Expand All @@ -86,6 +92,10 @@
import io.quarkus.runtime.graal.DisabledSSLContext;
import io.quarkus.runtime.ssl.SslContextConfiguration;

/**
* This is mostly a copy from {@link org.jboss.resteasy.microprofile.client.RestClientBuilderImpl}. It is required to
* remove the reference to org.jboss.resteasy.cdi.CdiInjectorFactory so we don't require the RESTEasy CDI dependency.
*/
public class QuarkusRestClientBuilder implements RestClientBuilder {

private static final String RESTEASY_PROPERTY_PREFIX = "resteasy.";
Expand All @@ -94,16 +104,22 @@ public class QuarkusRestClientBuilder implements RestClientBuilder {
private static final Logger LOGGER = Logger.getLogger(QuarkusRestClientBuilder.class);
private static final DefaultMediaTypeFilter DEFAULT_MEDIA_TYPE_FILTER = new DefaultMediaTypeFilter();
private static final String TLS_TRUST_ALL = "quarkus.tls.trust-all";

private static final Collection<Method> IGNORED_METHODS = new ArrayList<>();
public static final MethodInjectionFilter METHOD_INJECTION_FILTER = new MethodInjectionFilter();
public static final ClientHeadersRequestFilter HEADERS_REQUEST_FILTER = new ClientHeadersRequestFilter();

static ResteasyProviderFactory PROVIDER_FACTORY;

static {
Collections.addAll(IGNORED_METHODS, Closeable.class.getMethods());
Collections.addAll(IGNORED_METHODS, AutoCloseable.class.getMethods());
}

public static void setProviderFactory(ResteasyProviderFactory providerFactory) {
PROVIDER_FACTORY = providerFactory;
}

private final MultivaluedMap<String, Object> headers;

public QuarkusRestClientBuilder() {
builderDelegate = new MpClientBuilderImpl();

Expand All @@ -126,6 +142,7 @@ public QuarkusRestClientBuilder() {
} catch (Throwable e) {

}
headers = new Headers<>();
}

public Configuration getConfigurationWrapper() {
Expand All @@ -148,6 +165,13 @@ public RestClientBuilder queryParamStyle(QueryParamStyle queryParamStyle) {
return this;
}

@Override
public RestClientBuilder header(final String name, final Object value) {
geoand marked this conversation as resolved.
Show resolved Hide resolved
headers.add(Objects.requireNonNull(name, "A header name is required."),
Objects.requireNonNull(value, "Value for header is required."));
return this;
}

@Override
public RestClientBuilder proxyAddress(String host, int port) {
if (host == null) {
Expand Down Expand Up @@ -288,7 +312,7 @@ public <T> T build(Class<T> aClass, ClientHttpEngine httpEngine)
}
resteasyClientBuilder.register(DEFAULT_MEDIA_TYPE_FILTER);
resteasyClientBuilder.register(METHOD_INJECTION_FILTER);
resteasyClientBuilder.register(HEADERS_REQUEST_FILTER);
resteasyClientBuilder.register(new ClientHeadersRequestFilter(headers));
register(new MpPublisherMessageBodyReader(executorService));
resteasyClientBuilder.sslContext(sslContext);
resteasyClientBuilder.trustStore(trustStore);
Expand Down Expand Up @@ -551,7 +575,7 @@ private void verifyBeanPathParam(Class<?> beanType, Map<String, Object> paramMap

private <T> void verifyInterface(Class<T> typeDef) {

Method[] methods = typeDef.getMethods();
Method[] methods = resolveMethods(typeDef);

// multiple verbs
for (Method method : methods) {
Expand Down Expand Up @@ -806,6 +830,16 @@ private static BeanManager getBeanManager() {
}
}

private static Method[] resolveMethods(final Class<?> type) {
// If the type extends Closeable or AutoCloseable, we need to filter out their methods
if (AutoCloseable.class.isAssignableFrom(type)) {
return Stream.of(type.getMethods())
.filter(method -> !IGNORED_METHODS.contains(method))
.toArray(Method[]::new);
Comment on lines +836 to +838
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm definitely no fan of this, but we can let it go in :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is a copy from the original RESTEasy Builder code:
resteasy/resteasy-microprofile@cb58ceb

Initially, we used the one provided by RESTEasy, but we made a copy to remove references to their CDI integration and also to remove the CDI dependency. They diverged a bit, so I've tried to reconcile them again for consistency.

}
return type.getMethods();
}

private final MpClientBuilderImpl builderDelegate;

private final ConfigurationWrapper configurationWrapper;
Expand Down
97 changes: 97 additions & 0 deletions ...ment/src/test/java/io/quarkus/rest/client/reactive/headers/DefaultBuilderHeadersTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
package io.quarkus.rest.client.reactive.headers;

import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.io.IOException;
import java.net.URI;
import java.util.List;
import java.util.stream.Collectors;

import jakarta.json.Json;
import jakarta.json.JsonArray;
import jakarta.json.JsonArrayBuilder;
import jakarta.json.JsonObject;
import jakarta.json.JsonObjectBuilder;
import jakarta.json.JsonString;
import jakarta.json.JsonValue;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.HeaderParam;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.client.ClientRequestContext;
import jakarta.ws.rs.client.ClientRequestFilter;
import jakarta.ws.rs.core.MultivaluedMap;
import jakarta.ws.rs.core.Response;

import org.eclipse.microprofile.rest.client.RestClientBuilder;
import org.eclipse.microprofile.rest.client.annotation.ClientHeaderParam;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusUnitTest;
import io.quarkus.test.common.http.TestHTTPResource;

public class DefaultBuilderHeadersTest {
@RegisterExtension
static final QuarkusUnitTest TEST = new QuarkusUnitTest().withApplicationRoot(jar -> {
});

@TestHTTPResource
URI baseUri;

@Test
void headers() {
RestClientBuilder builder = RestClientBuilder.newBuilder().baseUri("http://localhost:8080/");
builder.register(ReturnWithAllDuplicateClientHeadersFilter.class);
builder.header("InterfaceAndBuilderHeader", "builder");
ClientBuilderHeaderMethodClient client = builder.build(ClientBuilderHeaderMethodClient.class);

checkHeaders(client.getAllHeaders("headerparam"), "method");
}

@Path("/")
public interface ClientBuilderHeaderMethodClient {
@GET
@ClientHeaderParam(name = "InterfaceAndBuilderHeader", value = "method")
JsonObject getAllHeaders(@HeaderParam("HeaderParam") String param);
}

public static class ReturnWithAllDuplicateClientHeadersFilter implements ClientRequestFilter {

@Override
public void filter(ClientRequestContext clientRequestContext) throws IOException {
JsonObjectBuilder allClientHeaders = Json.createObjectBuilder();
MultivaluedMap<String, Object> clientHeaders = clientRequestContext.getHeaders();
for (String headerName : clientHeaders.keySet()) {
List<Object> header = clientHeaders.get(headerName);
final JsonArrayBuilder headerValues = Json.createArrayBuilder();
header.forEach(h -> headerValues.add(h.toString()));
allClientHeaders.add(headerName, headerValues);
}
clientRequestContext.abortWith(Response.ok(allClientHeaders.build()).build());
}
}

private static void checkHeaders(final JsonObject headers, final String clientHeaderParamName) {
final List<String> clientRequestHeaders = headerValues(headers, "InterfaceAndBuilderHeader");

assertTrue(clientRequestHeaders.contains("builder"),
"Header InterfaceAndBuilderHeader did not container \"builder\": " + clientRequestHeaders);
assertTrue(clientRequestHeaders.contains(clientHeaderParamName),
"Header InterfaceAndBuilderHeader did not container \"" + clientHeaderParamName + "\": "
+ clientRequestHeaders);

final List<String> headerParamHeaders = headerValues(headers, "HeaderParam");
assertTrue(headerParamHeaders.contains("headerparam"),
"Header HeaderParam did not container \"headerparam\": " + headerParamHeaders);
}

private static List<String> headerValues(final JsonObject headers, final String headerName) {
final JsonArray headerValues = headers.getJsonArray(headerName);
assertNotNull(headerValues,
String.format("Expected header '%s' to be present in %s", headerName, headers));
return headerValues.stream().map(
v -> (v.getValueType() == JsonValue.ValueType.STRING ? ((JsonString) v).getString() : v.toString()))
.collect(Collectors.toList());
}
}
31 changes: 31 additions & 0 deletions .../main/java/io/quarkus/rest/client/reactive/runtime/DefaultClientHeadersRequestFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package io.quarkus.rest.client.reactive.runtime;

import java.util.List;
import java.util.function.BiConsumer;

import jakarta.annotation.Priority;
import jakarta.ws.rs.client.ClientRequestContext;
import jakarta.ws.rs.client.ClientRequestFilter;
import jakarta.ws.rs.core.MultivaluedMap;

import org.jboss.resteasy.reactive.common.util.CaseInsensitiveMap;

@Priority(Integer.MIN_VALUE + 10)
public class DefaultClientHeadersRequestFilter implements ClientRequestFilter {
private final MultivaluedMap<String, Object> defaultHeaders;

public DefaultClientHeadersRequestFilter(final MultivaluedMap<String, Object> defaultHeaders) {
this.defaultHeaders = new CaseInsensitiveMap<>();
this.defaultHeaders.putAll(defaultHeaders);
}

@Override
public void filter(ClientRequestContext requestContext) {
this.defaultHeaders.forEach(new BiConsumer<String, List<Object>>() {
@Override
public void accept(final String name, final List<Object> values) {
requestContext.getHeaders().addAll(name, values);
}
});
}
}
12 changes: 12 additions & 0 deletions .../runtime/src/main/java/io/quarkus/rest/client/reactive/runtime/RestClientBuilderImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.TimeUnit;
Expand All @@ -21,6 +22,7 @@

import jakarta.ws.rs.RuntimeType;
import jakarta.ws.rs.core.Configuration;
import jakarta.ws.rs.core.MultivaluedMap;
import jakarta.ws.rs.ext.ParamConverterProvider;

import org.eclipse.microprofile.config.ConfigProvider;
Expand All @@ -40,6 +42,7 @@
import org.jboss.resteasy.reactive.client.impl.multipart.PausableHttpPostRequestEncoder;
import org.jboss.resteasy.reactive.common.jaxrs.ConfigurationImpl;
import org.jboss.resteasy.reactive.common.jaxrs.MultiQueryParamMode;
import org.jboss.resteasy.reactive.common.util.CaseInsensitiveMap;

import io.quarkus.arc.Arc;
import io.quarkus.arc.ArcContainer;
Expand Down Expand Up @@ -70,6 +73,7 @@ public class RestClientBuilderImpl implements RestClientBuilder {
private URI uri;
private boolean followRedirects;
private QueryParamStyle queryParamStyle;
private MultivaluedMap<String, Object> headers = new CaseInsensitiveMap<>();

private String multipartPostEncoderMode;
private String proxyHost;
Expand Down Expand Up @@ -387,6 +391,13 @@ public RestClientBuilderImpl queryParamStyle(final QueryParamStyle style) {
return this;
}

@Override
public RestClientBuilder header(final String name, final Object value) {
headers.add(Objects.requireNonNull(name, "A header name is required."),
Objects.requireNonNull(value, "Value for header is required."));
return this;
}

@Override
public <T> T build(Class<T> aClass) throws IllegalStateException, RestClientDefinitionException {
ArcContainer arcContainer = Arc.container();
Expand Down Expand Up @@ -454,6 +465,7 @@ public <T> T build(Class<T> aClass) throws IllegalStateException, RestClientDefi
}

clientBuilder.multiQueryParamMode(toMultiQueryParamMode(queryParamStyle));
clientBuilder.register(new DefaultClientHeadersRequestFilter(headers));

Boolean effectiveTrustAll = trustAll;
if (effectiveTrustAll == null) {
Expand Down
7 changes: 4 additions & 3 deletions ...e/src/main/java/org/jboss/resteasy/reactive/client/handlers/ClientSendRequestHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import org.jboss.resteasy.reactive.common.core.Serialisers;
import org.jboss.resteasy.reactive.common.util.MultivaluedTreeMap;

import io.netty.handler.codec.DecoderException;
import io.netty.handler.codec.http.HttpHeaderValues;
import io.netty.handler.codec.http.LastHttpContent;
import io.netty.handler.codec.http.multipart.InterfaceHttpData;
Expand Down Expand Up @@ -384,10 +385,10 @@ public void handle(AsyncResult<Buffer> ar) {
.onFailure(new Handler<>() {
@Override
public void handle(Throwable failure) {
if (failure instanceof HttpClosedException) {
if (failure instanceof HttpClosedException || failure instanceof DecoderException) {
// This is because of the Rest Client TCK
// HttpClosedException is a runtime exception. If we complete with that exception, it gets
// unwrapped by the rest client proxy and thus fails the TCK.
// HttpClosedException / DecoderException are runtime exceptions. If we complete with that
// exception, it gets unwrapped by the rest client proxy and thus fails the TCK.
// By creating an IOException, we avoid that and provide a meaningful exception (because
// it's an I/O exception)
requestContext.resume(new ProcessingException(new IOException(failure.getMessage())));
Expand Down
5 changes: 4 additions & 1 deletion tcks/microprofile-rest-client-reactive/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

<properties>
<wiremock.server.port>8766</wiremock.server.port>
<microprofile-rest-client-tck.version>3.0.1</microprofile-rest-client-tck.version>
<microprofile-rest-client-tck.version>4.0</microprofile-rest-client-tck.version>
<!--
Disabling the enforcer because of:

Expand Down Expand Up @@ -81,6 +81,9 @@
<!-- TODO fix this-->
<exclude>org.eclipse.microprofile.rest.client.tck.jsonb.InvokeWithJsonBProviderTest</exclude>

<!-- Missing implementation - See org.jboss.resteasy.reactive.common.jaxrs.RuntimeDelegateImpl.createEntityPartBuilder -->
<exclude>**/EntityPartTest</exclude>
radcortez marked this conversation as resolved.
Show resolved Hide resolved

<!-- These are flaky -->
<exclude>org.eclipse.microprofile.rest.client.tck.FollowRedirectsTest</exclude>
<exclude>org.eclipse.microprofile.rest.client.tck.cditests.CDIFollowRedirectsTest</exclude>
Expand Down
2 changes: 1 addition & 1 deletion tcks/microprofile-rest-client/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<name>Quarkus - TCK - MicroProfile REST Client</name>

<properties>
<microprofile-rest-client-tck.version>3.0.1</microprofile-rest-client-tck.version>
<microprofile-rest-client-tck.version>4.0</microprofile-rest-client-tck.version>
<!--
Disabling the enforcer because of:

Expand Down
Loading