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

Add trim support for Amazon.Lambda.Annotations #1610

Merged
merged 3 commits into from Nov 15, 2023

Conversation

normj
Copy link
Member

@normj normj commented Nov 14, 2023

Description of changes:
Add trimming support for Lambda Annotations. Using trimming requires users to use the SourceGeneratorLambdaJsonSerializer lambda serializer so that JSON serialization is not being done via reflection.

The HttpResults class was modified to use the JsonSerializerContext that the SourceGeneratorLambdaJsonSerializer is registered with via the generic parameter. It uses the JsonSerializerContext to serialize the HTTP response body. It means the user has to register any types returned via HttpResults in the body with the JsonSerializerContext via the JsonSerializable attribute.

Another slight change to HttpResults is it used to handle the serialization as soon as part of the constructor but there is no way to get the JsonSerializerContext into the constructor. So I changed body serialization to the when we serialize the HttpResults. This affected the unit tests because they were unintended reusing the same IHttpResult for each serialization format. So I change the test to create a new instance every time we run the serializer.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@@ -14,9 +14,9 @@ public class HttpResultsTest
[Fact]
public void OkNoBody()
{
var result = HttpResults.Ok();
var result = () => HttpResults.Ok();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the change mention in the description where ValidateResult runs across the different combinations of API Gateway responses. The unit tests were change to pass in a Func instead of a single instance so we didn't reuse the same IHttpResult for each iteration in ValidateResult.

@@ -162,9 +162,9 @@ public void Execute(GeneratorExecutionContext context)
}
}

var serializerString = GetSerializerAttribute(context, lambdaMethodModel);
var serializerInfo = GetSerializerInfoAttribute(context, lambdaMethodModel);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was changed to GetSerializerInfoAttribute because now we are getting the optional JsonSerializerContext as well as the serializer name.

Copy link
Contributor

@jeastham1993 jeastham1993 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looks good to me. Testing with .NET 8 and native AOT and it works perfectly.

@normj normj merged commit 146ce67 into feature/annotations-executable Nov 15, 2023
2 of 3 checks passed
@normj normj deleted the normj/annotations-trimming branch November 15, 2023 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants