-
Notifications
You must be signed in to change notification settings - Fork 81
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
Move away from Jackson #1323
base: 5.0.x
Are you sure you want to change the base?
Move away from Jackson #1323
Changes from 10 commits
f8d1ac3
fadb85b
0fc14d6
d22a685
8e6df2c
f791a7c
46ad900
0233b83
0d4ca45
3f00ec0
6446fc0
02afcb5
8089f30
dca8a6c
ce9dcbf
cb73990
6fecc10
df8d67c
599267d
745a03d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,10 +15,10 @@ | |
*/ | ||
package io.micronaut.function.aws.proxy; | ||
|
||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import io.micronaut.context.ApplicationContextProvider; | ||
import io.micronaut.function.aws.MicronautLambdaContext; | ||
import io.micronaut.jackson.codec.JsonMediaTypeCodec; | ||
import io.micronaut.json.JsonMapper; | ||
import io.micronaut.json.codec.JsonMediaTypeCodec; | ||
import io.micronaut.web.router.Router; | ||
|
||
/** | ||
|
@@ -40,9 +40,9 @@ public interface MicronautLambdaContainerContext extends ApplicationContextProvi | |
JsonMediaTypeCodec getJsonCodec(); | ||
|
||
/** | ||
* @return The Jackson's {@link ObjectMapper} | ||
* @return The {@link JsonMapper} | ||
*/ | ||
default ObjectMapper getObjectMapper() { | ||
return getJsonCodec().getObjectMapper(); | ||
default JsonMapper getObjectMapper() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. JsonCodec returns a JsonMapper, which isn't an ObjectMapper 😔 |
||
return getJsonCodec().getJsonMapper(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,12 +19,18 @@ | |
import com.amazonaws.serverless.proxy.AwsProxySecurityContextWriter; | ||
import com.amazonaws.serverless.proxy.internal.jaxrs.AwsProxySecurityContext; | ||
import com.amazonaws.serverless.proxy.internal.testutils.Timer; | ||
import com.amazonaws.serverless.proxy.model.*; | ||
import com.amazonaws.serverless.proxy.model.AlbContext; | ||
import com.amazonaws.serverless.proxy.model.ApiGatewayAuthorizerContext; | ||
import com.amazonaws.serverless.proxy.model.ApiGatewayRequestIdentity; | ||
import com.amazonaws.serverless.proxy.model.AwsProxyRequest; | ||
import com.amazonaws.serverless.proxy.model.AwsProxyRequestContext; | ||
import com.amazonaws.serverless.proxy.model.AwsProxyResponse; | ||
import com.amazonaws.serverless.proxy.model.CognitoAuthorizerClaims; | ||
import com.amazonaws.serverless.proxy.model.ContainerConfig; | ||
import com.amazonaws.serverless.proxy.model.ErrorModel; | ||
import com.amazonaws.serverless.proxy.model.Headers; | ||
import com.amazonaws.serverless.proxy.model.MultiValuedTreeMap; | ||
import com.amazonaws.services.lambda.runtime.Context; | ||
import com.fasterxml.jackson.databind.JsonNode; | ||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import com.fasterxml.jackson.databind.ObjectReader; | ||
import com.fasterxml.jackson.databind.ObjectWriter; | ||
import io.micronaut.context.ApplicationContext; | ||
import io.micronaut.context.ApplicationContextBuilder; | ||
import io.micronaut.context.ApplicationContextProvider; | ||
|
@@ -55,8 +61,11 @@ | |
import io.micronaut.http.server.exceptions.response.ErrorContext; | ||
import io.micronaut.http.server.exceptions.response.ErrorResponseProcessor; | ||
import io.micronaut.inject.qualifiers.Qualifiers; | ||
import io.micronaut.jackson.codec.JsonMediaTypeCodec; | ||
import io.micronaut.json.JsonMapper; | ||
import io.micronaut.json.codec.JsonMediaTypeCodec; | ||
import io.micronaut.scheduling.executor.ExecutorSelector; | ||
import io.micronaut.serde.ObjectMapper; | ||
import io.micronaut.serde.annotation.SerdeImport; | ||
import io.micronaut.web.router.MethodBasedRouteMatch; | ||
import io.micronaut.web.router.RouteInfo; | ||
import io.micronaut.web.router.RouteMatch; | ||
|
@@ -77,6 +86,7 @@ | |
import java.util.Collection; | ||
import java.util.HashSet; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Optional; | ||
import java.util.Set; | ||
import java.util.concurrent.CountDownLatch; | ||
|
@@ -107,6 +117,18 @@ | |
AwsProxySecurityContext.class | ||
} | ||
) | ||
@SerdeImport(AlbContext.class) | ||
@SerdeImport(ApiGatewayAuthorizerContext.class) | ||
@SerdeImport(ApiGatewayRequestIdentity.class) | ||
@SerdeImport(AwsProxyRequest.class) | ||
@SerdeImport(AwsProxyRequestContext.class) | ||
@SerdeImport(AwsProxyResponse.class) | ||
@SerdeImport(CognitoAuthorizerClaims.class) | ||
@SerdeImport(ContainerConfig.class) | ||
@SerdeImport(ErrorModel.class) | ||
@SerdeImport(Headers.class) | ||
@SerdeImport(MultiValuedTreeMap.class) | ||
@SerdeImport(AwsProxySecurityContext.class) | ||
public final class MicronautLambdaContainerHandler | ||
extends AbstractLambdaContainerHandler<AwsProxyRequest, AwsProxyResponse, MicronautAwsProxyRequest<?>, MicronautAwsProxyResponse<?>> implements ApplicationContextProvider, Closeable, AutoCloseable { | ||
|
||
|
@@ -202,18 +224,18 @@ public ApplicationContext getApplicationContext() { | |
} | ||
|
||
@Override | ||
protected ObjectMapper objectMapper() { | ||
protected JsonMapper objectMapper() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not use |
||
return lambdaContainerEnvironment.getObjectMapper(); | ||
} | ||
|
||
@Override | ||
protected ObjectWriter writerFor(Class<AwsProxyResponse> responseClass) { | ||
return objectMapper().writerFor(responseClass); | ||
return (outputStream, value) -> outputStream.write(objectMapper().writeValueAsBytes(value)); | ||
} | ||
|
||
@Override | ||
protected ObjectReader readerFor(Class<AwsProxyRequest> requestClass) { | ||
return objectMapper().readerFor(requestClass); | ||
protected ObjectReader<AwsProxyRequest> readerFor(Class<AwsProxyRequest> requestClass) { | ||
return input -> objectMapper().readValue(input, Argument.of(requestClass)); | ||
} | ||
|
||
@Override | ||
|
@@ -422,12 +444,10 @@ private void decodeRequestBody(MicronautAwsProxyRequest<?> containerRequest, Rou | |
bodyArgument = bodyArgument.getFirstTypeVariable().orElse(Argument.OBJECT_ARGUMENT); | ||
} | ||
final Object decoded = lambdaContainerEnvironment.getJsonCodec().decode(bodyArgument, body.get()); | ||
((MicronautAwsProxyRequest) containerRequest) | ||
.setDecodedBody(decoded); | ||
((MicronautAwsProxyRequest) containerRequest).setDecodedBody(decoded); | ||
} else { | ||
final JsonNode jsonNode = lambdaContainerEnvironment.getJsonCodec().decode(JsonNode.class, body.get()); | ||
((MicronautAwsProxyRequest) containerRequest) | ||
.setDecodedBody(jsonNode); | ||
final Map<?, ?> node = lambdaContainerEnvironment.getJsonCodec().decode(Map.class, body.get()); | ||
((MicronautAwsProxyRequest) containerRequest).setDecodedBody(node); | ||
Comment on lines
+449
to
+450
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can't decode to a serde JsonNode here (it fails to create one), so I fell back to a Map. This will break if the body is not a map (ie: a simple json string), but I'm not sure if Lambdas can receive simple strings |
||
} | ||
} | ||
} | ||
|
@@ -467,7 +487,7 @@ private void applyRouteConfig(MicronautAwsProxyResponse<?> containerResponse, Ro | |
if (!containerResponse.getContentType().isPresent()) { | ||
finalRoute.getAnnotationMetadata().getValue(Produces.class, String.class).ifPresent(containerResponse::contentType); | ||
} | ||
finalRoute.getAnnotationMetadata().getValue(Status.class, HttpStatus.class).ifPresent(httpStatus -> containerResponse.status(httpStatus)); | ||
finalRoute.getAnnotationMetadata().getValue(Status.class, HttpStatus.class).ifPresent(containerResponse::status); | ||
} | ||
|
||
private void handlePossibleErrorStatus( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a
import io.micronaut.serde.ObjectMapper;
. Why not return it instead?