Repository: solid/community-server
Pull request #101 (2020-09-01 09:08:05)

feat: Create RepresentationConverter that chains other converters
So I changed my mind and did make some sort of meta converter. This one is still sort of simple since you have to define what the intermadiate types are going to be (otherwise I would have to think about the best interface changes to request this information from the converters), but in practice this is going to cover most cases I think.

Haven't actually tested it with real converters yet, wanted to first check out the other PR with the new converter for that.

rubensworks: > This one is still sort of simple since you have to define what the intermadiate types are going to be (otherwise I would have to think about the best interface changes to request this information from the converters

It's a bit unfortunate that we have to define types manually in the config with this.

How much effort do you think it would be change these interfaces? Maybe it's not that much work?
joachimvh: > How much effort do you think it would be change these interfaces? Maybe it's not that much work?

Well depends on how far you want to go, I was thinking of just having a getter for the supported input/output types, but then perhaps you could have some converters that can't go from all their supported inputs to all their supported output.

Perhaps also some of their output types are more expensive to convert to if they have multiple, which you can avoid with the manual case (or extending the interface mentioned above with some weights, but you can see how you can keep thinking of more cases).

But these are potentially all edge cases and I could ignore them and add a simple getter interface.
rubensworks: > I was thinking of just having a getter for the supported input/output types, but then perhaps you could have some converters that can't go from all their supported inputs to all their supported output.

In Comunica, parsers and serializers must expose a hash that maps content types to priorities. AFAICS, something like this should cover most cases.

The recently added generic parser and serializer expose this information as well, so you'd get that for free with them.
joachimvh: The `ChainedConverter` now dynamically determines the "path" that needs to be taken. Slight performance hit (although this is probably quite small compared to the actual conversion), but more dynamic.

If you're wondering why I created `TypedRepresentationConverter` instead of just adding the functions to `RepresentationConverter`, the reason is that otherwise we wouldn't be able to use a `CompositeAsyncHandler` anymore for that type.
joachimvh: > Is there anything that blocks us from wiring this up in the config already?

Nothing, except that I didn't really test this yet on an actual case because I first wanted to get the interface right.
joachimvh: I'll look into writing a small integration test for this and then adding it to the config.
joachimvh: Can confirm that with the default config you can now PUT a turtle file to the store and then GET JSON-LD.

Patch:
diff --git a/config/presets/storage_wrapper.json b/config/presets/storage_wrapper.json
index dce8d1f24..d545df89a 100644
--- a/config/presets/storage_wrapper.json
+++ b/config/presets/storage_wrapper.json
@@ -24,6 +24,29 @@
       }
     },
 
+    {
+      "@id": "urn:solid-server:default:ResourceStore_Converting_RdfToQuad",
+      "@type": "RdfToQuadConverter"
+    },
+
+    {
+      "@id": "urn:solid-server:default:ResourceStore_Converting_QuadToRdf",
+      "@type": "QuadToRdfConverter"
+    },
+
+    {
+      "@id": "urn:solid-server:default:ResourceStore_Converting_Chained",
+      "@type": "ChainedConverter",
+      "ChainedConverter:_converters": [
+        {
+          "@id": "urn:solid-server:default:ResourceStore_Converting_RdfToQuad"
+        },
+        {
+          "@id": "urn:solid-server:default:ResourceStore_Converting_QuadToRdf"
+        }
+      ]
+    },
+
     {
       "@id": "urn:solid-server:default:ResourceStore_Converting",
       "@type": "RepresentationConvertingStore",
@@ -34,10 +57,13 @@
         "@type": "CompositeAsyncHandler",
         "CompositeAsyncHandler:_handlers": [
           {
-            "@type": "RdfToQuadConverter"
+            "@id": "urn:solid-server:default:ResourceStore_Converting_RdfToQuad"
+          },
+          {
+            "@id": "urn:solid-server:default:ResourceStore_Converting_QuadToRdf"
           },
           {
-            "@type": "QuadToRdfConverter"
+            "@id": "urn:solid-server:default:ResourceStore_Converting_Chained"
           }
         ]
       }
diff --git a/index.ts b/index.ts
index 64ef4c048..5a2972888 100644
--- a/index.ts
+++ b/index.ts
@@ -65,11 +65,13 @@ export * from './src/server/HttpRequest';
 export * from './src/server/HttpResponse';
 
 // Storage/Conversion
