fix: overlay legacy evidence labels instead of shadowing the whole descriptor
Review finding I1: the static-key-first resolution order protected the entire legacy fixture (image and labels), when only the hand-authored labels ever needed protecting -- ResolvedAsset carries no label fields, so nothing else could recover them, but a real descriptor's publicPath/width/height were never actually at risk of mismatching. Narrowed resolveWith so a resolved descriptor's data fields always win; the legacy registry only overlays triggerLabel/dialogLabel for keys it recognizes, and only supplies the full descriptor when nothing else resolves the key at all. A future backend asset colliding with the legacy key now degrades to a wrong caption, never a wrong image. Review finding I2: renamed and rewrote a test whose title claimed a READY-vs-QUARANTINED same-key guarantee its body never constructed. It now puts both a QUARANTINED and a READY entry under one assetKey and asserts the READY one wins. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
9d91001a31
commit
e2c1d076f4
@@ -60,19 +60,36 @@ function fromDescriptor(descriptor: ResolvedAssetLike): EvidenceAsset {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Shared resolution order for both factories below: the legacy static key always
|
* Shared resolution order for both factories below. The legacy registry only
|
||||||
* wins on a match (protects the one pre-existing hardcoded fixture from splitting
|
* needs to protect the *labels* it hand-authored for the one pre-existing key
|
||||||
* across render surfaces mid-migration), then the caller-supplied lookup (the real
|
* (`ResolvedAsset` carries no label fields at all, so nothing else can supply
|
||||||
* backend-sourced descriptor), then the safe placeholder.
|
* them) -- the image itself always comes from whichever descriptor actually
|
||||||
|
* resolved the key, catalog or server block, when one exists. This way a future
|
||||||
|
* backend `Asset` whose `assetKey` collides with the legacy slug degrades to,
|
||||||
|
* at worst, a wrong caption on the right image -- never a wrong image shown
|
||||||
|
* under a right caption. Only when no descriptor resolves the key at all does
|
||||||
|
* the legacy registry supply the image too (today's actual Instant Preview
|
||||||
|
* fallback path, with no catalog to consult).
|
||||||
*/
|
*/
|
||||||
function resolveWith(
|
function resolveWith(
|
||||||
key: string,
|
key: string,
|
||||||
lookup: (key: string) => ResolvedAssetLike | undefined,
|
lookup: (key: string) => ResolvedAssetLike | undefined,
|
||||||
): EvidenceAsset {
|
): EvidenceAsset {
|
||||||
const legacy = LEGACY_STATIC_EVIDENCE_ASSETS[key];
|
const legacy = LEGACY_STATIC_EVIDENCE_ASSETS[key];
|
||||||
if (legacy) return legacy;
|
|
||||||
const descriptor = lookup(key);
|
const descriptor = lookup(key);
|
||||||
return descriptor ? fromDescriptor(descriptor) : MISSING_EVIDENCE_ASSET;
|
|
||||||
|
if (descriptor) {
|
||||||
|
const resolved = fromDescriptor(descriptor);
|
||||||
|
return legacy
|
||||||
|
? Object.freeze({
|
||||||
|
...resolved,
|
||||||
|
triggerLabel: legacy.triggerLabel,
|
||||||
|
dialogLabel: legacy.dialogLabel,
|
||||||
|
})
|
||||||
|
: resolved;
|
||||||
|
}
|
||||||
|
|
||||||
|
return legacy ?? MISSING_EVIDENCE_ASSET;
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Instant Preview: resolves against the Asset list the editor has loaded. */
|
/** Instant Preview: resolves against the Asset list the editor has loaded. */
|
||||||
|
|||||||
@@ -75,20 +75,54 @@ test("resolves from the server render model blocks", () => {
|
|||||||
assert.equal(resolve("boundary").width, 800);
|
assert.equal(resolve("boundary").width, 800);
|
||||||
});
|
});
|
||||||
|
|
||||||
test("a QUARANTINED asset never wins over the placeholder even when a READY asset with the same key exists later", () => {
|
test("a READY asset wins over a QUARANTINED asset sharing the same key", () => {
|
||||||
const resolve = createAssetCatalogResolver([
|
const resolve = createAssetCatalogResolver([
|
||||||
{ ...(READY as object), managementStatus: "QUARANTINED" } as never,
|
{ ...(READY as object), managementStatus: "QUARANTINED" } as never,
|
||||||
|
READY,
|
||||||
]);
|
]);
|
||||||
assert.equal(resolve("boundary").src, "");
|
assert.equal(resolve("boundary").src, "/media/boundary.svg");
|
||||||
});
|
});
|
||||||
|
|
||||||
test("does not let a real backend asset shadow the legacy static key's byte-identical output", () => {
|
test("keeps the legacy key's hand-authored labels when a server block resolves it via descriptor (today's real Preview/Snapshot path)", () => {
|
||||||
|
const resolve = createResolvedAssetResolver([
|
||||||
|
{
|
||||||
|
type: "EVIDENCE_FIGURE",
|
||||||
|
key: "fetch-strategy-boundary",
|
||||||
|
asset: {
|
||||||
|
assetId: "00000000-0000-4000-8000-000000000001",
|
||||||
|
assetKey: "fetch-strategy-boundary",
|
||||||
|
mediaType: "image/svg+xml",
|
||||||
|
publicPath: "/media/fetch-strategy-boundary.svg",
|
||||||
|
width: 1080,
|
||||||
|
height: 420,
|
||||||
|
decorative: false,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
assert.deepEqual(resolve("fetch-strategy-boundary"), {
|
||||||
|
src: "/media/fetch-strategy-boundary.svg",
|
||||||
|
width: 1080,
|
||||||
|
height: 420,
|
||||||
|
triggerLabel: "Fetch Join과 Batch Fetch 비교 다이어그램 크게 보기",
|
||||||
|
dialogLabel: "Fetch Join과 Batch Fetch의 페이징 경계 확대",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
test("a colliding backend descriptor's publicPath wins over the legacy src, but the legacy labels still win", () => {
|
||||||
const resolve = createAssetCatalogResolver([
|
const resolve = createAssetCatalogResolver([
|
||||||
{
|
{
|
||||||
...(READY as object),
|
...(READY as object),
|
||||||
assetKey: "fetch-strategy-boundary",
|
assetKey: "fetch-strategy-boundary",
|
||||||
publicPath: "/media/should-not-win.svg",
|
publicPath: "/media/should-win.svg",
|
||||||
|
width: 999,
|
||||||
|
height: 111,
|
||||||
} as never,
|
} as never,
|
||||||
]);
|
]);
|
||||||
assert.equal(resolve("fetch-strategy-boundary").src, "/media/fetch-strategy-boundary.svg");
|
const asset = resolve("fetch-strategy-boundary");
|
||||||
|
|
||||||
|
assert.equal(asset.src, "/media/should-win.svg");
|
||||||
|
assert.equal(asset.width, 999);
|
||||||
|
assert.equal(asset.height, 111);
|
||||||
|
assert.equal(asset.triggerLabel, "Fetch Join과 Batch Fetch 비교 다이어그램 크게 보기");
|
||||||
|
assert.equal(asset.dialogLabel, "Fetch Join과 Batch Fetch의 페이징 경계 확대");
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user