diff --git a/README.md b/README.md index 62f1e65..53eeccf 100644 --- a/README.md +++ b/README.md @@ -118,7 +118,7 @@ agility sync [options] | Option | Type | Default | Description | | --------------- | ------- | ------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `--overwrite` | boolean | `false` | Conflict-scoped override. By default, when a target item has its own changes that conflict with the source (both sides changed since the last sync), the CLI skips it as a conflict to avoid data loss. An item changed only on the target, with the source unchanged, is kept with a warning instead (listed under "Target changes kept" and as `targetChangeKept` warnings in `--jsonSummary`); it isn't a conflict, and a later source change to it is. This applies to content, pages, models, containers, assets and galleries. With `--overwrite`, both kinds are overwritten with the source version. Non-conflicting updates are applied either way. Also covers a mapped target item that no longer exists (deleted or unpublished on the target): by default that is reported as a conflict and skipped, with `--overwrite` the item is recreated and the mapping repointed. | +| `--overwrite` | boolean | `false` | Conflict-scoped override. By default, when a target item has its own changes that conflict with the source (both sides changed since the last sync), the CLI skips it as a conflict to avoid data loss. An item changed only on the target, with the source unchanged, is kept with a warning instead (listed under "Target changes kept" and as `targetChangeKept` warnings in `--jsonSummary`); it isn't a conflict, and a later source change to it is. This applies to content, pages, models, containers, assets, galleries, page templates, URL redirections and list order (page templates and URL redirections have no version, so the CLI stores a hash of each side in the mapping to tell which one changed; a mapping from an older CLI has none, so the first sync after upgrading lets the source win and records them). A page template kept on the target holds back the pages that use it only when it has no section for one of their zones. With `--overwrite`, both kinds are overwritten with the source version. Non-conflicting updates are applied either way. Also covers a mapped target item that no longer exists (deleted or unpublished on the target): by default that is reported as a conflict and skipped, with `--overwrite` the item is recreated and the mapping repointed. | | `--syncDeletes` | boolean | `false` | After syncing, delete from the target the entities that were deleted in the source since the last sync (URL redirections, pages, content, containers, templates, models, assets, galleries — dependents first). Only entities the CLI previously synced (that have a mapping) are considered; nothing created directly on the target is touched. A target entity edited since the last sync is reported as a conflict and kept unless `--overwrite` is also set. An entity that something kept on the target still uses (a child page, a page or item that links to the content, a container's items, ...) is kept and reported. Content and page deletes are confirmed against the source first, so an item that was only unpublished is never deleted. Cannot be combined with `--pages`, `--containers`, `--models` or `--models-with-deps`; use `--elements` to limit the types. Works with `--preflight`. Not read from `.env`. See [Syncing deletes](#syncing-deletes). | | `--allowMassDeletes` | boolean | `false` | With `--syncDeletes`: allow a run to delete more than half (and more than 10) of the mapped entities of one type. Without it such a run deletes nothing and fails, because a stale or partial local cache looks the same as a mass delete. | | `--autoPublish` | string | _(disabled)_ | Automatically publish synced items that were published in the source instance. Values: `content`, `pages`, `both`. If flag is provided without a value, defaults to `both`. Items that are only in staging (not published) in the source are skipped. | @@ -194,6 +194,7 @@ A sync puts each synced content list on the target in the source's item order. I - The new order applies to the target's staging and live list at once. No item gets a new version, so nothing needs publishing for the order to show. - Items that exist only on the target keep their relative order after the synced ones. - A scoped run (`--pages`, `--containers`, `--models`) only orders the lists its content is in, but sends every synced item of those lists, so their order stays whole. +- Each run records, per list, the source's order and the order it left on the target. A list reordered on the target since then, while the source's order didn't change, keeps the target's order with a warning ("Target changes kept"); reordered on both sides, it's a conflict. `--overwrite` sets the source's order either way. Adding or deleting an item isn't a reorder, and neither is a list that still shows the order the last sync replaced (the Fetch API shows an order change a few minutes later; the list is skipped until it does). The first sync after upgrading has no record, so it sets the source's order, as before, and records it. - `--preflight` lists the lists it would reorder under a `List order` phase, and `--jsonSummary` reports them as the `listOrder` phase. ### Auto-Publish diff --git a/ci/README.md b/ci/README.md index 8f9a4e2..68b0c6e 100644 --- a/ci/README.md +++ b/ci/README.md @@ -37,17 +37,23 @@ directions: - **Odd rounds** edit A and `sync` A → B. - **Even rounds** edit B and `reverse-sync` B → A. -- **Every round** also edits the other instance (drift) and one item on both sides (a conflict). +- **Every round** also edits the other instance (drift: a content item, a list's order, a page + template's section name and a URL redirection's destination) and one item on both sides (a + conflict). Before syncing, the run waits for the Fetch API to show each edit, a list's new order + included (it shows up later than a new version). The redirection is one the run adds to A before the first sync; the Management SDK can + only delete redirections, so the run saves it through the Management API's batch endpoint and + waits for the Fetch API to list each edit, as the CLI does. The changes, in `ci/permutation/changes.mjs`, are discovered from whatever each instance holds: | Area | Changes | | ---------- | -------------------------------------------------------------------------------------------------- | | Content | edit, add, add a linked item together with an item that links to it, delete, publish and unpublish | -| Ordering | swap two items in a list; reverse the components in a page zone | +| Ordering | move a list's last item to the top (as drag and drop does, no new versions); reverse the components in a page zone | | Pages | edit title and menu text, add a page, delete it later | | Components | remove one from a zone | | Models | add a field, change a label, rename a field | +| Templates | rename a section's display name | The `quick` tier runs a fixed set covering every area in two rounds. `beta` (4 rounds) and `nightly` (6 rounds) pick changes at random from a seed printed in the summary. Replay one with @@ -61,6 +67,11 @@ every sync it checks: conflict; - **drift:** an edit made only on the receiving side is overwritten, or kept with a "target changed" warning (not a conflict), never silently kept; +- **drift:listOrder, drift:template, drift:urlRedirection:** a list reorder, a template edit or a + redirection edit made only on the receiving + side is kept with a "target changed" warning, or reported as a conflict when the round's own changes + touched the same list or template. These have no version, so the sync tells which side changed + from what it recorded at the last sync (PROD-3078); overwriting them without `--overwrite` fails; - **resolve:** when the sync kept such edits, the same sync with `--overwrite` settles them, as an editor would, before the checks below; - **verify:** `agility verify` finds no differences. A verify that finds some is tried once more a diff --git a/ci/permutation/changes.mjs b/ci/permutation/changes.mjs index 7e26d71..8cf1f48 100644 --- a/ci/permutation/changes.mjs +++ b/ci/permutation/changes.mjs @@ -206,21 +206,39 @@ export const CHANGES = { return { area: "content", change: "delete", detail: `#${id}` }; }, + // A reorder as the Manager's drag and drop makes it: the list item-order endpoint renumbers the + // list in place, with no new versions. The last item moves to the top. (Saving items with + // swapped itemOrder values doesn't reliably reorder: the server can leave the order as it was.) async "content.reorder"(ctx) { - const l = ctx.r.pick(await listsWithItems(ctx.c, ctx.guid, ctx.locale, 2)); + // `ctx.avoidLists`: lists the round already reordered on the other side. The same move on both sides + // leaves them in the same order again, which is no change to report. + const avoid = new Set((ctx.avoidLists ?? []).map((x) => x.toLowerCase())); + const candidates = (await listsWithItems(ctx.c, ctx.guid, ctx.locale, 2)).filter( + (x) => x.items.length <= 30 && !avoid.has(x.list.referenceName.toLowerCase()) + ); + const l = ctx.r.pick(candidates); if (!l) return null; - const [a, b] = [l.items[0], l.items[l.items.length - 1]]; - const fa = await ctx.c.contentMethods.getContentItem(a.contentID, ctx.guid, ctx.locale); - const fb = await ctx.c.contentMethods.getContentItem(b.contentID, ctx.guid, ctx.locale); - const oa = fa.properties.itemOrder; - fa.properties.itemOrder = fb.properties.itemOrder; - fb.properties.itemOrder = oa; - await save(ctx.c, ctx.guid, ctx.locale, fa); - await save(ctx.c, ctx.guid, ctx.locale, fb); + const readOrder = async () => { + const full = await Promise.all( + l.items.map((x) => ctx.c.contentMethods.getContentItem(x.contentID, ctx.guid, ctx.locale)) + ); + return full + .map((x) => [x.contentID, x.properties?.itemOrder ?? Number.MAX_SAFE_INTEGER]) + .sort((a, b) => a[1] - b[1] || a[0] - b[0]); + }; + const before = (await readOrder()).map(([id]) => id); + const wanted = [before[before.length - 1], ...before.slice(0, -1)]; + await ctx.c.contentMethods.setContentListItemOrder(l.list.referenceName, ctx.guid, ctx.locale, wanted, true); + const after = await readOrder(); + if (after.map(([id]) => id).join() !== wanted.join()) { + throw new Error(`${l.list.referenceName} is in order ${after.map(([id]) => id).join(", ")}, not ${wanted.join(", ")}`); + } return { area: "ordering", - change: "list-swap", - detail: `${l.list.referenceName} #${a.contentID} ↔ #${b.contentID}`, + change: "list-reorder", + detail: `${l.list.referenceName} #${wanted[0]} moved to the top`, + entity: l.list.referenceName, + orders: { guid: ctx.guid, items: after }, }; }, @@ -234,6 +252,25 @@ export const CHANGES = { return { area: "content", change: published ? "unpublish" : "publish", detail: `#${it.contentID}` }; }, + // A section's display name, not its reference name: a page's zones stay where they are. + async "template.section-rename"(ctx) { + const all = await ctx.c.pageMethods.getPageTemplates(ctx.guid, ctx.locale, true); + const t = ctx.r.pick(all.filter((x) => x.pageTemplateID > 0)); + if (!t) return null; + const full = await ctx.c.pageMethods.getPageTemplate(ctx.guid, ctx.locale, t.pageTemplateID); + const s = full?.contentSectionDefinitions?.[0]; + if (!s) return null; + const base = String(s.pageItemTemplateName ?? s.pageItemTemplateReferenceName).replace(/ \(r\d+[st]\)$/, ""); + s.pageItemTemplateName = `${base} (${tag(ctx)})`; + await ctx.c.pageMethods.savePageTemplate(ctx.guid, ctx.locale, full); + return { + area: "templates", + change: "section-rename", + detail: `template ${full.pageTemplateName} section ${s.pageItemTemplateReferenceName}`, + entity: full.pageTemplateName, + }; + }, + async "page.edit"(ctx) { const p = ctx.r.pick(await pages(ctx.c, ctx.guid, ctx.locale)); if (!p) return null; @@ -377,6 +414,14 @@ export async function drift(ctx, avoid = new Set()) { return CHANGES["content.edit"]({ ...ctx, avoid }); } +// Drift on the types with no version to compare (PROD-3078): the sync tells which side changed from +// what it recorded at the last sync. `kind` is the --jsonSummary warning type of a kept change, and +// `phase` the preflight phase a conflict is reported under. +export const STRUCTURAL_DRIFTS = [ + { kind: "listOrder", phase: "List order", change: "content.reorder" }, + { kind: "template", phase: "Templates", change: "template.section-rename" }, +]; + // The same item edited on both sides with different values: a conflict. `avoidDstId` is the item // the round's drift edited, so the two checks grade different items. export async function conflict(srcCtx, dstCtx, mappingLookup, avoidDstId = null) { diff --git a/ci/permutation/fixtures.mjs b/ci/permutation/fixtures.mjs index 1b0f908..a0d83f1 100644 --- a/ci/permutation/fixtures.mjs +++ b/ci/permutation/fixtures.mjs @@ -222,6 +222,40 @@ export async function waitForFetchVersions(token, guid, locale, wanted, { timeou throw new Error(`Fetch API still behind after ${timeoutMs / 1000}s on items ${behind.slice(0, 10).join(", ")}`); } +// An order change reaches the Fetch API's sync feed later than the item's new version. Wait until +// the feed shows each given item at the given itemOrder. Returns the seconds waited; throws on timeout. +export async function waitForFetchItemOrder(token, guid, locale, items, { timeoutMs = 10 * 60 * 1000 } = {}) { + if (!items.length) return 0; + 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(); + let behind = []; + while (Date.now() - started < timeoutMs) { + // The feed is a log: the last entry for an item is its current state + const order = new Map(); + let syncToken = 0; + for (let page = 0; page < 100; 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 ?? []) order.set(item.contentID, item.properties?.itemOrder); + if (!body.items?.length || !body.syncToken || body.syncToken === syncToken) break; + syncToken = body.syncToken; + } + behind = items.filter(([id, o]) => order.get(id) !== o).map(([id]) => id); + if (behind.length === 0) return Math.round((Date.now() - started) / 1000); + await new Promise((r) => setTimeout(r, 15_000)); + } + throw new Error(`Fetch API still shows the old order after ${timeoutMs / 1000}s for items ${behind.join(", ")}`); +} + // 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 = []; @@ -240,3 +274,89 @@ export async function checkReferenceCycles(c, guid, locale, fixture, toTarget) { } return problems; } + +// ── URL redirections (PROD-3078) ────────────────────────────────────────────────────────────── +// The Management SDK can only delete redirections, so these call the endpoints the CLI uses: the +// Management API's batch save to write, and the Fetch API's redirection list (what a pull reads). + +async function previewKey(token, guid) { + const { mgmt } = hosts(guid); + const res = await fetch(`${mgmt}/oauth/GetPreviewKey?guid=${guid}`, { + headers: { Authorization: `Bearer ${token}`, "Cache-Control": "no-cache" }, + }); + if (!res.ok) throw new Error(`GetPreviewKey: HTTP ${res.status}`); + return (await res.text()).replace(/^"|"$/g, ""); +} + +const sameOrigin = (a, b) => String(a ?? "").trim().toLowerCase() === String(b ?? "").trim().toLowerCase(); + +// The instance's redirections as the Fetch API lists them. +export async function listRedirections(token, guid) { + const { api } = hosts(guid); + const res = await fetch(`${api}/${guid}/preview/urlredirection`, { + headers: { APIKey: await previewKey(token, guid), "Cache-Control": "no-cache" }, + }); + if (!res.ok) throw new Error(`Fetch API urlredirection: HTTP ${res.status}`); + const body = await res.json(); + return body.urlRedirections ?? body.items ?? []; +} + +// Create (urlRedirectionID 0) or replace one redirection; returns its ID. A refusal is thrown. +export async function saveRedirection(token, guid, r) { + const { mgmt } = hosts(guid); + const res = await fetch(`${mgmt}/api/v1/instance/${guid}/url-redirections`, { + method: "POST", + headers: { Authorization: `Bearer ${token}`, "Content-Type": "application/json", "Cache-Control": "no-cache" }, + body: JSON.stringify([ + { + urlRedirectionID: r.urlRedirectionID ?? 0, + originUrl: r.originUrl, + destinationUrl: r.destinationUrl, + httpCode: r.statusCode ?? 301, + destinationLocale: r.destinationLocale ?? null, + originLocales: r.originLocales ?? null, + content: r.content ?? null, + }, + ]), + }); + if (!res.ok) throw new Error(`url-redirections save: HTTP ${res.status} ${(await res.text()).slice(0, 200)}`); + const out = await res.json(); + const done = out.created?.[0] ?? out.updated?.[0]; + if (!done) throw new Error(`url-redirections save skipped it: ${out.skipped?.[0]?.reason ?? JSON.stringify(out)}`); + return done.urlRedirectionID ?? r.urlRedirectionID; +} + +// Wait until the Fetch API lists the redirection with this origin in the wanted state. +export async function waitForRedirection(token, guid, originUrl, test, { timeoutMs = 10 * 60 * 1000 } = {}) { + const started = Date.now(); + while (Date.now() - started < timeoutMs) { + const found = (await listRedirections(token, guid)).find((x) => sameOrigin(x.originUrl, originUrl)); + if (found && test(found)) return Math.round((Date.now() - started) / 1000); + await new Promise((r) => setTimeout(r, 10_000)); + } + throw new Error(`redirection ${originUrl} not in the wanted state in the Fetch API after ${timeoutMs / 1000}s`); +} + +// One redirection on the source before the first sync, for later rounds to edit. +export async function seedRedirection(token, guid, seed) { + const originUrl = `~/perm-redirect-${seed}`; + const destinationUrl = "~/"; + await saveRedirection(token, guid, { originUrl, destinationUrl }); + const waited = await waitForRedirection(token, guid, originUrl, (x) => x.destinationUrl === destinationUrl); + return { originUrl, waited }; +} + +// Point the redirection somewhere new on one instance, as an editor would; waits for the Fetch API. +export async function editRedirection(token, guid, originUrl, tag) { + const current = (await listRedirections(token, guid)).find((x) => sameOrigin(x.originUrl, originUrl)); + if (!current) return null; + const destinationUrl = `~/perm-${tag}`; + await saveRedirection(token, guid, { ...current, urlRedirectionID: current.id, destinationUrl }); + const waited = await waitForRedirection(token, guid, originUrl, (x) => x.destinationUrl === destinationUrl); + return { + area: "redirections", + change: "edit", + detail: `${current.originUrl} → ${destinationUrl} (in the Fetch API after ${waited}s)`, + entity: current.originUrl, + }; +} diff --git a/ci/permutation/run.mjs b/ci/permutation/run.mjs index a738975..70fa504 100644 --- a/ci/permutation/run.mjs +++ b/ci/permutation/run.mjs @@ -14,6 +14,10 @@ // sync exit 0, no failed items // conflict the both-sides item ends at the synced side's value, or a preflight reported it // drift the other side's own edit was overwritten, or kept with a "target changed" warning +// drift:listOrder, drift:template, drift:urlRedirection +// a list reorder, template edit or redirection edit on the other side alone is kept +// with a "target changed" warning, or reported as a conflict if the round's own changes +// touched it too; never overwritten without --overwrite (PROD-3078) // resolve when the sync kept such edits: the same sync with --overwrite settles them // verify `agility verify` finds no differences (verify and noop: up to 3 tries, 90s apart) // noop a preflight in the same direction plans no creates, updates, deletes or conflicts @@ -28,13 +32,16 @@ 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 { CHANGES, QUICK_ROUNDS, STRUCTURAL_DRIFTS, client, conflict, drift, readField, rng } from "./changes.mjs"; import { changedSince, checkReferenceCycles, contentVersions, + editRedirection, + seedRedirection, seedReferenceCycles, waitForFetchSync, + waitForFetchItemOrder, waitForFetchVersions, } from "./fixtures.mjs"; @@ -124,7 +131,13 @@ function syncStep(round, direction, step = "sync", extra = []) { const conflicts = entries.filter((e) => e.action === "conflict").map((e) => e.detail ?? ""); const kept = entries.filter((e) => e.action === "skip" && /kept the target's version/.test(e.detail ?? "")); const names = (guid, id) => new RegExp(`/instance/${guid}/[^/]+/content/listitem-${id}(?!\\d)`); + const same = (a, b) => String(a ?? "").toLowerCase() === String(b ?? "").toLowerCase(); return { + // Whether the sync kept a target-only change to this entity (a targetChangeKept warning) + keptByType: (type, name) => + (s.json?.warnings ?? []).some((w) => w.code === "targetChangeKept" && w.type === type && same(w.name, name)), + // Whether a preflight reported a conflict on this entity in a phase + conflictIn: (phase, name) => entries.some((e) => e.action === "conflict" && e.phase === phase && same(e.name, name)), // Whether a preflight reported a conflict on this content item (its URL is in the detail) reported: (guid, id) => conflicts.some((d) => names(guid, id).test(d)), // Whether a preflight kept this content item's target-only change, with a warning @@ -193,6 +206,15 @@ try { record(0, "A→B", "apply:fixture-cycles", false, String(e?.message ?? e).slice(0, 200)); } +// A redirection for the rounds to edit on the receiving side (PROD-3078) +let redirection = null; +try { + redirection = await seedRedirection(AGILITY_TOKEN, A, SEED); + record(0, "A→B", "apply:fixture-redirection", true, `${redirection.originUrl}, in the Fetch API after ${redirection.waited}s`); +} catch (e) { + record(0, "A→B", "apply:fixture-redirection", false, String(e?.message ?? e).slice(0, 200)); +} + syncStep(0, "A→B"); if (cycles) { @@ -252,7 +274,7 @@ for (let i = 0; i < plan.length; i++) { let drifted = null; try { // The receiving side's copies of content items this round touched on the sending side: edits, - // list swaps (which save both items), publishes and deletes. Page and model changes name page IDs + // list reorders, publishes and deletes. Page and model changes name page IDs // or none, so only the content and ordering areas count. const editedThere = applied .filter((a) => a.area === "content" || a.area === "ordering") @@ -265,6 +287,33 @@ for (let i = 0; i < plan.length; i++) { record(round, direction, "apply:drift", false, String(e?.message ?? e).slice(0, 200)); } + // A list reorder and a template edit on the receiving side alone (PROD-3078) + const structural = []; + for (const d of STRUCTURAL_DRIFTS) { + try { + const avoidLists = applied.filter((x) => x.change === "list-reorder" && x.entity).map((x) => x.entity); + const did = await CHANGES[d.change]({ ...toCtx, avoidLists }); + if (did) { + structural.push({ ...d, ...did }); + applied.push({ name: `drift:${d.kind}`, ...did, area: "drift" }); + } else console.log(` (skipped drift:${d.kind}: nothing it applies to)`); + } catch (e) { + record(round, direction, `apply:drift:${d.kind}`, false, String(e?.message ?? e).slice(0, 200)); + } + } + + if (redirection) { + try { + const did = await editRedirection(AGILITY_TOKEN, to, redirection.originUrl, `r${round}d`); + if (did) { + structural.push({ kind: "urlRedirection", phase: "URL Redirections", ...did }); + applied.push({ name: "drift:urlRedirection", ...did, area: "drift" }); + } else record(round, direction, "apply:drift:urlRedirection", false, `${redirection.originUrl} isn't on ${to}`); + } catch (e) { + record(round, direction, "apply:drift:urlRedirection", false, String(e?.message ?? e).slice(0, 200)); + } + } + let both = null; try { const driftedId = drifted ? Number(/#(\d+)/.exec(drifted.detail)?.[1]) || null : null; @@ -274,7 +323,7 @@ for (let i = 0; i < plan.length; i++) { record(round, direction, "apply:conflict", false, String(e?.message ?? e).slice(0, 200)); } - rounds.push({ round, direction, changes: applied.map(({ sv, dv, ...rest }) => rest) }); + rounds.push({ round, direction, changes: applied.map(({ sv, dv, orders, ...rest }) => rest) }); console.log( `\n── round ${round} (${direction}): ${applied.map((a) => `${a.area}/${a.change}`).join(", ") || "no changes"}` ); @@ -289,8 +338,17 @@ for (let i = 0; i < plan.length; i++) { record(round, direction, "apply:fetch-catch-up", false, String(e?.message ?? e).slice(0, 200)); } } + // A list swap's new order shows in the Fetch API later than the items' new versions + for (const a of applied.filter((x) => x.orders)) { + try { + const waited = await waitForFetchItemOrder(AGILITY_TOKEN, a.orders.guid, LOCALE, a.orders.items); + console.log(` Fetch API showed the new order of ${a.entity ?? a.detail} on ${a.orders.guid} after ${waited}s`); + } catch (e) { + record(round, direction, "apply:fetch-order", false, String(e?.message ?? e).slice(0, 200)); + } + } - const { reported, keptWarned } = syncStep(round, direction); + const { reported, keptWarned, keptByType, conflictIn } = syncStep(round, direction); let kept = false; if (both) { @@ -334,6 +392,23 @@ for (let i = 0; i < plan.length; i++) { kept ||= !same; } + for (const d of structural) { + const warned = keptByType(d.kind, d.entity); + const conflicted = conflictIn(d.phase, d.entity); + record( + round, + direction, + `drift:${d.kind}`, + warned || conflicted, + warned + ? `${d.entity}: kept, with a target-changed warning` + : conflicted + ? `${d.entity}: reported as a conflict (the round's own changes touched it too)` + : `${d.entity}: the other side's edit was overwritten without --overwrite, or not seen` + ); + kept ||= warned || conflicted; + } + // The sync rightly kept the receiving side's own edits and reported them. Settle them the way an // editor would, by syncing again with --overwrite, so verify and the no-op check see one state. if (kept) syncStep(round, direction, "resolve", ["--overwrite"]); diff --git a/src/lib/list-order/sync-list-order.ts b/src/lib/list-order/sync-list-order.ts index 21c56ef..6d0cb85 100644 --- a/src/lib/list-order/sync-list-order.ts +++ b/src/lib/list-order/sync-list-order.ts @@ -12,6 +12,15 @@ * pulled order are sent, and lists that gained an item this run (it isn't in the pull yet). A mapped * item missing from the pull that this run didn't save is a stale mapping, not a new item: it doesn't * make the list differ, or every run would send that list again. + * + * Which side reordered (PROD-3078): after each run, the mapping type `listorder` (per locale) + * records each list's source order and the target order the run left. A list whose target order + * differs from the source's is then decided like any other entity (see kept-target-changes.ts): + * reordered on the source only, it's set to the source's order; on the target only, the target's + * order is kept with a warning; on both, it's a conflict; --overwrite sets the source's order in + * every case. A reorder is a change in the relative order of the items both the record and the + * list have, so an item added or deleted on either side isn't one. A list with no record yet (the + * first run after upgrading) gets the source's order, as before, and is recorded. */ import ansiColors from "ansi-colors"; @@ -19,6 +28,8 @@ import { state, getApiClient, getLoggerForGuid } from "../../core/state"; import { fileOperations } from "../../core/fileOperations"; import { GuidDataLoader } from "../pushers/guid-data-loader"; import { preflightReport } from "../preflight/preflight-report"; +import { keptTargetChanges } from "../pushers/kept-target-changes"; +import { PENDING_DETAIL, UNRECORDED_UPDATE_DETAIL } from "../shared/sync-fingerprint"; import { FailureDetail, PusherResult } from "../../types/sourceData"; export interface ListOrderOptions { @@ -42,6 +53,50 @@ export interface ListOrderPlan { contentIDs: number[]; } +/** + * A list's order as the last run left it (mapping type `listorder`, per locale). The `source*` / + * `target*` pairs transpose for a reverse sync like every other mapping record. + */ +export interface ListOrderRecord { + sourceGuid?: string; + targetGuid?: string; + sourceReferenceName: string; + targetReferenceName: string; + /** Source content IDs in the source's order */ + sourceContentIDs: number[]; + /** Target content IDs in the order the run left the target list */ + targetContentIDs: number[]; + /** + * The target's order before this run reordered it, kept until a pull shows the new order. The + * Fetch API shows an order change a while after it's made, and a pull in that window still shows + * the old order: that isn't a reorder. A reverse sync reads it as `sourceReplacedContentIDs`. + */ + targetReplacedContentIDs?: number[]; + sourceReplacedContentIDs?: number[]; +} + +export type ListOrderAction = + | "reorder" // set the target to the source's order + | "in-order" // nothing to send + | "keep" // reordered on the target only: keep it and warn + | "conflict" // reordered on both sides + | "pending"; // a side still shows the order the last run replaced (the Fetch API hasn't caught up): wait + +/** One list's decision, with the orders to record once it's settled. */ +export interface ListOrderDecision extends ListOrderPlan { + action: ListOrderAction; + /** The list differs and has no record yet: the source's order wins, as before */ + unrecorded: boolean; + /** --overwrite turned a keep or a conflict into a reorder */ + overwritten?: "keep" | "conflict"; + /** Source content IDs in the source's order, to record */ + sourceOrder: number[]; + /** Target content IDs in the order this run leaves them (if a reorder is sent, after it) */ + targetOrder: number[]; + /** For a reorder: the target's order it replaces */ + replacedOrder?: number[]; +} + const DELETED_STATE = 3; const lower = (value: string | undefined | null) => (value || "").toLowerCase(); @@ -76,8 +131,17 @@ function groupByList(items: ListItem[]): Map { return lists; } +/** Whether the items both orders have appear in a different relative order. */ +export function relativeOrderChanged(recorded: number[], current: number[]): boolean { + const inRecord = new Set(recorded); + const inCurrent = new Set(current); + const before = recorded.filter((id) => inCurrent.has(id)); + const now = current.filter((id) => inRecord.has(id)); + return before.some((id, i) => id !== now[i]); +} + /** - * Work out which lists need a new order on the target, for one locale. + * Decide each list's order on the target, for one locale. * * @param sourceItems every content item in the source pull for the locale (not just a scoped run's) * @param targetItems every content item in the target pull for the locale @@ -86,19 +150,24 @@ function groupByList(items: ListItem[]): Map { * @param scope when set, only these source lists (lower case) are considered, e.g. a scoped run's * @param savedThisRun target content IDs this run saved; a mapped item missing from the target pull * only counts as new when it's one of these + * @param records the locale's `listorder` records from the last run + * @param overwrite --overwrite: the source's order wins over a target reorder */ -export function planListOrders( +export function decideListOrders( locale: string, sourceItems: ListItem[], targetItems: ListItem[], contentMap: Map, containerMap: Map, scope?: Set, - savedThisRun: Set = new Set() -): ListOrderPlan[] { + savedThisRun: Set = new Set(), + records: ListOrderRecord[] = [], + overwrite = false +): ListOrderDecision[] { const sourceLists = groupByList(sourceItems); const targetLists = groupByList(targetItems); - const plans: ListOrderPlan[] = []; + const recordFor = new Map(records.filter((r) => r?.sourceReferenceName).map((r) => [lower(r.sourceReferenceName), r])); + const decisions: ListOrderDecision[] = []; for (const [sourceKey, items] of Array.from(sourceLists.entries())) { if (scope && !scope.has(sourceKey)) continue; @@ -107,8 +176,9 @@ export function planListOrders( if (!targetReferenceName) continue; // Source order, mapped. Two source items mapped to one target keep the first position. + const sourceSorted = sortByListOrder(items); const contentIDs: number[] = []; - for (const item of sortByListOrder(items)) { + for (const item of sourceSorted) { const targetID = contentMap.get(item.contentID); if (targetID && !contentIDs.includes(targetID)) contentIDs.push(targetID); } @@ -118,18 +188,105 @@ export function planListOrders( const current = sortByListOrder(targetLists.get(lower(targetReferenceName)) || []); const currentIDs = current.map((i) => i.contentID); const listed = new Set(contentIDs); - const expected = [...contentIDs.filter((id) => currentIDs.includes(id)), ...currentIDs.filter((id) => !listed.has(id))]; + const others = currentIDs.filter((id) => !listed.has(id)); + const expected = [...contentIDs.filter((id) => currentIDs.includes(id)), ...others]; const added = contentIDs.some((id) => !currentIDs.includes(id) && savedThisRun.has(id)); const differs = expected.some((id, i) => id !== currentIDs[i]); - if (!added && !differs && hasDefinedOrder(current)) continue; + const needsOrder = added || differs || !hasDefinedOrder(current); + + const sourceOrder = sourceSorted.map((i) => i.contentID); + // After a reorder: the listed items the target has (or gains this run), then its others. + const reordered = [...contentIDs.filter((id) => currentIDs.includes(id) || savedThisRun.has(id)), ...others]; + const base = { locale, sourceReferenceName: items[0].properties!.referenceName!, targetReferenceName, contentIDs, sourceOrder }; + + if (!needsOrder) { + decisions.push({ ...base, action: "in-order", unrecorded: false, targetOrder: currentIDs }); + continue; + } + + const record = recordFor.get(sourceKey); + if (!record || !Array.isArray(record.sourceContentIDs) || !Array.isArray(record.targetContentIDs)) { + decisions.push({ ...base, action: "reorder", unrecorded: true, targetOrder: reordered, replacedOrder: currentIDs }); + continue; + } + + // A side still in the order the last run replaced was pulled before the reorder reached the + // Fetch API: not a reorder. Syncing from a source in that state would put the old order back. + const moved = (recorded: number[], replaced: number[] | undefined, current: number[]) => { + if (!relativeOrderChanged(recorded, current)) return "same"; + if (Array.isArray(replaced) && !relativeOrderChanged(replaced, current)) return "pending"; + return "moved"; + }; + const source = moved(record.sourceContentIDs, record.sourceReplacedContentIDs, sourceOrder); + const target = moved(record.targetContentIDs, record.targetReplacedContentIDs, currentIDs); + const verdict: ListOrderAction = + target === "moved" + ? source === "moved" + ? "conflict" + : "keep" + : source === "moved" + ? "reorder" + : source === "pending" || target === "pending" + ? "pending" + : "reorder"; - plans.push({ locale, sourceReferenceName: items[0].properties!.referenceName!, targetReferenceName, contentIDs }); + if (verdict === "reorder" || ((verdict === "keep" || verdict === "conflict") && overwrite)) { + decisions.push({ + ...base, + action: "reorder", + unrecorded: false, + ...(verdict !== "reorder" ? { overwritten: verdict as "keep" | "conflict" } : {}), + targetOrder: reordered, + replacedOrder: currentIDs, + }); + } else { + decisions.push({ ...base, action: verdict, unrecorded: false, targetOrder: currentIDs }); + } } - return plans.sort((a, b) => lower(a.targetReferenceName).localeCompare(lower(b.targetReferenceName))); + return decisions.sort((a, b) => lower(a.targetReferenceName).localeCompare(lower(b.targetReferenceName))); } +/** + * Work out which lists need a new order on the target, for one locale, with no record of the last + * run: every list whose target order differs from the source's. See decideListOrders. + */ +export function planListOrders( + locale: string, + sourceItems: ListItem[], + targetItems: ListItem[], + contentMap: Map, + containerMap: Map, + scope?: Set, + savedThisRun: Set = new Set() +): ListOrderPlan[] { + return decideListOrders(locale, sourceItems, targetItems, contentMap, containerMap, scope, savedThisRun) + .filter((d) => d.action === "reorder") + .map(({ locale, sourceReferenceName, targetReferenceName, contentIDs }) => ({ + locale, + sourceReferenceName, + targetReferenceName, + contentIDs, + })); +} + +/** The record to store for a settled list; `replaced` is the target order a reorder replaced. */ +function toRecord(opts: ListOrderOptions, d: ListOrderDecision, targetOrder: number[], replaced?: number[]): ListOrderRecord { + return { + sourceGuid: opts.sourceGuid, + targetGuid: opts.targetGuid, + sourceReferenceName: d.sourceReferenceName, + targetReferenceName: d.targetReferenceName, + sourceContentIDs: d.sourceOrder, + targetContentIDs: targetOrder, + ...(replaced ? { targetReplacedContentIDs: replaced } : {}), + }; +} + +const sameOrder = (a: number[] | undefined, b: number[]) => + Array.isArray(a) && a.length === b.length && a.every((id, i) => id === b[i]); + /** source content ID -> target content ID. The last record for a source wins, as in the content mapper. */ export function contentMapFrom(records: any[]): Map { const map = new Map(); @@ -176,37 +333,107 @@ export async function runSyncListOrder( const sourceLoader = new GuidDataLoader(opts.sourceGuid); const targetLoader = new GuidDataLoader(opts.targetGuid); - const plans: ListOrderPlan[] = []; + const decisions: ListOrderDecision[] = []; + const recordsByLocale = new Map(); for (const locale of opts.locales) { const source = await sourceLoader.loadCompleteGuidEntities(locale); const target = await targetLoader.loadCompleteGuidEntities(locale); const itemRecords = fileOps.getMappingFile("item", opts.sourceGuid, opts.targetGuid, locale); + const orderRecords = fileOps.getMappingFile("listorder", opts.sourceGuid, opts.targetGuid, locale); + const records: ListOrderRecord[] = Array.isArray(orderRecords) ? orderRecords : []; + recordsByLocale.set(locale, records); const targetLists = Array.from(new Set((target.content || []).map((i: any) => i?.properties?.referenceName).filter(Boolean))); const scoped = scopedContent?.get(locale); const scope = scoped ? new Set(scoped.map((i) => lower(i.properties?.referenceName)).filter(Boolean)) : undefined; - plans.push( - ...planListOrders( + decisions.push( + ...decideListOrders( locale, source.content || [], target.content || [], contentMapFrom(Array.isArray(itemRecords) ? itemRecords : []), containerMapFrom(Array.isArray(containerRecords) ? containerRecords : [], targetLists as string[]), scope, - new Set(savedContentIDs?.get(locale) || []) + new Set(savedContentIDs?.get(locale) || []), + records, + !!state.overwrite ) ); } + // Settled lists' orders, written to the locale's `listorder` records at the end (not in preflight). + const settled = new Map(); + const settle = (d: ListOrderDecision, targetOrder: number[], replaced?: number[]) => { + const existing = recordsByLocale.get(d.locale)?.find((r) => lower(r.sourceReferenceName) === lower(d.sourceReferenceName)); + if ( + existing && + !replaced && + !existing.targetReplacedContentIDs && + !existing.sourceReplacedContentIDs && + sameOrder(existing.sourceContentIDs, d.sourceOrder) && + sameOrder(existing.targetContentIDs, targetOrder) + ) { + return; + } + if (!settled.has(d.locale)) settled.set(d.locale, []); + settled.get(d.locale)!.push(toRecord(opts, d, targetOrder, replaced)); + }; + + const plans = decisions.filter((d) => d.action === "reorder"); + const label = (d: ListOrderDecision) => `${d.targetReferenceName} [${d.locale}]`; + + // Lists left as they are: already in order (recorded), kept target reorders and conflicts (not recorded). + for (const d of decisions) { + if (d.action === "in-order") { + settle(d, d.targetOrder); + } else if (d.action === "keep") { + const change = { type: "listOrder" as const, name: d.targetReferenceName, locale: d.locale }; + keptTargetChanges.record(change); + console.warn(ansiColors.yellow(keptTargetChanges.formatLine(change))); + logger?.warning(`${label(d)}: reordered on the target since the last sync and not on the source; kept the target's order`); + preflightReport.record({ + phase: "List order", + action: "skip", + name: d.targetReferenceName, + locale: d.locale, + detail: "reordered on the target since the last sync and the source didn't; kept the target's order", + }); + result.skipped++; + } else if (d.action === "pending") { + logger?.info(`${label(d)}: ${PENDING_DETAIL}`); + preflightReport.record({ phase: "List order", action: "skip", name: d.targetReferenceName, locale: d.locale, detail: PENDING_DETAIL }); + result.skipped++; + } else if (d.action === "conflict") { + const reason = "reordered on both the source and the target since the last sync; use --overwrite to set the source's order"; + console.warn(ansiColors.yellow(`⚠️ Conflict detected List order ${label(d)} — ${reason}`)); + logger?.warning(`${label(d)}: conflict: ${reason}`); + preflightReport.record({ + phase: "List order", + action: "conflict", + name: d.targetReferenceName, + locale: d.locale, + detail: "reordered on both sides; use --overwrite to force", + }); + result.skipped++; + } + } + if (state.preflight) { for (const plan of plans) { + const why = plan.unrecorded + ? `; ${UNRECORDED_UPDATE_DETAIL}` + : plan.overwritten === "keep" + ? "; target reorder overwritten" + : plan.overwritten === "conflict" + ? "; conflict overwritten" + : ""; preflightReport.record({ phase: "List order", action: "update", name: plan.targetReferenceName, locale: plan.locale, - detail: `put ${plan.contentIDs.length} item(s) in the source's order`, + detail: `put ${plan.contentIDs.length} item(s) in the source's order${why}`, }); result.successful++; } @@ -216,7 +443,6 @@ export async function runSyncListOrder( const api = getApiClient(); for (const plan of plans) { - const label = `${plan.targetReferenceName} [${plan.locale}]`; try { const response: any = await api.contentMethods.setContentListItemOrder( plan.targetReferenceName, @@ -235,30 +461,44 @@ export async function runSyncListOrder( guid: opts.targetGuid, }; result.warningDetails!.push(detail); - logger?.warning(`${label}: ${detail.error}`); + logger?.warning(`${label(plan)}: ${detail.error}`); } + // Until the Fetch API shows the new order, a pull shows the one it replaced: not a reorder. + const changed = (response?.itemsUpdated ?? 0) > 0; + settle(plan, plan.targetOrder.filter((id) => !skippedIDs.includes(id)), changed ? plan.replacedOrder : undefined); if ((response?.itemsUpdated ?? 0) > 0) { result.successful++; - logger?.success(`↕ Reordered ${label}: ${response.itemsUpdated} item(s) moved (${(response.updatedContentIDs || []).join(", ")})`); - console.log(ansiColors.green(` ✓ reordered ${label}: ${response.itemsUpdated} item(s) moved`)); + logger?.success(`↕ Reordered ${label(plan)}: ${response.itemsUpdated} item(s) moved (${(response.updatedContentIDs || []).join(", ")})`); + console.log(ansiColors.green(` ✓ reordered ${label(plan)}: ${response.itemsUpdated} item(s) moved`)); } else { result.skipped++; - logger?.info(`${label}: already in the source's order`); + logger?.info(`${label(plan)}: already in the source's order`); } } catch (error: any) { result.failed++; result.status = "error"; const message = error?.message || String(error); result.failureDetails!.push({ name: plan.targetReferenceName, error: `list order failed: ${message}`, type: "content", locale: plan.locale, guid: opts.targetGuid }); - logger?.error(`Failed to set the order of ${label}: ${message}`); - console.log(ansiColors.red(` ✗ ${label}: list order failed — ${message}`)); + logger?.error(`Failed to set the order of ${label(plan)}: ${message}`); + console.log(ansiColors.red(` ✗ ${label(plan)}: list order failed — ${message}`)); } } + // Merge the settled lists into each locale's records: replace by source list, keep the rest. + for (const [locale, updates] of Array.from(settled.entries())) { + const updated = new Set(updates.map((r) => lower(r.sourceReferenceName))); + const kept = (recordsByLocale.get(locale) || []).filter((r) => !updated.has(lower(r.sourceReferenceName))); + fileOps.saveMappingFile([...kept, ...updates], "listorder", opts.sourceGuid, opts.targetGuid, locale); + } + + const leftAlone = decisions.filter((d) => d.action === "keep" || d.action === "conflict").length; + const pending = decisions.filter((d) => d.action === "pending").length; console.log( ansiColors.gray(`List order: `) + ansiColors.green(`${result.successful} reordered, `) + - ansiColors.yellow(`${result.skipped} already in order, `) + + ansiColors.yellow(`${result.skipped - leftAlone - pending} already in order, `) + + (pending > 0 ? ansiColors.gray(`${pending} waiting for the Fetch API, `) : "") + + (leftAlone > 0 ? ansiColors.yellow(`${leftAlone} target order(s) kept or in conflict, `) : "") + (result.failed > 0 ? ansiColors.red : ansiColors.gray)(`${result.failed} failed`) + ansiColors.gray(` (${plans.length} list(s) differed from the source)\n`) ); diff --git a/src/lib/list-order/tests/sync-list-order.test.ts b/src/lib/list-order/tests/sync-list-order.test.ts index 6c932c6..8a5477b 100644 --- a/src/lib/list-order/tests/sync-list-order.test.ts +++ b/src/lib/list-order/tests/sync-list-order.test.ts @@ -4,7 +4,17 @@ import * as path from "path"; import { state, resetState, setState } from "../../../core/state"; import { preflightReport } from "../../preflight/preflight-report"; -import { ListItem, containerMapFrom, contentMapFrom, planListOrders, runSyncListOrder } from "../sync-list-order"; +import { + ListItem, + ListOrderRecord, + containerMapFrom, + contentMapFrom, + decideListOrders, + planListOrders, + relativeOrderChanged, + runSyncListOrder, +} from "../sync-list-order"; +import { keptTargetChanges } from "../../pushers/kept-target-changes"; const LOCALE = "en-us"; @@ -156,6 +166,86 @@ describe("planListOrders", () => { }); }); +describe("relativeOrderChanged", () => { + it("compares only the items both orders have", () => { + expect(relativeOrderChanged([1, 2, 3], [1, 2, 3])).toBe(false); + expect(relativeOrderChanged([1, 2, 3], [1, 3, 2])).toBe(true); + // An item added anywhere, or one deleted, isn't a reorder + expect(relativeOrderChanged([1, 2, 3], [4, 1, 2, 3])).toBe(false); + expect(relativeOrderChanged([1, 2, 3], [1, 3])).toBe(false); + expect(relativeOrderChanged([1, 2, 3], [3, 9, 1])).toBe(true); + }); +}); + +describe("decideListOrders (PROD-3078: which side reordered)", () => { + // The last run left source 1, 2, 3 and target 101, 102, 103. + const synced: ListOrderRecord = { + sourceReferenceName: "links", + targetReferenceName: "links", + sourceContentIDs: [1, 2, 3], + targetContentIDs: [101, 102, 103], + }; + const inSourceOrder = [item(1, 0), item(2, 1), item(3, 2)]; + const sourceReordered = [item(3, 0), item(1, 1), item(2, 2)]; + const inTargetOrder = [item(101, 0), item(102, 1), item(103, 2)]; + const targetReordered = [item(102, 0), item(101, 1), item(103, 2)]; + + const decide = (source: ListItem[], target: ListItem[], records: ListOrderRecord[] = [synced], overwrite = false) => + decideListOrders(LOCALE, source, target, contentMap(1, 2, 3), containerMap, undefined, new Set(), records, overwrite); + + it("in order: nothing to send, with the orders to record", () => { + expect(decide(inSourceOrder, inTargetOrder)).toEqual([ + expect.objectContaining({ action: "in-order", sourceOrder: [1, 2, 3], targetOrder: [101, 102, 103] }), + ]); + }); + + it("reordered on the source only: sets the source's order", () => { + expect(decide(sourceReordered, inTargetOrder)).toEqual([ + expect.objectContaining({ action: "reorder", unrecorded: false, contentIDs: [103, 101, 102], targetOrder: [103, 101, 102] }), + ]); + }); + + it("reordered on the target only: kept", () => { + expect(decide(inSourceOrder, targetReordered)).toEqual([expect.objectContaining({ action: "keep", targetOrder: [102, 101, 103] })]); + }); + + it("reordered on the target only, with --overwrite: sets the source's order", () => { + expect(decide(inSourceOrder, targetReordered, [synced], true)).toEqual([ + expect.objectContaining({ action: "reorder", overwritten: "keep", contentIDs: [101, 102, 103] }), + ]); + }); + + it("reordered on both sides: a conflict, or the source's order with --overwrite", () => { + expect(decide(sourceReordered, targetReordered)).toEqual([expect.objectContaining({ action: "conflict" })]); + expect(decide(sourceReordered, targetReordered, [synced], true)).toEqual([ + expect.objectContaining({ action: "reorder", overwritten: "conflict" }), + ]); + }); + + it("reordered on both sides to the same order: in order", () => { + expect(decide(sourceReordered, [item(103, 0), item(101, 1), item(102, 2)])).toEqual([ + expect.objectContaining({ action: "in-order" }), + ]); + }); + + it("an item added on the target, at the top, isn't a target reorder", () => { + const target = [item(900, 0), ...inTargetOrder.map((i) => item(i.contentID, i.properties!.itemOrder! + 1))]; + expect(decide(inSourceOrder, target)).toEqual([expect.objectContaining({ action: "reorder", unrecorded: false })]); + }); + + it("no record (first run after upgrading): the source's order, as before", () => { + expect(decide(inSourceOrder, targetReordered, [])).toEqual([ + expect.objectContaining({ action: "reorder", unrecorded: true, contentIDs: [101, 102, 103] }), + ]); + }); + + it("matches the record by source list name, case-insensitively", () => { + expect(decide(inSourceOrder, targetReordered, [{ ...synced, sourceReferenceName: "LINKS" }])).toEqual([ + expect.objectContaining({ action: "keep" }), + ]); + }); +}); + describe("mapping helpers", () => { it("contentMapFrom lets the last record for a source win and drops incomplete ones", () => { const map = contentMapFrom([ @@ -291,4 +381,145 @@ describe("runSyncListOrder", () => { expect(api.contentMethods.setContentListItemOrder).not.toHaveBeenCalled(); expect(result.successful).toBe(0); }); + describe("records (PROD-3078)", () => { + const recordFile = path.join(`mappings/${SRC}-${TGT}`, LOCALE, "listorder", "mappings.json"); + const readRecords = () => JSON.parse(fs.readFileSync(path.join(root, recordFile), "utf8")); + // What the default fixtures look like once synced: source 1, 2, 3; target 101, 102, 103, then 900. + const syncedRecord = { + sourceGuid: SRC, + targetGuid: TGT, + sourceReferenceName: "links", + targetReferenceName: "links", + sourceContentIDs: [1, 2, 3], + targetContentIDs: [101, 102, 103, 900], + }; + + beforeEach(() => keptTargetChanges.reset()); + + it("records the orders a reorder left, and the order it replaced (the first run, with no record)", async () => { + await run(); + + expect(readRecords()).toEqual([{ ...syncedRecord, targetReplacedContentIDs: [900, 103, 101, 102] }]); + }); + + it("a pull that still shows the replaced order is waited on, not kept as a target reorder", async () => { + // The last run reordered 900, 103, 101, 102 to 101, 102, 103, 900; the Fetch API hasn't shown it yet. + writeJson(recordFile, [{ ...syncedRecord, targetReplacedContentIDs: [900, 103, 101, 102] }]); + + const result = await run(); + + expect(api.contentMethods.setContentListItemOrder).not.toHaveBeenCalled(); + expect(keptTargetChanges.hasEntries()).toBe(false); + expect(result).toMatchObject({ successful: 0, skipped: 1 }); + expect(readRecords()).toEqual([{ ...syncedRecord, targetReplacedContentIDs: [900, 103, 101, 102] }]); + }); + + it("drops the replaced order once a pull shows the new one", async () => { + writeJson(recordFile, [{ ...syncedRecord, targetReplacedContentIDs: [900, 103, 101, 102] }]); + [[101, 0], [102, 1], [103, 2], [900, 3]].forEach(([id, order]) => + writeJson(`${TGT}/${LOCALE}/item/${id}.json`, { contentID: id, properties: { state: 2, referenceName: "links", itemOrder: order } }) + ); + + await run(); + + expect(readRecords()).toEqual([syncedRecord]); + }); + + it("a reverse sync doesn't put back an order its source shows before the last reorder reached the Fetch API", async () => { + // The last run (this way round) reordered the source side from 3, 1, 2 to 1, 2, 3 ... seen as + // a reverse sync's record: the source's replaced order is 3, 1, 2, and the source pull still shows it. + [3, 1, 2].forEach((n, i) => + writeJson(`${SRC}/${LOCALE}/item/${n}.json`, { contentID: n, properties: { state: 2, referenceName: "links", itemOrder: i } }) + ); + writeJson(recordFile, [{ ...syncedRecord, sourceReplacedContentIDs: [3, 1, 2] }]); + [[101, 0], [102, 1], [103, 2], [900, 3]].forEach(([id, order]) => + writeJson(`${TGT}/${LOCALE}/item/${id}.json`, { contentID: id, properties: { state: 2, referenceName: "links", itemOrder: order } }) + ); + + await run(); + + expect(api.contentMethods.setContentListItemOrder).not.toHaveBeenCalled(); + expect(keptTargetChanges.hasEntries()).toBe(false); + }); + + it("records a list that's already in order without calling the API", async () => { + [[101, 0], [102, 1], [103, 2], [900, 3]].forEach(([id, order]) => + writeJson(`${TGT}/${LOCALE}/item/${id}.json`, { contentID: id, properties: { state: 2, referenceName: "links", itemOrder: order } }) + ); + + await run(); + + expect(api.contentMethods.setContentListItemOrder).not.toHaveBeenCalled(); + expect(readRecords()).toEqual([syncedRecord]); + }); + + it("keeps a target-only reorder with a warning, and leaves the record alone", async () => { + // The fixtures' target (900, 103, 101, 102) is a reorder of the recorded 101, 102, 103. + writeJson(recordFile, [syncedRecord]); + + const result = await run(); + + expect(api.contentMethods.setContentListItemOrder).not.toHaveBeenCalled(); + expect(result).toMatchObject({ successful: 0, skipped: 1, failed: 0 }); + expect(keptTargetChanges.getEntries()).toEqual([{ type: "listOrder", name: "links", locale: LOCALE }]); + expect(readRecords()).toEqual([syncedRecord]); + }); + + it("with --overwrite sets the source's order over a target reorder, and records it", async () => { + writeJson(recordFile, [{ ...syncedRecord, targetContentIDs: [101, 102, 103] }]); + state.overwrite = true; + + await run(); + + expect(api.contentMethods.setContentListItemOrder).toHaveBeenCalledWith("links", TGT, LOCALE, [101, 102, 103], true); + expect(keptTargetChanges.hasEntries()).toBe(false); + expect(readRecords()).toEqual([{ ...syncedRecord, targetReplacedContentIDs: [900, 103, 101, 102] }]); + }); + + it("reports a reorder on both sides as a preflight conflict", async () => { + writeJson(recordFile, [{ ...syncedRecord, sourceContentIDs: [2, 1, 3] }]); + state.preflight = true; + + await run(); + + expect(api.contentMethods.setContentListItemOrder).not.toHaveBeenCalled(); + expect(preflightReport.hasConflicts()).toBe(true); + }); + + it("labels a first-run reorder in preflight as having no sync record", async () => { + state.preflight = true; + + await run(); + + expect(preflightReport.toJSON().entries).toEqual( + expect.arrayContaining([expect.objectContaining({ phase: "List order", detail: expect.stringContaining("no sync record yet") })]) + ); + }); + + it("reads a reverse sync's records transposed", async () => { + // The pair was first synced TGT -> SRC, so its mapping folder is TGT-SRC, written that way round. + const pairDir = `mappings/${TGT}-${SRC}`; + fs.rmSync(path.join(root, `mappings/${SRC}-${TGT}`), { recursive: true, force: true }); + writeJson(`${pairDir}/${LOCALE}/item/mappings.json`, [1, 2, 3].map((n) => ({ sourceContentID: 100 + n, targetContentID: n }))); + writeJson(`${pairDir}/containers/mappings.json`, [{ sourceReferenceName: "links", targetReferenceName: "links" }]); + writeJson(`${pairDir}/${LOCALE}/listorder/mappings.json`, [ + { + sourceGuid: TGT, + targetGuid: SRC, + sourceReferenceName: "links", + targetReferenceName: "links", + sourceContentIDs: [101, 102, 103, 900], + targetContentIDs: [1, 2, 3], + }, + ]); + state.reverseSync = true; + state.mappingPair = { sourceGuid: TGT, targetGuid: SRC }; + + await run(); + + // Read this way round: source 1, 2, 3 unchanged; target 900, 103, 101, 102 reordered since 101, 102, 103, 900. + expect(keptTargetChanges.getEntries()).toEqual([expect.objectContaining({ type: "listOrder", name: "links" })]); + expect(api.contentMethods.setContentListItemOrder).not.toHaveBeenCalled(); + }); + }); }); diff --git a/src/lib/mappers/template-mapper.ts b/src/lib/mappers/template-mapper.ts index db57f1d..14307fb 100644 --- a/src/lib/mappers/template-mapper.ts +++ b/src/lib/mappers/template-mapper.ts @@ -2,7 +2,9 @@ import { fileOperations } from "../../core"; import { getSavedTarget } from "../../core/state"; import * as mgmtApi from "@agility/management-sdk"; import { SectionMapper } from "./section-mapper"; -interface TemplateMapping { +import { hashShape, SyncFingerprint } from "../shared/sync-fingerprint"; + +interface TemplateMapping extends SyncFingerprint { sourceGuid: string; targetGuid: string; sourcePageTemplateID: number; @@ -70,7 +72,11 @@ export class TemplateMapper { return templateData as mgmtApi.PageModel; } - addMapping(sourceTemplate: mgmtApi.PageModel, targetTemplate: mgmtApi.PageModel) { + /** + * Add or update the record for a source template. `fingerprint` is the hash of both sides once + * they agree (PROD-3078); leave it out to keep whatever the record has. + */ + addMapping(sourceTemplate: mgmtApi.PageModel, targetTemplate: mgmtApi.PageModel, fingerprint?: SyncFingerprint) { const targetMapping = this.getTemplateMapping(targetTemplate, "target"); const sourceMapping = this.getTemplateMapping(sourceTemplate, "source"); @@ -81,6 +87,7 @@ export class TemplateMapper { } if (targetMapping) { + if (fingerprint) Object.assign(targetMapping, fingerprint); this.updateMapping(sourceTemplate, targetTemplate, targetMapping); } else if (sourceMapping) { // PROD-2603: the source template already has a record but its target is gone (the pusher @@ -92,6 +99,7 @@ export class TemplateMapper { sourceMapping.targetPageTemplateID = targetTemplate.pageTemplateID; sourceMapping.sourcePageTemplateName = sourceTemplate.pageTemplateName; sourceMapping.targetPageTemplateName = targetTemplate.pageTemplateName; + if (fingerprint) Object.assign(sourceMapping, fingerprint); } else { const newMapping: TemplateMapping = { sourceGuid: this.sourceGuid, @@ -100,6 +108,7 @@ export class TemplateMapper { targetPageTemplateID: targetTemplate.pageTemplateID, sourcePageTemplateName: sourceTemplate.pageTemplateName, targetPageTemplateName: targetTemplate.pageTemplateName, + ...(fingerprint ?? {}), }; this.mappings.push(newMapping); @@ -132,6 +141,12 @@ export class TemplateMapper { this.fileOps.saveMappingFile(this.mappings, this.directory, this.sourceGuid, this.targetGuid); } + /** Store the hashes of both sides on an existing record, once they agree (PROD-3078). */ + recordFingerprint(mapping: TemplateMapping, fingerprint: SyncFingerprint) { + Object.assign(mapping, fingerprint); + this.saveMapping(); + } + // Templates have no lastModifiedDate, so change detection compares the source and target // structure directly. Per-instance IDs (pageItemTemplateID, pageTemplateID, contentViewID, // contentDefinitionID, itemContainerID) are excluded — they always differ between instances. @@ -190,3 +205,29 @@ export class TemplateMapper { return false; } } + +/** + * The fields hasTemplateChanged compares, without per-instance IDs, with sections in a stable + * order. Hashed, it is the fingerprint stored in the mapping to tell which side of a template + * changed since the last sync (PROD-3078, see sync-fingerprint.ts). + */ +export function normalizeTemplate(template: mgmtApi.PageModel) { + const sections = (template.contentSectionDefinitions || []) + .filter(Boolean) + .map((s) => ({ + referenceName: s.pageItemTemplateReferenceName ?? null, + name: s.pageItemTemplateName ?? null, + type: s.pageItemTemplateType ?? null, + itemOrder: s.itemOrder ?? null, + })) + .sort( + (a, b) => + (a.itemOrder ?? Number.MAX_SAFE_INTEGER) - (b.itemOrder ?? Number.MAX_SAFE_INTEGER) || + String(a.referenceName ?? "").localeCompare(String(b.referenceName ?? "")) + ); + return { name: template.pageTemplateName ?? null, sections }; +} + +export function templateHash(template: mgmtApi.PageModel): string { + return hashShape(normalizeTemplate(template)); +} diff --git a/src/lib/mappers/url-redirection-mapper.ts b/src/lib/mappers/url-redirection-mapper.ts index e9b987d..88c3068 100644 --- a/src/lib/mappers/url-redirection-mapper.ts +++ b/src/lib/mappers/url-redirection-mapper.ts @@ -1,6 +1,7 @@ import { fileOperations } from "../../core"; +import { SyncFingerprint } from "../shared/sync-fingerprint"; -export interface UrlRedirectionMapping { +export interface UrlRedirectionMapping extends SyncFingerprint { sourceGuid: string; targetGuid: string; sourceUrlRedirectionID: number; @@ -13,8 +14,8 @@ export interface UrlRedirectionMapping { * Maps source URL redirection IDs to target IDs across syncs. * * Unlike galleries/models, redirections carry no modifiedOn timestamp in the pulled - * data, so this mapper only tracks the ID pairing. Change detection is done by the - * pusher via a direct field comparison of the pulled source and target items. + * data. Besides the ID pairing, each record holds a hash of both sides as last synced + * (PROD-3078, see sync-fingerprint.ts), which tells the pusher which side changed since. */ export class UrlRedirectionMapper { private fileOps: fileOperations; @@ -39,7 +40,16 @@ export class UrlRedirectionMapper { return mapping; } - addMapping(sourceUrlRedirectionID: number, targetUrlRedirectionID: number, originUrl: string) { + /** + * Add or update the record for a source redirection. `fingerprint` is the hash of both sides + * once they agree; leave it out when they don't yet (e.g. a record adopted before its update). + */ + addMapping( + sourceUrlRedirectionID: number, + targetUrlRedirectionID: number, + originUrl: string, + fingerprint?: SyncFingerprint + ) { const existing = this.getMapping(sourceUrlRedirectionID, "source"); if (existing) { @@ -47,6 +57,7 @@ export class UrlRedirectionMapper { existing.targetGuid = this.targetGuid; existing.targetUrlRedirectionID = targetUrlRedirectionID; existing.originUrl = originUrl; + if (fingerprint) Object.assign(existing, fingerprint); } else { this.mappings.push({ sourceGuid: this.sourceGuid, @@ -54,6 +65,7 @@ export class UrlRedirectionMapper { sourceUrlRedirectionID, targetUrlRedirectionID, originUrl, + ...(fingerprint ?? {}), }); } diff --git a/src/lib/pushers/container-pusher.ts b/src/lib/pushers/container-pusher.ts index 468a33a..079fba1 100644 --- a/src/lib/pushers/container-pusher.ts +++ b/src/lib/pushers/container-pusher.ts @@ -1,6 +1,6 @@ import * as mgmtApi from "@agility/management-sdk"; import { ApiClient } from "@agility/management-sdk"; -import { getLoggerForGuid, state, recordSavedTarget } from "core/state"; +import { getLoggerForGuid, state, recordSavedTarget, getSavedTarget } from "core/state"; import { ContainerMapper } from "lib/mappers/container-mapper"; import { ModelMapper } from "lib/mappers/model-mapper"; import { Logs } from "core/logs"; @@ -69,6 +69,10 @@ export async function pushContainers( targetModelID = modelMapping.targetID; } } + // Preflight: a model this run would create has no mapping (nothing is written), but the model + // pusher recorded it as planned. Its containers would be created with it, so plan them too. + const modelPlanned = + state.preflight && targetModelID < 1 && !!getSavedTarget("model", sourceContainer.contentDefinitionID); try { // STEP 1: Find existing mapping @@ -248,7 +252,7 @@ export async function pushContainers( if (shouldCreate) { // Container doesn't exist - create new one - if (targetModelID < 1) { + if (targetModelID < 1 && !modelPlanned) { logger.container.skipped(sourceContainer, "Target model mapping not found", targetGuid); preflightReport.record({ phase: "Containers", @@ -258,8 +262,10 @@ export async function pushContainers( }); skipped++; } else if (state.preflight) { - // Preflight: would create, but skip the write. + // Preflight: would create, but skip the write. Record it as planned, so the content phase + // plans the container's new items too (hasValidMappings). preflightReport.record({ phase: "Containers", action: "create", name: sourceContainer.referenceName }); + recordSavedTarget("container", sourceContainer.contentViewID, sourceContainer, sourceContainer.referenceName); successful++; } else { // Container doesn't exist - create new one diff --git a/src/lib/pushers/content-pusher/util/has-valid-mappings.ts b/src/lib/pushers/content-pusher/util/has-valid-mappings.ts index d082093..47ece2b 100644 --- a/src/lib/pushers/content-pusher/util/has-valid-mappings.ts +++ b/src/lib/pushers/content-pusher/util/has-valid-mappings.ts @@ -1,9 +1,12 @@ import { ContentItem } from "@agility/management-sdk"; import { ContainerMapper } from "lib/mappers/container-mapper"; import { ModelMapper } from "lib/mappers/model-mapper"; +import { getSavedTargetBySourceReferenceName, state } from "core/state"; /** - * Checks if a content item has valid container and model mappings + * Checks if a content item has valid container and model mappings. In preflight, a container or + * model the run would create has no mapping (nothing is written), so one recorded as planned by its + * pusher counts too: its new items are reported as planned creates instead of silently left out. */ export function hasValidMappings( item: ContentItem, @@ -22,5 +25,9 @@ export function hasValidMappings( ); const sourceModel = modelMapper.getMappedEntity(sourceModelMapping, "source"); - return !!(sourceContainer && sourceModel); + if (sourceContainer && sourceModel) return true; + if (!state.preflight) return false; + const container = sourceContainer || getSavedTargetBySourceReferenceName("container", item.properties.referenceName); + const model = sourceModel || getSavedTargetBySourceReferenceName("model", item.properties.definitionName); + return !!(container && model); } diff --git a/src/lib/pushers/content-pusher/util/tests/has-valid-mappings.test.ts b/src/lib/pushers/content-pusher/util/tests/has-valid-mappings.test.ts index e03ac17..6c4a9f9 100644 --- a/src/lib/pushers/content-pusher/util/tests/has-valid-mappings.test.ts +++ b/src/lib/pushers/content-pusher/util/tests/has-valid-mappings.test.ts @@ -1,4 +1,4 @@ -import { resetState } from "core/state"; +import { resetState, recordSavedTarget, state } from "core/state"; import { hasValidMappings } from "../has-valid-mappings"; jest.mock("lib/mappers/container-mapper", () => ({ @@ -128,3 +128,31 @@ describe("hasValidMappings — case insensitivity", () => { expect(modelMapper.getModelMappingByReferenceName).toHaveBeenCalledWith(defName.toLowerCase(), "source"); }); }); + +// ─── preflight: planned container and model ─────────────────────────────────── + +describe("hasValidMappings — preflight, a container and model the run would create", () => { + const unmapped = () => [makeContainerMapper(null, null), makeModelMapper(null, null)] as const; + + it("counts both as valid once their pushers recorded them as planned", () => { + state.preflight = true; + recordSavedTarget("model", 10, { id: 10, referenceName: "MyModel" }, "MyModel"); + recordSavedTarget("container", 1, { contentViewID: 1, referenceName: "my-container" }, "my-container"); + const [containerMapper, modelMapper] = unmapped(); + expect(hasValidMappings(makeItem(), containerMapper, modelMapper)).toBe(true); + }); + + it("is still invalid when only the model is planned", () => { + state.preflight = true; + recordSavedTarget("model", 10, { id: 10, referenceName: "MyModel" }, "MyModel"); + const [containerMapper, modelMapper] = unmapped(); + expect(hasValidMappings(makeItem(), containerMapper, modelMapper)).toBe(false); + }); + + it("ignores planned entities outside preflight", () => { + recordSavedTarget("model", 10, { id: 10, referenceName: "MyModel" }, "MyModel"); + recordSavedTarget("container", 1, { contentViewID: 1, referenceName: "my-container" }, "my-container"); + const [containerMapper, modelMapper] = unmapped(); + expect(hasValidMappings(makeItem(), containerMapper, modelMapper)).toBe(false); + }); +}); diff --git a/src/lib/pushers/kept-target-changes.ts b/src/lib/pushers/kept-target-changes.ts index b75c9e3..9d688a7 100644 --- a/src/lib/pushers/kept-target-changes.ts +++ b/src/lib/pushers/kept-target-changes.ts @@ -9,7 +9,16 @@ * is still a conflict, decided by each pusher. A process-level singleton, like preflightReport. */ -export type KeptTargetChangeType = "content" | "asset" | "gallery" | "container" | "model" | "page"; +export type KeptTargetChangeType = + | "content" + | "asset" + | "gallery" + | "container" + | "model" + | "page" + | "template" + | "urlRedirection" + | "listOrder"; export interface KeptTargetChange { type: KeptTargetChangeType; @@ -50,6 +59,9 @@ const TYPE_LABELS: Record = { container: "container", model: "model", page: "page", + template: "page template", + urlRedirection: "URL redirection", + listOrder: "list order", }; const TYPE_PLURALS: Record = { @@ -59,6 +71,9 @@ const TYPE_PLURALS: Record = { container: "containers", model: "models", page: "pages", + template: "page templates", + urlRedirection: "URL redirections", + listOrder: "list orders", }; class KeptTargetChanges { diff --git a/src/lib/pushers/page-pusher/process-page.ts b/src/lib/pushers/page-pusher/process-page.ts index 3f1bf59..c9b21ee 100644 --- a/src/lib/pushers/page-pusher/process-page.ts +++ b/src/lib/pushers/page-pusher/process-page.ts @@ -86,12 +86,13 @@ export async function processPage({ }); return { status: "skip" }; } - // PROD-2603: the template pusher held this page's template back because its mapped target - // template is gone and --overwrite was not set. Saving the page would point it at the dead - // target template ID, so skip it; it pushes once the template is recreated. + // The template pusher held this page's template back, and --overwrite was not set: its mapped + // target template is gone (PROD-2603), so saving would point the page at a dead ID; or the + // target's template was left as it is (a kept change or a conflict, PROD-3078) without a + // section for one of the source's zones. Skip it; the reason says why. const heldBackReason = getHeldBackTemplate(page.templateName); if (heldBackReason) { - const detail = `page template ${page.templateName} is missing on the target and was not recreated (use --overwrite to recreate it)`; + const detail = heldBackReason; logger.page.skipped(page, `${detail}, skipping`, locale, channel, targetGuid); preflightReport.record({ phase: "Pages", action: "skip", name: page.name, locale, detail }); return { status: "skip" }; diff --git a/src/lib/pushers/template-pusher.ts b/src/lib/pushers/template-pusher.ts index 7fde680..43c143a 100644 --- a/src/lib/pushers/template-pusher.ts +++ b/src/lib/pushers/template-pusher.ts @@ -1,11 +1,38 @@ import * as mgmtApi from "@agility/management-sdk"; import { state, getLoggerForGuid, registerHeldBackTemplate, recordSavedTarget } from "../../core/state"; -import { TemplateMapper } from "lib/mappers/template-mapper"; +import { TemplateMapper, templateHash } from "lib/mappers/template-mapper"; import { ModelMapper } from "lib/mappers/model-mapper"; import { ContainerMapper } from "lib/mappers/container-mapper"; import { SectionMapper } from "lib/mappers/section-mapper"; import { FailureDetail, PusherResult } from "types/sourceData"; import { preflightReport } from "../preflight/preflight-report"; +import { keptTargetChanges } from "./kept-target-changes"; +import { decideSync, fingerprintFields, hasFingerprint, UNRECORDED_UPDATE_DETAIL } from "../shared/sync-fingerprint"; +import ansiColors from "ansi-colors"; + +/** + * Source sections with no counterpart on the target template, by the section ID mapping or by + * reference name: the zones translateZoneNames could only place by position or fold into the main + * zone. Used when the target's template is left as it is (a kept target change or a conflict). + */ +function unresolvedSections( + sourceTemplate: mgmtApi.PageModel, + targetTemplate: mgmtApi.PageModel, + sectionMapper: SectionMapper +): string[] { + const targetSections = targetTemplate.contentSectionDefinitions || []; + return (sourceTemplate.contentSectionDefinitions || []) + .filter((sourceSection) => { + if (!sourceSection) return false; + const mapping = + sourceSection.pageItemTemplateID != null + ? sectionMapper.getSectionMappingByID(sourceSection.pageItemTemplateID, "source") + : null; + if (mapping && targetSections.some((t) => t?.pageItemTemplateID === mapping.targetPageItemTemplateID)) return false; + return !targetSections.some((t) => t?.pageItemTemplateReferenceName === sourceSection.pageItemTemplateReferenceName); + }) + .map((s) => s.pageItemTemplateReferenceName); +} export async function pushTemplates( @@ -80,19 +107,82 @@ export async function pushTemplates( name: sourceTemplate.pageTemplateName, detail: reason, }); - registerHeldBackTemplate(sourceTemplate.pageTemplateName, reason); + registerHeldBackTemplate( + sourceTemplate.pageTemplateName, + `page template ${sourceTemplate.pageTemplateName} is missing on the target and was not recreated (use --overwrite to recreate it)` + ); skipped++; processedCount++; continue; } - // Templates have no lastModifiedDate, so compare the source and target - // structure directly: identical -> skip, different -> update (source wins), - // mapped but missing on target (only reached with --overwrite) -> fall through and recreate. + // Templates have no version or modified date. Whether the two sides differ is a structural + // comparison; which side changed since the last sync comes from the hashes stored in the + // mapping (PROD-3078, see sync-fingerprint.ts). A mapping with no hashes yet (older CLI) lets + // the source win, as before. Mapped but missing on the target (only reached with --overwrite): + // fall through and recreate. const templateChanged = templateMapper.hasTemplateChanged(sourceTemplate, targetTemplate, sectionMapper); + const sourceHash = templateHash(sourceTemplate); + const targetHash = targetTemplate ? templateHash(targetTemplate) : null; + const decision = + existingMapping && targetTemplate ? decideSync(!templateChanged, sourceHash, targetHash!, existingMapping) : null; + + if ((decision === "keep" || decision === "conflict") && !state.overwrite) { + // The target's template stays as it is. Pages that use it are still synced against it, + // unless one of the source's zones has no section on it to go to: placing those by + // position or folding them into the main zone would move components, so hold them back. + const missing = unresolvedSections(sourceTemplate, targetTemplate!, sectionMapper); + if (missing.length > 0) { + registerHeldBackTemplate( + sourceTemplate.pageTemplateName, + `page template ${sourceTemplate.pageTemplateName} was ${decision === "keep" ? "changed on the target and kept" : "changed on both sides (a conflict)"}, ` + + `and has no section for zone(s) ${missing.join(", ")} (use --overwrite to replace it with the source)` + ); + } + const heldBack = missing.length > 0 ? `; pages using it are held back (no section for ${missing.join(", ")})` : ""; + + if (decision === "keep") { + const change = { + type: "template" as const, + name: sourceTemplate.pageTemplateName, + sourceID: sourceTemplate.pageTemplateID, + targetID: targetTemplate!.pageTemplateID, + }; + keptTargetChanges.record(change); + console.warn(ansiColors.yellow(keptTargetChanges.formatLine(change))); + logger.template.skipped(sourceTemplate, `target changed since the last sync, kept${heldBack}`, targetGuid); + preflightReport.record({ + phase: "Templates", + action: "skip", + name: sourceTemplate.pageTemplateName, + detail: `target changed since the last sync and the source didn't; kept the target's version${heldBack}`, + }); + } else { + const reason = `source and target both changed since the last sync; use --overwrite to replace the target${heldBack}`; + console.warn(`⚠️ Conflict detected Template ${sourceTemplate.pageTemplateName} — ${reason}`); + logger.template.skipped(sourceTemplate, `conflict: ${reason}`, targetGuid); + preflightReport.record({ + phase: "Templates", + action: "conflict", + name: sourceTemplate.pageTemplateName, + detail: `source and target both changed; use --overwrite to force${heldBack}`, + }); + } + skipped++; + processedCount++; + continue; + } const shouldUpdate = existingMapping !== null && targetTemplate !== null && templateChanged; const shouldSkip = existingMapping !== null && targetTemplate !== null && !templateChanged; + const updateDetail = + decision === "update-unrecorded" + ? UNRECORDED_UPDATE_DETAIL + : decision === "keep" + ? "target change overwritten" + : decision === "conflict" + ? "conflict overwritten" + : undefined; if (shouldSkip) { // Backfill the section ID mapping for templates that are already in sync but predate @@ -131,6 +221,16 @@ export async function pushTemplates( } } + // Seed or refresh the hashes: an older mapping gets them here, and two sides edited to the + // same structure agree again. + if ( + !hasFingerprint(existingMapping) || + existingMapping.sourceHash !== sourceHash || + existingMapping.targetHash !== targetHash + ) { + templateMapper.recordFingerprint(existingMapping, fingerprintFields(sourceHash, targetHash!)); + } + logger.template.skipped(sourceTemplate, "Up to date, skipping", targetGuid); preflightReport.record({ phase: "Templates", @@ -146,6 +246,7 @@ export async function pushTemplates( phase: "Templates", action: shouldUpdate ? "update" : "create", name: sourceTemplate.pageTemplateName, + ...(shouldUpdate && updateDetail ? { detail: updateDetail } : {}), }); successful++; } @@ -210,7 +311,10 @@ export async function pushTemplates( try { const savedTemplate = await apiClient.pageMethods.savePageTemplate(targetGuid, locale, payload); - templateMapper.addMapping(sourceTemplate, savedTemplate); + // The target's hash comes from what the server saved, not from what was sent: if it + // normalises a compared field (renumbering itemOrder, say), hashing the payload would make + // the next pull look like a target edit and keep it with a false warning on every run. + templateMapper.addMapping(sourceTemplate, savedTemplate, fingerprintFields(sourceHash, templateHash(savedTemplate))); // The page pusher translates zone names against this template; the pulled copy lacks any zone added here. recordSavedTarget("template", sourceTemplate.pageTemplateID, savedTemplate, sourceTemplate.pageTemplateName); diff --git a/src/lib/pushers/tests/container-pusher.test.ts b/src/lib/pushers/tests/container-pusher.test.ts index f8534b6..b4701f3 100644 --- a/src/lib/pushers/tests/container-pusher.test.ts +++ b/src/lib/pushers/tests/container-pusher.test.ts @@ -112,6 +112,43 @@ describe("pushContainers — create path: no model mapping", () => { expect(result.skipped).toBe(1); expect(saveContainer).not.toHaveBeenCalled(); }); + + it("in preflight, plans the container of a model the run would create, and records it as planned", async () => { + const saveContainer = jest.fn(); + state.cachedApiClient = { containerMethods: { saveContainer } } as any; + state.preflight = true; + const { recordSavedTarget, getSavedTargetBySourceReferenceName } = await import("core/state"); + // what the model pusher records for a model it would create (keyed by the source model ID) + recordSavedTarget("model", 9998, { id: 9998, referenceName: "NewModel" }, "NewModel"); + const { preflightReport } = await import("lib/preflight/preflight-report"); + preflightReport.reset(); + + const { pushContainers } = await import("../container-pusher"); + const sourceContainer = makeContainer({ contentDefinitionID: 9998, referenceName: "NewModelList" }); + const result = await pushContainers([sourceContainer], []); + + expect(saveContainer).not.toHaveBeenCalled(); + expect(result.successful).toBe(1); + expect(preflightReport.getEntries()).toEqual([ + expect.objectContaining({ phase: "Containers", action: "create", name: "NewModelList" }), + ]); + expect(getSavedTargetBySourceReferenceName("container", "newmodellist")).toBe(sourceContainer); + }); + + it("in preflight, still skips a container whose model is neither mapped nor planned", async () => { + state.cachedApiClient = { containerMethods: { saveContainer: jest.fn() } } as any; + state.preflight = true; + const { preflightReport } = await import("lib/preflight/preflight-report"); + preflightReport.reset(); + + const { pushContainers } = await import("../container-pusher"); + const result = await pushContainers([makeContainer({ contentDefinitionID: 9997 })], []); + + expect(result.skipped).toBe(1); + expect(preflightReport.getEntries()).toEqual([ + expect.objectContaining({ action: "skip", detail: "target model mapping not found" }), + ]); + }); }); // ─── pushContainers — special case: contentDefinitionID === 1 ───────────────── diff --git a/src/lib/pushers/tests/kept-target-changes.test.ts b/src/lib/pushers/tests/kept-target-changes.test.ts index 0c599c3..e92a334 100644 --- a/src/lib/pushers/tests/kept-target-changes.test.ts +++ b/src/lib/pushers/tests/kept-target-changes.test.ts @@ -67,6 +67,9 @@ describe("keptTargetChanges.headline", () => { ["container", "1 container"], ["model", "1 model"], ["page", "1 page"], + ["template", "1 page template"], + ["urlRedirection", "1 URL redirection"], + ["listOrder", "1 list order"], ] as const)("uses the singular label for one %s", (type, label) => { keptTargetChanges.record({ type, name: "n", sourceID: 1 }); diff --git a/src/lib/pushers/tests/template-pusher.test.ts b/src/lib/pushers/tests/template-pusher.test.ts index 194b61b..e3568d7 100644 --- a/src/lib/pushers/tests/template-pusher.test.ts +++ b/src/lib/pushers/tests/template-pusher.test.ts @@ -483,7 +483,7 @@ describe("pushTemplates — stale mapping to a deleted target template (PROD-260 expect(record.targetPageTemplateID).toBe(2); const { getHeldBackTemplate } = await import("core/state"); - expect(getHeldBackTemplate("LeftSideBarTemplate")).toMatch(/no longer exists on the target/); + expect(getHeldBackTemplate("LeftSideBarTemplate")).toMatch(/is missing on the target and was not recreated/); }); it("is reported as a preflight conflict without --overwrite", async () => { @@ -587,3 +587,254 @@ describe("pushTemplates — saved template is what the mapper returns for the re ]); }); }); + +// ─── PROD-3078: which side changed, from the hashes stored at the last sync ────────── + +describe("pushTemplates — sync fingerprints (PROD-3078)", () => { + let nextID = 5000; + + function section(id: number, referenceName: string, itemOrder: number, overrides: Record = {}): any { + return { + pageItemTemplateID: id, + pageItemTemplateReferenceName: referenceName, + pageItemTemplateName: referenceName, + pageItemTemplateType: 0, + itemOrder, + ...overrides, + }; + } + + /** + * A source/target template pair as the last sync left it: mapped, sections mapped, and (unless + * `unrecorded`) the hashes of both sides stored. + */ + async function syncedPair(name: string, { unrecorded = false } = {}) { + const ids = { source: nextID++, target: nextID++, sMain: nextID++, sSide: nextID++, tMain: nextID++, tSide: nextID++ }; + const source = makeTemplate({ + pageTemplateID: ids.source, + pageTemplateName: name, + contentSectionDefinitions: [section(ids.sMain, "Main", 1), section(ids.sSide, "Side", 2)], + }); + const target = makeTemplate({ + pageTemplateID: ids.target, + pageTemplateName: name, + contentSectionDefinitions: [section(ids.tMain, "Main", 1), section(ids.tSide, "Side", 2)], + }); + const { templateHash } = await import("lib/mappers/template-mapper"); + const { fingerprintFields } = await import("lib/shared/sync-fingerprint"); + new TemplateMapper("src-tpl-u", "tgt-tpl-u").addMapping( + source, + target, + unrecorded ? undefined : fingerprintFields(templateHash(source), templateHash(target)) + ); + const sections = new SectionMapper("src-tpl-u", "tgt-tpl-u"); + sections.addMapping(source.contentSectionDefinitions[0], target.contentSectionDefinitions[0]); + sections.addMapping(source.contentSectionDefinitions[1], target.contentSectionDefinitions[1]); + return { source, target }; + } + + function edit(template: any, fn: (sections: any[]) => void): any { + const copy = JSON.parse(JSON.stringify(template)); + fn(copy.contentSectionDefinitions); + return copy; + } + + const record = (sourceID: number) => + new TemplateMapper("src-tpl-u", "tgt-tpl-u").getTemplateMappingByPageTemplateID(sourceID, "source")!; + + async function run(source: any, target: any) { + const savePageTemplate = jest.fn().mockImplementation(async (_guid, _locale, payload) => ({ + ...payload, + pageTemplateID: target.pageTemplateID, + contentSectionDefinitions: payload.contentSectionDefinitions.map((s: any, i: number) => ({ + ...s, + pageItemTemplateID: s.pageItemTemplateID > 0 ? s.pageItemTemplateID : 9000 + i, + })), + })); + state.cachedApiClient = { pageMethods: { savePageTemplate } } as any; + const { keptTargetChanges } = await import("../kept-target-changes"); + const { preflightReport } = await import("lib/preflight/preflight-report"); + const { clearHeldBackTemplateRegistry } = await import("core/state"); + keptTargetChanges.reset(); + preflightReport.reset(); + clearHeldBackTemplateRegistry(); + const { pushTemplates } = await import("../template-pusher"); + const result = await pushTemplates([source], [target], "en-us"); + return { result, savePageTemplate, keptTargetChanges, preflightReport }; + } + + it("ignores IDs and section array order", async () => { + const { templateHash } = await import("lib/mappers/template-mapper"); + const { source, target } = await syncedPair("HashShape"); + const reversed = edit(target, (s) => s.reverse()); + expect(templateHash(source)).toBe(templateHash(reversed)); + }); + + it("source changed only: updates and records the source's hash for both sides", async () => { + const { source, target } = await syncedPair("SourceOnly"); + const changed = edit(source, (s) => s.push(section(nextID++, "Footer", 3))); + + const { result, savePageTemplate } = await run(changed, target); + + expect(savePageTemplate).toHaveBeenCalledTimes(1); + expect(result.successful).toBe(1); + const { templateHash } = await import("lib/mappers/template-mapper"); + expect(record(source.pageTemplateID)).toEqual( + expect.objectContaining({ sourceHash: templateHash(changed), targetHash: templateHash(changed) }) + ); + }); + + it("records the target's hash from what the server saved, so its normalising isn't read as a target edit", async () => { + const { source, target } = await syncedPair("ServerNormalises"); + const changed = edit(source, (s) => (s[0].pageItemTemplateName = "Main (source)")); + const { templateHash } = await import("lib/mappers/template-mapper"); + const { pushTemplates } = await import("../template-pusher"); + const { keptTargetChanges } = await import("../kept-target-changes"); + keptTargetChanges.reset(); + // The server renumbers itemOrder on save, and the next pull shows the same. + const savePageTemplate = jest.fn().mockImplementation(async (_guid, _locale, payload) => ({ + ...payload, + pageTemplateID: target.pageTemplateID, + contentSectionDefinitions: payload.contentSectionDefinitions.map((s: any) => ({ ...s, itemOrder: s.itemOrder * 10 })), + })); + state.cachedApiClient = { pageMethods: { savePageTemplate } } as any; + + await pushTemplates([changed], [target], "en-us"); + const saved = await savePageTemplate.mock.results[0].value; + expect(record(source.pageTemplateID).targetHash).toBe(templateHash(saved)); + + await pushTemplates([changed], [saved], "en-us"); + + expect(keptTargetChanges.hasEntries()).toBe(false); + }); + + it("target changed only: keeps the target, warns, leaves the record alone, and doesn't hold pages back", async () => { + const { source, target } = await syncedPair("TargetOnly"); + const before = { ...record(source.pageTemplateID) }; + const changed = edit(target, (s) => s.push(section(nextID++, "TargetExtra", 3))); + + const { result, savePageTemplate, keptTargetChanges } = await run(source, changed); + + expect(savePageTemplate).not.toHaveBeenCalled(); + expect(result.skipped).toBe(1); + expect(keptTargetChanges.getEntries()).toEqual([ + expect.objectContaining({ type: "template", name: "TargetOnly", targetID: target.pageTemplateID }), + ]); + expect(record(source.pageTemplateID)).toEqual(before); + // Every source zone still has a section on the kept target template + const { getHeldBackTemplate } = await import("core/state"); + expect(getHeldBackTemplate("TargetOnly")).toBeUndefined(); + }); + + it("target changed only, removing a section the source's pages use: pages are held back", async () => { + const { source, target } = await syncedPair("TargetDroppedZone"); + const changed = edit(target, (s) => s.splice(1, 1)); + + await run(source, changed); + + const { getHeldBackTemplate } = await import("core/state"); + expect(getHeldBackTemplate("TargetDroppedZone")).toMatch(/changed on the target and kept.*no section for zone\(s\) Side/); + }); + + it("a section renamed on the target still resolves through the section mapping: pages aren't held back", async () => { + const { source, target } = await syncedPair("TargetRenamedZone"); + const changed = edit(target, (s) => (s[1].pageItemTemplateReferenceName = "Sidebar")); + + const { keptTargetChanges } = await run(source, changed); + + expect(keptTargetChanges.hasEntries()).toBe(true); + const { getHeldBackTemplate } = await import("core/state"); + expect(getHeldBackTemplate("TargetRenamedZone")).toBeUndefined(); + }); + + it("target changed only, with --overwrite: the source replaces it", async () => { + state.overwrite = true; + const { source, target } = await syncedPair("TargetOnlyOverwrite"); + const changed = edit(target, (s) => s.push(section(nextID++, "TargetExtra", 3))); + + const { savePageTemplate, keptTargetChanges } = await run(source, changed); + + expect(savePageTemplate).toHaveBeenCalledTimes(1); + expect(savePageTemplate.mock.calls[0][2].contentSectionDefinitions).toHaveLength(2); + expect(keptTargetChanges.hasEntries()).toBe(false); + }); + + it("both changed: a preflight conflict, nothing saved, record left alone", async () => { + const { source, target } = await syncedPair("BothChanged"); + const before = { ...record(source.pageTemplateID) }; + state.preflight = true; + + const { savePageTemplate, preflightReport, keptTargetChanges } = await run( + edit(source, (s) => (s[0].pageItemTemplateName = "Main (source)")), + edit(target, (s) => (s[0].pageItemTemplateName = "Main (target)")) + ); + + expect(savePageTemplate).not.toHaveBeenCalled(); + expect(preflightReport.getEntries()).toEqual([ + expect.objectContaining({ phase: "Templates", action: "conflict", name: "BothChanged" }), + ]); + expect(keptTargetChanges.hasEntries()).toBe(false); + expect(record(source.pageTemplateID)).toEqual(before); + }); + + it("both changed, with --overwrite: the source wins", async () => { + state.overwrite = true; + const { source, target } = await syncedPair("BothChangedOverwrite"); + + const { savePageTemplate } = await run( + edit(source, (s) => (s[0].pageItemTemplateName = "Main (source)")), + edit(target, (s) => (s[0].pageItemTemplateName = "Main (target)")) + ); + + expect(savePageTemplate.mock.calls[0][2].contentSectionDefinitions[0].pageItemTemplateName).toBe("Main (source)"); + }); + + it("both changed to the same structure: skipped, and the hashes refreshed", async () => { + const { source, target } = await syncedPair("BothSame"); + const s2 = edit(source, (s) => (s[0].pageItemTemplateName = "Main (both)")); + const t2 = edit(target, (s) => (s[0].pageItemTemplateName = "Main (both)")); + + const { savePageTemplate, result } = await run(s2, t2); + + expect(savePageTemplate).not.toHaveBeenCalled(); + expect(result.skipped).toBe(1); + const { templateHash } = await import("lib/mappers/template-mapper"); + expect(record(source.pageTemplateID).sourceHash).toBe(templateHash(s2)); + }); + + describe("mapping from an older CLI, with no hashes", () => { + it("in sync: skipped, and the hashes are recorded", async () => { + const { source, target } = await syncedPair("UnrecordedInSync", { unrecorded: true }); + expect(record(source.pageTemplateID).sourceHash).toBeUndefined(); + + const { savePageTemplate } = await run(source, target); + + expect(savePageTemplate).not.toHaveBeenCalled(); + expect(record(source.pageTemplateID)).toEqual(expect.objectContaining({ fingerprintVersion: 1 })); + }); + + it("different: the source wins, as before, labelled as having no sync record", async () => { + const { source, target } = await syncedPair("UnrecordedDiffers", { unrecorded: true }); + state.preflight = true; + + const { preflightReport, keptTargetChanges } = await run( + source, + edit(target, (s) => (s[0].pageItemTemplateName = "Main (target)")) + ); + + expect(keptTargetChanges.hasEntries()).toBe(false); + expect(preflightReport.getEntries()).toEqual([ + expect.objectContaining({ action: "update", detail: expect.stringContaining("no sync record yet") }), + ]); + }); + + it("different, in a real run: the source is saved and the hashes recorded", async () => { + const { source, target } = await syncedPair("UnrecordedDiffersRun", { unrecorded: true }); + + const { savePageTemplate } = await run(source, edit(target, (s) => (s[0].pageItemTemplateName = "Main (target)"))); + + expect(savePageTemplate).toHaveBeenCalledTimes(1); + expect(record(source.pageTemplateID)).toEqual(expect.objectContaining({ fingerprintVersion: 1 })); + }); + }); +}); diff --git a/src/lib/pushers/tests/url-redirection-pusher.test.ts b/src/lib/pushers/tests/url-redirection-pusher.test.ts index b518de6..42de205 100644 --- a/src/lib/pushers/tests/url-redirection-pusher.test.ts +++ b/src/lib/pushers/tests/url-redirection-pusher.test.ts @@ -122,7 +122,8 @@ describe("pushUrlRedirections — map-on-adopt via originUrl match", () => { const result = await pushUrlRedirections([source], [target]); - expect(mockAddMapping).toHaveBeenCalledWith(1, 20, source.originUrl); + // Identical: the hashes of both sides are recorded with the adopted mapping. + expect(mockAddMapping).toHaveBeenCalledWith(1, 20, source.originUrl, expect.objectContaining({ fingerprintVersion: 1 })); expect(result.skipped).toBe(1); expect(result.successful).toBe(0); expect(mockSaveUrlRedirections).not.toHaveBeenCalled(); @@ -188,18 +189,29 @@ describe("pushUrlRedirections — mapped item, identical target → skip", () => // ─── mapped item whose mapped target no longer exists → CREATE ──────────────── -describe("pushUrlRedirections — mapped target deleted → create", () => { - it("sends a CREATE (urlRedirectionID=0) when the mapping's target no longer exists in targetData", async () => { - const { pushUrlRedirections } = await import("../url-redirection-pusher"); +describe("pushUrlRedirections — mapped target deleted on the target", () => { + const orphanMapping = { sourceUrlRedirectionID: 6, targetUrlRedirectionID: 99, originUrl: "/orphan" }; + it("is a conflict without --overwrite: nothing is sent", async () => { + setState({ preflight: true }); + const { pushUrlRedirections } = await import("../url-redirection-pusher"); const source = makeSourceItem({ id: 6, originUrl: "/orphan", destinationUrl: "/dest", statusCode: 301 }); + mockGetMapping.mockReturnValue(orphanMapping); - // Mapping exists but target ID 99 is not in targetData. - mockGetMapping.mockReturnValue({ - sourceUrlRedirectionID: 6, - targetUrlRedirectionID: 99, - originUrl: "/orphan", - }); + const result = await pushUrlRedirections([source], []); + + expect(mockSaveUrlRedirections).not.toHaveBeenCalled(); + expect(result.skipped).toBe(1); + expect(preflightReport.getEntries()).toEqual([ + expect.objectContaining({ action: "conflict", name: "/orphan", detail: expect.stringContaining("no longer exists") }), + ]); + }); + + it("with --overwrite sends a CREATE (urlRedirectionID=0)", async () => { + setState({ overwrite: true }); + const { pushUrlRedirections } = await import("../url-redirection-pusher"); + const source = makeSourceItem({ id: 6, originUrl: "/orphan", destinationUrl: "/dest", statusCode: 301 }); + mockGetMapping.mockReturnValue(orphanMapping); mockSaveUrlRedirections.mockResolvedValue({ created: [{ index: 0, urlRedirectionID: 100 }], updated: [], skipped: [] }); const result = await pushUrlRedirections([source], []); @@ -266,8 +278,8 @@ describe("pushUrlRedirections — response handling", () => { const result = await pushUrlRedirections([s1, s2, s3], []); // addMapping called for created (source 10 → target 12) and updated (source 11 → target 5). - expect(mockAddMapping).toHaveBeenCalledWith(10, 12, "/a"); - expect(mockAddMapping).toHaveBeenCalledWith(11, 5, "/b"); + expect(mockAddMapping).toHaveBeenCalledWith(10, 12, "/a", expect.objectContaining({ fingerprintVersion: 1 })); + expect(mockAddMapping).toHaveBeenCalledWith(11, 5, "/b", expect.objectContaining({ fingerprintVersion: 1 })); expect(result.successful).toBe(2); expect(result.skipped).toBe(1); @@ -379,3 +391,206 @@ describe("pushUrlRedirections — preflight mode", () => { expect(mockSaveUrlRedirections).not.toHaveBeenCalled(); }); }); + +// ─── PROD-3078: which side changed, from the hashes stored at the last sync ────────── + +describe("pushUrlRedirections — sync fingerprints (PROD-3078)", () => { + // The state both sides agreed on at the last sync. + const synced = { id: 1, originUrl: "/old", destinationUrl: "/new", statusCode: 301 }; + const syncedTarget = { ...synced, id: 10 }; + + async function setup(mapping: Record | null) { + const mod = await import("../url-redirection-pusher"); + const { keptTargetChanges } = await import("../kept-target-changes"); + keptTargetChanges.reset(); + mockGetMapping.mockReturnValue(mapping); + mockSaveUrlRedirections.mockResolvedValue({ created: [], updated: [{ index: 0, urlRedirectionID: 10 }], skipped: [] }); + return { ...mod, keptTargetChanges }; + } + + async function recorded() { + const { redirectionHash } = await import("../url-redirection-pusher"); + const h = redirectionHash(synced as any); + return { + sourceUrlRedirectionID: 1, + targetUrlRedirectionID: 10, + originUrl: "/old", + sourceHash: h, + targetHash: redirectionHash(syncedTarget as any), + fingerprintVersion: 1, + }; + } + + it("ignores the ID, origin URL case and originLocales order", async () => { + const { redirectionHash } = await import("../url-redirection-pusher"); + expect(redirectionHash({ ...synced, originLocales: ["fr", "en"] } as any)).toBe( + redirectionHash({ ...synced, id: 99, originUrl: " /OLD ", originLocales: ["en", "fr"] } as any) + ); + }); + + it("source changed only: updates and records the new hash for both sides", async () => { + const { pushUrlRedirections, redirectionHash } = await setup(await recorded()); + const source = { ...synced, destinationUrl: "/newer" }; + + const result = await pushUrlRedirections([source as any], [syncedTarget as any]); + + expect(mockSaveUrlRedirections).toHaveBeenCalledTimes(1); + expect(result.successful).toBe(1); + const h = redirectionHash(source as any); + expect(mockAddMapping).toHaveBeenCalledWith(1, 10, "/old", { + sourceHash: h, + targetHash: h, + fingerprintVersion: 1, + // What it replaced, until a pull shows the update (the Fetch API lags) + targetReplacedHash: redirectionHash(syncedTarget as any), + }); + }); + + it("a target pull that still shows what the last sync replaced is waited on, not kept", async () => { + setState({ preflight: true }); + const { redirectionHash } = await import("../url-redirection-pusher"); + const old = { ...syncedTarget, destinationUrl: "/before-the-last-sync" }; + const { pushUrlRedirections, keptTargetChanges } = await setup({ + ...(await recorded()), + targetReplacedHash: redirectionHash(old as any), + }); + + await pushUrlRedirections([synced as any], [old as any]); + + expect(mockSaveUrlRedirections).not.toHaveBeenCalled(); + expect(keptTargetChanges.hasEntries()).toBe(false); + expect(preflightReport.getEntries()).toEqual([ + expect.objectContaining({ action: "skip", detail: expect.stringContaining("isn't in the Fetch API yet") }), + ]); + }); + + it("a source pull that still shows what the last (reverse) sync replaced isn't synced back", async () => { + const { redirectionHash } = await import("../url-redirection-pusher"); + const old = { ...synced, destinationUrl: "/before-the-last-sync" }; + const { pushUrlRedirections } = await setup({ ...(await recorded()), sourceReplacedHash: redirectionHash(old as any) }); + + await pushUrlRedirections([old as any], [syncedTarget as any]); + + expect(mockSaveUrlRedirections).not.toHaveBeenCalled(); + }); + + it("a stale target pull doesn't hide a later source change", async () => { + const { redirectionHash } = await import("../url-redirection-pusher"); + const old = { ...syncedTarget, destinationUrl: "/before-the-last-sync" }; + const { pushUrlRedirections } = await setup({ ...(await recorded()), targetReplacedHash: redirectionHash(old as any) }); + + await pushUrlRedirections([{ ...synced, destinationUrl: "/newer" } as any], [old as any]); + + expect(mockSaveUrlRedirections.mock.calls[0][1][0].destinationUrl).toBe("/newer"); + }); + + it("an in-sync pull clears the replaced hash", async () => { + const { pushUrlRedirections } = await setup({ ...(await recorded()), targetReplacedHash: "stale" }); + + await pushUrlRedirections([synced as any], [syncedTarget as any]); + + expect(mockAddMapping).toHaveBeenCalledWith(1, 10, "/old", expect.objectContaining({ targetReplacedHash: undefined })); + }); + + it("target changed only: keeps the target, warns, and leaves the record alone", async () => { + setState({ preflight: true }); + const { pushUrlRedirections, keptTargetChanges } = await setup(await recorded()); + const target = { ...syncedTarget, destinationUrl: "/edited-on-target" }; + + const result = await pushUrlRedirections([synced as any], [target as any]); + + expect(mockSaveUrlRedirections).not.toHaveBeenCalled(); + expect(mockAddMapping).not.toHaveBeenCalled(); + expect(result.skipped).toBe(1); + expect(keptTargetChanges.getEntries()).toEqual([ + expect.objectContaining({ type: "urlRedirection", name: "/old", sourceID: 1, targetID: 10 }), + ]); + expect(preflightReport.getEntries()).toEqual([expect.objectContaining({ action: "skip" })]); + }); + + it("target changed only, with --overwrite: the source replaces it", async () => { + setState({ overwrite: true }); + const { pushUrlRedirections, keptTargetChanges } = await setup(await recorded()); + const target = { ...syncedTarget, destinationUrl: "/edited-on-target" }; + + await pushUrlRedirections([synced as any], [target as any]); + + expect(mockSaveUrlRedirections).toHaveBeenCalledTimes(1); + expect(mockSaveUrlRedirections.mock.calls[0][1][0]).toEqual(expect.objectContaining({ urlRedirectionID: 10, destinationUrl: "/new" })); + expect(keptTargetChanges.hasEntries()).toBe(false); + }); + + it("both changed: a conflict, nothing sent, record left alone", async () => { + setState({ preflight: true }); + const { pushUrlRedirections, keptTargetChanges } = await setup(await recorded()); + + const result = await pushUrlRedirections( + [{ ...synced, destinationUrl: "/source-edit" } as any], + [{ ...syncedTarget, destinationUrl: "/target-edit" } as any] + ); + + expect(mockSaveUrlRedirections).not.toHaveBeenCalled(); + expect(mockAddMapping).not.toHaveBeenCalled(); + expect(result.skipped).toBe(1); + expect(keptTargetChanges.hasEntries()).toBe(false); + expect(preflightReport.hasConflicts()).toBe(true); + }); + + it("both changed, with --overwrite: the source wins", async () => { + setState({ overwrite: true }); + const { pushUrlRedirections } = await setup(await recorded()); + + await pushUrlRedirections( + [{ ...synced, destinationUrl: "/source-edit" } as any], + [{ ...syncedTarget, destinationUrl: "/target-edit" } as any] + ); + + expect(mockSaveUrlRedirections.mock.calls[0][1][0].destinationUrl).toBe("/source-edit"); + expect(preflightReport.hasConflicts()).toBe(false); + }); + + it("both changed to the same value: skipped, and the hashes refreshed", async () => { + const { pushUrlRedirections, redirectionHash } = await setup(await recorded()); + const source = { ...synced, destinationUrl: "/same-edit" }; + + const result = await pushUrlRedirections([source as any], [{ ...syncedTarget, destinationUrl: "/same-edit" } as any]); + + expect(mockSaveUrlRedirections).not.toHaveBeenCalled(); + expect(result.skipped).toBe(1); + const h = redirectionHash(source as any); + expect(mockAddMapping).toHaveBeenCalledWith(1, 10, "/old", { sourceHash: h, targetHash: h, fingerprintVersion: 1 }); + }); + + describe("mapping from an older CLI, with no hashes", () => { + const unrecorded = { sourceUrlRedirectionID: 1, targetUrlRedirectionID: 10, originUrl: "/old" }; + + it("in sync: skipped, and the hashes are recorded", async () => { + const { pushUrlRedirections } = await setup(unrecorded); + + await pushUrlRedirections([synced as any], [syncedTarget as any]); + + expect(mockSaveUrlRedirections).not.toHaveBeenCalled(); + expect(mockAddMapping).toHaveBeenCalledWith(1, 10, "/old", expect.objectContaining({ fingerprintVersion: 1 })); + }); + + it("different: the source wins, as before, labelled as having no sync record", async () => { + setState({ preflight: true }); + const { pushUrlRedirections, keptTargetChanges } = await setup(unrecorded); + + await pushUrlRedirections([synced as any], [{ ...syncedTarget, destinationUrl: "/edited-on-target" } as any]); + + expect(keptTargetChanges.hasEntries()).toBe(false); + expect(preflightReport.getEntries()).toEqual([ + expect.objectContaining({ action: "update", detail: expect.stringContaining("no sync record yet") }), + ]); + }); + + it("hashes from another fingerprint version count as missing", async () => { + const { pushUrlRedirections } = await setup({ ...(await recorded()), fingerprintVersion: 0 }); + + await pushUrlRedirections([synced as any], [{ ...syncedTarget, destinationUrl: "/edited-on-target" } as any]); + + expect(mockSaveUrlRedirections).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/src/lib/pushers/url-redirection-pusher.ts b/src/lib/pushers/url-redirection-pusher.ts index 7f12158..072a1ff 100644 --- a/src/lib/pushers/url-redirection-pusher.ts +++ b/src/lib/pushers/url-redirection-pusher.ts @@ -6,30 +6,49 @@ import { UrlRedirectionSyncItem, UrlRedirectionSavePayload } from "../../types/u import { preflightReport } from "../preflight/preflight-report"; import { saveUrlRedirections, MAX_URL_REDIRECTION_BATCH_SIZE } from "./url-redirection-api"; import { FailureDetail, PusherResult } from "../../types/sourceData"; +import { keptTargetChanges } from "./kept-target-changes"; +import { + decideSync, + fingerprintFields, + hashShape, + hasFingerprint, + PENDING_DETAIL, + UNRECORDED_UPDATE_DETAIL, +} from "../shared/sync-fingerprint"; /** A queued create/update: the source item plus the payload the API will receive. */ interface PendingSave { source: UrlRedirectionSyncItem; payload: UrlRedirectionSavePayload; action: "create" | "update"; + /** Hash of the source's shape; stored for both sides once the save lands (a full replace). */ + hash: string; + /** Hash of the target this save replaces (updates only), until a pull shows the replacement */ + replacedHash?: string; + /** Preflight detail, e.g. why the source wins */ + detail?: string; } const normalizeOrigin = (url: string | null | undefined): string => (url ?? "").trim().toLowerCase(); /** - * Pulled redirections carry no modifiedOn timestamp, so change detection is a direct - * field comparison of the pulled source and target items (there is no - * target-changed/conflict detection like galleries; an update is a full replace). + * The fields that make two redirections the same, without the per-instance ID. Pulled redirections + * carry no modifiedOn timestamp, so this shape is both the "differs" check and, hashed, the + * fingerprint stored in the mapping to tell which side changed (PROD-3078). */ -function areEquivalent(a: UrlRedirectionSyncItem, b: UrlRedirectionSyncItem): boolean { - return ( - normalizeOrigin(a.originUrl) === normalizeOrigin(b.originUrl) && - (a.destinationUrl ?? "").trim() === (b.destinationUrl ?? "").trim() && - (a.statusCode ?? 301) === (b.statusCode ?? 301) && - (a.destinationLocale ?? null) === (b.destinationLocale ?? null) && - JSON.stringify(a.originLocales ?? []) === JSON.stringify(b.originLocales ?? []) && - (a.content ?? null) === (b.content ?? null) - ); +export function normalizeRedirection(item: UrlRedirectionSyncItem) { + return { + originUrl: normalizeOrigin(item.originUrl), + destinationUrl: (item.destinationUrl ?? "").trim(), + statusCode: item.statusCode ?? 301, + destinationLocale: item.destinationLocale ?? null, + originLocales: [...(item.originLocales ?? [])].sort(), + content: item.content ?? null, + }; +} + +export function redirectionHash(item: UrlRedirectionSyncItem): string { + return hashShape(normalizeRedirection(item)); } function toSavePayload(item: UrlRedirectionSyncItem, targetUrlRedirectionID: number): UrlRedirectionSavePayload { @@ -56,7 +75,7 @@ export async function pushUrlRedirections( const redirections: UrlRedirectionSyncItem[] = sourceData || []; const targetRedirections: UrlRedirectionSyncItem[] = targetData || []; - const { sourceGuid, targetGuid } = state; + const { sourceGuid, targetGuid, overwrite } = state; const logger = getLoggerForGuid(sourceGuid) || new Logs("push", "urlRedirection", sourceGuid); @@ -73,8 +92,13 @@ export async function pushUrlRedirections( let overallStatus: "success" | "error" = "success"; const failureDetails: FailureDetail[] = []; - // Phase 1: classify every source redirection as create / update / skip. + // Phase 1: classify every source redirection as create / update / skip / kept / conflict. const pending: PendingSave[] = []; + const recordSkip = (source: UrlRedirectionSyncItem, detail: string, action: "skip" | "conflict" = "skip") => { + logger.urlRedirection.skipped(source, detail, targetGuid); + preflightReport.record({ phase: "URL Redirections", action, name: source.originUrl, detail }); + skipped++; + }; for (const source of redirections) { const mapping = mapper.getMapping(source.id, "source"); @@ -82,46 +106,98 @@ export async function pushUrlRedirections( ? targetRedirections.find((t) => t.id === mapping.targetUrlRedirectionID) : undefined; const targetByOrigin = targetRedirections.find((t) => normalizeOrigin(t.originUrl) === normalizeOrigin(source.originUrl)); + const sourceHash = redirectionHash(source); if (!mapping && targetByOrigin) { // Exists in target by origin URL but no mapping — adopt the mapping (map-on-adopt, // same self-heal behavior as galleries/models), then diff to decide update vs skip. - mapper.addMapping(source.id, targetByOrigin.id, source.originUrl); - if (areEquivalent(source, targetByOrigin)) { - logger.urlRedirection.skipped(source, "already exists in target by origin URL", targetGuid); - preflightReport.record({ - phase: "URL Redirections", - action: "skip", - name: source.originUrl, - detail: "already exists in target by origin URL", - }); - skipped++; + // Nothing records which side changed yet, so a difference is settled by the source. + const targetHash = redirectionHash(targetByOrigin); + if (sourceHash === targetHash) { + mapper.addMapping(source.id, targetByOrigin.id, source.originUrl, fingerprintFields(sourceHash, targetHash)); + recordSkip(source, "already exists in target by origin URL"); } else { - pending.push({ source, payload: toSavePayload(source, targetByOrigin.id), action: "update" }); + mapper.addMapping(source.id, targetByOrigin.id, source.originUrl); + pending.push({ + source, + payload: toSavePayload(source, targetByOrigin.id), + action: "update", + hash: sourceHash, + replacedHash: targetHash, + detail: UNRECORDED_UPDATE_DETAIL, + }); } } else if (mapping && targetById) { - if (areEquivalent(source, targetById)) { - logger.urlRedirection.skipped(source, "up to date, skipping", targetGuid); - preflightReport.record({ - phase: "URL Redirections", - action: "skip", - name: source.originUrl, - detail: "up to date", + const targetHash = redirectionHash(targetById); + const decision = decideSync(sourceHash === targetHash, sourceHash, targetHash, mapping); + const update = (detail?: string) => + pending.push({ + source, + payload: toSavePayload(source, targetById.id), + action: "update", + hash: sourceHash, + replacedHash: targetHash, + detail, }); - skipped++; + + if (decision === "skip") { + // Seed or refresh the hashes: an older mapping gets them here, and two sides edited to + // the same value agree again. + // A pull that shows the last save also clears the replaced state. + if ( + !hasFingerprint(mapping) || + mapping.sourceHash !== sourceHash || + mapping.targetHash !== targetHash || + mapping.targetReplacedHash || + mapping.sourceReplacedHash + ) { + mapper.addMapping(source.id, targetById.id, source.originUrl, fingerprintFields(sourceHash, targetHash)); + } + recordSkip(source, "up to date"); + } else if (decision === "pending") { + recordSkip(source, PENDING_DETAIL); + } else if (decision === "update") { + update(); + } else if (decision === "update-unrecorded") { + update(UNRECORDED_UPDATE_DETAIL); + } else if (overwrite) { + update(decision === "conflict" ? "conflict overwritten" : "target change overwritten"); + } else if (decision === "keep") { + const change = { + type: "urlRedirection" as const, + name: source.originUrl, + sourceID: source.id, + targetID: targetById.id, + }; + keptTargetChanges.record(change); + console.warn(ansiColors.yellow(keptTargetChanges.formatLine(change))); + recordSkip(source, "target changed since the last sync and the source didn't; kept the target's version"); } else { - pending.push({ source, payload: toSavePayload(source, targetById.id), action: "update" }); + recordSkip(source, "source and target both changed; use --overwrite to force", "conflict"); } + } else if (mapping && !overwrite) { + // The mapped target redirection was deleted on the target. Like a deleted target template + // (PROD-2603): a conflict, and --overwrite recreates it. + recordSkip( + source, + `mapped target redirection (ID: ${mapping.targetUrlRedirectionID}) no longer exists on the target; use --overwrite to recreate it`, + "conflict" + ); } else { - // No mapping and no origin match, or the mapped target redirection was deleted — create. - pending.push({ source, payload: toSavePayload(source, 0), action: "create" }); + // No mapping and no origin match, or (with --overwrite) the mapped target was deleted — create. + pending.push({ source, payload: toSavePayload(source, 0), action: "create", hash: sourceHash }); } } // Preflight (PROD-2203): report what WOULD happen without calling the API. if (state.preflight) { for (const item of pending) { - preflightReport.record({ phase: "URL Redirections", action: item.action, name: item.source.originUrl }); + preflightReport.record({ + phase: "URL Redirections", + action: item.action, + name: item.source.originUrl, + ...(item.detail ? { detail: item.detail } : {}), + }); successful++; } console.log( @@ -146,7 +222,12 @@ export async function pushUrlRedirections( for (const created of result.created) { const item = batch[created.index]; if (item && created.urlRedirectionID) { - mapper.addMapping(item.source.id, created.urlRedirectionID, item.source.originUrl); + mapper.addMapping( + item.source.id, + created.urlRedirectionID, + item.source.originUrl, + fingerprintFields(item.hash, item.hash) + ); logger.urlRedirection.created(item.source, "created", targetGuid); } successful++; @@ -158,7 +239,10 @@ export async function pushUrlRedirections( mapper.addMapping( item.source.id, updated.urlRedirectionID ?? item.payload.urlRedirectionID, - item.source.originUrl + item.source.originUrl, + // A full replace: the target now has the source's shape. The Fetch API shows it a + // while later; until then a pull shows the replaced redirection, which isn't an edit. + fingerprintFields(item.hash, item.hash, item.replacedHash) ); logger.urlRedirection.updated(item.source, "updated", targetGuid); } diff --git a/src/lib/shared/sync-fingerprint.ts b/src/lib/shared/sync-fingerprint.ts new file mode 100644 index 0000000..23cbc90 --- /dev/null +++ b/src/lib/shared/sync-fingerprint.ts @@ -0,0 +1,128 @@ +/** + * Sync fingerprints (PROD-3078): telling which side changed for entities with no version or + * modified date (page templates, URL redirections). + * + * After each sync that leaves both sides in agreement, the mapping record stores a hash of each + * side's normalised shape (`sourceHash` / `targetHash`, so reverse sync transposes them with the + * IDs). On the next sync, a side whose current hash differs from its stored one changed since. + * The normalised shape leaves out per-instance IDs and is the same shape the pushers' "differs" + * checks compare, so "changed" and "differs" can't disagree. + * + * The decision follows the rule the other types use (see kept-target-changes.ts): + * - the two sides are the same now: skip, and record the hashes (seeds old mappings too) + * - no hashes recorded (mapping from an older CLI): the source wins, as before, then record + * - only the source changed: update + * - only the target changed: keep the target and warn; the record isn't refreshed + * - both changed: conflict; the record isn't refreshed + * - --overwrite: the source wins in the last two cases + * - a side still shows what the last sync replaced (types pulled from the Fetch API, which shows + * a save a while later): skip until it catches up, so a stale pull is neither an edit nor + * synced back + */ + +import { createHash } from "crypto"; + +/** Bump when a normalised shape changes: older hashes then count as missing, not as changes. */ +export const FINGERPRINT_VERSION = 1; + +/** Fields a mapping record carries once fingerprinted. */ +export interface SyncFingerprint { + sourceHash?: string; + targetHash?: string; + fingerprintVersion?: number; + /** + * The target's hash before the sync replaced it, kept until a pull shows the replacement. Types + * pulled from the Fetch API (redirections) show a save a while later, and a pull in that window + * still shows what was replaced: that isn't an edit. A reverse sync reads it as + * `sourceReplacedHash`, the side it now syncs from. + */ + targetReplacedHash?: string; + sourceReplacedHash?: string; +} + +export type SyncDecision = + | "skip" // same on both sides now + | "update" // source changed only + | "update-unrecorded" // the sides differ and the mapping has no hashes yet: source wins + | "keep" // target changed only: keep it and warn + | "conflict" // both changed + | "pending"; // a side still shows what the last sync replaced (the Fetch API hasn't caught up): wait + +/** JSON with object keys sorted, so equal values always serialise the same way. */ +export function stableStringify(value: unknown): string { + if (value === undefined) return "null"; + if (value === null || typeof value !== "object") return JSON.stringify(value); + if (Array.isArray(value)) return `[${value.map(stableStringify).join(",")}]`; + const obj = value as Record; + const keys = Object.keys(obj) + .filter((k) => obj[k] !== undefined) + .sort(); + return `{${keys.map((k) => `${JSON.stringify(k)}:${stableStringify(obj[k])}`).join(",")}}`; +} + +export function hashShape(shape: unknown): string { + return createHash("sha1").update(stableStringify(shape)).digest("hex"); +} + +/** Whether the record carries hashes from the current fingerprint version. */ +export function hasFingerprint(record: SyncFingerprint | null | undefined): boolean { + return ( + !!record?.sourceHash && !!record?.targetHash && record.fingerprintVersion === FINGERPRINT_VERSION + ); +} + +/** + * Decide what to do with a mapped pair whose target still exists. + * + * @param inSync whether the two sides are the same now (the pusher's own "differs" check) + * @param sourceHash hash of the current source's normalised shape + * @param targetHash hash of the current target's normalised shape + * @param record the mapping record, with any hashes from the last sync + * + * --overwrite is applied by the caller: it turns "keep" and "conflict" into an update. + */ +export function decideSync( + inSync: boolean, + sourceHash: string, + targetHash: string, + record: SyncFingerprint | null | undefined +): SyncDecision { + if (inSync) return "skip"; + if (!hasFingerprint(record)) return "update-unrecorded"; + const r = record!; + // A side that still shows what the last sync replaced was pulled before the save reached the + // Fetch API: not an edit. Syncing from a source in that state would put the old version back. + const sourcePending = sourceHash !== r.sourceHash && !!r.sourceReplacedHash && sourceHash === r.sourceReplacedHash; + const targetPending = targetHash !== r.targetHash && !!r.targetReplacedHash && targetHash === r.targetReplacedHash; + const sourceChanged = sourceHash !== r.sourceHash && !sourcePending; + const targetChanged = targetHash !== r.targetHash && !targetPending; + if (sourceChanged && targetChanged) return "conflict"; + if (targetChanged) return "keep"; + if (sourceChanged) return "update"; + if (sourcePending || targetPending) return "pending"; + // Neither changed while the sides still differ (only possible if the normalisation and the + // "differs" check drift apart): the source wins, as before. + return "update"; +} + +/** + * The fields to store on a mapping record once the two sides agree. `targetReplacedHash` is the + * target's hash before this sync replaced it, for types pulled from the Fetch API; leaving it out + * clears any earlier one (it is written as undefined, which the mapping file drops). + */ +export function fingerprintFields(sourceHash: string, targetHash: string, targetReplacedHash?: string): SyncFingerprint { + return { + sourceHash, + targetHash, + fingerprintVersion: FINGERPRINT_VERSION, + targetReplacedHash, + sourceReplacedHash: undefined, + }; +} + +/** Preflight/log detail for an entity skipped until the Fetch API shows the last sync's change. */ +export const PENDING_DETAIL = "the last sync's change isn't in the Fetch API yet; skipped until it is"; + +/** Preflight/log detail for a source-wins update on a mapping with no hashes yet. */ +export const UNRECORDED_UPDATE_DETAIL = + "no sync record yet, so the source wins this time; target-only changes are kept from the next sync on"; diff --git a/src/lib/shared/tests/sync-fingerprint.test.ts b/src/lib/shared/tests/sync-fingerprint.test.ts new file mode 100644 index 0000000..d06b449 --- /dev/null +++ b/src/lib/shared/tests/sync-fingerprint.test.ts @@ -0,0 +1,77 @@ +import { transposeMappingRecord } from "../../../core/fileOperations"; +import { + FINGERPRINT_VERSION, + decideSync, + fingerprintFields, + hashShape, + hasFingerprint, + stableStringify, +} from "../sync-fingerprint"; + +describe("stableStringify / hashShape", () => { + it("serialises equal objects the same whatever their key order", () => { + expect(stableStringify({ b: 1, a: { d: [1, 2], c: null } })).toBe(stableStringify({ a: { c: null, d: [1, 2] }, b: 1 })); + expect(hashShape({ b: 1, a: 2 })).toBe(hashShape({ a: 2, b: 1 })); + }); + + it("keeps array order, and treats a missing key like an undefined one", () => { + expect(hashShape([1, 2])).not.toBe(hashShape([2, 1])); + expect(stableStringify({ a: 1, b: undefined })).toBe(stableStringify({ a: 1 })); + }); +}); + +describe("decideSync", () => { + const recorded = fingerprintFields("S", "T"); + + it("is a skip whenever the two sides are the same now", () => { + expect(decideSync(true, "S2", "T2", recorded)).toBe("skip"); + expect(decideSync(true, "S", "T", null)).toBe("skip"); + }); + + it("lets the source win when there are no hashes, or hashes from another version", () => { + expect(decideSync(false, "S2", "T", null)).toBe("update-unrecorded"); + expect(decideSync(false, "S2", "T", { sourceHash: "S", targetHash: "T" })).toBe("update-unrecorded"); + expect(decideSync(false, "S2", "T", { ...recorded, fingerprintVersion: FINGERPRINT_VERSION + 1 })).toBe( + "update-unrecorded" + ); + }); + + it("follows the rule: source only updates, target only keeps, both conflict", () => { + expect(decideSync(false, "S2", "T", recorded)).toBe("update"); + expect(decideSync(false, "S", "T2", recorded)).toBe("keep"); + expect(decideSync(false, "S2", "T2", recorded)).toBe("conflict"); + }); + + it("waits on a side that still shows what the last sync replaced", () => { + // The target pull predates the last sync's save + expect(decideSync(false, "S", "OLD", { ...recorded, targetReplacedHash: "OLD" })).toBe("pending"); + // A reverse sync: its source pull predates the save + expect(decideSync(false, "OLD", "T", { ...recorded, sourceReplacedHash: "OLD" })).toBe("pending"); + // A real change on the other side still counts + expect(decideSync(false, "S2", "OLD", { ...recorded, targetReplacedHash: "OLD" })).toBe("update"); + expect(decideSync(false, "OLD", "T2", { ...recorded, sourceReplacedHash: "OLD" })).toBe("keep"); + }); + + it("lets the source win when the sides differ but neither hash moved", () => { + expect(decideSync(false, "S", "T", recorded)).toBe("update"); + }); +}); + +describe("fingerprint fields on a mapping record", () => { + it("hasFingerprint needs both hashes and the current version", () => { + expect(hasFingerprint(fingerprintFields("S", "T"))).toBe(true); + expect(hasFingerprint({ sourceHash: "S", fingerprintVersion: FINGERPRINT_VERSION })).toBe(false); + expect(hasFingerprint(undefined)).toBe(false); + }); + + it("swap with the IDs when a reverse sync reads the record", () => { + const record = { sourcePageTemplateID: 1, targetPageTemplateID: 2, ...fingerprintFields("S", "T") }; + expect(transposeMappingRecord(record)).toEqual({ + sourcePageTemplateID: 2, + targetPageTemplateID: 1, + sourceHash: "T", + targetHash: "S", + fingerprintVersion: FINGERPRINT_VERSION, + }); + }); +}); diff --git a/src/types/sourceData.ts b/src/types/sourceData.ts index c2e1be8..61209dc 100644 --- a/src/types/sourceData.ts +++ b/src/types/sourceData.ts @@ -39,7 +39,8 @@ export type FailureDetailType = | "container" | "model" | "template" - | "urlRedirection"; + | "urlRedirection" + | "listOrder"; // PROD-3078: a list whose target order was kept (warnings only) /** * Individual failure detail with optional link metadata