From 3e4795663db7654484b69cc2dafcde03518d6afd Mon Sep 17 00:00:00 2001 From: William Stein Date: Sun, 25 Oct 2026 20:41:17 +0000 Subject: [PATCH] mapMergePatch: leave unchanged null-valued keys out of the patch A map field's key whose value is null (nbformat allows null in Jupyter metadata) was re-sent as null by every update of that map, even when the key didn't change: the second loop skipped only keys of the old map with a non-null value. Peers apply a null key as a delete, so the first edit of the map deleted the key everywhere (sagemathinc/cocalc-ai#1388). Skip every key the old map already has; the first loop handles them. Removed keys are still sent as null, and a null key that gets a value is sent with it. Both document backends share mapMergePatch. The change is wire-compatible: patches only get smaller. --- src/db-document.test.ts | 32 ++++++++++++++++++++++++++++++++ src/db-util.ts | 6 +++++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/db-document.test.ts b/src/db-document.test.ts index 108854a..199215e 100644 --- a/src/db-document.test.ts +++ b/src/db-document.test.ts @@ -105,6 +105,38 @@ describe.each(backends)("%s DbDocument", ({ name, codec }) => { expect(toPlain(meta)).toEqual({ a: 1, c: 3 }); }); + // A map field's key whose value is null (an imported Jupyter notebook's + // metadata can have one) stays when another key of the map changes: the + // patch carries only what changed, so a peer keeps the null key too. + it("leaves an unchanged null-valued map key out of an update's patch", () => { + const base = codec().fromString( + JSON.stringify({ id: 1, meta: { keep: null, a: 1 } }), + ) as unknown as TestDoc; + expect(toPlain(getField(base.getOne({ id: 1 }), "meta"))).toEqual({ keep: null, a: 1 }); + + const next = base.set({ id: 1, meta: { a: 2 } } as JsMap); + expect(toPlain(getField(next.getOne({ id: 1 }), "meta"))).toEqual({ keep: null, a: 2 }); + const patch = base.makePatch(next); + expect(patch).toEqual([1, [{ id: 1, meta: { a: 2 } }]]); + const peer = codec().fromString(base.toString()) as unknown as TestDoc; + const updated = peer.applyPatch(patch); + expect(toPlain(getField(updated.getOne({ id: 1 }), "meta"))).toEqual({ keep: null, a: 2 }); + }); + + it("still sends a removed map key, and a null key that gets a value", () => { + const base = codec().fromString( + JSON.stringify({ id: 1, meta: { keep: null, a: 1 } }), + ) as unknown as TestDoc; + const removed = base.set({ id: 1, meta: { a: null } } as JsMap); + expect(base.makePatch(removed)).toEqual([1, [{ id: 1, meta: { a: null } }]]); + expect( + toPlain(getField(base.applyPatch(base.makePatch(removed)).getOne({ id: 1 }), "meta")), + ).toEqual({ keep: null }); + + const valued = base.set({ id: 1, meta: { keep: 5 } } as JsMap); + expect(base.makePatch(valued)).toEqual([1, [{ id: 1, meta: { keep: 5 } }]]); + }); + if (name === "immer") { it("applies map patches without mutating frozen nested objects", () => { const empty = newDoc(); diff --git a/src/db-util.ts b/src/db-util.ts index 7d4e47d..3b38069 100644 --- a/src/db-util.ts +++ b/src/db-util.ts @@ -27,6 +27,10 @@ export function toStr(objs: JsMap[]): string { return lines.join("\n"); } +// The change that turns map obj1 into obj2: changed and added keys with their +// new values, removed keys as null. A key of obj1 that is unchanged is left +// out, even if its value is null: re-sending it as null would delete it from +// every peer that applies the patch. export function mapMergePatch(obj1: JsMap, obj2: JsMap): JsMap { const change: JsMap = {}; for (const key of Object.keys(obj1)) { @@ -36,7 +40,7 @@ export function mapMergePatch(obj1: JsMap, obj2: JsMap): JsMap { change[key] = val2 == null ? null : val2; } for (const key of Object.keys(obj2)) { - if (obj1[key] != null) continue; + if (Object.prototype.hasOwnProperty.call(obj1, key)) continue; change[key] = obj2[key]; } return change;