From 18d8ade6c43a15ff5841eb2ec1de1266aa21484f Mon Sep 17 00:00:00 2001 From: Charles Hudson Date: Sat, 27 Jun 2026 18:47:42 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9E=20fix(resolver):=20Fixing=20circul?= =?UTF-8?q?ar-reference=20handling=20to=20avoid=20call=20stack=20errors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixes circular-reference handling in optimized Contentful entry resolution by making generic Contentful entry fields pass-through data during schema validation. Resolved Contentful graphs can contain repeated or circular references, especially through rich-text `data.target` links. Previously, `EntryFields` used `z.json()`, which recursively walked arbitrary consumer fields. When `resolveOptimizedEntry` validated a selected optimization path against an entry graph containing cycles, Zod could overflow the stack and cause personalization resolution to fail. ## Changes - Changed `EntryFields` from recursive JSON validation to pass-through field validation with `z.any()`. - Updated schema documentation to clarify that consumer-defined Contentful fields are intentionally not strongly validated by SDK schemas. - Added schema tests proving: - arbitrary/circular consumer fields are accepted without recursive traversal; - Contentful `sys` metadata is still validated; - SDK-owned optimization fields such as `nt_config` and `nt_experiences` are still validated. - Added resolver regression coverage for circular Contentful-like graphs: - unrelated rich-text linked-entry cycles under the baseline graph; - rich-text linked-entry cycles inside the selected variant entry. ## Rationale The SDK only needs to validate and inspect SDK-owned fields and Contentful structural metadata. Consumer-authored entry fields can contain resolved Contentful entries, rich-text documents, assets, and repeated references. Treating those fields as JSON-only data is too strict for real resolved CDA graphs and can trigger recursive validation failures. This change keeps validation where it matters while avoiding traversal of arbitrary consumer content graphs. ## Validation - `pnpm exec eslint ...` on touched files - `pnpm exec prettier --check ...` on touched files - `pnpm --filter @contentful/optimization-api-schemas typecheck` - `pnpm --filter @contentful/optimization-api-schemas test:unit` - `pnpm --filter @contentful/optimization-api-client typecheck` - `pnpm --filter @contentful/optimization-core typecheck` - `pnpm --filter @contentful/optimization-core exec rstest run src/resolvers/OptimizedEntryResolver.test.ts` - `pnpm --filter @contentful/optimization-core test:unit` - `pnpm --filter @contentful/optimization-api-schemas size:check` - `pnpm --filter @contentful/optimization-api-client size:check` - `pnpm --filter @contentful/optimization-core size:check` [[NT-3567](https://contentful.atlassian.net/browse/NT-3567)] --- .../api-schemas/src/contentful/CtflEntry.ts | 14 +- .../src/contentful/OptimizationConfig.test.ts | 130 ++++++++++++++- .../resolvers/OptimizedEntryResolver.test.ts | 154 +++++++++++++++++- 3 files changed, 290 insertions(+), 8 deletions(-) diff --git a/packages/universal/api-schemas/src/contentful/CtflEntry.ts b/packages/universal/api-schemas/src/contentful/CtflEntry.ts index 876c8c7d9..5a8924599 100644 --- a/packages/universal/api-schemas/src/contentful/CtflEntry.ts +++ b/packages/universal/api-schemas/src/contentful/CtflEntry.ts @@ -4,13 +4,15 @@ import * as z from 'zod/mini' * Base Zod schema for entry fields. * * @remarks - * This is modeled as a catch-all map from string keys to JSON-compatible values. - * The strong typing ot consumer-specified Contentful Entry fields is not - * validated by these schemas. + * This is modeled as a catch-all map from string keys to pass-through values. + * The strong typing of consumer-specified Contentful Entry fields is not + * validated by these schemas. Arbitrary field values are intentionally treated + * as pass-through data so resolved Contentful entry graphs can contain circular + * references without recursive schema traversal. * * @public */ -export const EntryFields = z.catchall(z.object({}), z.json()) +export const EntryFields = z.catchall(z.object({}), z.any()) /** * TypeScript type inferred from {@link EntryFields}. @@ -175,8 +177,8 @@ export type EntrySys = z.infer * Zod schema describing a generic Contentful entry. * * @remarks - * This model is intentionally loose: `fields` is any JSON-compliant object and - * `metadata` is modeled as a catch-all object that must contain an array of + * This model is intentionally loose: arbitrary `fields` values are pass-through + * data and `metadata` is modeled as an object that must contain an array of * {@link TagLink} tags. * * @public diff --git a/packages/universal/api-schemas/src/contentful/OptimizationConfig.test.ts b/packages/universal/api-schemas/src/contentful/OptimizationConfig.test.ts index 1527a9329..03f11af4c 100644 --- a/packages/universal/api-schemas/src/contentful/OptimizationConfig.test.ts +++ b/packages/universal/api-schemas/src/contentful/OptimizationConfig.test.ts @@ -1,4 +1,10 @@ -import { normalizeOptimizationConfig, OptimizationEntry, type OptimizationConfig } from '.' +import { + CtflEntry, + normalizeOptimizationConfig, + OptimizationEntry, + OptimizedEntry, + type OptimizationConfig, +} from '.' const optimizationEntryBase = { metadata: { @@ -43,6 +49,43 @@ const optimizationEntryBase = { }, } +const entryBase = { + metadata: { + tags: [], + concepts: [], + }, + sys: { + type: 'Entry', + contentType: { + sys: { + type: 'Link', + linkType: 'ContentType', + id: 'article', + }, + }, + publishedVersion: 1, + id: 'entry-id', + createdAt: '2026-01-01T00:00:00.000Z', + updatedAt: '2026-01-01T00:00:00.000Z', + revision: 1, + space: { + sys: { + type: 'Link', + linkType: 'Space', + id: 'space-id', + }, + }, + environment: { + sys: { + type: 'Link', + linkType: 'Environment', + id: 'master', + }, + }, + }, + fields: {}, +} + describe('normalizeOptimizationConfig', () => { it('returns runtime-safe defaults for nullish configs', () => { expect(normalizeOptimizationConfig(undefined)).toEqual({ @@ -75,6 +118,52 @@ describe('normalizeOptimizationConfig', () => { }) }) +describe('CtflEntry', () => { + it('passes through arbitrary consumer fields without recursively validating them', () => { + const parent = { + ...entryBase, + fields: {}, + } + const child = { + ...entryBase, + sys: { + ...entryBase.sys, + id: 'child-entry-id', + }, + fields: { + richText: { + nodeType: 'document', + data: {}, + content: [ + { + nodeType: 'embedded-entry-block', + data: { target: parent }, + content: [], + }, + ], + }, + }, + } + parent.fields = { + relatedEntry: child, + } + + expect(CtflEntry.safeParse(parent).success).toBe(true) + }) + + it('still validates Contentful system metadata', () => { + expect( + CtflEntry.safeParse({ + ...entryBase, + sys: { + ...entryBase.sys, + id: 123, + }, + }).success, + ).toBe(false) + }) +}) + describe('OptimizationEntry', () => { it('does not fabricate nt_config during parsing', () => { const result = OptimizationEntry.safeParse(optimizationEntryBase) @@ -85,4 +174,43 @@ describe('OptimizationEntry', () => { expect(result.data.fields.nt_config).toBeUndefined() }) + + it('still validates optimization-owned config fields', () => { + expect( + OptimizationEntry.safeParse({ + ...optimizationEntryBase, + fields: { + ...optimizationEntryBase.fields, + nt_config: { + components: [ + { + type: 'EntryReplacement', + baseline: { id: 123 }, + variants: [], + }, + ], + }, + }, + }).success, + ).toBe(false) + }) +}) + +describe('OptimizedEntry', () => { + it('still requires valid optimization references', () => { + expect( + OptimizedEntry.safeParse({ + ...entryBase, + fields: { + nt_experiences: [ + { + sys: { + id: 'missing-link-shape', + }, + }, + ], + }, + }).success, + ).toBe(false) + }) }) diff --git a/packages/universal/core-sdk/src/resolvers/OptimizedEntryResolver.test.ts b/packages/universal/core-sdk/src/resolvers/OptimizedEntryResolver.test.ts index 4d273274d..da4f9242d 100644 --- a/packages/universal/core-sdk/src/resolvers/OptimizedEntryResolver.test.ts +++ b/packages/universal/core-sdk/src/resolvers/OptimizedEntryResolver.test.ts @@ -1,5 +1,6 @@ // OptimizedEntryResolver.test.ts import { + isEntry, isEntryReplacementComponent, isEntryReplacementVariant, isOptimizationEntry, @@ -11,7 +12,7 @@ import { type SelectedOptimizationArray, } from '@contentful/optimization-api-client/api-schemas' import { describe, expect, it, rs } from '@rstest/core' -import type { Entry } from 'contentful' +import type { Entry, EntrySkeletonType } from 'contentful' import { mockLogger } from 'mocks' import { optimizedEntry as optimizedEntryFixture } from '../test/fixtures/optimizedEntry' @@ -22,6 +23,8 @@ const mockedLogger = rs.mocked(mockLogger) const RESOLUTION_WARNING_BASE = 'Could not resolve optimized entry variant:' +type TestEntry = Entry + const getOptimizedEntry = (): OptimizedEntry => { if (!isOptimizedEntry(optimizedEntryFixture)) { throw new Error('Fixture optimizedEntry is not an OptimizedEntry') @@ -66,6 +69,100 @@ const getEuropeVariantConfig = (): EntryReplacementVariant => { return maybeVariant } +const createTestEntry = (id: string, fields: Record = {}): TestEntry => { + const entry: unknown = { + fields, + metadata: { tags: [] }, + sys: { + type: 'Entry', + id, + contentType: { + sys: { + type: 'Link', + linkType: 'ContentType', + id: 'testContentType', + }, + }, + publishedVersion: 1, + createdAt: '2026-01-01T00:00:00.000Z', + updatedAt: '2026-01-01T00:00:00.000Z', + revision: 1, + space: { + sys: { + type: 'Link', + linkType: 'Space', + id: 'testSpace', + }, + }, + environment: { + sys: { + type: 'Link', + linkType: 'Environment', + id: 'testEnvironment', + }, + }, + }, + } + + if (!isEntry(entry)) { + throw new Error(`Expected test entry ${id} to match the Contentful entry schema`) + } + + return entry +} + +const createRichTextLinkedEntryDocument = (target: TestEntry): Record => ({ + nodeType: 'document', + data: {}, + content: [ + { + nodeType: 'embedded-entry-block', + data: { target }, + content: [], + }, + ], +}) + +const createOptimizationEntry = ({ + baselineEntry, + variantEntry, +}: { + baselineEntry: TestEntry + variantEntry: TestEntry +}): TestEntry => + createTestEntry('experience-entry', { + nt_name: 'Personalized featured post', + nt_type: 'nt_personalization', + nt_experience_id: 'experience-entry', + nt_config: { + components: [ + { + type: 'EntryReplacement', + baseline: { id: baselineEntry.sys.id }, + variants: [{ id: variantEntry.sys.id }], + }, + ], + }, + nt_variants: [variantEntry], + }) + +const createSelectedOptimizations = ({ + baselineEntry, + variantEntry, +}: { + baselineEntry: TestEntry + variantEntry: TestEntry +}): SelectedOptimizationArray => [ + { + experienceId: 'experience-entry', + variantIndex: 1, + variants: { + [baselineEntry.sys.id]: variantEntry.sys.id, + }, + sticky: false, + }, +] + describe('OptimizedEntryResolver', () => { describe('getOptimizationEntry', () => { it('returns the matching optimization entry for a selected experience', () => { @@ -433,6 +530,61 @@ describe('OptimizedEntryResolver', () => { ) }) + it('resolves the selected variant when an unrelated rich-text linked entry graph contains a cycle', () => { + const baselineFields: Record = {} + const baselineEntry = createTestEntry('baseline-entry', baselineFields) + const variantEntry = createTestEntry('variant-entry', { + internalTitle: 'Selected variant', + }) + const linkedEntry = createTestEntry('linked-entry', { + text: createRichTextLinkedEntryDocument(baselineEntry), + }) + const optimizationEntry = createOptimizationEntry({ baselineEntry, variantEntry }) + baselineFields.nt_experiences = [optimizationEntry] + baselineFields.featuredPosts = [linkedEntry] + + const result = OptimizedEntryResolver.resolve( + baselineEntry, + createSelectedOptimizations({ baselineEntry, variantEntry }), + ) + + expect(result.entry).toBe(variantEntry) + expect(result.selectedOptimization).toEqual( + expect.objectContaining({ + experienceId: 'experience-entry', + variantIndex: 1, + }), + ) + }) + + it('resolves the selected variant when the variant entry contains a rich-text linked entry cycle', () => { + const baselineFields: Record = {} + const variantFields: Record = { + internalTitle: 'Selected variant', + } + const baselineEntry = createTestEntry('baseline-entry', baselineFields) + const variantEntry = createTestEntry('variant-entry', variantFields) + const linkedEntry = createTestEntry('variant-linked-entry', { + text: createRichTextLinkedEntryDocument(variantEntry), + }) + variantFields.featuredPosts = [linkedEntry] + const optimizationEntry = createOptimizationEntry({ baselineEntry, variantEntry }) + baselineFields.nt_experiences = [optimizationEntry] + + const result = OptimizedEntryResolver.resolve( + baselineEntry, + createSelectedOptimizations({ baselineEntry, variantEntry }), + ) + + expect(result.entry).toBe(variantEntry) + expect(result.selectedOptimization).toEqual( + expect.objectContaining({ + experienceId: 'experience-entry', + variantIndex: 1, + }), + ) + }) + it('returns resolved data and optimization context for a matched optimization', () => { const { optimizationContext, resolvedData } = OptimizedEntryResolver.resolveWithContext( optimizedEntryFixture,