fix: Only check relevant type triples

Due to the introduction of pim:Storage as a new type
the DataAccessorBasedStore started making wrong assumptions.
This commit is contained in:
Joachim Van Herwegen 2020-12-16 16:11:58 +01:00
parent 70cc3596db
commit a721684e6b
3 changed files with 39 additions and 14 deletions

View File

@ -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);
}

View File

@ -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<boolean> {
// 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);

View File

@ -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<void> => {
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<void> => {
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<void> => {
const resourceID = { path: `${root}container/` };
representation.metadata.add(RDF.type, toNamedNode(LDP.Container));