From dc7592ebc4863ce04e3ae140322d6bf1b7c6b416 Mon Sep 17 00:00:00 2001 From: Joachim Van Herwegen Date: Tue, 18 May 2021 16:00:11 +0200 Subject: [PATCH] fix: Add conditional parameters to ConstantConverter --- config/util/index/example.json | 7 ++- src/storage/conversion/ConstantConverter.ts | 50 ++++++++++++++-- .../conversion/ConstantConverter.test.ts | 59 ++++++++++++++++--- 3 files changed, 103 insertions(+), 13 deletions(-) diff --git a/config/util/index/example.json b/config/util/index/example.json index ecce03b9b..e60eeaa8f 100644 --- a/config/util/index/example.json +++ b/config/util/index/example.json @@ -8,8 +8,11 @@ ], "@id": "urn:solid-server:default:IndexConverter", "@type": "ConstantConverter", - "ConstantConverter:_contentType": "text/html", - "ConstantConverter:_filePath": "./node_modules/mashlib/dist/databrowser.html" + "contentType": "text/html", + "filePath": "./node_modules/mashlib/dist/databrowser.html", + "options_container": true, + "options_document": false, + "options_minQuality": 1 } ] } diff --git a/src/storage/conversion/ConstantConverter.ts b/src/storage/conversion/ConstantConverter.ts index 501fbcde8..040d5279a 100644 --- a/src/storage/conversion/ConstantConverter.ts +++ b/src/storage/conversion/ConstantConverter.ts @@ -2,10 +2,29 @@ import { createReadStream } from 'fs'; import { BasicRepresentation } from '../../ldp/representation/BasicRepresentation'; import type { Representation } from '../../ldp/representation/Representation'; import { NotImplementedHttpError } from '../../util/errors/NotImplementedHttpError'; -import { matchesMediaType, matchesMediaPreferences } from './ConversionUtil'; +import { isContainerIdentifier } from '../../util/PathUtil'; +import { matchesMediaType, getTypeWeight, cleanPreferences } from './ConversionUtil'; import { RepresentationConverter } from './RepresentationConverter'; import type { RepresentationConverterArgs } from './RepresentationConverter'; +/** + * Extra options for the ConstantConverter. + */ +export interface ConstantConverterOptions { + /** + * Whether this should trigger on containers. + */ + container?: boolean; + /** + * Whether this should trigger on documents. + */ + document?: boolean; + /** + * The minimum requested quality/preference before this should trigger. + */ + minQuality?: number; +} + /** * A {@link RepresentationConverter} that ensures * a representation for a certain content type is available. @@ -15,30 +34,53 @@ import type { RepresentationConverterArgs } from './RepresentationConverter'; * * This can for example be used to serve an index.html file, * which could then interactively load another representation. + * + * Options default to the most permissive values when not defined. */ export class ConstantConverter extends RepresentationConverter { private readonly filePath: string; private readonly contentType: string; + private readonly options: Required; /** * Creates a new constant converter. * * @param filePath - The path to the constant representation. * @param contentType - The content type of the constant representation. + * @param options - Extra options for the converter. */ - public constructor(filePath: string, contentType: string) { + public constructor(filePath: string, contentType: string, options: ConstantConverterOptions = {}) { super(); this.filePath = filePath; this.contentType = contentType; + this.options = { + container: options.container ?? true, + document: options.document ?? true, + minQuality: options.minQuality ?? 0, + }; } - public async canHandle({ preferences, representation }: RepresentationConverterArgs): Promise { + public async canHandle({ identifier, preferences, representation }: RepresentationConverterArgs): Promise { // Do not replace the representation if there is no preference for our content type if (!preferences.type) { throw new NotImplementedHttpError('No content type preferences specified'); } - if (!matchesMediaPreferences(this.contentType, { ...preferences.type, '*/*': 0 })) { + + // Do not replace the representation of unsupported resource types + const isContainer = isContainerIdentifier(identifier); + if (isContainer && !this.options.container) { + throw new NotImplementedHttpError('Containers are not supported'); + } + if (!isContainer && !this.options.document) { + throw new NotImplementedHttpError('Documents are not supported'); + } + + // Do not replace the representation if the preference weight is too low + const quality = getTypeWeight(this.contentType, cleanPreferences({ ...preferences.type, '*/*': 0 })); + if (quality === 0) { throw new NotImplementedHttpError(`No preference for ${this.contentType}`); + } else if (quality < this.options.minQuality) { + throw new NotImplementedHttpError(`Preference is lower than the specified minimum quality`); } // Do not replace the representation if it already has our content type diff --git a/test/unit/storage/conversion/ConstantConverter.test.ts b/test/unit/storage/conversion/ConstantConverter.test.ts index f71585358..27e81ff78 100644 --- a/test/unit/storage/conversion/ConstantConverter.test.ts +++ b/test/unit/storage/conversion/ConstantConverter.test.ts @@ -1,22 +1,47 @@ import fs from 'fs'; import arrayifyStream from 'arrayify-stream'; import { RepresentationMetadata } from '../../../../src/ldp/representation/RepresentationMetadata'; +import type { ConstantConverterOptions } from '../../../../src/storage/conversion/ConstantConverter'; import { ConstantConverter } from '../../../../src/storage/conversion/ConstantConverter'; const createReadStream = jest.spyOn(fs, 'createReadStream').mockReturnValue('file contents' as any); describe('A ConstantConverter', (): void => { const identifier = { path: 'identifier' }; + let options: ConstantConverterOptions; + let converter: ConstantConverter; - const converter = new ConstantConverter('abc/def/index.html', 'text/html'); + beforeEach(async(): Promise => { + options = { container: true, document: true, minQuality: 1 }; + converter = new ConstantConverter('abc/def/index.html', 'text/html', options); + }); it('does not support requests without content type preferences.', async(): Promise => { const preferences = {}; const representation = {} as any; const args = { identifier, representation, preferences }; - await expect(converter.canHandle(args)).rejects - .toThrow('No content type preferences specified'); + await expect(converter.canHandle(args)).rejects.toThrow('No content type preferences specified'); + }); + + it('does not support requests targeting documents if disabled in the options.', async(): Promise => { + const preferences = { type: { 'text/html': 1 }}; + const representation = { metadata: new RepresentationMetadata() } as any; + const args = { identifier, representation, preferences }; + + converter = new ConstantConverter('abc/def/index.html', 'text/html', { document: false }); + + await expect(converter.canHandle(args)).rejects.toThrow('Documents are not supported'); + }); + + it('does not support requests targeting containers if disabled in the options.', async(): Promise => { + const preferences = { type: { 'text/html': 1 }}; + const representation = { metadata: new RepresentationMetadata() } as any; + const args = { identifier: { path: 'container/' }, representation, preferences }; + + converter = new ConstantConverter('abc/def/index.html', 'text/html', { container: false }); + + await expect(converter.canHandle(args)).rejects.toThrow('Containers are not supported'); }); it('does not support requests without matching content type preference.', async(): Promise => { @@ -24,8 +49,15 @@ describe('A ConstantConverter', (): void => { const representation = {} as any; const args = { identifier, representation, preferences }; - await expect(converter.canHandle(args)).rejects - .toThrow('No preference for text/html'); + await expect(converter.canHandle(args)).rejects.toThrow('No preference for text/html'); + }); + + it('does not support requests not reaching the minimum preference quality.', async(): Promise => { + const preferences = { type: { 'text/html': 0.9 }}; + const representation = {} as any; + const args = { identifier, representation, preferences }; + + await expect(converter.canHandle(args)).rejects.toThrow('Preference is lower than the specified minimum quality'); }); it('does not support representations that are already in the right format.', async(): Promise => { @@ -34,8 +66,7 @@ describe('A ConstantConverter', (): void => { const representation = { metadata } as any; const args = { identifier, representation, preferences }; - await expect(converter.canHandle(args)).rejects - .toThrow('Representation is already text/html'); + await expect(converter.canHandle(args)).rejects.toThrow('Representation is already text/html'); }); it('supports representations with an unknown content type.', async(): Promise => { @@ -64,4 +95,18 @@ describe('A ConstantConverter', (): void => { expect(converted.metadata.contentType).toBe('text/html'); expect(await arrayifyStream(converted.data)).toEqual([ 'file contents' ]); }); + + it('defaults to the most permissive options.', async(): Promise => { + const preferences = { type: { 'text/html': 0.1 }}; + const metadata = new RepresentationMetadata(); + const representation = { metadata } as any; + const args = { identifier, representation, preferences }; + + converter = new ConstantConverter('abc/def/index.html', 'text/html'); + + await expect(converter.canHandle(args)).resolves.toBeUndefined(); + + args.identifier = { path: 'container/' }; + await expect(converter.canHandle(args)).resolves.toBeUndefined(); + }); });