fix(CaaSMapper): handle broken references (#252)

* fix(CaaSMapper): handle broken references by returning null for entries with null identifiers

---------

Co-authored-by: jan-philipp.nierlein <jan-philipp.nierlein@crownpeak.com>

Author: nierlein-espirit

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',
       }

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', () => {

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],