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.

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

Ensure that computed headers can override the default content-type #32072

Merged
merged 1 commit into from
Mar 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import static org.jboss.jandex.Type.Kind.CLASS;
import static org.jboss.jandex.Type.Kind.PARAMETERIZED_TYPE;
import static org.jboss.jandex.Type.Kind.PRIMITIVE;
import static org.jboss.resteasy.reactive.client.impl.RestClientRequestContext.DEFAULT_CONTENT_TYPE_PROP;
import static org.jboss.resteasy.reactive.common.processor.EndpointIndexer.extractProducesConsumesValues;
import static org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames.COMPLETION_STAGE;
import static org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames.CONSUMES;
Expand Down Expand Up @@ -2026,6 +2027,15 @@ private void handleReturn(ClassInfo restClientInterface, String defaultMediaType
} else if (formParams != null) {
mediaTypeValue = multipart ? MediaType.MULTIPART_FORM_DATA : MediaType.APPLICATION_FORM_URLENCODED;
}

if (mediaTypeValue.equals(defaultMediaType)) {
// we want to keep the default type around in order to ensure that Header manipulation code can override it without issue
tryBlock.invokeInterfaceMethod(
MethodDescriptor.ofMethod(Invocation.Builder.class, "property", Invocation.Builder.class, String.class,
Object.class),
builder, tryBlock.load(DEFAULT_CONTENT_TYPE_PROP), tryBlock.load(mediaTypeValue));
}

ResultHandle mediaType = tryBlock.invokeStaticMethod(
MethodDescriptor.ofMethod(MediaType.class, "valueOf", MediaType.class, String.class),
tryBlock.load(mediaTypeValue));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
import io.quarkus.rest.client.reactive.runtime.ComputedParamContextImpl;
import io.quarkus.rest.client.reactive.runtime.ConfigUtils;
import io.quarkus.rest.client.reactive.runtime.ExtendedHeaderFiller;
import io.quarkus.rest.client.reactive.runtime.HeaderFillerUtil;
import io.quarkus.rest.client.reactive.runtime.MicroProfileRestClientRequestFilter;
import io.quarkus.rest.client.reactive.runtime.NoOpHeaderFiller;
import io.quarkus.runtime.util.HashUtil;
Expand All @@ -91,8 +92,10 @@ class MicroProfileRestClientEnricher implements JaxrsClientReactiveEnricher {
Object.class);
private static final MethodDescriptor MAP_PUT_METHOD = MethodDescriptor.ofMethod(Map.class, "put", Object.class,
Object.class, Object.class);
private static final MethodDescriptor MAP_CONTAINS_KEY_METHOD = MethodDescriptor.ofMethod(Map.class, "containsKey",
boolean.class, Object.class);

private static final MethodDescriptor HEADER_FILLER_UTIL_SHOULD_ADD_HEADER = MethodDescriptor.ofMethod(
HeaderFillerUtil.class, "shouldAddHeader",
boolean.class, String.class, MultivaluedMap.class, ClientRequestContext.class);
private static final MethodDescriptor WEB_TARGET_IMPL_QUERY_PARAMS = MethodDescriptor.ofMethod(WebTargetImpl.class,
"queryParam", WebTargetImpl.class, String.class, Collection.class);

