Patch doesn't output correct JSON

[rdvanbuuren]

Is there any description available on how to use these PATCH function(s)?

Not sure if I should report any issues here, but when calling a method like PatchApplication from the Netcore client, the JSON isn't wrapped correctly with application { }. According to the API (https://fusionauth.io/docs/v1/tech/apis/applications#update-an-application), it should be the same like the PUT request:

When using the PATCH method, use the same request body documentation that is provided for the PUT request.

The client code only passes the keys/values, but doesn't wrap it in the correct request body:

public ClientResponse<ApplicationResponse> PatchApplication(Guid? applicationId, Dictionary<string, object> request) {
      return buildClient()
          .withUri("/api/application")
          .withUriSegment(applicationId)
          .withJSONBody(request)
          .withMethod("Patch")
          .go<ApplicationResponse>();
    }

Calling this method with the following:

var request = new Dictionary<string, object> {
  { "passwordlessConfiguration", false },
  { "registrationConfiguration", true }
};
client.PatchApplication(applicationId, request);

Will result in the following incorrect JSON:

{
  "passwordlessConfiguration": {
    "enabled": false
  },
  "registrationConfiguration": {
    "enabled": true
  }
}

The correct expected JSON should be:

{
  "application": {
    "passwordlessConfiguration": {
      "enabled": false
    },
    "registrationConfiguration": {
      "enabled": true
    }
  }
}

[matthew-altman]

@rdvanbuuren I think this may be a documentation issue. Since these libraries just use our API, and the PatchApplication() method uses this API https://fusionauth.io/docs/v1/tech/apis/applications#update-an-application it would be using the same JSON format as described there. So the Dictionary<string,object> would have to first contain a top level application with nested params under it, just as if you were generating the JSON for manually calling the API. We can't use the ApplicationRequest domain object since the null values will be ambiguous here. I'll look into updating the documentation to make this more clear. But at the same time I'll look into seeing if the client can be simplified in any way. Thanks!

[lukevp]

The Patch_Application_Test() unit test uses this approach... While this is workable, it would be much more .net friendly if these were strongly typed instead of being objects. Perhaps we could discuss some ways to accomplish that?

var response = test.client.PatchApplication(test.application.id, new Dictionary<string, object> {
        {
          "application", new Dictionary<string, object> {
            {"passwordlessConfiguration", new Dictionary<string, object> {
              {"enabled", false}
            }},
            {"registrationConfiguration", new Dictionary<string, object> {
              {"enabled", true}
            }}
          }
        }
      });
      test.assertSuccess(response);

[rdvanbuuren]

@lukevp I totally agree with you that we need strongly typed objects for this! Would it be possible to re-use the Request objects for the Patch methods as well, just like the Update methods?

For example, currently the Patch code looks like this:

public ClientResponse<ApplicationResponse> PatchApplication(Guid? applicationId, Dictionary<string, object> request) { }

And should become something like this, just like UpdateApplication:

public ClientResponse<ApplicationResponse> PatchApplication(Guid? applicationId, ApplicationRequest request) { }

Json serialization could then just filter out the null values:

public override IRESTClient withJSONBody(object body, bool removeEmpty = false)
{
  var settings = removeEmpty ? new JsonSerializerSettings { NullValueHandling = NullValueHandling.Ignore } : null;
  content = new StringContent(JsonConvert.SerializeObject(body, Formatting.None, settings), Encoding.UTF8,
  "application/json");
  return this;
}

This would be a breaking change in the code, but becomes so much better.

[robotdan]

There are some complications with using our typed objects for PATCH. We hope to support JSON Patch (RFC 6902) in the future to help in this area. See https://github.com/FusionAuth/fusionauth-issues/issues/441.

If all fields were null-able then we could use a different serialization strategy for the PATCH method to strip all nulls and condense the serialized JSON. Using this approach would limit the ability to remove a value using PATCH which is currently done via a null value.

I am not .NET smart at all, so feel free to suggest a better approach that what we're doing. If we could come up with a way to only serialize what we want, but also allow nulls for when the caller wants to remove a property that would be great.

[lukevp]

I want to make sure I understand - using untyped objects lets you define a sparse object so when serialized the JSON does not contain fields that do not have changes. If you serialize a .NET object, there's no way to tell if the intent of the user is to update a property to null or if it is just null because it wasn't set on that request. Is that right?

[rdvanbuuren]

@lukevp I understand. I'll ask around here at the company if someone would know a solution to overcome this problem.