Skip to content

fix(presentation-2): handle malformed manifests with incorrect structure #59

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
edsilv wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(presentation-2): handle malformed manifests with incorrect structure #59

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fda6bee
fix(presentation-2): handle malformed manifests with incorrect structure
edsilv Jan 7, 2026
585ebd6
ts error fix
edsilv Jan 7, 2026
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
35 changes: 35 additions & 0 deletions __tests__/presentation-2-parser/upgrade.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import goettingen from '../../fixtures/presentation-2/uni-goettingen.json';
import villanovaManifest from '../../fixtures/presentation-2/villanova-manifest.json';
import wikimediaProxy from '../../fixtures/presentation-2/wikimedia-proxy.json';
import malformedImageAnnotation from '../../fixtures/presentation-2/malformed-image-annotation.json';
import stAndrewsMalformed from '../../fixtures/presentation-2/st-andrews-malformed.json';
import { convertPresentation2, presentation2to3 } from '../../src/presentation-2';

describe('Presentation 2 to 3', () => {
Expand Down Expand Up @@ -2636,4 +2637,38 @@ describe('Presentation 2 to 3', () => {
expect(annotation?.motivation).toEqual('painting');
expect(annotation?.body).toBeDefined();
});

test('V2 context manifest with V3 structure (items instead of sequences)', () => {
// Some manifests (e.g., St. Andrews) have @context: "http://iiif.io/api/presentation/2/context.json"
// but use v3-style structure with items array instead of v2 sequences[0].canvases.
// The converter should detect this and handle it appropriately.
// Note: We don't validate strictly here because the source manifest may be missing
// required properties like canvas width/height - we just verify the structure converts.
const result = presentation2to3.traverseManifest(stAndrewsMalformed as any);

// Verify the canvas was correctly converted
expect(result.items).toBeDefined();
expect(result.items?.length).toBeGreaterThan(0);

const canvas = result.items?.[0];
expect(canvas).toBeDefined();
expect(canvas?.id).toEqual('https://collections.st-andrews.ac.uk/762345/manifest/canvas/406403');
expect(canvas?.type).toEqual('Canvas');

// Verify the annotation page and annotation were correctly converted
const annotationPage = canvas?.items?.[0];
expect(annotationPage).toBeDefined();
expect(annotationPage?.type).toEqual('AnnotationPage');

const annotation = annotationPage?.items?.[0];
expect(annotation).toBeDefined();
expect(annotation?.type).toEqual('Annotation');
expect(annotation?.motivation).toEqual('painting');

// Verify the body (3D model) was correctly converted
const body = annotation?.body as any;
expect(body).toBeDefined();
expect(body?.id).toEqual('https://collections.st-andrews.ac.uk/media/406403/406403.glb');
expect(body?.format).toEqual('model/gltf-binary');
Comment on lines +2641 to +2672
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test does not cover edge cases such as: canvases with missing id property, canvases with both v2 and v3 properties, annotations with array of bodies, annotations with missing target property, or empty annotation pages. Consider adding test cases for these scenarios to ensure robust handling.

Copilot uses AI. Check for mistakes.
});
});
62 changes: 62 additions & 0 deletions fixtures/presentation-2/st-andrews-malformed.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
{
"@context": "http://iiif.io/api/presentation/2/context.json",
"@type": "sc:Manifest",
"@id": "https://collections.st-andrews.ac.uk/762345/manifest",
"label": "Dura Den Fossil Fish",
"description": "Slab containing two fossilised Holopychius flemingi fish, which lived during the Devonian period, around 400 million years ago. The unusual bone structure of the fish, with fins attached to stumps, attracted the attention of prominent scientists including Charles Lyell. This fueled the debate on evolution as they could be interpreted as examples of Darwin's 'Transitional Forms' between fish with fins and animals with legs.<br/> <br/>The fossil is unusual as the fish are perfectly formed. Normally as they are subjected to lots of heat and pressure, flattened and often scavenged before found, you do not get a full skeleton. The fish appear clustered together as they were buried during a sandstorm whilst they were forced together as the rivers were drying up.",
"attribution": "© The University of St Andrews",
"logo": "https://collections.st-andrews.ac.uk/uv/logo.png",
"metadata": [
{ "label": "Identifier", "value": "GE1049" },
{ "label": "Title", "value": "Dura Den Fossil Fish" },
{
"label": "Record URL",
"value": "<a href='https://collections.st-andrews.ac.uk/item/dura-den-fossil-fish/762345'>https://collections.st-andrews.ac.uk/item/dura-den-fossil-fish/762345</a>"
},
{
"label": "Subjects",
"value": " <a href='https://collections.st-andrews.ac.uk/search/?query=Subject:\"geology\"&form=grid&mode=query'>geology</a>,<a href='https://collections.st-andrews.ac.uk/search/?query=Subject:\"fossil\"&form=grid&mode=query'>fossil</a>,<a href='https://collections.st-andrews.ac.uk/search/?query=Subject:\"fish\"&form=grid&mode=query'>fish</a>,<a href='https://collections.st-andrews.ac.uk/search/?query=Subject:\"dura%20den\"&form=grid&mode=query'>dura den</a>"
},
{ "label": "Department", "value": "Museums" },
{
"label": "Collection",
"value": "<a href='https://collections.st-andrews.ac.uk/collection/geology-collection/1004098'>Geology Collection</a>"
},
{ "label": "Record level", "value": "Item" },
{
"label": "Conditions of use",
"value": "CC BY-NC Creative Commons Attribution-NonCommercial 4.0 International Public License"
},
{ "label": "Credit line", "value": "Image Courtesy of the University of St Andrews Library, ID GE1049" },
{ "label": "IRN", "value": "762345" },
{
"label": "Image delivery",
"value": "Working in partnership with <a href='https://kakadusoftware.com/'>Kakadu</a> to deliver high quality image reproductions via IIIF."
}
],
"items": [
{
"id": "https://collections.st-andrews.ac.uk/762345/manifest/canvas/406403",
"type": "Canvas",
"items": [
{
"id": "https://collections.st-andrews.ac.uk/762345/manifest/canvas/406403/annotationpage/0",
"type": "AnnotationPage",
"items": [
{
"id": "https://collections.st-andrews.ac.uk/762345/manifest/canvas/406403/annotation/0",
"type": "Annotation",
"motivation": "painting",
"body": {
"id": "https://collections.st-andrews.ac.uk/media/406403/406403.glb",
"type": "Model",
"format": "model/gltf-binary"
},
"target": "https://collections.st-andrews.ac.uk/762345/manifest/canvas/406403"
}
]
}
]
}
]
}
74 changes: 74 additions & 0 deletions src/presentation-2/traverse.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,18 @@ export class Traverse<
}

