diff --git a/CONTENT-MODEL.md b/CONTENT-MODEL.md index 0a91406..f4b095e 100644 --- a/CONTENT-MODEL.md +++ b/CONTENT-MODEL.md @@ -23,7 +23,7 @@ storefront's `LayoutConfig`. A slot only renders if its template declares it. Yo | SAP template (page type) | SAP slot positions | Contentstack field uid | |---|---|---| | **Shell** (header/footer, every page) | `SiteLogo`, `SearchBox`, `MiniCart`, `NavigationBar`, `SiteContext`, `SiteLinks`, `HeaderLinks`, `Footer` | `site_logo`, `search_box`, `mini_cart`, `navigation_bar`, `site_context`, `site_links`, `header_links`, `footer` | -| **LandingPage2Template** (home/landing) | `Section1`, `Section2`, `Section2A`, `Section2B`, `Section2C`, `Section3`, `Section4`, `Section5` | `section1`, `section2`, `section2_a`, `section2_b`, `section2_c`, `section3`, `section4`, `section5` | +| **LandingPage2Template** (home/landing) | `Section1`, `Section2A`, `Section2B`, `Section2C`, `Section3`, `Section4`, `Section5` | `section1`, `section2_a`, `section2_b`, `section2_c`, `section3`, `section4`, `section5` | | **ContentPage1Template** (FAQ, terms, …) | `Section1`, `Section2A/B/C`, `Section3`, `BodyContent`, `SideContent` | `section1`, `section2_a/b/c`, `section3`, `body_content`, `side_content` | | **ProductDetailsPageTemplate** (PDP) | `Summary`, `UpSelling`, `CrossSelling`, `Tabs`, `PlaceholderContentSlot` | `summary`, `up_selling`, `cross_selling`, `tabs`, `placeholder_content_slot` | | **ProductListPageTemplate** (PLP/category/search) | `ProductLeftRefinements`, `ProductGridSlot`, `ProductListSlot`, `SearchResultsGridSlot` | `product_left_refinements`, `product_grid_slot`, `product_list_slot`, `search_results_grid_slot` | @@ -49,7 +49,7 @@ Each **slot field is a multi-reference** to the component content types below. | Content type (uid) | Template | Slot fields (reference) | |---|---|---| -| `landing_page` | LandingPage2Template | `section1`, `section2`, `section2_a`, `section2_b`, `section2_c`, `section3`, `section4`, `section5` | +| `landing_page` | LandingPage2Template | `section1`, `section2_a`, `section2_b`, `section2_c`, `section3`, `section4`, `section5` | | `content_page` | ContentPage1Template | `section1`, `section2_a`, `section2_b`, `section2_c`, `section3`, `body_content`, `side_content` | | `product_page` | ProductDetailsPageTemplate | `summary`, `up_selling`, `cross_selling`, `tabs`, `placeholder_content_slot` | | `category_page` | ProductListPageTemplate | `product_left_refinements`, `product_grid_slot`, `product_list_slot`, `search_results_grid_slot` | diff --git a/README.md b/README.md index 829bab9..79ed357 100644 --- a/README.md +++ b/README.md @@ -171,7 +171,6 @@ block, with each field's JSDoc on hover. The complete set: | `pageTypeMapping` | per-`PageType` `{ contentTypeUid, slugField?, sharedSlug? }` (shared-layout pages) | | `additionalSlotFields` | extra `{ fieldUid: 'SapSlotPosition' }` beyond the built-in slot map | | `componentContentType` | content type for standalone component lookups (else components ship in pages) | -| `componentTypeMapping` | block uid → SAP typeCode (for author-named blocks without a `type_code`) | | `includeReferences` | reference fields to expand; defaults to all slot + header/footer fields | | `accessControl` | presentation-level gating — see below | | `timeoutMs` | `10000` — Delivery API call timeout | diff --git a/import-export/starter-pack/content_types/landing_page.json b/import-export/starter-pack/content_types/landing_page.json index 856fa09..4cfca3f 100644 --- a/import-export/starter-pack/content_types/landing_page.json +++ b/import-export/starter-pack/content_types/landing_page.json @@ -75,29 +75,6 @@ "instruction": "Maps to SAP slot Section1. Top hero band. Leave empty to keep SAP's." } }, - { - "display_name": "Section 2", - "uid": "section2", - "data_type": "reference", - "reference_to": [ - "simple_responsive_banner_component", - "simple_banner_component", - "product_carousel_component", - "cms_paragraph_component", - "cms_tab_paragraph_component", - "cms_link_component", - "cms_flex_component" - ], - "mandatory": false, - "multiple": true, - "unique": false, - "non_localizable": false, - "field_metadata": { - "ref_multiple": true, - "ref_multiple_content_types": true, - "instruction": "Maps to SAP slot Section2. Leave empty to keep SAP's." - } - }, { "display_name": "Section 2A", "uid": "section2_a", diff --git a/import-export/starter-pack/generate-content-types.mjs b/import-export/starter-pack/generate-content-types.mjs index 229c64e..cc04a09 100644 --- a/import-export/starter-pack/generate-content-types.mjs +++ b/import-export/starter-pack/generate-content-types.mjs @@ -178,7 +178,9 @@ const contentTypes = [ 'CMS page on SAP LandingPage2Template (home / marketing landing).', [...pageMeta('LandingPage2Template'), slot('section1', 'Section 1', 'Section1', EDITORIAL, 'Top hero band. '), - slot('section2', 'Section 2', 'Section2'), + // NB: LandingPage2Template has no bare `Section2` render position (that + // belongs to CategoryPageTemplate), so this page type intentionally omits a + // `section2` slot field — only Section2A/2B/2C exist here. slot('section2_a', 'Section 2A', 'Section2A'), slot('section2_b', 'Section 2B', 'Section2B'), slot('section2_c', 'Section 2C', 'Section2C'), diff --git a/src/client/contentstack-client.service.ts b/src/client/contentstack-client.service.ts index 3c3e0c7..83d6056 100644 --- a/src/client/contentstack-client.service.ts +++ b/src/client/contentstack-client.service.ts @@ -229,10 +229,13 @@ export class ContentstackClientService { title?: string, includeRefs: string[] = [], locale?: string, + access?: EntryAccessOptions, ): Observable { const csLocale = this.resolveLocale(locale); const key = makeStateKey( - `cs-global:${contentTypeUid}:${title ?? '*'}:${csLocale ?? '*'}`, + `cs-global:${contentTypeUid}:${title ?? '*'}:${csLocale ?? '*'}${this.restrictions.cacheKeySuffix( + access?.permissions, + )}`, ); return this.withTransferState(key, () => { // Resolve the shell's component references inline (same as the page path), @@ -252,7 +255,14 @@ export class ContentstackClientService { query.where('title', QueryOperation.EQUALS, title); } return query.find().then((res) => { - const entry = res?.entries?.[0]; + let entry = res?.entries?.[0]; + // Filter gated content on every fetch (before the SSR TransferState write, + // and again on any client-side re-fetch) — the shell is gated like the + // page. gateRoot is false: the shell entry itself is never hidden, only + // its restricted nested components are stripped. + if (entry && access) { + entry = this.restrictions.sanitizeForTransfer(entry, access.permissions, false); + } if (entry && this.config.contentstack?.delivery?.livePreview) { this.tagForLivePreview(entry, contentTypeUid); } @@ -324,30 +334,38 @@ export class ContentstackClientService { access?.permissions, )}`, ); - return this.withTransferState(key, () => { - let entries = this.stack.contentType(contentTypeUid).entry(); - if (csLocale) { - entries = entries.locale(csLocale); - if (this.includeFallback) { - entries = entries.includeFallback(); + return this.withTransferState( + key, + () => { + let entries = this.stack.contentType(contentTypeUid).entry(); + if (csLocale) { + entries = entries.locale(csLocale); + if (this.includeFallback) { + entries = entries.includeFallback(); + } } - } - return entries - .query() - .where('uid', QueryOperation.INCLUDES, uids) - .find() - .then((res) => { - const list = res?.entries ?? []; - // Filter gated content BEFORE it is persisted to TransferState: - // each restricted entry is redacted to a tags-only stub so the adapter - // still counts it as "found" (no OCC refetch) without shipping content. - return access - ? list.map((entry) => - this.restrictions.sanitizeForTransfer(entry, access.permissions, access.gateRoot), - ) - : list; - }); - }); + return entries + .query() + .where('uid', QueryOperation.INCLUDES, uids) + .find() + .then((res) => { + const list = res?.entries ?? []; + // Filter gated content BEFORE it is persisted to TransferState: + // each restricted entry is redacted to a tags-only stub so the adapter + // still counts it as "found" (no OCC refetch) without shipping content. + return access + ? list.map((entry) => + this.restrictions.sanitizeForTransfer(entry, access.permissions, access.gateRoot), + ) + : list; + }); + }, + // Failure fallback: an empty array, never `undefined` — the return type is + // `ContentstackEntry[]` and callers (component adapter) flat-map the result, + // so a failed batch must degrade to "no entries" (→ OCC/shell fallback), + // not to a value that throws on `.map`/`.filter`. + [], + ); } /** diff --git a/src/cms/access/contentstack-restrictions.service.spec.ts b/src/cms/access/contentstack-restrictions.service.spec.ts index 415d959..a5d4bfa 100644 --- a/src/cms/access/contentstack-restrictions.service.spec.ts +++ b/src/cms/access/contentstack-restrictions.service.spec.ts @@ -110,6 +110,14 @@ describe('ContentstackRestrictionsService', () => { // Different audiences get different keys (no cross-serving). expect(svc.cacheKeySuffix(new Set(['_require-login']))).not.toBe(a); }); + + it('is injective even when a token contains the join delimiter', () => { + // Without per-token encoding, {'a|b'} and {'a','b'} would collide on `|`, + // letting one audience receive another's permission-filtered cache entry. + const single = svc.cacheKeySuffix(new Set(['a|b'])); + const pair = svc.cacheKeySuffix(new Set(['a', 'b'])); + expect(single).not.toBe(pair); + }); }); describe('redactEntry()', () => { diff --git a/src/cms/access/contentstack-restrictions.service.ts b/src/cms/access/contentstack-restrictions.service.ts index d67d60e..d546a75 100644 --- a/src/cms/access/contentstack-restrictions.service.ts +++ b/src/cms/access/contentstack-restrictions.service.ts @@ -117,7 +117,12 @@ export class ContentstackRestrictionsService { if (!permissions || !permissions.size) { return ''; } - return `:acl=${[...permissions].sort().join('|')}`; + // Encode each token before joining so the `|` delimiter can never collide: + // `encodeURIComponent` percent-escapes any literal `|` (→ `%7C`), so the + // suffix is injective — `{'a|b'}` and `{'a','b'}` map to distinct keys, and a + // token that happens to contain the delimiter can't leak another audience's + // cached, permission-filtered payload. + return `:acl=${[...permissions].sort().map(encodeURIComponent).join('|')}`; } /** Whether a value is a Contentstack entry node (an object carrying a string uid). */ diff --git a/src/cms/adapters/contentstack-cms-component.adapter.spec.ts b/src/cms/adapters/contentstack-cms-component.adapter.spec.ts index e403e81..5105534 100644 --- a/src/cms/adapters/contentstack-cms-component.adapter.spec.ts +++ b/src/cms/adapters/contentstack-cms-component.adapter.spec.ts @@ -1,4 +1,4 @@ -import { Observable, of } from 'rxjs'; +import { Observable, of, throwError } from 'rxjs'; import { ContentstackCmsComponentAdapter } from './contentstack-cms-component.adapter'; import { ContentstackRestrictionsService } from '../access/contentstack-restrictions.service'; import { ContentstackComponentTypeRegistry } from '../model/contentstack-component-type.registry'; @@ -414,4 +414,49 @@ describe('ContentstackCmsComponentAdapter', () => { expect(occComponentAdapter.findComponentsByIds).not.toHaveBeenCalled(); }); }); + + describe('batch-fetch error handling (#3)', () => { + const entry = (uid: string, type: string) => ({ + uid, + _content_type_uid: type, + created_at: '2026-01-01T00:00:00.000Z', + }); + + it('degrades a failed content-type group to empty instead of throwing, keeping healthy groups', () => { + // Two distinct learned types → two forkJoin branches; one fails outright. + const getEntriesByUids = jest.fn((type: string) => + type === 'type_b' + ? throwError(() => new Error('CMS down')) + : of([entry('bltaaaa', 'type_a')]), + ); + const { adapter } = create({ + registry: { bltaaaa: 'type_a', bltbbbb: 'type_b' }, + cs: { componentContentType: undefined }, // rely on learned types only + client: { getEntriesByUids }, + }); + + let res!: { uid: string }[]; + expect(() => { + res = firstValue(adapter.findComponentsByIds(['bltaaaa', 'bltbbbb'], ctx)); + }).not.toThrow(); + const uids = res.map((c) => c.uid); + expect(uids).toContain('bltaaaa'); // healthy group resolved + expect(uids).toContain('bltbbbb'); // failed group degraded to a benign shell + }); + + it('tolerates a client that emits a non-array (undefined) without crashing', () => { + const getEntriesByUids = jest.fn().mockReturnValue(of(undefined as never)); + const { adapter } = create({ + registry: { bltaaaa: 'type_a' }, + cs: { componentContentType: undefined }, + client: { getEntriesByUids }, + }); + + let res!: { uid: string }[]; + expect(() => { + res = firstValue(adapter.findComponentsByIds(['bltaaaa'], ctx)); + }).not.toThrow(); + expect(Array.isArray(res)).toBe(true); + }); + }); }); diff --git a/src/cms/adapters/contentstack-cms-component.adapter.ts b/src/cms/adapters/contentstack-cms-component.adapter.ts index 52e0d24..213b709 100644 --- a/src/cms/adapters/contentstack-cms-component.adapter.ts +++ b/src/cms/adapters/contentstack-cms-component.adapter.ts @@ -1,6 +1,6 @@ import { Inject, Injectable, Optional } from '@angular/core'; import { Observable, combineLatest, forkJoin, of } from 'rxjs'; -import { map, switchMap } from 'rxjs/operators'; +import { catchError, map, switchMap } from 'rxjs/operators'; import { CmsComponent, CmsComponentAdapter, @@ -168,13 +168,24 @@ export class ContentstackCmsComponentAdapter implements CmsComponentAdapter { switchMap(([locale, permissions]) => forkJoin( [...byType.entries()].map(([type, uids]) => - permissions + (permissions ? this.client.getEntriesByUids(type, uids, locale, { permissions, gateRoot: true }) - : this.client.getEntriesByUids(type, uids, locale), + : this.client.getEntriesByUids(type, uids, locale) + ).pipe( + // Defense in depth: one content-type group failing must not fail the + // whole batch (forkJoin errors if any source errors) — degrade that + // group to no entries so its ids fall back to OCC/shells below. + catchError(() => of([] as ContentstackEntry[])), + ), ), ).pipe( switchMap((groups: ContentstackEntry[][]) => { - const entries = groups.flat(); + // Guard against a malformed group (e.g. a client that ever emits a + // non-array) so `.flat()`/`.map()` can't throw on `undefined`. + const entries = groups + .filter((g): g is ContentstackEntry[] => Array.isArray(g)) + .flat() + .filter((e): e is ContentstackEntry => !!e); const accessible = permissions ? entries.filter((entry) => this.restrictions.isEntryAccessible(entry, permissions)) : entries; diff --git a/src/cms/adapters/contentstack-cms-page.adapter.spec.ts b/src/cms/adapters/contentstack-cms-page.adapter.spec.ts index d16a4fd..3ae20fb 100644 --- a/src/cms/adapters/contentstack-cms-page.adapter.spec.ts +++ b/src/cms/adapters/contentstack-cms-page.adapter.spec.ts @@ -331,6 +331,64 @@ describe('ContentstackCmsPageAdapter', () => { expect(client.getGlobalSlots).not.toHaveBeenCalled(); }); + describe('gating the shell (#4)', () => { + const GATING = { + accessControl: { enabled: true, accessField: 'access_tags', rolePrefix: '_require-' }, + }; + + it('threads permissions + gateRoot:false into the shell fetch when gating is on', () => { + const getGlobalSlots = jest.fn().mockReturnValue(of(undefined)); + const { adapter } = create({ + cs: { ...GATING, globalSlots: { contentType: 'global_slots', title: 'Global' } }, + user: { roles: ['b2badmingroup'] }, + client: { getPageBySlug: jest.fn().mockReturnValue(of(undefined)), getGlobalSlots }, + }); + + firstValue(adapter.load(ctx('home'))); + const access = getGlobalSlots.mock.calls[0][4]; + expect(access?.permissions).toBeInstanceOf(Set); + expect(access?.permissions.has('_require-login')).toBe(true); + expect(access?.permissions.has('_require-b2badmingroup')).toBe(true); + // The shell root is never hidden — only restricted nested components drop. + expect(access?.gateRoot).toBe(false); + }); + + it('passes the permission set into buildStructure so restricted shell components drop on CSR', () => { + const buildStructure = jest.fn().mockReturnValue({ slots: {}, components: [] }); + const { adapter } = create({ + cs: { ...GATING, globalSlots: { contentType: 'global_slots' } }, + user: { roles: [] }, // logged in, no roles + client: { + getPageBySlug: jest.fn().mockReturnValue(of(undefined)), + getGlobalSlots: jest.fn().mockReturnValue(of({ uid: 'global-entry' })), + }, + normalizer: { buildStructure }, + occ: { + load: jest + .fn() + .mockReturnValue(of({ page: { template: 'T', slots: {} }, components: [] })), + }, + }); + + firstValue(adapter.load(ctx('home'))); + const permissions = buildStructure.mock.calls[0][1] as Set; + expect(permissions).toBeInstanceOf(Set); + expect(permissions.has('_require-login')).toBe(true); + }); + + it('omits the access arg (unchanged behavior) when gating is off', () => { + const getGlobalSlots = jest.fn().mockReturnValue(of(undefined)); + const { adapter } = create({ + cs: { globalSlots: { contentType: 'global_slots' } }, + client: { getPageBySlug: jest.fn().mockReturnValue(of(undefined)), getGlobalSlots }, + }); + + firstValue(adapter.load(ctx('home'))); + // 4 args only — no 5th access argument when access control is off. + expect(getGlobalSlots.mock.calls[0][4]).toBeUndefined(); + }); + }); + it('always requests the flat-nav include chain for every nav field, independent of depth', () => { const { adapter, client } = create({ cs: { globalSlots: { contentType: 'global_slots' } }, diff --git a/src/cms/adapters/contentstack-cms-page.adapter.ts b/src/cms/adapters/contentstack-cms-page.adapter.ts index 44cf301..16494bc 100644 --- a/src/cms/adapters/contentstack-cms-page.adapter.ts +++ b/src/cms/adapters/contentstack-cms-page.adapter.ts @@ -130,7 +130,25 @@ export class ContentstackCmsPageAdapter implements CmsPageAdapter { }) : this.client.getPageBySlug(contentType, slugField, slug, includeRefs, locale); const global$ = global - ? this.client.getGlobalSlots(global.contentType, global.title, globalIncludeRefs, locale) + ? permissions + ? this.client.getGlobalSlots( + global.contentType, + global.title, + globalIncludeRefs, + locale, + { + permissions, + // The shell root is never hidden; only restricted nested shell + // components (e.g. a login-only header link) are filtered out. + gateRoot: false, + }, + ) + : this.client.getGlobalSlots( + global.contentType, + global.title, + globalIncludeRefs, + locale, + ) : of(undefined); // Hybrid base: the SAP page for this route. A CMS failure must never // break navigation, so degrade to no-base on error. @@ -154,7 +172,9 @@ export class ContentstackCmsPageAdapter implements CmsPageAdapter { } const csStructure = entry ? this.normalizer.convert(entry, {}, permissions) : undefined; - const globalStructure = globalEntry ? this.toGlobalStructure(globalEntry) : undefined; + const globalStructure = globalEntry + ? this.toGlobalStructure(globalEntry, permissions) + : undefined; // Nothing from Contentstack and no OCC base → not-found (as before). if (!csStructure && !occBase) { @@ -175,8 +195,15 @@ export class ContentstackCmsPageAdapter implements CmsPageAdapter { * base and the page: an authored shell overrides OCC's, and unauthored shell * slots fall through to OCC. */ - protected toGlobalStructure(globalEntry: ContentstackCmsPageEntry): CmsStructureModel { - const { slots, components } = this.normalizer.buildStructure(globalEntry); + protected toGlobalStructure( + globalEntry: ContentstackCmsPageEntry, + permissions?: Set, + ): CmsStructureModel { + // Filter the shell by the viewer's permissions, mirroring the page path + // (normalizer.convert(entry, {}, permissions)). This is the render-time gate; + // the client's sanitizeForTransfer independently strips restricted nested + // entries from every fetch (SSR + CSR) before they reach here — belt and braces. + const { slots, components } = this.normalizer.buildStructure(globalEntry, permissions); return { page: { slots }, components }; } diff --git a/src/cms/converters/components/contentstack-cms-navigation-component.normalizer.spec.ts b/src/cms/converters/components/contentstack-cms-navigation-component.normalizer.spec.ts index a12d3dc..4ba351a 100644 --- a/src/cms/converters/components/contentstack-cms-navigation-component.normalizer.spec.ts +++ b/src/cms/converters/components/contentstack-cms-navigation-component.normalizer.spec.ts @@ -133,4 +133,63 @@ describe('ContentstackCmsNavigationComponentNormalizer', () => { const component = normalizer.convert(flatComponent([withStub])); expect(component.navigationNode?.children?.[0].entries).toBeUndefined(); }); + + describe('malformed-data guards (no infinite recursion)', () => { + it('treats a self-referencing node (parent_id === node_id) as top-level', () => { + const build = () => normalizer.convert(flatComponent([node('Solo', 'Solo', 'Solo', 1)])); + // Must not recurse forever: complete and surface the node once, at the top. + const component = build(); + expect(component.navigationNode?.children?.map((n) => n.uid)).toEqual(['Solo']); + expect(component.navigationNode?.children?.[0].children).toBeUndefined(); + }); + + it('does not hang on a parent/child cycle (A→B→A); a pure cycle yields no roots', () => { + const component = normalizer.convert( + flatComponent([node('A', 'A', 'B', 1), node('B', 'B', 'A', 1)]), + ); + // Neither node is top-level (each names the other as parent), so the menu is + // empty — the point is that it terminates instead of overflowing the stack. + expect(component.navigationNode?.children).toEqual([]); + }); + + it('deduplicates a repeated node_id (first wins) rather than duplicating a subtree', () => { + const component = normalizer.convert( + flatComponent([ + node('Dup', 'First', '', 1, 'blt_first_link'), + node('Dup', 'Second', '', 2, 'blt_second_link'), + node('Child', 'Child', 'Dup', 1), + ]), + ); + const tops = component.navigationNode?.children ?? []; + expect(tops.map((n) => n.uid)).toEqual(['Dup']); // only one 'Dup', not two + expect(tops[0].title).toBe('First'); // first occurrence wins + expect(tops[0].children?.map((n) => n.uid)).toEqual(['Child']); + }); + + it('falls a blank node_id back to the entry uid instead of colliding with the root', () => { + const component = normalizer.convert( + flatComponent([node('', 'Empty', '', 1), node('Real', 'Real', '', 2)]), + ); + const tops = component.navigationNode?.children ?? []; + // The blank-id node takes its uid ('blt_') as identity, so it stays a normal + // top-level node and does NOT adopt the other root nodes as children. + expect(tops.map((n) => n.uid)).toEqual(['blt_', 'Real']); + expect(tops.find((n) => n.uid === 'blt_')?.children).toBeUndefined(); + }); + + it('ignores a disconnected parent cycle while still building the reachable tree', () => { + // Root → Mid is a real tree; the stray pair (X→Y→X) never reaches the root, + // so it is dropped rather than walked into forever. + const component = normalizer.convert( + flatComponent([ + node('Root', 'Root', '', 1), + node('Mid', 'Mid', 'Root', 1), + node('X', 'X', 'Y', 1), + node('Y', 'Y', 'X', 1), + ]), + ); + expect(component.navigationNode?.children?.map((n) => n.uid)).toEqual(['Root']); + expect(component.navigationNode?.children?.[0].children?.map((n) => n.uid)).toEqual(['Mid']); + }); + }); }); diff --git a/src/cms/converters/components/contentstack-cms-navigation-component.normalizer.ts b/src/cms/converters/components/contentstack-cms-navigation-component.normalizer.ts index c24d4a5..7f3b9a7 100644 --- a/src/cms/converters/components/contentstack-cms-navigation-component.normalizer.ts +++ b/src/cms/converters/components/contentstack-cms-navigation-component.normalizer.ts @@ -47,9 +47,31 @@ export class ContentstackCmsNavigationComponentNormalizer implements Converter< * top-level nodes, matching the single-root shape Spartacus expects. */ private buildFromFlat(nodes: ContentstackEntry[], rootUid: string): CmsNavigationNode { - const byParent = new Map(); + // Guard against malformed authoring data (duplicate ids, self-references, + // parent cycles) with three cheap invariants that make the recursion below + // provably terminating WITHOUT a per-node visited set: + // 1) unique identities — dedupe by node_id (falling back to the always- + // unique entry uid for a blank/non-string node_id); + // 2) no self-parenting — a node naming itself is reparented to the root; + // 3) a single parent per node (the data model gives each node one parent_id). + // Together these mean the root-reachable nodes form a forest: each node has + // exactly one path back to the root, so build() visits every node at most + // once. A parent cycle that never reaches the root is simply never entered + // (build starts at '') — so it terminates and drops the orphaned cycle. + + // 1) Dedupe by identity (first wins). + const byId = new Map(); for (const n of nodes) { - const key = this.parentKey(n); + const id = this.nodeId(n); + if (!byId.has(id)) { + byId.set(id, n); + } + } + + // 2) Group by parent; reparent a self-referencing node to the root. + const byParent = new Map(); + for (const n of byId.values()) { + const key = this.parentKey(n) === this.nodeId(n) ? '' : this.parentKey(n); const siblings = byParent.get(key) ?? []; siblings.push(n); byParent.set(key, siblings); @@ -58,9 +80,10 @@ export class ContentstackCmsNavigationComponentNormalizer implements Converter< siblings.sort((a, b) => this.sortOrder(a) - this.sortOrder(b)); } + // 3) Build the tree top-down. Linear in the number of nodes (see invariants). const build = (parentKey: string): CmsNavigationNode[] => (byParent.get(parentKey) ?? []).map((n) => { - const nodeId = (n['node_id'] as string) ?? n.uid; + const nodeId = this.nodeId(n); const node: CmsNavigationNode = { uid: nodeId, title: n['title'] as string }; const links = this.resolvedList(n['links']); @@ -77,6 +100,17 @@ export class ContentstackCmsNavigationComponentNormalizer implements Converter< return { uid: rootUid, children: build('') }; } + /** + * A node's stable identity: its `node_id` when a non-empty string, otherwise + * the entry uid. Using the uid for a blank/non-string node_id keeps every + * identity unique and non-empty, so it can never collide with the root + * sentinel ('') or another node. + */ + private nodeId(node: ContentstackEntry): string { + const id = node['node_id']; + return typeof id === 'string' && id.length ? id : node.uid; + } + /** The parent grouping key: a node's `parent_id`, normalized to '' for top level. */ private parentKey(node: ContentstackEntry): string { const parent = node['parent_id']; diff --git a/src/config/contentstack-config.ts b/src/config/contentstack-config.ts index 8422623..7b24623 100644 --- a/src/config/contentstack-config.ts +++ b/src/config/contentstack-config.ts @@ -208,13 +208,6 @@ export abstract class ContentstackConfig { */ pageTypeMapping?: Partial>; - /** - * Optional map of Contentstack block uid → Spartacus typeCode, consulted by - * the page normalizer when a block has no explicit `type_code` field. Lets an - * app map author-named blocks to stock component types without editing content. - */ - componentTypeMapping?: Record; - /** * Shared/global slots (header, footer, navigation, logo, …) authored once and * merged into every page. When set, the page adapter fetches this entry and