diff --git a/src/server/notifications/ComposedNotificationHandler.ts b/src/server/notifications/ComposedNotificationHandler.ts index e3b410e35..048859d89 100644 --- a/src/server/notifications/ComposedNotificationHandler.ts +++ b/src/server/notifications/ComposedNotificationHandler.ts @@ -1,3 +1,4 @@ +import { sameResourceState } from '../../storage/Conditions'; import type { NotificationGenerator } from './generate/NotificationGenerator'; import type { NotificationEmitter } from './NotificationEmitter'; import type { NotificationHandlerInput } from './NotificationHandler'; @@ -37,7 +38,7 @@ export class ComposedNotificationHandler extends NotificationHandler { const { state } = input.channel; // In case the state matches there is no need to send the notification - if (typeof state === 'string' && state === notification.state) { + if (typeof state === 'string' && notification.state && sameResourceState(state, notification.state)) { return; } diff --git a/src/storage/BasicConditions.ts b/src/storage/BasicConditions.ts index 8affe5736..25ad3c770 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, isCurrentETag } from './Conditions'; +import { getETag, sameResourceState } from './Conditions'; import type { Conditions } from './Conditions'; export interface BasicConditionsOptions { @@ -39,19 +39,21 @@ export class BasicConditions implements Conditions { 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; - } + const eTag = getETag(metadata); + if (eTag) { + // Helper function to see if an ETag matches the provided metadata + // eslint-disable-next-line func-style + let eTagMatches = (tag: string): boolean => sameResourceState(tag, eTag); + if (strict) { + eTagMatches = (tag: string): boolean => tag === eTag; + } - if (this.matchesETag && !this.matchesETag.includes('*') && !this.matchesETag.some(eTagMatches)) { - return false; - } - if (this.notMatchesETag?.some(eTagMatches)) { - return false; + 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 diff --git a/src/storage/Conditions.ts b/src/storage/Conditions.ts index 586252de3..baeb5a8fd 100644 --- a/src/storage/Conditions.ts +++ b/src/storage/Conditions.ts @@ -42,31 +42,21 @@ export interface Conditions { export function getETag(metadata: RepresentationMetadata): string | undefined { const modified = metadata.get(DC.terms.modified); const { contentType } = metadata; - if (modified && contentType) { + if (modified) { const date = new Date(modified.value); - return `"${date.getTime()}-${contentType}"`; + // It is possible for the content type to be undefined, + // such as when only the metadata returned by a `DataAccessor` is used. + 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. + * Validates whether 2 ETags correspond to the same state of a resource, + * independent of the representation the ETags correspond 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()}`; +export function sameResourceState(eTag1: string, eTag2: string): boolean { + // Since we base the ETag on the last modified date, + // we know the ETags match as long as the date part is the same. + return eTag1.split('-')[0] === eTag2.split('-')[0]; } diff --git a/test/unit/server/notifications/ComposedNotificationHandler.test.ts b/test/unit/server/notifications/ComposedNotificationHandler.test.ts index 51868151e..1c2584c07 100644 --- a/test/unit/server/notifications/ComposedNotificationHandler.test.ts +++ b/test/unit/server/notifications/ComposedNotificationHandler.test.ts @@ -18,7 +18,7 @@ describe('A ComposedNotificationHandler', (): void => { type: 'Update', object: 'http://example.com/foo', published: '123', - state: '123', + state: '"123456-text/turtle"', }; let channel: NotificationChannel; const representation = new BasicRepresentation(); @@ -66,8 +66,8 @@ describe('A ComposedNotificationHandler', (): void => { expect(emitter.handleSafe).toHaveBeenLastCalledWith({ channel, representation }); }); - it('does not emit the notification if its state matches the channel state.', async(): Promise => { - channel.state = notification.state; + it('does not emit the notification if it has the same resource state as the channel.', async(): Promise => { + channel.state = '"123456-application/ld+json"'; await expect(handler.handle({ channel, topic })).resolves.toBeUndefined(); expect(generator.handle).toHaveBeenCalledTimes(1); expect(generator.handle).toHaveBeenLastCalledWith({ channel, topic }); diff --git a/test/unit/storage/Conditions.test.ts b/test/unit/storage/Conditions.test.ts index 9a9eee469..9fe5c706d 100644 --- a/test/unit/storage/Conditions.test.ts +++ b/test/unit/storage/Conditions.test.ts @@ -1,5 +1,5 @@ import { RepresentationMetadata } from '../../../src/http/representation/RepresentationMetadata'; -import { getETag, isCurrentETag } from '../../../src/storage/Conditions'; +import { getETag, sameResourceState } from '../../../src/storage/Conditions'; import { CONTENT_TYPE, DC } from '../../../src/util/Vocabularies'; describe('Conditions', (): void => { @@ -13,41 +13,32 @@ describe('Conditions', (): void => { expect(getETag(metadata)).toBe(`"${now.getTime()}-text/turtle"`); }); - it('returns undefined if no date or content-type was found.', async(): Promise => { + it('creates a simpler ETag if no content type was found.', async(): Promise => { const now = new Date(); + expect(getETag(new RepresentationMetadata({ [DC.modified]: now.toISOString() }))).toBe(`"${now.getTime()}-"`); + }); + + it('returns undefined if no date found.', async(): Promise => { 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(); + describe('sameResourceState', (): void => { + const eTag = '"123456-text/turtle"'; + const eTagJson = '"123456-application/ld+json"'; + const eTagWrongTime = '"654321-text/turtle"'; - 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('returns true if the ETags are the same.', async(): Promise => { + expect(sameResourceState(eTag, eTag)).toBe(true); }); - 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 true if the ETags target the same timestamp.', async(): Promise => { + expect(sameResourceState(eTag, eTagJson)).toBe(true); }); - it('returns false if the metadata has no last modified date.', async(): Promise => { - const metadata = new RepresentationMetadata(); - expect(isCurrentETag('"ETag"', metadata)).toBe(false); + it('returns false if the timestamp differs.', async(): Promise => { + expect(sameResourceState(eTag, eTagWrongTime)).toBe(false); }); }); });