From 8491300f427bb066c55ff04053a549b8bb3af42b Mon Sep 17 00:00:00 2001 From: Joachim Van Herwegen Date: Fri, 7 May 2021 10:31:21 +0200 Subject: [PATCH] feat: Pass ResourceStore as param to PatchHandler This way the chain of ResourceStores is a bit easier to configure. This commit also updates the SparqlUpdatePatchHandler to keep the metadata and content-type of the resource that is being modified. --- config/presets/storage-wrapper.json | 4 +- src/storage/PatchingStore.ts | 2 +- src/storage/patch/PatchHandler.ts | 14 ++- src/storage/patch/SparqlUpdatePatchHandler.ts | 76 +++++++++++----- test/unit/storage/PatchingStore.test.ts | 2 +- .../patch/SparqlUpdatePatchHandler.test.ts | 86 +++++++++++++++---- 6 files changed, 139 insertions(+), 45 deletions(-) diff --git a/config/presets/storage-wrapper.json b/config/presets/storage-wrapper.json index e9735e8d0..618d45c2f 100644 --- a/config/presets/storage-wrapper.json +++ b/config/presets/storage-wrapper.json @@ -42,8 +42,8 @@ "patcher": { "@id": "urn:solid-server:default:PatchHandler", "@type": "SparqlUpdatePatchHandler", - "source": { - "@id": "urn:solid-server:default:ResourceStore_ToTurtle" + "converter": { + "@id": "urn:solid-server:default:RepresentationConverter" } } }, diff --git a/src/storage/PatchingStore.ts b/src/storage/PatchingStore.ts index 94d5a3ef8..d9770865b 100644 --- a/src/storage/PatchingStore.ts +++ b/src/storage/PatchingStore.ts @@ -23,7 +23,7 @@ export class PatchingStore extends Pass try { return await this.source.modifyResource(identifier, patch, conditions); } catch { - return this.patcher.handleSafe({ identifier, patch }); + return this.patcher.handleSafe({ source: this.source, identifier, patch }); } } } diff --git a/src/storage/patch/PatchHandler.ts b/src/storage/patch/PatchHandler.ts index a96b1ae45..7fe155d18 100644 --- a/src/storage/patch/PatchHandler.ts +++ b/src/storage/patch/PatchHandler.ts @@ -1,6 +1,16 @@ import type { Patch } from '../../ldp/http/Patch'; import type { ResourceIdentifier } from '../../ldp/representation/ResourceIdentifier'; import { AsyncHandler } from '../../util/handlers/AsyncHandler'; +import type { ResourceStore } from '../ResourceStore'; -export abstract class PatchHandler - extends AsyncHandler<{identifier: ResourceIdentifier; patch: Patch}, ResourceIdentifier[]> {} +export type PatchHandlerArgs = { + source: T; + identifier: ResourceIdentifier; + patch: Patch; +}; + +/** + * Executes the given Patch. + */ +export abstract class PatchHandler + extends AsyncHandler, ResourceIdentifier[]> {} diff --git a/src/storage/patch/SparqlUpdatePatchHandler.ts b/src/storage/patch/SparqlUpdatePatchHandler.ts index 9ecb38575..de07bcf5b 100644 --- a/src/storage/patch/SparqlUpdatePatchHandler.ts +++ b/src/storage/patch/SparqlUpdatePatchHandler.ts @@ -4,44 +4,56 @@ import { Store } from 'n3'; import type { BaseQuad } from 'rdf-js'; import { someTerms } from 'rdf-terms'; import { Algebra } from 'sparqlalgebrajs'; +import type { Patch } from '../../ldp/http/Patch'; import type { SparqlUpdatePatch } from '../../ldp/http/SparqlUpdatePatch'; import { BasicRepresentation } from '../../ldp/representation/BasicRepresentation'; +import { RepresentationMetadata } from '../../ldp/representation/RepresentationMetadata'; import type { ResourceIdentifier } from '../../ldp/representation/ResourceIdentifier'; import { getLoggerFor } from '../../logging/LogUtil'; import { INTERNAL_QUADS } from '../../util/ContentTypes'; import { NotFoundHttpError } from '../../util/errors/NotFoundHttpError'; import { NotImplementedHttpError } from '../../util/errors/NotImplementedHttpError'; +import type { RepresentationConverter } from '../conversion/RepresentationConverter'; import type { ResourceStore } from '../ResourceStore'; +import type { PatchHandlerArgs } from './PatchHandler'; import { PatchHandler } from './PatchHandler'; /** * PatchHandler that supports specific types of SPARQL updates. * Currently all DELETE/INSERT types are supported that have empty where bodies and no variables. + * + * Will try to keep the content-type and metadata of the original resource intact. + * In case this PATCH would create a new resource, it will have content-type `defaultType`. */ export class SparqlUpdatePatchHandler extends PatchHandler { protected readonly logger = getLoggerFor(this); - private readonly source: ResourceStore; + private readonly converter: RepresentationConverter; + private readonly defaultType: string; - public constructor(source: ResourceStore) { + public constructor(converter: RepresentationConverter, defaultType = 'text/turtle') { super(); - this.source = source; + this.converter = converter; + this.defaultType = defaultType; } - public async canHandle(input: {identifier: ResourceIdentifier; patch: SparqlUpdatePatch}): Promise { - if (typeof input.patch.algebra !== 'object') { - throw new NotImplementedHttpError('Only SPARQL update patch requests are supported'); + public async canHandle({ patch }: PatchHandlerArgs): Promise { + if (!this.isSparqlUpdate(patch)) { + throw new NotImplementedHttpError('Only SPARQL update patches are supported'); } } - public async handle(input: {identifier: ResourceIdentifier; patch: SparqlUpdatePatch}): - Promise { + public async handle(input: PatchHandlerArgs): Promise { // Verify the patch - const { identifier, patch } = input; - const op = patch.algebra; + const { source, identifier, patch } = input; + const op = (patch as SparqlUpdatePatch).algebra; this.validateUpdate(op); - return this.applyPatch(identifier, op); + return this.applyPatch(source, identifier, op); + } + + private isSparqlUpdate(patch: Patch): patch is SparqlUpdatePatch { + return typeof (patch as SparqlUpdatePatch).algebra === 'object'; } private isDeleteInsert(op: Algebra.Operation): op is Algebra.DeleteInsert { @@ -108,33 +120,51 @@ export class SparqlUpdatePatchHandler extends PatchHandler { /** * Apply the given algebra operation to the given identifier. */ - private async applyPatch(identifier: ResourceIdentifier, op: Algebra.Operation): Promise { - const store = new Store(); + private async applyPatch(source: ResourceStore, identifier: ResourceIdentifier, op: Algebra.Operation): + Promise { + // These are used to make sure we keep the original content-type and metadata + let contentType: string; + let metadata: RepresentationMetadata; + + const result = new Store(); try { // Read the quads of the current representation - const quads = await this.source.getRepresentation(identifier, - { type: { [INTERNAL_QUADS]: 1 }}); - const importEmitter = store.import(quads.data); + const representation = await source.getRepresentation(identifier, {}); + contentType = representation.metadata.contentType ?? this.defaultType; + const preferences = { type: { [INTERNAL_QUADS]: 1 }}; + const quads = await this.converter.handleSafe({ representation, identifier, preferences }); + // eslint-disable-next-line prefer-destructuring + metadata = quads.metadata; + + const importEmitter = result.import(quads.data); await new Promise((resolve, reject): void => { importEmitter.on('end', resolve); importEmitter.on('error', reject); }); - this.logger.debug(`${store.size} quads in ${identifier.path}.`); + this.logger.debug(`${result.size} quads in ${identifier.path}.`); } catch (error: unknown) { - // Solid, §5.1: "Clients who want to assign a URI to a resource, MUST use PUT and PATCH requests." + // Solid, §5.1: "When a successful PUT or PATCH request creates a resource, + // the server MUST use the effective request URI to assign the URI to that resource." // https://solid.github.io/specification/protocol#resource-type-heuristics if (!NotFoundHttpError.isInstance(error)) { throw error; } + contentType = this.defaultType; + metadata = new RepresentationMetadata(identifier, INTERNAL_QUADS); this.logger.debug(`Patching new resource ${identifier.path}.`); } - this.applyOperation(store, op); - this.logger.debug(`${store.size} quads will be stored to ${identifier.path}.`); + this.applyOperation(result, op); + this.logger.debug(`${result.size} quads will be stored to ${identifier.path}.`); - // Write the result - const patched = new BasicRepresentation(store.match() as Readable, INTERNAL_QUADS); - return this.source.setRepresentation(identifier, patched); + // Convert back to the original type and write the result + const patched = new BasicRepresentation(result.match() as Readable, metadata); + const converted = await this.converter.handleSafe({ + representation: patched, + identifier, + preferences: { type: { [contentType]: 1 }}, + }); + return source.setRepresentation(identifier, converted); } /** diff --git a/test/unit/storage/PatchingStore.test.ts b/test/unit/storage/PatchingStore.test.ts index 613a3bfee..f77a8e62e 100644 --- a/test/unit/storage/PatchingStore.test.ts +++ b/test/unit/storage/PatchingStore.test.ts @@ -35,6 +35,6 @@ describe('A PatchingStore', (): void => { expect(source.modifyResource).toHaveBeenLastCalledWith({ path: 'modifyPath' }, {}, undefined); await expect((source.modifyResource as jest.Mock).mock.results[0].value).rejects.toThrow('dummy'); expect(handleSafeFn).toHaveBeenCalledTimes(1); - expect(handleSafeFn).toHaveBeenLastCalledWith({ identifier: { path: 'modifyPath' }, patch: {}}); + expect(handleSafeFn).toHaveBeenLastCalledWith({ source, identifier: { path: 'modifyPath' }, patch: {}}); }); }); diff --git a/test/unit/storage/patch/SparqlUpdatePatchHandler.test.ts b/test/unit/storage/patch/SparqlUpdatePatchHandler.test.ts index 13af2ad80..69c62150c 100644 --- a/test/unit/storage/patch/SparqlUpdatePatchHandler.test.ts +++ b/test/unit/storage/patch/SparqlUpdatePatchHandler.test.ts @@ -6,6 +6,8 @@ import { translate } from 'sparqlalgebrajs'; import type { SparqlUpdatePatch } from '../../../../src/ldp/http/SparqlUpdatePatch'; import { BasicRepresentation } from '../../../../src/ldp/representation/BasicRepresentation'; import { RepresentationMetadata } from '../../../../src/ldp/representation/RepresentationMetadata'; +import type { RepresentationConverterArgs, + RepresentationConverter } from '../../../../src/storage/conversion/RepresentationConverter'; import { SparqlUpdatePatchHandler } from '../../../../src/storage/patch/SparqlUpdatePatchHandler'; import type { ResourceStore } from '../../../../src/storage/ResourceStore'; import { INTERNAL_QUADS } from '../../../../src/util/ContentTypes'; @@ -13,9 +15,12 @@ import { NotFoundHttpError } from '../../../../src/util/errors/NotFoundHttpError import { NotImplementedHttpError } from '../../../../src/util/errors/NotImplementedHttpError'; describe('A SparqlUpdatePatchHandler', (): void => { + let converter: RepresentationConverter; let handler: SparqlUpdatePatchHandler; let source: ResourceStore; let startQuads: Quad[]; + const dummyType = 'internal/not-quads'; + const identifier = { path: 'http://test.com/foo' }; const fullfilledDataInsert = 'INSERT DATA { :s1 :p1 :o1 . :s2 :p2 :o2 . }'; beforeEach(async(): Promise => { @@ -29,43 +34,60 @@ describe('A SparqlUpdatePatchHandler', (): void => { namedNode('http://test.com/startO2'), ) ]; + converter = { + handleSafe: jest.fn(async({ representation, preferences }: RepresentationConverterArgs): Promise => + new BasicRepresentation(representation.data, Object.keys(preferences.type!)[0])), + } as unknown as RepresentationConverter; + source = { - getRepresentation: jest.fn(async(): Promise => new BasicRepresentation(startQuads, 'internal/quads', false)), + getRepresentation: jest.fn(async(): Promise => new BasicRepresentation(startQuads, dummyType)), setRepresentation: jest.fn(), modifyResource: jest.fn(async(): Promise => { throw new Error('noModify'); }), } as unknown as ResourceStore; - handler = new SparqlUpdatePatchHandler(source); + handler = new SparqlUpdatePatchHandler(converter, dummyType); }); async function basicChecks(quads: Quad[]): Promise { expect(source.getRepresentation).toHaveBeenCalledTimes(1); - expect(source.getRepresentation).toHaveBeenLastCalledWith( - { path: 'path' }, { type: { [INTERNAL_QUADS]: 1 }}, - ); + expect(source.getRepresentation).toHaveBeenLastCalledWith(identifier, { }); + expect(converter.handleSafe).toHaveBeenCalledTimes(2); + expect(converter.handleSafe).toHaveBeenCalledWith({ + representation: await (source.getRepresentation as jest.Mock).mock.results[0].value, + identifier, + preferences: { type: { [INTERNAL_QUADS]: 1 }}, + }); + expect(converter.handleSafe).toHaveBeenLastCalledWith({ + representation: expect.objectContaining({ binary: false, metadata: expect.any(RepresentationMetadata) }), + identifier, + preferences: { type: { [dummyType]: 1 }}, + }); + expect(source.setRepresentation).toHaveBeenCalledTimes(1); const setParams = (source.setRepresentation as jest.Mock).mock.calls[0]; - expect(setParams[0]).toEqual({ path: 'path' }); + expect(setParams[0]).toEqual(identifier); expect(setParams[1]).toEqual(expect.objectContaining({ - binary: false, + binary: true, metadata: expect.any(RepresentationMetadata), })); - expect(setParams[1].metadata.contentType).toEqual(INTERNAL_QUADS); + expect(setParams[1].metadata.contentType).toEqual(dummyType); await expect(arrayifyStream(setParams[1].data)).resolves.toBeRdfIsomorphic(quads); return true; } async function handle(query: string): Promise { const sparqlPrefix = 'prefix : \n'; - await handler.handle({ identifier: { path: 'path' }, - patch: { algebra: translate(sparqlPrefix.concat(query), { quads: true }) } as SparqlUpdatePatch }); + await handler.handle({ + source, + identifier, + patch: { algebra: translate(sparqlPrefix.concat(query), { quads: true }) } as SparqlUpdatePatch, + }); } it('only accepts SPARQL updates.', async(): Promise => { - const input = { identifier: { path: 'path' }, - patch: { algebra: {}} as SparqlUpdatePatch }; + const input = { source, identifier, patch: { algebra: {}} as SparqlUpdatePatch }; await expect(handler.canHandle(input)).resolves.toBeUndefined(); delete (input.patch as any).algebra; await expect(handler.canHandle(input)).rejects.toThrow(NotImplementedHttpError); @@ -181,13 +203,45 @@ describe('A SparqlUpdatePatchHandler', (): void => { it('creates a new resource if it does not exist yet.', async(): Promise => { startQuads = []; - source.getRepresentation = jest.fn((): any => { - throw new NotFoundHttpError(); - }); + (source.getRepresentation as jest.Mock).mockRejectedValueOnce(new NotFoundHttpError()); const query = 'INSERT DATA { . }'; await handle(query); + + expect(converter.handleSafe).toHaveBeenCalledTimes(1); + expect(converter.handleSafe).toHaveBeenLastCalledWith({ + representation: expect.objectContaining({ binary: false, metadata: expect.any(RepresentationMetadata) }), + identifier, + preferences: { type: { [dummyType]: 1 }}, + }); + + const quads = + [ quad(namedNode('http://test.com/s1'), namedNode('http://test.com/p1'), namedNode('http://test.com/o1')) ]; + expect(source.setRepresentation).toHaveBeenCalledTimes(1); + const setParams = (source.setRepresentation as jest.Mock).mock.calls[0]; + expect(setParams[1].metadata.contentType).toEqual(dummyType); + await expect(arrayifyStream(setParams[1].data)).resolves.toBeRdfIsomorphic(quads); + }); + + it('can handle representations without content-type.', async(): Promise => { + (source.getRepresentation as jest.Mock).mockResolvedValueOnce( + new BasicRepresentation(startQuads, new RepresentationMetadata()), + ); + await handle(fullfilledDataInsert); expect(await basicChecks(startQuads.concat( - [ quad(namedNode('http://test.com/s1'), namedNode('http://test.com/p1'), namedNode('http://test.com/o1')) ], + [ quad(namedNode('http://test.com/s1'), namedNode('http://test.com/p1'), namedNode('http://test.com/o1')), + quad(namedNode('http://test.com/s2'), namedNode('http://test.com/p2'), namedNode('http://test.com/o2')) ], ))).toBe(true); }); + + it('defaults to text/turtle if no default type was set.', async(): Promise => { + handler = new SparqlUpdatePatchHandler(converter); + startQuads = []; + (source.getRepresentation as jest.Mock).mockRejectedValueOnce(new NotFoundHttpError()); + const query = 'INSERT DATA { . }'; + await handle(query); + + expect(source.setRepresentation).toHaveBeenCalledTimes(1); + const setParams = (source.setRepresentation as jest.Mock).mock.calls[0]; + expect(setParams[1].metadata.contentType).toEqual('text/turtle'); + }); });