From b968197586eeea147a8b112cebacc11b737ef2c1 Mon Sep 17 00:00:00 2001 From: Kevin Date: Wed, 22 Jul 2026 11:21:46 -0400 Subject: [PATCH] PROD-2309: skip content items whose target container mapping is stale MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A container mapping row can outlive the target container it points to (e.g. the container was deleted/soft-deleted on the target since the last sync). getMappedEntity then returns null, but the payload builder silently fell back to the SOURCE reference name, which doesn't resolve to any live container on the target — the server's batch engine dereferences it as null and throws the same opaque "Object reference not set to an instance of an object." Skip the item here instead, with a message that identifies the container (not a content reference) as the cause. Confirmed via the target instance's batch/error tables: all 10 items still failing after the original unresolved-content-reference fix (PR #188) shared this exact cause, independent of field shape — three of them carry no content-reference field at all. Co-Authored-By: Claude Sonnet 5 --- .../content-pusher/content-batch-processor.ts | 19 ++- .../tests/content-batch-processor.test.ts | 109 ++++++++++++++++++ 2 files changed, 127 insertions(+), 1 deletion(-) diff --git a/src/lib/pushers/content-pusher/content-batch-processor.ts b/src/lib/pushers/content-pusher/content-batch-processor.ts index da431522..7ec98d75 100644 --- a/src/lib/pushers/content-pusher/content-batch-processor.ts +++ b/src/lib/pushers/content-pusher/content-batch-processor.ts @@ -377,7 +377,24 @@ export class ContentBatchProcessor { const targetContainer = containerMapper.getMappedEntity(containerMapping, "target"); - // STEP 3.5: Guard against unresolved content references (PROD-2309). + // STEP 3.5a: Guard against a stale container mapping (PROD-2309 follow-up). + // The mapping row can outlive the target container it points to (e.g. the container + // was deleted/soft-deleted on the target since the last successful mapping) — the + // container's local cache file is gone, so getMappedEntity returns null here. + // Previously this fell through and the payload silently reused the SOURCE reference + // name (properties.referenceName fallback below), which doesn't resolve to any live + // container on the target; the server's batch engine then throws the same opaque + // NullReferenceException. Skip here instead, with a reason that makes clear it's the + // container — not a content reference — that's missing. + if (!targetContainer) { + throw new Error( + `Target container for '${contentItem.properties.referenceName}' (target contentViewID ${containerMapping.targetContentViewID}) ` + + `no longer exists on the target — it may have been deleted since the last sync. ` + + `Skipping to avoid a server-side NullReferenceException. Recreate the container or clear its mapping to resume syncing this content.` + ); + } + + // STEP 3.5b: Guard against unresolved content references (PROD-2309). // If a linked/nested content reference has no source→target mapping (e.g. a stale or // incomplete mapping), the field mapper leaves the SOURCE contentID in the payload; the // server's batch engine then dereferences a non-existent item and throws a 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 77a66394..bd84ed8a 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 @@ -507,3 +507,112 @@ describe("ContentBatchProcessor.processBatches — failed item logging", () => { expect(logger.content.created).toHaveBeenCalledTimes(1); }); }); + +// ─── prepareContentPayloads — stale container mapping (PROD-2309 follow-up) ── +// +// The mapping row can outlive the target container it points to (e.g. the container was +// deleted/soft-deleted on the target since the last successful mapping). Exercises the real +// ModelMapper/ContainerMapper file-based lookups (not mocked) so the guard is proven against +// the actual mapping-file shapes these classes read/write. + +function writeMappingFile(root: string, sourceGuid: string, targetGuid: string, type: string, rows: any[]): void { + const dir = path.join(root, "mappings", `${sourceGuid}-${targetGuid}`, type); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, "mappings.json"), JSON.stringify(rows)); +} + +function writeEntityFile(root: string, guid: string, folder: string, id: number, data: any): void { + const dir = path.join(root, guid, folder); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, `${id}.json`), JSON.stringify(data)); +} + +describe("ContentBatchProcessor.prepareContentPayloads — stale container mapping", () => { + function seedModelAndContainerMapping( + root: string, + sourceGuid: string, + targetGuid: string, + opts: { createTargetContainerFile: boolean } + ) { + writeEntityFile(root, sourceGuid, "models", 1, { + id: 1, + referenceName: "TestModel", + fields: [], + }); + writeMappingFile(root, 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(root, 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", + }, + ]); + + if (opts.createTargetContainerFile) { + writeEntityFile(root, targetGuid, "containers", 999, { + contentViewID: 999, + referenceName: "ref-1", + }); + } + // else: mapping row exists but the target container's cache file is absent — + // simulates the container having been deleted on the target since the last sync. + } + + it("skips the item and does not ship a payload when the target container is gone", async () => { + const sourceGuid = "pcp-stale-src"; + const targetGuid = "pcp-stale-tgt"; + seedModelAndContainerMapping(tmpDir, sourceGuid, targetGuid, { createTargetContainerFile: false }); + + const referenceMapper = new ContentItemMapper(sourceGuid, targetGuid, "en-us"); + const processor = new ContentBatchProcessor(makeConfig({ sourceGuid, targetGuid, referenceMapper })); + const item = makeContentItem(1); + item.properties.referenceName = "ref-1"; + item.properties.definitionName = "TestModel"; + + const consoleErrorSpy = jest.spyOn(console, "error"); + const result = await (processor as any).prepareContentPayloads([item], sourceGuid, targetGuid); + + expect(result.payloads).toHaveLength(0); + expect(result.includedItems).toHaveLength(0); + expect(result.skippedCount).toBe(1); + expect(consoleErrorSpy).toHaveBeenCalledWith( + expect.stringContaining("no longer exists on the target") + ); + }); + + it("ships the payload normally when the target container still exists", async () => { + const sourceGuid = "pcp-live-src"; + const targetGuid = "pcp-live-tgt"; + seedModelAndContainerMapping(tmpDir, sourceGuid, targetGuid, { createTargetContainerFile: true }); + + const referenceMapper = new ContentItemMapper(sourceGuid, targetGuid, "en-us"); + const processor = new ContentBatchProcessor(makeConfig({ sourceGuid, targetGuid, referenceMapper })); + const item = makeContentItem(1); + item.properties.referenceName = "ref-1"; + item.properties.definitionName = "TestModel"; + + const result = await (processor as any).prepareContentPayloads([item], sourceGuid, targetGuid); + + expect(result.skippedCount).toBe(0); + expect(result.payloads).toHaveLength(1); + expect(result.includedItems).toHaveLength(1); + expect(result.payloads[0].properties.referenceName).toBe("ref-1"); + }); +});