From 1260c5c14e26e70dc1dafa211ab35c7981c4bd22 Mon Sep 17 00:00:00 2001 From: Joachim Van Herwegen Date: Tue, 27 Oct 2020 12:16:44 +0100 Subject: [PATCH] feat: Store status, body and metadata in ResponseDescription --- index.ts | 27 ++++++---- src/ldp/AuthenticatedLdpHandler.ts | 2 +- src/ldp/http/BasicResponseWriter.ts | 28 +++++----- src/ldp/http/ErrorResponseWriter.ts | 2 +- src/ldp/http/ResponseWriter.ts | 2 +- .../response/CreatedResponseDescription.ts | 15 ++++++ .../http/response/OkResponseDescription.ts | 16 ++++++ .../http/response/ResetResponseDescription.ts | 10 ++++ src/ldp/http/response/ResponseDescription.ts | 22 ++++++++ src/ldp/operations/DeleteOperationHandler.ts | 5 +- src/ldp/operations/GetOperationHandler.ts | 5 +- src/ldp/operations/HeadOperationHandler.ts | 11 ++-- src/ldp/operations/OperationHandler.ts | 2 +- src/ldp/operations/PatchOperationHandler.ts | 5 +- src/ldp/operations/PostOperationHandler.ts | 5 +- src/ldp/operations/PutOperationHandler.ts | 5 +- src/ldp/operations/ResponseDescription.ts | 10 ---- src/util/UriConstants.ts | 1 + .../AuthenticatedLdpHandler.test.ts | 18 +++---- test/integration/Authorization.test.ts | 4 +- test/integration/FullConfig.acl.test.ts | 6 +-- test/integration/FullConfig.noAuth.test.ts | 33 +++++------- .../unit/ldp/http/BasicResponseWriter.test.ts | 53 +++++-------------- .../unit/ldp/http/ErrorResponseWriter.test.ts | 5 +- .../operations/DeleteOperationHandler.test.ts | 11 ++-- .../operations/GetOperationHandler.test.ts | 12 +++-- .../operations/HeadOperationHandler.test.ts | 29 ++++++---- .../operations/PatchOperationHandler.test.ts | 8 +-- .../operations/PostOperationHandler.test.ts | 11 ++-- .../operations/PutOperationHandler.test.ts | 8 +-- test/util/TestHelpers.ts | 22 ++------ 31 files changed, 209 insertions(+), 184 deletions(-) create mode 100644 src/ldp/http/response/CreatedResponseDescription.ts create mode 100644 src/ldp/http/response/OkResponseDescription.ts create mode 100644 src/ldp/http/response/ResetResponseDescription.ts create mode 100644 src/ldp/http/response/ResponseDescription.ts delete mode 100644 src/ldp/operations/ResponseDescription.ts diff --git a/index.ts b/index.ts index 35c80c9e9..abb6ace36 100644 --- a/index.ts +++ b/index.ts @@ -22,6 +22,12 @@ export * from './src/ldp/http/metadata/MetadataExtractor'; export * from './src/ldp/http/metadata/MetadataParser'; export * from './src/ldp/http/metadata/SlugParser'; +// LDP/HTTP/Response +export * from './src/ldp/http/response/CreatedResponseDescription'; +export * from './src/ldp/http/response/OkResponseDescription'; +export * from './src/ldp/http/response/ResetResponseDescription'; +export * from './src/ldp/http/response/ResponseDescription'; + // LDP/HTTP export * from './src/ldp/http/AcceptPreferenceParser'; export * from './src/ldp/http/BasicRequestParser'; @@ -38,16 +44,6 @@ export * from './src/ldp/http/SparqlUpdateBodyParser'; export * from './src/ldp/http/SparqlUpdatePatch'; export * from './src/ldp/http/TargetExtractor'; -// Logging -export * from './src/logging/LazyLogger'; -export * from './src/logging/LazyLoggerFactory'; -export * from './src/logging/Logger'; -export * from './src/logging/LoggerFactory'; -export * from './src/logging/LogLevel'; -export * from './src/logging/LogUtil'; -export * from './src/logging/VoidLoggerFactory'; -export * from './src/logging/WinstonLoggerFactory'; - // LDP/Operations export * from './src/ldp/operations/DeleteOperationHandler'; export * from './src/ldp/operations/GetOperationHandler'; @@ -57,7 +53,6 @@ export * from './src/ldp/operations/OperationHandler'; export * from './src/ldp/operations/PatchOperationHandler'; export * from './src/ldp/operations/PostOperationHandler'; export * from './src/ldp/operations/PutOperationHandler'; -export * from './src/ldp/operations/ResponseDescription'; // LDP/Permissions export * from './src/ldp/permissions/PermissionSet'; @@ -75,6 +70,16 @@ export * from './src/ldp/representation/ResourceIdentifier'; // LDP export * from './src/ldp/AuthenticatedLdpHandler'; +// Logging +export * from './src/logging/LazyLogger'; +export * from './src/logging/LazyLoggerFactory'; +export * from './src/logging/Logger'; +export * from './src/logging/LoggerFactory'; +export * from './src/logging/LogLevel'; +export * from './src/logging/LogUtil'; +export * from './src/logging/VoidLoggerFactory'; +export * from './src/logging/WinstonLoggerFactory'; + // Server export * from './src/server/ExpressHttpServer'; export * from './src/server/HttpHandler'; diff --git a/src/ldp/AuthenticatedLdpHandler.ts b/src/ldp/AuthenticatedLdpHandler.ts index a3adb9f65..cddbca4e7 100644 --- a/src/ldp/AuthenticatedLdpHandler.ts +++ b/src/ldp/AuthenticatedLdpHandler.ts @@ -5,10 +5,10 @@ import { HttpHandler } from '../server/HttpHandler'; import type { HttpRequest } from '../server/HttpRequest'; import type { HttpResponse } from '../server/HttpResponse'; import type { RequestParser } from './http/RequestParser'; +import type { ResponseDescription } from './http/response/ResponseDescription'; import type { ResponseWriter } from './http/ResponseWriter'; import type { Operation } from './operations/Operation'; import type { OperationHandler } from './operations/OperationHandler'; -import type { ResponseDescription } from './operations/ResponseDescription'; import type { PermissionSet } from './permissions/PermissionSet'; import type { PermissionsExtractor } from './permissions/PermissionsExtractor'; diff --git a/src/ldp/http/BasicResponseWriter.ts b/src/ldp/http/BasicResponseWriter.ts index 2870f95a0..06500ad23 100644 --- a/src/ldp/http/BasicResponseWriter.ts +++ b/src/ldp/http/BasicResponseWriter.ts @@ -1,36 +1,40 @@ import { getLoggerFor } from '../../logging/LogUtil'; import type { HttpResponse } from '../../server/HttpResponse'; +import { INTERNAL_QUADS } from '../../util/ContentTypes'; import { UnsupportedHttpError } from '../../util/errors/UnsupportedHttpError'; -import type { ResponseDescription } from '../operations/ResponseDescription'; +import { HTTP } from '../../util/UriConstants'; +import type { ResponseDescription } from './response/ResponseDescription'; import { ResponseWriter } from './ResponseWriter'; /** * Writes to an {@link HttpResponse} based on the incoming {@link ResponseDescription}. - * Still needs a way to write correct status codes for successful operations. */ export class BasicResponseWriter extends ResponseWriter { protected readonly logger = getLoggerFor(this); public async canHandle(input: { response: HttpResponse; result: ResponseDescription | Error }): Promise { - if ((input.result instanceof Error) || (input.result.body && !input.result.body.binary)) { - this.logger.warn('This writer can only write binary bodies'); - throw new UnsupportedHttpError('Only binary results are supported'); + if (input.result instanceof Error || input.result.metadata?.contentType === INTERNAL_QUADS) { + this.logger.warn('This writer only supports binary ResponseDescriptions'); + throw new UnsupportedHttpError('Only successful binary responses are supported'); } } public async handle(input: { response: HttpResponse; result: ResponseDescription }): Promise { - input.response.setHeader('location', input.result.identifier.path); - if (input.result.body) { - const contentType = input.result.body.metadata.contentType ?? 'text/plain'; + const location = input.result.metadata?.get(HTTP.location); + if (location) { + input.response.setHeader('location', location.value); + } + if (input.result.data) { + const contentType = input.result.metadata?.contentType ?? 'text/plain'; input.response.setHeader('content-type', contentType); } - input.response.writeHead(200); + input.response.writeHead(input.result.statusCode); - if (input.result.body) { - input.result.body.data.pipe(input.response); + if (input.result.data) { + input.result.data.pipe(input.response); } else { - // If there is an input body the response will end once the input stream ends + // If there is input data the response will end once the input stream ends input.response.end(); } } diff --git a/src/ldp/http/ErrorResponseWriter.ts b/src/ldp/http/ErrorResponseWriter.ts index f84e0b07a..43705ccb9 100644 --- a/src/ldp/http/ErrorResponseWriter.ts +++ b/src/ldp/http/ErrorResponseWriter.ts @@ -2,7 +2,7 @@ import { getLoggerFor } from '../../logging/LogUtil'; import type { HttpResponse } from '../../server/HttpResponse'; import { HttpError } from '../../util/errors/HttpError'; import { UnsupportedHttpError } from '../../util/errors/UnsupportedHttpError'; -import type { ResponseDescription } from '../operations/ResponseDescription'; +import type { ResponseDescription } from './response/ResponseDescription'; import { ResponseWriter } from './ResponseWriter'; /** diff --git a/src/ldp/http/ResponseWriter.ts b/src/ldp/http/ResponseWriter.ts index c0fd33496..ba7d722ab 100644 --- a/src/ldp/http/ResponseWriter.ts +++ b/src/ldp/http/ResponseWriter.ts @@ -1,6 +1,6 @@ import type { HttpResponse } from '../../server/HttpResponse'; import { AsyncHandler } from '../../util/AsyncHandler'; -import type { ResponseDescription } from '../operations/ResponseDescription'; +import type { ResponseDescription } from './response/ResponseDescription'; /** * Writes to the HttpResponse. diff --git a/src/ldp/http/response/CreatedResponseDescription.ts b/src/ldp/http/response/CreatedResponseDescription.ts new file mode 100644 index 000000000..229276d5c --- /dev/null +++ b/src/ldp/http/response/CreatedResponseDescription.ts @@ -0,0 +1,15 @@ +import { DataFactory } from 'n3'; +import { HTTP } from '../../../util/UriConstants'; +import { RepresentationMetadata } from '../../representation/RepresentationMetadata'; +import type { ResourceIdentifier } from '../../representation/ResourceIdentifier'; +import { ResponseDescription } from './ResponseDescription'; + +/** + * Corresponds to a 201 response, containing the relevant link metadata. + */ +export class CreatedResponseDescription extends ResponseDescription { + public constructor(location: ResourceIdentifier) { + const metadata = new RepresentationMetadata({ [HTTP.location]: DataFactory.namedNode(location.path) }); + super(201, metadata); + } +} diff --git a/src/ldp/http/response/OkResponseDescription.ts b/src/ldp/http/response/OkResponseDescription.ts new file mode 100644 index 000000000..d614721cc --- /dev/null +++ b/src/ldp/http/response/OkResponseDescription.ts @@ -0,0 +1,16 @@ +import type { Readable } from 'stream'; +import type { RepresentationMetadata } from '../../representation/RepresentationMetadata'; +import { ResponseDescription } from './ResponseDescription'; + +/** + * Corresponds to a 200 response, containing relevant metadata and potentially data. + */ +export class OkResponseDescription extends ResponseDescription { + /** + * @param metadata - Metadata concerning the response. + * @param data - Potential data. @ignored + */ + public constructor(metadata: RepresentationMetadata, data?: Readable) { + super(200, metadata, data); + } +} diff --git a/src/ldp/http/response/ResetResponseDescription.ts b/src/ldp/http/response/ResetResponseDescription.ts new file mode 100644 index 000000000..07535a29e --- /dev/null +++ b/src/ldp/http/response/ResetResponseDescription.ts @@ -0,0 +1,10 @@ +import { ResponseDescription } from './ResponseDescription'; + +/** + * Corresponds to a 205 response. + */ +export class ResetResponseDescription extends ResponseDescription { + public constructor() { + super(205); + } +} diff --git a/src/ldp/http/response/ResponseDescription.ts b/src/ldp/http/response/ResponseDescription.ts new file mode 100644 index 000000000..001a5f61a --- /dev/null +++ b/src/ldp/http/response/ResponseDescription.ts @@ -0,0 +1,22 @@ +import type { Readable } from 'stream'; +import type { RepresentationMetadata } from '../../representation/RepresentationMetadata'; + +/** + * The result of executing an operation. + */ +export class ResponseDescription { + public readonly statusCode: number; + public readonly metadata?: RepresentationMetadata; + public readonly data?: Readable; + + /** + * @param statusCode - Status code to return. + * @param metadata - Metadata corresponding to the response (and data potentially). + * @param data - Data that needs to be returned. @ignored + */ + public constructor(statusCode: number, metadata?: RepresentationMetadata, data?: Readable) { + this.statusCode = statusCode; + this.metadata = metadata; + this.data = data; + } +} diff --git a/src/ldp/operations/DeleteOperationHandler.ts b/src/ldp/operations/DeleteOperationHandler.ts index 2af3f6eca..f27020c1a 100644 --- a/src/ldp/operations/DeleteOperationHandler.ts +++ b/src/ldp/operations/DeleteOperationHandler.ts @@ -1,8 +1,9 @@ import type { ResourceStore } from '../../storage/ResourceStore'; import { UnsupportedHttpError } from '../../util/errors/UnsupportedHttpError'; +import { ResetResponseDescription } from '../http/response/ResetResponseDescription'; +import type { ResponseDescription } from '../http/response/ResponseDescription'; import type { Operation } from './Operation'; import { OperationHandler } from './OperationHandler'; -import type { ResponseDescription } from './ResponseDescription'; /** * Handles DELETE {@link Operation}s. @@ -24,6 +25,6 @@ export class DeleteOperationHandler extends OperationHandler { public async handle(input: Operation): Promise { await this.store.deleteResource(input.target); - return { identifier: input.target }; + return new ResetResponseDescription(); } } diff --git a/src/ldp/operations/GetOperationHandler.ts b/src/ldp/operations/GetOperationHandler.ts index 3f8913346..636977a26 100644 --- a/src/ldp/operations/GetOperationHandler.ts +++ b/src/ldp/operations/GetOperationHandler.ts @@ -1,8 +1,9 @@ import type { ResourceStore } from '../../storage/ResourceStore'; import { UnsupportedHttpError } from '../../util/errors/UnsupportedHttpError'; +import { OkResponseDescription } from '../http/response/OkResponseDescription'; +import type { ResponseDescription } from '../http/response/ResponseDescription'; import type { Operation } from './Operation'; import { OperationHandler } from './OperationHandler'; -import type { ResponseDescription } from './ResponseDescription'; /** * Handles GET {@link Operation}s. @@ -24,6 +25,6 @@ export class GetOperationHandler extends OperationHandler { public async handle(input: Operation): Promise { const body = await this.store.getRepresentation(input.target, input.preferences); - return { identifier: input.target, body }; + return new OkResponseDescription(body.metadata, body.data); } } diff --git a/src/ldp/operations/HeadOperationHandler.ts b/src/ldp/operations/HeadOperationHandler.ts index 3de4753b4..1af5722eb 100644 --- a/src/ldp/operations/HeadOperationHandler.ts +++ b/src/ldp/operations/HeadOperationHandler.ts @@ -1,9 +1,9 @@ -import { Readable } from 'stream'; import type { ResourceStore } from '../../storage/ResourceStore'; import { UnsupportedHttpError } from '../../util/errors/UnsupportedHttpError'; +import { OkResponseDescription } from '../http/response/OkResponseDescription'; +import type { ResponseDescription } from '../http/response/ResponseDescription'; import type { Operation } from './Operation'; import { OperationHandler } from './OperationHandler'; -import type { ResponseDescription } from './ResponseDescription'; /** * Handles HEAD {@link Operation}s. @@ -28,10 +28,7 @@ export class HeadOperationHandler extends OperationHandler { // Close the Readable as we will not return it. body.data.destroy(); - body.data = new Readable(); - body.data._read = function(): void { - body.data.push(null); - }; - return { identifier: input.target, body }; + + return new OkResponseDescription(body.metadata); } } diff --git a/src/ldp/operations/OperationHandler.ts b/src/ldp/operations/OperationHandler.ts index bd183216d..014336d70 100644 --- a/src/ldp/operations/OperationHandler.ts +++ b/src/ldp/operations/OperationHandler.ts @@ -1,6 +1,6 @@ import { AsyncHandler } from '../../util/AsyncHandler'; +import type { ResponseDescription } from '../http/response/ResponseDescription'; import type { Operation } from './Operation'; -import type { ResponseDescription } from './ResponseDescription'; /** * Handler for a specific operation type. diff --git a/src/ldp/operations/PatchOperationHandler.ts b/src/ldp/operations/PatchOperationHandler.ts index fa1f93e14..b46178479 100644 --- a/src/ldp/operations/PatchOperationHandler.ts +++ b/src/ldp/operations/PatchOperationHandler.ts @@ -1,9 +1,10 @@ import type { ResourceStore } from '../../storage/ResourceStore'; import { UnsupportedHttpError } from '../../util/errors/UnsupportedHttpError'; import type { Patch } from '../http/Patch'; +import { ResetResponseDescription } from '../http/response/ResetResponseDescription'; +import type { ResponseDescription } from '../http/response/ResponseDescription'; import type { Operation } from './Operation'; import { OperationHandler } from './OperationHandler'; -import type { ResponseDescription } from './ResponseDescription'; export class PatchOperationHandler extends OperationHandler { private readonly store: ResourceStore; @@ -21,6 +22,6 @@ export class PatchOperationHandler extends OperationHandler { public async handle(input: Operation): Promise { await this.store.modifyResource(input.target, input.body as Patch); - return { identifier: input.target }; + return new ResetResponseDescription(); } } diff --git a/src/ldp/operations/PostOperationHandler.ts b/src/ldp/operations/PostOperationHandler.ts index 768b71bfb..07f07200d 100644 --- a/src/ldp/operations/PostOperationHandler.ts +++ b/src/ldp/operations/PostOperationHandler.ts @@ -1,9 +1,10 @@ import { getLoggerFor } from '../../logging/LogUtil'; import type { ResourceStore } from '../../storage/ResourceStore'; import { UnsupportedHttpError } from '../../util/errors/UnsupportedHttpError'; +import { CreatedResponseDescription } from '../http/response/CreatedResponseDescription'; +import type { ResponseDescription } from '../http/response/ResponseDescription'; import type { Operation } from './Operation'; import { OperationHandler } from './OperationHandler'; -import type { ResponseDescription } from './ResponseDescription'; /** * Handles POST {@link Operation}s. @@ -31,6 +32,6 @@ export class PostOperationHandler extends OperationHandler { throw new UnsupportedHttpError('POST operations require a body'); } const identifier = await this.store.addResource(input.target, input.body); - return { identifier }; + return new CreatedResponseDescription(identifier); } } diff --git a/src/ldp/operations/PutOperationHandler.ts b/src/ldp/operations/PutOperationHandler.ts index 407d465eb..d530c98b5 100644 --- a/src/ldp/operations/PutOperationHandler.ts +++ b/src/ldp/operations/PutOperationHandler.ts @@ -1,9 +1,10 @@ import { getLoggerFor } from '../../logging/LogUtil'; import type { ResourceStore } from '../../storage/ResourceStore'; import { UnsupportedHttpError } from '../../util/errors/UnsupportedHttpError'; +import { ResetResponseDescription } from '../http/response/ResetResponseDescription'; +import type { ResponseDescription } from '../http/response/ResponseDescription'; import type { Operation } from './Operation'; import { OperationHandler } from './OperationHandler'; -import type { ResponseDescription } from './ResponseDescription'; /** * Handles PUT {@link Operation}s. @@ -31,6 +32,6 @@ export class PutOperationHandler extends OperationHandler { throw new UnsupportedHttpError('PUT operations require a body'); } await this.store.setRepresentation(input.target, input.body); - return { identifier: input.target }; + return new ResetResponseDescription(); } } diff --git a/src/ldp/operations/ResponseDescription.ts b/src/ldp/operations/ResponseDescription.ts deleted file mode 100644 index dad5cbe2b..000000000 --- a/src/ldp/operations/ResponseDescription.ts +++ /dev/null @@ -1,10 +0,0 @@ -import type { Representation } from '../representation/Representation'; -import type { ResourceIdentifier } from '../representation/ResourceIdentifier'; - -/** - * The result of executing an operation. - */ -export interface ResponseDescription { - identifier: ResourceIdentifier; - body?: Representation; -} diff --git a/src/util/UriConstants.ts b/src/util/UriConstants.ts index ee77a8fcc..861ba1e91 100644 --- a/src/util/UriConstants.ts +++ b/src/util/UriConstants.ts @@ -28,6 +28,7 @@ export const FOAF = { const HTTP_PREFIX = createNamespace('urn:solid:http:'); export const HTTP = { + location: HTTP_PREFIX('location'), slug: HTTP_PREFIX('slug'), }; diff --git a/test/integration/AuthenticatedLdpHandler.test.ts b/test/integration/AuthenticatedLdpHandler.test.ts index 9f8b02148..c61fbf1f0 100644 --- a/test/integration/AuthenticatedLdpHandler.test.ts +++ b/test/integration/AuthenticatedLdpHandler.test.ts @@ -20,7 +20,7 @@ describe('An integrated AuthenticatedLdpHandler', (): void => { { 'content-type': 'text/turtle', 'transfer-encoding': 'chunked' }, [ ' .' ], ); - expect(response.statusCode).toBe(200); + expect(response.statusCode).toBe(201); expect(response._getData()).toHaveLength(0); const id = response._getHeaders().location; expect(id).toContain(url.format(requestUrl)); @@ -38,13 +38,11 @@ describe('An integrated AuthenticatedLdpHandler', (): void => { expect(response._getData()).toContain( ' .', ); - expect(response._getHeaders().location).toBe(id); // DELETE response = await call(handler, requestUrl, 'DELETE', {}, []); - expect(response.statusCode).toBe(200); + expect(response.statusCode).toBe(205); expect(response._getData()).toHaveLength(0); - expect(response._getHeaders().location).toBe(url.format(requestUrl)); // GET response = await call( @@ -73,7 +71,7 @@ describe('An integrated AuthenticatedLdpHandler', (): void => { [ ' .', ' .' ], ); - expect(response.statusCode).toBe(200); + expect(response.statusCode).toBe(201); expect(response._getData()).toHaveLength(0); const id = response._getHeaders().location; expect(id).toContain(url.format(requestUrl)); @@ -90,9 +88,8 @@ describe('An integrated AuthenticatedLdpHandler', (): void => { 'WHERE {}', ], ); - expect(response.statusCode).toBe(200); + expect(response.statusCode).toBe(205); expect(response._getData()).toHaveLength(0); - expect(response._getHeaders().location).toBe(id); // GET requestUrl = new URL(id); @@ -107,7 +104,6 @@ describe('An integrated AuthenticatedLdpHandler', (): void => { expect(response._getBuffer().toString()).toContain( ' .', ); - expect(response._getHeaders().location).toBe(id); const parser = new Parser(); const triples = parser.parse(response._getBuffer().toString()); expect(triples).toBeRdfIsomorphic([ @@ -141,7 +137,7 @@ describe('An integrated AuthenticatedLdpHandler', (): void => { ' .', ], ); - expect(response.statusCode).toBe(200); + expect(response.statusCode).toBe(201); expect(response._getData()).toHaveLength(0); const id = response._getHeaders().location; expect(id).toContain(url.format(requestUrl)); @@ -155,9 +151,8 @@ describe('An integrated AuthenticatedLdpHandler', (): void => { { 'content-type': 'text/turtle', 'transfer-encoding': 'chunked' }, [ ' .' ], ); - expect(response.statusCode).toBe(200); + expect(response.statusCode).toBe(205); expect(response._getData()).toHaveLength(0); - expect(response._getHeaders().location).toBe(id); // GET requestUrl = new URL(id); @@ -169,7 +164,6 @@ describe('An integrated AuthenticatedLdpHandler', (): void => { [], ); expect(response.statusCode).toBe(200); - expect(response._getHeaders().location).toBe(id); const parser = new Parser(); const triples = parser.parse(response._getData()); expect(triples).toBeRdfIsomorphic([ diff --git a/test/integration/Authorization.test.ts b/test/integration/Authorization.test.ts index fca7a4c2f..480179a12 100644 --- a/test/integration/Authorization.test.ts +++ b/test/integration/Authorization.test.ts @@ -21,7 +21,7 @@ describe('A server with authorization', (): void => { { 'content-type': 'text/turtle', 'transfer-encoding': 'chunked' }, [ ' .' ], ); - expect(response.statusCode).toBe(200); + expect(response.statusCode).toBe(201); // PUT requestUrl = new URL('http://test.com/foo/bar'); @@ -32,7 +32,7 @@ describe('A server with authorization', (): void => { { 'content-type': 'text/turtle', 'transfer-encoding': 'chunked' }, [ ' .' ], ); - expect(response.statusCode).toBe(200); + expect(response.statusCode).toBe(205); }); it('can not create new entries if not allowed.', async(): Promise => { diff --git a/test/integration/FullConfig.acl.test.ts b/test/integration/FullConfig.acl.test.ts index 1cbd0ac6b..5f7ecf430 100644 --- a/test/integration/FullConfig.acl.test.ts +++ b/test/integration/FullConfig.acl.test.ts @@ -66,11 +66,10 @@ describe.each([ dataAccessorStore, inMemoryDataAccessorStore ])('A server using // Get file response = await fileHelper.getFile(id); expect(response.statusCode).toBe(200); - expect(response._getHeaders().location).toBe(id); expect(response._getBuffer().toString()).toContain('TESTFILE2'); // DELETE file - await fileHelper.deleteFile(id); + await fileHelper.deleteResource(id); await fileHelper.shouldNotExist(id); }); @@ -95,11 +94,10 @@ describe.each([ dataAccessorStore, inMemoryDataAccessorStore ])('A server using // GET permanent file response = await fileHelper.getFile('http://test.com/permanent.txt'); - expect(response._getHeaders().location).toBe('http://test.com/permanent.txt'); expect(response._getBuffer().toString()).toContain('TEST'); // Try to delete permanent file - response = await fileHelper.deleteFile('http://test.com/permanent.txt', true); + response = await fileHelper.deleteResource('http://test.com/permanent.txt', true); expect(response.statusCode).toBe(401); }); }); diff --git a/test/integration/FullConfig.noAuth.test.ts b/test/integration/FullConfig.noAuth.test.ts index d1fd35c5a..7c972d2b0 100644 --- a/test/integration/FullConfig.noAuth.test.ts +++ b/test/integration/FullConfig.noAuth.test.ts @@ -51,11 +51,10 @@ describe.each(configs)('A server using a %s', (name, configFn): void => { // GET response = await fileHelper.getFile(id); expect(response.statusCode).toBe(200); - expect(response._getHeaders().location).toBe(id); expect(response._getBuffer().toString()).toContain('TESTFILE0'); // DELETE - await fileHelper.deleteFile(id); + await fileHelper.deleteResource(id); await fileHelper.shouldNotExist(id); }); @@ -66,7 +65,6 @@ describe.each(configs)('A server using a %s', (name, configFn): void => { // GET response = await fileHelper.getFile(id); expect(response.statusCode).toBe(200); - expect(response._getHeaders().location).toBe(id); expect(response._getBuffer().toString()).toContain('TESTFILE0'); // PUT @@ -75,11 +73,10 @@ describe.each(configs)('A server using a %s', (name, configFn): void => { // GET response = await fileHelper.getFile(id); expect(response.statusCode).toBe(200); - expect(response._getHeaders().location).toBe(id); expect(response._getBuffer().toString()).toContain('TESTFILE1'); // DELETE - await fileHelper.deleteFile(id); + await fileHelper.deleteResource(id); await fileHelper.shouldNotExist(id); }); @@ -91,10 +88,9 @@ describe.each(configs)('A server using a %s', (name, configFn): void => { // GET response = await fileHelper.getFolder(id); expect(response.statusCode).toBe(200); - expect(response._getHeaders().location).toBe(id); // DELETE - await fileHelper.deleteFolder(id); + await fileHelper.deleteResource(id); await fileHelper.shouldNotExist(id); }); @@ -109,12 +105,11 @@ describe.each(configs)('A server using a %s', (name, configFn): void => { // GET File response = await fileHelper.getFile(id); expect(response.statusCode).toBe(200); - expect(response._getHeaders().location).toBe(id); // DELETE - await fileHelper.deleteFile(id); + await fileHelper.deleteResource(id); await fileHelper.shouldNotExist(id); - await fileHelper.deleteFolder('http://test.com/testfolder0/'); + await fileHelper.deleteResource('http://test.com/testfolder0/'); await fileHelper.shouldNotExist('http://test.com/testfolder0/'); }); @@ -132,9 +127,9 @@ describe.each(configs)('A server using a %s', (name, configFn): void => { expect(response._getData()).toContain('ConflictHttpError: Can only delete empty containers.'); // DELETE - await fileHelper.deleteFile('http://test.com/testfolder1/testfile0.txt'); + await fileHelper.deleteResource('http://test.com/testfolder1/testfile0.txt'); await fileHelper.shouldNotExist('http://test.com/testfolder1/testfile0.txt'); - await fileHelper.deleteFolder(folderId); + await fileHelper.deleteResource(folderId); await fileHelper.shouldNotExist(folderId); }); @@ -153,9 +148,9 @@ describe.each(configs)('A server using a %s', (name, configFn): void => { expect(response._getData()).toContain('ConflictHttpError: Can only delete empty containers.'); // DELETE - await fileHelper.deleteFolder(subFolderId); + await fileHelper.deleteResource(subFolderId); await fileHelper.shouldNotExist(subFolderId); - await fileHelper.deleteFolder(folderId); + await fileHelper.deleteResource(folderId); await fileHelper.shouldNotExist(folderId); }); @@ -174,16 +169,15 @@ describe.each(configs)('A server using a %s', (name, configFn): void => { response = await fileHelper.getFolder(folderId); expect(response.statusCode).toBe(200); - expect(response._getHeaders().location).toBe(folderId); expect(response._getBuffer().toString()).toContain(' .'); expect(response._getBuffer().toString()).toContain(' .'); // DELETE - await fileHelper.deleteFile(fileId); + await fileHelper.deleteResource(fileId); await fileHelper.shouldNotExist(fileId); - await fileHelper.deleteFolder(subFolderId); + await fileHelper.deleteResource(subFolderId); await fileHelper.shouldNotExist(subFolderId); - await fileHelper.deleteFolder(folderId); + await fileHelper.deleteResource(folderId); await fileHelper.shouldNotExist(folderId); }); @@ -194,11 +188,10 @@ describe.each(configs)('A server using a %s', (name, configFn): void => { // GET response = await fileHelper.getFile(fileId); expect(response.statusCode).toBe(200); - expect(response._getHeaders().location).toBe(fileId); expect(response._getHeaders()['content-type']).toBe('image/png'); // DELETE - await fileHelper.deleteFile(fileId); + await fileHelper.deleteResource(fileId); await fileHelper.shouldNotExist(fileId); }); }); diff --git a/test/unit/ldp/http/BasicResponseWriter.test.ts b/test/unit/ldp/http/BasicResponseWriter.test.ts index 2a4fde3d3..bbd139db4 100644 --- a/test/unit/ldp/http/BasicResponseWriter.test.ts +++ b/test/unit/ldp/http/BasicResponseWriter.test.ts @@ -3,75 +3,46 @@ import type { MockResponse } from 'node-mocks-http'; import { createResponse } from 'node-mocks-http'; import streamifyArray from 'streamify-array'; import { BasicResponseWriter } from '../../../../src/ldp/http/BasicResponseWriter'; -import type { ResponseDescription } from '../../../../src/ldp/operations/ResponseDescription'; -import { RepresentationMetadata } from '../../../../src/ldp/representation/RepresentationMetadata'; +import type { ResponseDescription } from '../../../../src/ldp/http/response/ResponseDescription'; import { UnsupportedHttpError } from '../../../../src/util/errors/UnsupportedHttpError'; -import { CONTENT_TYPE } from '../../../../src/util/UriConstants'; describe('A BasicResponseWriter', (): void => { const writer = new BasicResponseWriter(); let response: MockResponse; + let result: ResponseDescription; beforeEach(async(): Promise => { response = createResponse({ eventEmitter: EventEmitter }); + result = { statusCode: 201 }; }); - it('requires a description body, with a binary stream if there is data.', async(): Promise => { + it('requires the input to be a ResponseDescription.', async(): Promise => { await expect(writer.canHandle({ response, result: new Error('error') })) .rejects.toThrow(UnsupportedHttpError); - await expect(writer.canHandle({ response, result: { body: { binary: false }} as ResponseDescription })) - .rejects.toThrow(UnsupportedHttpError); - await expect(writer.canHandle({ response, result: { body: { binary: true }} as ResponseDescription })) + await expect(writer.canHandle({ response, result })) .resolves.toBeUndefined(); }); - it('responds with status code 200 and a location header if there is a description.', async(): Promise => { - await writer.handle({ response, result: { identifier: { path: 'path' }}}); + it('responds with the status code of the ResponseDescription.', async(): Promise => { + await writer.handle({ response, result }); expect(response._isEndCalled()).toBeTruthy(); - expect(response._getStatusCode()).toBe(200); - expect(response._getHeaders()).toMatchObject({ location: 'path' }); + expect(response._getStatusCode()).toBe(201); }); it('responds with a body if the description has a body.', async(): Promise => { - const body = { - binary: true, - data: streamifyArray([ ' .' ]), - metadata: new RepresentationMetadata(), - }; + const data = streamifyArray([ ' .' ]); + result = { statusCode: 201, data }; const end = new Promise((resolve): void => { response.on('end', (): void => { expect(response._isEndCalled()).toBeTruthy(); - expect(response._getStatusCode()).toBe(200); - expect(response._getHeaders()).toMatchObject({ location: 'path' }); + expect(response._getStatusCode()).toBe(201); expect(response._getData()).toEqual(' .'); resolve(); }); }); - await writer.handle({ response, result: { identifier: { path: 'path' }, body }}); - await end; - }); - - it('responds with a content-type if the metadata has it.', async(): Promise => { - const metadata = new RepresentationMetadata({ [CONTENT_TYPE]: 'text/turtle' }); - const body = { - binary: true, - data: streamifyArray([ ' .' ]), - metadata, - }; - - const end = new Promise((resolve): void => { - response.on('end', (): void => { - expect(response._isEndCalled()).toBeTruthy(); - expect(response._getStatusCode()).toBe(200); - expect(response._getHeaders()).toMatchObject({ location: 'path', 'content-type': 'text/turtle' }); - expect(response._getData()).toEqual(' .'); - resolve(); - }); - }); - - await writer.handle({ response, result: { identifier: { path: 'path' }, body }}); + await writer.handle({ response, result }); await end; }); }); diff --git a/test/unit/ldp/http/ErrorResponseWriter.test.ts b/test/unit/ldp/http/ErrorResponseWriter.test.ts index e348a5f49..0a4028b9b 100644 --- a/test/unit/ldp/http/ErrorResponseWriter.test.ts +++ b/test/unit/ldp/http/ErrorResponseWriter.test.ts @@ -2,7 +2,6 @@ import { EventEmitter } from 'events'; import type { MockResponse } from 'node-mocks-http'; import { createResponse } from 'node-mocks-http'; import { ErrorResponseWriter } from '../../../../src/ldp/http/ErrorResponseWriter'; -import type { ResponseDescription } from '../../../../src/ldp/operations/ResponseDescription'; import { UnsupportedHttpError } from '../../../../src/util/errors/UnsupportedHttpError'; describe('An ErrorResponseWriter', (): void => { @@ -16,9 +15,7 @@ describe('An ErrorResponseWriter', (): void => { it('requires the input to be an error.', async(): Promise => { await expect(writer.canHandle({ response, result: new Error('error') })) .resolves.toBeUndefined(); - await expect(writer.canHandle({ response, result: { body: { binary: false }} as ResponseDescription })) - .rejects.toThrow(UnsupportedHttpError); - await expect(writer.canHandle({ response, result: { body: { binary: true }} as ResponseDescription })) + await expect(writer.canHandle({ response, result: { statusCode: 200 }})) .rejects.toThrow(UnsupportedHttpError); }); diff --git a/test/unit/ldp/operations/DeleteOperationHandler.test.ts b/test/unit/ldp/operations/DeleteOperationHandler.test.ts index 7499c2712..b603b9c30 100644 --- a/test/unit/ldp/operations/DeleteOperationHandler.test.ts +++ b/test/unit/ldp/operations/DeleteOperationHandler.test.ts @@ -7,8 +7,7 @@ describe('A DeleteOperationHandler', (): void => { const store = {} as unknown as ResourceStore; const handler = new DeleteOperationHandler(store); beforeEach(async(): Promise => { - // eslint-disable-next-line @typescript-eslint/no-empty-function - store.deleteResource = jest.fn(async(): Promise => {}); + store.deleteResource = jest.fn(async(): Promise => undefined); }); it('only supports DELETE operations.', async(): Promise => { @@ -16,10 +15,12 @@ describe('A DeleteOperationHandler', (): void => { await expect(handler.canHandle({ method: 'GET' } as Operation)).rejects.toThrow(UnsupportedHttpError); }); - it('deletes the resource from the store and returns its identifier.', async(): Promise => { - await expect(handler.handle({ target: { path: 'url' }} as Operation)) - .resolves.toEqual({ identifier: { path: 'url' }}); + it('deletes the resource from the store and returns the correct response.', async(): Promise => { + const result = await handler.handle({ target: { path: 'url' }} as Operation); expect(store.deleteResource).toHaveBeenCalledTimes(1); expect(store.deleteResource).toHaveBeenLastCalledWith({ path: 'url' }); + expect(result.statusCode).toBe(205); + expect(result.metadata).toBeUndefined(); + expect(result.data).toBeUndefined(); }); }); diff --git a/test/unit/ldp/operations/GetOperationHandler.test.ts b/test/unit/ldp/operations/GetOperationHandler.test.ts index efc8d3cf1..f646766e9 100644 --- a/test/unit/ldp/operations/GetOperationHandler.test.ts +++ b/test/unit/ldp/operations/GetOperationHandler.test.ts @@ -6,7 +6,8 @@ import { UnsupportedHttpError } from '../../../../src/util/errors/UnsupportedHtt describe('A GetOperationHandler', (): void => { const store = { - getRepresentation: async(): Promise => ({ binary: false } as Representation), + getRepresentation: async(): Promise => + ({ binary: false, data: 'data', metadata: 'metadata' } as any), } as unknown as ResourceStore; const handler = new GetOperationHandler(store); @@ -15,9 +16,10 @@ describe('A GetOperationHandler', (): void => { await expect(handler.canHandle({ method: 'POST' } as Operation)).rejects.toThrow(UnsupportedHttpError); }); - it('returns the representation from the store with the input identifier.', async(): Promise => { - await expect(handler.handle({ target: { path: 'url' }} as Operation)).resolves.toEqual( - { identifier: { path: 'url' }, body: { binary: false }}, - ); + it('returns the representation from the store with the correct response.', async(): Promise => { + const result = await handler.handle({ target: { path: 'url' }} as Operation); + expect(result.statusCode).toBe(200); + expect(result.metadata).toBe('metadata'); + expect(result.data).toBe('data'); }); }); diff --git a/test/unit/ldp/operations/HeadOperationHandler.test.ts b/test/unit/ldp/operations/HeadOperationHandler.test.ts index d0914ed25..b5efd58aa 100644 --- a/test/unit/ldp/operations/HeadOperationHandler.test.ts +++ b/test/unit/ldp/operations/HeadOperationHandler.test.ts @@ -1,5 +1,4 @@ -import arrayifyStream from 'arrayify-stream'; -import streamifyArray from 'streamify-array'; +import type { Readable } from 'stream'; import { HeadOperationHandler } from '../../../../src/ldp/operations/HeadOperationHandler'; import type { Operation } from '../../../../src/ldp/operations/Operation'; import type { Representation } from '../../../../src/ldp/representation/Representation'; @@ -7,11 +6,18 @@ import type { ResourceStore } from '../../../../src/storage/ResourceStore'; import { UnsupportedHttpError } from '../../../../src/util/errors/UnsupportedHttpError'; describe('A HeadOperationHandler', (): void => { - const store = { - getRepresentation: async(): Promise => ({ binary: false, data: streamifyArray([ 1, 2, 3 ]) } as - Representation), - } as unknown as ResourceStore; - const handler = new HeadOperationHandler(store); + let store: ResourceStore; + let handler: HeadOperationHandler; + let data: Readable; + + beforeEach(async(): Promise => { + data = { destroy: jest.fn() } as any; + store = { + getRepresentation: async(): Promise => + ({ binary: false, data, metadata: 'metadata' } as any), + } as any; + handler = new HeadOperationHandler(store); + }); it('only supports HEAD operations.', async(): Promise => { await expect(handler.canHandle({ method: 'HEAD' } as Operation)).resolves.toBeUndefined(); @@ -19,10 +25,11 @@ describe('A HeadOperationHandler', (): void => { await expect(handler.canHandle({ method: 'POST' } as Operation)).rejects.toThrow(UnsupportedHttpError); }); - it('returns the representation from the store with the input identifier and empty data.', async(): Promise => { + it('returns the representation from the store with the correct response.', async(): Promise => { const result = await handler.handle({ target: { path: 'url' }} as Operation); - expect(result.identifier.path).toBe('url'); - expect(result.body?.binary).toBe(false); - await expect(arrayifyStream(result.body!.data)).resolves.toEqual([]); + expect(result.statusCode).toBe(200); + expect(result.metadata).toBe('metadata'); + expect(result.data).toBeUndefined(); + expect(data.destroy).toHaveBeenCalledTimes(1); }); }); diff --git a/test/unit/ldp/operations/PatchOperationHandler.test.ts b/test/unit/ldp/operations/PatchOperationHandler.test.ts index 5099a24c1..2c66b752d 100644 --- a/test/unit/ldp/operations/PatchOperationHandler.test.ts +++ b/test/unit/ldp/operations/PatchOperationHandler.test.ts @@ -15,10 +15,12 @@ describe('A PatchOperationHandler', (): void => { await expect(handler.canHandle({ method: 'GET' } as Operation)).rejects.toThrow(UnsupportedHttpError); }); - it('deletes the resource from the store and returns its identifier.', async(): Promise => { - await expect(handler.handle({ target: { path: 'url' }, body: { binary: false }} as Operation)) - .resolves.toEqual({ identifier: { path: 'url' }}); + it('deletes the resource from the store and returns the correct response.', async(): Promise => { + const result = await handler.handle({ target: { path: 'url' }, body: { binary: false }} as Operation); expect(store.modifyResource).toHaveBeenCalledTimes(1); expect(store.modifyResource).toHaveBeenLastCalledWith({ path: 'url' }, { binary: false }); + expect(result.statusCode).toBe(205); + expect(result.metadata).toBeUndefined(); + expect(result.data).toBeUndefined(); }); }); diff --git a/test/unit/ldp/operations/PostOperationHandler.test.ts b/test/unit/ldp/operations/PostOperationHandler.test.ts index 3eff9eba7..811c5e38b 100644 --- a/test/unit/ldp/operations/PostOperationHandler.test.ts +++ b/test/unit/ldp/operations/PostOperationHandler.test.ts @@ -1,8 +1,10 @@ import type { Operation } from '../../../../src/ldp/operations/Operation'; import { PostOperationHandler } from '../../../../src/ldp/operations/PostOperationHandler'; +import { RepresentationMetadata } from '../../../../src/ldp/representation/RepresentationMetadata'; import type { ResourceIdentifier } from '../../../../src/ldp/representation/ResourceIdentifier'; import type { ResourceStore } from '../../../../src/storage/ResourceStore'; import { UnsupportedHttpError } from '../../../../src/util/errors/UnsupportedHttpError'; +import { HTTP } from '../../../../src/util/UriConstants'; describe('A PostOperationHandler', (): void => { const store = { @@ -21,8 +23,11 @@ describe('A PostOperationHandler', (): void => { await expect(handler.handle({ method: 'POST' } as Operation)).rejects.toThrow(UnsupportedHttpError); }); - it('adds the given representation to the store and returns the new identifier.', async(): Promise => { - await expect(handler.handle({ method: 'POST', body: { }} as Operation)) - .resolves.toEqual({ identifier: { path: 'newPath' }}); + it('adds the given representation to the store and returns the correct response.', async(): Promise => { + const result = await handler.handle({ method: 'POST', body: { }} as Operation); + expect(result.statusCode).toBe(201); + expect(result.metadata).toBeInstanceOf(RepresentationMetadata); + expect(result.metadata?.get(HTTP.location)?.value).toBe('newPath'); + expect(result.data).toBeUndefined(); }); }); diff --git a/test/unit/ldp/operations/PutOperationHandler.test.ts b/test/unit/ldp/operations/PutOperationHandler.test.ts index a905d3af8..8bee0a4a2 100644 --- a/test/unit/ldp/operations/PutOperationHandler.test.ts +++ b/test/unit/ldp/operations/PutOperationHandler.test.ts @@ -16,11 +16,13 @@ describe('A PutOperationHandler', (): void => { await expect(handler.canHandle({ method: 'PUT' } as Operation)).resolves.toBeUndefined(); }); - it('sets the representation in the store and returns its identifier.', async(): Promise => { - await expect(handler.handle({ target: { path: 'url' }, body: {}} as Operation)) - .resolves.toEqual({ identifier: { path: 'url' }}); + it('sets the representation in the store and returns the correct response.', async(): Promise => { + const result = await handler.handle({ target: { path: 'url' }, body: {}} as Operation); expect(store.setRepresentation).toHaveBeenCalledTimes(1); expect(store.setRepresentation).toHaveBeenLastCalledWith({ path: 'url' }, {}); + expect(result.statusCode).toBe(205); + expect(result.metadata).toBeUndefined(); + expect(result.data).toBeUndefined(); }); it('errors when there is no body.', async(): Promise => { diff --git a/test/util/TestHelpers.ts b/test/util/TestHelpers.ts index 0d15f2baa..1926ba795 100644 --- a/test/util/TestHelpers.ts +++ b/test/util/TestHelpers.ts @@ -124,7 +124,7 @@ export class FileTestHelper { fileData, ); if (!mayFail) { - expect(response.statusCode).toBe(200); + expect(response.statusCode).toBe(201); expect(response._getData()).toHaveLength(0); expect(response._getHeaders().location).toContain(url.format(this.baseUrl)); } @@ -145,9 +145,8 @@ export class FileTestHelper { { 'content-type': contentType, 'transfer-encoding': 'chunked' }, fileData, ); - expect(response.statusCode).toBe(200); + expect(response.statusCode).toBe(205); expect(response._getData()).toHaveLength(0); - expect(response._getHeaders().location).toContain(url.format(putUrl)); return response; } @@ -159,14 +158,13 @@ export class FileTestHelper { return response; } - public async deleteFile(requestUrl: string, mayFail = false): Promise> { + public async deleteResource(requestUrl: string, mayFail = false): Promise> { const deleteUrl = new URL(requestUrl); const response = await this.simpleCall(deleteUrl, 'DELETE', {}); if (!mayFail) { - expect(response.statusCode).toBe(200); + expect(response.statusCode).toBe(205); expect(response._getData()).toHaveLength(0); - expect(response._getHeaders().location).toBe(url.format(requestUrl)); } return response; } @@ -182,7 +180,7 @@ export class FileTestHelper { 'transfer-encoding': 'chunked', }, ); - expect(response.statusCode).toBe(200); + expect(response.statusCode).toBe(201); expect(response._getData()).toHaveLength(0); expect(response._getHeaders().location).toContain(url.format(this.baseUrl)); return response; @@ -195,16 +193,6 @@ export class FileTestHelper { return await this.simpleCall(getUrl, 'GET', { accept: 'application/n-quads' }); } - public async deleteFolder(requestUrl: string): Promise> { - const deleteUrl = new URL(requestUrl); - - const response = await this.simpleCall(deleteUrl, 'DELETE', {}); - expect(response.statusCode).toBe(200); - expect(response._getData()).toHaveLength(0); - expect(response._getHeaders().location).toBe(url.format(requestUrl)); - return response; - } - public async shouldNotExist(requestUrl: string): Promise> { const getUrl = new URL(requestUrl);