Merge pull request #44 from IIIF-Commons/feature/avoid-partof-recursion

Fixed bug with partOf loop

__tests__/presentation-3-parser/iiif-traverse.test.ts

@@ -1,6 +1,7 @@
 import { Traverse } from '../../src/presentation-3';
 import { Canvas, Manifest } from '@iiif/presentation-3';
 import accompanying from '../../fixtures/presentation-3/accompanying-canvas.json';
+import hotspot from '../../fixtures/cookbook/0022-linking-with-a-hotspot.json';
 
 describe('utility/iiif-traverse', () => {
   const manifest = (): Manifest => ({
@@ -207,4 +208,26 @@ describe('utility/iiif-traverse', () => {
       'https://iiif.io/api/cookbook/recipe/0014-accompanyingcanvas/canvas/accompanying/annotation/image'
     );
   });
+
+  test('it can traverse hotspot canvas', () => {
+    const canvasIds: string[] = [];
+    const traversal = new Traverse({
+      canvas: [
+        (canvas) => {
+          canvasIds.push(canvas.id);
+          return canvas;
+        },
+      ],
+    });
+
+    traversal.traverseManifest(hotspot as any);
+
+    expect(canvasIds).toMatchInlineSnapshot(`
+      [
+        "https://iiif.io/api/cookbook/recipe/0022-linking-with-a-hotspot/canvas/p2",
+        "https://iiif.io/api/cookbook/recipe/0022-linking-with-a-hotspot/canvas/p1",
+        "https://iiif.io/api/cookbook/recipe/0022-linking-with-a-hotspot/canvas/p2",
+      ]
+    `);
+  });
 });

__tests__/presentation-3-parser/serializer.test.ts

@@ -1,5 +1,6 @@
 import { normalize, serialize, serializeConfigPresentation2, serializeConfigPresentation3 } from '../../src';
-import { Collection } from '@iiif/presentation-3';
+import { Collection, Manifest } from '@iiif/presentation-3';
+import hotspot from '../../fixtures/cookbook/0022-linking-with-a-hotspot.json';
 
 describe('serializer', () => {
   test('parse, then serialize', () => {
@@ -196,7 +197,7 @@ describe('serializer', () => {
             // ],
           },
         ],
-      } as const);
+      }) as const;
     const result = normalize(input());
 
     expect(result.resource).toBeDefined();
@@ -687,4 +688,24 @@ describe('serializer', () => {
 
     expect(serialized.items).toHaveLength(3);
   });
+
+  test('it can serialize circular reference', () => {
+    const result = normalize(JSON.parse(JSON.stringify(hotspot)));
+
+    expect(result.resource).toBeDefined();
+
+    const serialized = serialize<Manifest>(
+      {
+        mapping: result.mapping,
+        entities: result.entities,
+        requests: {},
+      },
+      result.resource,
+      serializeConfigPresentation3
+    );
+
+    // expect(serialized).toEqual(hotspot);
+
+    expect(serialized.items).toHaveLength(2);
+  });
 });

__tests__/presentation-3-parser/smoke.tests.ts

@@ -21,6 +21,9 @@ const skipThese: string[] = [
   // @todo
   //  - Updates to service.
   'ghent-choices.json',
+
+  // This is because the test is designed to infer the type of the resource.
+  'specific-resource-infer.json',
 ];
 
 describe('Smoke tests', async function () {

src/presentation-3/serialize-presentation-3.ts

@@ -106,8 +106,15 @@ function* descriptiveProperties(
 }
 
 function* linkingProperties(
-  entity: Partial<LinkingNormalized>
+  entity: Partial<LinkingNormalized>,
+  parent?: any
 ): Generator<any, any, Array<[keyof LinkingNormalized, any]>> {
+  let filteredPartOf = [];
+  for (let partOf of entity.partOf || []) {
+    if (partOf.type === 'Manifest' && parent.type === 'Manifest') continue;
+    filteredPartOf.push(yield partOf);
+  }
+
   return [
     ['seeAlso', filterEmpty(yield entity.seeAlso)],
     ['service', filterEmpty(filterService2Compat(entity.service))],
@@ -118,7 +125,7 @@ function* linkingProperties(
     ['logo', filterEmpty(yield (entity as ResourceProvider).logo)],
 
     // Don't yield these, they are references.
-    ['partOf', filterEmpty(yield entity.partOf)],
+    ['partOf', filterEmpty(filteredPartOf)],
     [
       'start',
       // @todo remove once types updated.
@@ -152,12 +159,16 @@ export const serializeConfigPresentation3: SerializeConfig = {
     ];
   },
 
-  Canvas: function* (entity) {
+  Canvas: function* (entity, state, { parent }) {
+    if (parent && parent.type !== 'Manifest' && parent.type !== 'Canvas') {
+      return [['id', entity.id]];
+    }
+
     return [
       // Items.
       ...technicalProperties(entity),
       ...(yield* descriptiveProperties(entity)),
-      ...(yield* linkingProperties(entity)),
+      ...(yield* linkingProperties(entity, parent)),
       ['items', yield entity.items],
       ['annotations', filterEmpty(yield entity.annotations)],
       ['navPlace', (entity as any).navPlace], // @todo remove when types are updated
@@ -230,7 +241,12 @@ export const serializeConfigPresentation3: SerializeConfig = {
             ...(body as any),
           };
 
-          single.source = yield body.source;
+          if (body.source.type !== 'Canvas') {
+            single.source = yield body.source;
+          } else {
+            single.source = body.source;
+          }
+
           resolved.push(compressSpecificResource(single, { allowSourceString: true }));
         } else {
           resolved.push(yield body);

src/presentation-3/serialize.ts

@@ -103,7 +103,8 @@ export function serialize<Return>(state: CompatibleStore, subject: Reference, co
     if (depth > 20) {
       throw new Error('Circular reference: ' + sub.id + ' ' + sub.type);
     }
-    const [resource, fullResource] = resolveIfExists(state, sub.type ? sub : sub.id, parent) || (sub.id && sub.type ? sub : null);
+    const [resource, fullResource] =
+      resolveIfExists(state, sub.type ? sub : sub.id, parent) || (sub.id && sub.type ? sub : null);
     if (!resource) {
       return UNSET;
     }

src/shared/expand-target.ts

@@ -44,6 +44,10 @@ export function expandTargetToSpecificResource(
     return expandTargetToSpecificResource(target.items[0]!);
   }
 
+  if (!target.type && 'source' in target) {
+    (target as any).type = 'SpecificResource';
+  }
+
   if (target.type === 'SpecificResource') {
     if (target.source.type === 'Canvas' && target.source.partOf && typeof target.source.partOf === 'string') {
       target.source.partOf = [

src/shared/is-specific-resource.ts

@@ -1,10 +1,14 @@
 import { SpecificResource } from '@iiif/presentation-3';
 
 export function isSpecificResource(resource: unknown): resource is SpecificResource {
-
   if (typeof resource === 'string') {
     return false;
   }
 
+  if (resource && !(resource as any).type && 'source' in (resource as any)) {
+    (resource as any).type = 'SpecificResource';
+    return true;
+  }
+
   return !!resource && (resource as any).type === 'SpecificResource';
 }