refactor: Make representation conversion unconditional.

config/presets/representation-conversion.json

@@ -79,21 +79,24 @@
 
     {
       "@id": "urn:solid-server:default:RepresentationConverter",
-      "@type": "WaterfallHandler",
+      "@type": "IfNeededConverter",
-      "WaterfallHandler:_handlers": [
+      "IfNeededConverter:_converter": {
-        {
+        "@type": "WaterfallHandler",
-          "@id": "urn:solid-server:default:ContentTypeReplacer"
+        "WaterfallHandler:_handlers": [
-        },
+          {
-        {
+            "@id": "urn:solid-server:default:ContentTypeReplacer"
-          "@id": "urn:solid-server:default:RdfToQuadConverter"
+          },
-        },
+          {
-        {
+            "@id": "urn:solid-server:default:RdfToQuadConverter"
-          "@id": "urn:solid-server:default:QuadToRdfConverter"
+          },
-        },
+          {
-        {
+            "@id": "urn:solid-server:default:QuadToRdfConverter"
-          "@id": "urn:solid-server:default:RdfRepresentationConverter"
+          },
-        }
+          {
-      ]
+            "@id": "urn:solid-server:default:RdfRepresentationConverter"
+          }
+        ]
+      }
     }
   ]
 }

src/storage/RepresentationConvertingStore.ts

@@ -3,26 +3,13 @@ import type { RepresentationPreferences } from '../ldp/representation/Representa
 import type { ResourceIdentifier } from '../ldp/representation/ResourceIdentifier';
 import { getLoggerFor } from '../logging/LogUtil';
 import type { Conditions } from './Conditions';
-import { IfNeededConverter } from './conversion/IfNeededConverter';
 import { PassthroughConverter } from './conversion/PassthroughConverter';
 import type { RepresentationConverter } from './conversion/RepresentationConverter';
 import { PassthroughStore } from './PassthroughStore';
 import type { ResourceStore } from './ResourceStore';
 
 /**
- * Store that overrides all functions that take or output a {@link Representation},
+ * Store that provides (optional) conversion of incoming and outgoing {@link Representation}s.
- * so `getRepresentation`, `addResource`, and `setRepresentation`.
- *
- * For incoming representations, they will be converted if an incoming converter and preferences have been set.
- * The converted Representation will be passed along.
- *
- * For outgoing representations, they will be converted if there is an outgoing converter.
- *
- * Conversions will only happen if required and will not happen if the Representation is already in the correct format.
- *
- * In the future this class should take the preferences of the request into account.
- * Even if there is a match with the output from the store,
- * if there is a low weight for that type conversions might still be preferred.
  */
 export class RepresentationConvertingStore<T extends ResourceStore = ResourceStore> extends PassthroughStore<T> {
   protected readonly logger = getLoggerFor(this);
@@ -41,8 +28,8 @@ export class RepresentationConvertingStore<T extends ResourceStore = ResourceSto
   }) {
     super(source);
     const { inConverter, outConverter, inType } = options;
-    this.inConverter = inConverter ? new IfNeededConverter(inConverter) : new PassthroughConverter();
+    this.inConverter = inConverter ?? new PassthroughConverter();
-    this.outConverter = outConverter ? new IfNeededConverter(outConverter) : new PassthroughConverter();
+    this.outConverter = outConverter ?? new PassthroughConverter();
     this.inPreferences = !inType ? {} : { type: { [inType]: 1 }};
   }
 

test/unit/storage/RepresentationConvertingStore.test.ts

@@ -1,111 +1,70 @@
 import type { Representation } from '../../../src/ldp/representation/Representation';
-import { RepresentationMetadata } from '../../../src/ldp/representation/RepresentationMetadata';
 import type { RepresentationConverter } from '../../../src/storage/conversion/RepresentationConverter';
 import { RepresentationConvertingStore } from '../../../src/storage/RepresentationConvertingStore';
 import type { ResourceStore } from '../../../src/storage/ResourceStore';
