Merge pull request #8 from marcolink/feat/move-by-lcs

feat: use LCS to find the least required move operations

Author: marcolink

README.md

@@ -12,8 +12,7 @@ Create [RFC 6902](https://datatracker.ietf.org/doc/html/rfc6902/) compliant JSON
 - Can diff any two [JSON](https://www.ecma-international.org/publications-and-standards/standards/ecma-404/)  compliant objects - returns differences as [JSON Patch](http://jsonpatch.com/).
 - Elegant array diffing by providing an `objectHash` to match array elements
 - Ignore specific keys by providing a `propertyFilter`
-- `move` operations are ALWAYS **appended at the end**, therefore, they can be ignored (if wanted) when the patch gets applied.
-- :paw_prints: ***Is it small?*** Zero dependencies - it's ~**7 KB** (uncompressed).
+- :paw_prints: ***Is it small?*** Zero dependencies - it's ~**3 KB** (minified).
 - :crystal_ball: ***Is it fast?*** I haven't done any performance comparison yet.
 - :hatched_chick: ***Is it stable?*** Test coverage is high, but it's still in its early days - bugs are expected.
 - The interface is inspired by [jsondiffpatch](https://github.com/benjamine/jsondiffpatch)
@@ -41,7 +40,7 @@ console.log(patch) // => [{op: 'replace', path: '/year', value: 1974}]
 ## Configuration
 
 ```typescript
-import { generateJSONPatch, JsonPatchConfig, JsonValue } from 'generate-json-patch';
+import { generateJSONPatch, JsonPatchConfig, JsonValue, ObjectHashContext } from 'generate-json-patch';
 
 generateJSONPatch({/*...*/}, {/*...*/}, {
     // called when comparing array elements
@@ -53,7 +52,7 @@ generateJSONPatch({/*...*/}, {/*...*/}, {
     },
     // called for every property on objects. Can be used to ignore sensitive or irrelevant 
     // properties when comparing data.
-    propertyFilter: function (propertyName: string, context: GeneratePatchContext) {
+    propertyFilter: function (propertyName: string, context: ObjectHashContext) {
         return !['sensitiveProperty'].includes(propertyName);
     },
     array: {
@@ -66,12 +65,12 @@ generateJSONPatch({/*...*/}, {/*...*/}, {
 ``` 
 
 ### Patch Context
-Both config function (`objectHash`, `propertyFilter`), receive a patch context as second parameter.
+Both config function (`objectHash`, `propertyFilter`), receive a context as second parameter.
 This allows for granular decision-making on the provided data.
 
 #### Example
 ```typescript
-import {generateJSONPatch, JsonPatchConfig, JsonValue, pathInfo} from 'generate-json-patch';
+import {generateJSONPatch, JsonPatchConfig, JsonValue, ObjectHashContext, pathInfo} from 'generate-json-patch';
 
 const before = {
     manufacturer: "Ford",
@@ -98,7 +97,7 @@ const after = {
 }
 
 const patch = generateJSONPatch(before, after, {
-    objectHash: function (value: JsonValue, context: GeneratePatchContext) {
+    objectHash: function (value: JsonValue, context: ObjectHashContext) {
         const {length, last} = pathInfo(context.path)
         if (length === 2 && last === 'engine') {
             return value.name
@@ -109,9 +108,8 @@ const patch = generateJSONPatch(before, after, {
 
 console.log(patch) // => [
 // { op: 'replace', path: '/engine/3/hp', value: 138 },
-// { op: 'move', from: '/engine/2',  path: '/engine/3' },
-// { op: 'move', from: '/engine/1',  path: '/engine/2' },
-// { op: 'move', from: '/engine/0',  path: '/engine/1' }]
+// { op: 'move', from: '/engine/3', path: '/engine/0' }
+// ]
 ```
 
 > For more examples, check out the [tests](./src/index.spec.ts)

package.json

@@ -30,7 +30,7 @@
   ],
   "scripts": {
     "build": "tsup src/index.ts",
-    "test": "ts-mocha -p tsconfig.json src/index.spec.ts",
+    "test": "ts-mocha -p tsconfig.json src/*.spec.ts",
     "test-watch": "npm run test -- -w --watch-files '**/*.ts'",
     "test-coverage": "nyc npm run test",
     "lint": "eslint --ext .ts ./src",

src/index.spec.ts

@@ -1,5 +1,5 @@
-import type { JsonValue, Patch } from './index';
-import { generateJSONPatch, GeneratePatchContext, pathInfo } from './index';
+import type { JsonValue, ObjectHashContext, Patch } from './index';
+import { generateJSONPatch, pathInfo } from './index';
 import { applyPatch, deepClone } from 'fast-json-patch';
 import { assert, expect } from 'chai';
 
@@ -238,7 +238,6 @@ describe('a generate json patch function', () => {
       });
 
       const patched = doPatch(before, patch);
-      // as long as we do not support move, the result will be different from 'after' in its order
       expect(patched).to.be.eql([
         { id: 2, paramOne: 'current' },
         { id: 1, paramOne: 'current' },
@@ -324,9 +323,9 @@ describe('a generate json patch function', () => {
           },
         },
         {
+          from: '/1',
           op: 'move',
-          from: '/0',
-          path: '/1',
+          path: '/0',
         },
       ]);
     });
@@ -535,24 +534,26 @@ describe('a generate json patch function', () => {
       };
 
       const patch = generateJSONPatch(before, after, {
-        objectHash: function (value: JsonValue, context: GeneratePatchContext) {
+        objectHash: function (value: JsonValue, context: ObjectHashContext) {
           const { length, last } = pathInfo(context.path);
           if (length === 2 && last === 'engine') {
             // @ts-ignore
             return value?.name;
           }
-          return JSON.stringify(value);
+          return context.index.toString();
         },
       });
 
+      const patched = doPatch(before, patch);
+      expect(patched).to.be.eql(after);
+
       expect(patch).to.be.eql([
         { op: 'replace', path: '/engine/3/hp', value: 138 },
-        { op: 'move', from: '/engine/2', path: '/engine/3' },
-        { op: 'move', from: '/engine/1', path: '/engine/2' },
-        { op: 'move', from: '/engine/0', path: '/engine/1' },
+        { op: 'move', from: '/engine/3', path: '/engine/0' },
       ]);
     });
   });