+export * from './src/storage/conversion/ChainedConverter';
 export * from './src/storage/conversion/QuadToRdfConverter';
-export * from './src/storage/conversion/RdfToQuadConverter';
 export * from './src/storage/conversion/QuadToTurtleConverter';
+export * from './src/storage/conversion/RdfToQuadConverter';
 export * from './src/storage/conversion/RepresentationConverter';
 export * from './src/storage/conversion/TurtleToQuadConverter';
+export * from './src/storage/conversion/TypedRepresentationConverter';
 
 // Storage/Patch
 export * from './src/storage/patch/PatchHandler';
diff --git a/package.json b/package.json
index e92e66f52..4f209030a 100644
--- a/package.json
+++ b/package.json
@@ -29,7 +29,7 @@
   },
   "husky": {
     "hooks": {
-      "pre-commit": "npm run build && npm run lint && npm run validate && npm run test"
+      "pre-commit": "npm run build && npm run lint && npm run test"
     }
   },
   "files": [
diff --git a/src/storage/conversion/ChainedConverter.ts b/src/storage/conversion/ChainedConverter.ts
new file mode 100644
index 000000000..f6683fd85
--- /dev/null
+++ b/src/storage/conversion/ChainedConverter.ts
@@ -0,0 +1,104 @@
+import { Representation } from '../../ldp/representation/Representation';
+import { RepresentationPreferences } from '../../ldp/representation/RepresentationPreferences';
+import { matchingMediaType } from '../../util/Util';
+import { RepresentationConverterArgs } from './RepresentationConverter';
+import { TypedRepresentationConverter } from './TypedRepresentationConverter';
+
+/**
+ * A meta converter that takes an array of other converters as input.
+ * It chains these converters by finding intermediate types that are supported by converters on either side.
+ */
+export class ChainedConverter extends TypedRepresentationConverter {
+  private readonly converters: TypedRepresentationConverter[];
+
+  /**
+   * Creates the chain of converters based on the input.
+   * The list of `converters` needs to be at least 2 long.
+   * @param converters - The chain of converters.
+   */
+  public constructor(converters: TypedRepresentationConverter[]) {
+    super();
+    if (converters.length < 2) {
+      throw new Error('At least 2 converters are required.');
+    }
+    this.converters = [ ...converters ];
+  }
+
+  protected get first(): TypedRepresentationConverter {
+    return this.converters[0];
+  }
+
+  protected get last(): TypedRepresentationConverter {
+    return this.converters[this.converters.length - 1];
+  }
+
+  public async getInputTypes(): Promise<{ [contentType: string]: number }> {
+    return this.first.getInputTypes();
+  }
+
+  public async getOutputTypes(): Promise<{ [contentType: string]: number }> {
+    return this.last.getOutputTypes();
+  }
+
+  public async canHandle(input: RepresentationConverterArgs): Promise<void> {
+    // We assume a chain can be constructed, otherwise there would be a configuration issue
+    // Check if the first converter can handle the input
+    const firstChain = await this.getMatchingType(this.converters[0], this.converters[1]);
+    const preferences: RepresentationPreferences = { type: [{ value: firstChain, weight: 1 }]};
+    await this.first.canHandle({ ...input, preferences });
+
+    // Check if the last converter can produce the output
+    const idx = this.converters.length - 1;
+    const lastChain = await this.getMatchingType(this.converters[idx - 1], this.converters[idx]);
+    const representation: Representation = { ...input.representation };
+    representation.metadata = { ...input.representation.metadata, contentType: lastChain };
+    await this.last.canHandle({ ...input, representation });
+  }
+
+  public async handle(input: RepresentationConverterArgs): Promise<Representation> {
+    const args = { ...input };
+    for (let i = 0; i < this.converters.length - 1; ++i) {
+      const value = await this.getMatchingType(this.converters[i], this.converters[i + 1]);
+      args.preferences = { type: [{ value, weight: 1 }]};
+      args.representation = await this.converters[i].handle(args);
+    }
+    args.preferences = input.preferences;
+    return this.last.handle(args);
+  }
+
+  /**
+   * Finds the best media type that can be used to chain 2 converters.
+   */
+  protected async getMatchingType(left: TypedRepresentationConverter, right: TypedRepresentationConverter):
+  Promise<string> {
+    const leftTypes = await left.getOutputTypes();
+    const rightTypes = await right.getInputTypes();
+    let bestMatch: { type: string; weight: number } = { type: 'invalid', weight: 0 };
+
+    // Try to find the matching type with the best weight
+    const leftKeys = Object.keys(leftTypes);
+    const rightKeys = Object.keys(rightTypes);
+    for (const leftType of leftKeys) {
+      const leftWeight = leftTypes[leftType];
+      if (leftWeight <= bestMatch.weight) {
+        continue;
+      }
+      for (const rightType of rightKeys) {
+        const rightWeight = rightTypes[rightType];
+        const weight = leftWeight * rightWeight;
+        if (weight > bestMatch.weight && matchingMediaType(leftType, rightType)) {
+          bestMatch = { type: leftType, weight };
+          if (weight === 1) {
+            return bestMatch.type;
+          }
+        }
+      }
+    }
+
+    if (bestMatch.weight === 0) {
+      throw new Error(`No match found between ${leftKeys} and ${rightKeys}`);
+    }
+
+    return bestMatch.type;
+  }
+}
diff --git a/src/storage/conversion/QuadToRdfConverter.ts b/src/storage/conversion/QuadToRdfConverter.ts
index 7ac5eb238..bf53442d3 100644
--- a/src/storage/conversion/QuadToRdfConverter.ts
+++ b/src/storage/conversion/QuadToRdfConverter.ts
@@ -5,12 +5,21 @@ import { RepresentationMetadata } from '../../ldp/representation/RepresentationM
 import { RepresentationPreferences } from '../../ldp/representation/RepresentationPreferences';
 import { CONTENT_TYPE_QUADS, DATA_TYPE_BINARY } from '../../util/ContentTypes';
 import { checkRequest, matchingTypes } from './ConversionUtil';
-import { RepresentationConverter, RepresentationConverterArgs } from './RepresentationConverter';
+import { RepresentationConverterArgs } from './RepresentationConverter';
+import { TypedRepresentationConverter } from './TypedRepresentationConverter';
 
 /**
  * Converts `internal/quads` to most major RDF serializations.
  */
-export class QuadToRdfConverter extends RepresentationConverter {
+export class QuadToRdfConverter extends TypedRepresentationConverter {
+  public async getInputTypes(): Promise<{ [contentType: string]: number }> {
+    return { [CONTENT_TYPE_QUADS]: 1 };
+  }
+
+  public async getOutputTypes(): Promise<{ [contentType: string]: number }> {
+    return rdfSerializer.getContentTypesPrioritized();
+  }
+
   public async canHandle(input: RepresentationConverterArgs): Promise<void> {
     checkRequest(input, [ CONTENT_TYPE_QUADS ], await rdfSerializer.getContentTypes());
   }
diff --git a/src/storage/conversion/RdfToQuadConverter.ts b/src/storage/conversion/RdfToQuadConverter.ts
index 73484691b..2c8542efa 100644
--- a/src/storage/conversion/RdfToQuadConverter.ts
+++ b/src/storage/conversion/RdfToQuadConverter.ts
@@ -5,12 +5,21 @@ import { RepresentationMetadata } from '../../ldp/representation/RepresentationM
 import { CONTENT_TYPE_QUADS, DATA_TYPE_QUAD } from '../../util/ContentTypes';
 import { UnsupportedHttpError } from '../../util/errors/UnsupportedHttpError';
 import { checkRequest } from './ConversionUtil';
-import { RepresentationConverter, RepresentationConverterArgs } from './RepresentationConverter';
+import { RepresentationConverterArgs } from './RepresentationConverter';
+import { TypedRepresentationConverter } from './TypedRepresentationConverter';
 
 /**
  * Converts most major RDF serializations to `internal/quads`.
  */
-export class RdfToQuadConverter extends RepresentationConverter {
+export class RdfToQuadConverter extends TypedRepresentationConverter {
+  public async getInputTypes(): Promise<{ [contentType: string]: number }> {
+    return rdfParser.getContentTypesPrioritized();
+  }
+
+  public async getOutputTypes(): Promise<{ [contentType: string]: number }> {
+    return { [CONTENT_TYPE_QUADS]: 1 };
+  }
+
   public async canHandle(input: RepresentationConverterArgs): Promise<void> {
     checkRequest(input, await rdfParser.getContentTypes(), [ CONTENT_TYPE_QUADS ]);
   }
diff --git a/src/storage/conversion/TypedRepresentationConverter.ts b/src/storage/conversion/TypedRepresentationConverter.ts
new file mode 100644
index 000000000..6a4bf8c56
--- /dev/null
+++ b/src/storage/conversion/TypedRepresentationConverter.ts
@@ -0,0 +1,20 @@
+import { RepresentationConverter } from './RepresentationConverter';
+
+/**
+ * A {@link RepresentationConverter} that allows requesting the supported types.
+ */
+export abstract class TypedRepresentationConverter extends RepresentationConverter {
+  /**
+   * Get a hash of all supported input content types for this converter, mapped to a numerical priority.
+   * The priority weight goes from 0 up to 1.
+   * @returns A promise resolving to a hash mapping content type to a priority number.
+   */
+  public abstract getInputTypes(): Promise<{ [contentType: string]: number }>;
+
+  /**
+   * Get a hash of all supported output content types for this converter, mapped to a numerical priority.
+   * The priority weight goes from 0 up to 1.
+   * @returns A promise resolving to a hash mapping content type to a priority number.
+   */
+  public abstract getOutputTypes(): Promise<{ [contentType: string]: number }>;
+}


Test patch:
diff --git a/test/integration/RepresentationConverter.test.ts b/test/integration/RepresentationConverter.test.ts
new file mode 100644
index 000000000..db0a5102b
--- /dev/null
+++ b/test/integration/RepresentationConverter.test.ts
@@ -0,0 +1,51 @@
+import streamifyArray from 'streamify-array';
+import { Representation } from '../../src/ldp/representation/Representation';
+import { ChainedConverter } from '../../src/storage/conversion/ChainedConverter';
+import { QuadToRdfConverter } from '../../src/storage/conversion/QuadToRdfConverter';
+import { RdfToQuadConverter } from '../../src/storage/conversion/RdfToQuadConverter';
+import { DATA_TYPE_BINARY } from '../../src/util/ContentTypes';
+import { readableToString } from '../../src/util/Util';
+
+describe('A ChainedConverter', (): void => {
+  const converters = [
+    new RdfToQuadConverter(),
+    new QuadToRdfConverter(),
+  ];
+  const converter = new ChainedConverter(converters);
+
+  it('can convert from JSON-LD to turtle.', async(): Promise<void> => {
+    const representation: Representation = {
+      dataType: DATA_TYPE_BINARY,
+      data: streamifyArray([ '{"@id": "http://test.com/s", "http://test.com/p": { "@id": "http://test.com/o" }}' ]),
+      metadata: { raw: [], contentType: 'application/ld+json' },
+    };
+
+    const result = await converter.handleSafe({
+      representation,
+      preferences: { type: [{ value: 'text/turtle', weight: 1 }]},
+      identifier: { path: 'path' },
+    });
+
+    await expect(readableToString(result.data)).resolves.toEqual('<http://test.com/s> <http://test.com/p> <http://test.com/o>.\n');
+    expect(result.metadata.contentType).toEqual('text/turtle');
+  });
+
+  it('can convert from turtle to JSON-LD.', async(): Promise<void> => {
+    const representation: Representation = {
+      dataType: DATA_TYPE_BINARY,
+      data: streamifyArray([ '<http://test.com/s> <http://test.com/p> <http://test.com/o>.' ]),
+      metadata: { raw: [], contentType: 'text/turtle' },
+    };
+
+    const result = await converter.handleSafe({
+      representation,
+      preferences: { type: [{ value: 'application/ld+json', weight: 1 }]},
+      identifier: { path: 'path' },
+    });
+
+    expect(JSON.parse(await readableToString(result.data))).toEqual(
+      [{ '@id': 'http://test.com/s', 'http://test.com/p': [{ '@id': 'http://test.com/o' }]}],
+    );
+    expect(result.metadata.contentType).toEqual('application/ld+json');
+  });
+});
diff --git a/test/unit/storage/conversion/ChainedConverter.test.ts b/test/unit/storage/conversion/ChainedConverter.test.ts
new file mode 100644
index 000000000..d010a0c6b
--- /dev/null
+++ b/test/unit/storage/conversion/ChainedConverter.test.ts
@@ -0,0 +1,106 @@
+import { Representation } from '../../../../src/ldp/representation/Representation';
+import { RepresentationPreferences } from '../../../../src/ldp/representation/RepresentationPreferences';
+import { ChainedConverter } from '../../../../src/storage/conversion/ChainedConverter';
+import { checkRequest } from '../../../../src/storage/conversion/ConversionUtil';
+import { RepresentationConverterArgs } from '../../../../src/storage/conversion/RepresentationConverter';
+import { TypedRepresentationConverter } from '../../../../src/storage/conversion/TypedRepresentationConverter';
+
+class DummyConverter extends TypedRepresentationConverter {
+  private readonly inTypes: { [contentType: string]: number };
+  private readonly outTypes: { [contentType: string]: number };
+
+  public constructor(inTypes: { [contentType: string]: number }, outTypes: { [contentType: string]: number }) {
+    super();
+    this.inTypes = inTypes;
+    this.outTypes = outTypes;
+  }
+
+  public async getInputTypes(): Promise<{ [contentType: string]: number }> {
+    return this.inTypes;
+  }
+
+  public async getOutputTypes(): Promise<{ [contentType: string]: number }> {
+    return this.outTypes;
+  }
+
+  public async canHandle(input: RepresentationConverterArgs): Promise<void> {
+    checkRequest(input, Object.keys(this.inTypes), Object.keys(this.outTypes));
+  }
+
+  public async handle(input: RepresentationConverterArgs): Promise<Representation> {
+    const representation: Representation = { ...input.representation };
+    representation.metadata = { ...input.representation.metadata, contentType: input.preferences.type![0].value };
+    return representation;
+  }
+}
+
+describe('A ChainedConverter', (): void => {
+  let converters: TypedRepresentationConverter[];
+  let converter: ChainedConverter;
+  let representation: Representation;
+  let preferences: RepresentationPreferences;
+  let args: RepresentationConverterArgs;
+
+  beforeEach(async(): Promise<void> => {
+    converters = [
+      new DummyConverter({ 'text/turtle': 1 }, { 'chain/1': 0.9, 'chain/x': 0.5 }),
+      new DummyConverter({ 'chain/*': 1, 'chain/x': 0.5 }, { 'chain/2': 1 }),
+      new DummyConverter({ 'chain/2': 1 }, { 'internal/quads': 1 }),
+    ];
+    converter = new ChainedConverter(converters);
+
+    representation = { metadata: { contentType: 'text/turtle' } as any } as Representation;
+    preferences = { type: [{ value: 'internal/quads', weight: 1 }]};
+    args = { representation, preferences, identifier: { path: 'path' }};
+  });
+
+  it('needs at least 2 converters.', async(): Promise<void> => {
+    expect((): any => new ChainedConverter([])).toThrow('At least 2 converters are required.');
+    expect((): any => new ChainedConverter([ converters[0] ])).toThrow('At least 2 converters are required.');
+    expect(new ChainedConverter([ converters[0], converters[1] ]))
+      .toBeInstanceOf(ChainedConverter);
+  });
+
+  it('supports the same inputs as the first converter of the chain.', async(): Promise<void> => {
+    await expect(converter.getInputTypes()).resolves.toEqual(await converters[0].getInputTypes());
+  });
+
+  it('supports the same outputs as the last converter of the chain.', async(): Promise<void> => {
+    await expect(converter.getOutputTypes()).resolves.toEqual(await converters[2].getOutputTypes());
+  });
+
+  it('can handle requests with the correct in- and output.', async(): Promise<void> => {
+    await expect(converter.canHandle(args)).resolves.toBeUndefined();
+  });
+
+  it('errors if the start of the chain does not support the representation type.', async(): Promise<void> => {
+    representation.metadata.contentType = 'bad/type';
+    await expect(converter.canHandle(args)).rejects.toThrow();
+  });
+
+  it('errors if the end of the chain does not support the preferences.', async(): Promise<void> => {
+    delete preferences.type;
+    await expect(converter.canHandle(args)).rejects.toThrow();
+  });
+
+  it('runs the data through the chain.', async(): Promise<void> => {
+    jest.spyOn(converters[0], 'handle');
+    jest.spyOn(converters[1], 'handle');
+    jest.spyOn(converters[2], 'handle');
+
+    const result = await converter.handle(args);
+    expect(result.metadata.contentType).toEqual('internal/quads');
+    expect((converters[0] as any).handle).toHaveBeenCalledTimes(1);
+    expect((converters[1] as any).handle).toHaveBeenCalledTimes(1);
+    expect((converters[2] as any).handle).toHaveBeenCalledTimes(1);
+  });
+
+  it('errors if there is no valid chain at runtime.', async(): Promise<void> => {
+    converters = [
+      new DummyConverter({ 'text/turtle': 1 }, { 'chain/1': 0.9, 'chain/x': 0.5 }),
+      new DummyConverter({ 'chain/2': 1 }, { 'internal/quads': 1 }),
+    ];
+    converter = new ChainedConverter(converters);
+    await expect(converter.handle(args)).rejects.toThrow();
+  });
+});
diff --git a/test/unit/storage/conversion/QuadToRdfConverter.test.ts b/test/unit/storage/conversion/QuadToRdfConverter.test.ts
index 324d2d0f2..2d44f9bc6 100644
--- a/test/unit/storage/conversion/QuadToRdfConverter.test.ts
+++ b/test/unit/storage/conversion/QuadToRdfConverter.test.ts
@@ -1,4 +1,5 @@
 import { namedNode, triple } from '@rdfjs/data-model';
+import rdfSerializer from 'rdf-serialize';
 import stringifyStream from 'stream-to-string';
 import streamifyArray from 'streamify-array';
 import { Representation } from '../../../../src/ldp/representation/Representation';
@@ -11,6 +12,14 @@ describe('A QuadToRdfConverter', (): void => {
   const converter = new QuadToRdfConverter();
   const identifier: ResourceIdentifier = { path: 'path' };
 
+  it('supports parsing quads.', async(): Promise<void> => {
+    await expect(converter.getInputTypes()).resolves.toEqual({ [CONTENT_TYPE_QUADS]: 1 });
+  });
+
+  it('supports serializing as the same types as rdfSerializer.', async(): Promise<void> => {
+    await expect(converter.getOutputTypes()).resolves.toEqual(await rdfSerializer.getContentTypesPrioritized());
+  });
+
   it('can handle quad to turtle conversions.', async(): Promise<void> => {
     const representation = { metadata: { contentType: CONTENT_TYPE_QUADS }} as Representation;
     const preferences: RepresentationPreferences = { type: [{ value: 'text/turtle', weight: 1 }]};
diff --git a/test/unit/storage/conversion/RdfToQuadConverter.test.ts b/test/unit/storage/conversion/RdfToQuadConverter.test.ts
index 10a0f8420..18ae8ed39 100644
--- a/test/unit/storage/conversion/RdfToQuadConverter.test.ts
+++ b/test/unit/storage/conversion/RdfToQuadConverter.test.ts
@@ -1,6 +1,7 @@
 import { Readable } from 'stream';
 import { namedNode, triple } from '@rdfjs/data-model';
 import arrayifyStream from 'arrayify-stream';
+import rdfParser from 'rdf-parse';
 import streamifyArray from 'streamify-array';
 import { Representation } from '../../../../src/ldp/representation/Representation';
 import { RepresentationPreferences } from '../../../../src/ldp/representation/RepresentationPreferences';
@@ -13,6 +14,14 @@ describe('A RdfToQuadConverter.test.ts', (): void => {
   const converter = new RdfToQuadConverter();
   const identifier: ResourceIdentifier = { path: 'path' };
 
+  it('supports parsing the same types as rdfParser.', async(): Promise<void> => {
+    await expect(converter.getInputTypes()).resolves.toEqual(await rdfParser.getContentTypesPrioritized());
+  });
+
+  it('supports serializing as quads.', async(): Promise<void> => {
+    await expect(converter.getOutputTypes()).resolves.toEqual({ [CONTENT_TYPE_QUADS]: 1 });
+  });
+
   it('can handle turtle to quad conversions.', async(): Promise<void> => {
     const representation = { metadata: { contentType: 'text/turtle' }} as Representation;
     const preferences: RepresentationPreferences = { type: [{ value: CONTENT_TYPE_QUADS, weight: 1 }]};