diff --git a/ci/permutation/fixtures.mjs b/ci/permutation/fixtures.mjs index c245f1a..d3d214a 100644 --- a/ci/permutation/fixtures.mjs +++ b/ci/permutation/fixtures.mjs @@ -104,6 +104,52 @@ export async function seedReferenceCycles(c, guid, locale) { }; } +// Same rule as the CLI's Auth.determineBaseUrl / determineFetchUrl: the GUID suffix picks the region. +const REGIONS = [ + ["us2", "mgmt-usa2", "api-usa2"], + ["d", "mgmt-dev", "api-dev"], + ["u", "mgmt", "api"], + ["c", "mgmt-ca", "api-ca"], + ["e", "mgmt-eu", "api-eu"], + ["a", "mgmt-aus", "api-aus"], +]; +function hosts(guid) { + const [, mgmt, api] = REGIONS.find(([suffix]) => guid.endsWith(suffix)) ?? ["", "mgmt", "api"]; + return { mgmt: `https://${mgmt}.aglty.io`, api: `https://${api}.aglty.io` }; +} + +// Content published through the Management API reaches the Fetch API's sync feed a minute or two +// later, and the CLI pulls from that feed. Wait until it lists every given item, or the timeout. +// Returns the seconds waited; throws on timeout. +export async function waitForFetchSync(token, guid, locale, ids, { timeoutMs = 10 * 60 * 1000 } = {}) { + const { mgmt, api } = hosts(guid); + const keyRes = await fetch(`${mgmt}/oauth/GetPreviewKey?guid=${guid}`, { + headers: { Authorization: `Bearer ${token}`, "Cache-Control": "no-cache" }, + }); + if (!keyRes.ok) throw new Error(`GetPreviewKey: HTTP ${keyRes.status}`); + const apiKey = (await keyRes.text()).replace(/^"|"$/g, ""); + + const started = Date.now(); + const missing = new Set(ids); + while (Date.now() - started < timeoutMs) { + // Read the whole feed from the start, a page at a time, as a full pull does + let syncToken = 0; + for (let page = 0; page < 100 && missing.size > 0; page++) { + const res = await fetch(`${api}/${guid}/preview/${locale}/sync/items?syncToken=${syncToken}&pageSize=500`, { + headers: { APIKey: apiKey, "Cache-Control": "no-cache" }, + }); + if (!res.ok) throw new Error(`Fetch API sync/items: HTTP ${res.status}`); + const body = await res.json(); + for (const item of body.items ?? []) missing.delete(item.contentID); + if (!body.items?.length || !body.syncToken || body.syncToken === syncToken) break; + syncToken = body.syncToken; + } + if (missing.size === 0) return Math.round((Date.now() - started) / 1000); + await new Promise((r) => setTimeout(r, 10_000)); + } + throw new Error(`items ${[...missing].join(", ")} not in the Fetch API sync feed after ${timeoutMs / 1000}s`); +} + // After a sync: each target copy's Next (and NextID) must hold the TARGET ID of its partner. export async function checkReferenceCycles(c, guid, locale, fixture, toTarget) { const problems = []; diff --git a/ci/permutation/run.mjs b/ci/permutation/run.mjs index bc23f28..10c9421 100644 --- a/ci/permutation/run.mjs +++ b/ci/permutation/run.mjs @@ -27,7 +27,7 @@ import { spawnSync } from "node:child_process"; import { appendFileSync, existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; import { join, resolve } from "node:path"; import { CHANGES, QUICK_ROUNDS, client, conflict, drift, readField, rng } from "./changes.mjs"; -import { checkReferenceCycles, seedReferenceCycles } from "./fixtures.mjs"; +import { checkReferenceCycles, seedReferenceCycles, waitForFetchSync } from "./fixtures.mjs"; const { SOURCE_GUID: A, TARGET_GUID: B, AGILITY_TOKEN, CLI } = process.env; const TIER = process.env.TIER || "quick"; @@ -130,12 +130,19 @@ function syncAndCheck(round, direction) { let cycles = null; try { cycles = await seedReferenceCycles(api, A, LOCALE); + // The first sync pulls from the Fetch API, which lists new items a minute or two after publish + const waited = await waitForFetchSync( + AGILITY_TOKEN, + A, + LOCALE, + cycles.links.map(([x]) => x) + ); record( 0, "A→B", "apply:fixture-cycles", true, - `A ↔ B and a self-reference (${cycles.links.map(([x]) => `#${x}`).join(", ")})` + `A ↔ B and a self-reference (${cycles.links.map(([x]) => `#${x}`).join(", ")}), in the Fetch API after ${waited}s` ); } catch (e) { record(0, "A→B", "apply:fixture-cycles", false, String(e?.message ?? e).slice(0, 200)); diff --git a/src/lib/mappers/content-item-mapper.ts b/src/lib/mappers/content-item-mapper.ts index 6d64815..f7258c0 100644 --- a/src/lib/mappers/content-item-mapper.ts +++ b/src/lib/mappers/content-item-mapper.ts @@ -159,6 +159,18 @@ export class ContentItemMapper { this.saveMapping(); } + /** + * PROD-3076: make the next sync treat this item's source as changed, so it is pushed again. Used + * when an item was created without its references (pass 1 of a reference cycle) and the update + * that fills them in failed. + */ + markSourceChanged(sourceContentID: number): void { + const mapping = this.getContentItemMappingByContentID(sourceContentID, "source"); + if (!mapping) return; + mapping.sourceVersionID = 0; + this.saveMapping(); + } + updateMapping( sourceContentItem: mgmtApi.ContentItem, targetContentItem: mgmtApi.ContentItem, diff --git a/src/lib/pushers/content-pusher/content-batch-processor.ts b/src/lib/pushers/content-pusher/content-batch-processor.ts index 58047e2..6fd2efe 100644 --- a/src/lib/pushers/content-pusher/content-batch-processor.ts +++ b/src/lib/pushers/content-pusher/content-batch-processor.ts @@ -495,7 +495,13 @@ export class ContentBatchProcessor { const isTargetContentItemDeleted = existingMapping && existingTargetContentItem == null; let existingContentID = -1; - if (isTargetContentItemDeleted && state.overwrite) { + // PROD-3076: pass 2 of a reference cycle updates an item pass 1 created moments ago; it is + // not in the pulled target data yet, so without this it would look deleted and be recreated. + const createdThisRun = !!this.config.trustSameRunMappings && !!existingMapping && !existingTargetContentItem; + + if (createdThisRun) { + existingContentID = existingMapping.targetContentID; + } else if (isTargetContentItemDeleted && state.overwrite) { existingContentID = existingMapping ? existingMapping?.targetContentID : -1; } else { existingContentID = existingTargetContentItem ? existingTargetContentItem.contentID : -1; @@ -585,7 +591,9 @@ export class ContentBatchProcessor { // Only an update of a target item known to exist (or a locale version of one found in // another locale) must come back with the same ID. expectedId: - existingTargetContentItem || (!existingMapping && existingContentID > 0) ? existingContentID : undefined, + existingTargetContentItem || createdThisRun || (!existingMapping && existingContentID > 0) + ? existingContentID + : undefined, }); } catch (error: any) { const prepareError = error.message || "payload preparation failed"; diff --git a/src/lib/pushers/content-pusher/content-pusher.ts b/src/lib/pushers/content-pusher/content-pusher.ts index 1c2c076..83b7884 100644 --- a/src/lib/pushers/content-pusher/content-pusher.ts +++ b/src/lib/pushers/content-pusher/content-pusher.ts @@ -10,6 +10,8 @@ import { ContentItem, Model } from "@agility/management-sdk"; import { ContainerMapper } from "lib/mappers/container-mapper"; import { getApiClient } from "core/state"; import { reportForeignCdnReferences } from "./util/report-foreign-cdn-references"; +import { BatchProcessingResult } from "./util/types"; +import { stripContentReferences } from "./util/strip-content-references"; /** * Push content to the target instance @@ -33,13 +35,15 @@ export async function pushContent(sourceData: ContentItem[], targetData: Content return { status: "success" as const, successful: 0, failed: 0, skipped: 0, publishableIds: [], failureDetails: [] }; } - // Deterministically classify content items based on list references (fulllist=true) - const { normalContentItems, linkedContentItems, skippedItems } = getContentItemTypes(contentItems, { - containerMapper, - modelMapper, - referenceMapper, - logger: logger as any, - }); + // Deterministically classify content items by references: linked (referenced, split into + // dependency levels), normal (unreferenced) and skipped (missing mappings) + const { normalContentItems, linkedContentItems, linkedContentLevels, linkedCycleReferences, skippedItems } = + getContentItemTypes(contentItems, { + containerMapper, + modelMapper, + referenceMapper, + logger: logger as any, + }); // PROD-2773: warn about asset URLs into a CDN container owned by neither instance. Reporting // only — runs before change detection (and in --preflight) and never alters an item. Items @@ -99,6 +103,37 @@ export async function pushContent(sourceData: ContentItem[], targetData: Content } }; + // Fold one processBatches result into the totals; register each failure so the next pass's + // parents and the page pusher can say why their reference is unresolved. Items in `heldBack` + // were saved as a pass-1 stub (PROD-3076): pass 2 counts them, and only pass 2's version is + // published, so their successes here are left out. + const recordBatchResult = (result: BatchProcessingResult, heldBack: Set = new Set()): void => { + const heldBackTargetIds = new Set( + (result.successfulItems || []) + .filter((item) => heldBack.has(item.originalContent?.contentID)) + .map((item) => item.newContentId) + ); + totalSuccessful += result.successCount - heldBackTargetIds.size; + totalFailed += result.failureCount; + totalSkipped += result.skippedCount; + allPublishableIds.push(...result.publishableIds.filter((id) => !heldBackTargetIds.has(id))); + for (const item of result.failedItems || []) { + const name = item.originalContent?.properties?.referenceName || "Unknown"; + const contentID = item.originalContent?.contentID; + allFailureDetails.push({ + name, + error: item.error, + type: "content", + contentID, + guid: sourceGuidStr, + locale, + }); + if (contentID) { + registerFailedContent(contentID, name, item.error, locale); + } + } + }; + try { // Import getApiClient for both batch configurations @@ -107,7 +142,8 @@ export async function pushContent(sourceData: ContentItem[], targetData: Content totalSkipped += skippedItems.length; } - // Process linked content items second (with dependencies) + // Process linked content items first, one dependency level at a time (leaves first), so + // the mappings each level references were written by an earlier level (PROD-3025). if (linkedContentItems.length > 0) { const linkedBatchConfig = { apiClient: getApiClient(), @@ -139,37 +175,31 @@ export async function pushContent(sourceData: ContentItem[], targetData: Content totalSuccessful += filteredLinkedContentItems.itemsToProcess.length; totalSkipped += filteredLinkedContentItems.skippedCount; } else { - const linkedBatchProcessor = new ContentBatchProcessor(linkedBatchConfig); - const linkedResult = await linkedBatchProcessor.processBatches( - filteredLinkedContentItems.itemsToProcess.reverse(), - logger, - "Linked Content" - ); + totalSkipped += filteredLinkedContentItems.skippedCount; + const levels = splitIntoLevels(filteredLinkedContentItems.itemsToProcess, linkedContentLevels); + const linkedBatchProcessor = new ContentBatchProcessor(linkedBatchConfig); + const cycleBatchProcessor = new ContentBatchProcessor({ ...linkedBatchConfig, trustSameRunMappings: true }); + for (let i = 0; i < levels.length; i++) { + const label = levels.length > 1 ? `Linked Content (level ${i + 1}/${levels.length})` : "Linked Content"; + + // PROD-3076: items in a reference cycle go out first with their references to not-yet- + // created cycle members blanked, then again in full once every member has a target ID. + const { toPush, stubbed } = stubCycleReferences(levels[i], linkedCycleReferences, referenceMapper, modelMapper); + const result = await linkedBatchProcessor.processBatches(toPush, logger, label); + recordBatchResult(result, new Set(stubbed.map((item) => item.contentID))); - totalSuccessful += linkedResult.successCount; - totalFailed += linkedResult.failureCount; - totalSkipped += filteredLinkedContentItems.skippedCount; - totalSkipped += linkedResult.skippedCount; - allPublishableIds.push(...linkedResult.publishableIds); - // Collect failure details for error summary and register in failed content registry - if (linkedResult.failedItems && linkedResult.failedItems.length > 0) { - linkedResult.failedItems.forEach((item) => { - const name = item.originalContent?.properties?.referenceName || "Unknown"; - const contentID = item.originalContent?.contentID; - allFailureDetails.push({ - name, - error: item.error, - type: "content", - contentID, - guid: sourceGuidStr, - locale, - }); - // Register in global registry so page pusher can provide better error messages - if (contentID) { - registerFailedContent(contentID, name, item.error, locale); + const created = new Set((result.successfulItems || []).map((item) => item.originalContent?.contentID)); + const toComplete = stubbed.filter((item) => created.has(item.contentID)); + if (toComplete.length > 0) { + const completed = await cycleBatchProcessor.processBatches(toComplete, logger, `${label}: completing reference cycles`); + recordBatchResult(completed); + // Saved without its references: make the next sync push it again + for (const failed of completed.failedItems || []) { + const contentID = failed.originalContent?.contentID; + if (contentID) referenceMapper.markSourceChanged(contentID); + } } - }); - } + } } } @@ -202,37 +232,15 @@ export async function pushContent(sourceData: ContentItem[], targetData: Content totalSuccessful += filteredNormalContentItems.itemsToProcess.length; totalSkipped += filteredNormalContentItems.skippedCount; } else { - const normalBatchProcessor = new ContentBatchProcessor(normalBatchConfig); - const normalResult = await normalBatchProcessor.processBatches( - filteredNormalContentItems.itemsToProcess as ContentItem[], - logger, - "Normal Content" - ); + const normalBatchProcessor = new ContentBatchProcessor(normalBatchConfig); + const normalResult = await normalBatchProcessor.processBatches( + filteredNormalContentItems.itemsToProcess as ContentItem[], + logger, + "Normal Content" + ); - totalSuccessful += normalResult.successCount; - totalFailed += normalResult.failureCount; - totalSkipped += filteredNormalContentItems.skippedCount; - totalSkipped += normalResult.skippedCount; - allPublishableIds.push(...normalResult.publishableIds); - // Collect failure details for error summary and register in failed content registry - if (normalResult.failedItems && normalResult.failedItems.length > 0) { - normalResult.failedItems.forEach((item) => { - const name = item.originalContent?.properties?.referenceName || "Unknown"; - const contentID = item.originalContent?.contentID; - allFailureDetails.push({ - name, - error: item.error, - type: "content", - contentID, - guid: sourceGuidStr, - locale, - }); - // Register in global registry so page pusher can provide better error messages - if (contentID) { - registerFailedContent(contentID, name, item.error, locale); - } - }); - } + totalSkipped += filteredNormalContentItems.skippedCount; + recordBatchResult(normalResult); } } @@ -257,3 +265,62 @@ export async function pushContent(sourceData: ContentItem[], targetData: Content }; } } + +/** + * Groups the items left after filtering by their dependency level (PROD-3025), dropping levels + * the filter emptied. An item missing from every level goes last, after everything it could need. + */ +export function splitIntoLevels(items: ContentItem[], levels: ContentItem[][]): ContentItem[][] { + const levelById = new Map(); + levels.forEach((level, i) => level.forEach((item) => levelById.set(item.contentID, i))); + + const grouped: ContentItem[][] = levels.map(() => []); + const unplaced: ContentItem[] = []; + for (const item of items) { + const level = levelById.get(item.contentID); + if (level === undefined) unplaced.push(item); + else grouped[level].push(item); + } + grouped.push(unplaced); + return grouped.filter((level) => level.length > 0); +} + +/** + * PROD-3076: for the items of one level, the copies to push in pass 1. An item in a reference cycle + * whose cycle partners (or itself) have no target ID yet goes out with those references blanked, + * and is returned in `stubbed` for pass 2. Everything else is pushed as-is. Partners that already + * have a mapping (a re-sync) are left alone, so a re-sync never blanks anything. + */ +export function stubCycleReferences( + items: ContentItem[], + cycleReferences: Map, + referenceMapper: ContentItemMapper, + modelMapper: ModelMapper +): { toPush: ContentItem[]; stubbed: ContentItem[] } { + const toPush: ContentItem[] = []; + const stubbed: ContentItem[] = []; + + for (const item of items) { + const unmapped = (cycleReferences.get(item.contentID) || []).filter( + (id) => !referenceMapper.getContentItemMappingByContentID(id, "source") + ); + if (unmapped.length === 0) { + toPush.push(item); + continue; + } + + const definitionName = item.properties?.definitionName?.toLowerCase(); + const modelMapping = definitionName ? modelMapper.getModelMappingByReferenceName(definitionName, "source") : null; + const sourceModel = modelMapping ? modelMapper.getMappedEntity(modelMapping, "source") : null; + const { fields, stripped } = stripContentReferences(item.fields, new Set(unmapped), sourceModel); + if (stripped === 0) { + toPush.push(item); + continue; + } + + toPush.push({ ...item, fields }); + stubbed.push(item); + } + + return { toPush, stubbed }; +} 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 38b26e0..b53eefc 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 @@ -625,6 +625,60 @@ describe("ContentBatchProcessor.prepareContentPayloads — stale container mappi }); }); +// ─── PROD-3076: updating an item created earlier in the same run ───────────── + +describe("ContentBatchProcessor.prepareContentPayloads — trustSameRunMappings (PROD-3076)", () => { + // The content item is mapped (source 1 → target 77) but target 77 is not in the pulled target data, + // which is the state pass 2 of a reference cycle sees: pass 1 created it moments ago. + 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" }, + ]); + 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" }); + const referenceMapper = new ContentItemMapper(sourceGuid, targetGuid, "en-us"); + referenceMapper.addMapping( + { contentID: 1, properties: { versionID: 1 } } as any, + { contentID: 77, properties: { versionID: 5 } } as any + ); + return referenceMapper; + } + + function item(): any { + const it = makeContentItem(1); + it.properties.referenceName = "ref-1"; + it.properties.definitionName = "TestModel"; + return it; + } + + it("updates the mapped target item instead of creating another one", async () => { + const referenceMapper = seed("trust-src", "trust-tgt"); + const processor = new ContentBatchProcessor( + makeConfig({ sourceGuid: "trust-src", targetGuid: "trust-tgt", referenceMapper, trustSameRunMappings: true }) + ); + + const result = await (processor as any).prepareContentPayloads([item()], "trust-src", "trust-tgt", makeLogger()); + + expect(result.prepared).toHaveLength(1); + expect(result.prepared[0].payload.contentID).toBe(77); + expect(result.prepared[0].expectedId).toBe(77); + }); + + 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 result = await (processor as any).prepareContentPayloads([item()], "notrust-src", "notrust-tgt", makeLogger()); + + expect(result.prepared[0].payload.contentID).toBe(-1); + }); +}); + // ─── PROD-2542: payload/source alignment ────────────────────────────────────── // // These use the REAL extractContentBatchResults (the module is mocked above for the other tests), so diff --git a/src/lib/pushers/content-pusher/tests/content-pusher-linked-levels.test.ts b/src/lib/pushers/content-pusher/tests/content-pusher-linked-levels.test.ts new file mode 100644 index 0000000..a3c6b44 --- /dev/null +++ b/src/lib/pushers/content-pusher/tests/content-pusher-linked-levels.test.ts @@ -0,0 +1,348 @@ +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; +import { resetState, setState, state, getFailedContent } from "core/state"; + +// PROD-3025: pushContent pushes the linked tier one dependency level at a time, leaves first. +// Classification, filtering and batching are mocked so only content-pusher's own ordering runs. + +jest.mock("../util/get-content-item-types", () => ({ + getContentItemTypes: jest.fn(), +})); + +jest.mock("../util/filter-content-items-for-processing", () => ({ + filterContentItemsForProcessing: jest.fn(), +})); + +const mockProcessBatches = jest.fn(); +jest.mock("../content-batch-processor", () => ({ + ContentBatchProcessor: jest.fn().mockImplementation(() => ({ processBatches: mockProcessBatches })), +})); + +import { getContentItemTypes } from "../util/get-content-item-types"; +import { filterContentItemsForProcessing } from "../util/filter-content-items-for-processing"; +import { pushContent, splitIntoLevels, stubCycleReferences } from "../content-pusher"; +import { ContentBatchProcessor } from "../content-batch-processor"; +import { ContentItemMapper } from "lib/mappers/content-item-mapper"; + +const mockTypes = getContentItemTypes as jest.Mock; +const mockFilter = filterContentItemsForProcessing as jest.Mock; + +let tmpDir: string; + +beforeAll(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "agility-cpl-")); +}); + +afterAll(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); +}); + +beforeEach(() => { + resetState(); + state.failedContentRegistry.clear(); + setState({ rootPath: tmpDir, sourceGuid: "src-guid", targetGuid: "tgt-guid" }); + const logger = { + content: { error: jest.fn(), created: jest.fn(), skipped: jest.fn() }, + warning: jest.fn(), + getGuid: () => "src-guid", + setGuid: jest.fn(), + }; + state.loggerRegistry = new Map([["src-guid", logger as any]]); + state.cachedApiClient = {} as any; + mockProcessBatches.mockReset(); + mockFilter.mockReset(); + jest.spyOn(console, "log").mockImplementation(() => {}); + jest.spyOn(console, "warn").mockImplementation(() => {}); + jest.spyOn(console, "error").mockImplementation(() => {}); +}); + +afterEach(() => { + jest.restoreAllMocks(); +}); + +function item(id: number): any { + return { contentID: id, properties: { referenceName: `ref-${id}`, definitionName: "Model", state: 2 }, fields: {} }; +} + +function filterResult(itemsToProcess: any[], skippedCount = 0): any { + return { itemsToProcess, itemsToSkip: [], skippedCount, itemsToFail: [] }; +} + +function batchResult(items: any[], failed: any[] = []): any { + return { + successCount: items.length, + failureCount: failed.length, + skippedCount: 0, + successfulItems: [], + failedItems: failed.map((f) => ({ originalContent: f, error: "boom" })), + publishableIds: items.map((i) => i.contentID + 1000), + }; +} + +describe("pushContent — linked dependency levels (PROD-3025)", () => { + it("pushes each level in its own processBatches call, leaves first, then the normal tier", async () => { + const category = item(1); + const post = item(2); + const featured = item(3); + mockTypes.mockReturnValue({ + linkedContentItems: [post, category], + linkedContentLevels: [[category], [post]], linkedCycleReferences: new Map(), + normalContentItems: [featured], + skippedItems: [], + }); + // The filter returns the linked tier in arbitrary order; the levels decide the push order + mockFilter.mockResolvedValueOnce(filterResult([post, category])).mockResolvedValueOnce(filterResult([featured])); + mockProcessBatches.mockImplementation(async (items: any[]) => batchResult(items)); + + const result = await pushContent([featured, post, category], [], "en-us"); + + const calls = mockProcessBatches.mock.calls.map((c) => [c[0].map((i: any) => i.contentID), c[2]]); + expect(calls).toEqual([ + [[1], "Linked Content (level 1/2)"], + [[2], "Linked Content (level 2/2)"], + [[3], "Normal Content"], + ]); + expect(result.status).toBe("success"); + expect(result.successful).toBe(3); + expect(result.publishableIds).toEqual([1001, 1002, 1003]); + }); + + it("adds up failures across levels and registers each one", async () => { + const leaf = item(1); + const mid = item(2); + mockTypes.mockReturnValue({ + linkedContentItems: [leaf, mid], + linkedContentLevels: [[leaf], [mid]], linkedCycleReferences: new Map(), + normalContentItems: [], + skippedItems: [], + }); + mockFilter.mockResolvedValueOnce(filterResult([leaf, mid], 2)); + mockProcessBatches + .mockResolvedValueOnce(batchResult([leaf])) + .mockResolvedValueOnce(batchResult([], [mid])); + + const result = await pushContent([leaf, mid], [], "en-us"); + + expect(result.status).toBe("error"); + expect(result.successful).toBe(1); + expect(result.failed).toBe(1); + expect(result.skipped).toBe(2); + expect(result.failureDetails).toEqual([expect.objectContaining({ contentID: 2, error: "boom" })]); + expect(getFailedContent(2, "en-us")).toBeDefined(); + }); + + it("does not call the batch processor for a level the filter emptied", async () => { + const leaf = item(1); + const mid = item(2); + mockTypes.mockReturnValue({ + linkedContentItems: [leaf, mid], + linkedContentLevels: [[leaf], [mid]], linkedCycleReferences: new Map(), + normalContentItems: [], + skippedItems: [], + }); + // leaf is unchanged on the target, so only mid is left to push + mockFilter.mockResolvedValueOnce(filterResult([mid], 1)); + mockProcessBatches.mockImplementation(async (items: any[]) => batchResult(items)); + + await pushContent([leaf, mid], [], "en-us"); + + expect(mockProcessBatches).toHaveBeenCalledTimes(1); + expect(mockProcessBatches.mock.calls[0][0]).toEqual([mid]); + expect(mockProcessBatches.mock.calls[0][2]).toBe("Linked Content"); + }); + + it("writes nothing in preflight", async () => { + setState({ preflight: true }); + const leaf = item(1); + const mid = item(2); + mockTypes.mockReturnValue({ + linkedContentItems: [leaf, mid], + linkedContentLevels: [[leaf], [mid]], linkedCycleReferences: new Map(), + normalContentItems: [], + skippedItems: [], + }); + mockFilter.mockResolvedValueOnce(filterResult([leaf, mid])); + + const result = await pushContent([leaf, mid], [], "en-us"); + + expect(mockProcessBatches).not.toHaveBeenCalled(); + expect(result.successful).toBe(2); + }); +}); + +describe("splitIntoLevels", () => { + it("groups items by level and drops empty levels", () => { + const a = item(1); + const b = item(2); + const c = item(3); + expect(splitIntoLevels([c, a], [[a], [b], [c]])).toEqual([[a], [c]]); + }); + + it("puts an item missing from every level last", () => { + const a = item(1); + const stray = item(9); + expect(splitIntoLevels([stray, a], [[a]])).toEqual([[a], [stray]]); + }); +}); + +// ─── reference cycles: two passes in the same run (PROD-3076) ──────────────── + +describe("pushContent — reference cycles (PROD-3076)", () => { + // A and B reference each other through a single-item linked field + function cycle(): { a: any; b: any } { + const a = item(1); + const b = item(2); + a.fields = { next: { contentid: 2, fulllist: false } }; + b.fields = { next: { contentid: 1, fulllist: false } }; + return { a, b }; + } + + function classify(a: any, b: any): void { + mockTypes.mockReturnValue({ + linkedContentItems: [a, b], + linkedContentLevels: [[a, b]], + linkedCycleReferences: new Map([ + [1, [2]], + [2, [1]], + ]), + normalContentItems: [], + skippedItems: [], + }); + mockFilter.mockResolvedValueOnce(filterResult([a, b])); + } + + // Every item sent succeeds; target ID = source ID + 1000 + function succeedAll(failIds: number[] = []): void { + mockProcessBatches.mockImplementation(async (items: any[]) => { + const ok = items.filter((i) => !failIds.includes(i.contentID)); + const bad = items.filter((i) => failIds.includes(i.contentID)); + return { + successCount: ok.length, + failureCount: bad.length, + skippedCount: 0, + successfulItems: ok.map((i) => ({ originalContent: i, newItem: {}, newContentId: i.contentID + 1000 })), + failedItems: bad.map((i) => ({ originalContent: i, error: "boom" })), + publishableIds: ok.map((i) => i.contentID + 1000), + }; + }); + } + + it("creates the members with the cycle references blanked, then sends them in full", async () => { + const { a, b } = cycle(); + classify(a, b); + succeedAll(); + + await pushContent([a, b], [], "en-us"); + + const [pass1, pass2] = mockProcessBatches.mock.calls; + expect(pass1[0].map((i: any) => i.fields.next.contentid)).toEqual([-1, -1]); + expect(pass1[2]).toBe("Linked Content"); + expect(pass2[0]).toEqual([a, b]); + expect(pass2[2]).toBe("Linked Content: completing reference cycles"); + // Pass 2 runs through a processor that updates the targets pass 1 just created + const configs = (ContentBatchProcessor as unknown as jest.Mock).mock.calls.map((c) => c[0]); + expect(configs.some((c) => c.trustSameRunMappings === true)).toBe(true); + // The source items themselves are never modified + expect(a.fields.next.contentid).toBe(2); + }); + + it("counts and publishes each member once", async () => { + const { a, b } = cycle(); + classify(a, b); + succeedAll(); + + const result = await pushContent([a, b], [], "en-us"); + + expect(result.successful).toBe(2); + expect(result.failed).toBe(0); + expect(result.publishableIds).toEqual([1001, 1002]); + }); + + it("reports a failed second pass and makes the next sync push the item again", async () => { + const { a, b } = cycle(); + classify(a, b); + let call = 0; + mockProcessBatches.mockImplementation(async (items: any[]) => { + call++; + const fail = call === 2 ? [2] : []; + const ok = items.filter((i) => !fail.includes(i.contentID)); + return { + successCount: ok.length, + failureCount: items.length - ok.length, + skippedCount: 0, + successfulItems: ok.map((i) => ({ originalContent: i, newItem: {}, newContentId: i.contentID + 1000 })), + failedItems: items.filter((i) => fail.includes(i.contentID)).map((i) => ({ originalContent: i, error: "boom" })), + publishableIds: ok.map((i) => i.contentID + 1000), + }; + }); + const markSpy = jest.spyOn(ContentItemMapper.prototype, "markSourceChanged"); + + const result = await pushContent([a, b], [], "en-us"); + + expect(result.successful).toBe(1); + expect(result.failed).toBe(1); + // The half-saved stub of B is not published + expect(result.publishableIds).toEqual([1001]); + expect(markSpy).toHaveBeenCalledWith(2); + }); + + it("does not run a second pass for a member whose stub failed", async () => { + const { a, b } = cycle(); + classify(a, b); + succeedAll([2]); + + const result = await pushContent([a, b], [], "en-us"); + + expect(mockProcessBatches.mock.calls[1][0]).toEqual([a]); + expect(result.successful).toBe(1); + expect(result.failed).toBe(1); + }); + + it("sends a cycle whose members are already mapped (a re-sync) once, unchanged", async () => { + const { a, b } = cycle(); + classify(a, b); + succeedAll(); + jest.spyOn(ContentItemMapper.prototype, "getContentItemMappingByContentID").mockReturnValue({} as any); + + await pushContent([a, b], [], "en-us"); + + expect(mockProcessBatches).toHaveBeenCalledTimes(1); + expect(mockProcessBatches.mock.calls[0][0]).toEqual([a, b]); + }); +}); + +describe("stubCycleReferences (PROD-3076)", () => { + const mapper = (mapped: number[]): any => ({ + getContentItemMappingByContentID: (id: number) => (mapped.includes(id) ? {} : null), + }); + const modelMapper: any = { getModelMappingByReferenceName: () => null, getMappedEntity: () => null }; + + it("blanks only the references to unmapped members", () => { + const a = item(1); + a.fields = { next: { contentid: 2 }, other: { contentid: 3 }, picks: { sortids: "2,3" } }; + const { toPush, stubbed } = stubCycleReferences([a], new Map([[1, [2, 3]]]), mapper([3]), modelMapper); + + expect(toPush[0].fields).toEqual({ next: { contentid: -1 }, other: { contentid: 3 }, picks: { sortids: "3" } }); + expect(stubbed).toEqual([a]); + }); + + it("passes items outside a cycle through untouched", () => { + const a = item(1); + a.fields = { next: { contentid: 2 } }; + const { toPush, stubbed } = stubCycleReferences([a], new Map(), mapper([]), modelMapper); + + expect(toPush).toEqual([a]); + expect(toPush[0]).toBe(a); + expect(stubbed).toEqual([]); + }); + + it("stubs a self-reference", () => { + const c = item(3); + c.fields = { next: { contentid: 3 } }; + const { toPush, stubbed } = stubCycleReferences([c], new Map([[3, [3]]]), mapper([]), modelMapper); + + expect(toPush[0].fields.next.contentid).toBe(-1); + expect(stubbed).toEqual([c]); + }); +}); diff --git a/src/lib/pushers/content-pusher/tests/content-pusher-validation.test.ts b/src/lib/pushers/content-pusher/tests/content-pusher-validation.test.ts index f76f814..92ef2a6 100644 --- a/src/lib/pushers/content-pusher/tests/content-pusher-validation.test.ts +++ b/src/lib/pushers/content-pusher/tests/content-pusher-validation.test.ts @@ -99,20 +99,14 @@ describe("pushContent — client-side validation failures (PROD-2542)", () => { const normalGood = item(2); mockTypes.mockReturnValue({ linkedContentItems: [linkedBad], + linkedContentLevels: [[linkedBad]], linkedCycleReferences: new Map(), normalContentItems: [normalBad, normalGood], skippedItems: [], }); mockFilter .mockResolvedValueOnce(filterResult([], [invalid(linkedBad)])) .mockResolvedValueOnce(filterResult([normalGood], [invalid(normalBad)])); - mockProcessBatches.mockResolvedValueOnce({ - successCount: 0, - failureCount: 0, - skippedCount: 0, - successfulItems: [], - failedItems: [], - publishableIds: [], - }); + // The linked tier is empty after filtering, so only the normal pass reaches the batch processor. mockProcessBatches.mockResolvedValueOnce({ successCount: 1, failureCount: 0, @@ -157,7 +151,7 @@ describe("pushContent — client-side validation failures (PROD-2542)", () => { it("registers a failure (locale-scoped) before the next pass runs", async () => { const linkedBad = item(10, "linked-bad"); const parent = item(20, "parent"); - mockTypes.mockReturnValue({ linkedContentItems: [linkedBad], normalContentItems: [parent], skippedItems: [] }); + mockTypes.mockReturnValue({ linkedContentItems: [linkedBad], linkedContentLevels: [[linkedBad]], linkedCycleReferences: new Map(), normalContentItems: [parent], skippedItems: [] }); let seenDuringNormalPass: any; mockFilter.mockResolvedValueOnce(filterResult([], [invalid(linkedBad)])).mockImplementationOnce(async () => { seenDuringNormalPass = getFailedContent(10, "en-us"); @@ -180,7 +174,7 @@ describe("pushContent — client-side validation failures (PROD-2542)", () => { setState({ preflight: true }); const bad = item(12942); const good = item(2); - mockTypes.mockReturnValue({ linkedContentItems: [], normalContentItems: [bad, good], skippedItems: [] }); + mockTypes.mockReturnValue({ linkedContentItems: [], linkedContentLevels: [], linkedCycleReferences: new Map(), normalContentItems: [bad, good], skippedItems: [] }); mockFilter.mockResolvedValueOnce(filterResult([good], [invalid(bad)])); const result = await pushContent([bad, good], [], "en-us"); diff --git a/src/lib/pushers/content-pusher/tests/content-pusher.test.ts b/src/lib/pushers/content-pusher/tests/content-pusher.test.ts index 33ad15b..7d881b9 100644 --- a/src/lib/pushers/content-pusher/tests/content-pusher.test.ts +++ b/src/lib/pushers/content-pusher/tests/content-pusher.test.ts @@ -125,6 +125,7 @@ describe("pushContent — batch processing error handling", () => { getContentItemTypes: jest.fn().mockReturnValue({ normalContentItems: [makeContentItem(1)], linkedContentItems: [], + linkedContentLevels: [], linkedCycleReferences: new Map(), skippedItems: [], }), })); diff --git a/src/lib/pushers/content-pusher/util/get-content-item-types.ts b/src/lib/pushers/content-pusher/util/get-content-item-types.ts index 37fc1a8..c603ff9 100644 --- a/src/lib/pushers/content-pusher/util/get-content-item-types.ts +++ b/src/lib/pushers/content-pusher/util/get-content-item-types.ts @@ -14,6 +14,14 @@ import { collectContentIDReferences } from "./collect-content-id-references"; * reference (fullList=true) or a single-item reference (contentid/sortids). Linked * items are pushed FIRST so their source→target contentID mapping exists before the * referencing item's fields are remapped (PROD-2341). + * Linked levels: the linked items split into dependency levels, leaves first (PROD-3025). + * Level 0 references no other linked item; level n references something at level n-1. + * Each level is pushed as its own batch, so every mapping a level needs already exists. + * Items in a reference cycle share one level (their references to each other can't resolve). + * Linked cycle references: for each linked item in a reference cycle (or that references itself), + * its by-ID references into that cycle (PROD-3076). The pusher creates the item with those + * references blanked, then fills them in once every member has a target ID. Whole-list + * references resolve by container name, so they never block and are not included. * Skipped items: Items without valid container/model mappings */ export function getContentItemTypes( @@ -27,6 +35,8 @@ export function getContentItemTypes( ): { normalContentItems: ContentItem[]; linkedContentItems: ContentItem[]; + linkedContentLevels: ContentItem[][]; + linkedCycleReferences: Map; skippedItems: ContentItem[]; } { const { containerMapper, modelMapper } = opts; @@ -71,7 +81,14 @@ export function getContentItemTypes( // Build final result arrays const { normalContentItems, linkedContentItems } = buildResultArrays(normalSet, linkedSet, allItemsById); - return { normalContentItems, linkedContentItems, skippedItems: skipped }; + const { levels: linkedContentLevels, cycleReferences: linkedCycleReferences } = buildLinkedLevels( + linkedSet, + itemsByReferenceName, + allItemsById, + modelMapper + ); + + return { normalContentItems, linkedContentItems, linkedContentLevels, linkedCycleReferences, skippedItems: skipped }; } /** @@ -127,17 +144,24 @@ function collectReferencedContentIDs( } } - // Single-item references → the specific referenced item, if it is in this push set - const sourceModel = getSourceModel(item, modelMapper); - for (const contentID of collectContentIDReferences(item.fields || {}, sourceModel)) { - if (allItemsById.has(contentID)) { - ids.push(contentID); - } - } - + ids.push(...collectIdReferences(item, allItemsById, modelMapper)); return ids; } +/** + * The single-item (by contentID) references of an item that point at another item in this push + * set. Unlike a whole-list reference, each needs the referenced item's target ID before the item + * can be saved. + */ +function collectIdReferences( + item: ContentItem, + allItemsById: Map, + modelMapper: ModelMapper +): number[] { + const sourceModel = getSourceModel(item, modelMapper); + return collectContentIDReferences(item.fields || {}, sourceModel).filter((contentID) => allItemsById.has(contentID)); +} + /** * Resolves an item's own source model (by its definitionName), so a Content-typed field's * companion-field name (LinkeContentDropdownValueField/SortIDFieldName) can be read off it. @@ -194,6 +218,110 @@ function markReferencedItems( } } +/** + * Splits the linked items into dependency levels, leaves first (PROD-3025). + * + * Only references between linked items count: a reference to anything else is either already + * mapped or can't be resolved by ordering. Self-references are ignored. Cycles are collapsed + * with Tarjan's strongly-connected-components algorithm, so the levels come from a DAG and work + * at any depth. Tarjan emits a component only after every component it reaches, so each one's + * level is known from its dependencies' levels as it is emitted. Iterative, so a long chain + * can't overflow the call stack. Also returns each cycle member's by-ID references into its cycle. + */ +function buildLinkedLevels( + linkedSet: Set, + itemsByReferenceName: Map, + allItemsById: Map, + modelMapper: ModelMapper +): { levels: ContentItem[][]; cycleReferences: Map } { + const linkedIds = Array.from(linkedSet); + const cycleReferences = new Map(); + const depsById = new Map(); + for (const id of linkedIds) { + const deps = collectReferencedContentIDs(allItemsById.get(id)!, itemsByReferenceName, allItemsById, modelMapper); + depsById.set(id, Array.from(new Set(deps)).filter((dep) => dep !== id && linkedSet.has(dep))); + } + + const indexOf = new Map(); + const lowLink = new Map(); + const onStack = new Set(); + const componentStack: number[] = []; + const levelOf = new Map(); + let nextIndex = 0; + + const visit = (id: number) => { + indexOf.set(id, nextIndex); + lowLink.set(id, nextIndex); + nextIndex++; + componentStack.push(id); + onStack.add(id); + }; + + for (const root of linkedIds) { + if (indexOf.has(root)) continue; + visit(root); + const work: Array<{ id: number; next: number }> = [{ id: root, next: 0 }]; + + while (work.length > 0) { + const frame = work[work.length - 1]; + const deps = depsById.get(frame.id)!; + + if (frame.next < deps.length) { + const dep = deps[frame.next++]; + if (!indexOf.has(dep)) { + visit(dep); + work.push({ id: dep, next: 0 }); + } else if (onStack.has(dep)) { + lowLink.set(frame.id, Math.min(lowLink.get(frame.id)!, indexOf.get(dep)!)); + } + continue; + } + + work.pop(); + if (work.length > 0) { + const parent = work[work.length - 1]; + lowLink.set(parent.id, Math.min(lowLink.get(parent.id)!, lowLink.get(frame.id)!)); + } + if (lowLink.get(frame.id) !== indexOf.get(frame.id)) continue; + + // frame.id roots a component: pop its members, then place it one level above its deepest dependency + const members: number[] = []; + let member: number; + do { + member = componentStack.pop()!; + onStack.delete(member); + members.push(member); + } while (member !== frame.id); + + const memberSet = new Set(members); + let level = 0; + for (const id of members) { + for (const dep of depsById.get(id)!) { + if (!memberSet.has(dep)) level = Math.max(level, levelOf.get(dep)! + 1); + } + } + for (const id of members) levelOf.set(id, level); + + // PROD-3076: by-ID references inside the component (a self-reference counts) can't be + // resolved by ordering - the pusher fills them in with a second pass. + for (const id of members) { + const intoCycle = collectIdReferences(allItemsById.get(id)!, allItemsById, modelMapper).filter((dep) => + memberSet.has(dep) + ); + if (intoCycle.length > 0) cycleReferences.set(id, Array.from(new Set(intoCycle))); + } + } + } + + // Group in linkedSet order so the output is deterministic + const levels: ContentItem[][] = []; + for (const id of linkedIds) { + const level = levelOf.get(id)!; + (levels[level] ||= []).push(allItemsById.get(id)!); + } + return { levels: levels.filter(Boolean), cycleReferences }; +} + /** * Builds final arrays from ID sets, using the allItemsById map for lookup */ diff --git a/src/lib/pushers/content-pusher/util/strip-content-references.ts b/src/lib/pushers/content-pusher/util/strip-content-references.ts new file mode 100644 index 0000000..b009f43 --- /dev/null +++ b/src/lib/pushers/content-pusher/util/strip-content-references.ts @@ -0,0 +1,58 @@ +import { getLinkedContentCompanionFields, findFieldKey } from "lib/content/linked-content-companion-fields"; + +/** + * PROD-3076: copy of an item's fields with its references to the given source contentIDs blanked, + * so the item can be created before the items it references exist (pass 1 of a reference cycle). + * + * Covers the same reference shapes as collectContentIDReferences: + * - `contentid` / `contentID` set to -1 ("no reference selected", which the field mapper and the + * PROD-2309 guard both treat as empty; the server stores the field empty, even a required one); + * - each ID removed from a comma-separated `sortids`; + * - each ID removed from a companion field (the model's LinkeContentDropdownValueField / + * SortIDFieldName, PROD-2446), a top-level sibling of the main field. + * + * Returns the new fields and how many references were blanked. The input is never modified. + */ +export function stripContentReferences( + fields: any, + ids: Set, + model?: any +): { fields: any; stripped: number } { + let stripped = 0; + + const withoutIds = (list: string): string => { + const parts = list.split(",").map((p) => p.trim()); + const kept = parts.filter((p) => !ids.has(parseInt(p))); + stripped += parts.length - kept.length; + return kept.join(","); + }; + + function walk(node: any): any { + if (Array.isArray(node)) return node.map(walk); + if (!node || typeof node !== "object") return node; + + const out: any = {}; + for (const [key, value] of Object.entries(node)) { + if ((key === "contentid" || key === "contentID") && typeof value === "number" && ids.has(value)) { + out[key] = -1; + stripped++; + } else if (key === "sortids" && typeof value === "string") { + out[key] = withoutIds(value); + } else { + out[key] = walk(value); + } + } + return out; + } + + const result = walk(fields || {}); + + for (const [, companionFieldName] of getLinkedContentCompanionFields(model)) { + const companionKey = findFieldKey(result, companionFieldName); + if (companionKey && typeof result[companionKey] === "string" && result[companionKey].trim()) { + result[companionKey] = withoutIds(result[companionKey]); + } + } + + return { fields: result, stripped }; +} diff --git a/src/lib/pushers/content-pusher/util/tests/get-content-item-types.test.ts b/src/lib/pushers/content-pusher/util/tests/get-content-item-types.test.ts index 7bd2c80..ffc3955 100644 --- a/src/lib/pushers/content-pusher/util/tests/get-content-item-types.test.ts +++ b/src/lib/pushers/content-pusher/util/tests/get-content-item-types.test.ts @@ -375,3 +375,179 @@ describe("getContentItemTypes — recursive references", () => { expect(() => getContentItemTypes([itemA, itemB], makeValidOpts())).not.toThrow(); }); }); + +// ─── dependency levels (PROD-3025) ─────────────────────────────────────────── + +describe("getContentItemTypes — linked dependency levels (PROD-3025)", () => { + const idsByLevel = (levels: any[][]) => levels.map((level) => level.map((i) => i.contentID)); + + it("splits a three-deep chain so the leaf is pushed before the item that references it", () => { + // FeaturedPost → Post → Category: Post and Category are both linked, but Category must go first + const category = makeItem("categories", "Category"); + const post = makeItem("posts", "Post", { category: { contentid: category.contentID } }); + const featured = makeItem("featuredposts", "FeaturedPost", { post: { contentid: post.contentID } }); + + const result = getContentItemTypes([featured, post, category], makeValidOpts()); + + expect(result.normalContentItems).toEqual([featured]); + expect(idsByLevel(result.linkedContentLevels)).toEqual([[category.contentID], [post.contentID]]); + }); + + it("gives every item of a 50-deep chain its own level, leaves first", () => { + const chain: any[] = [makeItem("item-0", "Model")]; + for (let i = 1; i < 50; i++) { + chain.push(makeItem(`item-${i}`, "Model", { next: { contentid: chain[i - 1].contentID } })); + } + + const result = getContentItemTypes([...chain].reverse(), makeValidOpts()); + + // The last item is unreferenced (normal); the other 49 form 49 levels in chain order + expect(result.normalContentItems).toEqual([chain[49]]); + expect(idsByLevel(result.linkedContentLevels)).toEqual(chain.slice(0, 49).map((i) => [i.contentID])); + }); + + it("places an item one level above its deepest dependency when depths differ", () => { + const leaf = makeItem("leaf", "Model"); + const mid = makeItem("mid", "Model", { leaf: { contentid: leaf.contentID } }); + // uneven depends on mid (level 1) and directly on leaf (level 0) → level 2 + const uneven = makeItem("uneven", "Model", { + mid: { contentid: mid.contentID }, + leaf: { contentid: leaf.contentID }, + }); + const top = makeItem("top", "Model", { uneven: { contentid: uneven.contentID } }); + + const result = getContentItemTypes([top, uneven, mid, leaf], makeValidOpts()); + + expect(idsByLevel(result.linkedContentLevels)).toEqual([[leaf.contentID], [mid.contentID], [uneven.contentID]]); + }); + + it("puts both sides of a diamond on the same level, above their shared dependency", () => { + const shared = makeItem("shared", "Model"); + const left = makeItem("left", "Model", { shared: { contentid: shared.contentID } }); + const right = makeItem("right", "Model", { shared: { contentid: shared.contentID } }); + const top = makeItem("top", "Model", { picks: { sortids: `${left.contentID},${right.contentID}` } }); + + const result = getContentItemTypes([top, left, right, shared], makeValidOpts()); + + expect(result.linkedContentLevels).toHaveLength(2); + expect(idsByLevel(result.linkedContentLevels)[0]).toEqual([shared.contentID]); + expect(idsByLevel(result.linkedContentLevels)[1].sort()).toEqual([left.contentID, right.contentID].sort()); + }); + + it("orders whole-list references the same way", () => { + const tag = makeItem("tags", "Tag"); + const post = makeItem("posts", "Post", { tags: { referencename: "tags", fulllist: true } }); + const page = makeItem("listing", "Listing", { posts: { referencename: "posts", fulllist: true } }); + + const result = getContentItemTypes([page, post, tag], makeValidOpts()); + + expect(idsByLevel(result.linkedContentLevels)).toEqual([[tag.contentID], [post.contentID]]); + }); + + it("keeps a cycle on one level, after what it depends on, and the item above it on the next", () => { + const leaf = makeItem("leaf", "Model"); + const a = makeItem("a", "Model", { b: { contentid: 0 }, leaf: { contentid: leaf.contentID } }); + const b = makeItem("b", "Model", { a: { contentid: a.contentID } }); + a.fields.b.contentid = b.contentID; + const top = makeItem("top", "Model", { a: { contentid: a.contentID } }); + + const result = getContentItemTypes([top, a, b, leaf], makeValidOpts()); + + expect(result.linkedContentLevels).toHaveLength(2); + expect(idsByLevel(result.linkedContentLevels)[0]).toEqual([leaf.contentID]); + expect(idsByLevel(result.linkedContentLevels)[1].sort()).toEqual([a.contentID, b.contentID].sort()); + }); + + it("ignores a self-reference when placing an item", () => { + const child = makeItem("child", "Model"); + const self = makeItem("self", "Model", { child: { contentid: child.contentID } }); + self.fields.me = { contentid: self.contentID }; + const parent = makeItem("parent", "Model", { self: { contentid: self.contentID } }); + + const result = getContentItemTypes([parent, self, child], makeValidOpts()); + + expect(idsByLevel(result.linkedContentLevels)).toEqual([[child.contentID], [self.contentID]]); + }); + + it("covers exactly the linked items", () => { + const category = makeItem("categories", "Category"); + const post = makeItem("posts", "Post", { category: { contentid: category.contentID } }); + const outside = makeItem("outside", "Model", { missing: { contentid: 9999 } }); + const featured = makeItem("featuredposts", "FeaturedPost", { post: { contentid: post.contentID } }); + + const result = getContentItemTypes([featured, post, category, outside], makeValidOpts()); + + const flattened = result.linkedContentLevels.flat().map((i) => i.contentID).sort(); + expect(flattened).toEqual(result.linkedContentItems.map((i) => i.contentID).sort()); + }); +}); + +// ─── reference cycles (PROD-3076) ──────────────────────────────────────────── + +describe("getContentItemTypes — linked cycle references (PROD-3076)", () => { + it("reports both sides of a two-item cycle with the reference into it", () => { + const a = makeItem("nodes", "Node"); + const b = makeItem("nodes", "Node", { next: { contentid: a.contentID } }); + a.fields = { next: { contentid: b.contentID } }; + const top = makeItem("top", "Model", { node: { contentid: a.contentID } }); + + const { linkedCycleReferences } = getContentItemTypes([top, a, b], makeValidOpts()); + + expect(linkedCycleReferences.get(a.contentID)).toEqual([b.contentID]); + expect(linkedCycleReferences.get(b.contentID)).toEqual([a.contentID]); + expect(linkedCycleReferences.has(top.contentID)).toBe(false); + }); + + it("reports a self-reference", () => { + const self = makeItem("nodes", "Node"); + self.fields = { next: { contentid: self.contentID } }; + + const { linkedCycleReferences } = getContentItemTypes([self], makeValidOpts()); + + expect(linkedCycleReferences.get(self.contentID)).toEqual([self.contentID]); + }); + + it("covers a three-item cycle and not the leaf it depends on", () => { + const leaf = makeItem("leaf", "Model"); + const a = makeItem("a", "Model"); + const b = makeItem("b", "Model"); + const c = makeItem("c", "Model"); + a.fields = { next: { contentid: b.contentID }, leaf: { contentid: leaf.contentID } }; + b.fields = { next: { contentid: c.contentID } }; + c.fields = { next: { contentid: a.contentID } }; + + const { linkedCycleReferences } = getContentItemTypes([a, b, c, leaf], makeValidOpts()); + + expect(linkedCycleReferences.get(a.contentID)).toEqual([b.contentID]); + expect(linkedCycleReferences.get(b.contentID)).toEqual([c.contentID]); + expect(linkedCycleReferences.get(c.contentID)).toEqual([a.contentID]); + expect(linkedCycleReferences.has(leaf.contentID)).toBe(false); + }); + + it("ignores a cycle made only of whole-list references, which never block", () => { + const a = makeItem("list-a", "ModelA", { other: { referencename: "list-b", fulllist: true } }); + const b = makeItem("list-b", "ModelB", { other: { referencename: "list-a", fulllist: true } }); + + const { linkedCycleReferences } = getContentItemTypes([a, b], makeValidOpts()); + + expect(linkedCycleReferences.size).toBe(0); + }); + + it("reports only the by-ID side of a cycle closed by a whole-list reference", () => { + const b = makeItem("list-b", "ModelB", { back: { referencename: "list-a", fulllist: true } }); + const a = makeItem("list-a", "ModelA", { pick: { contentid: b.contentID } }); + + const { linkedCycleReferences } = getContentItemTypes([a, b], makeValidOpts()); + + expect(linkedCycleReferences.get(a.contentID)).toEqual([b.contentID]); + expect(linkedCycleReferences.has(b.contentID)).toBe(false); + }); + + it("reports nothing for a plain chain", () => { + const leaf = makeItem("leaf", "Model"); + const mid = makeItem("mid", "Model", { leaf: { contentid: leaf.contentID } }); + const top = makeItem("top", "Model", { mid: { contentid: mid.contentID } }); + + expect(getContentItemTypes([top, mid, leaf], makeValidOpts()).linkedCycleReferences.size).toBe(0); + }); +}); diff --git a/src/lib/pushers/content-pusher/util/tests/strip-content-references.test.ts b/src/lib/pushers/content-pusher/util/tests/strip-content-references.test.ts new file mode 100644 index 0000000..999d035 --- /dev/null +++ b/src/lib/pushers/content-pusher/util/tests/strip-content-references.test.ts @@ -0,0 +1,52 @@ +import { stripContentReferences } from "../strip-content-references"; + +// PROD-3076: pass 1 of a reference cycle blanks only the references to the given source IDs. + +describe("stripContentReferences", () => { + it("sets a single-item reference to -1 and leaves others alone", () => { + const fields = { next: { contentid: 5, fulllist: false }, other: { contentid: 9, fulllist: false } }; + const { fields: out, stripped } = stripContentReferences(fields, new Set([5])); + expect(out).toEqual({ next: { contentid: -1, fulllist: false }, other: { contentid: 9, fulllist: false } }); + expect(stripped).toBe(1); + }); + + it("handles contentID spelled with a capital D", () => { + const { fields } = stripContentReferences({ link: { contentID: 5 } }, new Set([5])); + expect(fields.link.contentID).toBe(-1); + }); + + it("removes only the given IDs from sortids", () => { + const { fields, stripped } = stripContentReferences({ picks: { sortids: "3, 5,7" } }, new Set([5, 7])); + expect(fields.picks.sortids).toBe("3"); + expect(stripped).toBe(2); + }); + + it("finds references nested in arrays and objects", () => { + const fields = { rows: [{ cell: { contentid: 5 } }, { cell: { contentid: 6 } }] }; + const { fields: out, stripped } = stripContentReferences(fields, new Set([5, 6])); + expect(out.rows.map((r: any) => r.cell.contentid)).toEqual([-1, -1]); + expect(stripped).toBe(2); + }); + + it("removes the IDs from a companion field named by the model (PROD-2446)", () => { + const model = { fields: [{ name: "Next", settings: { LinkeContentDropdownValueField: "NextID" } }] }; + const fields = { next: { contentid: 5, fulllist: false }, nextID: "5" }; + const { fields: out, stripped } = stripContentReferences(fields, new Set([5]), model); + expect(out).toEqual({ next: { contentid: -1, fulllist: false }, nextID: "" }); + expect(stripped).toBe(2); + }); + + it("does not touch whole-list references or other values", () => { + const fields = { list: { referencename: "posts", fulllist: true }, title: "5", count: 5 }; + const { fields: out, stripped } = stripContentReferences(fields, new Set([5])); + expect(out).toEqual(fields); + expect(stripped).toBe(0); + }); + + it("never modifies the input", () => { + const fields = { next: { contentid: 5 }, picks: { sortids: "5,6" } }; + const before = JSON.stringify(fields); + stripContentReferences(fields, new Set([5])); + expect(JSON.stringify(fields)).toBe(before); + }); +}); diff --git a/src/lib/pushers/content-pusher/util/types.ts b/src/lib/pushers/content-pusher/util/types.ts index 39c057c..994bea7 100644 --- a/src/lib/pushers/content-pusher/util/types.ts +++ b/src/lib/pushers/content-pusher/util/types.ts @@ -15,6 +15,9 @@ export interface ContentBatchConfig { defaultAssetUrl?: string; // Default asset URL for content mapping targetData?: any; // Target instance data for checking existing content onBatchComplete?: (batchResult: BatchProcessingResult, batchNumber: number) => Promise; // Callback after each batch completes + // PROD-3076: update the target item a mapping points at even though it isn't in the pulled target + // data, because it was created earlier in this run (pass 2 of a reference cycle). + trustSameRunMappings?: boolean; } /** diff --git a/src/lib/pushers/page-pusher/process-page.ts b/src/lib/pushers/page-pusher/process-page.ts index 927c19d..ccacf09 100644 --- a/src/lib/pushers/page-pusher/process-page.ts +++ b/src/lib/pushers/page-pusher/process-page.ts @@ -390,9 +390,11 @@ export async function processPage({ // Fix zones format - ensure zones is always a defined object (never null/undefined) const formattedZones = mappedZones && typeof mappedZones === "object" ? mappedZones : {}; - // CRITICAL FIX: Ensure every page has a valid title field - // Folder pages often don't have titles, but API requires them - const pageTitle = page.title || page.menuText || page.name || "Untitled Page"; + // Send the source title as-is. The Fetch API (Content Sync) returns an empty title as null, and + // the Management API accepts a missing or empty title (folders ignore it), so null goes as "". + // Filling in the menu text or page name made an untitled page - typically a dynamic page, whose + // titles come from its titleFormula - differ from the source on every sync (PROD-3025). + const pageTitle = page.title ?? ""; const pageJSON = JSON.stringify(page, null, 2); const pageCopy = JSON.parse(pageJSON) as mgmtApi.PageItem; // Create a copy to avoid modifying original diff --git a/src/lib/pushers/page-pusher/tests/process-page.test.ts b/src/lib/pushers/page-pusher/tests/process-page.test.ts index d1af91b..917df53 100644 --- a/src/lib/pushers/page-pusher/tests/process-page.test.ts +++ b/src/lib/pushers/page-pusher/tests/process-page.test.ts @@ -328,6 +328,46 @@ describe("processPage — folder pages", () => { }); }); +// ─── page title in the save payload (PROD-3025) ────────────────────────────── + +describe("processPage — page title sent to the target (PROD-3025)", () => { + async function savedTitle(page: any): Promise { + const { TemplateMapper } = require("lib/mappers/template-mapper"); + TemplateMapper.mockImplementation(() => ({ + getTemplateMappingByPageTemplateName: jest.fn().mockReturnValue({ ref: "Main" }), + getMappedEntity: jest.fn().mockReturnValue({ contentSectionDefinitions: [] }), + })); + const apiClient = makeApiClient(); + await processPage(makeProps({ page, apiClient, pageMapper: makePageMapper() })); + expect(apiClient.pageMethods.savePage).toHaveBeenCalled(); + return apiClient.pageMethods.savePage.mock.calls[0][0].title; + } + + it("keeps an untitled dynamic page untitled instead of filling in its name", async () => { + // As pulled through the Fetch API: an empty title comes back as null + const page = makePage({ + name: "posts-dynamic", + pageType: "dynamic", + title: null, + menuText: "", + dynamic: { referenceName: "Posts", titleFormula: "##Title##", pageNameFormula: "##Slug##" }, + }); + expect(await savedTitle(page)).toBe(""); + }); + + it("keeps an empty title empty", async () => { + expect(await savedTitle(makePage({ title: "", menuText: "Blog" }))).toBe(""); + }); + + it("sends a page's own title unchanged", async () => { + expect(await savedTitle(makePage({ title: "About Us" }))).toBe("About Us"); + }); + + it("does not take the menu text or name for a page with no title", async () => { + expect(await savedTitle(makePage({ title: undefined, menuText: "Blog", name: "blog" }))).toBe(""); + }); +}); + // ─── successful save via batch ───────────────────────────────────────────────── describe("processPage — successful batch save", () => {