Introduce AllowMissingPathOnRemove about json-patch HOT 9 CLOSED

I'd be open to ApplyWithOptions, I think that's ok.

from json-patch.

@evanphx - I will be submitting a PR for this issue soon.

from json-patch.

I'd rather not continue to add these toplevel options. I think the right solution is, instead, to accumulate errors found trying to apply the patch and then return them all at once. That would allow you the ability to inspect the errors and disregard the error if they were all for removing a non-existentant path.

from json-patch.

Just to make sure I understand, are you suggesting to change the signature of func (p Patch) Apply(doc []byte) ([]byte, error) to func (p Patch) Apply(doc []byte) ([]byte, []error)?

Won’t this break existing clients?

Or are you thinking a new method?

Also, in order for this to work, it sounds like the method will be returning both a patched document and a slice of errors, so in case of errors will it just ignore the errored operations and just keep going, effectively returning a partially patched document? How will the user inspect the errors? By error type? Will this require the introduction of a new error type/types to capture the type of the offending operation?

In addition, how will hard errors such as the ones returned by json.Unmarshal, json.Marshal and json.MarshalIndent fit in this new approach?

from json-patch.

No, Apply would still return ([]byte, error) but within Apply, we'd accumulate the errors and then return, say, a PatchError type that contains all the accumulated errors.

I think that the non-patch related errors like the ones returned from json.Marshal would be "hard" errors, returning immediately.

from json-patch.

My only concern is that changing the type of the error returned by Apply might still break existing clients who are already inspecting the type/contents of the error.

I’m also wondering if there is any value in partially patched documents. What I mean is, “remove-missing-path” is the only missing-path related error that I can see that might be semantically benign and could be ignored for certain use cases.

If the goal is to avoid package-level global flags, can I suggest introducing a new method ApplyWithOptions instead? It will have a signature similar to Apply, except it will also accept a new parameter struct type (say ApplyOptions) with the same options you already have as global vars plus the new AllowMissingPathOnRemove. Apply can be modified to simply call ApplyWithOptions by passing the current global options through the ApplyOptions parameter.

This way you preserve backward compatibility of the current Apply (including error semantics), avoid introducing a new global variable and have an alternative method which takes call-level options.

from json-patch.

We'd be changing the concrete type only, the exposed type would still be error. No one is dependent on the concrete type returned now I doubt. And even if they are, I'm ok breaking that contract.

from json-patch.

I’ll put together a PR.

from json-patch.

Closing in favor of #117.

from json-patch.