Skip to content

Commit

Permalink
Cache Feign endpoints (#2973)
Browse files Browse the repository at this point in the history
  • Loading branch information
@pkoenig10
pkoenig10 authored Sep 30, 2024
1 parent db3fa40 commit 25f91b4
Show file tree
Hide file tree
Showing 5 changed files with 189 additions and 61 deletions.
3 changes: 3 additions & 0 deletions conjure-java-jaxrs-client/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ dependencies {
implementation 'com.palantir.tritium:tritium-registry'
implementation 'io.dropwizard.metrics:metrics-core'

annotationProcessor "org.immutables:value"
compileOnly 'org.immutables:value::annotations'

implementation project(":conjure-java-jackson-serialization")
implementation "com.google.guava:guava"
implementation "com.github.ben-manes.caffeine:caffeine"
Expand Down
37 changes: 22 additions & 15 deletions ...src/main/java/com/palantir/conjure/java/client/jaxrs/AbstractFeignJaxRsClientBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@
import com.palantir.conjure.java.client.jaxrs.feignimpl.InputStreamDelegateEncoder;
import com.palantir.conjure.java.client.jaxrs.feignimpl.Java8OptionalAwareContract;
import com.palantir.conjure.java.client.jaxrs.feignimpl.Java8OptionalAwareDecoder;
import com.palantir.conjure.java.client.jaxrs.feignimpl.MethodHeaderEnrichmentContract;
import com.palantir.conjure.java.client.jaxrs.feignimpl.NeverReturnNullDecoder;
import com.palantir.conjure.java.client.jaxrs.feignimpl.PathTemplateHeaderEnrichmentContract;
import com.palantir.conjure.java.client.jaxrs.feignimpl.SlashEncodingContract;
import com.palantir.conjure.java.client.jaxrs.feignimpl.TextDelegateDecoder;
import com.palantir.conjure.java.client.jaxrs.feignimpl.TextDelegateEncoder;
Expand Down Expand Up @@ -180,28 +180,35 @@ public int hashCode() {
}

private static Contract createContract() {
return new EndpointNameHeaderEnrichmentContract(
new PathTemplateHeaderEnrichmentContract(new SlashEncodingContract(new Java8OptionalAwareContract(
new GuavaOptionalAwareContract(new CompatibleJaxRsContract())))));
Contract contract = new CompatibleJaxRsContract();
contract = new GuavaOptionalAwareContract(contract);
contract = new Java8OptionalAwareContract(contract);
contract = new SlashEncodingContract(contract);
contract = new MethodHeaderEnrichmentContract(contract);
contract = new EndpointNameHeaderEnrichmentContract(contract);
return contract;
}

private static Decoder createDecoder(
@Safe String clientNameForLogging, ObjectMapper jsonMapper, ObjectMapper cborMapper) {
return new NeverReturnNullDecoder(
new Java8OptionalAwareDecoder(new GuavaOptionalAwareDecoder(new EmptyContainerDecoder(
jsonMapper,
new InputStreamDelegateDecoder(
clientNameForLogging,
new TextDelegateDecoder(
new CborDelegateDecoder(cborMapper, new JacksonDecoder(jsonMapper))))))));
Decoder decoder = new JacksonDecoder(jsonMapper);
decoder = new CborDelegateDecoder(cborMapper, decoder);
decoder = new TextDelegateDecoder(decoder);
decoder = new InputStreamDelegateDecoder(clientNameForLogging, decoder);
decoder = new EmptyContainerDecoder(jsonMapper, decoder);
decoder = new GuavaOptionalAwareDecoder(decoder);
decoder = new Java8OptionalAwareDecoder(decoder);
decoder = new NeverReturnNullDecoder(decoder);
return decoder;
}

private static Encoder createEncoder(
@Safe String clientNameForLogging, ObjectMapper jsonMapper, ObjectMapper cborMapper) {
return new InputStreamDelegateEncoder(
clientNameForLogging,
new TextDelegateEncoder(
new CborDelegateEncoder(cborMapper, new ConjureFeignJacksonEncoder(jsonMapper))));
Encoder encoder = new ConjureFeignJacksonEncoder(jsonMapper);
encoder = new CborDelegateEncoder(cborMapper, encoder);
encoder = new TextDelegateEncoder(encoder);
encoder = new InputStreamDelegateEncoder(clientNameForLogging, encoder);
return encoder;
}

private static void verifyClientUsageAnnotations(Class<?> serviceClass) {
Expand Down
110 changes: 75 additions & 35 deletions ...axrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/DialogueFeignClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.google.common.util.concurrent.UncheckedExecutionException;
import com.palantir.conjure.java.api.errors.UnknownRemoteException;
import com.palantir.conjure.java.client.jaxrs.feignimpl.EndpointNameHeaderEnrichmentContract;
import com.palantir.conjure.java.client.jaxrs.feignimpl.MethodHeaderEnrichmentContract;
import com.palantir.dialogue.Channel;
import com.palantir.dialogue.ConjureRuntime;
import com.palantir.dialogue.Deserializer;
Expand Down Expand Up @@ -57,24 +58,29 @@
import java.util.List;
import java.util.Locale;
import java.util.Optional;
import java.util.OptionalLong;
import java.util.concurrent.ConcurrentHashMap;
import org.immutables.value.Value;

/**
* {@link DialogueFeignClient} is an adapter from {@link feign.Client} to {@link Channel Dialogue Channel}
* taking advantage of the superior observability and stability provided by Dialogue.
*/
final class DialogueFeignClient implements feign.Client {

private static final String PATH_TEMPLATE = "hr-path-template";
private static final Splitter pathSplitter = Splitter.on('/');
private static final Splitter querySplitter = Splitter.on('&').omitEmptyStrings();
private static final Splitter queryValueSplitter = Splitter.on('=');
private static final String REQUEST_URL_PATH_PARAM = "request-url";
private static final Splitter PATH_SPLITTER = Splitter.on('/');
private static final Splitter QUERY_SPLITTER = Splitter.on('&').omitEmptyStrings();
private static final Splitter QUERY_VALUE_SPLITTER = Splitter.on('=');

private final ConjureRuntime runtime;
private final Channel channel;
private final String baseUrl;
private final String serviceName;
private final String version;

private final ConcurrentHashMap<FeignEndpointKey, EndpointChannel> endpointChannels = new ConcurrentHashMap<>();

DialogueFeignClient(Class<?> jaxrsInterface, Channel channel, ConjureRuntime runtime, String baseUrl) {
this.channel = Preconditions.checkNotNull(channel, "Channel is required");
this.baseUrl = Preconditions.checkNotNull(baseUrl, "Base URL is required");
Expand All @@ -88,20 +94,22 @@ final class DialogueFeignClient implements feign.Client {
@Override
public feign.Response execute(Request request, Request.Options _options) throws IOException {
com.palantir.dialogue.Request.Builder builder = com.palantir.dialogue.Request.builder();
Optional<RequestBody> body = requestBody(request);
if (body.isPresent()) {
builder.body(body);
builder.putHeaderParams(HttpHeaders.CONTENT_LENGTH, Integer.toString(request.body().length));
}

builder.putPathParams(REQUEST_URL_PATH_PARAM, request.url());

builder.body(requestBody(request));

request.headers().forEach((headerName, values) -> {
if (includeRequestHeader(headerName)) {
builder.putAllHeaderParams(headerName, values);
}
});

EndpointChannel endpointChannel =
endpointChannels.computeIfAbsent(FeignEndpointKey.of(request), this::toEndpointChannel);

try {
return runtime.clients()
.callBlocking(toEndpointChannel(request), builder.build(), FeignResponseDeserializer.INSTANCE);
return runtime.clients().callBlocking(endpointChannel, builder.build(), FeignResponseDeserializer.INSTANCE);
} catch (UncheckedExecutionException e) {
// Rethrow IOException to match standard feign behavior
Throwable cause = e.getCause();
Expand All @@ -112,8 +120,8 @@ public feign.Response execute(Request request, Request.Options _options) throws
}
}

private EndpointChannel toEndpointChannel(Request feignRequest) {
Endpoint endpoint = new FeignRequestEndpoint(feignRequest);
private EndpointChannel toEndpointChannel(FeignEndpointKey key) {
Endpoint endpoint = new FeignEndpoint(key.httpMethod(), key.endpointName());
return dialogueRequest -> channel.execute(endpoint, dialogueRequest);
}

Expand All @@ -125,8 +133,7 @@ private static boolean includeRequestHeader(String headerName) {
if (HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(headerName)) {
return false;
}
// Tracing path template is informational only
if (PATH_TEMPLATE.equalsIgnoreCase(headerName)) {
if (MethodHeaderEnrichmentContract.METHOD_HEADER.equalsIgnoreCase(headerName)) {
return false;
}
if (EndpointNameHeaderEnrichmentContract.ENDPOINT_NAME_HEADER.equalsIgnoreCase(headerName)) {
Expand All @@ -149,10 +156,8 @@ private static Optional<RequestBody> requestBody(Request request) {
return Optional.empty();
}
Optional<String> maybeContentType = getFirstHeader(request, HttpHeaders.CONTENT_TYPE);
if (!maybeContentType.isPresent()) {
if (requestBodyContent.length == 0) {
return Optional.empty();
}
if (maybeContentType.isEmpty() && requestBodyContent.length == 0) {
return Optional.empty();
}
return Optional.of(new ByteArrayRequestBody(
requestBodyContent,
Expand All @@ -173,10 +178,12 @@ private static final class ByteArrayRequestBody implements RequestBody {

private final byte[] buffer;
private final String contentType;
private final OptionalLong contentLength;

ByteArrayRequestBody(byte[] buffer, String contentType) {
this.buffer = buffer;
this.contentType = contentType;
this.contentLength = OptionalLong.of(buffer.length);
}

@Override
Expand All @@ -194,6 +201,11 @@ public boolean repeatable() {
return true;
}

@Override
public OptionalLong contentLength() {
return contentLength;
}

@Override
public void close() {
// nothing to do
Expand Down Expand Up @@ -341,26 +353,32 @@ public Exception decode(String _methodKey, feign.Response response) {
}
}

private final class FeignRequestEndpoint implements Endpoint {
private final Request request;
private final String endpoint;
private final HttpMethod method;
private final class FeignEndpoint implements Endpoint {

FeignRequestEndpoint(Request request) {
this.request = request;
this.method = HttpMethod.valueOf(request.method().toUpperCase(Locale.ENGLISH));
endpoint = getFirstHeader(request, EndpointNameHeaderEnrichmentContract.ENDPOINT_NAME_HEADER)
.orElse("feign");
private final HttpMethod httpMethod;
private final String endpointName;

FeignEndpoint(HttpMethod httpMethod, String endpointName) {
this.httpMethod = httpMethod;
this.endpointName = endpointName;
}

@Override
public void renderPath(ListMultimap<String, String> _params, UrlBuilder url) {
String target = request.url();
public void renderPath(ListMultimap<String, String> params, UrlBuilder url) {
List<String> requestUrls = params.get(REQUEST_URL_PATH_PARAM);
Preconditions.checkState(
params.size() == 1 && requestUrls.size() == 1,
"Unexpected path parameters",
SafeArg.of("params", params.size()),
SafeArg.of("requestUrls", requestUrls.size()));

String target = requestUrls.get(0);
Preconditions.checkState(
target.startsWith(baseUrl),
"Request URL must start with base url",
UnsafeArg.of("requestUrl", target),
UnsafeArg.of("baseUrl", baseUrl));

int trailingOffset = 0;
// If the trailing section starts with a slash, ignore it to prevent duplicate leading slashes.
if (target.length() > baseUrl.length() && target.charAt(baseUrl.length()) == '/') {
Expand All @@ -370,14 +388,14 @@ public void renderPath(ListMultimap<String, String> _params, UrlBuilder url) {
int queryParamsStart = trailing.indexOf('?');
String queryPortion = queryParamsStart == -1 ? trailing : trailing.substring(0, queryParamsStart);
if (!queryPortion.isEmpty()) {
for (String pathSegment : pathSplitter.split(queryPortion)) {
for (String pathSegment : PATH_SPLITTER.split(queryPortion)) {
url.pathSegment(urlDecode(pathSegment));
}
}
if (queryParamsStart != -1) {
String querySegments = trailing.substring(queryParamsStart + 1);
for (String querySegment : querySplitter.split(querySegments)) {
List<String> keyValuePair = queryValueSplitter.splitToList(querySegment);
for (String querySegment : QUERY_SPLITTER.split(querySegments)) {
List<String> keyValuePair = QUERY_VALUE_SPLITTER.splitToList(querySegment);
if (keyValuePair.size() != 2) {
throw new SafeIllegalStateException(
"Expected two parameters",
Expand All @@ -391,7 +409,7 @@ public void renderPath(ListMultimap<String, String> _params, UrlBuilder url) {

@Override
public HttpMethod httpMethod() {
return method;
return httpMethod;
}

@Override
Expand All @@ -401,12 +419,34 @@ public String serviceName() {

@Override
public String endpointName() {
return endpoint;
return endpointName;
}

@Override
public String version() {
return version;
}
}

@Value.Immutable(builder = false, prehash = true)
interface FeignEndpointKey {

@Value.Parameter
HttpMethod httpMethod();

@Value.Parameter
String method();

@Value.Parameter
String endpointName();

private static FeignEndpointKey of(Request request) {
return ImmutableFeignEndpointKey.of(
HttpMethod.valueOf(request.method().toUpperCase(Locale.ENGLISH)),
getFirstHeader(request, MethodHeaderEnrichmentContract.METHOD_HEADER)
.orElse(""),
getFirstHeader(request, EndpointNameHeaderEnrichmentContract.ENDPOINT_NAME_HEADER)
.orElse("feign"));
}
}
}
40 changes: 40 additions & 0 deletions ...java/com/palantir/conjure/java/client/jaxrs/feignimpl/MethodHeaderEnrichmentContract.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* (c) Copyright 2017 Palantir Technologies Inc. All rights reserved.
*
* Licensed 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 com.palantir.conjure.java.client.jaxrs.feignimpl;

import feign.Contract;
import feign.MethodMetadata;
import java.lang.reflect.Method;

/**
* Contract to capture the method name and pass it to a feign client.
*
* This should be considered internal API and should not be depended upon.
*/
public final class MethodHeaderEnrichmentContract extends AbstractDelegatingContract {

public static final String METHOD_HEADER = "dialogue-method";

public MethodHeaderEnrichmentContract(Contract delegate) {
super(delegate);
}

@Override
protected void processMetadata(Class<?> _targetType, Method method, MethodMetadata metadata) {
metadata.template().header(METHOD_HEADER, method.toString());
}
}
Loading

0 comments on commit 25f91b4

Please sign in to comment.