Rework Amazon.Lambda.Serialization.SystemTextJson #636

normj · 2020-04-19T01:00:56Z

Issue #, if available:
#624
#628

Description of changes:
Amazon.Lambda.Serialization.SystemTextJson unintentionally used camel casing when serializing response objects to JSON. This is different then both Amazon.Lambda.Serialization.Json and the default behavior of System.Text.Json.

Fixing the behavior in LambdaJsonSerializer would be runtime breaking behavior and there is no metrics to know how impactful the change would be path. Instead I took the safer approach and created a new serializer called DefaultLambdaJsonSerializer that would use System.Text.Json's default casing behavior which is to use the casing matching the properties of the .NET property. For developers that did want the camel casing behavior there is also a new serializer called CamelCaseLambdaJsonSerializer.

LambdaJsonSerializer has been marked as Obsolete with no plans to change it going forward.

Here is a link to a zip file of a preview build of all of the packages for anybody that would like to test the change. https://normj-packages.s3.us-west-2.amazonaws.com/rework-serialization-preview2.zip

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

…e same as Amazon.Lambda.Serialization.Json.

bjorg · 2020-04-19T02:17:45Z

Libraries/src/Amazon.Lambda.Serialization.SystemTextJson/DefaultLambdaJsonSerializer.cs

+                IgnoreNullValues = true,
+                PropertyNameCaseInsensitive = true,
+                PropertyNamingPolicy = new AwsNamingPolicy()
+            };


A bit more modern notation, but functionality the same:

_options = new JsonSerializerOptions() { IgnoreNullValues = true, PropertyNameCaseInsensitive = true, PropertyNamingPolicy = new AwsNamingPolicy(), Converters = { new DateTimeConverter(), new MemoryStreamConverter(), new ConstantClassConverter() } };

bjorg · 2020-04-19T03:58:08Z

Libraries/src/Amazon.Lambda.Serialization.SystemTextJson/DefaultLambdaJsonSerializer.cs