traverseManifestItems(manifest: Manifest): Manifest {
// Handle malformed manifests that have v2 @context but use v3 structure (items instead of sequences).
// Convert v3 items structure to v2 sequences structure before traversal.
if (!manifest.sequences && (manifest as any).items && Array.isArray((manifest as any).items)) {
manifest.sequences = [
{
'@id': `${manifest['@id']}/sequence/0`,
Comment on lines +235 to +237
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code does not validate that manifest['@id'] is defined before using it in a template literal on line 237. If manifest['@id'] is undefined or null, this will result in a sequence ID like undefined/sequence/0. Add a check or provide a fallback value.

Suggested change
manifest.sequences = [
{
'@id': `${manifest['@id']}/sequence/0`,
const manifestId = manifest['@id'];
const sequenceId = manifestId ? `${manifestId}/sequence/0` : 'sequence/0';
manifest.sequences = [
{
'@id': sequenceId,

Copilot uses AI. Check for mistakes.
'@type': 'sc:Sequence',
canvases: (manifest as any).items.map((item: any) => this.convertV3CanvasToV2(item)),
} as any,
];
delete (manifest as any).items;
Comment on lines +234 to +242
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Directly mutating the input manifest object by setting manifest.sequences and deleting (manifest as any).items can cause side effects if the same manifest object is used elsewhere. Consider creating a shallow copy of the manifest before mutation, or document that this function mutates the input.

Copilot uses AI. Check for mistakes.
}
if (manifest.sequences) {
manifest.sequences = manifest.sequences.map((sequence) => this.traverseSequence(sequence));
}
Expand All @@ -238,6 +250,68 @@ export class Traverse<
return manifest;
}

/**
* Convert a v3-style Canvas to v2-style Canvas structure.
* Handles manifests with v2 context but v3 structure.
*/
convertV3CanvasToV2(canvas: any): any {
// If it's already v2 style, return as-is
if (canvas['@type'] || canvas['@id']) {
Comment on lines +258 to +259
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The detection logic for v2-style canvases is incorrect. A canvas with @type or @id could still have v3 structure with items arrays. The check should verify the presence of v2-specific properties like images array or absence of v3-specific properties like items array, not just the presence of @type or @id.

Suggested change
// If it's already v2 style, return as-is
if (canvas['@type'] || canvas['@id']) {
// If it's already v2 style (has images) or not clearly v3 (no items), return as-is
const hasV3Items = Array.isArray((canvas as any).items);
const hasV2Images = Array.isArray((canvas as any).images);
if (hasV2Images || !hasV3Items) {

Copilot uses AI. Check for mistakes.
return canvas;
}

const v2Canvas: any = {
'@id': canvas.id,
'@type': 'sc:Canvas',
label: canvas.label,
height: canvas.height,
width: canvas.width,
};

// Convert v3 items (AnnotationPages) to v2 images array
if (canvas.items && Array.isArray(canvas.items)) {
v2Canvas.images = [];
for (const annotationPage of canvas.items) {
if (annotationPage.items && Array.isArray(annotationPage.items)) {
for (const annotation of annotationPage.items) {
v2Canvas.images.push(this.convertV3AnnotationToV2(annotation, canvas.id));
Comment on lines +264 to +277
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code does not validate that canvas.id is defined before using it on line 264 and 277. If a v3 canvas is missing the id property, this will result in undefined values being assigned to @id properties. Add validation to handle missing IDs gracefully.

Copilot uses AI. Check for mistakes.
}
}
}
Comment on lines +273 to +280
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If all annotation pages or annotations are empty (no items), this will result in an empty images array. While this is valid, it might be more appropriate to leave images undefined if there are no actual images to preserve consistency with how v2 manifests typically handle missing images arrays.

Suggested change
v2Canvas.images = [];
for (const annotationPage of canvas.items) {
if (annotationPage.items && Array.isArray(annotationPage.items)) {
for (const annotation of annotationPage.items) {
v2Canvas.images.push(this.convertV3AnnotationToV2(annotation, canvas.id));
}
}
}
const images: any[] = [];
for (const annotationPage of canvas.items) {
if (annotationPage.items && Array.isArray(annotationPage.items)) {
for (const annotation of annotationPage.items) {
images.push(this.convertV3AnnotationToV2(annotation, canvas.id));
}
}
}
if (images.length > 0) {
v2Canvas.images = images;
}

Copilot uses AI. Check for mistakes.
}

return v2Canvas;
Comment on lines +263 to +283
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The conversion does not handle additional canvas properties like thumbnail, metadata, seeAlso, rendering, service, and other descriptive or linking properties that may be present on v3 canvases. These properties should be copied over to the v2 canvas structure to preserve all data.

Copilot uses AI. Check for mistakes.
}

/**
* Convert a v3-style Annotation to v2-style Annotation structure.
*/
convertV3AnnotationToV2(annotation: any, canvasId: string): any {
return {
'@id': annotation.id,
Comment on lines +290 to +291
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code does not validate that annotation.id is defined before using it on line 291. If a v3 annotation is missing the id property, this will result in an undefined value for the @id property. Add validation to handle missing annotation IDs.

Suggested change
return {
'@id': annotation.id,
const hasValidId =
annotation &&
typeof annotation.id === 'string' &&
annotation.id.length > 0;
// Fallback: if the v3 annotation has no id, derive a stable identifier
// from its target (if available) or from the canvasId to avoid an
// undefined @id in the v2 annotation.
const fallbackId =
(annotation && typeof annotation.target === 'string' && annotation.target) ||
canvasId;
return {
'@id': hasValidId ? annotation.id : fallbackId,

Copilot uses AI. Check for mistakes.
'@type': 'oa:Annotation',
motivation: annotation.motivation === 'painting' ? 'sc:painting' : annotation.motivation,
on: annotation.target || canvasId,
resource: this.convertV3BodyToV2Resource(annotation.body),
};
Comment on lines +289 to +296
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The conversion does not handle the case where annotation.body is an array of bodies. In IIIF Presentation API v3, the body property can be an array of resources. The code should check if annotation.body is an array and handle multiple bodies appropriately (e.g., by converting each or taking the first one).

Copilot uses AI. Check for mistakes.
Comment on lines +289 to +296
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The conversion does not handle additional annotation properties like metadata, label, or stylesheet that may be present on v3 annotations. These properties should be preserved during the conversion.

Copilot uses AI. Check for mistakes.
}

/**
* Convert a v3-style body to v2-style resource.
*/
convertV3BodyToV2Resource(body: any): any {
if (!body) return undefined;

return {
'@id': body.id,
'@type': body.type === 'Image' ? 'dctypes:Image' : `dctypes:${body.type}`,
format: body.format,
height: body.height,
width: body.width,
service: body.service,
};
Comment on lines +305 to +312
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code does not validate that body.id is defined before using it on line 306. If a v3 annotation body is missing the id property, this will result in an undefined value for the @id property. Add validation to handle missing body IDs.

Suggested change
return {
'@id': body.id,
'@type': body.type === 'Image' ? 'dctypes:Image' : `dctypes:${body.type}`,
format: body.format,
height: body.height,
width: body.width,
service: body.service,
};
const id = body.id ?? body['@id'];
const type =
body.type != null
? body.type === 'Image'
? 'dctypes:Image'
: `dctypes:${body.type}`
: undefined;
const resource: any = {
format: body.format,
height: body.height,
width: body.width,
service: body.service,
};
if (id) {
resource['@id'] = id;
}
if (type) {
resource['@type'] = type;
}
return resource;

Copilot uses AI. Check for mistakes.
Comment on lines +305 to +312
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The conversion assumes body.type is always defined, but does not handle the case where body.type is undefined or null. This could result in a malformed @type property like dctypes:undefined. Add a check to handle this case or use a default type.

Suggested change
return {
'@id': body.id,
'@type': body.type === 'Image' ? 'dctypes:Image' : `dctypes:${body.type}`,
format: body.format,
height: body.height,
width: body.width,
service: body.service,
};
const resource: any = {
'@id': body.id,
format: body.format,
height: body.height,
width: body.width,
service: body.service,
};
if (body.type) {
resource['@type'] =
body.type === 'Image' ? 'dctypes:Image' : `dctypes:${body.type}`;
}
return resource;

Copilot uses AI. Check for mistakes.
}
Comment on lines +302 to +313
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The conversion does not handle additional body properties like label, metadata, thumbnail, seeAlso, rendering, and language that may be present on v3 annotation bodies. These properties should be preserved during the conversion to ensure no data is lost.

Copilot uses AI. Check for mistakes.

traverseSequence(sequence: Sequence): T['Sequence'] {
return this.traverseType(
this.traverseDescriptive(this.traverseLinking(this.traverseSequenceItems(sequence))),
Expand Down