From e9c79aac369828073e9f2bb9dc02b86d8868b7cd Mon Sep 17 00:00:00 2001 From: Samppa Saarela Date: Thu, 20 Nov 2025 11:02:27 +0200 Subject: [PATCH 1/5] chore: improve tests for object arrays Signed-off-by: Samppa Saarela --- packages/diff/src/Diff.spec.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/diff/src/Diff.spec.ts b/packages/diff/src/Diff.spec.ts index a2a2fc5..617e5b2 100644 --- a/packages/diff/src/Diff.spec.ts +++ b/packages/diff/src/Diff.spec.ts @@ -33,20 +33,22 @@ describe('Diff', () => { object: { name: 'Alexis', }, - array: [0] + array: [ + { name:'Foo' } + ] }; const newObject = {}; describe('remove nested object', () => { test('with includeObjects: false', () => { const paths = defaultDiff.changedPaths(oldObject, newObject); - const expected = new Set(['$.object.name', '$.array[0]']); + const expected = new Set(['$.object.name', '$.array[0].name']); expect(paths).toEqual(expected); }); test('with includeObjects: true', () => { const paths = new Diff({ includeObjects: true }).changedPaths(oldObject, newObject); - const expected = new Set(['$.object', '$.object.name', '$.array', '$.array[0]']); + const expected = new Set(['$.object', '$.object.name', '$.array', '$.array[0]', '$.array[0].name']); expect(paths).toEqual(expected); }); }); @@ -54,13 +56,13 @@ describe('Diff', () => { describe('add nested object', () => { test('with includeObjects: false', () => { const paths = defaultDiff.changedPaths(newObject, oldObject); - const expected = new Set(['$.object.name', '$.array[0]']); + const expected = new Set(['$.object.name', '$.array[0].name']); expect(paths).toEqual(expected); }); test('with includeObjects: true', () => { const diff = new Diff({ includeObjects: true }).changedPaths(newObject, oldObject); - const expected = new Set(['$.object', '$.object.name', '$.array', '$.array[0]']); + const expected = new Set(['$.object', '$.object.name', '$.array', '$.array[0]', '$.array[0].name']); expect(diff).toEqual(expected); }); }); From c537ac8c2aabc24977481ed7da84250aa620e3d8 Mon Sep 17 00:00:00 2001 From: Samppa Saarela Date: Thu, 27 Nov 2025 10:10:05 +0200 Subject: [PATCH 2/5] feat: better support for patching Old Diff was designed primarily for change detection but used also internally for patching. While path/value "Changes" can be sent over line and applied, they are not very good for that (i.e. result in unnecessary large payloads). While deletions from the head of an array, with this new DiffNode, still result in a large diff/patch, this feature addresses how new object/array values are handled: `DiffNode.patch` contains the new value as-is in case the old value is missing (or it's type has changed). Also deleted object values will not be "exploaded" into path/value -deletions. For example ```ts const a = {}; const b = { nested: { foo: 'bar' } } new DiffNode({ oldValue: a, newValue: b }).patch // => [ { path: '$.nested', value: { foo: 'bar' } } ] new DiffNode({ oldValue: b, newValue: a }).patch // => [ { path: '$.nested' } ] ``` `DiffNode.patch` can be used to patch objects using `Path.set` just like the results of the `Diff.changeset`. Signed-off-by: Samppa Saarela --- packages/diff/src/Diff.spec.ts | 67 ++++++-- packages/diff/src/Diff.ts | 134 ++++------------ packages/diff/src/DiffNode.spec.ts | 115 ++++++++++++++ packages/diff/src/DiffNode.ts | 235 +++++++++++++++++++++++++++++ packages/diff/src/VersionInfo.ts | 3 +- packages/diff/src/index.ts | 1 + packages/path/src/Path.spec.ts | 1 - packages/path/src/Path.ts | 2 +- 8 files changed, 439 insertions(+), 119 deletions(-) create mode 100644 packages/diff/src/DiffNode.spec.ts create mode 100644 packages/diff/src/DiffNode.ts diff --git a/packages/diff/src/Diff.spec.ts b/packages/diff/src/Diff.spec.ts index 617e5b2..447b132 100644 --- a/packages/diff/src/Diff.spec.ts +++ b/packages/diff/src/Diff.spec.ts @@ -1,20 +1,67 @@ import { describe, test, expect } from 'vitest'; -import { Change, Diff } from './Diff.js'; -import { Path } from '@finnair/path'; +import { Diff } from './Diff.js'; +import { Node, Path } from '@finnair/path'; +import { Change } from './DiffNode.js'; describe('Diff', () => { const defaultDiff = new Diff(); - describe('allPaths', () => { - const object = { object: { string: "string"}, array: [0], 'undefined': undefined, 'null': null }; - test('all paths with default filter (without undefined values)', () => { - const paths = defaultDiff.allPaths(object); - expect(paths).toEqual(new Set([ '$.object.string', '$.array[0]', '$.null'])); + describe('helpers', () => { + const object = { + object: { string: "string"}, + array: [0], + 'undefined': undefined, + 'null': null + }; + describe('allPaths', () => { + test('with default filter (without undefined values)', () => { + const paths = defaultDiff.allPaths(object); + expect(paths).toEqual(new Set([ '$.object.string', '$.array[0]', '$.null'])); + }); + test('including undefined paths', () => { + const paths = new Diff({ filter: () => true }).allPaths(object); + expect(paths).toEqual(new Set([ '$.object.string', '$.array[0]', '$.undefined', '$.null'])); + }); + test('including objects', () => { + const paths = new Diff({ includeObjects: true}).allPaths(object); + expect(paths).toEqual(new Set([ '$.object', '$.object.string', '$.array', '$.array[0]', '$.null'])); + }); + test('array', () => { + const paths = new Diff().allPaths([object]); + expect(paths).toEqual(new Set([ '$[0].object.string', '$[0].array[0]', '$[0].null' ])); + }); }); - test('all, including undefined paths', () => { - const paths = new Diff({ filter: () => true }).allPaths(object); - expect(paths).toEqual(new Set([ '$.object.string', '$.array[0]', '$.undefined', '$.null'])); + + describe('pathsAndValues', () => { + test('all pathsAndValues with default filter (without undefined values)', () => { + const pathsAndValues = defaultDiff.pathsAndValues(object); + expect(pathsAndValues).toEqual(new Map([ + ['$.object.string', { path: Path.of('object', 'string'), value: 'string' }], + ['$.array[0]', { path: Path.of('array', 0), value: 0 }], + ['$.null', { path: Path.of('null'), value: null }], + ])); + }); + test('all, including undefined pathsAndValues', () => { + const pathsAndValues = new Diff({ filter: () => true }).pathsAndValues(object); + expect(pathsAndValues).toEqual(new Map([ + ['$.object.string', { path: Path.of('object', 'string'), value: 'string' }], + ['$.array[0]', { path: Path.of('array', 0), value: 0 }], + ['$.undefined', { path: Path.of('undefined'), value: undefined }], + ['$.null', { path: Path.of('null'), value: null }], + ])); + }); + test('including objects', () => { + const pathsAndValues = new Diff({ includeObjects: true}).pathsAndValues(object); + expect(pathsAndValues).toEqual(new Map([ + ['$', { path: Path.ROOT, value: {} }], + ['$.object', { path: Path.of('object'), value: {} }], + ['$.object.string', { path: Path.of('object', 'string'), value: 'string' }], + ['$.array', { path: Path.of('array'), value: [] }], + ['$.array[0]', { path: Path.of('array', 0), value: 0 }], + ['$.null', { path: Path.of('null'), value: null }], + ])); + }); }); }); diff --git a/packages/diff/src/Diff.ts b/packages/diff/src/Diff.ts index 01ee728..bd7bed5 100644 --- a/packages/diff/src/Diff.ts +++ b/packages/diff/src/Diff.ts @@ -1,127 +1,49 @@ -import { Node, Path } from '@finnair/path'; +import { Node } from '@finnair/path'; +import { arrayOrPlainObject, Change, DiffNode, DiffNodeConfig } from './DiffNode'; -export const defaultDiffFilter = (_path: Path, value: any) => value !== undefined; - -export interface DiffConfig { - readonly filter?: DiffFilter; - readonly isPrimitive?: (value: any, path: Path) => boolean; - readonly isEqual?: (a: any, b: any, path: Path) => boolean; +export interface DiffConfig extends DiffNodeConfig { readonly includeObjects?: boolean; } -export interface DiffFilter { - (path: Path, value: any): boolean; -} - -export interface Change { - readonly path: Path; - readonly oldValue?: any; - readonly newValue?: any; -} - -const OBJECT = Object.freeze({}); -const ARRAY = Object.freeze([]); - -const primitiveTypes: any = { - 'boolean': true, - 'number': true, - 'string': true, - 'bigint': true, - 'symbol': true, -}; - -function isPrimitive(value: any) { - return value === null || value === undefined || !!primitiveTypes[typeof value]; -} - export class Diff { constructor(private readonly config?: DiffConfig) {} allPaths(value: any) { - const paths = new Set(); - this.collectPathsAndValues( - value, - (node: Node) => { - paths.add(node.path.toJSON()); - }, - ); - return paths; + return this.changedPaths(getBaseValue(value), value); } - changedPaths(a: T, b: T) { - return new Set(this.changeset(a, b).keys()); + changedPaths(oldValue: T, newValue: T) { + const paths = new Set(); + const diffNode = new DiffNode({ oldValue, newValue }, this.config); + for (const path of diffNode.getChangedPaths(this.config?.includeObjects)) { + paths.add(path.toJSON()); + } + return paths; } - changeset(a: T, b: T): Map { + changeset(oldValue: T, newValue: T): Map { const changeset = new Map(); - const aMap = this.pathsAndValues(a); - this.collectPathsAndValues( - b, - (bNode: Node) => { - const path = bNode.path; - const bValue = bNode.value; - const pathStr = path.toJSON(); - if (aMap.has(pathStr)) { - const aNode = aMap.get(pathStr); - const aValue = aNode?.value; - aMap.delete(pathStr); - if (!this.isEqual(aValue, bValue, path)) { - changeset.set(pathStr, { path, oldValue: aValue, newValue: this.getNewValue(bValue) }); - } - } else { - changeset.set(pathStr, { path, newValue: this.getNewValue(bValue) }); - } - }, - Path.ROOT - ); - aMap.forEach((node, pathStr) => changeset.set(pathStr, { path: node.path, oldValue: node.value })); + const diffNode = new DiffNode({ oldValue, newValue }, this.config); + for (const change of diffNode.getScalarChanges(this.config?.includeObjects)) { + changeset.set(change.path.toJSON(), change); + } return changeset; } - pathsAndValues(value: any) { - const map: Map = new Map(); - this.collectPathsAndValues(value, (node: Node) => map.set(node.path.toJSON(), { path: node.path, value: node.value }), Path.ROOT); - return map; - } - - private collectPathsAndValues(value: any, collector: (node: Node) => void, path: Path = Path.ROOT) { - if ((this.config?.filter ?? defaultDiffFilter)(path, value)) { - if (isPrimitive(value) || this.config?.isPrimitive?.(value, path)) { - collector({ path, value }); - } else if (typeof value === 'object') { - if (Array.isArray(value)) { - if (this.config?.includeObjects) { - collector({ path, value: ARRAY }); - } - value.forEach((element, index) => this.collectPathsAndValues(element, collector, path.index(index))); - } else if (value.constructor === Object) { - if (this.config?.includeObjects) { - collector({ path, value: OBJECT }); - } - Object.keys(value).forEach((key) => - this.collectPathsAndValues(value[key], collector, path.property(key)) - ); - } else { - throw new Error(`only primitives, arrays and plain objects are supported, got "${value.constructor.name}"`); - } - } + pathsAndValues(value: any): Map { + const map = new Map(); + const diffNode = new DiffNode({ newValue: value }, this.config); + for (const change of diffNode.getScalarChanges(this.config?.includeObjects)) { + map.set(change.path.toJSON(), { path: change.path, value: change.newValue }); } + return map; } +} - private isEqual(a: any, b: any, path: Path): boolean { - if (a === b) { - return true; - } - return !!this.config?.isEqual?.(a, b, path); - } - - private getNewValue(value: any) { - if (value === OBJECT) { - return {}; - } - if (value === ARRAY) { - return []; - } - return value; +function getBaseValue(value: any) { + switch (arrayOrPlainObject(value)) { + case 'object': return {}; + case 'array': return []; + default: return undefined; } } diff --git a/packages/diff/src/DiffNode.spec.ts b/packages/diff/src/DiffNode.spec.ts new file mode 100644 index 0000000..9eec042 --- /dev/null +++ b/packages/diff/src/DiffNode.spec.ts @@ -0,0 +1,115 @@ + +import { describe, test, expect } from 'vitest'; +import { DiffNode, DiffNodeConfig } from './DiffNode'; +import { Path } from '@finnair/path'; + +const strictOptionalPropertiesConfig: DiffNodeConfig = { filter: () => true }; + +describe('DiffNode', () => { + describe('getPatch', () => { + const object: any = { + string: 'string', + undefined: undefined, + object: { number: 1 }, + array: [ 1, { boolean: true } ], + }; + + test('new string value', () => { + expect(Array.from(new DiffNode({ newValue: 'string' }).patch)) + .toEqual([{ path: Path.ROOT, value: 'string' }]); + }); + test('new object value', () => { + expect(Array.from(new DiffNode({ newValue: object }).patch)) + .toEqual([{ path: Path.ROOT, value: object }]); + }); + test('nested modification', () => { + const clone = structuredClone(object); + delete clone.object; + clone.array[1].boolean = false; + clone.array[1].newProp = 'newProp'; + + expect(Array.from(new DiffNode({ oldValue: object, newValue: clone }).patch)) + .toEqual([ + { path: Path.of('object') }, + { path: Path.of('array', 1, 'boolean'), value: false }, + { path: Path.of('array', 1, 'newProp'), value: 'newProp' }, + ]); + expect(Array.from(new DiffNode({ oldValue: clone, newValue: object }).patch)) + .toEqual([ + { path: Path.of('array', 1, 'boolean'), value: true }, + { path: Path.of('array', 1, 'newProp') }, + { path: Path.of('object'), value: { number: 1 } }, + ]); + }); + }); + describe('type changes', () => { + test('undefined to string', () => { + const change = new DiffNode({ oldValue: undefined, newValue: 'string'}, strictOptionalPropertiesConfig).getScalarChange(true)!; + expect(change).toEqual({ path: Path.ROOT, oldValue: undefined, newValue: 'string' }); + expect('oldValue' in change).toBe(true); + }); + test('missing to string', () => { + const change = new DiffNode({ newValue: 'string' }, strictOptionalPropertiesConfig).getScalarChange(true)!; + expect(change).toEqual({ path: Path.ROOT, newValue: 'string' }); + expect('oldValue' in change).toBe(false); + }); + describe('array to object', () => { + const node = new DiffNode({ oldValue: [1], newValue: {'0': 1} }, strictOptionalPropertiesConfig); + test('scalar', () => { + expect(Array.from(node.getScalarChanges(true))).toEqual([ + { path: Path.ROOT, oldValue: [], newValue: {} }, + { path: Path.of(0), oldValue: 1 }, + { path: Path.of('0'), newValue: 1 }, + ]); + }); + test('patch', () => { + expect(Array.from(node.patch)).toEqual([ + { path: Path.ROOT, value: { '0': 1 } } + ]); + }); + }); + describe('object to array', () => { + const node = new DiffNode({ oldValue: {'0': 1}, newValue: [1] }, strictOptionalPropertiesConfig); + test('scalar', () => { + expect(Array.from(node.getScalarChanges(true))).toEqual([ + { path: Path.ROOT, oldValue: {}, newValue: [] }, + { path: Path.of('0'), oldValue: 1 }, + { path: Path.of(0), newValue: 1 }, + ]); + }); + test('patch', () => { + expect(Array.from(node.patch)).toEqual([ + { path: Path.ROOT, value: [1] } + ]); + }); + }); + describe('string to object', () => { + const node = new DiffNode({ oldValue: 'string', newValue: { string: 'string' } }); + test('scalar', () => { + expect(Array.from(node.getScalarChanges(true))).toEqual([ + { path: Path.ROOT, oldValue: 'string', newValue: {} }, + { path: Path.of('string'), newValue: 'string' }, + ]); + }); + test('patch', () => { + expect(Array.from(node.patch)).toEqual([ + { path: Path.ROOT, value: { string: 'string' } } + ]); + }); + }); + describe('object to boolean', () => { + const node = new DiffNode({ oldValue: { boolean: true }, newValue: true }); + test('scalar', () => { + expect(Array.from(node.getScalarChanges(true))).toEqual([ + { path: Path.ROOT, oldValue: {}, newValue: true }, + { path: Path.of('boolean'), oldValue: true }, + ]); + }); + test('patch', () => { + expect(Array.from(node.patch)).toEqual([ + { path: Path.ROOT, value: true } + ]); + }); + }); + }); +}); diff --git a/packages/diff/src/DiffNode.ts b/packages/diff/src/DiffNode.ts new file mode 100644 index 0000000..7268018 --- /dev/null +++ b/packages/diff/src/DiffNode.ts @@ -0,0 +1,235 @@ +import { Path } from "@finnair/path"; + +export interface DiffNodeConfig { + readonly filter?: DiffFilter; + readonly isPrimitive?: (value: any, path: Path) => boolean; + readonly isEqual?: (a: any, b: any, path: Path) => boolean; +} + +export interface DiffFilter { + (path: Path, value: any): boolean; +} + +export interface Patch { + readonly path: Path; + readonly value?: any; +} + +export interface Change { + readonly path: Path; + readonly newValue?: any; + readonly oldValue?: any; +} + +export const defaultDiffFilter = (_path: Path, value: any) => value !== undefined; + +export type ValueType = 'primitive' | 'object' | 'array' | undefined; + +const primitiveTypes: any = { + 'boolean': true, + 'number': true, + 'string': true, + 'bigint': true, + 'symbol': true, +}; + +function isPrimitive(value: any) { + return value === null || value === undefined || !!primitiveTypes[typeof value]; +} + +interface DiffNodeProps { + path?: Path, + oldValue?: any, + newValue?: any, +} + +export class DiffNode { + public readonly path: Path; + public readonly oldType: ValueType; + public readonly oldValue?: any; + public readonly newType: ValueType; + public readonly newValue?: any; + private _children: undefined | Map; + constructor( + props: DiffNodeProps, + private readonly config?: DiffNodeConfig + ) { + this.path = props.path ?? Path.ROOT; + this.oldType = 'oldValue' in props ? this.getValueType(props.oldValue) : undefined; + this.oldValue = props.oldValue; + this.newType = 'newValue' in props ? this.getValueType(props.newValue) : undefined; + this.newValue = props.newValue; + } + + get children(): undefined | Map { + if (this.isComposite && this._children === undefined) { + const nodeProps = new Map(); + if (isCompositeType(this.oldType)) { + this.collectNodes(this.oldType, this.oldValue, 'oldValue', nodeProps); + } + if (isCompositeType(this.newType)) { + this.collectNodes(this.newType, this.newValue, 'newValue', nodeProps); + } + this._children = new Map(); + for (const [key, props] of nodeProps.entries()) { + this._children.set(key, new DiffNode(props, this.config)); + } + } + return this._children; + } + + getScalarChange(includeObjects = false): undefined | Change { + if (this.isScalarChange(includeObjects)) { + const change: { -readonly [P in keyof Change]: Change[P] } = { + path: this.path, + }; + if (this.oldType) { + change.oldValue = scalarValue(this.oldType, this.oldValue); + } + if (this.newType) { + change.newValue = scalarValue(this.newType, this.newValue); + } + return change as Change; + } + return undefined; + } + + isScalarChange(includeObjects = false) { + return this.isChange && (this.isPrimitive || (this.isComposite && includeObjects)); + } + + getScalarChanges(includeObjects = false): Iterable { + return { + [Symbol.iterator]: () => scalarGenerator(this, includeObjects) + }; + } + + get patch(): Iterable { + return { + [Symbol.iterator]: () => patchGenerator(this) + }; + } + + getChangedPaths(includeObjects = false): Iterable { + return { + [Symbol.iterator]: () => changedPathGenerator(this, includeObjects) + }; + } + + get isChange(): boolean { + if (this.newType === this.oldType) { + if (this.newValue === this.oldValue) { + return false; + } else if (this.newType === 'primitive') { + return !this.config?.isEqual?.(this.oldValue, this.newValue, this.path); + } + // both are objects or arrays + return false; + } + // Type has changed! + return true; + } + + get isPrimitive(): boolean { + return isIncludedPrimitiveType(this.oldType) || isIncludedPrimitiveType(this.newType); + } + + get isComposite(): boolean { + return isCompositeType(this.oldType) || isCompositeType(this.newType); + } + + private collectNodes(type: 'object' | 'array', value: any, role: 'oldValue' | 'newValue', nodeProps: Map) { + const register = (key: string | number, value: any) => { + const props = nodeProps.get(key); + if (props) { + props[role] = value; + } else { + nodeProps.set(key, { path: this.path.child(key), [role]: value}); + } + }; + + if (type === 'object') { + Object.entries(value).forEach(([key, value]) => register(key, value)); + } else { + value.forEach((value: any, index: number) => register(index, value)); + } + } + + private getValueType(value: any): undefined | ValueType { + if ((this.config?.filter ?? defaultDiffFilter)(this.path, value)) { + if (isPrimitive(value) || this.config?.isPrimitive?.(value, this.path)) { + return 'primitive'; + } + const compositeType = arrayOrPlainObject(value); + if (compositeType) { + return compositeType; + } + throw new Error(`only primitives, arrays and plain objects are supported, got "${value?.constructor.name}"`); + } + return undefined; + } +} + +function scalarValue(valueType: ValueType, value: any) { + switch (valueType) { + case 'object': return {}; + case 'array': return []; + default: return value; + } +} + +export function arrayOrPlainObject(value: any): undefined | 'array' | 'object' { + if (value && typeof value === 'object') { + if (Array.isArray(value)) { + return 'array'; + } else if (value.constructor === Object) { + return 'object'; + } + } + return undefined; +} + +function isIncludedPrimitiveType(valueType: ValueType): valueType is 'primitive' | undefined { + return valueType === 'primitive'; +} + +function isCompositeType(valueType: ValueType): valueType is 'object' | 'array' { + return valueType === 'object' || valueType === 'array'; +} + +function* scalarGenerator(node: DiffNode, includeObjects = false): Generator { + const change = node.getScalarChange(includeObjects); + if (change) { + yield change; + } + if (node.children) { + for (const child of node.children.values()) { + yield* scalarGenerator(child, includeObjects); + } + } +} + +function* changedPathGenerator(node: DiffNode, includeObjects = false): Generator { + if (node.isScalarChange(includeObjects)) { + yield node.path; + } + if (node.children) { + for (const child of node.children.values()) { + yield* changedPathGenerator(child, includeObjects); + } + } +} + +function* patchGenerator(node: DiffNode): Generator { + if (node.isChange) { + if (node.newType === undefined) { + yield { path: node.path } + } else { + yield { path: node.path, value: node.newValue } + } + } else if (node.children) { + for (const child of node.children.values()) { + yield* patchGenerator(child); + } + } +} diff --git a/packages/diff/src/VersionInfo.ts b/packages/diff/src/VersionInfo.ts index af11c90..633ecb2 100644 --- a/packages/diff/src/VersionInfo.ts +++ b/packages/diff/src/VersionInfo.ts @@ -1,6 +1,7 @@ import { Path, PathMatcher } from '@finnair/path'; import { parsePath, parsePathMatcher } from '@finnair/path-parser'; -import { Change, Diff } from './Diff.js'; +import { Diff } from './Diff.js'; +import { Change } from './DiffNode.js'; export interface VersionInfoConfig { readonly diff: Diff; diff --git a/packages/diff/src/index.ts b/packages/diff/src/index.ts index c8d87f5..4d6900d 100644 --- a/packages/diff/src/index.ts +++ b/packages/diff/src/index.ts @@ -1,2 +1,3 @@ export * from './Diff.js'; +export * from './DiffNode.js' export * from './VersionInfo.js'; diff --git a/packages/path/src/Path.spec.ts b/packages/path/src/Path.spec.ts index a34ab8c..f084287 100644 --- a/packages/path/src/Path.spec.ts +++ b/packages/path/src/Path.spec.ts @@ -2,7 +2,6 @@ import { describe, test, expect } from 'vitest' import { Path, PathComponent } from './Path.js'; import { PathMatcher } from './PathMatcher.js'; import { AnyProperty, AnyIndex } from './matchers.js'; -import path from 'path'; describe('path', () => { test('toJSON', () => expect(Path.property('s p a c e s').index(5).property('regular').toJSON()).toEqual('$["s p a c e s"][5].regular')); diff --git a/packages/path/src/Path.ts b/packages/path/src/Path.ts index 084adc7..6a10020 100644 --- a/packages/path/src/Path.ts +++ b/packages/path/src/Path.ts @@ -12,7 +12,7 @@ export class Path { Object.freeze(this.path); Object.freeze(this); } - + index(index: number): Path { Path.validateIndex(index); return new Path(this.path.concat(index)); From 4126ae45812da78da289bc468cebf4623872cc11 Mon Sep 17 00:00:00 2001 From: Samppa Saarela Date: Thu, 27 Nov 2025 12:12:06 +0200 Subject: [PATCH 3/5] feat: support patch in VersionInfo NOTE: This commit introduces a slight alteration to the result of `VersionInfo.paths` for a case where there is no previous version or it's type has changed significantly and includeObjects is true: the root object is also included. This makes sense and fixes an oversight of not including it in the first place. Signed-off-by: Samppa Saarela --- packages/diff/src/Diff.ts | 35 +++++++++++++++------- packages/diff/src/VersionInfo.spec.ts | 43 ++++++++++++++++++++------- packages/diff/src/VersionInfo.ts | 41 +++++++++++++++++++++---- 3 files changed, 93 insertions(+), 26 deletions(-) diff --git a/packages/diff/src/Diff.ts b/packages/diff/src/Diff.ts index bd7bed5..9bdcb66 100644 --- a/packages/diff/src/Diff.ts +++ b/packages/diff/src/Diff.ts @@ -6,34 +6,49 @@ export interface DiffConfig extends DiffNodeConfig { } export class Diff { - constructor(private readonly config?: DiffConfig) {} + constructor(public readonly config?: DiffConfig) {} allPaths(value: any) { - return this.changedPaths(getBaseValue(value), value); + return Diff.allPaths(value, this.config); } changedPaths(oldValue: T, newValue: T) { + return Diff.changedPaths(oldValue, newValue, this.config); + } + + changeset(oldValue: T, newValue: T): Map { + return Diff.changeset(oldValue, newValue, this.config); + } + + pathsAndValues(value: any): Map { + return Diff.pathsAndValues(value, this.config); + } + + static allPaths(value: any, config?: DiffConfig) { + return Diff.changedPaths(getBaseValue(value), value, config); + } + static changedPaths(oldValue: T, newValue: T, config?: DiffConfig) { const paths = new Set(); - const diffNode = new DiffNode({ oldValue, newValue }, this.config); - for (const path of diffNode.getChangedPaths(this.config?.includeObjects)) { + const diffNode = new DiffNode({ oldValue, newValue }, config); + for (const path of diffNode.getChangedPaths(config?.includeObjects)) { paths.add(path.toJSON()); } return paths; } - changeset(oldValue: T, newValue: T): Map { + static changeset(oldValue: T, newValue: T, config?: DiffConfig): Map { const changeset = new Map(); - const diffNode = new DiffNode({ oldValue, newValue }, this.config); - for (const change of diffNode.getScalarChanges(this.config?.includeObjects)) { + const diffNode = new DiffNode({ oldValue, newValue }, config); + for (const change of diffNode.getScalarChanges(config?.includeObjects)) { changeset.set(change.path.toJSON(), change); } return changeset; } - pathsAndValues(value: any): Map { + static pathsAndValues(value: any, config?: DiffConfig): Map { const map = new Map(); - const diffNode = new DiffNode({ newValue: value }, this.config); - for (const change of diffNode.getScalarChanges(this.config?.includeObjects)) { + const diffNode = new DiffNode({ newValue: value }, config); + for (const change of diffNode.getScalarChanges(config?.includeObjects)) { map.set(change.path.toJSON(), { path: change.path, value: change.newValue }); } return map; diff --git a/packages/diff/src/VersionInfo.spec.ts b/packages/diff/src/VersionInfo.spec.ts index 86c4186..10d118b 100644 --- a/packages/diff/src/VersionInfo.spec.ts +++ b/packages/diff/src/VersionInfo.spec.ts @@ -1,7 +1,8 @@ import { describe, test, expect } from 'vitest'; import { AnyIndex, Path, PathMatcher } from '@finnair/path'; -import { Change, Diff } from './Diff.js'; +import { Diff } from './Diff.js'; import { VersionInfo, VersionInfoConfig } from './VersionInfo.js'; +import { Change } from './DiffNode.js'; describe('VersionInfo', () => { const a: any = Object.freeze({ @@ -25,7 +26,7 @@ describe('VersionInfo', () => { }); const config: VersionInfoConfig = { - diff: new Diff({ + diffConfig: { isPrimitive: (value: any) => value instanceof Date, isEqual: (a: any, b: any) => { if (a instanceof Date && b instanceof Date) { @@ -34,9 +35,10 @@ describe('VersionInfo', () => { return false; }, filter: (path: Path, value: any) => path.length === 0 || !String(path.componentAt(0)).startsWith('_'), - }), + }, previousValues: [PathMatcher.of('id')], }; + // Support still deprecated diff parameter const altConfig: VersionInfoConfig = { diff: new Diff({ filter: (_path: Path, value: any) => !(value instanceof Date) }) }; const mapFn = (o: any) => { @@ -48,8 +50,6 @@ describe('VersionInfo', () => { }; const asyncMapFn = async (o: any) => mapFn(o); - - const expectedChangedPaths = new Set([ '$.id', '$.name.first', '$.name.last', '$.null', '$.undefined' ]); const mappedChanges = new Map([ ['$.firstName', { path: Path.of('firstName'), oldValue: 'first', newValue: 'second' }], @@ -65,6 +65,14 @@ describe('VersionInfo', () => { test('changes', async () => { const version = new VersionInfo(b, a, config); + const expectedChangedPaths = new Set([ '$.id', '$.name.first', '$.name.last', '$.null', '$.undefined' ]); + const extpectedPatch = [ + { path: Path.of('id'), value: 12345 }, + { path: Path.of('name', 'first'), value: 'second' }, + { path: Path.of('name', 'last'), value: 'last' }, + { path: Path.of('null') }, + { path: Path.of('undefined') }, + ]; expect(version.changes).toEqual(new Map([ ['$.id', { path: Path.of('id'), oldValue: 1234, newValue: 12345 }], @@ -74,6 +82,7 @@ describe('VersionInfo', () => { ['$.undefined', { path: Path.of('undefined'), oldValue: undefined }], ])); expect(version.changedPaths).toEqual(expectedChangedPaths); + expect(version.patch).toEqual(extpectedPatch); expect(version.paths).toEqual(version.changedPaths); expect(version.previousValues).toEqual({ id: 1234 }); expect(version.toJSON()).toEqual({ @@ -97,9 +106,9 @@ describe('VersionInfo', () => { expect((await version.mapAsync(asyncMapFn, altConfig)).changedPaths).toEqual(new Set(['$.firstName', '$.lastName'])); }); - test('apply chnanges', () => { + test('apply changes', () => { const version = new VersionInfo(b, a, config); - const aClone = { ...a, name: {...a.name} } + const aClone = structuredClone(a); expect(aClone).toEqual(a); version.changes!.forEach((change) => change.path.set(aClone, change.newValue)); @@ -109,6 +118,18 @@ describe('VersionInfo', () => { expect(aClone).toEqual(b); }); + test('apply patch', () => { + const version = new VersionInfo(b, a, config); + const aClone = structuredClone(a); + expect(aClone).toEqual(a); + + version.patch.forEach((patch) => patch.path.set(aClone, patch.value)); + + expect(a).not.toEqual(b); + aClone._timestamp = b._timestamp; + expect(aClone).toEqual(b); + }); + test('no changes', () => { const c = {...b, _timestamp: new Date(Date.UTC(2024, 8, 13)) }; @@ -116,6 +137,7 @@ describe('VersionInfo', () => { expect(version.paths).toEqual(new Set()); expect(version.changedPaths).toEqual(version.paths); + expect(version.patch).toEqual([]); expect(version.previousValues).toBeUndefined(); expect(version.toJSON()).toEqual({ current: c, changedPaths: [] }); expect(version.matches('$.name')).toBe(false); @@ -126,8 +148,9 @@ describe('VersionInfo', () => { const version = new VersionInfo(a, undefined, config); expect(version.changedPaths).toBeUndefined(); - expect(version.paths).toEqual(new Set(['$.id', '$.name.first', '$.null', '$.undefined'])); + expect(version.paths).toEqual(new Set(['$', '$.id', '$.name.first', '$.null', '$.undefined'])); expect(version.changes).toBeUndefined(); + expect(version.patch).toEqual([{ path: Path.ROOT, value: a }]) expect(version.matches('$.name')).toBe(true); expect(version.matchesAny(['$.foo', '$.name'])).toBe(true); expect(version.previousValues).toBeUndefined(); @@ -139,7 +162,7 @@ describe('VersionInfo', () => { lastName: undefined, timestamp: a._timestamp, }); - expect((await version.mapAsync(asyncMapFn)).paths).toEqual(new Set(['$.firstName', '$.lastName', '$.timestamp'])); + expect((await version.mapAsync(asyncMapFn)).paths).toEqual(new Set(['$', '$.firstName', '$.lastName', '$.timestamp'])); }); test('no previousValues matcher', () => { @@ -148,7 +171,7 @@ describe('VersionInfo', () => { test('array as root', () => { expect(new VersionInfo([2], [1], { - diff: new Diff(), + // Default diff/diffConfig previousValues: [PathMatcher.of(AnyIndex)], }).toJSON()).toEqual({ current: [2], diff --git a/packages/diff/src/VersionInfo.ts b/packages/diff/src/VersionInfo.ts index 633ecb2..f0d6bd0 100644 --- a/packages/diff/src/VersionInfo.ts +++ b/packages/diff/src/VersionInfo.ts @@ -1,10 +1,14 @@ import { Path, PathMatcher } from '@finnair/path'; import { parsePath, parsePathMatcher } from '@finnair/path-parser'; -import { Diff } from './Diff.js'; -import { Change } from './DiffNode.js'; +import { Diff, DiffConfig } from './Diff.js'; +import { Change, DiffNode, Patch } from './DiffNode.js'; export interface VersionInfoConfig { - readonly diff: Diff; + /** + * @deprecated use diffConfig instead + */ + readonly diff?: Diff; + readonly diffConfig?: DiffConfig; readonly previousValues?: PathMatcher[]; } @@ -14,13 +18,17 @@ export class VersionInfo { private _changes?: Map; private _paths?: Set; private _previousValues?: any; + private _diffNode?: DiffNode; public readonly config: VersionInfoConfig; constructor( public readonly current: L, public readonly previous?: L, config?: VersionInfoConfig ) { - this.config = config ?? { diff: new Diff({}) } + this.config = { + diffConfig: config?.diffConfig ?? config?.diff?.config, + previousValues: config?.previousValues, + }; } map(fn: (version: L) => T, config?: VersionInfoConfig) { return new VersionInfo( @@ -39,7 +47,10 @@ export class VersionInfo { get changes(): undefined | Map { if (this.previous) { if (this._changes === undefined) { - this._changes = this.config.diff.changeset(this.previous, this.current); + this._changes = new Map(); + for (const change of this.diffNode.getScalarChanges(this.config.diffConfig?.includeObjects)) { + this._changes.set(change.path.toJSON(), change); + } } return this._changes; } @@ -59,7 +70,10 @@ export class VersionInfo { if (this.previous) { this._paths = this.changedPaths!; } else { - this._paths = new Set(this.config.diff.allPaths(this.current)); + this._paths = new Set(); + for (const path of this.diffNode.getChangedPaths(this.config.diffConfig?.includeObjects)) { + this._paths.add(path.toJSON()); + } } } return this._paths; @@ -82,6 +96,21 @@ export class VersionInfo { } return undefined; } + get diffNode(): DiffNode { + if (this._diffNode === undefined) { + this._diffNode = new DiffNode( + { + oldValue: this.previous, + newValue: this.current, + }, + this.config.diffConfig + ); + } + return this._diffNode; + } + get patch(): Patch[] { + return Array.from(this.diffNode.patch); + } matches(pathExpression: string | PathMatcher) { const matcher = VersionInfo.toMatcher(pathExpression); if (this.previous) { From 08c2b30b321cb820f1d0b1266b82944878b6c328 Mon Sep 17 00:00:00 2001 From: Samppa Saarela Date: Fri, 28 Nov 2025 09:25:59 +0200 Subject: [PATCH 4/5] chore: rename function Signed-off-by: Samppa Saarela --- packages/diff/src/DiffNode.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/diff/src/DiffNode.ts b/packages/diff/src/DiffNode.ts index 7268018..9ff9a51 100644 --- a/packages/diff/src/DiffNode.ts +++ b/packages/diff/src/DiffNode.ts @@ -131,7 +131,7 @@ export class DiffNode { } get isPrimitive(): boolean { - return isIncludedPrimitiveType(this.oldType) || isIncludedPrimitiveType(this.newType); + return isPrimitiveType(this.oldType) || isPrimitiveType(this.newType); } get isComposite(): boolean { @@ -189,7 +189,7 @@ export function arrayOrPlainObject(value: any): undefined | 'array' | 'object' { return undefined; } -function isIncludedPrimitiveType(valueType: ValueType): valueType is 'primitive' | undefined { +function isPrimitiveType(valueType: ValueType): valueType is 'primitive' | undefined { return valueType === 'primitive'; } From 8315144e5ecaa351c4e8e39fd18d266c9dbae5fa Mon Sep 17 00:00:00 2001 From: Samppa Saarela Date: Fri, 28 Nov 2025 11:31:10 +0200 Subject: [PATCH 5/5] chore: review fixes Signed-off-by: Samppa Saarela --- packages/diff/src/DiffNode.ts | 21 +++++++++++---------- packages/diff/src/VersionInfo.ts | 8 +------- packages/path/src/Path.ts | 2 +- 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/packages/diff/src/DiffNode.ts b/packages/diff/src/DiffNode.ts index 9ff9a51..475d6d2 100644 --- a/packages/diff/src/DiffNode.ts +++ b/packages/diff/src/DiffNode.ts @@ -156,17 +156,18 @@ export class DiffNode { } private getValueType(value: any): undefined | ValueType { - if ((this.config?.filter ?? defaultDiffFilter)(this.path, value)) { - if (isPrimitive(value) || this.config?.isPrimitive?.(value, this.path)) { - return 'primitive'; - } - const compositeType = arrayOrPlainObject(value); - if (compositeType) { - return compositeType; - } - throw new Error(`only primitives, arrays and plain objects are supported, got "${value?.constructor.name}"`); + const filter = this.config?.filter ?? defaultDiffFilter; + if (!filter(this.path, value)) { + return undefined; } - return undefined; + if (isPrimitive(value) || this.config?.isPrimitive?.(value, this.path)) { + return 'primitive'; + } + const compositeType = arrayOrPlainObject(value); + if (compositeType) { + return compositeType; + } + throw new Error(`only primitives, arrays and plain objects are supported, got "${value?.constructor.name}"`); } } diff --git a/packages/diff/src/VersionInfo.ts b/packages/diff/src/VersionInfo.ts index f0d6bd0..af2a824 100644 --- a/packages/diff/src/VersionInfo.ts +++ b/packages/diff/src/VersionInfo.ts @@ -98,13 +98,7 @@ export class VersionInfo { } get diffNode(): DiffNode { if (this._diffNode === undefined) { - this._diffNode = new DiffNode( - { - oldValue: this.previous, - newValue: this.current, - }, - this.config.diffConfig - ); + this._diffNode = new DiffNode({ oldValue: this.previous, newValue: this.current }, this.config.diffConfig); } return this._diffNode; } diff --git a/packages/path/src/Path.ts b/packages/path/src/Path.ts index 6a10020..084adc7 100644 --- a/packages/path/src/Path.ts +++ b/packages/path/src/Path.ts @@ -12,7 +12,7 @@ export class Path { Object.freeze(this.path); Object.freeze(this); } - + index(index: number): Path { Path.validateIndex(index); return new Path(this.path.concat(index));