diff --git a/integrationtests/FSXAProxyApiRemoteProjects.test.ts b/integrationtests/FSXAProxyApiRemoteProjects.test.ts index 12f28d2..6b6e4ef 100644 --- a/integrationtests/FSXAProxyApiRemoteProjects.test.ts +++ b/integrationtests/FSXAProxyApiRemoteProjects.test.ts @@ -155,7 +155,7 @@ describe('FSXAProxyAPIRemoteProjects should resolve references', () => { return res.status === 200 }, { - timeoutMs: 10000, + timeoutMs: 20000, pollIntervalMs: 500, errorMessage: 'PageRef not available in CaaS after creation', } diff --git a/src/modules/CaaSMapper.spec.ts b/src/modules/CaaSMapper.spec.ts index 39dff5a..75aaefa 100644 --- a/src/modules/CaaSMapper.spec.ts +++ b/src/modules/CaaSMapper.spec.ts @@ -18,6 +18,7 @@ import { CaaSApi_CMSInputTextArea, CaaSApi_CMSInputToggle, CaaSApi_Content2Section, + CaaSApi_DataEntries, CaaSApi_FSCatalog, CaaSApi_FSDataset, CaaSApi_FSIndex, @@ -701,6 +702,28 @@ describe('CaaSMapper', () => { expect(mapper.mapDataEntries).not.toHaveBeenCalled() expect(mapper.registerReferencedItem).not.toHaveBeenCalled() }) + it('should return null and not register a reference if the DatasetReference target has a null identifier (broken reference)', async () => { + const api = createApi() + const mapper = new CaaSMapper(api, 'de', {}, createLogger()) + mapper.registerReferencedItem = jest.fn() + const entry = createDatasetReference() + ;(entry.value as any).target.identifier = null + await expect( + mapper.mapDataEntry(entry, createPath()) + ).resolves.toBeNull() + expect(mapper.registerReferencedItem).not.toHaveBeenCalled() + }) + it('should return null and not register a reference if the DatasetReference target itself is null (broken reference)', async () => { + const api = createApi() + const mapper = new CaaSMapper(api, 'de', {}, createLogger()) + mapper.registerReferencedItem = jest.fn() + const entry = createDatasetReference() + ;(entry.value as any).target = null + await expect( + mapper.mapDataEntry(entry, createPath()) + ).resolves.toBeNull() + expect(mapper.registerReferencedItem).not.toHaveBeenCalled() + }) }) describe('CMS_INPUT_TOGGLE', () => { @@ -929,6 +952,30 @@ describe('CaaSMapper', () => { entry.value!.remoteProject ) }) + it('should return null and not register a reference on Media entries with a null identifier (broken reference)', async () => { + const api = createApi() + const mapper = new CaaSMapper(api, 'de', {}, createLogger()) + mapper.registerReferencedItem = jest.fn() + const path = createPath() + const entry: CaaSApi_FSReference = { + name: faker.lorem.word(), + value: { + fsType: 'Media', + name: faker.lorem.word(), + identifier: null as any, + uid: faker.lorem.word(), + uidType: 'MEDIASTORE_LEAF', + url: faker.lorem.word(), + mediaType: 'PICTURE', + remoteProject: 'main', + } as any, + fsType: 'FS_REFERENCE', + } + await expect( + mapper.mapDataEntry(entry, path) + ).resolves.toBeNull() + expect(mapper.registerReferencedItem).not.toHaveBeenCalled() + }) it('should handle PageRef & GCAPage separately', async () => { const api = createApi() const mapper = new CaaSMapper(api, 'de', {}, createLogger()) @@ -971,6 +1018,27 @@ describe('CaaSMapper', () => { expectedGCARef ) }) + it('should return null on PageRef/GCAPage entries with a null identifier (broken reference)', async () => { + const api = createApi() + const mapper = new CaaSMapper(api, 'de', {}, createLogger()) + const path = createPath() + const entry: CaaSApi_FSReference = { + name: faker.lorem.word(), + value: { + fsType: 'PageRef', + name: faker.lorem.word(), + identifier: null as any, + uid: faker.lorem.word(), + uidType: 'SITESTORE_LEAF', + url: faker.lorem.word(), + remoteProject: 'remote-project', + }, + fsType: 'FS_REFERENCE', + } + await expect(mapper.mapDataEntry(entry, path)).resolves.toBeNull() + entry.value!.fsType = 'GCAPage' + await expect(mapper.mapDataEntry(entry, path)).resolves.toBeNull() + }) it('should return corrupted entries as-is', async () => { const api = createApi() const mapper = new CaaSMapper(api, 'de', {}, createLogger()) @@ -1170,6 +1238,57 @@ describe('CaaSMapper', () => { {} ) }) + // Regression test for CAAS-2680: a single broken Media reference anywhere in + // an element used to throw synchronously (unifyId -> null.indexOf) and reject + // the whole document. It must now be skipped (mapped to null) while the rest + // of the element is returned normally. Uses the real mapper (no mocks) so an + // unguarded null identifier would actually throw here. + it('should skip a broken/null Media reference and still return the rest of the element', async () => { + const mapper = new CaaSMapper(createApi(), 'de', {}, createLogger()) + const entries = { + headline: { + fsType: 'CMS_INPUT_TEXT', + name: 'headline', + value: 'Weihnachtssocken', + }, + brokenImage: { + fsType: 'FS_REFERENCE', + name: 'brokenImage', + value: { + fsType: 'Media', + name: faker.lorem.word(), + identifier: null as any, + uid: '01_221021_4f3_weihnachtssocken_stage_b_1', + uidType: 'MEDIASTORE_LEAF', + url: faker.lorem.word(), + mediaType: 'PICTURE', + remoteProject: 'main', + }, + }, + validImage: { + fsType: 'FS_REFERENCE', + name: 'validImage', + value: { + fsType: 'Media', + name: faker.lorem.word(), + identifier: faker.string.uuid(), + uid: faker.lorem.word(), + uidType: 'MEDIASTORE_LEAF', + url: faker.lorem.word(), + mediaType: 'PICTURE', + }, + }, + } as any as CaaSApi_DataEntries + + const result = await mapper.mapDataEntries(entries, createPath()) + + // the broken reference is dropped, not thrown + expect(result.brokenImage).toBeNull() + // the rest of the element survives + expect(result.headline).toBe('Weihnachtssocken') + expect(result.validImage).not.toBeNull() + expect(typeof result.validImage).toBe('string') + }) }) describe('mapSection', () => { diff --git a/src/modules/CaaSMapper.ts b/src/modules/CaaSMapper.ts index e3e046f..786a8e3 100644 --- a/src/modules/CaaSMapper.ts +++ b/src/modules/CaaSMapper.ts @@ -364,6 +364,13 @@ export class CaaSMapper { ) ) } else if (entry.value.fsType === 'DatasetReference') { + if (!entry.value.target?.identifier) { + this.logger.warn( + 'Skipping DatasetReference with a broken/null identifier', + { path: path.join('/') } + ) + return null + } return this.registerReferencedItem( entry.value.target.identifier, path, @@ -413,12 +420,26 @@ export class CaaSMapper { case 'FS_REFERENCE': if (!entry.value) return null if (entry.value.fsType === 'Media') { + if (!entry.value.identifier) { + this.logger.warn( + 'Skipping Media reference with a broken/null identifier', + { path: path.join('/') } + ) + return null + } return this.registerReferencedItem( entry.value.identifier, path, entry.value.remoteProject || remoteProjectId ) } else if (['PageRef', 'GCAPage'].includes(entry.value.fsType)) { + if (!entry.value.identifier) { + this.logger.warn( + 'Skipping PageRef/GCAPage reference with a broken/null identifier', + { path: path.join('/') } + ) + return null + } const reference: Reference = { type: 'Reference', referenceId: entry.value.identifier, @@ -437,7 +458,13 @@ export class CaaSMapper { .map((record, index) => { const identifier: string | undefined = record?.value?.target?.identifier - if (!identifier) return null + if (!identifier) { + this.logger.warn( + 'Skipping FS_INDEX record with a broken/null identifier', + { path: [...path, index].join('/') } + ) + return null + } return this.registerReferencedItem( identifier, [...path, index],