fusionauth-issues icon indicating copy to clipboard operation
fusionauth-issues copied to clipboard

Better support for JSON Patch via RFC 6902 or RFC 7396

Open robotdan opened this issue 5 years ago • 9 comments

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.

Related

  • https://github.com/FusionAuth/fusionauth-issues/issues/441 (this)
  • https://github.com/FusionAuth/fusionauth-issues/issues/534
  • https://github.com/FusionAuth/fusionauth-issues/issues/667
  • https://github.com/FusionAuth/fusionauth-issues/issues/706
  • https://github.com/FusionAuth/fusionauth-issues/issues/956
  • https://github.com/FusionAuth/fusionauth-issues/issues/982
  • https://fusionauth.io/community/forum/topic/939/lambda-reconcile-does-not-remove-role-from-registration/8

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

robotdan avatar Jan 16 '20 20:01 robotdan

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.

matthew-altman avatar Jan 24 '20 17:01 matthew-altman

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

rdvanbuuren avatar Feb 04 '20 14:02 rdvanbuuren

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);

robotdan avatar Feb 05 '20 15:02 robotdan

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.

trevorr avatar Jun 04 '20 20:06 trevorr

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 avatar Jun 15 '20 14:06 robotdan

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.

robotdan avatar Nov 05 '20 06:11 robotdan

Would love to see support for JSON PATCH!

darwinshameran avatar Dec 01 '21 18:12 darwinshameran

@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/

mooreds avatar Dec 01 '21 20:12 mooreds

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 avatar Aug 31 '22 19:08 robotdan

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.

robotdan avatar Sep 09 '22 06:09 robotdan