-
Notifications
You must be signed in to change notification settings - Fork 479
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
Update serialization support for Amazon.Lambda.CognitoEvents & Code Generated JsonSerlizationContext's #1567
Comments
Needs review with the team. |
Hi @Simonl9l I didn't add the JsonSerializationContext into the event packages because outside of AOT/Trimming this would increase the size of the assemblies if users weren't using the source generator. Also in event packages with multiple events there can be collisions of you add them all to the serialization context. For example in API Gateway event package if I added both the REST API and HTTP API request/response they cause a collision because the share some of the same names of types. So if we can't always create the monolithic serialization context for each package we have to figure out a scheme for grouping the contexts together. All this work was done before AOT was a think for .NET. I agree now that we have AOT we need to figure out a strategy to make this work. We adding a serialization context in the event package help because the converters are in the A good call on the |
@normj Thanks for the considered response! As we understand it, if say to serialize you specify the type into which you want to deserialize, that implicitly includes namespaces per Do you have a specific example - as perhaps we're not following? Patterns we have seen elsewhere is to add a serialization context's that are more granular on the basis they can all be chained in with .Net 7 - at least in regard to the minimal AI Perhaps:
An issue we have run into elsewhere, is where packages have been published in a trimmed state, such that you can't create ones own serializer contexts as there is no Type Info available. It also worth noting that From a 10,000ft AoT perspective we see the biggest challenges with the AWS dotnet package library including SDK and it's significant use of reflection - especially in the DI extensions. We believe that significant portions (inc Minimal API) of APS.NET Core will support AoT in .Net 8! Glad to help per Of note per (*) |
@norm & @ashishdhingra gentlemen, its been a while and I find myself back here specifically as it relates to AoT. However getting our efforts to fully AoT is a journey give dependency support, of some of the libraries we're using. To ensure we're using the code generates serilaization correctly for AoT (as we get there), were using the Per the #1567 (comment) above, the reply only covered the second part of my feature request. We're use 3rd party libraries that do have their own serialization contexts. In the spirit of alignment with .Net (8.0) it would be great if when using Relates to the .Net8.0 support are there any plans here in the short to medium term in they area. It's also great to see Thanks again for all the efforts! |
Ah after some more digging...this is possible: var builder = LambdaBootstrapBuilder.Create<JsonElement, JsonElement>(FunctionHandler, new SourceGeneratorLambdaJsonSerializer<LambdaSerializationContext>(
options =>
{
options.TypeInfoResolverChain.Add(MyOtherSerializationContext.Default);
}));
private async Task<JsonElement> FunctionHandler(JsonElement request, ILambdaContext context)
{
...
} |
Describe the feature
Per https://github.com/aws/aws-lambda-dotnet/tree/master/Libraries/src/Amazon.Lambda.CognitoEvents
It would likely be valuable to the dev community if the packages such that this support
system.text.json
code generated serialization and providedJsonSerlizationContext
(s).Use Case
The value proposition, specifically in regard to
Amazon.Lambda.CognitoEvents
is that Lambda's implementing Cognito Trigger, say via the Annotations library or raw Lambda Handler implementation have specific response timing constraints/requirements that make AoT a requirement, that in turn requires code generated serialization.This likely applies to all the Lambda Event packages, that in some cases is more applicable to Minimal API than Annotations.
Whist one can can define one's own code generated serialization contexts for type sin other packages, it can have undesirable effects with any trimmed assemblies as the metadata to generate the serializer is not available - see dotnet/runtime#78029.
Proposed Solution
These
JsonSerlizationContext
(s) can be added into the package and can then be used explicit, or configured into Lambda by chaining theIJsonTypeInfoResolver
resolvers. For example:The
system.text.json
support to combination of serialization contexts:and configured in say a Minimal API context with (.Net7)
However the
SourceGeneratorLambdaJsonSerializer
would need some updates to accept theIJsonTypeInfoResolver
from the combine.Other Information
Related there see to be numerous ongoing conversation regarding
System.Text.Json
support for pre-existing contracts such as[DataMember]
etc. but expect that ton of these solutions supportSystem.Text.Json
code generation and JsonSerlizationContext's.Whilst this code mostly was updated a year ago, it seem to make the premise that
#if NETCOREAPP3_1
be the correct define to determine if to use[DataMember]
or[JsonPropertyName]
attributes in support ofSystem.Text.Json
.Per the MSFT docs NETCOREAPP3_1 preprocessor symbol is not defined when targeting .NET 5 - Recommended Action seems to indicate it should use
#if NETCOREAPP
.Acknowledgements
AWS .NET SDK and/or Package version used
Amazon.Lambda.CognitoEvents 2.1.1
Targeted .NET Platform
.Net 7 and above
Operating System and version
All
The text was updated successfully, but these errors were encountered: