From d7434df8089cdd5c5f040f774710c62331f86ad9 Mon Sep 17 00:00:00 2001 From: Joachim Van Herwegen Date: Tue, 15 Dec 2020 14:32:34 +0100 Subject: [PATCH] feat: ExtensionBasedMapper no longer throws if there is no file --- src/storage/accessors/FileDataAccessor.ts | 4 ++-- src/storage/mapping/ExtensionBasedMapper.ts | 13 ++++--------- src/storage/mapping/FileIdentifierMapper.ts | 3 ++- .../accessors/FileDataAccessor.test.ts | 19 +++++++++++++------ .../mapping/ExtensionBasedMapper.test.ts | 16 ++++++++++++---- 5 files changed, 33 insertions(+), 22 deletions(-) diff --git a/src/storage/accessors/FileDataAccessor.ts b/src/storage/accessors/FileDataAccessor.ts index 9bd71978d..bf0298fb1 100644 --- a/src/storage/accessors/FileDataAccessor.ts +++ b/src/storage/accessors/FileDataAccessor.ts @@ -350,8 +350,8 @@ export class FileDataAccessor implements DataAccessor { await fsPromises.unlink(oldLink.filePath); } } catch (error: unknown) { - // Ignore it if the file didn't exist yet - if (!(error instanceof NotFoundHttpError)) { + // Ignore it if the file didn't exist yet and couldn't be unlinked + if (!isSystemError(error) || error.code !== 'ENOENT') { throw error; } } diff --git a/src/storage/mapping/ExtensionBasedMapper.ts b/src/storage/mapping/ExtensionBasedMapper.ts index 9e3a10185..ef214a92f 100644 --- a/src/storage/mapping/ExtensionBasedMapper.ts +++ b/src/storage/mapping/ExtensionBasedMapper.ts @@ -4,7 +4,6 @@ import * as mime from 'mime-types'; import type { ResourceIdentifier } from '../../ldp/representation/ResourceIdentifier'; import { getLoggerFor } from '../../logging/LogUtil'; import { APPLICATION_OCTET_STREAM, TEXT_TURTLE } from '../../util/ContentTypes'; -import { NotFoundHttpError } from '../../util/errors/NotFoundHttpError'; import { NotImplementedHttpError } from '../../util/errors/NotImplementedHttpError'; import { encodeUriPathComponents, @@ -98,22 +97,18 @@ export class ExtensionBasedMapper implements FileIdentifierMapper { ); } catch { // Parent folder does not exist (or is not a folder) - this.logger.warn(`No parent folder for ${identifier.path} found at ${folder}`); - throw new NotFoundHttpError(); } - // File doesn't exist - if (!fileName) { - this.logger.warn(`File for URL ${identifier.path} does not exist in ${folder}`); - throw new NotFoundHttpError(); + // Matching file found + if (fileName) { + filePath = joinPath(folder, fileName); } - filePath = joinPath(folder, fileName); this.logger.info(`The path for ${identifier.path} is ${filePath}`); return { identifier, filePath, - contentType: this.getContentTypeFromExtension(fileName), + contentType: this.getContentTypeFromExtension(filePath), }; } diff --git a/src/storage/mapping/FileIdentifierMapper.ts b/src/storage/mapping/FileIdentifierMapper.ts index e55aa07a7..e87c85951 100644 --- a/src/storage/mapping/FileIdentifierMapper.ts +++ b/src/storage/mapping/FileIdentifierMapper.ts @@ -29,7 +29,8 @@ export interface FileIdentifierMapper { mapFilePathToUrl: (filePath: string, isContainer: boolean) => Promise; /** * Maps the given resource identifier / URL to a file path. - * Determines the content-type if no content-type was provided. + * Determines the content-type if no content-type was provided by finding the corresponding file. + * If there is no corresponding file a file path will be generated. * For containers the content-type input gets ignored. * @param identifier - The input identifier. * @param contentType - The (optional) content-type of the resource. diff --git a/test/unit/storage/accessors/FileDataAccessor.test.ts b/test/unit/storage/accessors/FileDataAccessor.test.ts index 769313d74..e6b99a20d 100644 --- a/test/unit/storage/accessors/FileDataAccessor.test.ts +++ b/test/unit/storage/accessors/FileDataAccessor.test.ts @@ -223,20 +223,27 @@ describe('A FileDataAccessor', (): void => { }); }); + it('does not try to update the content-type if there is no original file.', async(): Promise => { + metadata.identifier = DataFactory.namedNode(`${base}resource.txt`); + metadata.contentType = 'text/turtle'; + metadata.add('new', 'metadata'); + await expect(accessor.writeDocument({ path: `${base}resource.txt` }, data, metadata)) + .resolves.toBeUndefined(); + expect(cache.data).toEqual({ + 'resource.txt$.ttl': 'data', + 'resource.txt.meta': expect.stringMatching(`<${base}resource.txt> "metadata".`), + }); + }); + it('throws an error if there is an issue deleting the original file.', async(): Promise => { cache.data = { 'resource$.ttl': ' .' }; jest.requireMock('fs').promises.unlink = (): any => { const error = new Error('error') as SystemError; - error.code = 'ENOENT'; + error.code = 'EISDIR'; error.syscall = 'unlink'; throw error; }; - // `unlink` throwing ENOENT should not be an issue if the content-type does not change - metadata.contentType = 'text/turtle'; - await expect(accessor.writeDocument({ path: `${base}resource` }, data, metadata)) - .resolves.toBeUndefined(); - metadata.contentType = 'text/plain'; await expect(accessor.writeDocument({ path: `${base}resource` }, data, metadata)) .rejects.toThrow(new Error('error')); diff --git a/test/unit/storage/mapping/ExtensionBasedMapper.test.ts b/test/unit/storage/mapping/ExtensionBasedMapper.test.ts index 2a0296285..75c357f8d 100644 --- a/test/unit/storage/mapping/ExtensionBasedMapper.test.ts +++ b/test/unit/storage/mapping/ExtensionBasedMapper.test.ts @@ -50,16 +50,24 @@ describe('An ExtensionBasedMapper', (): void => { .rejects.toThrow(new BadRequestHttpError('Identifiers cannot contain a dollar sign before their extension')); }); - it('throws 404 when looking in a folder that does not exist.', async(): Promise => { + it('determines content-type by extension when looking in a folder that does not exist.', async(): Promise => { fsPromises.readdir.mockImplementation((): void => { throw new Error('does not exist'); }); - await expect(mapper.mapUrlToFilePath({ path: `${base}no/test.txt` })).rejects.toThrow(NotFoundHttpError); + await expect(mapper.mapUrlToFilePath({ path: `${base}no/test.txt` })).resolves.toEqual({ + identifier: { path: `${base}no/test.txt` }, + filePath: `${rootFilepath}no/test.txt`, + contentType: 'text/plain', + }); }); - it('throws 404 when looking for a file that does not exist.', async(): Promise => { + it('determines content-type by extension when looking for a file that does not exist.', async(): Promise => { fsPromises.readdir.mockReturnValue([ 'test.ttl' ]); - await expect(mapper.mapUrlToFilePath({ path: `${base}test.txt` })).rejects.toThrow(NotFoundHttpError); + await expect(mapper.mapUrlToFilePath({ path: `${base}test.txt` })).resolves.toEqual({ + identifier: { path: `${base}test.txt` }, + filePath: `${rootFilepath}test.txt`, + contentType: 'text/plain', + }); }); it('determines the content-type based on the extension.', async(): Promise => {