From 0bd73115cc15696bde0b9a3c9d70db3c38da5fd9 Mon Sep 17 00:00:00 2001 From: Ruben Verborgh Date: Tue, 5 Jan 2021 00:43:25 +0100 Subject: [PATCH] refactor: Simplify supportsMediaTypeConversion arguments. --- src/storage/conversion/ChainedConverter.ts | 8 +-- src/storage/conversion/ConversionUtil.ts | 26 ++++----- .../TypedRepresentationConverter.ts | 5 +- .../storage/conversion/ConversionUtil.test.ts | 55 +++++-------------- 4 files changed, 29 insertions(+), 65 deletions(-) diff --git a/src/storage/conversion/ChainedConverter.ts b/src/storage/conversion/ChainedConverter.ts index 10fdecbc1..4337b441e 100644 --- a/src/storage/conversion/ChainedConverter.ts +++ b/src/storage/conversion/ChainedConverter.ts @@ -1,7 +1,7 @@ import type { Representation } from '../../ldp/representation/Representation'; import type { ValuePreferences } from '../../ldp/representation/RepresentationPreferences'; import { getLoggerFor } from '../../logging/LogUtil'; -import { supportsConversion, matchesMediaType } from './ConversionUtil'; +import { matchesMediaType } from './ConversionUtil'; import type { RepresentationConverterArgs } from './RepresentationConverter'; import { TypedRepresentationConverter } from './TypedRepresentationConverter'; @@ -43,12 +43,6 @@ export class ChainedConverter extends TypedRepresentationConverter { return this.last.getOutputTypes(); } - public async canHandle(input: RepresentationConverterArgs): Promise { - // We assume a chain can be constructed, otherwise there would be a configuration issue - // So we only check if the input can be parsed and the preferred type can be written - supportsConversion(input, await this.first.getInputTypes(), await this.last.getOutputTypes()); - } - public async handle(input: RepresentationConverterArgs): Promise { const args = { ...input }; for (let i = 0; i < this.converters.length - 1; ++i) { diff --git a/src/storage/conversion/ConversionUtil.ts b/src/storage/conversion/ConversionUtil.ts index 68017da4e..1cfc9e40f 100644 --- a/src/storage/conversion/ConversionUtil.ts +++ b/src/storage/conversion/ConversionUtil.ts @@ -1,9 +1,7 @@ import type { ValuePreferences } from '../../ldp/representation/RepresentationPreferences'; import { INTERNAL_ALL } from '../../util/ContentTypes'; -import { BadRequestHttpError } from '../../util/errors/BadRequestHttpError'; import { InternalServerError } from '../../util/errors/InternalServerError'; import { NotImplementedHttpError } from '../../util/errors/NotImplementedHttpError'; -import type { RepresentationConverterArgs } from './RepresentationConverter'; /** * Filters media types based on the given preferences. @@ -91,20 +89,20 @@ export const matchesMediaType = (mediaA: string, mediaB: string): boolean => { * - Checks if the input type is supported by the parser. * - Checks if the parser can produce one of the preferred output types. * Throws an error with details if conversion is not possible. - * @param request - Incoming arguments. - * @param supportedIn - Media types that can be parsed by the converter. - * @param supportedOut - Media types that can be produced by the converter. + * @param inputType - Actual input type. + * @param outputTypes - Acceptable output types. + * @param convertorIn - Media types that can be parsed by the converter. + * @param convertorOut - Media types that can be produced by the converter. */ -export const supportsConversion = (request: RepresentationConverterArgs, supportedIn: ValuePreferences, - supportedOut: ValuePreferences): void => { - const inType = request.representation.metadata.contentType; - if (!inType) { - throw new BadRequestHttpError('No content type indicated on request.'); - } - if (!Object.keys(supportedIn).some((type): boolean => matchesMediaType(inType, type)) || - matchingMediaTypes(request.preferences.type, supportedOut).length === 0) { +export const supportsMediaTypeConversion = ( + inputType = 'unknown', outputTypes: ValuePreferences = {}, + convertorIn: ValuePreferences = {}, convertorOut: ValuePreferences = {}, +): void => { + if (!Object.keys(convertorIn).some((type): boolean => matchesMediaType(inputType, type)) || + matchingMediaTypes(outputTypes, convertorOut).length === 0) { throw new NotImplementedHttpError( - `Can only convert from ${Object.keys(supportedIn)} to ${Object.keys(supportedOut)}.`, + `Cannot convert from ${inputType} to ${Object.keys(outputTypes) + }, only from ${Object.keys(convertorIn)} to ${Object.keys(convertorOut)}.`, ); } }; diff --git a/src/storage/conversion/TypedRepresentationConverter.ts b/src/storage/conversion/TypedRepresentationConverter.ts index b142d7428..eb1e33658 100644 --- a/src/storage/conversion/TypedRepresentationConverter.ts +++ b/src/storage/conversion/TypedRepresentationConverter.ts @@ -1,5 +1,5 @@ import type { ValuePreferences } from '../../ldp/representation/RepresentationPreferences'; -import { supportsConversion } from './ConversionUtil'; +import { supportsMediaTypeConversion } from './ConversionUtil'; import { RepresentationConverter } from './RepresentationConverter'; import type { RepresentationConverterArgs } from './RepresentationConverter'; @@ -22,7 +22,8 @@ export abstract class TypedRepresentationConverter extends RepresentationConvert */ public async canHandle(args: RepresentationConverterArgs): Promise { const types = [ this.getInputTypes(), this.getOutputTypes() ]; + const { contentType } = args.representation.metadata; const [ inputTypes, outputTypes ] = await Promise.all(types); - supportsConversion(args, inputTypes, outputTypes); + supportsMediaTypeConversion(contentType, args.preferences.type, inputTypes, outputTypes); } } diff --git a/test/unit/storage/conversion/ConversionUtil.test.ts b/test/unit/storage/conversion/ConversionUtil.test.ts index b3451901c..f53619e22 100644 --- a/test/unit/storage/conversion/ConversionUtil.test.ts +++ b/test/unit/storage/conversion/ConversionUtil.test.ts @@ -1,63 +1,34 @@ -import type { Representation } from '../../../../src/ldp/representation/Representation'; -import { RepresentationMetadata } from '../../../../src/ldp/representation/RepresentationMetadata'; -import type { - ValuePreferences, - RepresentationPreferences, -} from '../../../../src/ldp/representation/RepresentationPreferences'; -import type { ResourceIdentifier } from '../../../../src/ldp/representation/ResourceIdentifier'; +import type { ValuePreferences } from '../../../../src/ldp/representation/RepresentationPreferences'; import { matchesMediaType, matchingMediaTypes, - supportsConversion, + supportsMediaTypeConversion, } from '../../../../src/storage/conversion/ConversionUtil'; import { InternalServerError } from '../../../../src/util/errors/InternalServerError'; describe('ConversionUtil', (): void => { - const identifier: ResourceIdentifier = { path: 'path' }; - let representation: Representation; - let metadata: RepresentationMetadata; + describe('supportsMediaTypeConversion', (): void => { + it('requires preferences.', async(): Promise => { + expect((): any => supportsMediaTypeConversion()).toThrow(); + }); - beforeEach(async(): Promise => { - metadata = new RepresentationMetadata(); - representation = { metadata } as Representation; - }); - - describe('#supportsConversion', (): void => { it('requires an input type.', async(): Promise => { - const preferences: RepresentationPreferences = {}; - expect((): any => supportsConversion({ identifier, representation, preferences }, - { 'a/x': 1 }, - { 'a/x': 1 })) - .toThrow('No content type indicated on request.'); + expect((): any => supportsMediaTypeConversion(undefined, { 'b/x': 1 }, { 'a/x': 1 }, { 'a/x': 1 })) + .toThrow('Cannot convert from unknown to b/x, only from a/x to a/x.'); }); it('requires a matching input type.', async(): Promise => { - metadata.contentType = 'a/x'; - const preferences: RepresentationPreferences = - { type: { 'b/x': 1 }}; - expect((): any => supportsConversion({ identifier, representation, preferences }, - { 'c/x': 1 }, - { 'a/x': 1 })) - .toThrow('Can only convert from c/x to a/x.'); + expect((): any => supportsMediaTypeConversion('a/x', { 'b/x': 1 }, { 'c/x': 1 }, { 'a/x': 1 })) + .toThrow('Cannot convert from a/x to b/x, only from c/x to a/x.'); }); it('requires a matching output type.', async(): Promise => { - metadata.contentType = 'a/x'; - const preferences: RepresentationPreferences = - { type: { 'b/x': 1 }}; - expect((): any => supportsConversion({ identifier, representation, preferences }, - { 'a/x': 1 }, - { 'c/x': 1 })) - .toThrow('Can only convert from a/x to c/x.'); + expect((): any => supportsMediaTypeConversion('a/x', { 'b/x': 1 }, { 'a/x': 1 }, { 'c/x': 1 })) + .toThrow('Cannot convert from a/x to b/x, only from a/x to c/x.'); }); it('succeeds with a valid input and output type.', async(): Promise => { - metadata.contentType = 'a/x'; - const preferences: RepresentationPreferences = - { type: { 'b/x': 1 }}; - expect(supportsConversion({ identifier, representation, preferences }, - { 'a/x': 1 }, - { 'b/x': 1 })) + expect(supportsMediaTypeConversion('a/x', { 'b/x': 1 }, { 'a/x': 1 }, { 'b/x': 1 })) .toBeUndefined(); }); });