+            _options = new JsonSerializerOptions()
+            {
+                IgnoreNullValues = true,
+                PropertyNameCaseInsensitive = true,


Is PropertyNameCaseInsensitive = true still needed?

Going from JSON to .NET Object you would still need it.

bjorg · 2020-04-20T15:53:40Z

Libraries/src/Amazon.Lambda.Serialization.SystemTextJson/DefaultLambdaJsonSerializer.cs

+        /// Amazon.Lambda.Serialization.SystemTextJson's default settings have been applied.
+        /// </summary>
+        /// <param name="customizer"></param>
+        public DefaultLambdaJsonSerializer(Action<JsonSerializerOptions> customizer)


An alternative would be to make _options protected so it can be accessed by a derived class.

If I'm not mistaken, the only way to customize the base class is by constructor since it's the AWS Lambda Runtime for .NET Core that instantiates this class, correct?

And by making protected, I meant adding a property. For example,

protected JsonSerializerOptions SerializerOptions => _options;

I need the _options object so I can add my own Serialize(Stream,Type) method to the serializer (needed by my middleware).

I was hoping I could use the Action<> callback in the constructor to store a reference, but alas, that's not legal in C#.

// does not work, because the lambda function has no 'this' context in the 'base' invocation public LambdaJsonSerializer() : base(options => _myOptions = options) { }

I turned the options into a protected property;

martincostello · 2020-04-20T17:01:11Z

...azon.Lambda.ApplicationLoadBalancerEvents/Amazon.Lambda.ApplicationLoadBalancerEvents.csproj

        <AssemblyName>Amazon.Lambda.ApplicationLoadBalancerEvents</AssemblyName>
        <PackageId>Amazon.Lambda.ApplicationLoadBalancerEvents</PackageId>
        <PackageTags>AWS;Amazon;Lambda</PackageTags>
    </PropertyGroup>
+
+    <PropertyGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
+        <DefineConstants>NETSTANDARD_2_0</DefineConstants>


Why not use the built-in NETSTANDARD_20 and NETCOREAPP_31 hash-defines?

+1. Although it looks like they are NETSTANDARD2_0 and NETCOREAPP3_1 - https://docs.microsoft.com/en-us/dotnet/core/tutorials/libraries

Thanks, I didn't realize there were built in symbols. Updated PR to use them.

… to it.

boblodgett · 2020-04-22T20:42:06Z

...azon.Lambda.ApplicationLoadBalancerEvents/Amazon.Lambda.ApplicationLoadBalancerEvents.csproj

        <AssemblyName>Amazon.Lambda.ApplicationLoadBalancerEvents</AssemblyName>
        <PackageId>Amazon.Lambda.ApplicationLoadBalancerEvents</PackageId>
        <PackageTags>AWS;Amazon;Lambda</PackageTags>
    </PropertyGroup>
+
+    <PropertyGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
+        <DefineConstants>NETSTANDARD_2_0</DefineConstants>


+1. Although it looks like they are NETSTANDARD2_0 and NETCOREAPP3_1 - https://docs.microsoft.com/en-us/dotnet/core/tutorials/libraries

boblodgett · 2020-04-22T20:48:29Z

Libraries/src/Amazon.Lambda.Serialization.SystemTextJson/AwsNamingPolicy.cs

+
+        /// <summary>
+        /// Creates the AWS Naming policy. If the name matches one of the reserved AWS words it will return the
+        /// appropriate mapping for it. Otherwise the name will be return as is like the JsonDefaultNamingPolicy.


return => returned or remove "be" before return.

boblodgett · 2020-04-22T20:54:00Z

Libraries/src/Amazon.Lambda.Serialization.SystemTextJson/AwsNamingPolicy.cs

+            if (_fallbackNamingPolicy == null)
+            {
+                // If no naming policy given then just return the name like the JsonDefaultNamingPolicy policy.
+                // https://github.com/dotnet/runtime/blob/master/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonDefaultNamingPolicy.cs
+                return name;
+            }
+            else
+            {
+                return _fallbackNamingPolicy.ConvertName(name);
+            }


nit: if .ConvertName(name) can't be null then you could do:

return __fallbackNamingPolicy?.ConvertName(name) ?? name;

boblodgett · 2020-04-22T21:02:23Z

Libraries/src/Amazon.Lambda.Serialization.SystemTextJson/DefaultLambdaJsonSerializer.cs

+            if (string.Equals(Environment.GetEnvironmentVariable(DEBUG_ENVIRONMENT_VARIABLE_NAME), "true", StringComparison.OrdinalIgnoreCase))
+            {
+                this._debug = true;
+            }            


nit: should be ok to do this...

this._debug = string.Equals(Environment.GetEnvironmentVariable(DEBUG_ENVIRONMENT_VARIABLE_NAME), "true", StringComparison.OrdinalIgnoreCase);

normj · 2020-04-28T03:28:57Z

I have merged into dev. I'll post back when the change has been published.

normj · 2020-04-29T00:57:51Z

Version 2.0.0 went with this PR

Rework Amazon.Lambda.Serialization.SystemTextJson to handle casing th…

d013d09

…e same as Amazon.Lambda.Serialization.Json.

bjorg reviewed Apr 19, 2020

View reviewed changes

bjorg reviewed Apr 20, 2020

View reviewed changes

martincostello reviewed Apr 20, 2020

View reviewed changes

Make the json options protected for custom serializers to have access…

c8906a0

… to it.

normj force-pushed the rework-serialization branch from abdfe05 to c8906a0 Compare April 21, 2020 03:13

boblodgett requested review from boblodgett and SrutiG April 22, 2020 18:06

boblodgett reviewed Apr 22, 2020

View reviewed changes

normj added 2 commits April 22, 2020 23:24

Switch to built in build symbols

7960351

Handle PR comments

0f0f1a3

boblodgett approved these changes Apr 23, 2020

View reviewed changes

SrutiG approved these changes Apr 23, 2020

View reviewed changes

bjorg approved these changes Apr 26, 2020

View reviewed changes

normj merged commit bfb2cbb into dev Apr 28, 2020

normj mentioned this pull request Apr 28, 2020

Respect JsonSerializerOptions in debug mode #629

Closed

normj deleted the rework-serialization branch October 7, 2021 03:25

normj restored the rework-serialization branch October 7, 2021 03:25

normj deleted the rework-serialization branch October 7, 2021 03:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework Amazon.Lambda.Serialization.SystemTextJson #636

Rework Amazon.Lambda.Serialization.SystemTextJson #636

normj commented Apr 19, 2020 •

edited

Loading

bjorg Apr 19, 2020

normj Apr 21, 2020

bjorg Apr 19, 2020

normj Apr 21, 2020

bjorg Apr 20, 2020

bjorg Apr 20, 2020

bjorg Apr 20, 2020 •

edited

Loading

normj Apr 21, 2020

martincostello Apr 20, 2020

boblodgett Apr 22, 2020

normj Apr 23, 2020

boblodgett Apr 22, 2020

boblodgett Apr 22, 2020

normj Apr 23, 2020

boblodgett Apr 22, 2020

normj Apr 23, 2020

boblodgett Apr 22, 2020

normj Apr 23, 2020

normj commented Apr 28, 2020

normj commented Apr 29, 2020

Rework Amazon.Lambda.Serialization.SystemTextJson #636

Rework Amazon.Lambda.Serialization.SystemTextJson #636

Conversation

normj commented Apr 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bjorg Apr 20, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

normj commented Apr 28, 2020

normj commented Apr 29, 2020

normj commented Apr 19, 2020 •

edited

Loading

bjorg Apr 20, 2020 •

edited

Loading