Add validation for pre-flight checks about json-patch HOT 6 CLOSED

Rather than have a new function, I'd rather that DecodePatch just did a validate pass always, rather than have folks use a separate validation.

from json-patch.

Rather than have a new function, I'd rather that DecodePatch just did a validate pass always, rather than have folks use a separate validation.

Hi @evanphx, thank you for the quick response.

I was looking into your suggestion and looks like it might have a side effect for users using "DecodePatch()" together with "ApplyWithOptions()". It seems "ApplyOptions" has a direct effect on the existing validations. For example, "ApplyOptions.AllowMissingPathOnRemove" dictates the validation flow of a "Remove" operation. This means validations under "DecodePatch()" must consider "AllowMissingPathOnRemove" as well.

Wouldn't it be better to implement "Validate()" and "ValidateWithOptions()" methods for the "Patch" object returned by "DecodePatch()"? This should minimize this side effect and enable "pre-flight" validation before applying the patch.

What do you think?

Hi again @evanphx.

I raised a PR for this: #180
It seems that the "ApplyOptions" do not actually effect the validation behavior as they require the document for context:

• SupportNegativeIndices: Looks like this option is used only when we know the numeric value in the path refers to an array element in the document (after resolving the element).
• AccumulatedCopySizeLimit: Can only be done while applying the path on the document.
• AllowMissingPathOnRemove and EnsurePathExistsOnAdd: Both refer to the existence of the specified path in the document.

Please let me know what you think or whether you have any comments.

Thank you.

from json-patch.

@evanphx , If you agree that this may be done, we can implement this.

from json-patch.

Makes perfect sense. We are working intently on this and will provide the fixe to DecodePatch ASAP.

from json-patch.

Rather than have a new function, I'd rather that DecodePatch just did a validate pass always, rather than have folks use a separate validation.

Hi @evanphx, thank you for the quick response.

I was looking into your suggestion and looks like it might have a side effect for users using "DecodePatch()" together with "ApplyWithOptions()". It seems "ApplyOptions" has a direct effect on the existing validations. For example, "ApplyOptions.AllowMissingPathOnRemove" dictates the validation flow of a "Remove" operation.
This means validations under "DecodePatch()" must consider "AllowMissingPathOnRemove" as well.

Wouldn't it be better to implement "Validate()" and "ValidateWithOptions()" methods for the "Patch" object returned by "DecodePatch()"? This should minimize this side effect and enable "pre-flight" validation before applying the patch.

What do you think?

from json-patch.

Hi @radwaretaltr,

I see your point, but I guess I'm wondering when would someone not want validation? Most folks would call Validate before hand, so why not do it by default. I understand that AllowMissingPathOnRemove will change the validations, but in a way that's exactly what that option is supposed to be doing.

from json-patch.