diff --git a/src/authorization/AcpReader.ts b/src/authorization/AcpReader.ts index 3d1d6d7fe..e9b842147 100644 --- a/src/authorization/AcpReader.ts +++ b/src/authorization/AcpReader.ts @@ -15,6 +15,7 @@ import { InternalServerError } from '../util/errors/InternalServerError'; import { NotFoundHttpError } from '../util/errors/NotFoundHttpError'; import type { IdentifierStrategy } from '../util/identifiers/IdentifierStrategy'; import { IdentifierMap } from '../util/map/IdentifierMap'; +import { getDefault } from '../util/map/MapUtil'; import { readableToQuads } from '../util/StreamUtil'; import { ACL } from '../util/Vocabularies'; import { getAccessControlledResources } from './AcpUtil'; @@ -78,12 +79,8 @@ export class AcpReader extends PermissionReader { // Extract all the policies relevant for the target const identifiers = this.getAncestorIdentifiers(target); for (const identifier of identifiers) { - let acrs = resourceCache.get(identifier); - if (!acrs) { - const data = await this.readAcrData(identifier); - acrs = [ ...getAccessControlledResources(data) ]; - resourceCache.set(identifier, acrs); - } + const acrs = await getDefault(resourceCache, identifier, async(): Promise => + [ ...getAccessControlledResources(await this.readAcrData(identifier)) ]); const size = policies.length; policies.push(...this.getEffectivePolicies(target, acrs)); this.logger.debug(`Found ${policies.length - size} policies relevant for ${target.path} in ${identifier.path}`); diff --git a/src/authorization/PathBasedReader.ts b/src/authorization/PathBasedReader.ts index 6d858a1b5..533425d26 100644 --- a/src/authorization/PathBasedReader.ts +++ b/src/authorization/PathBasedReader.ts @@ -1,6 +1,7 @@ import { getLoggerFor } from '../logging/LogUtil'; import { concat } from '../util/IterableUtil'; import { IdentifierMap, IdentifierSetMultiMap } from '../util/map/IdentifierMap'; +import { getDefault } from '../util/map/MapUtil'; import { ensureTrailingSlash, trimTrailingSlashes } from '../util/PathUtil'; import type { PermissionReaderInput } from './PermissionReader'; import { PermissionReader } from './PermissionReader'; @@ -44,11 +45,7 @@ export class PathBasedReader extends PermissionReader { for (const [ identifier, modes ] of accessMap) { const reader = this.findReader(identifier.path); if (reader) { - let matches = result.get(reader); - if (!matches) { - matches = new IdentifierSetMultiMap(); - result.set(reader, matches); - } + const matches = getDefault(result, reader, (): AccessMap => new IdentifierSetMultiMap()); matches.set(identifier, modes); } } diff --git a/src/authorization/UnionPermissionReader.ts b/src/authorization/UnionPermissionReader.ts index ad35b7977..7972d9ff3 100644 --- a/src/authorization/UnionPermissionReader.ts +++ b/src/authorization/UnionPermissionReader.ts @@ -27,7 +27,7 @@ export class UnionPermissionReader extends UnionHandler { private mergePermissionMaps(permissionMap: PermissionMap, result: PermissionMap): void { for (const [ identifier, permissionSet ] of permissionMap) { for (const [ credential, permission ] of Object.entries(permissionSet) as [keyof PermissionSet, Permission][]) { - const resultSet = getDefault(result, identifier, {}); + const resultSet = getDefault(result, identifier, (): PermissionSet => ({})); resultSet[credential] = this.mergePermissions(permission, resultSet[credential]); } } diff --git a/src/storage/conversion/BaseTypedRepresentationConverter.ts b/src/storage/conversion/BaseTypedRepresentationConverter.ts index e5e3c40b6..b4f3c3d2d 100644 --- a/src/storage/conversion/BaseTypedRepresentationConverter.ts +++ b/src/storage/conversion/BaseTypedRepresentationConverter.ts @@ -1,10 +1,10 @@ import type { ValuePreferences } from '../../http/representation/RepresentationPreferences'; import { NotImplementedHttpError } from '../../util/errors/NotImplementedHttpError'; +import type { PromiseOrValue } from '../../util/PromiseUtil'; import { getConversionTarget, getTypeWeight, preferencesToString } from './ConversionUtil'; import type { RepresentationConverterArgs } from './RepresentationConverter'; import { TypedRepresentationConverter } from './TypedRepresentationConverter'; -type PromiseOrValue = T | Promise; type ValuePreferencesArg = PromiseOrValue | PromiseOrValue | diff --git a/src/util/PromiseUtil.ts b/src/util/PromiseUtil.ts index 8d38b3ae9..a8cb1609d 100644 --- a/src/util/PromiseUtil.ts +++ b/src/util/PromiseUtil.ts @@ -1,5 +1,29 @@ +import { types } from 'util'; import { createAggregateError } from './errors/HttpErrorUtil'; +export type PromiseOrValue = T | Promise; + +/** + * Verifies if the given value is a Promise or not. + * @param object - Object to check. + */ +export function isPromise(object: PromiseOrValue): object is Promise { + return types.isPromise(object); +} + +/** + * Calls `callback` with the resolved value of `object`. + * In case `object` is a Promise, the result will also be a Promise, + * otherwise the result will be sync. + */ +export function resolvePromiseOrValue(object: PromiseOrValue, callback: (val: TIn) => TOut): +PromiseOrValue { + if (isPromise(object)) { + return object.then((val): TOut => callback(val)); + } + return callback(object); +} + // eslint-disable-next-line @typescript-eslint/no-empty-function function noop(): void {} diff --git a/src/util/StreamUtil.ts b/src/util/StreamUtil.ts index 306ec4eff..0831e0495 100644 --- a/src/util/StreamUtil.ts +++ b/src/util/StreamUtil.ts @@ -10,6 +10,7 @@ import { isHttpRequest } from '../server/HttpRequest'; import { InternalServerError } from './errors/InternalServerError'; import type { Guarded } from './GuardedStream'; import { guardStream } from './GuardedStream'; +import type { PromiseOrValue } from './PromiseUtil'; export const endOfStream = promisify(eos); @@ -119,12 +120,12 @@ export interface AsyncTransformOptions extends DuplexOptions { /** * Transforms data from the source by calling the `push` method */ - transform?: (this: Transform, data: T, encoding: string) => any | Promise; + transform?: (this: Transform, data: T, encoding: string) => PromiseOrValue; /** * Performs any final actions after the source has ended */ - flush?: (this: Transform) => any | Promise; + flush?: (this: Transform) => PromiseOrValue; } /** diff --git a/src/util/locking/EqualReadWriteLocker.ts b/src/util/locking/EqualReadWriteLocker.ts index d57b554c7..e381cd1e0 100644 --- a/src/util/locking/EqualReadWriteLocker.ts +++ b/src/util/locking/EqualReadWriteLocker.ts @@ -1,4 +1,5 @@ import type { ResourceIdentifier } from '../../http/representation/ResourceIdentifier'; +import type { PromiseOrValue } from '../PromiseUtil'; import type { ReadWriteLocker } from './ReadWriteLocker'; import type { ResourceLocker } from './ResourceLocker'; @@ -12,11 +13,11 @@ export class EqualReadWriteLocker implements ReadWriteLocker { this.locker = locker; } - public async withReadLock(identifier: ResourceIdentifier, whileLocked: () => (Promise | T)): Promise { + public async withReadLock(identifier: ResourceIdentifier, whileLocked: () => PromiseOrValue): Promise { return this.withLock(identifier, whileLocked); } - public async withWriteLock(identifier: ResourceIdentifier, whileLocked: () => (Promise | T)): Promise { + public async withWriteLock(identifier: ResourceIdentifier, whileLocked: () => PromiseOrValue): Promise { return this.withLock(identifier, whileLocked); } @@ -26,7 +27,7 @@ export class EqualReadWriteLocker implements ReadWriteLocker { * @param identifier - Identifier of resource that needs to be locked. * @param whileLocked - Function to resolve while the resource is locked. */ - private async withLock(identifier: ResourceIdentifier, whileLocked: () => T | Promise): Promise { + private async withLock(identifier: ResourceIdentifier, whileLocked: () => PromiseOrValue): Promise { await this.locker.acquire(identifier); try { return await whileLocked(); diff --git a/src/util/locking/ExpiringReadWriteLocker.ts b/src/util/locking/ExpiringReadWriteLocker.ts index 249dc8de0..fb9356e62 100644 --- a/src/util/locking/ExpiringReadWriteLocker.ts +++ b/src/util/locking/ExpiringReadWriteLocker.ts @@ -1,4 +1,5 @@ import type { ResourceIdentifier } from '../../http/representation/ResourceIdentifier'; +import type { PromiseOrValue } from '../PromiseUtil'; import type { ReadWriteLocker } from './ReadWriteLocker'; /** @@ -14,7 +15,7 @@ export interface ExpiringReadWriteLocker extends ReadWriteLocker { * @param whileLocked - A function to execute while the resource is locked. * Receives a callback as input parameter to maintain the lock. */ - withReadLock: (identifier: ResourceIdentifier, whileLocked: (maintainLock: () => void) => T | Promise) + withReadLock: (identifier: ResourceIdentifier, whileLocked: (maintainLock: () => void) => PromiseOrValue) => Promise; /** @@ -26,6 +27,6 @@ export interface ExpiringReadWriteLocker extends ReadWriteLocker { * @param whileLocked - A function to execute while the resource is locked. * Receives a callback as input parameter to maintain the lock. */ - withWriteLock: (identifier: ResourceIdentifier, whileLocked: (maintainLock: () => void) => T | Promise) + withWriteLock: (identifier: ResourceIdentifier, whileLocked: (maintainLock: () => void) => PromiseOrValue) => Promise; } diff --git a/src/util/locking/GreedyReadWriteLocker.ts b/src/util/locking/GreedyReadWriteLocker.ts index 86abfd0d5..5023db4e8 100644 --- a/src/util/locking/GreedyReadWriteLocker.ts +++ b/src/util/locking/GreedyReadWriteLocker.ts @@ -2,6 +2,7 @@ import type { ResourceIdentifier } from '../../http/representation/ResourceIdent import type { KeyValueStorage } from '../../storage/keyvalue/KeyValueStorage'; import { ForbiddenHttpError } from '../errors/ForbiddenHttpError'; import { InternalServerError } from '../errors/InternalServerError'; +import type { PromiseOrValue } from '../PromiseUtil'; import type { ReadWriteLocker } from './ReadWriteLocker'; import type { ResourceLocker } from './ResourceLocker'; @@ -43,7 +44,7 @@ export class GreedyReadWriteLocker implements ReadWriteLocker { this.suffixes = suffixes; } - public async withReadLock(identifier: ResourceIdentifier, whileLocked: () => (Promise | T)): Promise { + public async withReadLock(identifier: ResourceIdentifier, whileLocked: () => PromiseOrValue): Promise { await this.preReadSetup(identifier); try { return await whileLocked(); @@ -52,7 +53,7 @@ export class GreedyReadWriteLocker implements ReadWriteLocker { } } - public async withWriteLock(identifier: ResourceIdentifier, whileLocked: () => (Promise | T)): Promise { + public async withWriteLock(identifier: ResourceIdentifier, whileLocked: () => PromiseOrValue): Promise { if (identifier.path.endsWith(`.${this.suffixes.count}`)) { throw new ForbiddenHttpError('This resource is used for internal purposes.'); } @@ -117,7 +118,7 @@ export class GreedyReadWriteLocker implements ReadWriteLocker { /** * Safely runs an action on the count. */ - private async withInternalReadLock(identifier: ResourceIdentifier, whileLocked: () => (Promise | T)): + private async withInternalReadLock(identifier: ResourceIdentifier, whileLocked: () => PromiseOrValue): Promise { const read = this.getReadLockKey(identifier); await this.locker.acquire(read); diff --git a/src/util/locking/ReadWriteLocker.ts b/src/util/locking/ReadWriteLocker.ts index eee10b3e3..8fc0a9b49 100644 --- a/src/util/locking/ReadWriteLocker.ts +++ b/src/util/locking/ReadWriteLocker.ts @@ -1,4 +1,5 @@ import type { ResourceIdentifier } from '../../http/representation/ResourceIdentifier'; +import type { PromiseOrValue } from '../PromiseUtil'; /** * Allows the locking of resources which is needed for non-atomic {@link ResourceStore}s. @@ -14,7 +15,7 @@ export interface ReadWriteLocker { * * @returns A promise resolving when the lock is released. */ - withReadLock: (identifier: ResourceIdentifier, whileLocked: () => T | Promise) => Promise; + withReadLock: (identifier: ResourceIdentifier, whileLocked: () => PromiseOrValue) => Promise; /** * Run the given function while the resource is locked. @@ -26,5 +27,5 @@ export interface ReadWriteLocker { * * @returns A promise resolving when the lock is released. */ - withWriteLock: (identifier: ResourceIdentifier, whileLocked: () => T | Promise) => Promise; + withWriteLock: (identifier: ResourceIdentifier, whileLocked: () => PromiseOrValue) => Promise; } diff --git a/src/util/locking/RedisLocker.ts b/src/util/locking/RedisLocker.ts index d2bff1bfc..5d0843fa8 100644 --- a/src/util/locking/RedisLocker.ts +++ b/src/util/locking/RedisLocker.ts @@ -5,6 +5,7 @@ import type { Initializable } from '../../init/Initializable'; import { getLoggerFor } from '../../logging/LogUtil'; import type { AttemptSettings } from '../LockUtils'; import { retryFunction } from '../LockUtils'; +import type { PromiseOrValue } from '../PromiseUtil'; import type { ReadWriteLocker } from './ReadWriteLocker'; import type { ResourceLocker } from './ResourceLocker'; import type { RedisResourceLock, RedisReadWriteLock, RedisAnswer } from './scripts/RedisLuaScripts'; @@ -127,7 +128,7 @@ export class RedisLocker implements ReadWriteLocker, ResourceLocker, Initializab }; } - public async withReadLock(identifier: ResourceIdentifier, whileLocked: () => (Promise | T)): Promise { + public async withReadLock(identifier: ResourceIdentifier, whileLocked: () => PromiseOrValue): Promise { const key = this.getReadWriteKey(identifier); await retryFunction( this.swallowFalse(this.redisRw.acquireReadLock.bind(this.redisRw, key)), @@ -143,7 +144,7 @@ export class RedisLocker implements ReadWriteLocker, ResourceLocker, Initializab } } - public async withWriteLock(identifier: ResourceIdentifier, whileLocked: () => (Promise | T)): Promise { + public async withWriteLock(identifier: ResourceIdentifier, whileLocked: () => PromiseOrValue): Promise { const key = this.getReadWriteKey(identifier); await retryFunction( this.swallowFalse(this.redisRw.acquireWriteLock.bind(this.redisRw, key)), diff --git a/src/util/locking/VoidLocker.ts b/src/util/locking/VoidLocker.ts index bc79f792e..2d976737d 100644 --- a/src/util/locking/VoidLocker.ts +++ b/src/util/locking/VoidLocker.ts @@ -1,5 +1,6 @@ import type { ResourceIdentifier } from '../../http/representation/ResourceIdentifier'; import { getLoggerFor } from '../../logging/LogUtil'; +import type { PromiseOrValue } from '../PromiseUtil'; import type { ExpiringReadWriteLocker } from './ExpiringReadWriteLocker'; /** @@ -20,14 +21,14 @@ export class VoidLocker implements ExpiringReadWriteLocker { public async withReadLock( identifier: ResourceIdentifier, - whileLocked: (maintainLock: () => void) => T | Promise, + whileLocked: (maintainLock: () => void) => PromiseOrValue, ): Promise { return whileLocked(noop); } public async withWriteLock( identifier: ResourceIdentifier, - whileLocked: (maintainLock: () => void) => T | Promise, + whileLocked: (maintainLock: () => void) => PromiseOrValue, ): Promise { return whileLocked(noop); } diff --git a/src/util/locking/WrappedExpiringReadWriteLocker.ts b/src/util/locking/WrappedExpiringReadWriteLocker.ts index 6bac5c8be..613f882fd 100644 --- a/src/util/locking/WrappedExpiringReadWriteLocker.ts +++ b/src/util/locking/WrappedExpiringReadWriteLocker.ts @@ -1,6 +1,7 @@ import type { ResourceIdentifier } from '../../http/representation/ResourceIdentifier'; import { getLoggerFor } from '../../logging/LogUtil'; import { InternalServerError } from '../errors/InternalServerError'; +import type { PromiseOrValue } from '../PromiseUtil'; import type { ExpiringReadWriteLocker } from './ExpiringReadWriteLocker'; import type { ReadWriteLocker } from './ReadWriteLocker'; import Timeout = NodeJS.Timeout; @@ -24,12 +25,12 @@ export class WrappedExpiringReadWriteLocker implements ExpiringReadWriteLocker { } public async withReadLock(identifier: ResourceIdentifier, - whileLocked: (maintainLock: () => void) => T | Promise): Promise { + whileLocked: (maintainLock: () => void) => PromiseOrValue): Promise { return this.locker.withReadLock(identifier, async(): Promise => this.expiringPromise(identifier, whileLocked)); } public async withWriteLock(identifier: ResourceIdentifier, - whileLocked: (maintainLock: () => void) => T | Promise): Promise { + whileLocked: (maintainLock: () => void) => PromiseOrValue): Promise { return this.locker.withWriteLock(identifier, async(): Promise => this.expiringPromise(identifier, whileLocked)); } @@ -39,7 +40,7 @@ export class WrappedExpiringReadWriteLocker implements ExpiringReadWriteLocker { * it receives. The ResourceIdentifier is only used for logging. */ private async expiringPromise(identifier: ResourceIdentifier, - whileLocked: (maintainLock: () => void) => T | Promise): Promise { + whileLocked: (maintainLock: () => void) => PromiseOrValue): Promise { let timer: Timeout; let createTimeout: () => Timeout; diff --git a/src/util/map/MapUtil.ts b/src/util/map/MapUtil.ts index 07d9af6ca..b26ee8939 100644 --- a/src/util/map/MapUtil.ts +++ b/src/util/map/MapUtil.ts @@ -1,3 +1,5 @@ +import { resolvePromiseOrValue } from '../PromiseUtil'; +import type { PromiseOrValue } from '../PromiseUtil'; import type { SetMultiMap } from './SetMultiMap'; export type MapKey = T extends Map ? TKey : never; @@ -42,18 +44,33 @@ export function modify>(map: T, options: ModifyO /** * Finds the result of calling `map.get(key)`. - * If there is no result, it instead returns the default value. + * If there is no result, it instead returns the result of the default function. * The Map will also be updated to assign that default value to the given key. * * @param map - Map to use. * @param key - Key to find the value for. - * @param defaultValue - Value to insert and return if no result was found. + * @param defaultFn - Function to generate default value to insert and return if no result was found. */ -export function getDefault(map: Map, key: TKey, defaultValue: TValue): TValue { +export function getDefault(map: Map, key: TKey, defaultFn: () => TValue): TValue; +/** + * Finds the result of calling `map.get(key)`. + * If there is no result, it instead returns the result of the default function. + * The Map will also be updated to assign the resolved default value to the given key. + * + * @param map - Map to use. + * @param key - Key to find the value for. + * @param defaultFn - Function to generate default value to insert and return if no result was found. + */ +export function getDefault(map: Map, key: TKey, defaultFn: () => Promise): +Promise; +export function getDefault(map: Map, key: TKey, defaultFn: () => PromiseOrValue): +PromiseOrValue { const value = map.get(key); if (value) { return value; } - map.set(key, defaultValue); - return defaultValue; + return resolvePromiseOrValue(defaultFn(), (val): TValue => { + map.set(key, val); + return val; + }); } diff --git a/test/unit/storage/LockingResourceStore.test.ts b/test/unit/storage/LockingResourceStore.test.ts index 730eccfe0..009d2b74b 100644 --- a/test/unit/storage/LockingResourceStore.test.ts +++ b/test/unit/storage/LockingResourceStore.test.ts @@ -6,6 +6,7 @@ import type { ResourceIdentifier } from '../../../src/http/representation/Resour import { LockingResourceStore } from '../../../src/storage/LockingResourceStore'; import type { ResourceStore } from '../../../src/storage/ResourceStore'; import type { ExpiringReadWriteLocker } from '../../../src/util/locking/ExpiringReadWriteLocker'; +import type { PromiseOrValue } from '../../../src/util/PromiseUtil'; import { guardedStreamFrom } from '../../../src/util/StreamUtil'; import { flushPromises } from '../../util/Util'; @@ -47,7 +48,7 @@ describe('A LockingResourceStore', (): void => { locker = { withReadLock: jest.fn(async (id: ResourceIdentifier, - whileLocked: (maintainLock: () => void) => T | Promise): Promise => { + whileLocked: (maintainLock: () => void) => PromiseOrValue): Promise => { order.push('lock read'); try { // Allows simulating a timeout event @@ -61,7 +62,7 @@ describe('A LockingResourceStore', (): void => { } }), withWriteLock: jest.fn(async (identifier: ResourceIdentifier, - whileLocked: (maintainLock: () => void) => T | Promise): Promise => { + whileLocked: (maintainLock: () => void) => PromiseOrValue): Promise => { order.push('lock write'); try { return await whileLocked(emptyFn); diff --git a/test/unit/util/locking/WrappedExpiringReadWriteLocker.test.ts b/test/unit/util/locking/WrappedExpiringReadWriteLocker.test.ts index 05602e425..9f98bc447 100644 --- a/test/unit/util/locking/WrappedExpiringReadWriteLocker.test.ts +++ b/test/unit/util/locking/WrappedExpiringReadWriteLocker.test.ts @@ -1,6 +1,7 @@ import type { ResourceIdentifier } from '../../../../src/http/representation/ResourceIdentifier'; import type { ReadWriteLocker } from '../../../../src/util/locking/ReadWriteLocker'; import { WrappedExpiringReadWriteLocker } from '../../../../src/util/locking/WrappedExpiringReadWriteLocker'; +import type { PromiseOrValue } from '../../../../src/util/PromiseUtil'; jest.useFakeTimers(); @@ -14,9 +15,9 @@ describe('A WrappedExpiringReadWriteLocker', (): void => { beforeEach(async(): Promise => { wrappedLocker = { - withReadLock: jest.fn(async(id: ResourceIdentifier, whileLocked: () => T | Promise): + withReadLock: jest.fn(async(id: ResourceIdentifier, whileLocked: () => PromiseOrValue): Promise => whileLocked()), - withWriteLock: jest.fn(async(id: ResourceIdentifier, whileLocked: () => T | Promise): + withWriteLock: jest.fn(async(id: ResourceIdentifier, whileLocked: () => PromiseOrValue): Promise => whileLocked()), }; diff --git a/test/unit/util/map/MapUtil.test.ts b/test/unit/util/map/MapUtil.test.ts index d9fcc808e..4d652e433 100644 --- a/test/unit/util/map/MapUtil.test.ts +++ b/test/unit/util/map/MapUtil.test.ts @@ -44,12 +44,17 @@ describe('MapUtil', (): void => { describe('#getDefault', (): void => { it('returns the value it finds in the Map for the given key.', async(): Promise => { const map = new Map([[ key1, 123 ]]); - expect(getDefault(map, key1, 999)).toBe(123); + expect(getDefault(map, key1, (): number => 999)).toBe(123); }); it('returns the default value if it finds no value for the given key.', async(): Promise => { const map = new Map([[ key1, 123 ]]); - expect(getDefault(map, key2, 999)).toBe(999); + expect(getDefault(map, key2, (): number => 999)).toBe(999); + }); + + it('can handle async default functions.', async(): Promise => { + const map = new Map([[ key1, 123 ]]); + await expect(getDefault(map, key2, async(): Promise => 999)).resolves.toBe(999); }); }); });