Skip to content

Commit

Permalink
OpenFeignGH-57 Delegate Template Parameter creation to Method Handlers
Browse files Browse the repository at this point in the history
Related to OpenFeign#57

This change moves `TemplateParameter` object creation to `AbstractTargetMethodHandler` to reduce the overhead of `Contract` parsing and reduce the responsibility of `Contract` implementations to class metadata and not collaborating object construction.
  • Loading branch information
kdavisk6 committed Mar 28, 2021
1 parent cba47c0 commit a059858
Show file tree
Hide file tree
Showing 11 changed files with 640 additions and 94 deletions.
29 changes: 15 additions & 14 deletions core/src/main/java/feign/TargetMethodDefinition.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public final class TargetMethodDefinition {
private final transient TypeDefinition returnType;
private final UriTemplate template;
private final Collection<HttpHeader> headers;
private final Map<Integer, TemplateParameter> parameterMap;
private final Map<Integer, TargetMethodParameterDefinition> parameterMap;
private final Integer bodyArgumentIndex;
private final boolean followRedirects;
private final long connectTimeout;
Expand Down Expand Up @@ -90,7 +90,7 @@ public static Builder from(TargetMethodDefinition targetMethodDefinition) {
}

targetMethodDefinition.parameterMap.forEach(
builder::templateParameter);
builder::parameterDefinition);
targetMethodDefinition.headers.forEach(builder::header);