Expand Down Expand Up @@ -520,8 +523,8 @@ private void addHeaderParam(MethodInfo declaringMethod, MethodCreator fillHeader

// if headers are set here, they were set with @HeaderParam, which should take precedence of MP ways
BytecodeCreator fillHeaders = fillHeadersCreator
.ifFalse(fillHeadersCreator.invokeInterfaceMethod(MAP_CONTAINS_KEY_METHOD, headerMap,
fillHeadersCreator.load(headerName)))
.ifTrue(fillHeadersCreator.invokeStaticMethod(HEADER_FILLER_UTIL_SHOULD_ADD_HEADER,
fillHeadersCreator.load(headerName), headerMap, requestContext))
.trueBranch();

if (values.length > 1 || !(values[0].startsWith("{") && values[0].endsWith("}"))) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import org.eclipse.microprofile.rest.client.annotation.RegisterProvider;
import org.eclipse.microprofile.rest.client.inject.RegisterRestClient;

import io.quarkus.rest.client.reactive.ComputedParamContext;
import io.quarkus.rest.client.reactive.NotBody;
import io.quarkus.rest.client.reactive.TestJacksonBasicMessageBodyReader;

Expand All @@ -23,4 +24,15 @@ public interface MultipleHeadersBindingClient {
@Path("/describe-request")
@ClientHeaderParam(name = "header-from-properties", value = "${header.value}")
RequestData call(@HeaderParam("jaxrs-style-header") String headerValue, @NotBody String usedForComputingContentType);

@GET
@Path("/describe-request")
@ClientHeaderParam(name = "header-from-properties", value = "${header.value}")
@ClientHeaderParam(name = "Content-Type", value = "{calculateContentType}")
RequestData call(@HeaderParam("jaxrs-style-header") String headerValue, @NotBody String usedForComputingContentType,
String unusedBody);

default String calculateContentType(ComputedParamContext context) {
return "application/json;param2=" + context.methodParameters().get(1).value();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public void shouldNotPassIncomingHeaders() {
}

@Test
public void shouldSetHeadersFromMultipleBindings() {
public void shouldSetHeadersFromMultipleBindingsAndNoBody() {
String headerValue = "my-header-value";
Map<String, List<String>> headers = multipleHeadersBindingClient.call(headerValue, "test").getHeaders();
// Verify: @RegisterClientHeaders(MyHeadersFactory.class)
Expand All @@ -85,4 +85,22 @@ public void shouldSetHeadersFromMultipleBindings() {
assertThat(headers.get("jaxrs-style-header")).containsExactly(headerValue);
}

@Test
public void shouldSetHeadersFromMultipleBindingsAndBody() {
String headerValue = "my-header-value";
Map<String, List<String>> headers = multipleHeadersBindingClient.call(headerValue, "test", "unused-body").getHeaders();
// Verify: @RegisterClientHeaders(MyHeadersFactory.class)
assertThat(headers.get("foo")).containsExactly("bar");
// Verify: @ClientHeaderParam(name = "my-header", value = "constant-header-value")
assertThat(headers.get("my-header")).containsExactly("constant-header-value");
// Verify: @ClientHeaderParam(name = "computed-header", value = "{...ComputedHeader.get}")
assertThat(headers.get("computed-header")).containsExactly("From " + ComputedHeader.class.getName());
// Verify: @ClientHeaderParam(name = "Content-Type", value = "{calculateContentType}")
assertThat(headers.get("Content-Type")).containsExactly("application/json;param2=test");
// Verify: @ClientHeaderParam(name = "header-from-properties", value = "${header.value}")
assertThat(headers.get("header-from-properties")).containsExactly("from property file");
// Verify: @HeaderParam("jaxrs-style-header")
assertThat(headers.get("jaxrs-style-header")).containsExactly(headerValue);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package io.quarkus.rest.client.reactive.runtime;

import static org.jboss.resteasy.reactive.client.impl.RestClientRequestContext.DEFAULT_CONTENT_TYPE_PROP;

import jakarta.ws.rs.client.ClientRequestContext;
import jakarta.ws.rs.core.HttpHeaders;
import jakarta.ws.rs.core.MultivaluedMap;

@SuppressWarnings("unused")
public final class HeaderFillerUtil {

private HeaderFillerUtil() {
}

public static boolean shouldAddHeader(String name, MultivaluedMap<String, String> headers, ClientRequestContext context) {
String existingValue = headers.getFirst(name);
if (existingValue == null) {
// if the header is part of the existing headers, we should add it
return true;
}

if (HttpHeaders.CONTENT_TYPE.equals(name)) {
Object defaultContentType = context.getProperty(DEFAULT_CONTENT_TYPE_PROP);
if (defaultContentType == null) {
return true;
} else {
// if the header is the Content-Type, then we should update if its current value equals what determined at build time (and therefore no other code has changed it)
return existingValue.equals(defaultContentType);
}
} else {
return false;
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -565,11 +565,13 @@ private void setVertxHeaders(HttpClientRequest httpClientRequest, MultivaluedMap
private void setEntityRelatedHeaders(MultivaluedMap<String, String> headerMap, Entity<?> entity) {
if (entity.getVariant() != null) {
Variant v = entity.getVariant();
headerMap.putSingle(HttpHeaders.CONTENT_TYPE, v.getMediaType().toString());
if (v.getLanguageString() != null) {
if (!headerMap.containsKey(HttpHeaders.CONTENT_TYPE)) {
headerMap.putSingle(HttpHeaders.CONTENT_TYPE, v.getMediaType().toString());
}
if ((v.getLanguageString() != null) && !headerMap.containsKey(HttpHeaders.CONTENT_LANGUAGE)) {
headerMap.putSingle(HttpHeaders.CONTENT_LANGUAGE, v.getLanguageString());
}
if (v.getEncoding() != null) {
if ((v.getEncoding() != null) && !headerMap.containsKey(HttpHeaders.CONTENT_ENCODING)) {
headerMap.putSingle(HttpHeaders.CONTENT_ENCODING, v.getEncoding());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ public class RestClientRequestContext extends AbstractResteasyReactiveContext<Re

public static final String INVOKED_METHOD_PROP = "org.eclipse.microprofile.rest.client.invokedMethod";
public static final String INVOKED_METHOD_PARAMETERS_PROP = "io.quarkus.rest.client.invokedMethodParameters";
public static final String DEFAULT_CONTENT_TYPE_PROP = "io.quarkus.rest.client.defaultContentType";
private static final String TMP_FILE_PATH_KEY = "tmp_file_path";

private final HttpClient httpClient;
Expand Down