From 1498c09661f7855048f6ea91fba31a131213236c Mon Sep 17 00:00:00 2001 From: Marcin Warpechowski Date: Tue, 2 Jul 2019 11:17:34 +0200 Subject: [PATCH] change the API for generating test operations was: extra boolean argument on "observe" and extra object argument on "generate" and "compare" is: extra boolean argument "generate" and "compare" also this commit adds way more tests for test operation generation --- README.md | 24 +-- src/duplex.ts | 25 ++-- test/spec/duplexBenchmark.js | 20 +-- test/spec/duplexSpec.js | 275 +++++++++++++++++++++++++++++++---- 4 files changed, 279 insertions(+), 65 deletions(-) diff --git a/README.md b/README.md index 3701489a..e1e9f262 100644 --- a/README.md +++ b/README.md @@ -154,15 +154,15 @@ var patch = jsonpatch.generate(observer); // ]; ``` -Generating patches with test operations: +Generating patches with test operations for values in the first object: ```js var document = { firstName: "Joachim", lastName: "Wester", contactDetails: { phoneNumbers: [ { number:"555-123" }] } }; -var observer = jsonpatch.observe(document, undefined, true); +var observer = jsonpatch.observe(document); document.firstName = "Albert"; document.contactDetails.phoneNumbers[0].number = "123"; document.contactDetails.phoneNumbers.push({ number:"456" }); -var patch = jsonpatch.generate(observer); +var patch = jsonpatch.generate(observer, true); // patch == [ // { op: "test", path: "/firstName", value: "Joachim"}, // { op: "replace", path: "/firstName", value: "Albert"}, @@ -181,12 +181,12 @@ var diff = jsonpatch.compare(documentA, documentB); //diff == [{op: "replace", path: "/user/lastName", value: "Collins"}] ``` -Comparing two object trees with test operations: +Comparing two object trees with test operations for values in the first object: ```js var documentA = {user: {firstName: "Albert", lastName: "Einstein"}}; var documentB = {user: {firstName: "Albert", lastName: "Collins"}}; -var diff = jsonpatch.compare(documentA, documentB, {inversible: true}); +var diff = jsonpatch.compare(documentA, documentB, true); //diff == [ // {op: "test", path: "/user/lastName", value: "Einstein"}, // {op: "replace", path: "/user/lastName", value: "Collins"} @@ -292,17 +292,17 @@ Retrieves a value from a JSON document by a JSON pointer. Returns the value. -#### `jsonpatch.observe(document: any, callback?: Function, inversible: boolean = false): Observer` +#### `jsonpatch.observe(document: any, callback?: Function): Observer` Sets up an deep observer on `document` that listens for changes in object tree. When changes are detected, the optional -callback is called with the generated patches array as the parameter. If inversible is true, then observer will generate test operations. +callback is called with the generated patches array as the parameter. Returns `observer`. -#### `jsonpatch.generate(document: any, observer: Observer, opts?: { inversible: boolean }): Operation[]` +#### `jsonpatch.generate(document: any, observer: Observer, invertible = false): Operation[]` If there are pending changes in `obj`, returns them synchronously. If a `callback` was defined in `observe` -method, it will be triggered synchronously as well. If opts.inversible is undefined then fallback to observer.inversible. +method, it will be triggered synchronously as well. If `invertible` is true, then each change will be preceded by a test operation of the value before the change. If there are no pending changes in `obj`, returns an empty array (length 0). @@ -312,9 +312,9 @@ Destroys the observer set up on `document`. Any remaining changes are delivered synchronously (as in `jsonpatch.generate`). Note: this is different that ES6/7 `Object.unobserve`, which delivers remaining changes asynchronously. -#### `jsonpatch.compare(document1: any, document2: any, opts?: { inversible: boolean }): Operation[]` +#### `jsonpatch.compare(document1: any, document2: any, invertible = false): Operation[]` -Compares object trees `document1` and `document2` and returns the difference relative to `document1` as a patches array. If opts.inversible is true, test operations will be generated. +Compares object trees `document1` and `document2` and returns the difference relative to `document1` as a patches array. If `invertible` is true, then each change will be preceded by a test operation of the value in `document1`. If there are no differences, returns an empty array (length 0). @@ -376,7 +376,7 @@ Functions `applyPatch`, `applyOperation`, and `validate` accept a `validate`/ `v If you pass a validator, it will be called with four parameters for each operation, `function(operation, index, tree, existingPath)` and it is expected to throw `JsonPatchError` when your conditions are not met. - `operation` The operation it self. -- `index` `operation`'s index in the patch array (if application). +- `index` `operation`'s index in the patch array (if application). - `tree` The object that is supposed to be patched. - `existingPath` the path `operation` points to. diff --git a/src/duplex.ts b/src/duplex.ts index 16a52267..8a3e28ef 100644 --- a/src/duplex.ts +++ b/src/duplex.ts @@ -17,7 +17,6 @@ export interface Observer { patches: Operation[]; unobserve: () => void; callback: (patches: Operation[]) => void; - inversible: boolean; } var beforeDict = new WeakMap(); @@ -64,7 +63,7 @@ export function unobserve(root: T, observer: Observer) { /** * Observes changes made to an object, which can then be retrieved using generate */ -export function observe(obj: Object|Array, callback?: (patches: Operation[]) => void, inversible: boolean = false): Observer { +export function observe(obj: Object|Array, callback?: (patches: Operation[]) => void): Observer { var patches = []; var observer; var mirror = getMirror(obj); @@ -81,7 +80,7 @@ export function observe(obj: Object|Array, callback?: (patches: Operation[ return observer; } - observer = { inversible }; + observer = {}; mirror.value = _deepClone(obj); @@ -145,11 +144,10 @@ export function observe(obj: Object|Array, callback?: (patches: Operation[ /** * Generate an array of patches from an observer */ -export function generate(observer: Observer, opts: { inversible?: boolean } = {}): Operation[] { +export function generate(observer: Observer, invertible = false): Operation[] { var mirror = beforeDict.get(observer.object); - var inversible = typeof opts.inversible !== "undefined" ? opts.inversible : observer.inversible; - _generate(mirror.value, observer.object, observer.patches, "", { inversible }); + _generate(mirror.value, observer.object, observer.patches, "", invertible); if (observer.patches.length) { applyPatch(mirror.value, observer.patches); } @@ -164,7 +162,7 @@ export function generate(observer: Observer, opts: { inversible?: boo } // Dirty check if obj is different from mirror, generate patches and update mirror -function _generate(mirror, obj, patches, path, opts = { inversible: false }) { +function _generate(mirror, obj, patches, path, invertible) { if (obj === mirror) { return; } @@ -177,7 +175,6 @@ function _generate(mirror, obj, patches, path, opts = { inversible: false }) { var oldKeys = _objectKeys(mirror); var changed = false; var deleted = false; - var { inversible } = opts; //if ever "move" operation is implemented here, make sure this test runs OK: "should not generate the same patch twice (move)" @@ -189,22 +186,22 @@ function _generate(mirror, obj, patches, path, opts = { inversible: false }) { var newVal = obj[key]; if (typeof oldVal == "object" && oldVal != null && typeof newVal == "object" && newVal != null) { - _generate(oldVal, newVal, patches, path + "/" + escapePathComponent(key), opts); + _generate(oldVal, newVal, patches, path + "/" + escapePathComponent(key), invertible); } else { if (oldVal !== newVal) { changed = true; - if (inversible) patches.push({ op: "test", path: path + "/" + escapePathComponent(key), value: _deepClone(oldVal) }); + if (invertible) patches.push({ op: "test", path: path + "/" + escapePathComponent(key), value: _deepClone(oldVal) }); patches.push({ op: "replace", path: path + "/" + escapePathComponent(key), value: _deepClone(newVal) }); } } } else if(Array.isArray(mirror) === Array.isArray(obj)) { - if (inversible) patches.push({ op: "test", path: path + "/" + escapePathComponent(key), value: _deepClone(oldVal) }) + if (invertible) patches.push({ op: "test", path: path + "/" + escapePathComponent(key), value: _deepClone(oldVal) }) patches.push({ op: "remove", path: path + "/" + escapePathComponent(key) }); deleted = true; // property has been deleted } else { - if (inversible) patches.push({ op: "test", path, value: mirror }); + if (invertible) patches.push({ op: "test", path, value: mirror }); patches.push({ op: "replace", path, value: obj }); changed = true; } @@ -224,8 +221,8 @@ function _generate(mirror, obj, patches, path, opts = { inversible: false }) { /** * Create an array of patches from the differences in two objects */ -export function compare(tree1: Object | Array, tree2: Object | Array, opts?: { inversible: boolean }): Operation[] { +export function compare(tree1: Object | Array, tree2: Object | Array, invertible = false): Operation[] { var patches = []; - _generate(tree1, tree2, patches, '', opts); + _generate(tree1, tree2, patches, '', invertible); return patches; } diff --git a/test/spec/duplexBenchmark.js b/test/spec/duplexBenchmark.js index 4a1edb1f..7e583793 100644 --- a/test/spec/duplexBenchmark.js +++ b/test/spec/duplexBenchmark.js @@ -146,7 +146,7 @@ suite.add('compare operation same but deep objects', { }); // Benchmark generating test operations -suite.add('generate operation, with inversible set to true', { +suite.add('generate operation, invertible = true', { setup: function() { var obj = { firstName: 'Albert', @@ -160,7 +160,7 @@ suite.add('generate operation, with inversible set to true', { } ] }; - var observer = jsonpatch.observe(obj, undefined, true); + var observer = jsonpatch.observe(obj); }, fn: function() { obj.firstName = 'Joachim'; @@ -168,10 +168,10 @@ suite.add('generate operation, with inversible set to true', { obj.phoneNumbers[0].number = '123'; obj.phoneNumbers[1].number = '456'; - var patches = jsonpatch.generate(observer); + var patches = jsonpatch.generate(observer, true); } }); -suite.add('generate operation and re-apply, with inversible set to true', { +suite.add('generate operation and re-apply, invertible = true', { setup: function() { var obj = { firstName: 'Albert', @@ -185,7 +185,7 @@ suite.add('generate operation and re-apply, with inversible set to true', { } ] }; - var observer = jsonpatch.observe(obj, undefined, true); + var observer = jsonpatch.observe(obj); }, fn: function() { obj.firstName = 'Joachim'; @@ -193,7 +193,7 @@ suite.add('generate operation and re-apply, with inversible set to true', { obj.phoneNumbers[0].number = '123'; obj.phoneNumbers[1].number = '456'; - var patches = jsonpatch.generate(observer); + var patches = jsonpatch.generate(observer, true); obj2 = { firstName: 'Albert', lastName: 'Einstein', @@ -210,7 +210,7 @@ suite.add('generate operation and re-apply, with inversible set to true', { jsonpatch.applyPatch(obj2, patches); } }); -suite.add('compare operation, with inversible set to true', { +suite.add('compare operation, invertible = true', { setup: function() { var obj = { firstName: 'Albert', @@ -238,11 +238,11 @@ suite.add('compare operation, with inversible set to true', { }; }, fn: function() { - var patches = jsonpatch.compare(obj, obj2, { inversible: true }); + var patches = jsonpatch.compare(obj, obj2, true); } }); -suite.add('compare operation same but deep objects, with inversible set to true', { +suite.add('compare operation same but deep objects, invertible = true', { setup: function() { var depth = 10; @@ -271,7 +271,7 @@ suite.add('compare operation same but deep objects, with inversible set to true' var obj2 = obj; }, fn: function() { - var patches = jsonpatch.compare(obj, obj2, { inversible: true }); + var patches = jsonpatch.compare(obj, obj2, true); } }); diff --git a/test/spec/duplexSpec.js b/test/spec/duplexSpec.js index ff6f1351..496b8214 100644 --- a/test/spec/duplexSpec.js +++ b/test/spec/duplexSpec.js @@ -199,8 +199,43 @@ describe('duplex', function() { person1.firstName = 'Alexander'; person2.firstName = 'Lucas'; - var patch1 = jsonpatch.generate(observer1, { inversible: true }); - var patch2 = jsonpatch.generate(observer2, { inversible: true }); + var patch1 = jsonpatch.generate(observer1); + var patch2 = jsonpatch.generate(observer2); + + expect(patch1).toReallyEqual([ + { + op: 'replace', + path: '/firstName', + value: 'Alexander' + } + ]); + expect(patch2).toReallyEqual([ + { + op: 'replace', + path: '/firstName', + value: 'Lucas' + } + ]); + }); + + it('should generate test and replace (2 observers, invertible = true)', function() { + var person1 = { + firstName: 'Alexandra', + lastName: 'Galbreath' + }; + var person2 = { + firstName: 'Lisa', + lastName: 'Mendoza' + }; + + var observer1 = jsonpatch.observe(person1); + var observer2 = jsonpatch.observe(person2); + + person1.firstName = 'Alexander'; + person2.firstName = 'Lucas'; + + var patch1 = jsonpatch.generate(observer1, true); + var patch2 = jsonpatch.generate(observer2, true); expect(patch1).toReallyEqual([ { @@ -242,10 +277,60 @@ describe('duplex', function() { ] }; - var observer = jsonpatch.observe(obj, undefined, true); + var observer = jsonpatch.observe(obj); obj.firstName = 'Marcin'; var patches = jsonpatch.generate(observer); + expect(patches).toReallyEqual([ + { + op: 'replace', + path: '/firstName', + value: 'Marcin' + } + ]); + + obj.lastName = 'Warp'; + patches = jsonpatch.generate(observer); //first patch should NOT be reported again here + expect(patches).toReallyEqual([ + { + op: 'replace', + path: '/lastName', + value: 'Warp' + } + ]); + + expect(obj).toReallyEqual({ + firstName: 'Marcin', + lastName: 'Warp', + phoneNumbers: [ + { + number: '12345' + }, + { + number: '45353' + } + ] + }); //objects should be still the same + }); + + it('should generate test and replace (double change, shallow object, invertible = true)', function() { + obj = { + firstName: 'Albert', + lastName: 'Einstein', + phoneNumbers: [ + { + number: '12345' + }, + { + number: '45353' + } + ] + }; + + var observer = jsonpatch.observe(obj); + obj.firstName = 'Marcin'; + + var patches = jsonpatch.generate(observer, true); expect(patches).toReallyEqual([ { op: "test", @@ -260,7 +345,7 @@ describe('duplex', function() { ]); obj.lastName = 'Warp'; - patches = jsonpatch.generate(observer); //first patch should NOT be reported again here + patches = jsonpatch.generate(observer, true); //first patch should NOT be reported again here expect(patches).toReallyEqual([ { op: "test", @@ -302,10 +387,60 @@ describe('duplex', function() { ] }; - var observer = jsonpatch.observe(obj, undefined, true); + var observer = jsonpatch.observe(obj); obj.phoneNumbers[0].number = '123'; var patches = jsonpatch.generate(observer); + expect(patches).toReallyEqual([ + { + op: 'replace', + path: '/phoneNumbers/0/number', + value: '123' + } + ]); + + obj.phoneNumbers[1].number = '456'; + patches = jsonpatch.generate(observer); //first patch should NOT be reported again here + expect(patches).toReallyEqual([ + { + op: 'replace', + path: '/phoneNumbers/1/number', + value: '456' + } + ]); + + expect(obj).toReallyEqual({ + firstName: 'Albert', + lastName: 'Einstein', + phoneNumbers: [ + { + number: '123' + }, + { + number: '456' + } + ] + }); //objects should be still the same + }); + + it('should generate replace (double change, deep object, invertible = true)', function() { + obj = { + firstName: 'Albert', + lastName: 'Einstein', + phoneNumbers: [ + { + number: '12345' + }, + { + number: '45353' + } + ] + }; + + var observer = jsonpatch.observe(obj); + obj.phoneNumbers[0].number = '123'; + + var patches = jsonpatch.generate(observer, true); expect(patches).toReallyEqual([ { op: "test", @@ -320,7 +455,7 @@ describe('duplex', function() { ]); obj.phoneNumbers[1].number = '456'; - patches = jsonpatch.generate(observer); //first patch should NOT be reported again here + patches = jsonpatch.generate(observer, true); //first patch should NOT be reported again here expect(patches).toReallyEqual([ { op: "test", @@ -505,7 +640,7 @@ describe('duplex', function() { expect(obj2).toEqualInJson(obj); }); - it('should generate remove (array indexes should be sorted descending)', function() { + it('should generate remove (array indexes should be sorted descending, invertible = true)', function() { obj = { items: ['a', 'b', 'c'] }; @@ -514,18 +649,28 @@ describe('duplex', function() { obj.items.pop(); obj.items.pop(); - patches = jsonpatch.generate(observer); + patches = jsonpatch.generate(observer, true); //array indexes must be sorted descending, otherwise there is an index collision in apply expect(patches).toReallyEqual([ + { + op: 'test', + path: '/items/2', + value: 'c' + }, { op: 'remove', path: '/items/2' }, + { + op: 'test', + path: '/items/1', + value: 'b' + }, { op: 'remove', path: '/items/1' - } + }, ]); obj2 = { @@ -1460,45 +1605,91 @@ describe('duplex', function() { describe('compare', function() { it('Replacing a root array with an object should be handled well', function() { - const obj = {}; - var patches = jsonpatch.compare(['jack'], obj); + const objA = ['jack']; + const objB = {}; + var patches = jsonpatch.compare(objA, objB); expect(patches).toEqual([ { op: 'replace', path: '', - value: obj + value: objB + } + ]); + + }); + it('Replacing a root array with an object should be handled well (invertible = true)', function() { + + const objA = ['jack']; + const objB = {}; + var patches = jsonpatch.compare(objA, objB, true); + expect(patches).toEqual([ + { + op: 'test', + path: '', + value: objA + }, + { + op: 'replace', + path: '', + value: objB } ]); }); it('Replacing an array with an object should be handled well', function() { + const arr = ['jack']; + const obj = {}; + var patches = jsonpatch.compare({arr: arr}, {arr: obj}); + expect(patches).toEqual([ + { + op: 'replace', + path: '/arr', + value: obj + } + ]); + + }); + it('Replacing an array with an object should be handled well (invertible = true)', function() { + + const arr = ['jack']; const obj = {}; - var patches = jsonpatch.compare({arr: ['jack']}, {arr: obj}); + var patches = jsonpatch.compare({arr: arr}, {arr: obj}, true); expect(patches).toEqual([ + { + op: 'test', + path: '/arr', + value: arr + }, { op: 'replace', path: '/arr', value: obj } ]); - + }); it('Replacing an array that nested in an object with an object nested in an an object should be handled well', function() { + const arr = ['jack']; const obj = {}; - var patches = jsonpatch.compare({arr: {deeperArray: ['jack']}}, {arr: {deeperArray: obj}}); + var patches = jsonpatch.compare({arr: {deeperArray: arr}}, {arr: {deeperArray: obj}}, true); expect(patches).toEqual([ + { + op: 'test', + path: '/arr/deeperArray', + value: arr + }, { op: 'replace', path: '/arr/deeperArray', value: obj } ]); - + }); - it('should return an add for a property that does not exist in the first obj', function() { + it('should return an add for a property that does not exist in the first obj, without a test operation (invertible = true)', function() { var objA = { user: { firstName: 'Albert' @@ -1511,7 +1702,8 @@ describe('duplex', function() { } }; - expect(jsonpatch.compare(objA, objB)).toReallyEqual([ + expect(jsonpatch.compare(objA, objB, true)).toReallyEqual([ + // no test operation, because undefined values are not serialized to JSON { op: 'add', path: '/user/lastName', @@ -1520,7 +1712,7 @@ describe('duplex', function() { ]); }); - it('should return a remove for a property that does not exist in the second obj', function() { + it('should return test and remove for a property that does not exist in the second obj (invertible = true)', function() { var objA = { user: { firstName: 'Albert', @@ -1533,7 +1725,12 @@ describe('duplex', function() { } }; - expect(jsonpatch.compare(objA, objB)).toReallyEqual([ + expect(jsonpatch.compare(objA, objB, true)).toReallyEqual([ + { + op: 'test', + path: '/user/lastName', + value: 'Einstein' + }, { op: 'remove', path: '/user/lastName' @@ -1541,7 +1738,7 @@ describe('duplex', function() { ]); }); - it('should return a replace for a property that exists in both', function() { + it('should return test and replace for a property that exists in both (invertible = true)', function() { var objA = { user: { firstName: 'Albert', @@ -1555,7 +1752,12 @@ describe('duplex', function() { } }; - expect(jsonpatch.compare(objA, objB)).toReallyEqual([ + expect(jsonpatch.compare(objA, objB, true)).toReallyEqual([ + { + op: 'test', + path: '/user/lastName', + value: 'Einstein' + }, { op: 'replace', path: '/user/lastName', @@ -1564,7 +1766,7 @@ describe('duplex', function() { ]); }); - it('should replace null with object', function() { + it('should replace null with object (invertible = true)', function() { var objA = { user: null }; @@ -1572,7 +1774,12 @@ describe('duplex', function() { user: {} }; - expect(jsonpatch.compare(objA, objB)).toReallyEqual([ + expect(jsonpatch.compare(objA, objB, true)).toReallyEqual([ + { + op: 'test', + path: '/user', + value: null + }, { op: 'replace', path: '/user', @@ -1581,7 +1788,7 @@ describe('duplex', function() { ]); }); - it('should replace object with null', function() { + it('should replace object with null (invertible = true)', function() { var objA = { user: {} }; @@ -1589,7 +1796,12 @@ describe('duplex', function() { user: null }; - expect(jsonpatch.compare(objA, objB)).toReallyEqual([ + expect(jsonpatch.compare(objA, objB, true)).toReallyEqual([ + { + op: 'test', + path: '/user', + value: {} + }, { op: 'replace', path: '/user', @@ -1598,7 +1810,7 @@ describe('duplex', function() { ]); }); - it('should not remove undefined', function() { + it('should not remove undefined (invertible = true)', function() { var objA = { user: undefined }; @@ -1606,7 +1818,7 @@ describe('duplex', function() { user: undefined }; - expect(jsonpatch.compare(objA, objB)).toReallyEqual([]); + expect(jsonpatch.compare(objA, objB, true)).toReallyEqual([]); }); it('should replace 0 with empty string', function() { @@ -1617,7 +1829,12 @@ describe('duplex', function() { user: '' }; - expect(jsonpatch.compare(objA, objB)).toReallyEqual([ + expect(jsonpatch.compare(objA, objB, true)).toReallyEqual([ + { + op: 'test', + path: '/user', + value: 0 + }, { op: 'replace', path: '/user',