Better support for JSON Patch via RFC 6902 or RFC 7396

robotdan commented 4 years ago

Full support for JSON Patch

Description

FusionAuth supports the HTTP PATCH method but this has some limitations. Merging, replacing or moving values is difficult.

Examples

Array Merges

Initial State

{
  "data': {
     "foo": [ "bar "] 
  }
}

PATCH Request

{
  "data': {
     "foo": [ "bar ", "baz" ] 
  }
}

Result

{
  "data': {
     "foo": [ "bar ", "bar", "baz" ] 
  }
}

This result may be what you want, or it may not be, the incoming request does not tell us enough to identify if the values should be replaced or appended. Because the data structure represented here is assumed to be an Array, duplicates are allowed and the incoming values are appended. This means that you cannot remove values without multiple API calls. You could use a GET and PUT or make two PATCH calls to clear using null and then set again.

Solution

The JSON Patch RFC attempts to solve this problem by providing syntax for the intended operations. Ideally we'd find a library that implements RFC 6902 with Jackson and can be added transparently to our existing serialization / de-serialization strategy.

Additional Context

http://jsonpatch.com/ https://tools.ietf.org/html/rfc6902

Java libs that implement RFC 6902 https://github.com/ebay/bsonpatch https://github.com/java-json-tools/json-patch

Per comment https://github.com/FusionAuth/fusionauth-issues/issues/441#issuecomment-639092237, RFC 7396 maybe adequate. https://tools.ietf.org/html/rfc7396

Jackson plugin supporting RFC 7396 https://github.com/jeffnelson/jackson-merge-patch

matthew-altman commented 4 years ago

May be able to implement merge-patch in Prime https://tools.ietf.org/html/rfc7396 Or can just use existing Jackson behavior to make the deep merge and array replacement work as expected.

rdvanbuuren commented 4 years ago

Issue moved to https://github.com/FusionAuth/fusionauth-netcore-client/issues/9

robotdan commented 4 years ago

This issue is to support the JSON PATCH specification through our HTTP PATCH implementation, if you have particular issues with the PATCH method as it is, you can open issues in the corresponding client or issues project.

The JSON does need to look just like the normal PUT or POST requests, so your properties need to be in the "application" object.

Will this work for you?

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

trevorr commented 4 years ago

FWIW, I think something as simple as RFC 7396 Merge Patch is adequate most of the time and causes the least surprise (e.g. overwriting as opposed to appending to arrays). RFC 6902 JSON Patch is nice to have, but I've rarely seen it used in practice.

robotdan commented 4 years ago

Thanks for that feedback @trevorr that is helpful. It looks like there is a Jackson plugin in support of RFC 7396, this may be helpful. https://github.com/jeffnelson/jackson-merge-patch

robotdan commented 3 years ago

I stubbed out a test to prove the algorithm in RFC 7396 using the pseudocode provided. This seems to pass for the test cases they provide.

This is just a test, but if it works, we should be able to use this in the JacksonContentHandler to handle a Content-Type of application/merge-patch+json. https://github.com/prime-framework/prime-mvc/commit/94f562331bf92662431f06e7d78784f238f98b19

@voidmain this may be fairly easy to just support RFC 7396 which would at least be an improvement from the base Jackson capability when using readerForUpdating.

darwinshameran commented 2 years ago

Would love to see support for JSON PATCH!

mooreds commented 2 years ago

@darwinshameran please vote the issue up (with a 'thumbs up'), as that is how we determine community support.

More here: https://fusionauth.io/docs/v1/tech/core-concepts/roadmap/

robotdan commented 2 years ago

Planning to support both RFC 6902 or RFC 7396 and still preserve backwards compatibility.

Setting Content-Type will yield:

application/json - current behavior which works but is kinda busted for arrays.
application/json-patch+json - use JSON Patch syntax per RFC 6902
application/merge-patch+json - use JSON Patch syntax per RFC 7396

robotdan commented 2 years ago

Internal doc:

Update PATCH doc for APIs to indicate the warning regarding arrays should be limited to using application/json and that it is preferred for the caller to utilize application/json-patch+json or application/merge-patch+json.
Review all open issues re: PATCH and suggest they be re-attempted using one of these new values for Content-Type.

FusionAuth / fusionauth-issues