From fbbccb0cf1f7980c008ecc06cabccb8c08b48b5f Mon Sep 17 00:00:00 2001 From: Wannes Kerckhove Date: Tue, 26 Apr 2022 17:09:19 +0200 Subject: [PATCH] fix: Stop creating meta files for each new resource #1217 --- RELEASE_NOTES.md | 4 + config/ldp/metadata-parser/default.json | 2 - config/storage/backend/global-quota-file.json | 9 ++ config/storage/backend/pod-quota-file.json | 9 ++ config/storage/backend/quota/quota-file.json | 14 +- src/index.ts | 1 + .../accessors/FilterMetadataDataAccessor.ts | 55 ++++++++ src/util/QuadUtil.ts | 22 ++++ src/util/Vocabularies.ts | 1 + test/integration/Quota.test.ts | 25 ++++ .../FilterMetadataDataAccessor.test.ts | 120 ++++++++++++++++++ 11 files changed, 259 insertions(+), 3 deletions(-) create mode 100644 src/storage/accessors/FilterMetadataDataAccessor.ts create mode 100644 test/unit/storage/accessors/FilterMetadataDataAccessor.test.ts diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 350abb33a..b7ca27a31 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -26,6 +26,10 @@ The following changes pertain to the imports in the default configs: The following changes are relevant for v3 custom configs that replaced certain features. - `config/app/variables/cli.json` was changed to support the new `YargsCliExtractor` format. - `config/util/resource-locker/memory.json` had the locker @type changed from `SingleThreadedResourceLocker` to `MemoryResourceLocker`. +- The content-length parser has been moved from the default configuration to the quota configurations. + - `/ldp/metadata-parser/default.json` + - `/storage/backend/*-quota-file.json` + - `/storage/backend/quota/quota-file.json` ### Interface changes These changes are relevant if you wrote custom modules for the server that depend on existing interfaces. diff --git a/config/ldp/metadata-parser/default.json b/config/ldp/metadata-parser/default.json index 22437db8e..d88517fd4 100644 --- a/config/ldp/metadata-parser/default.json +++ b/config/ldp/metadata-parser/default.json @@ -1,7 +1,6 @@ { "@context": "https://linkedsoftwaredependencies.org/bundles/npm/@solid/community-server/^5.0.0/components/context.jsonld", "import": [ - "css:config/ldp/metadata-parser/parsers/content-length.json", "css:config/ldp/metadata-parser/parsers/content-type.json", "css:config/ldp/metadata-parser/parsers/link.json", "css:config/ldp/metadata-parser/parsers/plain-json-ld-filter.json", @@ -13,7 +12,6 @@ "@id": "urn:solid-server:default:MetadataParser", "@type": "ParallelHandler", "handlers": [ - { "@id": "urn:solid-server:default:ContentLengthParser" }, { "@id": "urn:solid-server:default:ContentTypeParser" }, { "@id": "urn:solid-server:default:LinkRelParser" }, { "@id": "urn:solid-server:default:PlainJsonLdFilter" }, diff --git a/config/storage/backend/global-quota-file.json b/config/storage/backend/global-quota-file.json index 1781728ad..22f4de4d4 100644 --- a/config/storage/backend/global-quota-file.json +++ b/config/storage/backend/global-quota-file.json @@ -1,6 +1,7 @@ { "@context": "https://linkedsoftwaredependencies.org/bundles/npm/@solid/community-server/^5.0.0/components/context.jsonld", "import": [ + "css:config/ldp/metadata-parser/parsers/content-length.json", "css:config/storage/backend/quota/global-quota-file.json", "css:config/storage/backend/quota/quota-file.json" ], @@ -12,6 +13,14 @@ "identifierStrategy": { "@id": "urn:solid-server:default:IdentifierStrategy" }, "auxiliaryStrategy": { "@id": "urn:solid-server:default:AuxiliaryStrategy" }, "accessor": { "@id": "urn:solid-server:default:FileDataAccessor" } + }, + { + "comment": "Add content-length parser to the MetadataParser.", + "@id": "urn:solid-server:default:MetadataParser", + "@type": "ParallelHandler", + "handlers": [ + { "@id": "urn:solid-server:default:ContentLengthParser" } + ] } ] } diff --git a/config/storage/backend/pod-quota-file.json b/config/storage/backend/pod-quota-file.json index df557229c..b5b639157 100644 --- a/config/storage/backend/pod-quota-file.json +++ b/config/storage/backend/pod-quota-file.json @@ -1,6 +1,7 @@ { "@context": "https://linkedsoftwaredependencies.org/bundles/npm/@solid/community-server/^5.0.0/components/context.jsonld", "import": [ + "css:config/ldp/metadata-parser/parsers/content-length.json", "css:config/storage/backend/quota/pod-quota-file.json", "css:config/storage/backend/quota/quota-file.json" ], @@ -12,6 +13,14 @@ "identifierStrategy": { "@id": "urn:solid-server:default:IdentifierStrategy" }, "auxiliaryStrategy": { "@id": "urn:solid-server:default:AuxiliaryStrategy" }, "accessor": { "@id": "urn:solid-server:default:FileDataAccessor" } + }, + { + "comment": "Add content-length parser to the MetadataParser.", + "@id": "urn:solid-server:default:MetadataParser", + "@type": "ParallelHandler", + "handlers": [ + { "@id": "urn:solid-server:default:ContentLengthParser" } + ] } ] } diff --git a/config/storage/backend/quota/quota-file.json b/config/storage/backend/quota/quota-file.json index 9a7986796..1bb02dc95 100644 --- a/config/storage/backend/quota/quota-file.json +++ b/config/storage/backend/quota/quota-file.json @@ -28,10 +28,22 @@ { "comment": "Simple wrapper for another DataAccessor but adds validation", - "@id": "urn:solid-server:default:FileDataAccessor", + "@id": "urn:solid-server:default:ValidatingFileDataAccessor", "@type": "ValidatingDataAccessor", "accessor": { "@id": "urn:solid-server:default:AtomicFileDataAccessor" }, "validator": { "@id": "urn:solid-server:default:QuotaValidator" } + }, + { + "comment": "Removes content-length metadata", + "@id": "urn:solid-server:default:FileDataAccessor", + "@type": "FilterMetadataDataAccessor", + "accessor": { "@id": "urn:solid-server:default:ValidatingFileDataAccessor" }, + "filters": [ + { + "@type": "FilterPattern", + "predicate": "http://www.w3.org/2011/http-headers#content-length" + } + ] } ] } diff --git a/src/index.ts b/src/index.ts index 29496ffd4..0f3fb398e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -287,6 +287,7 @@ export * from './storage/accessors/AtomicDataAccessor'; export * from './storage/accessors/AtomicFileDataAccessor'; export * from './storage/accessors/DataAccessor'; export * from './storage/accessors/FileDataAccessor'; +export * from './storage/accessors/FilterMetadataDataAccessor'; export * from './storage/accessors/InMemoryDataAccessor'; export * from './storage/accessors/PassthroughDataAccessor'; export * from './storage/accessors/SparqlDataAccessor'; diff --git a/src/storage/accessors/FilterMetadataDataAccessor.ts b/src/storage/accessors/FilterMetadataDataAccessor.ts new file mode 100644 index 000000000..06d2ab111 --- /dev/null +++ b/src/storage/accessors/FilterMetadataDataAccessor.ts @@ -0,0 +1,55 @@ +import type { Readable } from 'stream'; +import type { RepresentationMetadata } from '../../http/representation/RepresentationMetadata'; +import type { ResourceIdentifier } from '../../http/representation/ResourceIdentifier'; +import type { Guarded } from '../../util/GuardedStream'; +import type { FilterPattern } from '../../util/QuadUtil'; +import type { DataAccessor } from './DataAccessor'; +import { PassthroughDataAccessor } from './PassthroughDataAccessor'; + +/** + * A FilterMetadataDataAccessor wraps a DataAccessor such that specific metadata properties + * can be filtered before passing on the call to the wrapped DataAccessor. + */ +export class FilterMetadataDataAccessor extends PassthroughDataAccessor { + private readonly filters: FilterPattern[]; + + /** + * Construct an instance of FilterMetadataDataAccessor. + * + * @param accessor - The DataAccessor to wrap. + * @param filters - Filter patterns to be used for metadata removal. + */ + public constructor(accessor: DataAccessor, filters: FilterPattern[]) { + super(accessor); + this.filters = filters; + } + + public async writeDocument( + identifier: ResourceIdentifier, + data: Guarded, + metadata: RepresentationMetadata, + ): Promise { + this.applyFilters(metadata); + return this.accessor.writeDocument(identifier, data, metadata); + } + + public async writeContainer(identifier: ResourceIdentifier, metadata: RepresentationMetadata): Promise { + this.applyFilters(metadata); + return this.accessor.writeContainer(identifier, metadata); + } + + /** + * Utility function that removes metadata entries, + * based on the configured filter patterns. + * + * @param metadata - Metadata for the request. + */ + private applyFilters(metadata: RepresentationMetadata): void { + for (const filter of this.filters) { + // Find the matching quads. + const matchingQuads = metadata.quads(filter.subject, filter.predicate, filter.object); + // Remove the resulset. + metadata.removeQuads(matchingQuads); + } + } +} diff --git a/src/util/QuadUtil.ts b/src/util/QuadUtil.ts index 9f50029a7..dfba2e4af 100644 --- a/src/util/QuadUtil.ts +++ b/src/util/QuadUtil.ts @@ -1,10 +1,12 @@ import type { Readable } from 'stream'; +import type { NamedNode } from '@rdfjs/types'; import arrayifyStream from 'arrayify-stream'; import type { ParserOptions } from 'n3'; import { StreamParser, StreamWriter } from 'n3'; import type { Quad } from 'rdf-js'; import type { Guarded } from './GuardedStream'; import { guardedStreamFrom, pipeSafely } from './StreamUtil'; +import { toNamedTerm } from './TermUtil'; /** * Helper function for serializing an array of quads, with as result a Readable object. @@ -42,3 +44,23 @@ export function uniqueQuads(quads: Quad[]): Quad[] { return result; }, []); } + +/** + * Represents a triple pattern to be used as a filter. + */ +export class FilterPattern { + public readonly subject: NamedNode | null; + public readonly predicate: NamedNode | null; + public readonly object: NamedNode | null; + + /** + * @param subject - Optionally filter based on a specific subject. + * @param predicate - Optionally filter based on a predicate. + * @param object - Optionally filter based on a specific object. + */ + public constructor(subject?: string, predicate?: string, object?: string) { + this.subject = typeof subject !== 'undefined' ? toNamedTerm(subject) : null; + this.predicate = typeof predicate !== 'undefined' ? toNamedTerm(predicate) : null; + this.object = typeof object !== 'undefined' ? toNamedTerm(object) : null; + } +} diff --git a/src/util/Vocabularies.ts b/src/util/Vocabularies.ts index 2126e9a6f..2c435fbd9 100644 --- a/src/util/Vocabularies.ts +++ b/src/util/Vocabularies.ts @@ -179,6 +179,7 @@ export const XSD = createUriAndTermNamespace('http://www.w3.org/2001/XMLSchema#' ); // Alias for commonly used types +export const CONTENT_LENGTH = HH['content-length']; export const CONTENT_LENGTH_TERM = HH.terms['content-length']; export const CONTENT_TYPE = MA.format; export const CONTENT_TYPE_TERM = MA.terms.format; diff --git a/test/integration/Quota.test.ts b/test/integration/Quota.test.ts index 99f6642b1..a9e579c63 100644 --- a/test/integration/Quota.test.ts +++ b/test/integration/Quota.test.ts @@ -2,6 +2,7 @@ import { promises as fsPromises } from 'fs'; import type { Stats } from 'fs'; import fetch from 'cross-fetch'; import type { Response } from 'cross-fetch'; +import { pathExists } from 'fs-extra'; import { joinFilePath, joinUrl } from '../../src'; import type { App } from '../../src'; import { getPort } from '../util/Util'; @@ -151,6 +152,18 @@ describe('A quota server', (): void => { await expect(response2).resolves.toBeDefined(); expect((await response2).status).toBe(413); }); + + it('should not generate metadata files (the only possible entry content-length is removed after quota validation).', + async(): Promise => { + const testFile3 = `${pod1}/test3.txt`; + const response1 = performSimplePutWithLength(testFile3, 100); + await expect(response1).resolves.toBeDefined(); + expect((await response1).status).toBe(201); + + // Validate that a meta file was not created + const check = await pathExists(`${rootFilePath}/${podName1}/test3.txt.meta`); + expect(check).toBe(false); + }); }); /** Test the general functionality of the server using global quota */ @@ -218,5 +231,17 @@ describe('A quota server', (): void => { const awaitedRes2 = await response2; expect(awaitedRes2.status).toBe(413); }); + + it('should not generate metadata files (the only possible entry content-length is removed after quota validation).', + async(): Promise => { + const testFile3 = `${pod1}/test5.txt`; + const response1 = performSimplePutWithLength(testFile3, 100); + await expect(response1).resolves.toBeDefined(); + expect((await response1).status).toBe(201); + + // Validate that a meta file was not created + const check = await pathExists(`${rootFilePath}/${podName1}/test5.txt.meta`); + expect(check).toBe(false); + }); }); }); diff --git a/test/unit/storage/accessors/FilterMetadataDataAccessor.test.ts b/test/unit/storage/accessors/FilterMetadataDataAccessor.test.ts new file mode 100644 index 000000000..365f83bf0 --- /dev/null +++ b/test/unit/storage/accessors/FilterMetadataDataAccessor.test.ts @@ -0,0 +1,120 @@ +import { APPLICATION_JSON, CONTENT_LENGTH, CONTENT_TYPE, FilterPattern, toNamedTerm } from '../../../../src'; +import { RepresentationMetadata } from '../../../../src/http/representation/RepresentationMetadata'; +import type { DataAccessor } from '../../../../src/storage/accessors/DataAccessor'; +import { FilterMetadataDataAccessor } from '../../../../src/storage/accessors/FilterMetadataDataAccessor'; +import { guardedStreamFrom } from '../../../../src/util/StreamUtil'; + +describe('FilterMetadataDataAccessor', (): void => { + let childAccessor: jest.Mocked; + + const mockIdentifier = { path: 'http://localhost/test.txt' }; + const mockData = guardedStreamFrom('test string'); + + beforeEach(async(): Promise => { + jest.clearAllMocks(); + childAccessor = { + writeDocument: jest.fn(), + writeContainer: jest.fn(), + } as any; + childAccessor.getChildren = jest.fn(); + }); + + it('removes only the matching metadata properties when calling writeDocument.', async(): Promise => { + const filterMetadataAccessor = new FilterMetadataDataAccessor(childAccessor, + [ new FilterPattern(undefined, CONTENT_LENGTH) ]); + const mockMetadata = new RepresentationMetadata(); + mockMetadata.contentLength = 40; + mockMetadata.contentType = APPLICATION_JSON; + await filterMetadataAccessor.writeDocument(mockIdentifier, mockData, mockMetadata); + expect(childAccessor.writeDocument).toHaveBeenCalledTimes(1); + expect(childAccessor.writeDocument).toHaveBeenLastCalledWith(mockIdentifier, mockData, mockMetadata); + expect(mockMetadata.contentLength).toBeUndefined(); + expect(mockMetadata.contentType).toBe(APPLICATION_JSON); + }); + + it('supports multiple filter patterns when calling writeDocument.', async(): Promise => { + const filters = [ + new FilterPattern(undefined, CONTENT_LENGTH), + new FilterPattern(undefined, CONTENT_TYPE), + ]; + const filterMetadataAccessor = new FilterMetadataDataAccessor(childAccessor, filters); + const mockMetadata = new RepresentationMetadata(); + mockMetadata.contentLength = 40; + mockMetadata.contentType = APPLICATION_JSON; + await filterMetadataAccessor.writeDocument(mockIdentifier, mockData, mockMetadata); + expect(childAccessor.writeDocument).toHaveBeenCalledTimes(1); + expect(childAccessor.writeDocument).toHaveBeenLastCalledWith(mockIdentifier, mockData, mockMetadata); + expect(mockMetadata.contentLength).toBeUndefined(); + expect(mockMetadata.contentType).toBeUndefined(); + }); + + it('removes only the matching metadata properties when calling writeContainer.', async(): Promise => { + const filterMetadataAccessor = new FilterMetadataDataAccessor(childAccessor, + [ new FilterPattern(undefined, CONTENT_LENGTH) ]); + const mockMetadata = new RepresentationMetadata(); + mockMetadata.contentLength = 40; + mockMetadata.contentType = APPLICATION_JSON; + await filterMetadataAccessor.writeContainer(mockIdentifier, mockMetadata); + expect(childAccessor.writeContainer).toHaveBeenCalledTimes(1); + expect(childAccessor.writeContainer).toHaveBeenLastCalledWith(mockIdentifier, mockMetadata); + expect(mockMetadata.contentLength).toBeUndefined(); + expect(mockMetadata.contentType).toBe(APPLICATION_JSON); + }); + + it('supports multiple filter patterns when calling writeContainer.', async(): Promise => { + const filters = [ + new FilterPattern(undefined, CONTENT_LENGTH), + new FilterPattern(undefined, CONTENT_TYPE), + ]; + const filterMetadataAccessor = new FilterMetadataDataAccessor(childAccessor, filters); + const mockMetadata = new RepresentationMetadata(); + mockMetadata.contentLength = 40; + mockMetadata.contentType = APPLICATION_JSON; + await filterMetadataAccessor.writeContainer(mockIdentifier, mockMetadata); + expect(childAccessor.writeContainer).toHaveBeenCalledTimes(1); + expect(childAccessor.writeContainer).toHaveBeenLastCalledWith(mockIdentifier, mockMetadata); + expect(mockMetadata.contentLength).toBeUndefined(); + expect(mockMetadata.contentType).toBeUndefined(); + }); + + it('an empty filter matches all metadata entries, and thus everything is removed.', async(): Promise => { + const filters = [ new FilterPattern() ]; + const filterMetadataAccessor = new FilterMetadataDataAccessor(childAccessor, filters); + const mockMetadata = new RepresentationMetadata(); + mockMetadata.contentLength = 40; + mockMetadata.contentType = APPLICATION_JSON; + await filterMetadataAccessor.writeContainer(mockIdentifier, mockMetadata); + expect(childAccessor.writeContainer).toHaveBeenCalledTimes(1); + expect(childAccessor.writeContainer).toHaveBeenLastCalledWith(mockIdentifier, mockMetadata); + expect(mockMetadata.contentLength).toBeUndefined(); + expect(mockMetadata.contentType).toBeUndefined(); + expect(mockMetadata.quads()).toHaveLength(0); + }); + + it('supports filtering based on subject.', async(): Promise => { + const subject = 'http://example.org/resource/test1'; + const filters = [ new FilterPattern(subject) ]; + const filterMetadataAccessor = new FilterMetadataDataAccessor(childAccessor, filters); + const mockMetadata = new RepresentationMetadata(); + mockMetadata.addQuad(subject, toNamedTerm('http://xmlns.com/foaf/0.1/name'), 'Alice'); + expect(mockMetadata.quads(subject)).toHaveLength(1); + await filterMetadataAccessor.writeDocument(mockIdentifier, mockData, mockMetadata); + expect(childAccessor.writeDocument).toHaveBeenCalledTimes(1); + expect(childAccessor.writeDocument).toHaveBeenLastCalledWith(mockIdentifier, mockData, mockMetadata); + expect(mockMetadata.quads(subject)).toHaveLength(0); + }); + + it('supports filtering based on object.', async(): Promise => { + const subject = 'http://example.org/resource/test1'; + const object = 'http://example.org/resource/test2'; + const filters = [ new FilterPattern(undefined, undefined, object) ]; + const filterMetadataAccessor = new FilterMetadataDataAccessor(childAccessor, filters); + const mockMetadata = new RepresentationMetadata(); + mockMetadata.addQuad(subject, toNamedTerm('http://xmlns.com/foaf/0.1/knows'), toNamedTerm(object)); + expect(mockMetadata.quads(subject)).toHaveLength(1); + await filterMetadataAccessor.writeContainer(mockIdentifier, mockMetadata); + expect(childAccessor.writeContainer).toHaveBeenCalledTimes(1); + expect(childAccessor.writeContainer).toHaveBeenLastCalledWith(mockIdentifier, mockMetadata); + expect(mockMetadata.quads(subject)).toHaveLength(0); + }); +});