Implement jsonpatch.validate with tests. #29.

[joshua-mcginnis]

#29

[sonnyp]

Array.isArray() isn't available on IE8, check json-patch.ts method _isArray()

[sonnyp]

@joshua-mcginnis cool, thanks!

I took the liberty to comment the diff, additionally:

.js are actually built from the .ts files: http://www.typescriptlang.org/
regular JavaScript will work just fine

You can build the .js with

npm install -g typescript
tsc --sourcemap json-patch-duplex.ts
tsc --sourcemap json-patch.ts

2. I think it makes more sense to add jsonpatch.isValid to json-patch.ts rather than to json-patch-duplex.ts

[joshua-mcginnis]

@sonnyp will typescript not let me return mixed types?

return error === '' ? error : true;

Gives me:
error TS2226: Type of conditional '{}' must be identical to 'string' or 'boolean'.

If that's the case, then how should we return the validation errors if isValid always returns bool. I'd like not to not throw as that will make implementation uglier (try/catch).

[sonnyp]

@joshua-mcginnis sorry no idea, I don't know typescript specifics but I'd say it should work

[Starcounter-Jack]

Me or @warpech could
have a look when we're back from vacation

Sent from my iPhone

On 21 Jun 2014, at 00:17, Sonny Piers notifications@github.com wrote:

@joshua-mcginnis sorry no idea, I don't know typescript

—
Reply to this email directly or view it on GitHub.

[skotchio]

Hey guys how about merging this pull request?

[sonnyp]

Guys can you take a look at https://github.com/sonnyp/JSON-Patch/compare/validate and tell me if it's okay for you?

Please note that it doesn't test for path validity (in JSON Pointer terms).

A (better?) alternative for patch validation might be to use JSON Schema.

Usage:

var patch = MYPATCH;
var lang = {
  'PATCH_TYPE': 'Patch document must be an array.',
  'OPERATION_TYPE': 'Operation must be an object.',
  'OP_VALUE': '"op" property must add, remove, replace, move, copy or test',
  'PATH_TYPE': '"path" property must be a string',
  'FROM_TYPE': '"from" string property is required for move and copy operations',
  'VALUE_REQUIRED': '"value" property is required for add, replace and test operations '
};

//if undefined or falsy, validate will break after first error
var mutltiple = true;

var errors = jsonpatch.validate(patch, multiple);
//error
if (errors.length) {
  errors.forEach(function(error) {
    //error.error is the message
    //error.index is the index of the item that errored (undefined for 'PATCH_TYPE')
    if (typeof error.index === 'number')
      console.log(patch[error.index] + ' ' + local[error.error]);
    else
      console.log(local[error.error]);
  });
}
//no error
else {
  console.log('patch valid');
}

[skotchio]

@sonnyp why don't you want add validation of individual operations as JSON-Patch specification requires?

For example for add operation: http://tools.ietf.org/html/rfc6902#page-5

Because this operation is designed to add to existing objects and
arrays, its target location will often not exist.  Although the
pointer's error handling algorithm will thus be invoked, this
specification defines the error handling behavior for "add" pointers
to ignore that error and add the value as specified.

However, the object itself or an array containing it does need to
exist, and it remains an error for that not to be the case.  For
example, an "add" with a target location of "/a/b" starting with this
document:

{ "a": { "foo": 1 } }

is not an error, because "a" exists, and "b" will be added to its
value.  It is an error in this document:

{ "q": { "bar": 2 } }

because "a" does not exist.

I think we should have robust module for patch applying and be sure consistency

[sonnyp]

@vovan22 because in that case you don't know if the path is valid until you get the document to patch.
So basically you need to try and apply the patch before knowing there's something wrong with it.

[skotchio]

@sonnyp ok. But why do you think that this is enough? Look at implementation of another jsonpatch library code https://github.com/bruth/jsonpatch-js/blob/master/jsonpatch.js#L242 as you can see there is AddPatch.apply method that check index bounds which described by JSON-Patch documentation (look at my previous post).

Do you think fast-json-patch module don't nesessary to implement?

[skotchio]

Hey guys does anybody could merge this?

[warpech]

This will be merged in 0.5.1

[warpech]

I used the changes from @sonnyp as basis for what I find a more flexible solution.

Please see #29 for description of changes

[sonnyp]

@skotchio I'm not against an other type of validation.
The specification doesn't require to validate the patch before applying it.
I don't know the specifics of JSON Patch error handling, please feel free to PR or raise an issue. I believe the use case here is to validate the syntax of the patch, isn't?