From 20cd7b12d498a8d74de711b9773b449ee5440542 Mon Sep 17 00:00:00 2001 From: Kevin Date: Mon, 2 Nov 2026 16:53:36 -0500 Subject: [PATCH 1/2] Content sync: send a link to an item deleted from the source as blank When an item links to another that is deleted from the source (and --syncDeletes removes its copy and mapping), the link can never resolve, so the unresolved-reference guard (PROD-2309) blocked the linking item for good: it could not be updated again, even with --overwrite. The v1.3.0 beta tag's live test hit it when a deleted cycle item's partner needed an update. A link to a content ID with no mapping and no file in the source pull (which writes every live item and removes deleted ones) is now blanked, using stripContentReferences, and the item is sent with a warning naming the dropped link. A link to an item that exists but isn't mapped yet still fails, and nothing is treated as deleted when no source content was pulled. The source pull's file list is read once per processor. Co-Authored-By: Claude Opus 5.5 --- .../content-pusher/content-batch-processor.ts | 47 ++++++- .../tests/content-batch-processor.test.ts | 127 ++++++++++++++++-- 2 files changed, 163 insertions(+), 11 deletions(-) diff --git a/src/lib/pushers/content-pusher/content-batch-processor.ts b/src/lib/pushers/content-pusher/content-batch-processor.ts index 6fd2efe..4bf1d47 100644 --- a/src/lib/pushers/content-pusher/content-batch-processor.ts +++ b/src/lib/pushers/content-pusher/content-batch-processor.ts @@ -18,6 +18,7 @@ import { } from "./util/detect-deleted-linked-list-references"; import { fileOperations } from "core/fileOperations"; import { collectUnresolvedContentReferences } from "./util/has-unresolved-content-references"; +import { stripContentReferences } from "./util/strip-content-references"; import { Logs } from "core/logs"; import { state, getFailedContent } from "core/state"; @@ -49,6 +50,7 @@ export class ContentBatchProcessor { // that still exist in the source. Used to tell a *deleted* linked-list reference apart // from a merely "missing" one when clarifying create failures. private liveSourceContainerRefNames?: Set; + private sourceContentFileNames?: Set; constructor(config: ContentBatchConfig) { this.config = { @@ -57,6 +59,23 @@ export class ContentBatchProcessor { }; } + /** + * Whether a source content item no longer exists: no file for it in the source pull, which a pull + * writes for every live item and removes for a deleted one. False when nothing of the source's + * content was pulled for this locale, so a missing pull never reads as "deleted". + */ + private isGoneFromSource(contentID: number): boolean { + if (this.sourceContentFileNames === undefined) { + try { + const sourceFileOps = new fileOperations(this.config.sourceGuid, this.config.locale); + this.sourceContentFileNames = new Set(sourceFileOps.listFilesInFolder("item", ".json")); + } catch { + this.sourceContentFileNames = new Set(); + } + } + return this.sourceContentFileNames.size > 0 && !this.sourceContentFileNames.has(`${contentID}.json`); + } + /** * PROD-2306: Build (and cache) the set of container/linked-list reference names still * present in the source. When a source linked list is deleted, the pull removes its @@ -360,7 +379,7 @@ export class ContentBatchProcessor { const containerMapper = new ContainerMapper(sourceGuid, targetGuid); const assetMapper = new AssetMapper(sourceGuid, targetGuid); - for (const contentItem of contentBatch) { + for (let contentItem of contentBatch) { if (contentItem.properties.definitionName.toLowerCase() === "richtextarea" && contentItem.fields.textblob) { //if this is a RichText item, we don't need to do the extra processing - just upload it as is @@ -460,7 +479,7 @@ export class ContentBatchProcessor { // NullReferenceException, which reaches the operator as an opaque // "Object reference not set to an instance of an object." Skip the item here with a // precise, actionable reason instead of shipping a payload that is guaranteed to fail. - const unresolvedRefs = collectUnresolvedContentReferences( + let unresolvedRefs = collectUnresolvedContentReferences( contentItem.fields || {}, this.config.referenceMapper, "", @@ -468,6 +487,30 @@ export class ContentBatchProcessor { // setting, needed to recognize an unresolved reference living only in a companion field. sourceModel ); + + // A link to an item that is gone from the source (deleted; its copy and mapping removed by + // --syncDeletes) can never resolve, so the guard below would block this item for good. + // The source points at nothing there, so send the item with that link blank, as the source + // has it, and say so. A link to an item that exists but isn't mapped yet still fails. + const goneIDs = Array.from(new Set(unresolvedRefs.map((r) => r.contentID))).filter((id) => + this.isGoneFromSource(id) + ); + if (goneIDs.length > 0) { + const { fields } = stripContentReferences(contentItem.fields || {}, new Set(goneIDs), sourceModel); + contentItem = { ...contentItem, fields }; + logger.warning( + `Content item ${contentItem.contentID} (${contentItem.properties?.referenceName}) links to item(s) ` + + `${goneIDs.join(", ")}, which no longer exist in the source; those links are sent blank.`, + contentItem + ); + unresolvedRefs = collectUnresolvedContentReferences( + contentItem.fields || {}, + this.config.referenceMapper, + "", + sourceModel + ); + } + if (unresolvedRefs.length > 0) { // PROD-2542: when a referenced child failed earlier in this locale (e.g. skipped // client-side for a missing required field), say so — that's the actual cause. diff --git a/src/lib/pushers/content-pusher/tests/content-batch-processor.test.ts b/src/lib/pushers/content-pusher/tests/content-batch-processor.test.ts index b53eefc..25a5783 100644 --- a/src/lib/pushers/content-pusher/tests/content-batch-processor.test.ts +++ b/src/lib/pushers/content-pusher/tests/content-batch-processor.test.ts @@ -600,9 +600,7 @@ describe("ContentBatchProcessor.prepareContentPayloads — stale container mappi expect(result.skippedCount).toBe(0); expect(result.failedItems).toHaveLength(1); expect(result.failedItems[0].error).toEqual(expect.stringContaining("no longer exists on the target")); - expect(consoleErrorSpy).toHaveBeenCalledWith( - expect.stringContaining("no longer exists on the target") - ); + expect(consoleErrorSpy).toHaveBeenCalledWith(expect.stringContaining("no longer exists on the target")); }); it("ships the payload normally when the target container still exists", async () => { @@ -633,12 +631,28 @@ describe("ContentBatchProcessor.prepareContentPayloads — trustSameRunMappings function seed(sourceGuid: string, targetGuid: string): ContentItemMapper { writeEntityFile(tmpDir, sourceGuid, "models", 1, { id: 1, referenceName: "TestModel", fields: [] }); writeMappingFile(tmpDir, sourceGuid, targetGuid, "models", [ - { sourceGuid, targetGuid, sourceID: 1, targetID: 1, sourceReferenceName: "TestModel", targetReferenceName: "TestModel", - sourceLastModifiedDate: "07/01/2026 12:00PM", targetLastModifiedDate: "07/01/2026 12:00PM" }, + { + sourceGuid, + targetGuid, + sourceID: 1, + targetID: 1, + sourceReferenceName: "TestModel", + targetReferenceName: "TestModel", + sourceLastModifiedDate: "07/01/2026 12:00PM", + targetLastModifiedDate: "07/01/2026 12:00PM", + }, ]); writeMappingFile(tmpDir, sourceGuid, targetGuid, "containers", [ - { sourceGuid, targetGuid, sourceContentViewID: 10, targetContentViewID: 999, sourceReferenceName: "ref-1", - targetReferenceName: "ref-1", sourceLastModifiedDate: "07/01/2026 12:00PM", targetLastModifiedDate: "07/01/2026 12:00PM" }, + { + sourceGuid, + targetGuid, + sourceContentViewID: 10, + targetContentViewID: 999, + sourceReferenceName: "ref-1", + targetReferenceName: "ref-1", + sourceLastModifiedDate: "07/01/2026 12:00PM", + targetLastModifiedDate: "07/01/2026 12:00PM", + }, ]); writeEntityFile(tmpDir, targetGuid, "containers", 999, { contentViewID: 999, referenceName: "ref-1" }); const referenceMapper = new ContentItemMapper(sourceGuid, targetGuid, "en-us"); @@ -671,14 +685,109 @@ describe("ContentBatchProcessor.prepareContentPayloads — trustSameRunMappings it("without the option, treats the missing target as gone (unchanged behavior)", async () => { const referenceMapper = seed("notrust-src", "notrust-tgt"); - const processor = new ContentBatchProcessor(makeConfig({ sourceGuid: "notrust-src", targetGuid: "notrust-tgt", referenceMapper })); + const processor = new ContentBatchProcessor( + makeConfig({ sourceGuid: "notrust-src", targetGuid: "notrust-tgt", referenceMapper }) + ); - const result = await (processor as any).prepareContentPayloads([item()], "notrust-src", "notrust-tgt", makeLogger()); + const result = await (processor as any).prepareContentPayloads( + [item()], + "notrust-src", + "notrust-tgt", + makeLogger() + ); expect(result.prepared[0].payload.contentID).toBe(-1); }); }); +// ─── links to items deleted from the source ─────────────────────────────────── + +describe("ContentBatchProcessor.prepareContentPayloads — links to items gone from the source", () => { + function seed(sourceGuid: string, targetGuid: string, sourceItemIds: number[]): ContentItemMapper { + writeEntityFile(tmpDir, sourceGuid, "models", 1, { id: 1, referenceName: "TestModel", fields: [] }); + writeMappingFile(tmpDir, sourceGuid, targetGuid, "models", [ + { + sourceGuid, + targetGuid, + sourceID: 1, + targetID: 1, + sourceReferenceName: "TestModel", + targetReferenceName: "TestModel", + sourceLastModifiedDate: "07/01/2026 12:00PM", + targetLastModifiedDate: "07/01/2026 12:00PM", + }, + ]); + writeMappingFile(tmpDir, sourceGuid, targetGuid, "containers", [ + { + sourceGuid, + targetGuid, + sourceContentViewID: 10, + targetContentViewID: 999, + sourceReferenceName: "ref-1", + targetReferenceName: "ref-1", + sourceLastModifiedDate: "07/01/2026 12:00PM", + targetLastModifiedDate: "07/01/2026 12:00PM", + }, + ]); + writeEntityFile(tmpDir, targetGuid, "containers", 999, { contentViewID: 999, referenceName: "ref-1" }); + // the source pull: one file per live item + for (const id of sourceItemIds) writeEntityFile(tmpDir, sourceGuid, "en-us/item", id, { contentID: id }); + return new ContentItemMapper(sourceGuid, targetGuid, "en-us"); + } + + function linking(to: number): any { + const it = makeContentItem(1); + it.properties.referenceName = "ref-1"; + it.properties.definitionName = "TestModel"; + it.fields = { title: "A", next: { contentid: to, fulllist: false } }; + return it; + } + + const logger = () => ({ ...makeLogger(), warning: jest.fn() }); + + it("sends the item with the link blank, and warns, when the linked item is gone from the source", async () => { + const referenceMapper = seed("gone-src", "gone-tgt", [1]); + const processor = new ContentBatchProcessor( + makeConfig({ sourceGuid: "gone-src", targetGuid: "gone-tgt", referenceMapper }) + ); + const log = logger(); + + const result = await (processor as any).prepareContentPayloads([linking(2)], "gone-src", "gone-tgt", log); + + expect(result.failedItems).toHaveLength(0); + expect(result.prepared).toHaveLength(1); + expect(JSON.stringify(result.prepared[0].payload.fields.next)).not.toContain("2"); + expect(log.warning).toHaveBeenCalledWith( + expect.stringContaining("no longer exist in the source"), + expect.anything() + ); + }); + + it("still fails a link to an item that exists in the source but isn't mapped yet", async () => { + const referenceMapper = seed("live-src", "live-tgt", [1, 2]); + const processor = new ContentBatchProcessor( + makeConfig({ sourceGuid: "live-src", targetGuid: "live-tgt", referenceMapper }) + ); + + const result = await (processor as any).prepareContentPayloads([linking(2)], "live-src", "live-tgt", logger()); + + expect(result.prepared).toHaveLength(0); + expect(result.failedItems[0].error).toMatch(/Unresolved content reference/); + }); + + it("doesn't treat a link as gone when no source content was pulled", async () => { + const referenceMapper = seed("nopull-src", "nopull-tgt", []); + const processor = new ContentBatchProcessor( + makeConfig({ sourceGuid: "nopull-src", targetGuid: "nopull-tgt", referenceMapper }) + ); + + const result = await (processor as any).prepareContentPayloads([linking(2)], "nopull-src", "nopull-tgt", logger()); + + expect(result.prepared).toHaveLength(0); + expect(result.failedItems[0].error).toMatch(/Unresolved content reference/); + }); +}); + // ─── PROD-2542: payload/source alignment ────────────────────────────────────── // // These use the REAL extractContentBatchResults (the module is mocked above for the other tests), so From 647e12057c8b12625f49df1ccdb324e6b472fb68 Mon Sep 17 00:00:00 2001 From: Kevin Date: Mon, 2 Nov 2026 16:53:36 -0500 Subject: [PATCH 2/2] verify: treat a link to a deleted item as no link A link to an item that no longer exists on its side and has no mapping now drops out of the compared reference list, so it equals an empty link (the sync sends such a link blank) as well as the same kind of link on the other side. Before, an empty link on one side and a link to a deleted item on the other were reported as a mismatch. Co-Authored-By: Claude Opus 5.5 --- src/lib/verify/canonicalize.ts | 20 ++++++++++++++++---- src/lib/verify/tests/check-content.test.ts | 7 +++++++ src/lib/verify/verify-context.ts | 9 +-------- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/lib/verify/canonicalize.ts b/src/lib/verify/canonicalize.ts index d811a75..e8894f1 100644 --- a/src/lib/verify/canonicalize.ts +++ b/src/lib/verify/canonicalize.ts @@ -18,6 +18,14 @@ * numbers and numeric strings compare equal, as do booleans and "true"/"false". */ import { getLinkedContentCompanionFields } from "../content/linked-content-companion-fields"; + +/** + * What a context's contentRef returns for a link to an item that no longer exists on its side and + * has no mapping (deleted, e.g. by --syncDeletes along with its copy). Such a link points at nothing, + * so it is dropped, and compares equal to an empty link or to the same kind of link on the other side. + */ +export const DANGLING_CONTENT_REF = "dangling"; +const live = (ref: string) => ref !== DANGLING_CONTENT_REF; import { AssetPrefixPair, stripTrailingSlash } from "./mapping-index"; export type Side = "source" | "target"; @@ -294,12 +302,14 @@ function canonicalLinkedRef(lowered: Record, ctx: CanonicalContext) let ids: string[] = []; const sortids = lowered.sortids; if (typeof sortids === "string" && sortids.trim()) { - ids = parseIdList(sortids).map((id) => ctx.contentRef(id)); + ids = parseIdList(sortids) + .map((id) => ctx.contentRef(id)) + .filter(live); } else if (typeof sortids === "number" && sortids > 0) { - ids = [ctx.contentRef(sortids)]; + ids = [ctx.contentRef(sortids)].filter(live); } else { const contentid = typeof lowered.contentid === "string" ? parseInt(lowered.contentid, 10) : lowered.contentid; - if (typeof contentid === "number" && contentid > 0) ids = [ctx.contentRef(contentid)]; + if (typeof contentid === "number" && contentid > 0) ids = [ctx.contentRef(contentid)].filter(live); } const fullList = lowered.fulllist === true || String(lowered.fulllist).toLowerCase() === "true"; @@ -341,7 +351,9 @@ export function canonicalValue(value: any, ctx: CanonicalContext, fieldType?: st if (typeof value === "string" || typeof value === "number") { const text = String(value); if (type === "content" && ID_LIST.test(text)) { - const ids = parseIdList(text).map((id) => ctx.contentRef(id)); + const ids = parseIdList(text) + .map((id) => ctx.contentRef(id)) + .filter(live); return ids.length > 0 ? { "@ids": ids } : undefined; } if (type === "photogallery" && /^\d+$/.test(text.trim()) && parseInt(text, 10) > 0) { diff --git a/src/lib/verify/tests/check-content.test.ts b/src/lib/verify/tests/check-content.test.ts index 073b8ea..918c34c 100644 --- a/src/lib/verify/tests/check-content.test.ts +++ b/src/lib/verify/tests/check-content.test.ts @@ -84,6 +84,13 @@ describe("checkContent", () => { expect(collector.findings).toEqual([]); }); + it("treats a link to an item deleted from the target as equal to an empty link", () => { + // The sync sent 1 with its link to the deleted item 2 blanked; 101 on the target still points at + // its own deleted partner 102. Both point at nothing. + const collector = run([item(1, { title: "A" })], [item(101, { title: "A", related: "102" })], [[1, 101]]); + expect(collector.findings).toEqual([]); + }); + it("still fails a link to an unmapped item that exists in the source", () => { const collector = run( [item(1, { title: "A", related: { contentid: 2, fulllist: false } }), item(2, { title: "B" })], diff --git a/src/lib/verify/verify-context.ts b/src/lib/verify/verify-context.ts index 5e74580..1fe0f30 100644 --- a/src/lib/verify/verify-context.ts +++ b/src/lib/verify/verify-context.ts @@ -15,7 +15,7 @@ import { GuidDataLoader, GuidEntities, ModelFilterOptions } from "../pushers/gui import { SitemapHierarchy } from "../pushers/page-pusher/sitemap-hierarchy"; import { SitemapNode } from "../../types/syncAnalysis"; import { LocaleMappingIndex, MappingIndex } from "./mapping-index"; -import { AssetUrlTranslator, buildAssetUrlTranslator, CanonicalContext } from "./canonicalize"; +import { AssetUrlTranslator, buildAssetUrlTranslator, CanonicalContext, DANGLING_CONTENT_REF } from "./canonicalize"; export const UNPUBLISHED_STATE = 7; @@ -220,13 +220,6 @@ export function createVerifyContext(input: { }; } -/** - * A content reference to an item that no longer exists on its side and has no mapping, e.g. both - * halves of a pair `--syncDeletes` removed. The same token on both sides, so two such references - * compare equal instead of `unmapped:` against a bare target ID. - */ -const DANGLING_CONTENT_REF = "dangling"; - /** Canonicalization context for SOURCE values: every ID is translated to its target. */ export function sourceCanonicalContext(ctx: VerifyContext): CanonicalContext { return {