+
   describe('with property filter', () => {
     it('ignores property on root filter', () => {
       const before = {
@@ -655,7 +656,7 @@ describe('a generate json patch function', () => {
 });
 
 function doPatch(json: JsonValue, patch: Patch) {
-  return applyPatch(deepClone(json), patch, true, true).newDocument;
+  return applyPatch(deepClone(json), patch, true, false).newDocument;
 }
 
 function expectPatchedEqualsAfter(before: JsonValue, after: JsonValue) {

src/index.ts

@@ -1,6 +1,6 @@
-export type JsonObject =
-  | { [Key in string]: JsonValue }
-  | { [Key in string]?: JsonValue };
+import { moveOperations } from './move-operations';
+
+export type JsonObject = { [Key in string]: JsonValue | undefined };
 
 export type JsonArray = JsonValue[] | readonly JsonValue[];
 
@@ -134,21 +134,8 @@ export function generateJSONPatch(
       return;
     }
 
-    // For movement, it's already iterating from back to front
-    for (let i = rightHashes.length - 1; i >= 0; i--) {
-      const hash = rightHashes[i];
-      const targetIndex = rightHashes.indexOf(hash);
-      const currentIndex = targetHashes.indexOf(hash);
-
-      if (currentIndex !== targetIndex) {
-        patch.push({
-          op: 'move',
-          from: `${path}/${currentIndex}`,
-          path: `${path}/${targetIndex}`,
-        });
-        moveArrayElement(targetHashes, currentIndex, targetIndex);
-      }
-    }
+    const moveOps = moveOperations(targetHashes, rightHashes, path);
+    patch.push(...moveOps);
   }
 
   function compareObjects(
@@ -240,10 +227,6 @@ function isJsonObject(value: JsonValue): value is JsonObject {
   return value?.constructor === Object;
 }
 
-function moveArrayElement(array: any[], from: number, to: number): void {
-  array.splice(to, 0, array.splice(from, 1)[0]);
-}
-
 export type PathInfoResult = {
   segments: string[];
   length: number;

src/move-operations.spec.ts

@@ -0,0 +1,72 @@
+import { expect } from 'chai';
+import { longestCommonSequence, moveOperations } from './move-operations';
+
+describe('a moveOperations function', () => {
+  it('should return an empty array when given two empty arrays', () => {
+    expect(moveOperations([], [])).to.eql([]);
+  });
+  it('should return an empty array when given two arrays with the same values', () => {
+    expect(moveOperations(['1', '2', '3'], ['1', '2', '3'])).to.eql([]);
+  });
+  it('should return a single move operation when given two arrays with one value moved', () => {
+    expect(moveOperations(['1', '2', '3'], ['1', '3', '2'])).to.eql([
+      { op: 'move', from: '/2', path: '/1' },
+    ]);
+  });
+  it('should return a single move operation when given two arrays with one value moved and different length', () => {
+    expect(moveOperations(['1', '2', '3', '4'], ['1', '3', '2'])).to.eql([
+      { op: 'move', from: '/2', path: '/1' },
+    ]);
+  });
+  it('should return an empty array when given two arrays with a removed key', () => {
+    expect(moveOperations(['0', '1', '2'], ['0', '1', '2', '3'])).to.eql([]);
+  });
+  it('should return an empty array when given two arrays with aan added key', () => {
+    expect(moveOperations(['0', '1', '2', '3'], ['0', '1', '2'])).to.eql([]);
+  });
+});
+
+describe('a longestCommonSequence function', () => {
+  it('should return an empty array when given two empty arrays', () => {
+    expect(longestCommonSequence([], [])).to.eql({
+      length: 0,
+      sequence: [],
+      offset: null,
+    });
+  });
+  it('should return the full sequence when given two identical arrays', () => {
+    expect(longestCommonSequence(['1', '2', '3'], ['1', '2', '3'])).to.eql({
+      length: 3,
+      sequence: ['1', '2', '3'],
+      offset: 0,
+    });
+  });
+  it('should return the a sequence at the beginning of an array', () => {
+    expect(
+      longestCommonSequence(['1', '2', '3', '4'], ['4', '1', '2', '3'])
+    ).to.eql({
+      length: 3,
+      sequence: ['1', '2', '3'],
+      offset: 0,
+    });
+  });
+  it('should return the a sequence at the end of an array', () => {
+    expect(
+      longestCommonSequence(['4', '1', '2', '3'], ['1', '2', '3', '4'])
+    ).to.eql({
+      length: 3,
+      sequence: ['1', '2', '3'],
+      offset: 1,
+    });
+  });
+
+  it('should return the a sequence at the end of an array', () => {
+    expect(longestCommonSequence(['0', '1', '2'], ['0', '1', '2', '3'])).to.eql(
+      {
+        length: 3,
+        sequence: ['0', '1', '2'],
+        offset: 0,
+      }
+    );
+  });
+});

src/move-operations.ts

@@ -0,0 +1,73 @@
+import { MoveOperation } from './index';
+
+export function longestCommonSequence(
+  leftHashes: string[],
+  rightHashes: string[]
+) {
+  const m = leftHashes.length;
+  const n = rightHashes.length;
+  const dp: number[][] = Array.from({ length: m + 1 }, () =>
+    Array(n + 1).fill(0)
+  );
+  let longestSequence: string[] = [];
+  let offset: number | null = null;
+
+  for (let i = 1; i <= m; i++) {
+    for (let j = 1; j <= n; j++) {
+      if (leftHashes[i - 1] === rightHashes[j - 1]) {
+        dp[i][j] = dp[i - 1][j - 1] + 1;
+        if (dp[i][j] > longestSequence.length) {
+          longestSequence = leftHashes.slice(i - dp[i][j], i);
+          offset = i - dp[i][j];
+        }
+      } else {
+        dp[i][j] = 0;
+      }
+    }
+  }
+  return {
+    length: longestSequence.length,
+    sequence: longestSequence,
+    offset,
+  };
+}
+
+export function moveOperations(
+  leftHashes: string[],
+  rightHashes: string[],
+  currentPath = ''
+): MoveOperation[] {
+  const { sequence } = longestCommonSequence(leftHashes, rightHashes);
+  const operations: MoveOperation[] = [];
+  let workingArr = [...leftHashes];
+  let lcsIndex = 0;
+  let targetIndex = 0;
+
+  while (targetIndex < rightHashes.length) {
+    const targetValue = rightHashes[targetIndex];
+
+    if (sequence[lcsIndex] === targetValue) {
+      lcsIndex++;
+      targetIndex++;
+      continue;
+    }
+
+    const sourceIndex = workingArr.indexOf(targetValue);
+
+    if (sourceIndex !== targetIndex && sourceIndex !== -1) {
+      operations.push({
+        op: 'move',
+        from: `${currentPath}/${sourceIndex}`,
+        path: `${currentPath}/${targetIndex}`,
+      });
+
+      // Update the working array to reflect the move
+      const [movedItem] = workingArr.splice(sourceIndex, 1);
+      workingArr.splice(targetIndex, 0, movedItem);
+    }
+
+    targetIndex++;
+  }
+
+  return operations;
+}