feat: Only accept NamedNodes as predicates for metadata

* refactor: move toCachedNamedNode (private)

* chore: only NamedNodes predicates in removes

* feat: enforce NamedNode predicates in most cases

* feat: getAll only accepts NamedNodes

* feat: toCachedNamedNode only accepts string arg

* tests: use NamedNodes for getAll calls

* test: remove unnecessary string check for coverage

* tests: fix NamedNodes in new tests after rebase

* feat: metadatawriters store NamedNodes

* refactor: toCachedNamedNode as utility function

* fix: double write of linkRelMap

* test: use the CONTENT_TYPE constant

Author: MisterTimn

src/http/auxiliary/LinkMetadataGenerator.ts

@@ -1,4 +1,5 @@
 import { DataFactory } from 'n3';
+import type { NamedNode } from 'rdf-js';
 import { SOLID_META } from '../../util/Vocabularies';
 import type { RepresentationMetadata } from '../representation/RepresentationMetadata';
 import type { AuxiliaryIdentifierStrategy } from './AuxiliaryIdentifierStrategy';
@@ -11,12 +12,12 @@ import { MetadataGenerator } from './MetadataGenerator';
  * In case the input is metadata of an auxiliary resource no metadata will be added
  */
 export class LinkMetadataGenerator extends MetadataGenerator {
-  private readonly link: string;
+  private readonly link: NamedNode;
   private readonly identifierStrategy: AuxiliaryIdentifierStrategy;
 
   public constructor(link: string, identifierStrategy: AuxiliaryIdentifierStrategy) {
     super();
-    this.link = link;
+    this.link = DataFactory.namedNode(link);
     this.identifierStrategy = identifierStrategy;
   }
 

src/http/input/metadata/SlugParser.ts

@@ -19,7 +19,7 @@ export class SlugParser extends MetadataParser {
         throw new BadRequestHttpError('Request has multiple Slug headers');
       }
       this.logger.debug(`Request Slug is '${slug}'.`);
-      input.metadata.set(SOLID_HTTP.slug, slug);
+      input.metadata.set(SOLID_HTTP.terms.slug, slug);
     }
   }
 }

src/http/output/metadata/LinkRelMetadataWriter.ts

@@ -1,3 +1,5 @@
+import type { NamedNode } from 'n3';
+import { DataFactory } from 'n3';
 import { getLoggerFor } from '../../../logging/LogUtil';
 import type { HttpResponse } from '../../../server/HttpResponse';
 import { addHeader } from '../../../util/HeaderUtil';
