feat: Emit container pub event on PUT.

Closes https://github.com/solid/community-server/issues/612
CommunitySolidServer · Feb 24, 2021 · c3cff55 · c3cff55
1 parent 6edc255
commit c3cff55
Show file tree

Hide file tree

Showing 15 changed files with 85 additions and 123 deletions.
diff --git a/config/presets/storage-wrapper.json b/config/presets/storage-wrapper.json
@@ -6,9 +6,6 @@
       "@type": "MonitoringStore",
       "MonitoringStore:_source": {
         "@id": "urn:solid-server:default:ResourceStore_Locking"
-      },
-      "MonitoringStore:_identifierStrategy": {
-        "@id": "urn:solid-server:default:IdentifierStrategy"
       }
     },
 

diff --git a/src/storage/DataAccessorBasedStore.ts b/src/storage/DataAccessorBasedStore.ts
@@ -200,13 +200,12 @@ export class DataAccessorBasedStore implements ResourceStore {
     // Solid, §5.4: "When a contained resource is deleted, the server MUST also delete the associated auxiliary
     // resources"
     // https://solid.github.io/specification/protocol#deleting-resources
-    const deleted = [];
+    const deleted = [ identifier ];
     if (!this.auxiliaryStrategy.isAuxiliaryIdentifier(identifier)) {
       const auxiliaries = this.auxiliaryStrategy.getAuxiliaryIdentifiers(identifier);
       deleted.push(...await this.safelyDeleteAuxiliaryResources(auxiliaries));
     }
-
-    deleted.unshift(...await this.accessor.deleteResource(identifier));
+    await this.accessor.deleteResource(identifier);
     return deleted;
   }
 

diff --git a/src/storage/MonitoringStore.ts b/src/storage/MonitoringStore.ts
@@ -3,7 +3,6 @@ import type { Patch } from '../ldp/http/Patch';
 import type { Representation } from '../ldp/representation/Representation';
 import type { RepresentationPreferences } from '../ldp/representation/RepresentationPreferences';
 import type { ResourceIdentifier } from '../ldp/representation/ResourceIdentifier';
-import type { IdentifierStrategy } from '../util/identifiers/IdentifierStrategy';
 import type { Conditions } from './Conditions';
 import type { ResourceStore } from './ResourceStore';
 