/* return the populated builder */
Expand All @@ -109,7 +109,7 @@ private TargetMethodDefinition(Target<?> target,
UriTemplate template,
HttpMethod method,
Collection<HttpHeader> headers,
Map<Integer, TemplateParameter> parameterMap,
Map<Integer, TargetMethodParameterDefinition> parameterMap,
Integer bodyArgumentIndex,
boolean followRedirects,
long connectTimeout,
Expand Down Expand Up @@ -163,12 +163,12 @@ public TypeDefinition getReturnType() {
}

/**
* Template Parameter registered at the specified method argument index.
* {@link TargetMethodParameterDefinition} registered at the specified method argument index.
*
* @param argumentIndex of the parameter.
* @return the Template Parameter registered, if one exists.
* @return the {@link TargetMethodParameterDefinition} registered, if one exists.
*/
public Optional<TemplateParameter> getTemplateParameter(Integer argumentIndex) {
public Optional<TargetMethodParameterDefinition> getParameterDefinition(Integer argumentIndex) {
return Optional.ofNullable(parameterMap.get(argumentIndex));
}

Expand Down Expand Up @@ -218,11 +218,11 @@ public Collection<HttpHeader> getHeaders() {
}

/**
* The Template Parameters registered for this method.
* The {@link TargetMethodParameterDefinition}s registered for this method.
*
* @return the parameters registered.
*/
public Collection<TemplateParameter> getTemplateParameters() {
public Collection<TargetMethodParameterDefinition> getParameterDefinitions() {
return Collections.unmodifiableCollection(this.parameterMap.values());
}

Expand Down Expand Up @@ -338,7 +338,8 @@ public static class Builder {
private UriTemplate template;
private HttpMethod method = HttpMethod.GET;
private final Collection<HttpHeader> headers = new CopyOnWriteArraySet<>();
private final Map<Integer, TemplateParameter> parameterMap = new ConcurrentHashMap<>();
private final Map<Integer, TargetMethodParameterDefinition> parameterMap =
new ConcurrentHashMap<>();
private Integer bodyArgumentIndex = -1;
private boolean followRedirects;
private long connectTimeout = RequestOptions.DEFAULT_CONNECT_TIMEOUT;
Expand Down Expand Up @@ -426,15 +427,15 @@ public Builder uri(String uri) {
}

/**
* Registers a {@link TemplateParameter} at the method argument index.
* Registers a {@link TargetMethodParameterDefinition} at the method argument index.
*
* @param argumentIndex in the method signature.
* @param templateParameter to register.
* @param definition to register.
* @return the reference chain.
*/
public Builder templateParameter(
Integer argumentIndex, TemplateParameter templateParameter) {
this.parameterMap.put(argumentIndex, templateParameter);
public Builder parameterDefinition(
Integer argumentIndex, TargetMethodParameterDefinition definition) {
this.parameterMap.put(argumentIndex, definition);
return this;
}

Expand Down
194 changes: 194 additions & 0 deletions core/src/main/java/feign/TargetMethodParameterDefinition.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
/*
* Copyright 2019-2021 OpenFeign Contributors
*
* 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 feign;

import feign.support.Assert;
import java.util.Locale;
import java.util.Objects;
import java.util.StringJoiner;
import net.jcip.annotations.Immutable;
import net.jcip.annotations.ThreadSafe;

/**
* Definition of a Method Parameter on a Target Method.
*/
@ThreadSafe
@Immutable
public class TargetMethodParameterDefinition {

private final String name;
private final String type;
private final Integer index;
private final String expanderClassName;

public static Builder builder() {
return new Builder();
}

/**
* Create a new {@link TargetMethodParameterDefinition}.
*
* @param name of the parameter.
* @param type of the parameter.
* @param index of the parameter in the method definition.
* @param expanderClassName of the expander to use when resolving this parameter.
*/
private TargetMethodParameterDefinition(String name, String type, Integer index,
String expanderClassName) {
Assert.isNotEmpty(name, "name is required.");
Assert.isNotEmpty(type, "type is required.");
Assert.isNotNull(index, "argument index is required.");
Assert.isTrue(index, idx -> idx >= 0, "argument index must be a positive number");
Assert.isNotEmpty(expanderClassName, "expander class name is required.");
this.name = name;
this.type = type;
this.index = index;
this.expanderClassName = expanderClassName;
}

/**
* The Name of the Parameter.
*
* @return parameter name.
*/
public String getName() {
return name;
}

/**
* Fully Qualified Class Name for the parameter type.
*
* @return parameter type.
*/
public String getType() {
return this.type;
}

/**
* Argument Index of the Parameter.
*
* @return the argument index.
*/
public Integer getIndex() {
return index;
}

/**
* Fully Qualified Class Name of the {@link feign.template.ExpressionExpander} to use when
* expanding this parameter value.
*
* @return the fully qualified class name.
*/
public String getExpanderClassName() {
return expanderClassName;
}

@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (!(obj instanceof TargetMethodParameterDefinition)) {
return false;
}
TargetMethodParameterDefinition that = (TargetMethodParameterDefinition) obj;
return getName().equalsIgnoreCase(that.getName()) && getType().equals(that.getType())
&& getIndex().equals(that.getIndex())
&& getExpanderClassName().equals(that.getExpanderClassName());
}

@Override
public int hashCode() {
return Objects.hash(getName(), getType(), getIndex(), getExpanderClassName());
}

@Override
public String toString() {
return new StringJoiner(", ",
TargetMethodParameterDefinition.class.getSimpleName() + "[", "]")
.add("name='" + name + "'")
.add("type='" + type + "'")
.add("index=" + index)
.add("expanderClassName='" + expanderClassName + "'")
.toString();
}

/**
* Builder for a Target Method Parameter Definition.
*/
public static class Builder {

private String name;
private String type;
private Integer index;
private String expanderClassName;

/**
* Parameter Name.
*
* @param name of the parameter.
* @return a builder instance for chaining.
*/
public Builder name(String name) {
this.name = name;
return this;
}

/**
* Parameter Argument Index.
*
* @param index of the parameter in the method definition.
* @return a builder instance for chaining.
*/
public Builder index(Integer index) {
this.index = index;
return this;
}

/**
* Fully Qualified Class name of the parameter type.
*
* @param type of the parameter.
* @return a builder instance for chaining.
*/
public Builder type(String type) {
this.type = type;
return this;
}

/**
* Expression Expander Fully Qualified Class name to use when resolving this parameter value.
*
* @param expanderClassName of the {@link feign.template.ExpressionExpander}
* @return a builder instance for chaining.
*/
public Builder expanderClassName(String expanderClassName) {
this.expanderClassName = expanderClassName;
return this;
}

/**
* Create a new {@link TargetMethodParameterDefinition} from the builder properties.
*
* @return a new {@link TargetMethodParameterDefinition} instance.
*/
public TargetMethodParameterDefinition build() {
return new TargetMethodParameterDefinition(this.name, this.type, this.index,
this.expanderClassName);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2019-2020 OpenFeign Contributors
* Copyright 2019-2021 OpenFeign Contributors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -74,7 +74,7 @@ public Collection<TargetMethodDefinition> apply(Target<?> target) {

/* determine if implicit body parameter identification is required */
if (methodMetadata.getBody() == -1
&& parameters.length > methodMetadata.getTemplateParameters().size()) {
&& parameters.length > methodMetadata.getParameterDefinitions().size()) {
/* there are parameters on this method that are not registered. in these cases, we
* allow users to define which parameter they want as the Request Body without an explicit
* annotation, look for that parameter and register it.
Expand Down Expand Up @@ -137,5 +137,4 @@ protected abstract void processAnnotationsOnMethod(Class<?> targetType, Method m
protected abstract void processAnnotationsOnParameter(Parameter parameter, Integer parameterIndex,
TargetMethodDefinition.Builder targetMethodDefinitionBuilder);


}
37 changes: 15 additions & 22 deletions core/src/main/java/feign/contract/FeignContract.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2019-2020 OpenFeign Contributors
* Copyright 2019-2021 OpenFeign Contributors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -17,13 +17,11 @@
package feign.contract;

import feign.TargetMethodDefinition;
import feign.TargetMethodParameterDefinition;
import feign.http.HttpHeader;
import feign.http.HttpMethod;
import feign.support.StringUtils;
import feign.template.ExpanderRegistry;
import feign.template.ExpressionExpander;
import feign.template.SimpleTemplateParameter;
import feign.template.expander.CachingExpanderRegistry;
import feign.template.expander.DefaultExpander;
import java.lang.reflect.Method;
import java.lang.reflect.Parameter;
Expand All @@ -37,8 +35,6 @@
*/
public class FeignContract extends AbstractAnnotationDrivenContract {

private final ExpanderRegistry expanderRegistry = new CachingExpanderRegistry();

/**
* Creates a new Feign Contract.
*/
Expand Down Expand Up @@ -165,21 +161,20 @@ private void processHeader(Header header, TargetMethodDefinition.Builder targetM
*/
private void processParameter(Param parameter, Integer index, Class<?> type,
TargetMethodDefinition.Builder targetMethodDefinition) {
String name = parameter.value();
Class<? extends ExpressionExpander> expanderClass = parameter.expander();

/* inspect the type annotated */
ExpressionExpander expander;
if (this.isCustomExpander(expanderClass)) {
/* retrieve an instance of the custom expander */
expander = this.expanderRegistry.getExpander(expanderClass);
} else {
/* retrieve the expander from the registry by the parameter type */
expander = this.expanderRegistry.getExpanderByType(type);
}
String name = parameter.value();
String typeClass = type.getCanonicalName();

targetMethodDefinition.templateParameter(
index, new SimpleTemplateParameter(name, expander));
Class<? extends ExpressionExpander> expanderClass = parameter.expander();
String expanderClassName = expanderClass.getName();

targetMethodDefinition.parameterDefinition(
index, TargetMethodParameterDefinition.builder()
.name(name)
.index(index)
.type(typeClass)
.expanderClassName(expanderClassName)
.build());
}

/**
Expand Down Expand Up @@ -212,7 +207,5 @@ private Type getMethodReturnType(Method method) {
return method.getGenericReturnType();
}

private boolean isCustomExpander(Class<? extends ExpressionExpander> expanderClass) {
return DefaultExpander.class != expanderClass;
}

}
Loading

0 comments on commit a059858

Please sign in to comment.