@@ -9,19 +11,23 @@ import { MetadataWriter } from './MetadataWriter';
  * The values of the objects will be put in a Link header with the corresponding "rel" value.
  */
 export class LinkRelMetadataWriter extends MetadataWriter {
-  private readonly linkRelMap: Record<string, string>;
+  private readonly linkRelMap: Map<NamedNode, string>;
   protected readonly logger = getLoggerFor(this);
 
   public constructor(linkRelMap: Record<string, string>) {
     super();
-    this.linkRelMap = linkRelMap;
+
+    this.linkRelMap = new Map<NamedNode, string>();
+    for (const [ key, value ] of Object.entries(linkRelMap)) {
+      this.linkRelMap.set(DataFactory.namedNode(key), value);
+    }
   }
 
   public async handle(input: { response: HttpResponse; metadata: RepresentationMetadata }): Promise<void> {
-    const keys = Object.keys(this.linkRelMap);
-    this.logger.debug(`Available link relations: ${keys.length}`);
-    for (const key of keys) {
-      const values = input.metadata.getAll(key).map((term): string => `<${term.value}>; rel="${this.linkRelMap[key]}"`);
+    this.logger.debug(`Available link relations: ${this.linkRelMap.size}`);
+    for (const [ predicate, relValue ] of this.linkRelMap) {
+      const values = input.metadata.getAll(predicate)
+        .map((term): string => `<${term.value}>; rel="${relValue}"`);
       if (values.length > 0) {
         this.logger.debug(`Adding Link header ${values}`);
         addHeader(input.response, 'Link', values);

src/http/output/metadata/MappedMetadataWriter.ts

@@ -1,3 +1,5 @@
+import type { NamedNode } from 'n3';
+import { DataFactory } from 'n3';
 import type { HttpResponse } from '../../../server/HttpResponse';
 import { addHeader } from '../../../util/HeaderUtil';
 import type { RepresentationMetadata } from '../../representation/RepresentationMetadata';
@@ -8,11 +10,15 @@ import { MetadataWriter } from './MetadataWriter';
  * The header value(s) will be the same as the corresponding object value(s).
  */
 export class MappedMetadataWriter extends MetadataWriter {
-  private readonly headerMap: [string, string][];
+  private readonly headerMap: Map<NamedNode, string>;
 
   public constructor(headerMap: Record<string, string>) {
     super();
-    this.headerMap = Object.entries(headerMap);
+
+    this.headerMap = new Map<NamedNode, string>();
+    for (const [ key, value ] of Object.entries(headerMap)) {
+      this.headerMap.set(DataFactory.namedNode(key), value);
+    }
   }
 
   public async handle(input: { response: HttpResponse; metadata: RepresentationMetadata }): Promise<void> {

src/http/representation/RepresentationMetadata.ts

@@ -4,7 +4,7 @@ import { getLoggerFor } from '../../logging/LogUtil';
 import { InternalServerError } from '../../util/errors/InternalServerError';
 import type { ContentType } from '../../util/HeaderUtil';
 import { parseContentType } from '../../util/HeaderUtil';
-import { toNamedTerm, toObjectTerm, toCachedNamedNode, isTerm, toLiteral } from '../../util/TermUtil';
+import { toNamedTerm, toObjectTerm, isTerm, toLiteral } from '../../util/TermUtil';
 import { CONTENT_TYPE_TERM, CONTENT_LENGTH_TERM, XSD, SOLID_META, RDFS } from '../../util/Vocabularies';
 import type { ResourceIdentifier } from './ResourceIdentifier';
 import { isResourceIdentifier } from './ResourceIdentifier';
@@ -21,6 +21,22 @@ export function isRepresentationMetadata(object: any): object is RepresentationM
   return typeof object?.setMetadata === 'function';
 }
 
+// Caches named node conversions
+const cachedNamedNodes: Record<string, NamedNode> = {};
+
+/**
+ * Converts the incoming name (URI or shorthand) to a named node.
+ * The generated terms get cached to reduce the number of created nodes,
+ * so only use this for internal constants!
+ * @param name - Predicate to potentially transform.
+ */
+function toCachedNamedNode(name: string): NamedNode {
+  if (!(name in cachedNamedNodes)) {
+    cachedNamedNodes[name] = DataFactory.namedNode(name);
+  }
+  return cachedNamedNodes[name];
+}
+
 /**
  * Stores the metadata triples and provides methods for easy access.
  * Most functions return the metadata object to allow for chaining.
@@ -116,7 +132,7 @@ export class RepresentationMetadata {
    */
   public quads(
     subject: NamedNode | BlankNode | string | null = null,
-    predicate: NamedNode | string | null = null,
+    predicate: NamedNode | null = null,
     object: NamedNode | BlankNode | Literal | string | null = null,
     graph: MetadataGraph | null = null,
   ): Quad[] {
@@ -167,12 +183,12 @@ export class RepresentationMetadata {
    */
   public addQuad(
     subject: NamedNode | BlankNode | string,
-    predicate: NamedNode | string,
+    predicate: NamedNode,
     object: NamedNode | BlankNode | Literal | string,
     graph?: MetadataGraph,
   ): this {
     this.store.addQuad(toNamedTerm(subject),
-      toCachedNamedNode(predicate),
+      predicate,
       toObjectTerm(object, true),
       graph ? toNamedTerm(graph) : undefined);
     return this;
@@ -194,12 +210,12 @@ export class RepresentationMetadata {
    */
   public removeQuad(
     subject: NamedNode | BlankNode | string,
-    predicate: NamedNode | string,
+    predicate: NamedNode,
     object: NamedNode | BlankNode | Literal | string,
     graph?: MetadataGraph,
   ): this {
     const quads = this.quads(toNamedTerm(subject),
-      toCachedNamedNode(predicate),
+      predicate,
       toObjectTerm(object, true),
       graph ? toNamedTerm(graph) : undefined);
     return this.removeQuads(quads);
@@ -219,7 +235,7 @@ export class RepresentationMetadata {
    * @param object - Value(s) to add.
    * @param graph - Optional graph of where to add the values to.
    */
-  public add(predicate: NamedNode | string, object: MetadataValue, graph?: MetadataGraph): this {
+  public add(predicate: NamedNode, object: MetadataValue, graph?: MetadataGraph): this {
     return this.forQuads(predicate, object, (pred, obj): any => this.addQuad(this.id, pred, obj, graph));
   }
 
@@ -229,20 +245,19 @@ export class RepresentationMetadata {
    * @param object - Value(s) to remove.
    * @param graph - Optional graph of where to remove the values from.
    */
-  public remove(predicate: NamedNode | string, object: MetadataValue, graph?: MetadataGraph): this {
+  public remove(predicate: NamedNode, object: MetadataValue, graph?: MetadataGraph): this {
     return this.forQuads(predicate, object, (pred, obj): any => this.removeQuad(this.id, pred, obj, graph));
   }
 
   /**
    * Helper function to simplify add/remove
    * Runs the given function on all predicate/object pairs, but only converts the predicate to a named node once.
    */
-  private forQuads(predicate: NamedNode | string, object: MetadataValue,
+  private forQuads(predicate: NamedNode, object: MetadataValue,
     forFn: (pred: NamedNode, obj: NamedNode | Literal) => void): this {
-    const predicateNode = toCachedNamedNode(predicate);
     const objects = Array.isArray(object) ? object : [ object ];
     for (const obj of objects) {
-      forFn(predicateNode, toObjectTerm(obj, true));
+      forFn(predicate, toObjectTerm(obj, true));
     }
     return this;
   }
@@ -252,8 +267,8 @@ export class RepresentationMetadata {
    * @param predicate - Predicate to remove.
    * @param graph - Optional graph where to remove from.
    */
-  public removeAll(predicate: NamedNode | string, graph?: MetadataGraph): this {
-    this.removeQuads(this.store.getQuads(this.id, toCachedNamedNode(predicate), null, graph ?? null));
+  public removeAll(predicate: NamedNode, graph?: MetadataGraph): this {
+    this.removeQuads(this.store.getQuads(this.id, predicate, null, graph ?? null));
     return this;
   }
 
@@ -278,8 +293,8 @@ export class RepresentationMetadata {
    *
    * @returns An array with all matches.
    */
-  public getAll(predicate: NamedNode | string, graph?: MetadataGraph): Term[] {
-    return this.store.getQuads(this.id, toCachedNamedNode(predicate), null, graph ?? null)
+  public getAll(predicate: NamedNode, graph?: MetadataGraph): Term[] {
+    return this.store.getQuads(this.id, predicate, null, graph ?? null)
       .map((quad): Term => quad.object);
   }
 
@@ -292,15 +307,15 @@ export class RepresentationMetadata {
    *
    * @returns The corresponding value. Undefined if there is no match
    */
-  public get(predicate: NamedNode | string, graph?: MetadataGraph): Term | undefined {
+  public get(predicate: NamedNode, graph?: MetadataGraph): Term | undefined {
     const terms = this.getAll(predicate, graph);
     if (terms.length === 0) {
       return;
     }
     if (terms.length > 1) {
-      this.logger.error(`Multiple results for ${typeof predicate === 'string' ? predicate : predicate.value}`);
+      this.logger.error(`Multiple results for ${predicate.value}`);
       throw new InternalServerError(
-        `Multiple results for ${typeof predicate === 'string' ? predicate : predicate.value}`,
+        `Multiple results for ${predicate.value}`,
       );
     }
     return terms[0];
@@ -313,7 +328,7 @@ export class RepresentationMetadata {
    * @param object - Value(s) to set.
    * @param graph - Optional graph where the triple should be stored.
    */
-  public set(predicate: NamedNode | string, object?: MetadataValue, graph?: MetadataGraph): this {
+  public set(predicate: NamedNode, object?: MetadataValue, graph?: MetadataGraph): this {
     this.removeAll(predicate, graph);
     if (object) {
       this.add(predicate, object, graph);

src/storage/DataAccessorBasedStore.ts

@@ -29,7 +29,6 @@ import {
 import { parseQuads } from '../util/QuadUtil';
 import { addResourceMetadata, updateModifiedDate } from '../util/ResourceUtil';
 import {
-  CONTENT_TYPE,
   DC,
   SOLID_HTTP,
   LDP,
@@ -39,6 +38,7 @@ import {
   XSD,
   SOLID_META,
   PREFERRED_PREFIX_TERM,
+  CONTENT_TYPE_TERM,
 } from '../util/Vocabularies';
 import type { DataAccessor } from './accessors/DataAccessor';
 import type { Conditions } from './Conditions';
@@ -435,7 +435,7 @@ export class DataAccessorBasedStore implements ResourceStore {
     }
 
     // Input content type doesn't matter anymore
-    representation.metadata.removeAll(CONTENT_TYPE);
+    representation.metadata.removeAll(CONTENT_TYPE_TERM);
 
     // Container data is stored in the metadata
     representation.metadata.addQuads(quads);
@@ -516,8 +516,8 @@ export class DataAccessorBasedStore implements ResourceStore {
   Promise<ResourceIdentifier> {
     // Get all values needed for naming the resource
     const isContainer = this.isContainerType(metadata);
-    const slug = metadata.get(SOLID_HTTP.slug)?.value;
-    metadata.removeAll(SOLID_HTTP.slug);
+    const slug = metadata.get(SOLID_HTTP.terms.slug)?.value;
+    metadata.removeAll(SOLID_HTTP.terms.slug);
 
     let newID: ResourceIdentifier = this.createURI(container, isContainer, slug);
 
@@ -544,7 +544,7 @@ export class DataAccessorBasedStore implements ResourceStore {
    * @param metadata - Metadata of the (new) resource.
    */
   protected isContainerType(metadata: RepresentationMetadata): boolean {
-    return this.hasContainerType(metadata.getAll(RDF.type));
+    return this.hasContainerType(metadata.getAll(RDF.terms.type));
   }
 
   /**
@@ -558,7 +558,7 @@ export class DataAccessorBasedStore implements ResourceStore {
    * Verifies if this is the metadata of a root storage container.
    */
   protected isRootStorage(metadata: RepresentationMetadata): boolean {
-    return metadata.getAll(RDF.type).some((term): boolean => term.value === PIM.Storage);
+    return metadata.getAll(RDF.terms.type).some((term): boolean => term.value === PIM.Storage);
   }
 
   /**

src/storage/accessors/FileDataAccessor.ts

@@ -16,7 +16,7 @@ import { joinFilePath, isContainerIdentifier } from '../../util/PathUtil';
 import { parseQuads, serializeQuads } from '../../util/QuadUtil';
 import { addResourceMetadata, updateModifiedDate } from '../../util/ResourceUtil';
 import { toLiteral, toNamedTerm } from '../../util/TermUtil';
-import { CONTENT_TYPE, DC, IANA, LDP, POSIX, RDF, SOLID_META, XSD } from '../../util/Vocabularies';
+import { CONTENT_TYPE_TERM, DC, IANA, LDP, POSIX, RDF, SOLID_META, XSD } from '../../util/Vocabularies';
 import type { FileIdentifierMapper, ResourceLink } from '../mapping/FileIdentifierMapper';
 import type { DataAccessor } from './DataAccessor';
 
@@ -174,7 +174,7 @@ export class FileDataAccessor implements DataAccessor {
   private async getFileMetadata(link: ResourceLink, stats: Stats):
   Promise<RepresentationMetadata> {
     return (await this.getBaseMetadata(link, stats, false))
-      .set(CONTENT_TYPE, link.contentType);
+      .set(CONTENT_TYPE_TERM, link.contentType);
   }
 
   /**
@@ -202,7 +202,7 @@ export class FileDataAccessor implements DataAccessor {
     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);
+    metadata.removeAll(CONTENT_TYPE_TERM);
     const quads = metadata.quads();
     const metadataLink = await this.resourceMapper.mapUrlToFilePath(link.identifier, true);
     let wroteMetadata: boolean;

src/storage/accessors/SparqlDataAccessor.ts

@@ -27,7 +27,7 @@ import { guardStream } from '../../util/GuardedStream';
 import type { Guarded } from '../../util/GuardedStream';
 import type { IdentifierStrategy } from '../../util/identifiers/IdentifierStrategy';
 import { isContainerIdentifier } from '../../util/PathUtil';
-import { CONTENT_TYPE, LDP } from '../../util/Vocabularies';
+import { LDP, CONTENT_TYPE_TERM } from '../../util/Vocabularies';
 import type { DataAccessor } from './DataAccessor';
 
 const { defaultGraph, namedNode, quad, variable } = DataFactory;
@@ -132,7 +132,7 @@ export class SparqlDataAccessor implements DataAccessor {
     }
 
     // Not relevant since all content is triples
-    metadata.removeAll(CONTENT_TYPE);
+    metadata.removeAll(CONTENT_TYPE_TERM);
 
     return this.sendSparqlUpdate(this.sparqlInsert(name, metadata, parent, triples));
   }

src/storage/quota/PodQuotaStrategy.ts

@@ -57,7 +57,7 @@ export class PodQuotaStrategy extends QuotaStrategy {
       throw error;
     }
 
-    const hasPimStorageMetadata = metadata!.getAll(RDF.type)
+    const hasPimStorageMetadata = metadata!.getAll(RDF.terms.type)
       .some((term): boolean => term.value === PIM.Storage);
 
     return hasPimStorageMetadata ? identifier : this.searchPimStorage(parent);

src/util/TermUtil.ts

@@ -1,35 +1,8 @@
 import { DataFactory } from 'n3';
 import type { NamedNode, Literal, Term } from 'rdf-js';
-import { CONTENT_TYPE_TERM } from './Vocabularies';
 
 const { namedNode, literal } = DataFactory;
 
-// Shorthands for commonly used predicates
-const shorthands: Record<string, NamedNode> = {
-  contentType: CONTENT_TYPE_TERM,
-};
-
-// Caches named node conversions
-const cachedNamedNodes: Record<string, NamedNode> = {
-  ...shorthands,
-};
-
-/**
- * Converts the incoming name (URI or shorthand) to a named node.
- * The generated terms get cached to reduce the number of created nodes,
- * so only use this for internal constants!
- * @param name - Predicate to potentially transform.
- */
-export function toCachedNamedNode(name: NamedNode | string): NamedNode {
-  if (typeof name !== 'string') {
-    return name;
-  }
-  if (!(name in cachedNamedNodes)) {
-    cachedNamedNodes[name] = namedNode(name);
-  }
-  return cachedNamedNodes[name];
-}
-
 /**
  * @param input - Checks if this is a {@link Term}.
  */

test/integration/LockingResourceStore.test.ts

@@ -45,7 +45,7 @@ describe('A LockingResourceStore', (): void => {
 
     // Initialize store
     const metadata = new RepresentationMetadata({ path: base }, TEXT_TURTLE);
-    metadata.add(RDF.type, PIM.terms.Storage);
+    metadata.add(RDF.terms.type, PIM.terms.Storage);
     await source.setRepresentation({ path: base }, new BasicRepresentation([], metadata));
 
     locker = new EqualReadWriteLocker(new SingleThreadedResourceLocker());