diff --git a/src/http/ldp/GetOperationHandler.ts b/src/http/ldp/GetOperationHandler.ts index 6525016d1..5f7de96ba 100644 --- a/src/http/ldp/GetOperationHandler.ts +++ b/src/http/ldp/GetOperationHandler.ts @@ -1,5 +1,6 @@ import type { ResourceStore } from '../../storage/ResourceStore'; import { NotImplementedHttpError } from '../../util/errors/NotImplementedHttpError'; +import { assertReadConditions } from '../../util/ResourceUtil'; import { OkResponseDescription } from '../output/response/OkResponseDescription'; import type { ResponseDescription } from '../output/response/ResponseDescription'; import type { OperationHandlerInput } from './OperationHandler'; @@ -26,6 +27,9 @@ export class GetOperationHandler extends OperationHandler { public async handle({ operation }: OperationHandlerInput): Promise { const body = await this.store.getRepresentation(operation.target, operation.preferences, operation.conditions); + // Check whether the cached representation is still valid or it is necessary to send a new representation + assertReadConditions(body, operation.conditions); + return new OkResponseDescription(body.metadata, body.data); } } diff --git a/src/http/ldp/HeadOperationHandler.ts b/src/http/ldp/HeadOperationHandler.ts index 6bb7676da..276c57229 100644 --- a/src/http/ldp/HeadOperationHandler.ts +++ b/src/http/ldp/HeadOperationHandler.ts @@ -1,5 +1,6 @@ import type { ResourceStore } from '../../storage/ResourceStore'; import { NotImplementedHttpError } from '../../util/errors/NotImplementedHttpError'; +import { assertReadConditions } from '../../util/ResourceUtil'; import { OkResponseDescription } from '../output/response/OkResponseDescription'; import type { ResponseDescription } from '../output/response/ResponseDescription'; import type { OperationHandlerInput } from './OperationHandler'; @@ -29,6 +30,10 @@ export class HeadOperationHandler extends OperationHandler { // Close the Readable as we will not return it. body.data.destroy(); + // Check whether the cached representation is still valid or it is necessary to send a new representation. + // Generally it doesn't make much sense to use condition headers with a HEAD request, but it should be supported. + assertReadConditions(body, operation.conditions); + return new OkResponseDescription(body.metadata); } } diff --git a/src/storage/BasicConditions.ts b/src/storage/BasicConditions.ts index e87acaf99..8affe5736 100644 --- a/src/storage/BasicConditions.ts +++ b/src/storage/BasicConditions.ts @@ -1,6 +1,6 @@ import type { RepresentationMetadata } from '../http/representation/RepresentationMetadata'; import { DC } from '../util/Vocabularies'; -import { getETag } from './Conditions'; +import { getETag, isCurrentETag } from './Conditions'; import type { Conditions } from './Conditions'; export interface BasicConditionsOptions { @@ -26,40 +26,43 @@ export class BasicConditions implements Conditions { this.unmodifiedSince = options.unmodifiedSince; } - public matchesMetadata(metadata?: RepresentationMetadata): boolean { + public matchesMetadata(metadata?: RepresentationMetadata, strict?: boolean): boolean { if (!metadata) { // RFC7232: ...If-Match... If the field-value is "*", the condition is false if the origin server // does not have a current representation for the target resource. return !this.matchesETag?.includes('*'); } - const modified = metadata.get(DC.terms.modified); - const modifiedDate = modified ? new Date(modified.value) : undefined; - const etag = getETag(metadata); - return this.matches(etag, modifiedDate); - } - - public matches(eTag?: string, lastModified?: Date): boolean { // RFC7232: ...If-None-Match... If the field-value is "*", the condition is false if the origin server // has a current representation for the target resource. if (this.notMatchesETag?.includes('*')) { return false; } - if (eTag) { - if (this.matchesETag && !this.matchesETag.includes(eTag) && !this.matchesETag.includes('*')) { - return false; - } - if (this.notMatchesETag?.includes(eTag)) { - return false; - } + // Helper function to see if an ETag matches the provided metadata + // eslint-disable-next-line func-style + let eTagMatches = (tag: string): boolean => isCurrentETag(tag, metadata); + if (strict) { + const eTag = getETag(metadata); + eTagMatches = (tag: string): boolean => tag === eTag; } - if (lastModified) { - if (this.modifiedSince && lastModified < this.modifiedSince) { + if (this.matchesETag && !this.matchesETag.includes('*') && !this.matchesETag.some(eTagMatches)) { + return false; + } + if (this.notMatchesETag?.some(eTagMatches)) { + return false; + } + + // In practice, this will only be undefined on a backend + // that doesn't store the modified date. + const modified = metadata.get(DC.terms.modified); + if (modified) { + const modifiedDate = new Date(modified.value); + if (this.modifiedSince && modifiedDate < this.modifiedSince) { return false; } - if (this.unmodifiedSince && lastModified > this.unmodifiedSince) { + if (this.unmodifiedSince && modifiedDate > this.unmodifiedSince) { return false; } } diff --git a/src/storage/Conditions.ts b/src/storage/Conditions.ts index c31f8d247..586252de3 100644 --- a/src/storage/Conditions.ts +++ b/src/storage/Conditions.ts @@ -25,16 +25,12 @@ export interface Conditions { /** * Checks validity based on the given metadata. * @param metadata - Metadata of the representation. Undefined if the resource does not exist. + * @param strict - How to compare the ETag related headers. + * If true, exact string matching will be used to compare with the ETag for the given metadata. + * If false, it will take into account that content negotiation might still happen + * which can change the ETag. */ - matchesMetadata: (metadata?: RepresentationMetadata) => boolean; - /** - * Checks validity based on the given ETag and/or date. - * This function assumes the resource being checked exists. - * If not, the `matchesMetadata` function should be used. - * @param eTag - Condition based on ETag. - * @param lastModified - Condition based on last modified date. - */ - matches: (eTag?: string, lastModified?: Date) => boolean; + matchesMetadata: (metadata?: RepresentationMetadata, strict?: boolean) => boolean; } /** @@ -45,8 +41,32 @@ export interface Conditions { */ export function getETag(metadata: RepresentationMetadata): string | undefined { const modified = metadata.get(DC.terms.modified); - if (modified) { + const { contentType } = metadata; + if (modified && contentType) { const date = new Date(modified.value); - return `"${date.getTime()}"`; + return `"${date.getTime()}-${contentType}"`; } } + +/** + * Validates whether a given ETag corresponds to the current state of the resource, + * independent of the representation the ETag corresponds to. + * Assumes ETags are made with the {@link getETag} function. + * Since we base the ETag on the last modified date, + * we know the ETag still matches as long as that didn't change. + * + * @param eTag - ETag to validate. + * @param metadata - Metadata of the resource. + * + * @returns `true` if the ETag represents the current state of the resource. + */ +export function isCurrentETag(eTag: string, metadata: RepresentationMetadata): boolean { + const modified = metadata.get(DC.terms.modified); + if (!modified) { + return false; + } + const time = eTag.split('-', 1)[0]; + const date = new Date(modified.value); + // `time` will still have the initial`"` of the ETag string + return time === `"${date.getTime()}`; +} diff --git a/src/storage/DataAccessorBasedStore.ts b/src/storage/DataAccessorBasedStore.ts index bc9860da5..15452a848 100644 --- a/src/storage/DataAccessorBasedStore.ts +++ b/src/storage/DataAccessorBasedStore.ts @@ -7,7 +7,6 @@ import { BasicRepresentation } from '../http/representation/BasicRepresentation' import type { Patch } from '../http/representation/Patch'; import type { Representation } from '../http/representation/Representation'; import { RepresentationMetadata } from '../http/representation/RepresentationMetadata'; -import type { RepresentationPreferences } from '../http/representation/RepresentationPreferences'; import type { ResourceIdentifier } from '../http/representation/ResourceIdentifier'; import { getLoggerFor } from '../logging/LogUtil'; import { INTERNAL_QUADS } from '../util/ContentTypes'; @@ -18,7 +17,6 @@ import { ForbiddenHttpError } from '../util/errors/ForbiddenHttpError'; import { MethodNotAllowedHttpError } from '../util/errors/MethodNotAllowedHttpError'; import { NotFoundHttpError } from '../util/errors/NotFoundHttpError'; import { NotImplementedHttpError } from '../util/errors/NotImplementedHttpError'; -import { NotModifiedHttpError } from '../util/errors/NotModifiedHttpError'; import { PreconditionFailedHttpError } from '../util/errors/PreconditionFailedHttpError'; import type { IdentifierStrategy } from '../util/identifiers/IdentifierStrategy'; import { concat } from '../util/IterableUtil'; @@ -105,8 +103,7 @@ export class DataAccessorBasedStore implements ResourceStore { } } - public async getRepresentation(identifier: ResourceIdentifier, - preferences?: RepresentationPreferences, conditions?: Conditions): Promise { + public async getRepresentation(identifier: ResourceIdentifier): Promise { this.validateIdentifier(identifier); let isMetadata = false; @@ -119,8 +116,6 @@ export class DataAccessorBasedStore implements ResourceStore { let metadata = await this.accessor.getMetadata(identifier); let representation: Representation; - this.validateConditions(true, conditions, metadata); - // 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" @@ -186,7 +181,7 @@ export class DataAccessorBasedStore implements ResourceStore { throw new MethodNotAllowedHttpError([ 'POST' ], 'The given path is not a container.'); } - this.validateConditions(false, conditions, parentMetadata); + this.validateConditions(conditions, parentMetadata); // Solid, §5.1: "Servers MAY allow clients to suggest the URI of a resource created through POST, // using the HTTP Slug header as defined in [RFC5023]. @@ -251,7 +246,7 @@ export class DataAccessorBasedStore implements ResourceStore { await this.accessor.canHandle(representation); } - this.validateConditions(false, conditions, oldMetadata); + this.validateConditions(conditions, oldMetadata); if (this.metadataStrategy.isAuxiliaryIdentifier(identifier)) { return await this.writeMetadata(identifier, representation); @@ -273,7 +268,7 @@ export class DataAccessorBasedStore implements ResourceStore { } } - this.validateConditions(false, conditions, metadata); + this.validateConditions(conditions, metadata); } throw new NotImplementedHttpError('Patches are not supported by the default store.'); @@ -314,7 +309,7 @@ export class DataAccessorBasedStore implements ResourceStore { throw new ConflictHttpError('Can only delete empty containers.'); } - this.validateConditions(false, conditions, metadata); + this.validateConditions(conditions, metadata); // Solid, §5.4: "When a contained resource is deleted, // the server MUST also delete the associated auxiliary resources" @@ -352,13 +347,10 @@ export class DataAccessorBasedStore implements ResourceStore { /** * Verify if the given metadata matches the conditions. */ - protected validateConditions(read: boolean, conditions?: Conditions, metadata?: RepresentationMetadata): void { + protected validateConditions(conditions?: Conditions, metadata?: RepresentationMetadata): void { // The 412 (Precondition Failed) status code indicates // that one or more conditions given in the request header fields evaluated to false when tested on the server. if (conditions && !conditions.matchesMetadata(metadata)) { - if (read) { - throw new NotModifiedHttpError(); - } throw new PreconditionFailedHttpError(); } } diff --git a/src/util/ResourceUtil.ts b/src/util/ResourceUtil.ts index f06e593e1..43de0b121 100644 --- a/src/util/ResourceUtil.ts +++ b/src/util/ResourceUtil.ts @@ -3,6 +3,8 @@ import { DataFactory } from 'n3'; import { BasicRepresentation } from '../http/representation/BasicRepresentation'; import type { Representation } from '../http/representation/Representation'; import { RepresentationMetadata } from '../http/representation/RepresentationMetadata'; +import type { Conditions } from '../storage/Conditions'; +import { NotModifiedHttpError } from './errors/NotModifiedHttpError'; import { guardedStreamFrom } from './StreamUtil'; import { toLiteral } from './TermUtil'; import { CONTENT_TYPE_TERM, DC, LDP, RDF, SOLID_META, XSD } from './Vocabularies'; @@ -65,3 +67,25 @@ export async function cloneRepresentation(representation: Representation): Promi representation.data = guardedStreamFrom(data); return result; } + +/** + * Verify whether the given {@link Representation} matches the given conditions. + * If not, destroy the data stream and throw a {@link NotModifiedHttpError}. + * If `conditions` is not defined, nothing will happen. + * + * This uses the strict conditions check which takes the content type into account; + * therefore, this should only be called after content negotiation, when it is certain what the output will be. + * + * Note that browsers only keep track of one ETag, and the Vary header has no impact on this, + * meaning the browser could send us the ETag for a Turtle resource even though it is requesting JSON-LD; + * this is why we have to check ETags after content negotiation. + * + * @param body - The representation to compare the conditions against. + * @param conditions - The conditions to assert. + */ +export function assertReadConditions(body: Representation, conditions?: Conditions): void { + if (conditions && !conditions.matchesMetadata(body.metadata, true)) { + body.data.destroy(); + throw new NotModifiedHttpError(); + } +} diff --git a/test/integration/Conditions.test.ts b/test/integration/Conditions.test.ts index a57cf83ee..574681e95 100644 --- a/test/integration/Conditions.test.ts +++ b/test/integration/Conditions.test.ts @@ -189,6 +189,13 @@ describe.each(stores)('A server supporting conditions with %s', (name, { storeCo headers: { 'if-none-match': eTag! }, }); expect(response.status).toBe(304); + + // GET succeeds if the ETag header doesn't match + response = await fetch(baseUrl, { + method: 'GET', + headers: { 'if-none-match': '"123456"' }, + }); + expect(response.status).toBe(200); }); it('prevents operations if the "if-unmodified-since" header is before the modified date.', async(): Promise => { @@ -218,4 +225,22 @@ describe.each(stores)('A server supporting conditions with %s', (name, { storeCo }); expect(response.status).toBe(205); }); + + it('returns different ETags for different content-types.', async(): Promise => { + let response = await getResource(baseUrl, { accept: 'text/turtle' }, { contentType: 'text/turtle' }); + const eTagTurtle = response.headers.get('ETag'); + response = await getResource(baseUrl, { accept: 'application/ld+json' }, { contentType: 'application/ld+json' }); + const eTagJson = response.headers.get('ETag'); + expect(eTagTurtle).not.toEqual(eTagJson); + + // Both ETags can be used on the same resource + response = await fetch(baseUrl, { headers: { 'if-none-match': eTagTurtle!, accept: 'text/turtle' }}); + expect(response.status).toBe(304); + response = await fetch(baseUrl, { headers: { 'if-none-match': eTagJson!, accept: 'application/ld+json' }}); + expect(response.status).toBe(304); + + // But not for the other representation + response = await fetch(baseUrl, { headers: { 'if-none-match': eTagTurtle!, accept: 'application/ld+json' }}); + expect(response.status).toBe(200); + }); }); diff --git a/test/unit/http/ldp/GetOperationHandler.test.ts b/test/unit/http/ldp/GetOperationHandler.test.ts index b667c70fc..20044c6e1 100644 --- a/test/unit/http/ldp/GetOperationHandler.test.ts +++ b/test/unit/http/ldp/GetOperationHandler.test.ts @@ -1,10 +1,13 @@ +import type { Readable } from 'stream'; import { GetOperationHandler } from '../../../../src/http/ldp/GetOperationHandler'; import type { Operation } from '../../../../src/http/Operation'; import { BasicRepresentation } from '../../../../src/http/representation/BasicRepresentation'; import type { Representation } from '../../../../src/http/representation/Representation'; +import { RepresentationMetadata } from '../../../../src/http/representation/RepresentationMetadata'; import { BasicConditions } from '../../../../src/storage/BasicConditions'; import type { ResourceStore } from '../../../../src/storage/ResourceStore'; import { NotImplementedHttpError } from '../../../../src/util/errors/NotImplementedHttpError'; +import { NotModifiedHttpError } from '../../../../src/util/errors/NotModifiedHttpError'; describe('A GetOperationHandler', (): void => { let operation: Operation; @@ -13,12 +16,15 @@ describe('A GetOperationHandler', (): void => { const body = new BasicRepresentation(); let store: ResourceStore; let handler: GetOperationHandler; + let data: Readable; + const metadata = new RepresentationMetadata(); beforeEach(async(): Promise => { operation = { method: 'GET', target: { path: 'http://test.com/foo' }, preferences, conditions, body }; + data = { destroy: jest.fn() } as any; store = { getRepresentation: jest.fn(async(): Promise => - ({ binary: false, data: 'data', metadata: 'metadata' } as any)), + ({ binary: false, data, metadata } as any)), } as unknown as ResourceStore; handler = new GetOperationHandler(store); @@ -33,9 +39,17 @@ describe('A GetOperationHandler', (): void => { it('returns the representation from the store with the correct response.', async(): Promise => { const result = await handler.handle({ operation }); expect(result.statusCode).toBe(200); - expect(result.metadata).toBe('metadata'); - expect(result.data).toBe('data'); + expect(result.metadata).toBe(metadata); + expect(result.data).toBe(data); expect(store.getRepresentation).toHaveBeenCalledTimes(1); expect(store.getRepresentation).toHaveBeenLastCalledWith(operation.target, preferences, conditions); }); + + it('returns a 304 if the conditions do not match.', async(): Promise => { + operation.conditions = { + matchesMetadata: (): boolean => false, + }; + await expect(handler.handle({ operation })).rejects.toThrow(NotModifiedHttpError); + expect(data.destroy).toHaveBeenCalledTimes(1); + }); }); diff --git a/test/unit/http/ldp/HeadOperationHandler.test.ts b/test/unit/http/ldp/HeadOperationHandler.test.ts index 141c87715..140d98fca 100644 --- a/test/unit/http/ldp/HeadOperationHandler.test.ts +++ b/test/unit/http/ldp/HeadOperationHandler.test.ts @@ -3,9 +3,11 @@ import { HeadOperationHandler } from '../../../../src/http/ldp/HeadOperationHand import type { Operation } from '../../../../src/http/Operation'; import { BasicRepresentation } from '../../../../src/http/representation/BasicRepresentation'; import type { Representation } from '../../../../src/http/representation/Representation'; +import { RepresentationMetadata } from '../../../../src/http/representation/RepresentationMetadata'; import { BasicConditions } from '../../../../src/storage/BasicConditions'; import type { ResourceStore } from '../../../../src/storage/ResourceStore'; import { NotImplementedHttpError } from '../../../../src/util/errors/NotImplementedHttpError'; +import { NotModifiedHttpError } from '../../../../src/util/errors/NotModifiedHttpError'; describe('A HeadOperationHandler', (): void => { let operation: Operation; @@ -15,13 +17,14 @@ describe('A HeadOperationHandler', (): void => { let store: ResourceStore; let handler: HeadOperationHandler; let data: Readable; + const metadata = new RepresentationMetadata(); beforeEach(async(): Promise => { operation = { method: 'HEAD', target: { path: 'http://test.com/foo' }, preferences, conditions, body }; data = { destroy: jest.fn() } as any; store = { getRepresentation: jest.fn(async(): Promise => - ({ binary: false, data, metadata: 'metadata' } as any)), + ({ binary: false, data, metadata } as any)), } as any; handler = new HeadOperationHandler(store); @@ -38,10 +41,18 @@ describe('A HeadOperationHandler', (): void => { it('returns the representation from the store with the correct response.', async(): Promise => { const result = await handler.handle({ operation }); expect(result.statusCode).toBe(200); - expect(result.metadata).toBe('metadata'); + expect(result.metadata).toBe(metadata); expect(result.data).toBeUndefined(); expect(data.destroy).toHaveBeenCalledTimes(1); expect(store.getRepresentation).toHaveBeenCalledTimes(1); expect(store.getRepresentation).toHaveBeenLastCalledWith(operation.target, preferences, conditions); }); + + it('returns a 304 if the conditions do not match.', async(): Promise => { + operation.conditions = { + matchesMetadata: (): boolean => false, + }; + await expect(handler.handle({ operation })).rejects.toThrow(NotModifiedHttpError); + expect(data.destroy).toHaveBeenCalledTimes(2); + }); }); diff --git a/test/unit/http/output/metadata/ModifiedMetadataWriter.test.ts b/test/unit/http/output/metadata/ModifiedMetadataWriter.test.ts index 824fd2b1f..d0ad42bbd 100644 --- a/test/unit/http/output/metadata/ModifiedMetadataWriter.test.ts +++ b/test/unit/http/output/metadata/ModifiedMetadataWriter.test.ts @@ -2,21 +2,22 @@ import { createResponse } from 'node-mocks-http'; import { ModifiedMetadataWriter } from '../../../../../src/http/output/metadata/ModifiedMetadataWriter'; import { RepresentationMetadata } from '../../../../../src/http/representation/RepresentationMetadata'; import type { HttpResponse } from '../../../../../src/server/HttpResponse'; +import { getETag } from '../../../../../src/storage/Conditions'; import { updateModifiedDate } from '../../../../../src/util/ResourceUtil'; -import { DC } from '../../../../../src/util/Vocabularies'; +import { CONTENT_TYPE, DC } from '../../../../../src/util/Vocabularies'; describe('A ModifiedMetadataWriter', (): void => { const writer = new ModifiedMetadataWriter(); it('adds the Last-Modified and ETag header if there is dc:modified metadata.', async(): Promise => { const response = createResponse() as HttpResponse; - const metadata = new RepresentationMetadata(); + const metadata = new RepresentationMetadata({ [CONTENT_TYPE]: 'text/turtle' }); updateModifiedDate(metadata); const dateTime = metadata.get(DC.terms.modified)!.value; await expect(writer.handle({ response, metadata })).resolves.toBeUndefined(); expect(response.getHeaders()).toEqual({ 'last-modified': new Date(dateTime).toUTCString(), - etag: `"${new Date(dateTime).getTime()}"`, + etag: getETag(metadata), }); }); diff --git a/test/unit/storage/BasicConditions.test.ts b/test/unit/storage/BasicConditions.test.ts index 27bca8710..fa79c6db6 100644 --- a/test/unit/storage/BasicConditions.test.ts +++ b/test/unit/storage/BasicConditions.test.ts @@ -1,71 +1,82 @@ import { RepresentationMetadata } from '../../../src/http/representation/RepresentationMetadata'; import { BasicConditions } from '../../../src/storage/BasicConditions'; import { getETag } from '../../../src/storage/Conditions'; -import { DC } from '../../../src/util/Vocabularies'; +import { CONTENT_TYPE, DC } from '../../../src/util/Vocabularies'; + +function getMetadata(modified: Date, type = 'application/ld+json'): RepresentationMetadata { + return new RepresentationMetadata({ + [DC.modified]: `${modified.toISOString()}`, + [CONTENT_TYPE]: type, + }); +} describe('A BasicConditions', (): void => { const now = new Date(2020, 10, 20); const tomorrow = new Date(2020, 10, 21); const yesterday = new Date(2020, 10, 19); - const eTags = [ '123456', 'abcdefg' ]; + const turtleTag = getETag(getMetadata(now, 'text/turtle'))!; + const jsonLdTag = getETag(getMetadata(now))!; it('copies the input parameters.', async(): Promise => { + const eTags = [ '123456', 'abcdefg' ]; const options = { matchesETag: eTags, notMatchesETag: eTags, modifiedSince: now, unmodifiedSince: now }; expect(new BasicConditions(options)).toMatchObject(options); }); it('always returns false if notMatchesETag contains *.', async(): Promise => { const conditions = new BasicConditions({ notMatchesETag: [ '*' ]}); - expect(conditions.matches()).toBe(false); + expect(conditions.matchesMetadata(new RepresentationMetadata())).toBe(false); }); - it('requires matchesETag to contain the provided ETag.', async(): Promise => { - const conditions = new BasicConditions({ matchesETag: [ '1234' ]}); - expect(conditions.matches('abcd')).toBe(false); - expect(conditions.matches('1234')).toBe(true); + it('requires matchesETag to match the provided ETag timestamp.', async(): Promise => { + const conditions = new BasicConditions({ matchesETag: [ turtleTag ]}); + expect(conditions.matchesMetadata(getMetadata(yesterday))).toBe(false); + expect(conditions.matchesMetadata(getMetadata(now))).toBe(true); + }); + + it('requires matchesETag to match the exact provided ETag in strict mode.', async(): Promise => { + const turtleConditions = new BasicConditions({ matchesETag: [ turtleTag ]}); + const jsonLdConditions = new BasicConditions({ matchesETag: [ jsonLdTag ]}); + expect(turtleConditions.matchesMetadata(getMetadata(now), true)).toBe(false); + expect(jsonLdConditions.matchesMetadata(getMetadata(now), true)).toBe(true); }); it('supports all ETags if matchesETag contains *.', async(): Promise => { const conditions = new BasicConditions({ matchesETag: [ '*' ]}); - expect(conditions.matches('abcd')).toBe(true); - expect(conditions.matches('1234')).toBe(true); + expect(conditions.matchesMetadata(getMetadata(yesterday))).toBe(true); + expect(conditions.matchesMetadata(getMetadata(now))).toBe(true); }); - it('requires notMatchesETag to not contain the provided ETag.', async(): Promise => { - const conditions = new BasicConditions({ notMatchesETag: [ '1234' ]}); - expect(conditions.matches('1234')).toBe(false); - expect(conditions.matches('abcd')).toBe(true); + it('requires notMatchesETag to not match the provided ETag timestamp.', async(): Promise => { + const conditions = new BasicConditions({ notMatchesETag: [ turtleTag ]}); + expect(conditions.matchesMetadata(getMetadata(yesterday))).toBe(true); + expect(conditions.matchesMetadata(getMetadata(now))).toBe(false); + }); + + it('requires notMatchesETag to not match the exact provided ETag in strict mode.', async(): Promise => { + const turtleConditions = new BasicConditions({ notMatchesETag: [ turtleTag ]}); + const jsonLdConditions = new BasicConditions({ notMatchesETag: [ jsonLdTag ]}); + expect(turtleConditions.matchesMetadata(getMetadata(now), true)).toBe(true); + expect(jsonLdConditions.matchesMetadata(getMetadata(now), true)).toBe(false); }); it('requires lastModified to be after modifiedSince.', async(): Promise => { const conditions = new BasicConditions({ modifiedSince: now }); - expect(conditions.matches(undefined, yesterday)).toBe(false); - expect(conditions.matches(undefined, tomorrow)).toBe(true); + expect(conditions.matchesMetadata(getMetadata(yesterday))).toBe(false); + expect(conditions.matchesMetadata(getMetadata(tomorrow))).toBe(true); }); it('requires lastModified to be before unmodifiedSince.', async(): Promise => { const conditions = new BasicConditions({ unmodifiedSince: now }); - expect(conditions.matches(undefined, tomorrow)).toBe(false); - expect(conditions.matches(undefined, yesterday)).toBe(true); - }); - - it('can match based on the last modified date in the metadata.', async(): Promise => { - const metadata = new RepresentationMetadata({ [DC.modified]: now.toISOString() }); - const conditions = new BasicConditions({ - modifiedSince: yesterday, - unmodifiedSince: tomorrow, - matchesETag: [ getETag(metadata)! ], - notMatchesETag: [ '123456' ], - }); - expect(conditions.matchesMetadata(metadata)).toBe(true); + expect(conditions.matchesMetadata(getMetadata(yesterday))).toBe(true); + expect(conditions.matchesMetadata(getMetadata(tomorrow))).toBe(false); }); it('matches if no date is found in the metadata.', async(): Promise => { - const metadata = new RepresentationMetadata(); + const metadata = new RepresentationMetadata({ [CONTENT_TYPE]: 'text/turtle' }); const conditions = new BasicConditions({ modifiedSince: yesterday, unmodifiedSince: tomorrow, - matchesETag: [ getETag(metadata)! ], notMatchesETag: [ '123456' ], }); expect(conditions.matchesMetadata(metadata)).toBe(true); diff --git a/test/unit/storage/Conditions.test.ts b/test/unit/storage/Conditions.test.ts index 98a44e589..9a9eee469 100644 --- a/test/unit/storage/Conditions.test.ts +++ b/test/unit/storage/Conditions.test.ts @@ -1,17 +1,53 @@ import { RepresentationMetadata } from '../../../src/http/representation/RepresentationMetadata'; -import { getETag } from '../../../src/storage/Conditions'; -import { DC } from '../../../src/util/Vocabularies'; +import { getETag, isCurrentETag } from '../../../src/storage/Conditions'; +import { CONTENT_TYPE, DC } from '../../../src/util/Vocabularies'; describe('Conditions', (): void => { describe('#getETag', (): void => { - it('creates an ETag based on the date last modified.', async(): Promise => { + it('creates an ETag based on the date last modified and content-type.', async(): Promise => { const now = new Date(); - const metadata = new RepresentationMetadata({ [DC.modified]: now.toISOString() }); - expect(getETag(metadata)).toBe(`"${now.getTime()}"`); + const metadata = new RepresentationMetadata({ + [DC.modified]: now.toISOString(), + [CONTENT_TYPE]: 'text/turtle', + }); + expect(getETag(metadata)).toBe(`"${now.getTime()}-text/turtle"`); }); - it('returns undefined if no date was found.', async(): Promise => { + it('returns undefined if no date or content-type was found.', async(): Promise => { + const now = new Date(); expect(getETag(new RepresentationMetadata())).toBeUndefined(); + expect(getETag(new RepresentationMetadata({ [DC.modified]: now.toISOString() }))).toBeUndefined(); + expect(getETag(new RepresentationMetadata({ [CONTENT_TYPE]: 'text/turtle' }))).toBeUndefined(); + }); + }); + + describe('#isCurrentETag', (): void => { + const now = new Date(); + + it('compares an ETag with the current resource state.', async(): Promise => { + const metadata = new RepresentationMetadata({ + [DC.modified]: now.toISOString(), + [CONTENT_TYPE]: 'text/turtle', + }); + const eTag = getETag(metadata)!; + expect(isCurrentETag(eTag, metadata)).toBe(true); + expect(isCurrentETag('"ETag"', metadata)).toBe(false); + }); + + it('ignores the content-type.', async(): Promise => { + const metadata = new RepresentationMetadata({ + [DC.modified]: now.toISOString(), + [CONTENT_TYPE]: 'text/turtle', + }); + const eTag = getETag(metadata)!; + metadata.contentType = 'application/ld+json'; + expect(isCurrentETag(eTag, metadata)).toBe(true); + expect(isCurrentETag('"ETag"', metadata)).toBe(false); + }); + + it('returns false if the metadata has no last modified date.', async(): Promise => { + const metadata = new RepresentationMetadata(); + expect(isCurrentETag('"ETag"', metadata)).toBe(false); }); }); }); diff --git a/test/unit/storage/DataAccessorBasedStore.test.ts b/test/unit/storage/DataAccessorBasedStore.test.ts index 8d9f3d543..2ccda135b 100644 --- a/test/unit/storage/DataAccessorBasedStore.test.ts +++ b/test/unit/storage/DataAccessorBasedStore.test.ts @@ -19,7 +19,6 @@ import { ForbiddenHttpError } from '../../../src/util/errors/ForbiddenHttpError' import { MethodNotAllowedHttpError } from '../../../src/util/errors/MethodNotAllowedHttpError'; import { NotFoundHttpError } from '../../../src/util/errors/NotFoundHttpError'; import { NotImplementedHttpError } from '../../../src/util/errors/NotImplementedHttpError'; -import { NotModifiedHttpError } from '../../../src/util/errors/NotModifiedHttpError'; import { PreconditionFailedHttpError } from '../../../src/util/errors/PreconditionFailedHttpError'; import type { Guarded } from '../../../src/util/GuardedStream'; import { SingleRootIdentifierStrategy } from '../../../src/util/identifiers/SingleRootIdentifierStrategy'; @@ -218,18 +217,6 @@ describe('A DataAccessorBasedStore', (): void => { ); expect(result.metadata.contentType).toBe(INTERNAL_QUADS); }); - - it('throws a 304 if the request is a read type error.', async(): Promise => { - const resourceID = { path: root }; - const conditions = new BasicConditions({ notMatchesETag: [ '*' ]}); - await expect(store.getRepresentation(resourceID, undefined, conditions)).rejects.toThrow(NotModifiedHttpError); - }); - - it('has conditions but throws no error.', async(): Promise => { - const resourceID = { path: root }; - const conditions = new BasicConditions({ matchesETag: [ '*' ]}); - await expect(store.getRepresentation(resourceID, undefined, conditions)).resolves.toBeTruthy(); - }); }); describe('adding a Resource', (): void => { diff --git a/test/unit/util/ResourceUtil.test.ts b/test/unit/util/ResourceUtil.test.ts index f177f9658..d70b9187b 100644 --- a/test/unit/util/ResourceUtil.test.ts +++ b/test/unit/util/ResourceUtil.test.ts @@ -1,9 +1,18 @@ import 'jest-rdf'; +import type { Readable } from 'stream'; import type { NamedNode, Literal } from 'n3'; import { BasicRepresentation } from '../../../src/http/representation/BasicRepresentation'; import type { Representation } from '../../../src/http/representation/Representation'; import { RepresentationMetadata } from '../../../src/http/representation/RepresentationMetadata'; -import { addTemplateMetadata, cloneRepresentation, updateModifiedDate } from '../../../src/util/ResourceUtil'; +import type { Conditions } from '../../../src/storage/Conditions'; +import { NotModifiedHttpError } from '../../../src/util/errors/NotModifiedHttpError'; +import type { Guarded } from '../../../src/util/GuardedStream'; +import { + addTemplateMetadata, + assertReadConditions, + cloneRepresentation, + updateModifiedDate, +} from '../../../src/util/ResourceUtil'; import { CONTENT_TYPE_TERM, DC, SOLID_META, XSD } from '../../../src/util/Vocabularies'; describe('ResourceUtil', (): void => { @@ -59,4 +68,36 @@ describe('ResourceUtil', (): void => { expect(representation.metadata.contentType).not.toBe(res.metadata.contentType); }); }); + + describe('#assertReadConditions', (): void => { + let data: jest.Mocked>; + + beforeEach(async(): Promise => { + data = { + destroy: jest.fn(), + } as any; + representation.data = data; + }); + + it('does nothing if the conditions are undefined.', async(): Promise => { + expect((): any => assertReadConditions(representation)).not.toThrow(); + expect(data.destroy).toHaveBeenCalledTimes(0); + }); + + it('does nothing if the conditions match.', async(): Promise => { + const conditions: Conditions = { + matchesMetadata: (): boolean => true, + }; + expect((): any => assertReadConditions(representation, conditions)).not.toThrow(); + expect(data.destroy).toHaveBeenCalledTimes(0); + }); + + it('throws a NotModifiedHttpError if the conditions do not match.', async(): Promise => { + const conditions: Conditions = { + matchesMetadata: (): boolean => false, + }; + expect((): any => assertReadConditions(representation, conditions)).toThrow(NotModifiedHttpError); + expect(data.destroy).toHaveBeenCalledTimes(1); + }); + }); });