Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide space for additional context/metadata #27

Open
mitar opened this issue Oct 12, 2019 · 1 comment
Open

Provide space for additional context/metadata #27

mitar opened this issue Oct 12, 2019 · 1 comment

Comments

@mitar
Copy link

mitar commented Oct 12, 2019

Reading through issues it looks to me that multiple proposals could be addressed by having additional standardized space for (optional) context/metadata: #14, #11, #5, #18

It looks like many paint points are about what to do when data for which JSON patch should be applied has changed. And proposals are trying to propose more complicated operations to make operations more robust.

I think this is not the best approach because it is just an arm race. More complicated operations allow sometimes some operations to correctly apply when data changed, but for some other data changes operations might still fail. Moreover, operations are trickier and trickier to write, because you have to start thinking about how to make them be robust, which is not necessary always easy to do in an automatic manner.

So I would propose that JSON patch is kept simply and by standard is always meant to be a simple diff against a known version of JSON data.

The question of how to apply such JSON patch when underlying JSON data changes should not be specified by this standard and is left to various algorithms to solve. That can be done through various merge conflict resolution approaches, operation transform-based approaches, maybe even CRDTs, etc. Because different applications maybe have different requirements here, I think this standard should just propose a way to provide additional context/metadata so that those algorithms can do their job (better).

A good side of this approach is also that it does not have an arm race. You are not trying to solve ad-hoc some types of data modifications, but you look at all possible data modifications which can happen to state and how could JSON patch be still applied.

Currently, the problem is that JSON is an array, Maybe we should make it an object with one property for steps of the patch, so:

{
  "<more fields>": <value>,
  "steps": [
    ...
  ]
}

This would make it much easier to extend. And add some global metadata to the whole set of steps. So we would just standardize this structure, but not fields.

Additional metadata could go into each step as well, by simply allowing additional fields, but not specifying which those fields are.

Now, allowing this additional fields could for example address the #18 in much better way. Instead of doing operations on array by value, you would still do them by index, but also, for example, add additional metadata: value at index, value before index, value after index. When applying the patch you could check if those values still match, before applying it. If not, you could have some logic to try to fuzzy apply it, using not just value at index, but also information what values should be fore or after.

Another approach for #18, mentioned there as a problem if underlying query for which you are making a patch changes, is that you could store into global metadata a cursor ID for which this patch is meant to apply. Then the server-side could know which existing open cursor to use to apply the patch, instead of redoing a query and potentially getting a different order of elements to patch.

Other issues try to make smarter operations, but this should be the task for the merging algorithm, to translate an old patch at old version to a new patch at new version. You do not need more complicated operations, but you just translate the patch between versions. Like operational transform does.

@mitar
Copy link
Author

mitar commented Oct 12, 2019

(In this view the test operator is also unnecessary in this standard. But I would leave it for backwards compatibility.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant