From 676350046631b75b136ad01ccca0ce6a64104526 Mon Sep 17 00:00:00 2001 From: Ruben Verborgh Date: Sat, 16 Jan 2021 22:44:37 +0100 Subject: [PATCH] feat: Add IfNeededConverter and PassthroughConverter. --- src/index.ts | 2 + src/storage/RepresentationConvertingStore.ts | 76 +++---------- src/storage/conversion/IfNeededConverter.ts | 61 +++++++++++ .../conversion/PassthroughConverter.ts | 12 ++ .../conversion/IfNeededConverter.test.ts | 103 ++++++++++++++++++ .../conversion/PassthroughConverter.test.ts | 20 ++++ 6 files changed, 213 insertions(+), 61 deletions(-) create mode 100644 src/storage/conversion/IfNeededConverter.ts create mode 100644 src/storage/conversion/PassthroughConverter.ts create mode 100644 test/unit/storage/conversion/IfNeededConverter.test.ts create mode 100644 test/unit/storage/conversion/PassthroughConverter.test.ts diff --git a/src/index.ts b/src/index.ts index 92b17fdde..185f02166 100644 --- a/src/index.ts +++ b/src/index.ts @@ -132,6 +132,8 @@ export * from './storage/accessors/SparqlDataAccessor'; // Storage/Conversion export * from './storage/conversion/ChainedConverter'; export * from './storage/conversion/ContentTypeReplacer'; +export * from './storage/conversion/IfNeededConverter'; +export * from './storage/conversion/PassthroughConverter'; export * from './storage/conversion/ConversionUtil'; export * from './storage/conversion/QuadToRdfConverter'; export * from './storage/conversion/RdfToQuadConverter'; diff --git a/src/storage/RepresentationConvertingStore.ts b/src/storage/RepresentationConvertingStore.ts index b912ae38e..548219e7d 100644 --- a/src/storage/RepresentationConvertingStore.ts +++ b/src/storage/RepresentationConvertingStore.ts @@ -2,10 +2,10 @@ import type { Representation } from '../ldp/representation/Representation'; import type { RepresentationPreferences } from '../ldp/representation/RepresentationPreferences'; import type { ResourceIdentifier } from '../ldp/representation/ResourceIdentifier'; import { getLoggerFor } from '../logging/LogUtil'; -import { InternalServerError } from '../util/errors/InternalServerError'; import type { Conditions } from './Conditions'; -import { matchingMediaTypes } from './conversion/ConversionUtil'; -import type { RepresentationConverter, RepresentationConverterArgs } from './conversion/RepresentationConverter'; +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'; @@ -27,10 +27,9 @@ import type { ResourceStore } from './ResourceStore'; export class RepresentationConvertingStore extends PassthroughStore { protected readonly logger = getLoggerFor(this); - private readonly inConverter?: RepresentationConverter; - private readonly outConverter?: RepresentationConverter; - - private readonly inType?: string; + private readonly inConverter: RepresentationConverter; + private readonly outConverter: RepresentationConverter; + private readonly inPreferences: RepresentationPreferences; /** * TODO: This should take RepresentationPreferences instead of a type string when supported by Components.js. @@ -41,74 +40,29 @@ export class RepresentationConvertingStore { const representation = await super.getRepresentation(identifier, preferences, conditions); - return this.convertRepresentation({ identifier, representation, preferences }, this.outConverter); + return this.outConverter.handleSafe({ identifier, representation, preferences }); } - public async addResource(container: ResourceIdentifier, representation: Representation, + public async addResource(identifier: ResourceIdentifier, representation: Representation, conditions?: Conditions): Promise { // We can potentially run into problems here if we convert a turtle document where the base IRI is required, // since we don't know the resource IRI yet at this point. - representation = await this.convertInRepresentation(container, representation); - return this.source.addResource(container, representation, conditions); + representation = await this.inConverter.handleSafe({ identifier, representation, preferences: this.inPreferences }); + return this.source.addResource(identifier, representation, conditions); } public async setRepresentation(identifier: ResourceIdentifier, representation: Representation, conditions?: Conditions): Promise { - representation = await this.convertInRepresentation(identifier, representation); + representation = await this.inConverter.handleSafe({ identifier, representation, preferences: this.inPreferences }); return this.source.setRepresentation(identifier, representation, conditions); } - - /** - * Helper function that checks if the given representation matches the given preferences. - */ - private matchesPreferences(representation: Representation, preferences: RepresentationPreferences): boolean { - const { contentType } = representation.metadata; - - if (!contentType) { - throw new InternalServerError('Content-Type is required for data conversion.'); - } - - // Check if there is a result if we try to map the preferences to the content-type - return matchingMediaTypes(preferences.type, { [contentType]: 1 }).length > 0; - } - - /** - * Helper function that converts a Representation using the given args and converter, - * if the conversion is necessary and there is a converter. - */ - private async convertRepresentation(input: RepresentationConverterArgs, converter?: RepresentationConverter): - Promise { - if (!converter || !input.preferences.type || this.matchesPreferences(input.representation, input.preferences)) { - return input.representation; - } - this.logger.debug(`Conversion needed for ${input.identifier - .path} from ${input.representation.metadata.contentType} to satisfy ${Object.entries(input.preferences.type) - .map(([ value, weight ]): string => `${value};q=${weight}`).join(', ')}`); - - const converted = await converter.handleSafe(input); - this.logger.info(`Converted representation for ${input.identifier - .path} from ${input.representation.metadata.contentType} to ${converted.metadata.contentType}`); - return converted; - } - - /** - * Helper function that converts an incoming representation if necessary. - */ - private async convertInRepresentation(identifier: ResourceIdentifier, representation: Representation): - Promise { - if (!this.inType) { - return representation; - } - const preferences: RepresentationPreferences = { type: { [this.inType]: 1 }}; - - return this.convertRepresentation({ identifier, representation, preferences }, this.inConverter); - } } diff --git a/src/storage/conversion/IfNeededConverter.ts b/src/storage/conversion/IfNeededConverter.ts new file mode 100644 index 000000000..535f447d9 --- /dev/null +++ b/src/storage/conversion/IfNeededConverter.ts @@ -0,0 +1,61 @@ +import type { Representation } from '../../ldp/representation/Representation'; +import { getLoggerFor } from '../../logging/LogUtil'; +import { InternalServerError } from '../../util/errors/InternalServerError'; +import { matchingMediaTypes } from './ConversionUtil'; +import { RepresentationConverter } from './RepresentationConverter'; +import type { RepresentationConverterArgs } from './RepresentationConverter'; + +/** + * A {@link RepresentationConverter} that only converts representations + * that are not compatible with the preferences. + */ +export class IfNeededConverter extends RepresentationConverter { + private readonly converter: RepresentationConverter; + protected readonly logger = getLoggerFor(this); + + public constructor(converter: RepresentationConverter) { + super(); + this.converter = converter; + } + + public async canHandle(args: RepresentationConverterArgs): Promise { + if (this.needsConversion(args)) { + await this.converter.canHandle(args); + } + } + + public async handle(args: RepresentationConverterArgs): Promise { + return !this.needsConversion(args) ? args.representation : this.convert(args, false); + } + + public async handleSafe(args: RepresentationConverterArgs): Promise { + return !this.needsConversion(args) ? args.representation : this.convert(args, true); + } + + protected needsConversion({ identifier, representation, preferences }: RepresentationConverterArgs): boolean { + // No conversion needed if no preferences were specified + if (!preferences.type) { + return false; + } + + // No conversion is needed if there are any matches for the provided content type + const { contentType } = representation.metadata; + if (!contentType) { + throw new InternalServerError('Content-Type is required for data conversion.'); + } + const noMatchingMediaType = matchingMediaTypes(preferences.type, { [contentType]: 1 }).length === 0; + if (noMatchingMediaType) { + this.logger.debug(`Conversion needed for ${identifier + .path} from ${representation.metadata.contentType} to satisfy ${Object.entries(preferences.type) + .map(([ value, weight ]): string => `${value};q=${weight}`).join(', ')}`); + } + return noMatchingMediaType; + } + + protected async convert(args: RepresentationConverterArgs, safely: boolean): Promise { + const converted = await (safely ? this.converter.handleSafe(args) : this.converter.handle(args)); + this.logger.info(`Converted representation for ${args.identifier + .path} from ${args.representation.metadata.contentType} to ${converted.metadata.contentType}`); + return converted; + } +} diff --git a/src/storage/conversion/PassthroughConverter.ts b/src/storage/conversion/PassthroughConverter.ts new file mode 100644 index 000000000..0dbaca78b --- /dev/null +++ b/src/storage/conversion/PassthroughConverter.ts @@ -0,0 +1,12 @@ +import type { Representation } from '../../ldp/representation/Representation'; +import { RepresentationConverter } from './RepresentationConverter'; +import type { RepresentationConverterArgs } from './RepresentationConverter'; + +/** + * A {@link RepresentationConverter} that does not perform any conversion. + */ +export class PassthroughConverter extends RepresentationConverter { + public async handle({ representation }: RepresentationConverterArgs): Promise { + return representation; + } +} diff --git a/test/unit/storage/conversion/IfNeededConverter.test.ts b/test/unit/storage/conversion/IfNeededConverter.test.ts new file mode 100644 index 000000000..90b52a616 --- /dev/null +++ b/test/unit/storage/conversion/IfNeededConverter.test.ts @@ -0,0 +1,103 @@ +import type { Representation } from '../../../../src/ldp/representation/Representation'; +import { IfNeededConverter } from '../../../../src/storage/conversion/IfNeededConverter'; +import type { + RepresentationConverter, +} from '../../../../src/storage/conversion/RepresentationConverter'; + +describe('An IfNeededConverter', (): void => { + const identifier = { path: 'identifier' }; + const representation: Representation = { + metadata: { contentType: 'text/turtle' }, + } as any; + const converted = { + metadata: { contentType: 'application/ld+json' }, + }; + + const innerConverter: jest.Mocked = { + canHandle: jest.fn().mockResolvedValue(true), + handle: jest.fn().mockResolvedValue(converted), + handleSafe: jest.fn().mockResolvedValue(converted), + } as any; + + const converter = new IfNeededConverter(innerConverter); + + afterEach((): void => { + jest.clearAllMocks(); + }); + + it('performs no conversion when there are no content type preferences.', async(): Promise => { + const preferences = {}; + const args = { identifier, representation, preferences }; + + await expect(converter.canHandle(args)).resolves.toBeUndefined(); + await expect(converter.handle(args)).resolves.toBe(representation); + await expect(converter.handleSafe(args)).resolves.toBe(representation); + + expect(innerConverter.canHandle).toHaveBeenCalledTimes(0); + expect(innerConverter.handle).toHaveBeenCalledTimes(0); + expect(innerConverter.handleSafe).toHaveBeenCalledTimes(0); + }); + + it('errors if no content type is specified on the representation.', async(): Promise => { + const preferences = { type: { 'text/turtle': 1 }}; + const args = { identifier, representation: { metadata: {}} as any, preferences }; + + await expect(converter.handleSafe(args)).rejects + .toThrow('Content-Type is required for data conversion.'); + + expect(innerConverter.canHandle).toHaveBeenCalledTimes(0); + expect(innerConverter.handle).toHaveBeenCalledTimes(0); + expect(innerConverter.handleSafe).toHaveBeenCalledTimes(0); + }); + + it('performs no conversion when the content type matches the preferences.', async(): Promise => { + const preferences = { type: { 'text/turtle': 1 }}; + const args = { identifier, representation, preferences }; + + await expect(converter.handleSafe(args)).resolves.toBe(representation); + + expect(innerConverter.canHandle).toHaveBeenCalledTimes(0); + expect(innerConverter.handle).toHaveBeenCalledTimes(0); + expect(innerConverter.handleSafe).toHaveBeenCalledTimes(0); + }); + + it('performs a conversion when the content type matches the preferences.', async(): Promise => { + const preferences = { type: { 'text/turtle': 0 }}; + const args = { identifier, representation, preferences }; + + await expect(converter.handleSafe(args)).resolves.toBe(converted); + + expect(innerConverter.canHandle).toHaveBeenCalledTimes(0); + expect(innerConverter.handle).toHaveBeenCalledTimes(0); + expect(innerConverter.handleSafe).toHaveBeenCalledTimes(1); + expect(innerConverter.handleSafe).toHaveBeenCalledWith(args); + }); + + it('does not support conversion when the inner converter does not support it.', async(): Promise => { + const preferences = { type: { 'text/turtle': 0 }}; + const args = { identifier, representation, preferences }; + const error = new Error('unsupported'); + innerConverter.canHandle.mockRejectedValueOnce(error); + + await expect(converter.canHandle(args)).rejects.toThrow(error); + + expect(innerConverter.canHandle).toHaveBeenCalledTimes(1); + expect(innerConverter.canHandle).toHaveBeenCalledWith(args); + }); + + it('supports conversion when the inner converter supports it.', async(): Promise => { + const preferences = { type: { 'text/turtle': 0 }}; + const args = { identifier, representation, preferences }; + + await expect(converter.canHandle(args)).resolves.toBeUndefined(); + + expect(innerConverter.canHandle).toHaveBeenCalledTimes(1); + expect(innerConverter.canHandle).toHaveBeenCalledWith(args); + + await expect(converter.handle(args)).resolves.toBe(converted); + + expect(innerConverter.canHandle).toHaveBeenCalledTimes(1); + expect(innerConverter.handle).toHaveBeenCalledTimes(1); + expect(innerConverter.handle).toHaveBeenCalledWith(args); + }); +}); diff --git a/test/unit/storage/conversion/PassthroughConverter.test.ts b/test/unit/storage/conversion/PassthroughConverter.test.ts new file mode 100644 index 000000000..c64411dce --- /dev/null +++ b/test/unit/storage/conversion/PassthroughConverter.test.ts @@ -0,0 +1,20 @@ +import { PassthroughConverter } from '../../../../src/storage/conversion/PassthroughConverter'; + +describe('A PassthroughConverter', (): void => { + const representation = {}; + const args = { representation } as any; + + const converter = new PassthroughConverter(); + + it('supports any conversion.', async(): Promise => { + await expect(converter.canHandle(args)).resolves.toBeUndefined(); + }); + + it('returns the original representation on handle.', async(): Promise => { + await expect(converter.handle(args)).resolves.toBe(representation); + }); + + it('returns the original representation on handleSafe.', async(): Promise => { + await expect(converter.handleSafe(args)).resolves.toBe(representation); + }); +});