Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion integrationtests/FSXAProxyApiRemoteProjects.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
}
Expand Down
119 changes: 119 additions & 0 deletions src/modules/CaaSMapper.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
CaaSApi_CMSInputTextArea,
CaaSApi_CMSInputToggle,
CaaSApi_Content2Section,
CaaSApi_DataEntries,
CaaSApi_FSCatalog,
CaaSApi_FSDataset,
CaaSApi_FSIndex,
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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', () => {
Expand Down
29 changes: 28 additions & 1 deletion src/modules/CaaSMapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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],
Expand Down
Loading