From 97c534b2bf3a4b7821397ef46aa0ae724a023bb5 Mon Sep 17 00:00:00 2001 From: Joachim Van Herwegen Date: Fri, 13 Aug 2021 16:48:44 +0200 Subject: [PATCH] feat: Keep track of last modified date of resources --- src/storage/DataAccessorBasedStore.ts | 32 ++++++-- src/storage/accessors/FileDataAccessor.ts | 13 ++- src/util/ResourceUtil.ts | 15 +++- .../storage/DataAccessorBasedStore.test.ts | 79 ++++++++++++------- .../accessors/FileDataAccessor.test.ts | 11 ++- test/unit/util/ResourceUtil.test.ts | 30 +++++-- 6 files changed, 130 insertions(+), 50 deletions(-) diff --git a/src/storage/DataAccessorBasedStore.ts b/src/storage/DataAccessorBasedStore.ts index 90da48364..08baa6be9 100644 --- a/src/storage/DataAccessorBasedStore.ts +++ b/src/storage/DataAccessorBasedStore.ts @@ -25,7 +25,7 @@ import { toCanonicalUriPath, } from '../util/PathUtil'; import { parseQuads } from '../util/QuadUtil'; -import { addResourceMetadata } from '../util/ResourceUtil'; +import { addResourceMetadata, updateModifiedDate } from '../util/ResourceUtil'; import { CONTENT_TYPE, DC, @@ -164,7 +164,7 @@ export class DataAccessorBasedStore implements ResourceStore { const newID = await this.createSafeUri(container, representation.metadata); // Write the data. New containers should never be made for a POST request. - await this.writeData(newID, representation, isContainerIdentifier(newID), false); + await this.writeData(newID, representation, isContainerIdentifier(newID), false, false); return newID; } @@ -197,7 +197,7 @@ export class DataAccessorBasedStore implements ResourceStore { } // Potentially have to create containers if it didn't exist yet - return this.writeData(identifier, representation, isContainer, !oldMetadata); + return this.writeData(identifier, representation, isContainer, !oldMetadata, Boolean(oldMetadata)); } public async modifyResource(): Promise { @@ -237,6 +237,17 @@ export class DataAccessorBasedStore implements ResourceStore { const auxiliaries = this.auxiliaryStrategy.getAuxiliaryIdentifiers(identifier); deleted.push(...await this.safelyDeleteAuxiliaryResources(auxiliaries)); } + + if (!this.identifierStrategy.isRootContainer(identifier)) { + const container = this.identifierStrategy.getParentContainer(identifier); + deleted.push(container); + + // Update modified date of parent + const parentMetadata = await this.accessor.getMetadata(container); + updateModifiedDate(parentMetadata); + await this.accessor.writeContainer(container, parentMetadata); + } + await this.accessor.deleteResource(identifier); return deleted; } @@ -300,11 +311,12 @@ export class DataAccessorBasedStore implements ResourceStore { * @param representation - Corresponding Representation. * @param isContainer - Is the incoming resource a container? * @param createContainers - Should parent containers (potentially) be created? + * @param exists - If the resource already exists. * * @returns Identifiers of resources that were possibly modified. */ protected async writeData(identifier: ResourceIdentifier, representation: Representation, isContainer: boolean, - createContainers?: boolean): Promise { + createContainers: boolean, exists: boolean): Promise { // Make sure the metadata has the correct identifier and correct type quads // Need to do this before handling container data to have the correct identifier representation.metadata.identifier = DataFactory.namedNode(identifier.path); @@ -320,12 +332,15 @@ export class DataAccessorBasedStore implements ResourceStore { await this.auxiliaryStrategy.validate(representation); } + // Add date modified metadata + updateModifiedDate(representation.metadata); + // Root container should not have a parent container // Solid, ยง5.3: "Servers MUST create intermediate containers and include corresponding containment triples // in container representations derived from the URI path component of PUT and PATCH requests." // https://solid.github.io/specification/protocol#writing-resources const modified = []; - if (!this.identifierStrategy.isRootContainer(identifier)) { + if (!this.identifierStrategy.isRootContainer(identifier) && !exists) { const container = this.identifierStrategy.getParentContainer(identifier); if (!createContainers) { modified.push(container); @@ -333,6 +348,11 @@ export class DataAccessorBasedStore implements ResourceStore { const created = await this.createRecursiveContainers(container); modified.push(...created.length === 0 ? [ container ] : created); } + + // Parent container is also modified + const parentMetadata = await this.accessor.getMetadata(container); + updateModifiedDate(parentMetadata); + await this.accessor.writeContainer(container, parentMetadata); } // Remove all generated metadata to prevent it from being stored permanently @@ -534,7 +554,7 @@ export class DataAccessorBasedStore implements ResourceStore { const ancestors = this.identifierStrategy.isRootContainer(container) ? [] : await this.createRecursiveContainers(this.identifierStrategy.getParentContainer(container)); - await this.writeData(container, new BasicRepresentation([], container), true); + await this.writeData(container, new BasicRepresentation([], container), true, false, false); return [ ...ancestors, container ]; } } diff --git a/src/storage/accessors/FileDataAccessor.ts b/src/storage/accessors/FileDataAccessor.ts index 7738c4943..02bf7d4a4 100644 --- a/src/storage/accessors/FileDataAccessor.ts +++ b/src/storage/accessors/FileDataAccessor.ts @@ -12,7 +12,7 @@ import { guardStream } from '../../util/GuardedStream'; import type { Guarded } from '../../util/GuardedStream'; import { joinFilePath, isContainerIdentifier } from '../../util/PathUtil'; import { parseQuads, serializeQuads } from '../../util/QuadUtil'; -import { addResourceMetadata } from '../../util/ResourceUtil'; +import { addResourceMetadata, updateModifiedDate } from '../../util/ResourceUtil'; import { toLiteral } from '../../util/TermUtil'; import { CONTENT_TYPE, DC, LDP, POSIX, RDF, SOLID_META, XSD } from '../../util/Vocabularies'; import type { FileIdentifierMapper, ResourceLink } from '../mapping/FileIdentifierMapper'; @@ -193,9 +193,10 @@ export class FileDataAccessor implements DataAccessor { */ private async writeMetadata(link: ResourceLink, metadata: RepresentationMetadata): Promise { // These are stored by file system conventions - metadata.remove(RDF.type, LDP.terms.Resource); - metadata.remove(RDF.type, LDP.terms.Container); - metadata.remove(RDF.type, LDP.terms.BasicContainer); + metadata.remove(RDF.terms.type, LDP.terms.Resource); + metadata.remove(RDF.terms.type, LDP.terms.Container); + metadata.remove(RDF.terms.type, LDP.terms.BasicContainer); + metadata.removeAll(DC.terms.modified); metadata.removeAll(CONTENT_TYPE); const quads = metadata.quads(); const metadataLink = await this.resourceMapper.mapUrlToFilePath(link.identifier, true); @@ -303,9 +304,7 @@ export class FileDataAccessor implements DataAccessor { * @param stats - Stats of the file/directory corresponding to the resource. */ private addPosixMetadata(metadata: RepresentationMetadata, stats: Stats): void { - metadata.add(DC.terms.modified, - toLiteral(stats.mtime.toISOString(), XSD.terms.dateTime), - SOLID_META.terms.ResponseMetadata); + updateModifiedDate(metadata, stats.mtime); metadata.add(POSIX.terms.mtime, toLiteral(Math.floor(stats.mtime.getTime() / 1000), XSD.terms.integer), SOLID_META.terms.ResponseMetadata); diff --git a/src/util/ResourceUtil.ts b/src/util/ResourceUtil.ts index 6a4d80607..d7507f933 100644 --- a/src/util/ResourceUtil.ts +++ b/src/util/ResourceUtil.ts @@ -3,8 +3,9 @@ import { BasicRepresentation } from '../ldp/representation/BasicRepresentation'; import type { Representation } from '../ldp/representation/Representation'; import { RepresentationMetadata } from '../ldp/representation/RepresentationMetadata'; import { guardedStreamFrom } from './StreamUtil'; +import { toLiteral } from './TermUtil'; -import { LDP, RDF } from './Vocabularies'; +import { DC, LDP, RDF, XSD } from './Vocabularies'; /** * Helper function to generate type quads for a Container or Resource. @@ -21,6 +22,18 @@ export function addResourceMetadata(metadata: RepresentationMetadata, isContaine metadata.add(RDF.terms.type, LDP.terms.Resource); } +/** + * Updates the dc:modified time to the given time. + * @param metadata - Metadata to update. + * @param date - Last modified date. Defaults to current time. + */ +export function updateModifiedDate(metadata: RepresentationMetadata, date = new Date()): void { + // Milliseconds get lost in some serializations, potentially causing mismatches + const lastModified = new Date(date); + lastModified.setMilliseconds(0); + metadata.set(DC.terms.modified, toLiteral(lastModified.toISOString(), XSD.terms.dateTime)); +} + /** * Helper function to clone a representation, the original representation can still be used. * This function loads the entire stream in memory. diff --git a/test/unit/storage/DataAccessorBasedStore.test.ts b/test/unit/storage/DataAccessorBasedStore.test.ts index 98822b5a0..cd799399a 100644 --- a/test/unit/storage/DataAccessorBasedStore.test.ts +++ b/test/unit/storage/DataAccessorBasedStore.test.ts @@ -21,7 +21,7 @@ import type { Guarded } from '../../../src/util/GuardedStream'; import { SingleRootIdentifierStrategy } from '../../../src/util/identifiers/SingleRootIdentifierStrategy'; import { trimTrailingSlashes } from '../../../src/util/PathUtil'; import { guardedStreamFrom } from '../../../src/util/StreamUtil'; -import { CONTENT_TYPE, SOLID_HTTP, LDP, PIM, RDF, SOLID_META } from '../../../src/util/Vocabularies'; +import { CONTENT_TYPE, SOLID_HTTP, LDP, PIM, RDF, SOLID_META, DC } from '../../../src/util/Vocabularies'; const { namedNode, quad } = DataFactory; class SimpleDataAccessor implements DataAccessor { @@ -117,6 +117,9 @@ class SimpleSuffixStrategy implements AuxiliaryStrategy { } describe('A DataAccessorBasedStore', (): void => { + const now = new Date(1234567489); + const later = new Date(987654321); + let mockDate: jest.SpyInstance; let store: DataAccessorBasedStore; let accessor: SimpleDataAccessor; const root = 'http://test.com/'; @@ -127,6 +130,8 @@ describe('A DataAccessorBasedStore', (): void => { const resourceData = 'text'; beforeEach(async(): Promise => { + mockDate = jest.spyOn(global, 'Date').mockReturnValue(now as any); + accessor = new SimpleDataAccessor(); auxiliaryStrategy = new SimpleSuffixStrategy('.dummy'); @@ -242,6 +247,7 @@ describe('A DataAccessorBasedStore', (): void => { path: expect.stringMatching(new RegExp(`^${root}[^/]+$`, 'u')), }); await expect(arrayifyStream(accessor.data[result.path].data)).resolves.toEqual([ resourceData ]); + expect(accessor.data[result.path].metadata.get(DC.terms.modified)?.value).toBe(now.toISOString()); }); it('can write containers.', async(): Promise => { @@ -256,8 +262,9 @@ describe('A DataAccessorBasedStore', (): void => { expect(accessor.data[result.path]).toBeTruthy(); expect(accessor.data[result.path].metadata.contentType).toBeUndefined(); - const { data } = await store.getRepresentation(result); + const { data, metadata } = await store.getRepresentation(result); const quads: Quad[] = await arrayifyStream(data); + expect(metadata.get(DC.terms.modified)?.value).toBe(now.toISOString()); expect(quads.some((entry): boolean => entry.subject.value === result.path && entry.object.value === 'http://test.com/coolContainer')).toBeTruthy(); }); @@ -345,7 +352,7 @@ describe('A DataAccessorBasedStore', (): void => { const resourceID = { path: `${root}` }; representation.metadata.removeAll(RDF.type); representation.metadata.contentType = 'text/turtle'; - representation.data = guardedStreamFrom([ `<${`${root}`}> a .` ]); + representation.data = guardedStreamFrom([ `<${root}> a .` ]); await expect(store.setRepresentation(resourceID, representation)).resolves .toEqual([{ path: `${root}` }]); @@ -382,6 +389,8 @@ describe('A DataAccessorBasedStore', (): void => { { path: `${root}resource` }, ]); await expect(arrayifyStream(accessor.data[resourceID.path].data)).resolves.toEqual([ resourceData ]); + expect(accessor.data[resourceID.path].metadata.get(DC.terms.modified)?.value).toBe(now.toISOString()); + expect(accessor.data[root].metadata.get(DC.terms.modified)?.value).toBe(now.toISOString()); }); it('can write containers.', async(): Promise => { @@ -390,13 +399,37 @@ describe('A DataAccessorBasedStore', (): void => { // Generate based on URI representation.metadata.removeAll(RDF.type); representation.metadata.contentType = 'text/turtle'; - representation.data = guardedStreamFrom([ `<${`${root}resource/`}> a .` ]); + representation.data = guardedStreamFrom([ `<${root}resource/> a .` ]); await expect(store.setRepresentation(resourceID, representation)).resolves.toEqual([ { path: root }, { path: `${root}container/` }, ]); expect(accessor.data[resourceID.path]).toBeTruthy(); expect(accessor.data[resourceID.path].metadata.contentType).toBeUndefined(); + expect(accessor.data[resourceID.path].metadata.get(DC.terms.modified)?.value).toBe(now.toISOString()); + expect(accessor.data[root].metadata.get(DC.terms.modified)?.value).toBe(now.toISOString()); + }); + + it('can overwrite resources which does not update parent metadata.', async(): Promise => { + const resourceID = { path: `${root}resource` }; + await expect(store.setRepresentation(resourceID, representation)).resolves.toEqual([ + { path: root }, + { path: `${root}resource` }, + ]); + await expect(arrayifyStream(accessor.data[resourceID.path].data)).resolves.toEqual([ resourceData ]); + expect(accessor.data[resourceID.path].metadata.get(DC.terms.modified)?.value).toBe(now.toISOString()); + expect(accessor.data[root].metadata.get(DC.terms.modified)?.value).toBe(now.toISOString()); + + // Parent metadata does not get updated if the resource already exists + representation = new BasicRepresentation('updatedText', 'text/plain'); + mockDate.mockReturnValue(later); + await expect(store.setRepresentation(resourceID, representation)).resolves.toEqual([ + { path: `${root}resource` }, + ]); + await expect(arrayifyStream(accessor.data[resourceID.path].data)).resolves.toEqual([ 'updatedText' ]); + expect(accessor.data[resourceID.path].metadata.get(DC.terms.modified)?.value).toBe(later.toISOString()); + expect(accessor.data[root].metadata.get(DC.terms.modified)?.value).toBe(now.toISOString()); + mockDate.mockReturnValue(now); }); it('does not write generated metadata.', async(): Promise => { @@ -446,7 +479,7 @@ describe('A DataAccessorBasedStore', (): void => { representation.metadata.contentType = 'text/turtle'; representation.metadata.identifier = DataFactory.namedNode(`${root}resource/`); representation.data = guardedStreamFrom( - [ `<${`${root}resource/`}> .` ], + [ `<${root}resource/> .` ], ); const result = store.setRepresentation(resourceID, representation); await expect(result).rejects.toThrow(ConflictHttpError); @@ -540,8 +573,18 @@ describe('A DataAccessorBasedStore', (): void => { accessor.data[`${root}resource`] = representation; await expect(store.deleteResource({ path: `${root}resource` })).resolves.toEqual([ { path: `${root}resource` }, + { path: root }, ]); expect(accessor.data[`${root}resource`]).toBeUndefined(); + expect(accessor.data[root].metadata.get(DC.terms.modified)?.value).toBe(now.toISOString()); + }); + + it('will delete root non-storage containers.', async(): Promise => { + accessor.data[root] = new BasicRepresentation(representation.data, containerMetadata); + await expect(store.deleteResource({ path: root })).resolves.toEqual([ + { path: root }, + ]); + expect(accessor.data[root]).toBeUndefined(); }); it('will delete a root storage auxiliary resource of a non-root container.', async(): Promise => { @@ -551,6 +594,7 @@ describe('A DataAccessorBasedStore', (): void => { auxiliaryStrategy.isRootRequired = jest.fn().mockReturnValue(true); await expect(store.deleteResource({ path: `${root}container/.dummy` })).resolves.toEqual([ { path: `${root}container/.dummy` }, + { path: `${root}container/` }, ]); expect(accessor.data[`${root}container/.dummy`]).toBeUndefined(); }); @@ -561,6 +605,7 @@ describe('A DataAccessorBasedStore', (): void => { await expect(store.deleteResource({ path: `${root}container/` })).resolves.toEqual([ { path: `${root}container/` }, { path: `${root}container/.dummy` }, + { path: root }, ]); expect(accessor.data[`${root}container/`]).toBeUndefined(); expect(accessor.data[`${root}container/.dummy`]).toBeUndefined(); @@ -580,6 +625,7 @@ describe('A DataAccessorBasedStore', (): void => { logger.error = jest.fn(); await expect(store.deleteResource({ path: `${root}resource` })).resolves.toEqual([ { path: `${root}resource` }, + { path: root }, ]); expect(accessor.data[`${root}resource`]).toBeUndefined(); expect(accessor.data[`${root}resource.dummy`]).not.toBeUndefined(); @@ -588,29 +634,6 @@ describe('A DataAccessorBasedStore', (): void => { 'Error deleting auxiliary resource http://test.com/resource.dummy: auxiliary error!', ); }); - - it('can also handle auxiliary deletion to throw non-native errors.', async(): Promise => { - accessor.data[`${root}resource`] = representation; - accessor.data[`${root}resource.dummy`] = representation; - const deleteFn = accessor.deleteResource; - accessor.deleteResource = jest.fn(async(identifier: ResourceIdentifier): Promise => { - if (auxiliaryStrategy.isAuxiliaryIdentifier(identifier)) { - throw 'auxiliary error!'; - } - await deleteFn.call(accessor, identifier); - }); - const { logger } = store as any; - logger.error = jest.fn(); - await expect(store.deleteResource({ path: `${root}resource` })).resolves.toEqual([ - { path: `${root}resource` }, - ]); - expect(accessor.data[`${root}resource`]).toBeUndefined(); - expect(accessor.data[`${root}resource.dummy`]).not.toBeUndefined(); - expect(logger.error).toHaveBeenCalledTimes(1); - expect(logger.error).toHaveBeenLastCalledWith( - 'Error deleting auxiliary resource http://test.com/resource.dummy: Unknown error: auxiliary error!', - ); - }); }); describe('resource Exists', (): void => { diff --git a/test/unit/storage/accessors/FileDataAccessor.test.ts b/test/unit/storage/accessors/FileDataAccessor.test.ts index da3396683..35d4a857e 100644 --- a/test/unit/storage/accessors/FileDataAccessor.test.ts +++ b/test/unit/storage/accessors/FileDataAccessor.test.ts @@ -21,6 +21,8 @@ jest.mock('fs'); const rootFilePath = 'uploads'; const now = new Date(); +// All relevant functions do not care about the milliseconds or remove them +now.setMilliseconds(0); describe('A FileDataAccessor', (): void => { const base = 'http://test.com/'; @@ -103,7 +105,8 @@ describe('A FileDataAccessor', (): void => { expect(metadata.get(POSIX.size)).toEqualRdfTerm(toLiteral('data'.length, XSD.terms.integer)); expect(metadata.get(DC.modified)).toEqualRdfTerm(toLiteral(now.toISOString(), XSD.terms.dateTime)); expect(metadata.get(POSIX.mtime)).toEqualRdfTerm(toLiteral(Math.floor(now.getTime() / 1000), XSD.terms.integer)); - expect(metadata.quads(null, null, null, SOLID_META.terms.ResponseMetadata)).toHaveLength(3); + // `dc:modified` is in the default graph + expect(metadata.quads(null, null, null, SOLID_META.terms.ResponseMetadata)).toHaveLength(2); }); it('does not generate size metadata for a container.', async(): Promise => { @@ -123,7 +126,8 @@ describe('A FileDataAccessor', (): void => { expect(metadata.get(POSIX.size)).toBeUndefined(); expect(metadata.get(DC.modified)).toEqualRdfTerm(toLiteral(now.toISOString(), XSD.terms.dateTime)); expect(metadata.get(POSIX.mtime)).toEqualRdfTerm(toLiteral(Math.floor(now.getTime() / 1000), XSD.terms.integer)); - expect(metadata.quads(null, null, null, SOLID_META.terms.ResponseMetadata)).toHaveLength(2); + // `dc:modified` is in the default graph + expect(metadata.quads(null, null, null, SOLID_META.terms.ResponseMetadata)).toHaveLength(1); }); it('generates metadata for container child resources.', async(): Promise => { @@ -139,8 +143,9 @@ describe('A FileDataAccessor', (): void => { expect(child.get(DC.modified)).toEqualRdfTerm(toLiteral(now.toISOString(), XSD.terms.dateTime)); expect(child.get(POSIX.mtime)).toEqualRdfTerm(toLiteral(Math.floor(now.getTime() / 1000), XSD.terms.integer)); + // `dc:modified` is in the default graph expect(child.quads(null, null, null, SOLID_META.terms.ResponseMetadata)) - .toHaveLength(isContainerPath(child.identifier.value) ? 2 : 3); + .toHaveLength(isContainerPath(child.identifier.value) ? 1 : 2); } }); diff --git a/test/unit/util/ResourceUtil.test.ts b/test/unit/util/ResourceUtil.test.ts index 67dfa6846..cc96e0397 100644 --- a/test/unit/util/ResourceUtil.test.ts +++ b/test/unit/util/ResourceUtil.test.ts @@ -1,7 +1,10 @@ +import 'jest-rdf'; +import type { Literal } from 'n3'; import { BasicRepresentation } from '../../../src/ldp/representation/BasicRepresentation'; import type { Representation } from '../../../src/ldp/representation/Representation'; -import * as resourceUtils from '../../../src/util/ResourceUtil'; -import 'jest-rdf'; +import { RepresentationMetadata } from '../../../src/ldp/representation/RepresentationMetadata'; +import { cloneRepresentation, updateModifiedDate } from '../../../src/util/ResourceUtil'; +import { DC, XSD } from '../../../src/util/Vocabularies'; describe('ResourceUtil', (): void => { let representation: Representation; @@ -10,16 +13,33 @@ describe('ResourceUtil', (): void => { representation = new BasicRepresentation('data', 'metadata'); }); - describe('cloneRepresentation', (): void => { + describe('#updateModifiedDate', (): void => { + it('adds the given date without milliseconds as last modified date.', async(): Promise => { + const date = new Date(); + date.setMilliseconds(500); + const metadata = new RepresentationMetadata(); + updateModifiedDate(metadata, date); + const lastModified = metadata.get(DC.terms.modified); + expect(lastModified?.termType).toBe('Literal'); + const lastModifiedDate = new Date(lastModified!.value); + expect(date.getTime() - lastModifiedDate.getTime()).toBe(500); + + date.setMilliseconds(0); + expect(lastModified?.value).toBe(date.toISOString()); + expect((lastModified as Literal).datatype).toEqualRdfTerm(XSD.terms.dateTime); + }); + }); + + describe('#cloneRepresentation', (): void => { it('returns a clone of the passed representation.', async(): Promise => { - const res = await resourceUtils.cloneRepresentation(representation); + const res = await cloneRepresentation(representation); expect(res.binary).toBe(representation.binary); expect(res.metadata.identifier).toBe(representation.metadata.identifier); expect(res.metadata.contentType).toBe(representation.metadata.contentType); }); it('ensures that original representation does not update when the clone is updated.', async(): Promise => { - const res = await resourceUtils.cloneRepresentation(representation); + const res = await cloneRepresentation(representation); res.metadata.contentType = 'typetype'; expect(representation.metadata.contentType).not.toBe(res.metadata.contentType); });