firebase / firebase-admin-dotnet

Firebase Admin .NET SDK
https://firebase.google.com/docs/admin/setup
Apache License 2.0
369 stars 131 forks source link

Address feedback for user import unit tests #193

Closed MathBunny closed 4 years ago

MathBunny commented 4 years ago

This is a separate pull request because the other PR was getting too big (note that it will merge back into the other bulk-user-import branch).

Addressed feedback from the last round of review. Another note is that I brought up the Base64Url encoding prior, I think the implementation is ok as it is similar to the Java behavior and it is verified by the SignerKey being properly URL encoded in the tests here.

One remark is the addition of NullValueHandling = NullValueHandling.Ignore for all properties (except Uid) when being serialized for ImportUserRecordArgs.

Newtonsoft has the ability to choose to ignore null values when the serialize function is invoked, but in this case PostAndDeserializeAsync handles the serialization using NewtonsoftJsonSerializer.Instance.CreateJsonHttpContent and it would require editing the PostAndDeserializeAsync method. Ignoring the null values reduces the size of the serialized payload in the POST request.