@@ -14,55 +13,43 @@ import type { ResourceStore } from './ResourceStore';
 export class MonitoringStore<T extends ResourceStore = ResourceStore>
   extends EventEmitter implements ResourceStore {
   private readonly source: T;
-  private readonly identifierStrategy: IdentifierStrategy;
 
-  public constructor(source: T, identifierStrategy: IdentifierStrategy) {
+  public constructor(source: T) {
     super();
     this.source = source;
-    this.identifierStrategy = identifierStrategy;
+  }
+
+  public async getRepresentation(identifier: ResourceIdentifier, preferences: RepresentationPreferences,
+    conditions?: Conditions): Promise<Representation> {
+    return this.source.getRepresentation(identifier, preferences, conditions);
   }
 
   public async addResource(container: ResourceIdentifier, representation: Representation,
     conditions?: Conditions): Promise<ResourceIdentifier> {
     const identifier = await this.source.addResource(container, representation, conditions);
-
-    // Both the container contents and the resource itself have changed
-    this.emit('changed', container);
-    this.emit('changed', identifier);
-
+    this.emitChanged([ container, identifier ]);
     return identifier;
   }
 
   public async deleteResource(identifier: ResourceIdentifier,
     conditions?: Conditions): Promise<ResourceIdentifier[]> {
-    const modified = await this.source.deleteResource(identifier, conditions);
-
-    // Both the container contents and the resource itself have changed
-    if (!this.identifierStrategy.isRootContainer(identifier)) {
-      const container = this.identifierStrategy.getParentContainer(identifier);
-      this.emit('changed', container);
-    }
-    this.emit('changed', identifier);
-
-    return modified;
+    return this.emitChanged(await this.source.deleteResource(identifier, conditions));
   }
 
-  public async getRepresentation(identifier: ResourceIdentifier, preferences: RepresentationPreferences,
-    conditions?: Conditions): Promise<Representation> {
-    return this.source.getRepresentation(identifier, preferences, conditions);
+  public async setRepresentation(identifier: ResourceIdentifier, representation: Representation,
+    conditions?: Conditions): Promise<ResourceIdentifier[]> {
+    return this.emitChanged(await this.source.setRepresentation(identifier, representation, conditions));
   }
 
   public async modifyResource(identifier: ResourceIdentifier, patch: Patch,
     conditions?: Conditions): Promise<ResourceIdentifier[]> {
-    const modified = await this.source.modifyResource(identifier, patch, conditions);
-    this.emit('changed', identifier);
-    return modified;
+    return this.emitChanged(await this.source.modifyResource(identifier, patch, conditions));
   }
 
-  public async setRepresentation(identifier: ResourceIdentifier, representation: Representation,
-    conditions?: Conditions): Promise<ResourceIdentifier[]> {
-    const modified = await this.source.setRepresentation(identifier, representation, conditions);
-    this.emit('changed', identifier);
-    return modified;
+  private emitChanged(identifiers: ResourceIdentifier[]): typeof identifiers {
+    for (const identifier of identifiers) {
+      this.emit('changed', identifier);
+    }
+    return identifiers;
   }
 }
diff --git a/src/storage/accessors/DataAccessor.ts b/src/storage/accessors/DataAccessor.ts
@@ -63,8 +63,6 @@ export interface DataAccessor {
    * https://solid.github.io/specification/protocol#deleting-resources
    *
    * @param identifier - Resource to delete.
-   *
-   * @returns Identifiers of resources that were possibly modified.
    */
-  deleteResource: (identifier: ResourceIdentifier) => Promise<ResourceIdentifier[]>;
+  deleteResource: (identifier: ResourceIdentifier) => Promise<void>;
 }
diff --git a/src/storage/accessors/FileDataAccessor.ts b/src/storage/accessors/FileDataAccessor.ts
@@ -117,15 +117,12 @@ export class FileDataAccessor implements DataAccessor {
   /**
    * Removes the corresponding file/folder (and metadata file).
    */
-  public async deleteResource(identifier: ResourceIdentifier): Promise<ResourceIdentifier[]> {
+  public async deleteResource(identifier: ResourceIdentifier): Promise<void> {
     const link = await this.resourceMapper.mapUrlToFilePath(identifier);
-    const metadataLink = await this.getMetadataLink(link.identifier);
     const stats = await this.getStats(link.filePath);
-    const modified: ResourceIdentifier[] = [ identifier ];
 
     try {
-      await fsPromises.unlink(metadataLink.filePath);
-      modified.push(metadataLink.identifier);
+      await fsPromises.unlink((await this.getMetadataLink(link.identifier)).filePath);
     } catch (error: unknown) {
       // Ignore if it doesn't exist
       if (!isSystemError(error) || error.code !== 'ENOENT') {
@@ -140,8 +137,6 @@ export class FileDataAccessor implements DataAccessor {
     } else {
       throw new NotFoundHttpError();
     }
-
-    return modified;
   }
 
   /**

diff --git a/src/storage/accessors/InMemoryDataAccessor.ts b/src/storage/accessors/InMemoryDataAccessor.ts
@@ -83,14 +83,13 @@ export class InMemoryDataAccessor implements DataAccessor {
     }
   }
 
-  public async deleteResource(identifier: ResourceIdentifier): Promise<ResourceIdentifier[]> {
+  public async deleteResource(identifier: ResourceIdentifier): Promise<void> {
     const { parent, name } = this.getParentEntry(identifier);
     if (!parent.entries[name]) {
       throw new NotFoundHttpError();
     }
     // eslint-disable-next-line @typescript-eslint/no-dynamic-delete
     delete parent.entries[name];
-    return [ identifier ];
   }
 
   private isDataEntry(entry: CacheEntry): entry is DataEntry {

diff --git a/src/storage/accessors/SparqlDataAccessor.ts b/src/storage/accessors/SparqlDataAccessor.ts
@@ -134,10 +134,9 @@ export class SparqlDataAccessor implements DataAccessor {
   /**
    * Removes all graph data relevant to the given identifier.
    */
-  public async deleteResource(identifier: ResourceIdentifier): Promise<ResourceIdentifier[]> {
+  public async deleteResource(identifier: ResourceIdentifier): Promise<void> {
     const { name, parent } = this.getRelatedNames(identifier);
-    await this.sendSparqlUpdate(this.sparqlDelete(name, parent));
-    return [ identifier ];
+    return this.sendSparqlUpdate(this.sparqlDelete(name, parent));
   }
 
   /**

diff --git a/src/storage/patch/SparqlUpdatePatchHandler.ts b/src/storage/patch/SparqlUpdatePatchHandler.ts
@@ -41,8 +41,7 @@ export class SparqlUpdatePatchHandler extends PatchHandler {
     const op = patch.algebra;
     this.validateUpdate(op);
 
-    await this.applyPatch(identifier, op);
-    return [ identifier ];
+    return this.applyPatch(identifier, op);
   }
 
   private isDeleteInsert(op: Algebra.Operation): op is Algebra.DeleteInsert {
@@ -109,7 +108,7 @@ export class SparqlUpdatePatchHandler extends PatchHandler {
   /**
    * Apply the given algebra operation to the given identifier.
    */
-  private async applyPatch(identifier: ResourceIdentifier, op: Algebra.Operation): Promise<void> {
+  private async applyPatch(identifier: ResourceIdentifier, op: Algebra.Operation): Promise<ResourceIdentifier[]> {
     const store = new Store<BaseQuad>();
     try {
       // Read the quads of the current representation
@@ -134,7 +133,8 @@ export class SparqlUpdatePatchHandler extends PatchHandler {
     this.logger.debug(`${store.size} quads will be stored to ${identifier.path}.`);
 
     // Write the result
-    await this.source.setRepresentation(identifier, new BasicRepresentation(store.match() as Readable, INTERNAL_QUADS));
+    const patched = new BasicRepresentation(store.match() as Readable, INTERNAL_QUADS);
+    return this.source.setRepresentation(identifier, patched);
   }
 
   /**

diff --git a/test/integration/WebSocketsProtocol.test.ts b/test/integration/WebSocketsProtocol.test.ts
@@ -68,14 +68,20 @@ describe('A server with the Solid WebSockets API behind a proxy', (): void => {
       ]);
     });
 
-    describe('when the client subscribes to a resource', (): void => {
+    describe('when the client subscribes to resources', (): void => {
       beforeAll(async(): Promise<void> => {
-        client.send(`sub https://example.pod/my-resource`);
+        client.send('sub https://example.pod/my-resource');
+        client.send('sub https://example.pod/other-resource');
+        client.send('sub https://example.pod/');
         await new Promise((resolve): any => client.once('message', resolve));
       });
 
       it('acknowledges the subscription.', async(): Promise<void> => {
-        expect(messages).toEqual([ `ack https://example.pod/my-resource` ]);
+        expect(messages).toEqual([
+          'ack https://example.pod/my-resource',
+          'ack https://example.pod/other-resource',
+          'ack https://example.pod/',
+        ]);
       });
 
       it('notifies the client of resource updates.', async(): Promise<void> => {
@@ -87,7 +93,10 @@ describe('A server with the Solid WebSockets API behind a proxy', (): void => {
           },
           body: '{}',
         });
-        expect(messages).toEqual([ `pub https://example.pod/my-resource` ]);
+        expect(messages).toEqual([
+          'pub https://example.pod/',
+          'pub https://example.pod/my-resource',
+        ]);
       });
     });
   });

diff --git a/test/unit/storage/DataAccessorBasedStore.test.ts b/test/unit/storage/DataAccessorBasedStore.test.ts
@@ -40,11 +40,10 @@ class SimpleDataAccessor implements DataAccessor {
     }
   }
 
-  public async deleteResource(identifier: ResourceIdentifier): Promise<ResourceIdentifier[]> {
+  public async deleteResource(identifier: ResourceIdentifier): Promise<void> {
     this.checkExists(identifier);
     // eslint-disable-next-line @typescript-eslint/no-dynamic-delete
     delete this.data[identifier.path];
-    return [ identifier ];
   }
 
   public async getData(identifier: ResourceIdentifier): Promise<Guarded<Readable>> {
@@ -563,12 +562,11 @@ describe('A DataAccessorBasedStore', (): void => {
       accessor.data[`${root}resource`] = representation;
       accessor.data[`${root}resource.dummy`] = representation;
       const deleteFn = accessor.deleteResource;
-      accessor.deleteResource = jest.fn(async(identifier: ResourceIdentifier): Promise<ResourceIdentifier[]> => {
+      accessor.deleteResource = jest.fn(async(identifier: ResourceIdentifier): Promise<void> => {
         if (auxStrategy.isAuxiliaryIdentifier(identifier)) {
           throw new Error('auxiliary error!');
         }
         await deleteFn.call(accessor, identifier);
-        return [ identifier ];
       });
       const { logger } = store as any;
       logger.error = jest.fn();
@@ -587,12 +585,11 @@ describe('A DataAccessorBasedStore', (): void => {
       accessor.data[`${root}resource`] = representation;
       accessor.data[`${root}resource.dummy`] = representation;
       const deleteFn = accessor.deleteResource;
-      accessor.deleteResource = jest.fn(async(identifier: ResourceIdentifier): Promise<ResourceIdentifier[]> => {
+      accessor.deleteResource = jest.fn(async(identifier: ResourceIdentifier): Promise<void> => {
         if (auxStrategy.isAuxiliaryIdentifier(identifier)) {
           throw 'auxiliary error!';
         }
         await deleteFn.call(accessor, identifier);
-        return [ identifier ];
       });
       const { logger } = store as any;
       logger.error = jest.fn();