Comments (6)
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.
Related Issues (20)
- Performance Degradation Starting In v5.2.0 HOT 2
- Invalid spec implementation
- RFC Link in README.md returns a 404 error
- nil dereference on empty "replace" on root
- RFC6902 limitations when the document structure is not known in advance HOT 2
- Panic when adding to a non-existing array HOT 1
- jsonpatch.Equal : strange behavior on handling literal unicode string equality HOT 1
- replace operation does not apply: doc is missing path: missing value HOT 2
- github.com/pkg/errors is in maintenance mode HOT 7
- Is this project still maintained:) HOT 1
- Unable to access invalid index: 1: invalid index referenced HOT 2
- Can not run "add" on root object
- CreateMergePatch: Numbers may get truncated due to always being treated as float64 HOT 3
- move fails with "missing value" if from is "" HOT 4
- `>` character is not decoded properly when applying json patches HOT 3
- Regression in 5.8.0: cannot create `jsonpatch.Operation` object HOT 6
- add on `null` does not work anymore ? HOT 10
- For V4 and V5, the apply results of the values that do not exist in replace are inconsistent. HOT 1
- Dependency github.com/pkg/errors was archived HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from json-patch.