How to check for failed apply() operations?

[MarkHerhold]

It appears that the docs are out of date for the apply() operation as everything appears to return an array now.

What is the appropriate way to check for a failed apply() operation in v1.0.0? In previous versions, false would be returned.

[tomalec]

Briefly, now we are returning result for each individual operation object. So, you can also get the results of other types. for example you get the elements removed.

@joozek78 Could you update the docs?

[joozek78]

You can see the updated docs here. Basically, you now get an array of results and probably the last item (as previously only last operation result was returned) holds your result. So:

var patches = [{op: "replace", path: "version", value: 1}, {op: "test", path: "version", value: "0"}]
// the old way:
// var success = jsonpatch.apply(obj, patches);
// the new way:
var success = jsonpatch.apply(obj, patches)[patches.length - 1]

[MarkHerhold]

So what indicates a failure in any of the individual patch operations?

[joozek78]

With the validate flag an exception will be thrown, as before. If the test fails, the result array will contain false at index respective for the position of test operation in patch. So, if your test operation is third operation in patch, then third element of the results array will contain false.

[MarkHerhold]

@joozek78 Thanks for the response.

So if I understand correctly, to get an overall patch success / failure (true/false) result, we need to:

• catch any errors thrown from apply()
• ensure that all test operations return true

Unfortunately we cannot take a shortcut here and check that all results are not false because remove/replace operations could result in a false value in the array.

Here's the code I came up with to wrap the apply operation:

const jsonpatch = require("fast-json-patch");
const zip = require("lodash.zip");

let obj = { version: 0, isGreen: false };
// the test will fail
let patches = [{op: "replace", path: "/version", value: 1}, {op: "test", path: "/version", value: "0"}, {op: "remove", path: "/isGreen"}]

// applies a patch to an object
// returns true if successful, false otherwise
function patch(obj, patches) {
    let results;
    try {
        results = jsonpatch.apply(obj, patches, true);
    } catch(err) {
        return false;
    }

    return zip(patches.map((d) => d.op), results) // combine the patch operations and the results
        .filter((d) => d[0] === 'test') // filter out everything but the test operations
        .map((d) => d[1]) // get the test results
        .reduce((p, c) => c && p, true); // reduce all true/false values to a single value, false taking precidence
}

var successful = patch(obj, patches);

console.log('patch result was ' + (successful ? 'successful' : 'unsuccessful'));

Does this sound logical? Unfortunately v1.0.0 adds more work for this specific use case.

[tomalec]

@MarkHerhold You're right that such simple check become quite complicated. Probably you can optimize your code a little bit, assuming that operation is successful only if it applied all the operation objects => check the length and last test operation.

On the other hand we could think about adding custom property (like .successful) to the array returned by apply. It may not be 100% clean and kosher, but for sure it will be simple.

WDYT?

[joozek78]

I think it's okay - it's making the most popular task the simplest

On Mon, 5 Sep 2016 at 12:53 Tomek Wytrębowicz [email protected] wrote:

@MarkHerhold https://github.com/MarkHerhold You're right that such simple check become quite complicated. Probably you can optimize your code a little bit, assuming that operation is successful only if it applied all the operation objects => check the length and last test operation.

On the other hand we could think about adding custom property (like .successful) to the array returned by apply. It may not be 100% clean and kosher, but for sure it will be simple.

WDYT?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Starcounter-Jack/JSON-Patch/issues/116#issuecomment-244718875, or mute the thread https://github.com/notifications/unsubscribe-auth/AFvBISXcw8zlaCS3eBQcF2MTGMZXuSTfks5qm_TEgaJpZM4JTaA- .

[MarkHerhold]

I would be in favor of adding a .successful boolean result to the array for this use case. Presently I have the code above as a wrapper in my own project and would rather not see another module published to handle this very specific check. So if we are :+1: on the approach I can look at PRing a change.

Yay/nay?

[tomalec]

Unless @warpech has any comments. We are :+1: to go. PR is naturally welcome :)

[alshakero]

I guess this issue is rendered irrelevant with the new API. WDYT @tomalec?

[tomalec]

I think.I think it still is relevant.

Especially, as @MarkHerhold stated it's hard to get single true/false answer for entire patch.

I think the flag mentioned above could be a solution.

[alshakero]

Doesn't applyOperation throw an exception when it fails now?

[tomalec]

So the solution to the problem wold be to just

try {
    results = jsonpatch.applyPatch(obj, patches, true);
    success = true;
} catch(err) {
    success = false;
}

?