From a721684e6b5f67b921e81caa8502da9dde401889 Mon Sep 17 00:00:00 2001 From: Joachim Van Herwegen Date: Wed, 16 Dec 2020 16:11:58 +0100 Subject: [PATCH] fix: Only check relevant type triples Due to the introduction of pim:Storage as a new type the DataAccessorBasedStore started making wrong assumptions. --- src/storage/DataAccessorBasedStore.ts | 27 ++++++++++++------- src/storage/accessors/FileDataAccessor.ts | 6 +++-- .../storage/DataAccessorBasedStore.test.ts | 20 ++++++++++++-- 3 files changed, 39 insertions(+), 14 deletions(-) diff --git a/src/storage/DataAccessorBasedStore.ts b/src/storage/DataAccessorBasedStore.ts index da97b8e34..cad36e803 100644 --- a/src/storage/DataAccessorBasedStore.ts +++ b/src/storage/DataAccessorBasedStore.ts @@ -1,6 +1,6 @@ import arrayifyStream from 'arrayify-stream'; import { DataFactory } from 'n3'; -import type { Quad } from 'rdf-js'; +import type { Quad, Term } from 'rdf-js'; import { v4 as uuid } from 'uuid'; import type { Representation } from '../ldp/representation/Representation'; import { RepresentationMetadata } from '../ldp/representation/RepresentationMetadata'; @@ -8,6 +8,7 @@ import type { ResourceIdentifier } from '../ldp/representation/ResourceIdentifie import { INTERNAL_QUADS } from '../util/ContentTypes'; import { BadRequestHttpError } from '../util/errors/BadRequestHttpError'; import { ConflictHttpError } from '../util/errors/ConflictHttpError'; +import { InternalServerError } from '../util/errors/InternalServerError'; import { MethodNotAllowedHttpError } from '../util/errors/MethodNotAllowedHttpError'; import { NotFoundHttpError } from '../util/errors/NotFoundHttpError'; import { NotImplementedHttpError } from '../util/errors/NotImplementedHttpError'; @@ -318,14 +319,13 @@ export class DataAccessorBasedStore implements ResourceStore { * @param suffix - Suffix of the URI. Can be the full URI, but only the last part is required. */ protected isNewContainer(metadata: RepresentationMetadata, suffix?: string): boolean { - let isContainer: boolean; - try { - isContainer = this.isExistingContainer(metadata); - } catch { - const slug = suffix ?? metadata.get(HTTP.slug)?.value; - isContainer = Boolean(slug && isContainerPath(slug)); + // Should not use `isExistingContainer` since the metadata might contain unrelated type triples + // It's not because there is no container type triple that the new resource is not a container + if (this.hasContainerType(metadata.getAll(RDF.type))) { + return true; } - return isContainer; + const slug = suffix ?? metadata.get(HTTP.slug)?.value; + return Boolean(slug && isContainerPath(slug)); } /** @@ -335,9 +335,16 @@ export class DataAccessorBasedStore implements ResourceStore { */ protected isExistingContainer(metadata: RepresentationMetadata): boolean { const types = metadata.getAll(RDF.type); - if (types.length === 0) { - throw new Error('Unknown resource type.'); + if (!types.some((type): boolean => type.value === LDP.Resource)) { + throw new InternalServerError('Unknown resource type.'); } + return this.hasContainerType(types); + } + + /** + * Checks in a list of types if any of them match a Container type. + */ + protected hasContainerType(types: Term[]): boolean { return types.some((type): boolean => type.value === LDP.Container || type.value === LDP.BasicContainer); } diff --git a/src/storage/accessors/FileDataAccessor.ts b/src/storage/accessors/FileDataAccessor.ts index 744d67d70..6fef0fdb3 100644 --- a/src/storage/accessors/FileDataAccessor.ts +++ b/src/storage/accessors/FileDataAccessor.ts @@ -16,7 +16,7 @@ import type { Guarded } from '../../util/GuardedStream'; import { isContainerIdentifier } from '../../util/PathUtil'; import { parseQuads, pushQuad, serializeQuads } from '../../util/QuadUtil'; import { generateContainmentQuads, generateResourceQuads } from '../../util/ResourceUtil'; -import { CONTENT_TYPE, DCTERMS, POSIX, RDF, XSD } from '../../util/UriConstants'; +import { CONTENT_TYPE, DCTERMS, LDP, POSIX, RDF, XSD } from '../../util/UriConstants'; import { toNamedNode, toTypedLiteral } from '../../util/UriUtil'; import type { FileIdentifierMapper, ResourceLink } from '../mapping/FileIdentifierMapper'; import type { DataAccessor } from './DataAccessor'; @@ -210,7 +210,9 @@ export class FileDataAccessor implements DataAccessor { */ private async writeMetadata(link: ResourceLink, metadata: RepresentationMetadata): Promise { // These are stored by file system conventions - metadata.removeAll(RDF.type); + metadata.remove(RDF.type, toNamedNode(LDP.Resource)); + metadata.remove(RDF.type, toNamedNode(LDP.Container)); + metadata.remove(RDF.type, toNamedNode(LDP.BasicContainer)); metadata.removeAll(CONTENT_TYPE); const quads = metadata.quads(); const metadataLink = await this.getMetadataLink(link.identifier); diff --git a/test/unit/storage/DataAccessorBasedStore.test.ts b/test/unit/storage/DataAccessorBasedStore.test.ts index 3512dc413..560f092d8 100644 --- a/test/unit/storage/DataAccessorBasedStore.test.ts +++ b/test/unit/storage/DataAccessorBasedStore.test.ts @@ -10,6 +10,7 @@ import { DataAccessorBasedStore } from '../../../src/storage/DataAccessorBasedSt import { INTERNAL_QUADS } from '../../../src/util/ContentTypes'; import { BadRequestHttpError } from '../../../src/util/errors/BadRequestHttpError'; import { ConflictHttpError } from '../../../src/util/errors/ConflictHttpError'; +import { InternalServerError } from '../../../src/util/errors/InternalServerError'; import { MethodNotAllowedHttpError } from '../../../src/util/errors/MethodNotAllowedHttpError'; import { NotFoundHttpError } from '../../../src/util/errors/NotFoundHttpError'; import { NotImplementedHttpError } from '../../../src/util/errors/NotImplementedHttpError'; @@ -83,7 +84,11 @@ describe('A DataAccessorBasedStore', (): void => { store = new DataAccessorBasedStore(accessor, identifierStrategy); containerMetadata = new RepresentationMetadata( - { [RDF.type]: [ DataFactory.namedNode(LDP.Container), DataFactory.namedNode(LDP.BasicContainer) ]}, + { [RDF.type]: [ + DataFactory.namedNode(LDP.Resource), + DataFactory.namedNode(LDP.Container), + DataFactory.namedNode(LDP.BasicContainer), + ]}, ); accessor.data[root] = { metadata: containerMetadata } as Representation; @@ -270,12 +275,23 @@ describe('A DataAccessorBasedStore', (): void => { }); it('will error if the ending slash does not match its resource type.', async(): Promise => { - const resourceID = { path: `${root}resource/` }; + const resourceID = { path: `${root}resource` }; + representation.metadata.add(RDF.type, toNamedNode(LDP.Container)); await expect(store.setRepresentation(resourceID, representation)).rejects.toThrow( new BadRequestHttpError('Containers should have a `/` at the end of their path, resources should not.'), ); }); + it('will error if the DataAccessor did not store the required type triples.', async(): Promise => { + const resourceID = { path: `${root}resource` }; + accessor.data[resourceID.path] = representation; + representation.metadata.identifier = namedNode(resourceID.path); + representation.metadata.removeAll(RDF.type); + await expect(store.setRepresentation(resourceID, representation)).rejects.toThrow( + new InternalServerError('Unknown resource type.'), + ); + }); + it('errors when trying to create a container with non-RDF data.', async(): Promise => { const resourceID = { path: `${root}container/` }; representation.metadata.add(RDF.type, toNamedNode(LDP.Container));