From 0c047234e32f6c459f2dad2011014cd8195b43fd Mon Sep 17 00:00:00 2001 From: Joachim Van Herwegen Date: Tue, 2 Feb 2021 15:37:19 +0100 Subject: [PATCH] feat: Support auxiliary behaviour in DataAccessorBasedStore --- .../storage/backend/storage-filesystem.json | 3 + .../storage/backend/storage-memory.json | 3 + .../backend/storage-sparql-endpoint.json | 3 + .../representation/RepresentationMetadata.ts | 1 - src/storage/DataAccessorBasedStore.ts | 82 ++++++++- src/storage/ResourceStore.ts | 3 + test/integration/LockingResourceStore.test.ts | 1 + .../storage/DataAccessorBasedStore.test.ts | 164 +++++++++++++++++- 8 files changed, 247 insertions(+), 13 deletions(-) diff --git a/config/presets/storage/backend/storage-filesystem.json b/config/presets/storage/backend/storage-filesystem.json index 7d041eae8..38bb8328f 100644 --- a/config/presets/storage/backend/storage-filesystem.json +++ b/config/presets/storage/backend/storage-filesystem.json @@ -28,6 +28,9 @@ }, "DataAccessorBasedStore:_identifierStrategy": { "@id": "urn:solid-server:default:IdentifierStrategy" + }, + "DataAccessorBasedStore:_auxiliaryStrategy": { + "@id": "urn:solid-server:default:AuxiliaryStrategy" } } ] diff --git a/config/presets/storage/backend/storage-memory.json b/config/presets/storage/backend/storage-memory.json index 31fdea201..7d75437dc 100644 --- a/config/presets/storage/backend/storage-memory.json +++ b/config/presets/storage/backend/storage-memory.json @@ -16,6 +16,9 @@ }, "DataAccessorBasedStore:_identifierStrategy": { "@id": "urn:solid-server:default:IdentifierStrategy" + }, + "DataAccessorBasedStore:_auxiliaryStrategy": { + "@id": "urn:solid-server:default:AuxiliaryStrategy" } } ] diff --git a/config/presets/storage/backend/storage-sparql-endpoint.json b/config/presets/storage/backend/storage-sparql-endpoint.json index ba7883c8f..3ed999079 100644 --- a/config/presets/storage/backend/storage-sparql-endpoint.json +++ b/config/presets/storage/backend/storage-sparql-endpoint.json @@ -20,6 +20,9 @@ }, "DataAccessorBasedStore:_identifierStrategy": { "@id": "urn:solid-server:default:IdentifierStrategy" + }, + "DataAccessorBasedStore:_auxiliaryStrategy": { + "@id": "urn:solid-server:default:AuxiliaryStrategy" } }, diff --git a/src/ldp/representation/RepresentationMetadata.ts b/src/ldp/representation/RepresentationMetadata.ts index 824a5f9b5..be82e4c7a 100644 --- a/src/ldp/representation/RepresentationMetadata.ts +++ b/src/ldp/representation/RepresentationMetadata.ts @@ -216,7 +216,6 @@ export class RepresentationMetadata { return this.forQuads(predicate, object, (pred, obj): any => this.removeQuad(this.id, pred, obj)); } - // TODO: test all 3 /** * Helper function to simplify add/remove * Runs the given function on all predicate/object pairs, but only converts the predicate to a named node once. diff --git a/src/storage/DataAccessorBasedStore.ts b/src/storage/DataAccessorBasedStore.ts index c03dd75ca..c28a21e82 100644 --- a/src/storage/DataAccessorBasedStore.ts +++ b/src/storage/DataAccessorBasedStore.ts @@ -1,11 +1,13 @@ import arrayifyStream from 'arrayify-stream'; import { DataFactory } from 'n3'; -import type { Quad, Term } from 'rdf-js'; +import type { NamedNode, Quad, Term } from 'rdf-js'; import { v4 as uuid } from 'uuid'; +import type { AuxiliaryStrategy } from '../ldp/auxiliary/AuxiliaryStrategy'; import { BasicRepresentation } from '../ldp/representation/BasicRepresentation'; import type { Representation } from '../ldp/representation/Representation'; import type { RepresentationMetadata } from '../ldp/representation/RepresentationMetadata'; import type { ResourceIdentifier } from '../ldp/representation/ResourceIdentifier'; +import { getLoggerFor } from '../logging/LogUtil'; import { INTERNAL_QUADS } from '../util/ContentTypes'; import { BadRequestHttpError } from '../util/errors/BadRequestHttpError'; import { ConflictHttpError } from '../util/errors/ConflictHttpError'; @@ -51,12 +53,17 @@ import type { ResourceStore } from './ResourceStore'; * but the main disadvantage is that sometimes multiple calls are required where a specific store might only need one. */ export class DataAccessorBasedStore implements ResourceStore { + protected readonly logger = getLoggerFor(this); + private readonly accessor: DataAccessor; private readonly identifierStrategy: IdentifierStrategy; + private readonly auxiliaryStrategy: AuxiliaryStrategy; - public constructor(accessor: DataAccessor, identifierStrategy: IdentifierStrategy) { + public constructor(accessor: DataAccessor, identifierStrategy: IdentifierStrategy, + auxiliaryStrategy: AuxiliaryStrategy) { this.accessor = accessor; this.identifierStrategy = identifierStrategy; + this.auxiliaryStrategy = auxiliaryStrategy; } public async getRepresentation(identifier: ResourceIdentifier): Promise { @@ -66,7 +73,17 @@ export class DataAccessorBasedStore implements ResourceStore { const metadata = await this.accessor.getMetadata(identifier); let representation: Representation; + // Potentially add auxiliary related metadata + // Solid, §4.3: "Clients can discover auxiliary resources associated with a subject resource by making an HTTP HEAD + // or GET request on the target URL, and checking the HTTP Link header with the rel parameter" + // https://solid.github.io/specification/protocol#auxiliary-resources + await this.auxiliaryStrategy.addMetadata(metadata); + if (isContainerPath(metadata.identifier.value)) { + // Remove containment references of auxiliary resources + const auxContains = this.getContainedAuxiliaryResources(metadata); + metadata.remove(LDP.terms.contains, auxContains); + // Generate a container representation from the metadata const data = metadata.quads(); metadata.addQuad(DC.terms.namespace, VANN.terms.preferredNamespacePrefix, 'dc'); @@ -159,13 +176,32 @@ export class DataAccessorBasedStore implements ResourceStore { if (this.isRootStorage(metadata)) { throw new MethodNotAllowedHttpError('Cannot delete a root storage container.'); } + if (this.auxiliaryStrategy.isAuxiliaryIdentifier(identifier) && this.auxiliaryStrategy.isRootRequired(identifier)) { + const associatedIdentifier = this.auxiliaryStrategy.getAssociatedIdentifier(identifier); + const parentMetadata = await this.accessor.getMetadata(associatedIdentifier); + if (this.isRootStorage(parentMetadata)) { + throw new MethodNotAllowedHttpError(`Cannot delete ${identifier.path} from a root storage container.`); + } + } + // Solid, §5.4: "When a DELETE request is made to a container, the server MUST delete the container // if it contains no resources. If the container contains resources, // the server MUST respond with the 409 status code and response body describing the error." // https://solid.github.io/specification/protocol#deleting-resources - if (metadata.getAll(LDP.contains).length > 0) { - throw new ConflictHttpError('Can only delete empty containers.'); + if (isContainerIdentifier(identifier)) { + // Auxiliary resources are not counted when deleting a container since they will also be deleted + const auxContains = this.getContainedAuxiliaryResources(metadata); + if (metadata.getAll(LDP.contains).length > auxContains.length) { + throw new ConflictHttpError('Can only delete empty containers.'); + } } + // Solid, §5.4: "When a contained resource is deleted, the server MUST also delete the associated auxiliary + // resources" + // https://solid.github.io/specification/protocol#deleting-resources + if (!this.auxiliaryStrategy.isAuxiliaryIdentifier(identifier)) { + await this.safelyDeleteAuxiliaryResources(this.auxiliaryStrategy.getAuxiliaryIdentifiers(identifier)); + } + return this.accessor.deleteResource(identifier); } @@ -237,10 +273,16 @@ export class DataAccessorBasedStore implements ResourceStore { metadata.identifier = DataFactory.namedNode(identifier.path); metadata.addQuads(generateResourceQuads(metadata.identifier, isContainer)); + // Validate container data if (isContainer) { await this.handleContainerData(representation); } + // Validate auxiliary data + if (this.auxiliaryStrategy.isAuxiliaryIdentifier(identifier)) { + await this.auxiliaryStrategy.validate(representation); + } + // Root container should not have a parent container // Solid, §5.3: "Servers MUST create intermediate containers and include corresponding containment triples // in container representations derived from the URI path component of PUT and PATCH requests." @@ -326,6 +368,13 @@ export class DataAccessorBasedStore implements ResourceStore { let newID: ResourceIdentifier = this.createURI(container, isContainer, slug); + // Solid, §5.3: "When a POST method request with the Slug header targets an auxiliary resource, + // the server MUST respond with the 403 status code and response body describing the error." + // https://solid.github.io/specification/protocol#writing-resources + if (this.auxiliaryStrategy.isAuxiliaryIdentifier(newID)) { + throw new ForbiddenHttpError('Slug bodies that would result in an auxiliary resource are forbidden'); + } + // Make sure we don't already have a resource with this exact name (or with differing trailing slash) const withSlash = ensureTrailingSlash(newID.path); const withoutSlash = trimTrailingSlashes(newID.path); @@ -366,6 +415,31 @@ export class DataAccessorBasedStore implements ResourceStore { return metadata.getAll(RDF.type).some((term): boolean => term.value === PIM.Storage); } + /** + * Extracts the identifiers of all auxiliary resources contained within the given metadata. + */ + protected getContainedAuxiliaryResources(metadata: RepresentationMetadata): NamedNode[] { + return metadata.getAll(LDP.terms.contains).filter((object): boolean => + this.auxiliaryStrategy.isAuxiliaryIdentifier({ path: object.value })) as NamedNode[]; + } + + /** + * Deletes the given array of auxiliary identifiers. + * Does not throw an error if something goes wrong. + */ + protected async safelyDeleteAuxiliaryResources(identifiers: ResourceIdentifier[]): Promise { + return Promise.all(identifiers.map(async(identifier): Promise => { + try { + await this.accessor.deleteResource(identifier); + } catch (error: unknown) { + if (!NotFoundHttpError.isInstance(error)) { + const errorMsg = isNativeError(error) ? error.message : error; + this.logger.error(`Problem deleting auxiliary resource ${identifier.path}: ${errorMsg}`); + } + } + })); + } + /** * Create containers starting from the root until the given identifier corresponds to an existing container. * Will throw errors if the identifier of the last existing "container" corresponds to an existing document. diff --git a/src/storage/ResourceStore.ts b/src/storage/ResourceStore.ts index a9e1d657d..e35b0bd91 100644 --- a/src/storage/ResourceStore.ts +++ b/src/storage/ResourceStore.ts @@ -11,6 +11,9 @@ import type { Conditions } from './Conditions'; * dedicated method needs to be called. A fifth method enables the optimization * of partial updates with PATCH. It is up to the implementer of the interface to * (not) make an implementation atomic. + * + * ResourceStores are also responsible for taking auxiliary resources into account + * should those be relevant to the store. */ export interface ResourceStore { /** diff --git a/test/integration/LockingResourceStore.test.ts b/test/integration/LockingResourceStore.test.ts index abc068169..e24bf6ba5 100644 --- a/test/integration/LockingResourceStore.test.ts +++ b/test/integration/LockingResourceStore.test.ts @@ -38,6 +38,7 @@ describe('A LockingResourceStore', (): void => { source = new DataAccessorBasedStore( new InMemoryDataAccessor(base), new SingleRootIdentifierStrategy(base), + strategy, ); // Initialize store diff --git a/test/unit/storage/DataAccessorBasedStore.test.ts b/test/unit/storage/DataAccessorBasedStore.test.ts index 4fcb51412..b0b40f2af 100644 --- a/test/unit/storage/DataAccessorBasedStore.test.ts +++ b/test/unit/storage/DataAccessorBasedStore.test.ts @@ -3,6 +3,8 @@ import type { Readable } from 'stream'; import arrayifyStream from 'arrayify-stream'; import type { Quad } from 'n3'; import { DataFactory } from 'n3'; +import type { AuxiliaryStrategy } from '../../../src/ldp/auxiliary/AuxiliaryStrategy'; +import { BasicRepresentation } from '../../../src/ldp/representation/BasicRepresentation'; import type { Representation } from '../../../src/ldp/representation/Representation'; import { RepresentationMetadata } from '../../../src/ldp/representation/RepresentationMetadata'; import type { ResourceIdentifier } from '../../../src/ldp/representation/ResourceIdentifier'; @@ -69,11 +71,50 @@ class SimpleDataAccessor implements DataAccessor { } } +class SimpleSuffixStrategy implements AuxiliaryStrategy { + private readonly suffix: string; + + public constructor(suffix: string) { + this.suffix = suffix; + } + + public getAuxiliaryIdentifier(identifier: ResourceIdentifier): ResourceIdentifier { + return { path: `${identifier.path}${this.suffix}` }; + } + + public getAuxiliaryIdentifiers(identifier: ResourceIdentifier): ResourceIdentifier[] { + return [ this.getAuxiliaryIdentifier(identifier) ]; + } + + public isAuxiliaryIdentifier(identifier: ResourceIdentifier): boolean { + return identifier.path.endsWith(this.suffix); + } + + public getAssociatedIdentifier(identifier: ResourceIdentifier): ResourceIdentifier { + return { path: identifier.path.slice(0, -this.suffix.length) }; + } + + public isRootRequired(): boolean { + return false; + } + + public async addMetadata(metadata: RepresentationMetadata): Promise { + const identifier = { path: metadata.identifier.value }; + // Random triple to test on + metadata.add(identifier.path, this.getAuxiliaryIdentifier(identifier).path); + } + + public async validate(): Promise { + // Always validates + } +} + describe('A DataAccessorBasedStore', (): void => { let store: DataAccessorBasedStore; let accessor: SimpleDataAccessor; const root = 'http://test.com/'; const identifierStrategy = new SingleRootIdentifierStrategy(root); + let auxStrategy: AuxiliaryStrategy; let containerMetadata: RepresentationMetadata; let representation: Representation; const resourceData = 'text'; @@ -81,7 +122,8 @@ describe('A DataAccessorBasedStore', (): void => { beforeEach(async(): Promise => { accessor = new SimpleDataAccessor(); - store = new DataAccessorBasedStore(accessor, identifierStrategy); + auxStrategy = new SimpleSuffixStrategy('.dummy'); + store = new DataAccessorBasedStore(accessor, identifierStrategy, auxStrategy); containerMetadata = new RepresentationMetadata( { [RDF.type]: [ @@ -110,22 +152,41 @@ describe('A DataAccessorBasedStore', (): void => { it('will return the stored representation for resources.', async(): Promise => { const resourceID = { path: `${root}resource` }; + representation.metadata.identifier = DataFactory.namedNode(resourceID.path); accessor.data[resourceID.path] = representation; const result = await store.getRepresentation(resourceID); expect(result).toMatchObject({ binary: true }); expect(await arrayifyStream(result.data)).toEqual([ resourceData ]); expect(result.metadata.contentType).toEqual('text/plain'); + expect(result.metadata.get(resourceID.path)?.value).toBe(auxStrategy.getAuxiliaryIdentifier(resourceID).path); }); it('will return a data stream that matches the metadata for containers.', async(): Promise => { const resourceID = { path: `${root}container/` }; containerMetadata.identifier = namedNode(resourceID.path); accessor.data[resourceID.path] = { metadata: containerMetadata } as Representation; - const metaQuads = containerMetadata.quads(); + const metaMirror = new RepresentationMetadata(containerMetadata); + await auxStrategy.addMetadata(metaMirror); const result = await store.getRepresentation(resourceID); expect(result).toMatchObject({ binary: false }); - expect(await arrayifyStream(result.data)).toBeRdfIsomorphic(metaQuads); + expect(await arrayifyStream(result.data)).toBeRdfIsomorphic(metaMirror.quads()); expect(result.metadata.contentType).toEqual(INTERNAL_QUADS); + expect(result.metadata.get(resourceID.path)?.value).toBe(auxStrategy.getAuxiliaryIdentifier(resourceID).path); + }); + + it('will remove containment triples referencing auxiliary resources.', async(): Promise => { + const resourceID = { path: `${root}container/` }; + containerMetadata.identifier = namedNode(resourceID.path); + containerMetadata.add(LDP.terms.contains, [ + DataFactory.namedNode(`${root}container/.dummy`), + DataFactory.namedNode(`${root}container/resource`), + DataFactory.namedNode(`${root}container/resource.dummy`), + ]); + accessor.data[resourceID.path] = { metadata: containerMetadata } as Representation; + const result = await store.getRepresentation(resourceID); + const contains = result.metadata.getAll(LDP.terms.contains); + expect(contains).toHaveLength(1); + expect(contains[0].value).toEqual(`${root}container/resource`); }); }); @@ -229,6 +290,15 @@ describe('A DataAccessorBasedStore', (): void => { path: expect.stringMatching(new RegExp(`^${root}[^/]+/$`, 'u')), }); }); + + it('errors if the slug would cause an auxiliary resource URI to be generated.', async(): Promise => { + const resourceID = { path: root }; + representation.metadata.removeAll(RDF.type); + representation.metadata.add(HTTP.slug, 'test.dummy'); + const result = store.addResource(resourceID, representation); + await expect(result).rejects.toThrow(ForbiddenHttpError); + await expect(result).rejects.toThrow('Slug bodies that would result in an auxiliary resource are forbidden'); + }); }); describe('setting a Representation', (): void => { @@ -287,6 +357,12 @@ describe('A DataAccessorBasedStore', (): void => { await expect(store.setRepresentation(resourceID, representation)).rejects.toThrow(BadRequestHttpError); }); + it('errors when trying to create an auxiliary resource with invalid data.', async(): Promise => { + const resourceID = { path: `${root}resource.dummy` }; + auxStrategy.validate = jest.fn().mockRejectedValue(new Error('bad data!')); + await expect(store.setRepresentation(resourceID, representation)).rejects.toThrow('bad data!'); + }); + it('can write resources.', async(): Promise => { const resourceID = { path: `${root}resource` }; await expect(store.setRepresentation(resourceID, representation)).resolves.toBeUndefined(); @@ -390,16 +466,29 @@ describe('A DataAccessorBasedStore', (): void => { it('will error when deleting a root storage container.', async(): Promise => { representation.metadata.add(RDF.type, PIM.terms.Storage); - accessor.data[`${root}container`] = representation; - const result = store.deleteResource({ path: `${root}container` }); + accessor.data[`${root}container/`] = representation; + const result = store.deleteResource({ path: `${root}container/` }); await expect(result).rejects.toThrow(MethodNotAllowedHttpError); await expect(result).rejects.toThrow('Cannot delete a root storage container.'); }); + it('will error when deleting an auxiliary of a root storage container if not allowed.', async(): Promise => { + const storageMetadata = new RepresentationMetadata(representation.metadata); + storageMetadata.add(RDF.type, PIM.terms.Storage); + accessor.data[`${root}container/`] = new BasicRepresentation(representation.data, storageMetadata); + accessor.data[`${root}container/.dummy`] = representation; + auxStrategy.isRootRequired = jest.fn().mockReturnValue(true); + const result = store.deleteResource({ path: `${root}container/.dummy` }); + await expect(result).rejects.toThrow(MethodNotAllowedHttpError); + await expect(result).rejects.toThrow( + 'Cannot delete http://test.com/container/.dummy from a root storage container.', + ); + }); + it('will error when deleting non-empty containers.', async(): Promise => { - accessor.data[`${root}container`] = representation; - accessor.data[`${root}container`].metadata.add(LDP.contains, DataFactory.namedNode(`${root}otherThing`)); - const result = store.deleteResource({ path: `${root}container` }); + accessor.data[`${root}container/`] = representation; + accessor.data[`${root}container/`].metadata.add(LDP.contains, DataFactory.namedNode(`${root}otherThing`)); + const result = store.deleteResource({ path: `${root}container/` }); await expect(result).rejects.toThrow(ConflictHttpError); await expect(result).rejects.toThrow('Can only delete empty containers.'); }); @@ -409,5 +498,64 @@ describe('A DataAccessorBasedStore', (): void => { await expect(store.deleteResource({ path: `${root}resource` })).resolves.toBeUndefined(); expect(accessor.data[`${root}resource`]).toBeUndefined(); }); + + it('will delete a root storage auxiliary resource of a non-root container.', async(): Promise => { + const storageMetadata = new RepresentationMetadata(representation.metadata); + accessor.data[`${root}container/`] = new BasicRepresentation(representation.data, storageMetadata); + accessor.data[`${root}container/.dummy`] = representation; + auxStrategy.isRootRequired = jest.fn().mockReturnValue(true); + await expect(store.deleteResource({ path: `${root}container/.dummy` })).resolves.toBeUndefined(); + expect(accessor.data[`${root}container/.dummy`]).toBeUndefined(); + }); + + it('will delete related auxiliary resources.', async(): Promise => { + accessor.data[`${root}container/`] = representation; + accessor.data[`${root}container/.dummy`] = representation; + await expect(store.deleteResource({ path: `${root}container/` })).resolves.toBeUndefined(); + expect(accessor.data[`${root}container/`]).toBeUndefined(); + expect(accessor.data[`${root}container/.dummy`]).toBeUndefined(); + }); + + it('will still delete a resource if deleting auxiliary resources causes errors.', async(): Promise => { + accessor.data[`${root}resource`] = representation; + accessor.data[`${root}resource.dummy`] = representation; + const deleteFn = accessor.deleteResource; + accessor.deleteResource = jest.fn(async(identifier: ResourceIdentifier): Promise => { + if (auxStrategy.isAuxiliaryIdentifier(identifier)) { + throw new Error('auxiliary error!'); + } + await deleteFn.call(accessor, identifier); + }); + const { logger } = store as any; + logger.error = jest.fn(); + await expect(store.deleteResource({ path: `${root}resource` })).resolves.toBeUndefined(); + expect(accessor.data[`${root}resource`]).toBeUndefined(); + expect(accessor.data[`${root}resource.dummy`]).not.toBeUndefined(); + expect(logger.error).toHaveBeenCalledTimes(1); + expect(logger.error).toHaveBeenLastCalledWith( + 'Problem deleting auxiliary resource http://test.com/resource.dummy: auxiliary error!', + ); + }); + + it('can also handle auxiliary deletion to throw non-native errors.', async(): Promise => { + accessor.data[`${root}resource`] = representation; + accessor.data[`${root}resource.dummy`] = representation; + const deleteFn = accessor.deleteResource; + accessor.deleteResource = jest.fn(async(identifier: ResourceIdentifier): Promise => { + if (auxStrategy.isAuxiliaryIdentifier(identifier)) { + throw 'auxiliary error!'; + } + await deleteFn.call(accessor, identifier); + }); + const { logger } = store as any; + logger.error = jest.fn(); + await expect(store.deleteResource({ path: `${root}resource` })).resolves.toBeUndefined(); + expect(accessor.data[`${root}resource`]).toBeUndefined(); + expect(accessor.data[`${root}resource.dummy`]).not.toBeUndefined(); + expect(logger.error).toHaveBeenCalledTimes(1); + expect(logger.error).toHaveBeenLastCalledWith( + 'Problem deleting auxiliary resource http://test.com/resource.dummy: auxiliary error!', + ); + }); }); });