Skip to content
Permalink
Browse files Browse the repository at this point in the history
Use ConversionContext constants where possible instead of class (#2356)
Changes
-------
 * Added ArgumentConversionContext constants in ConversionContext
 * Replaced Argument.of and use of argument classes with
ConversionContext constants where possible
 * Added getFirst method in ConvertibleMultiValues that accepts
ArgumentConversionContent parameter

Partially addresses issue #2355
  • Loading branch information
fabienrenaud authored and graemerocher committed Nov 18, 2019
1 parent 07424ba commit b8ec32c
Show file tree
Hide file tree
Showing 18 changed files with 145 additions and 59 deletions.
Expand Up @@ -41,6 +41,31 @@ public interface ConversionContext extends AnnotationMetadataProvider, TypeVaria
ConversionContext DEFAULT = new ConversionContext() {
};

/**
* Constant for Boolean argument.
*/
ArgumentConversionContext<Boolean> BOOLEAN = ConversionContext.of(Argument.BOOLEAN);

/**
* Constant for Integer argument.
*/
ArgumentConversionContext<Integer> INT = ConversionContext.of(Argument.INT);

/**
* Constant for Long argument.
*/
ArgumentConversionContext<Long> LONG = ConversionContext.of(Argument.LONG);

/**
* Constant for String argument.
*/
ArgumentConversionContext<String> STRING = ConversionContext.of(Argument.STRING);

/**
* Constant for List<String> argument.
*/
ArgumentConversionContext<List<String>> LIST_OF_STRING = ConversionContext.of(Argument.LIST_OF_STRING);

/**
* In the case where the type to be converted contains generic type arguments this map will return
* the concrete types of those arguments. For example for the {@link Map} type two keys will be present
Expand Down
Expand Up @@ -19,13 +19,7 @@
import io.micronaut.core.type.Argument;

import java.nio.charset.Charset;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.*;

/**
* Default implementation of the {@link ConversionContext} interface.
Expand Down Expand Up @@ -114,6 +108,25 @@ public Argument<T> getArgument() {
return argument;
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
DefaultArgumentConversionContext<?> that = (DefaultArgumentConversionContext<?>) o;
return Objects.equals(getArgument(), that.getArgument()) &&
Objects.equals(finalLocale, that.finalLocale) &&
Objects.equals(finalCharset, that.finalCharset);
}

@Override
public int hashCode() {
return Objects.hash(argument, finalLocale, finalCharset);
}

@Override
public String toString() {
return argument.toString();
Expand Down
Expand Up @@ -15,6 +15,7 @@
*/
package io.micronaut.core.convert.value;

import io.micronaut.core.convert.ArgumentConversionContext;
import io.micronaut.core.convert.ConversionContext;
import io.micronaut.core.convert.ConversionService;
import io.micronaut.core.reflect.GenericTypeUtils;
Expand Down Expand Up @@ -170,6 +171,22 @@ default <T> Optional<T> getFirst(CharSequence name, Argument<T> requiredType) {
return Optional.empty();
}

/**
* Find a header and convert it to the given type.
*
* @param name The name of the header
* @param conversionContext The conversion context
* @param <T> The generic type
* @return If the header is presented and can be converted an optional of the value otherwise {@link Optional#empty()}
*/
default <T> Optional<T> getFirst(CharSequence name, ArgumentConversionContext<T> conversionContext) {
V v = get(name);
if (v != null) {
return ConversionService.SHARED.convert(v, conversionContext);
}
return Optional.empty();
}

/**
* Find a header and convert it to the given type.
*
Expand Down
Expand Up @@ -70,7 +70,7 @@ default Class<V> getValueType() {
* @return True if it is
*/
default boolean contains(String name) {
return get(name, Object.class).isPresent();
return get(name, Argument.OBJECT_ARGUMENT).isPresent();
}

/**
Expand Down
28 changes: 19 additions & 9 deletions core/src/main/java/io/micronaut/core/type/Argument.java
Expand Up @@ -40,59 +40,64 @@
*/
public interface Argument<T> extends TypeVariableResolver, AnnotatedElement, Type {

/**
* Constant for string argument.
*/
Argument<String> STRING = Argument.of(String.class);

/**
* Constant for int argument. Used by generated code, do not remove.
*/
@SuppressWarnings("unused")
Argument INT = Argument.of(int.class);
Argument<Integer> INT = Argument.of(int.class);

/**
* Constant for long argument. Used by generated code, do not remove.
*/
@SuppressWarnings("unused")
Argument LONG = Argument.of(long.class);
Argument<Long> LONG = Argument.of(long.class);

/**
* Constant for float argument. Used by generated code, do not remove.
*/
@SuppressWarnings("unused")
Argument FLOAT = Argument.of(float.class);
Argument<Float> FLOAT = Argument.of(float.class);

/**
* Constant for double argument. Used by generated code, do not remove.
*/
@SuppressWarnings("unused")
Argument DOUBLE = Argument.of(double.class);
Argument<Double> DOUBLE = Argument.of(double.class);

/**
* Constant for void argument. Used by generated code, do not remove.
*/
@SuppressWarnings("unused")
Argument VOID = Argument.of(void.class);
Argument<Void> VOID = Argument.of(void.class);

/**
* Constant for byte argument. Used by generated code, do not remove.
*/
@SuppressWarnings("unused")
Argument BYTE = Argument.of(byte.class);
Argument<Byte> BYTE = Argument.of(byte.class);

/**
* Constant for boolean argument. Used by generated code, do not remove.
*/
@SuppressWarnings("unused")
Argument BOOLEAN = Argument.of(boolean.class);
Argument<Boolean> BOOLEAN = Argument.of(boolean.class);

/**
* Constant char argument. Used by generated code, do not remove.
*/
@SuppressWarnings("unused")
Argument CHAR = Argument.of(char.class);
Argument<Character> CHAR = Argument.of(char.class);

/**
* Constant short argument. Used by generated code, do not remove.
*/
@SuppressWarnings("unused")
Argument SHORT = Argument.of(short.class);
Argument<Short> SHORT = Argument.of(short.class);

/**
* Constant representing zero arguments. Used by generated code, do not remove.
Expand All @@ -107,6 +112,11 @@
@SuppressWarnings("unused")
Argument<Object> OBJECT_ARGUMENT = of(Object.class);

/**
* Constant for List<String> argument.
*/
Argument<List<String>> LIST_OF_STRING = Argument.listOf(String.class);

/**
* @return The name of the argument
*/
Expand Down
Expand Up @@ -47,11 +47,7 @@
* @param genericTypes The generic types
*/
public DefaultArgument(Class<T> type, String name, AnnotationMetadata annotationMetadata, Argument... genericTypes) {
this.type = type;
this.name = name;
this.annotationMetadata = annotationMetadata != null ? annotationMetadata : AnnotationMetadata.EMPTY_METADATA;
this.typeParameters = initializeTypeParameters(genericTypes);
this.typeParameterArray = genericTypes;
this(type, name, annotationMetadata, initializeTypeParameters(genericTypes), genericTypes);
}

/**
Expand Down Expand Up @@ -184,7 +180,7 @@ public int hashCode() {
return Objects.hash(type, name, typeParameters);
}

private Map<String, Argument<?>> initializeTypeParameters(Argument[] genericTypes) {
private static Map<String, Argument<?>> initializeTypeParameters(Argument[] genericTypes) {
Map<String, Argument<?>> typeParameters;
if (genericTypes != null && genericTypes.length > 0) {
typeParameters = new LinkedHashMap<>(genericTypes.length);
Expand Down
Expand Up @@ -71,7 +71,7 @@ default Argument<?> getErrorType(MediaType mediaType) {
} else if (mediaType.equals(MediaType.APPLICATION_VND_ERROR_TYPE)) {
return Argument.of(VndError.class);
} else {
return Argument.of(String.class);
return Argument.STRING;
}
}
}
Expand Up @@ -141,6 +141,7 @@ class RoutingInBoundHandler extends SimpleChannelInboundHandler<io.micronaut.htt
private static final Logger LOG = LoggerFactory.getLogger(RoutingInBoundHandler.class);
private static final Pattern IGNORABLE_ERROR_MESSAGE = Pattern.compile(
"^.*(?:connection.*(?:reset|closed|abort|broken)|broken.*pipe).*$", Pattern.CASE_INSENSITIVE);
private static final Argument ARGUMENT_PART_DATA = Argument.of(PartData.class);

private final Router router;
private final ExecutorSelector executorSelector;
Expand Down Expand Up @@ -769,7 +770,7 @@ protected void doOnNext(Object message) {
Argument typeVariable;

if (StreamingFileUpload.class.isAssignableFrom(argument.getType())) {
typeVariable = Argument.of(PartData.class);
typeVariable = ARGUMENT_PART_DATA;
} else {
typeVariable = argument.getFirstTypeVariable().orElse(Argument.OBJECT_ARGUMENT);
}
Expand All @@ -784,7 +785,7 @@ protected void doOnNext(Object message) {
if (Publishers.isConvertibleToPublisher(typeVariableType)) {
boolean streamingFileUpload = StreamingFileUpload.class.isAssignableFrom(typeVariableType);
if (streamingFileUpload) {
typeVariable = Argument.of(PartData.class);
typeVariable = ARGUMENT_PART_DATA;
} else {
typeVariable = typeVariable.getFirstTypeVariable().orElse(Argument.OBJECT_ARGUMENT);
}
Expand Down
Expand Up @@ -49,6 +49,8 @@
public class CompletableFutureBodyBinder extends DefaultBodyAnnotationBinder<CompletableFuture>
implements NonBlockingBodyArgumentBinder<CompletableFuture> {

private static final Argument<CompletableFuture> TYPE = Argument.of(CompletableFuture.class);

private final BeanLocator beanLocator;
private final HttpServerConfiguration httpServerConfiguration;

Expand All @@ -65,7 +67,7 @@ public CompletableFutureBodyBinder(BeanLocator beanLocator, HttpServerConfigurat

@Override
public Argument<CompletableFuture> argumentType() {
return Argument.of(CompletableFuture.class);
return TYPE;
}

@Override
Expand Down
Expand Up @@ -54,6 +54,7 @@
public class PublisherBodyBinder extends DefaultBodyAnnotationBinder<Publisher> implements NonBlockingBodyArgumentBinder<Publisher> {

private static final Logger LOG = LoggerFactory.getLogger(NettyHttpServer.class);
private static final Argument<Publisher> TYPE = Argument.of(Publisher.class);

private final BeanLocator beanLocator;
private final HttpServerConfiguration httpServerConfiguration;
Expand All @@ -71,7 +72,7 @@ public PublisherBodyBinder(ConversionService conversionService, BeanLocator bean

@Override
public Argument<Publisher> argumentType() {
return Argument.of(Publisher.class);
return TYPE;
}

@Override
Expand Down
Expand Up @@ -15,6 +15,7 @@
*/
package io.micronaut.http.server.netty.cors

import io.micronaut.core.convert.ConversionContext
import io.micronaut.core.type.Argument
import io.micronaut.http.HttpHeaders
import io.micronaut.http.HttpMethod
Expand Down Expand Up @@ -146,7 +147,7 @@ class CorsFilterSpec extends Specification {
2 * headers.getOrigin() >> Optional.of('http://www.foo.com')
1 * request.getMethod() >> HttpMethod.GET
!result.isPresent()
0 * headers.get(ACCESS_CONTROL_REQUEST_HEADERS, Argument.of(List,String))
0 * headers.get(ACCESS_CONTROL_REQUEST_HEADERS, ConversionContext.of(Argument.of(List,String)))
}
void "test preflight handleRequest with disallowed header"() {
Expand All @@ -170,8 +171,8 @@ class CorsFilterSpec extends Specification {
then: "the request is rejected because bar is not allowed"
2 * headers.getOrigin() >> Optional.of('http://www.foo.com')
1 * headers.getFirst(ACCESS_CONTROL_REQUEST_METHOD, HttpMethod.class) >> Optional.of(HttpMethod.GET)
1 * headers.get(ACCESS_CONTROL_REQUEST_HEADERS, Argument.of(List,String)) >> ['foo', 'bar']
1 * headers.getFirst(ACCESS_CONTROL_REQUEST_METHOD, ConversionContext.of(HttpMethod.class)) >> Optional.of(HttpMethod.GET)
1 * headers.get(ACCESS_CONTROL_REQUEST_HEADERS, ConversionContext.of(Argument.of(List,String))) >> ['foo', 'bar']
result.get().status == HttpStatus.FORBIDDEN
}
Expand All @@ -196,8 +197,8 @@ class CorsFilterSpec extends Specification {
then: "the request is successful"
4 * headers.getOrigin() >> Optional.of('http://www.foo.com')
2 * headers.getFirst(ACCESS_CONTROL_REQUEST_METHOD, HttpMethod.class) >> Optional.of(HttpMethod.GET)
2 * headers.get(ACCESS_CONTROL_REQUEST_HEADERS, Argument.of(List,String)) >> Optional.of(['foo'])
2 * headers.getFirst(ACCESS_CONTROL_REQUEST_METHOD, ConversionContext.of(HttpMethod.class)) >> Optional.of(HttpMethod.GET)
2 * headers.get(ACCESS_CONTROL_REQUEST_HEADERS, ConversionContext.of(Argument.of(List,String))) >> Optional.of(['foo'])
result.get().status == HttpStatus.OK
}
Expand Down Expand Up @@ -274,8 +275,8 @@ class CorsFilterSpec extends Specification {
HttpResponse response = corsHandler.handleRequest(request).get()
then: "the response is not modified"
2 * headers.get(ACCESS_CONTROL_REQUEST_HEADERS, Argument.of(List,String)) >> Optional.of(['X-Header', 'Y-Header'])
1 * headers.getFirst(ACCESS_CONTROL_REQUEST_METHOD, HttpMethod.class) >> Optional.of(HttpMethod.GET)
2 * headers.get(ACCESS_CONTROL_REQUEST_HEADERS, ConversionContext.of(Argument.of(List,String))) >> Optional.of(['X-Header', 'Y-Header'])
1 * headers.getFirst(ACCESS_CONTROL_REQUEST_METHOD, ConversionContext.of(HttpMethod.class)) >> Optional.of(HttpMethod.GET)
response.getHeaders().get(ACCESS_CONTROL_ALLOW_METHODS) == 'GET'
response.getHeaders().get(ACCESS_CONTROL_ALLOW_ORIGIN) == 'http://www.foo.com' // The origin is echo'd
response.getHeaders().get(VARY) == 'Origin' // The vary header is set
Expand Down Expand Up @@ -305,8 +306,8 @@ class CorsFilterSpec extends Specification {
HttpResponse response = corsHandler.handleRequest(request).get()
then: "the response is not modified"
2 * headers.get(ACCESS_CONTROL_REQUEST_HEADERS, Argument.of(List,String)) >> Optional.of(['X-Header', 'Y-Header'])
1 * headers.getFirst(ACCESS_CONTROL_REQUEST_METHOD, HttpMethod.class) >> Optional.of(HttpMethod.GET)
2 * headers.get(ACCESS_CONTROL_REQUEST_HEADERS, ConversionContext.of(Argument.of(List,String))) >> Optional.of(['X-Header', 'Y-Header'])
1 * headers.getFirst(ACCESS_CONTROL_REQUEST_METHOD, ConversionContext.of(HttpMethod.class)) >> Optional.of(HttpMethod.GET)
response.getHeaders().get(ACCESS_CONTROL_ALLOW_METHODS) == 'GET'
response.getHeaders().get(ACCESS_CONTROL_ALLOW_ORIGIN) == 'http://www.foo.com' // The origin is echo'd
response.getHeaders().get(VARY) == 'Origin' // The vary header is set
Expand Down

0 comments on commit b8ec32c

Please sign in to comment.