-import { InternalServerError } from '../../../src/util/errors/InternalServerError';
-import { CONTENT_TYPE } from '../../../src/util/Vocabularies';
 
 describe('A RepresentationConvertingStore', (): void => {
-  let store: RepresentationConvertingStore;
+  const identifier = { path: 'identifier' };
-  let source: ResourceStore;
+  const metadata = { contentType: 'text/turtle' };
-  let inConverter: RepresentationConverter;
+  const representation: Representation = { binary: true, data: 'data', metadata } as any;
-  let outConverter: RepresentationConverter;
+  const preferences = { type: { 'text/plain': 1, 'text/turtle': 0 }};
-  const convertedIn = { metadata: {}};
+
-  const convertedOut = { metadata: {}};
+  const sourceRepresentation = { data: 'data' };
+  const source: ResourceStore = {
+    getRepresentation: jest.fn().mockResolvedValue(sourceRepresentation),
+    addResource: jest.fn(),
+    setRepresentation: jest.fn(),
+  } as any;
+
+  const convertedIn = { in: true };
+  const convertedOut = { out: true };
+  const inConverter: RepresentationConverter = { handleSafe: jest.fn().mockResolvedValue(convertedIn) } as any;
+  const outConverter: RepresentationConverter = { handleSafe: jest.fn().mockResolvedValue(convertedOut) } as any;
+
   const inType = 'text/turtle';
-  const metadata = new RepresentationMetadata('text/turtle');
+  const store = new RepresentationConvertingStore(source, { inType, inConverter, outConverter });
-  let representation: Representation;
 
   beforeEach(async(): Promise<void> => {
-    source = {
+    jest.clearAllMocks();
-      getRepresentation: jest.fn(async(): Promise<any> => ({ data: 'data', metadata })),
-      addResource: jest.fn(),
-      setRepresentation: jest.fn(),
-    } as any;
-
-    inConverter = { handleSafe: jest.fn(async(): Promise<any> => convertedIn) } as any;
-    outConverter = { handleSafe: jest.fn(async(): Promise<any> => convertedOut) } as any;
-
-    store = new RepresentationConvertingStore(source, { inType, inConverter, outConverter });
-    representation = { binary: true, data: 'data', metadata } as any;
   });
 
-  it('returns the Representation from the source if no changes are required.', async(): Promise<void> => {
+  it('calls the outgoing converter when retrieving a representation.', async(): Promise<void> => {
-    const result = await store.getRepresentation({ path: 'path' },
+    await expect(store.getRepresentation(identifier, preferences)).resolves.toEqual(convertedOut);
-      { type: { 'application/*': 0, 'text/turtle': 1 }});
-    expect(result).toEqual({
-      data: 'data',
-      metadata: expect.any(RepresentationMetadata),
-    });
-    expect(result.metadata.contentType).toEqual('text/turtle');
-    expect(source.getRepresentation).toHaveBeenCalledTimes(1);
-    expect(source.getRepresentation).toHaveBeenLastCalledWith(
-      { path: 'path' },
-      { type: { 'application/*': 0, 'text/turtle': 1 }},
-      undefined,
-    );
-    expect(outConverter.handleSafe).toHaveBeenCalledTimes(0);
-  });
-
-  it('returns the Representation from the source if there are no preferences.', async(): Promise<void> => {
-    const result = await store.getRepresentation({ path: 'path' }, {});
-    expect(result).toEqual({
-      data: 'data',
-      metadata: expect.any(RepresentationMetadata),
-    });
-    expect(result.metadata.contentType).toEqual('text/turtle');
-    expect(source.getRepresentation).toHaveBeenCalledTimes(1);
-    expect(source.getRepresentation).toHaveBeenLastCalledWith(
-      { path: 'path' }, {}, undefined,
-    );
-    expect(outConverter.handleSafe).toHaveBeenCalledTimes(0);
-  });
-
-  it('calls the converter if another output is preferred.', async(): Promise<void> => {
-    await expect(store.getRepresentation({ path: 'path' },
-      { type: { 'text/plain': 1, 'text/turtle': 0 }})).resolves.toEqual(convertedOut);
     expect(source.getRepresentation).toHaveBeenCalledTimes(1);
     expect(outConverter.handleSafe).toHaveBeenCalledTimes(1);
-    expect(outConverter.handleSafe).toHaveBeenLastCalledWith({
+    expect(outConverter.handleSafe).toHaveBeenNthCalledWith(1, {
-      identifier: { path: 'path' },
+      identifier,
-      representation: { data: 'data', metadata },
+      representation: sourceRepresentation,
-      preferences: { type: { 'text/plain': 1, 'text/turtle': 0 }},
+      preferences,
     });
   });
 
-  it('keeps the representation if the conversion is not required.', async(): Promise<void> => {
+  it('calls the incoming converter when adding resources.', async(): Promise<void> => {
-    const id = { path: 'identifier' };
+    await expect(store.addResource(identifier, representation, 'conditions' as any)).resolves.toBeUndefined();
-
-    await expect(store.addResource(id, representation, 'conditions' as any)).resolves.toBeUndefined();
-    expect(source.addResource).toHaveBeenLastCalledWith(id, representation, 'conditions');
-
-    await expect(store.setRepresentation(id, representation, 'conditions' as any)).resolves.toBeUndefined();
-    expect(inConverter.handleSafe).toHaveBeenCalledTimes(0);
-    expect(source.setRepresentation).toHaveBeenLastCalledWith(id, representation, 'conditions');
-
-    store = new RepresentationConvertingStore(source, {});
-    await expect(store.addResource(id, representation, 'conditions' as any)).resolves.toBeUndefined();
-    expect(source.addResource).toHaveBeenLastCalledWith(id, representation, 'conditions');
-  });
-
-  it('converts the data if it is required.', async(): Promise<void> => {
-    metadata.contentType = 'text/plain';
-    const id = { path: 'identifier' };
-
-    await expect(store.addResource(id, representation, 'conditions' as any)).resolves.toBeUndefined();
     expect(inConverter.handleSafe).toHaveBeenCalledTimes(1);
-    expect(source.addResource).toHaveBeenLastCalledWith(id, convertedIn, 'conditions');
+    expect(inConverter.handleSafe).toHaveBeenNthCalledWith(1, {
-
+      identifier,
-    await expect(store.setRepresentation(id, representation, 'conditions' as any)).resolves.toBeUndefined();
+      representation,
-    expect(inConverter.handleSafe).toHaveBeenCalledTimes(2);
+      preferences: { type: { 'text/turtle': 1 }},
-    expect(source.setRepresentation).toHaveBeenLastCalledWith(id, convertedIn, 'conditions');
+    });
+    expect(source.addResource).toHaveBeenLastCalledWith(identifier, convertedIn, 'conditions');
   });
 
-  it('throws an error if no content-type is provided.', async(): Promise<void> => {
+  it('calls the incoming converter when setting representations.', async(): Promise<void> => {
-    metadata.removeAll(CONTENT_TYPE);
+    await expect(store.setRepresentation(identifier, representation, 'conditions' as any)).resolves.toBeUndefined();
-    const id = { path: 'identifier' };
+    expect(inConverter.handleSafe).toHaveBeenCalledTimes(1);
+    expect(inConverter.handleSafe).toHaveBeenNthCalledWith(1, {
+      identifier,
+      representation,
+      preferences: { type: { 'text/turtle': 1 }},
+    });
+    expect(source.setRepresentation).toHaveBeenLastCalledWith(identifier, convertedIn, 'conditions');
+  });
 
-    await expect(store.addResource(id, representation, 'conditions' as any)).rejects.toThrow(InternalServerError);
+  it('does not perform any conversions when constructed with empty arguments.', async(): Promise<void> => {
+    const noArgStore = new RepresentationConvertingStore(source, {});
+    await expect(noArgStore.getRepresentation(identifier, preferences)).resolves.toEqual(sourceRepresentation);
+    await expect(noArgStore.addResource(identifier, representation)).resolves.toBeUndefined();
+    await expect(noArgStore.setRepresentation(identifier, representation)).resolves.toBeUndefined();
   });
 });