From e2e2d0864f9b81c56fea61067acdc011347b2283 Mon Sep 17 00:00:00 2001 From: Thomas Dupont Date: Tue, 19 Apr 2022 11:50:13 +0200 Subject: [PATCH] feat: add a Redis based Read-Write Locker refactor: more elegant way of providing default attemptSettings to constructor style(jsdoc): rewording of jsdoc comment fix: RegExp(/regex/) => /regex/ fix: Replace Error with InternalServerError docs: jsdoc for RedisReadWriteLocker class feat: make RedisReadWriteLocker a ResourceLocker too test: coverage back to 100% refactor: linting fix style(jsdoc): Add explanation to tryRedisFn() method refactor: remove RedisResourceLocker fix: bug in lua script chore(deps): update ioredis, remove redlock refactor: removed RedisResourceLocker in favor of generic RedisLocker class test: add redis lua scripts tests and integrate all 3 redis integration tests in 1 refactor: remove .vscode folder from index refactor: Add some typing and change redis references to Redis in comments refactor: more changes after PR review refactor: remove redis.json refactor: rename redis-rw.json to redis.json docs: added readme and release notes --- RELEASE_NOTES.md | 4 + config/util/README.md | 2 +- config/util/resource-locker/redis.json | 22 +- package-lock.json | 173 ++---- package.json | 5 +- src/index.ts | 2 +- src/util/locking/RedisLocker.ts | 204 +++++++ src/util/locking/RedisResourceLocker.ts | 178 ------ src/util/locking/scripts/RedisLuaScripts.ts | 151 +++++ test/integration/RedisLocker.test.ts | 402 +++++++++++++ .../RedisResourceLockerIntegration.test.ts | 164 ------ ...th-redlock.json => server-redis-lock.json} | 9 +- test/unit/util/locking/RedisLocker.test.ts | 539 ++++++++++++++++++ .../util/locking/RedisResourceLocker.test.ts | 205 ------- test/util/Util.ts | 2 +- 15 files changed, 1357 insertions(+), 705 deletions(-) create mode 100644 src/util/locking/RedisLocker.ts delete mode 100644 src/util/locking/RedisResourceLocker.ts create mode 100644 src/util/locking/scripts/RedisLuaScripts.ts create mode 100644 test/integration/RedisLocker.test.ts delete mode 100644 test/integration/RedisResourceLockerIntegration.test.ts rename test/integration/config/{run-with-redlock.json => server-redis-lock.json} (89%) create mode 100644 test/unit/util/locking/RedisLocker.test.ts delete mode 100644 test/unit/util/locking/RedisResourceLocker.test.ts diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 8888320d3..ab1b0c668 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -10,6 +10,8 @@ - A new default configuration `config/https-file-cli.json` that can set the HTTPS parameters through the CLI has been added. This is also an example of how to add CLI parameters through a custom configuration. +- A new RedisLocker has been added to replace the old RedisResourceLocker class. + It allows for true threadsafe read/write locking. ### Configuration changes You might need to make changes to your v3 configuration if you use a custom config. @@ -39,6 +41,8 @@ These changes are relevant if you wrote custom modules for the server that depen - `RepresentationMetadata` no longer accepts strings for predicates in any of its functions. - `CombinedSettingsResolver` parameter `computers` has been renamed to `resolvers`. - `IdentityProviderFactory` requires an additional `credentialStorage` parameter. +- The `RedisResourceLocker` class has been removed and the `RedisLocker`class was added instead. + `RedisLocker` implements both the `ResourceLocker` and `ReadWriteLocker` interface. ## v3.0.0 ### New features diff --git a/config/util/README.md b/config/util/README.md index 5dc8dc3c2..4e407af5e 100644 --- a/config/util/README.md +++ b/config/util/README.md @@ -38,7 +38,7 @@ to the ChainedConverter list. Which locking mechanism to use to for example prevent 2 write simultaneous write requests. * *debug-void*: No locking mechanism, does not prevent simultaneous read/writes. * *memory*: Uses an in-memory locking mechanism. -* *redis*: Uses a Redis store for locking. +* *redis*: Uses a Redis store for locking that supports threadsafe read-write locking. ## Variables Various variables used by other options. diff --git a/config/util/resource-locker/redis.json b/config/util/resource-locker/redis.json index 9f3ee717e..5fc450790 100644 --- a/config/util/resource-locker/redis.json +++ b/config/util/resource-locker/redis.json @@ -2,27 +2,23 @@ "@context": "https://linkedsoftwaredependencies.org/bundles/npm/@solid/community-server/^3.0.0/components/context.jsonld", "@graph": [ { - "comment": "Allows multiple simultaneous read operations. Locks are stored in memory. Locks expire after inactivity.", + "comment": "Allows multiple simultaneous read operations. All locks are threadsafe.", "@id": "urn:solid-server:default:ResourceLocker", "@type": "WrappedExpiringReadWriteLocker", "locker": { - "@type": "GreedyReadWriteLocker", - "locker": { - "@id": "urn:solid-server:default:RedisResourceLocker", - "@type": "RedisResourceLocker", - "redisClients": [ "6379" ] - }, - "storage": { "@id": "urn:solid-server:default:LockStorage" }, - "suffixes_count": "count", - "suffixes_read": "read", - "suffixes_write": "write" + "@id": "urn:solid-server:default:RedisLocker", + "@type": "RedisLocker" }, "expiration": 3000 }, { - "comment": "Makes sure the redis connection is closed when the application needs to stop.", + "comment": "Makes sure the redis connection is closed when the application needs to stop. Also deletes still-existing locks and counters.", "@id": "urn:solid-server:default:Finalizer", - "ParallelFinalizer:_finalizers": [ { "@id": "urn:solid-server:default:RedisResourceLocker" } ] + "ParallelFinalizer:_finalizers": [ + { + "@id": "urn:solid-server:default:RedisLocker" + } + ] } ] } diff --git a/package-lock.json b/package-lock.json index 0e0b5801e..b7f744dfe 100644 --- a/package-lock.json +++ b/package-lock.json @@ -16,7 +16,6 @@ "@types/cors": "^2.8.12", "@types/end-of-stream": "^1.4.1", "@types/fs-extra": "^9.0.13", - "@types/ioredis": "^4.28.10", "@types/lodash.orderby": "^4.6.6", "@types/marked": "^4.0.2", "@types/mime-types": "^2.1.1", @@ -26,7 +25,6 @@ "@types/oidc-provider": "^7.8.1", "@types/pump": "^1.1.1", "@types/punycode": "^2.1.0", - "@types/redlock": "^4.0.1", "@types/sparqljs": "^3.1.3", "@types/url-join": "^4.0.1", "@types/uuid": "^8.3.4", @@ -44,7 +42,7 @@ "fetch-sparql-endpoint": "^2.4.0", "fs-extra": "^10.0.0", "handlebars": "^4.7.7", - "ioredis": "^4.28.5", + "ioredis": "^5.0.4", "jose": "^4.4.0", "lodash.orderby": "^4.6.0", "marked": "^4.0.12", @@ -58,7 +56,6 @@ "rdf-parse": "^1.9.1", "rdf-serialize": "^1.2.0", "rdf-terms": "^1.7.1", - "redlock": "^4.2.0", "sparqlalgebrajs": "^4.0.2", "sparqljs": "^3.5.1", "url-join": "^4.0.1", @@ -3776,6 +3773,11 @@ "integrity": "sha512-LcImhJqqPsNl/OlULzEEK2rYevty0eh1zaOLVz3lnydEU1DQkeaJ8fKBxKdp5/QjCtnIYcaDjh5U11PGh29Dgg==", "dev": true }, + "node_modules/@ioredis/commands": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/@ioredis/commands/-/commands-1.1.1.tgz", + "integrity": "sha512-fsR4P/ROllzf/7lXYyElUJCheWdTJVJvOTps8v9IWKFATxR61ANOlnoPqhH099xYLrJGpc2ZQ28B3rMeUt5VQg==" + }, "node_modules/@istanbuljs/load-nyc-config": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/@istanbuljs/load-nyc-config/-/load-nyc-config-1.1.0.tgz", @@ -4319,11 +4321,6 @@ "@types/node": "*" } }, - "node_modules/@types/bluebird": { - "version": "3.5.33", - "resolved": "https://registry.npmjs.org/@types/bluebird/-/bluebird-3.5.33.tgz", - "integrity": "sha512-ndEo1xvnYeHxm7I/5sF6tBvnsA4Tdi3zj1keRKRs12SP+2ye2A27NDJ1B6PqkfMbGAcT+mqQVqbZRIrhfOp5PQ==" - }, "node_modules/@types/body-parser": { "version": "1.19.0", "resolved": "https://registry.npmjs.org/@types/body-parser/-/body-parser-1.19.0.tgz", @@ -4463,14 +4460,6 @@ "@types/node": "*" } }, - "node_modules/@types/ioredis": { - "version": "4.28.10", - "resolved": "https://registry.npmjs.org/@types/ioredis/-/ioredis-4.28.10.tgz", - "integrity": "sha512-69LyhUgrXdgcNDv7ogs1qXZomnfOEnSmrmMFqKgt1XMJxmoOSG/u3wYy13yACIfKuMJ8IhKgHafDO3sx19zVQQ==", - "dependencies": { - "@types/node": "*" - } - }, "node_modules/@types/istanbul-lib-coverage": { "version": "2.0.3", "resolved": "https://registry.npmjs.org/@types/istanbul-lib-coverage/-/istanbul-lib-coverage-2.0.3.tgz", @@ -4694,14 +4683,6 @@ "safe-buffer": "*" } }, - "node_modules/@types/redlock": { - "version": "4.0.1", - "resolved": "https://registry.npmjs.org/@types/redlock/-/redlock-4.0.1.tgz", - "integrity": "sha512-YrPEPHOgLlWtsg/Ocv/gthGDQpx3HXFPhBvCNuBKBKUoMBzvCjCTC95dmJ0WLZgO+fgTxGQFyk6ZO/+/zG5mRg==", - "dependencies": { - "@types/bluebird": "*" - } - }, "node_modules/@types/responselike": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/@types/responselike/-/responselike-1.0.0.tgz", @@ -5757,11 +5738,6 @@ "node": ">=8" } }, - "node_modules/bluebird": { - "version": "3.7.2", - "resolved": "https://registry.npmjs.org/bluebird/-/bluebird-3.7.2.tgz", - "integrity": "sha512-XpNj6GDQzdfW+r2Wnn7xiSAd7TM3jzkxGXBGTtWKuSXv1xUV+azxAm8jdWZN06QTQk+2N2XB9jRDkvbmQmcRtg==" - }, "node_modules/boolbase": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/boolbase/-/boolbase-1.0.0.tgz", @@ -6663,9 +6639,9 @@ } }, "node_modules/debug": { - "version": "4.3.3", - "resolved": "https://registry.npmjs.org/debug/-/debug-4.3.3.tgz", - "integrity": "sha512-/zxw5+vh1Tfv+4Qn7a5nsbcJKPaSvCDhojn6FEl9vupwK2VCSDtEiEtqr8DFtzYFOdz63LBkxec7DYuc2jon6Q==", + "version": "4.3.4", + "resolved": "https://registry.npmjs.org/debug/-/debug-4.3.4.tgz", + "integrity": "sha512-PRWFHuSU3eDtQJPvnNY7Jcket1j0t5OuOsFzPPzsekD52Zl8qUfFIPEiswXqIvHWGVHOgX+7G/vCNNhehwxfkQ==", "dependencies": { "ms": "2.1.2" }, @@ -6799,9 +6775,9 @@ "integrity": "sha1-hMbhWbgZBP3KWaDvRM2HDTElD5o=" }, "node_modules/denque": { - "version": "1.5.0", - "resolved": "https://registry.npmjs.org/denque/-/denque-1.5.0.tgz", - "integrity": "sha512-CYiCSgIF1p6EUByQPlGkKnP1M9g0ZV3qMIrqMqZqdwazygIA/YP2vrbcyl1h/WppKJTdl1F85cXIle+394iDAQ==", + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/denque/-/denque-2.0.1.tgz", + "integrity": "sha512-tfiWc6BQLXNLpNiR5iGd0Ocu3P3VpxfzFiqubLgMfhfOw9WyvgJBd46CClNn9k3qfbjvT//0cf7AlYRX/OslMQ==", "engines": { "node": ">=0.10" } @@ -9510,24 +9486,22 @@ } }, "node_modules/ioredis": { - "version": "4.28.5", - "resolved": "https://registry.npmjs.org/ioredis/-/ioredis-4.28.5.tgz", - "integrity": "sha512-3GYo0GJtLqgNXj4YhrisLaNNvWSNwSS2wS4OELGfGxH8I69+XfNdnmV1AyN+ZqMh0i7eX+SWjrwFKDBDgfBC1A==", + "version": "5.0.4", + "resolved": "https://registry.npmjs.org/ioredis/-/ioredis-5.0.4.tgz", + "integrity": "sha512-qFJw3MnPNsJF1lcIOP3vztbsasOXK3nDdNAgjQj7t7/Bn/w10PGchTOpqylQNxjzPbLoYDu34LjeJtSWiKBntQ==", "dependencies": { + "@ioredis/commands": "^1.1.1", "cluster-key-slot": "^1.1.0", - "debug": "^4.3.1", - "denque": "^1.1.0", + "debug": "^4.3.4", + "denque": "^2.0.1", "lodash.defaults": "^4.2.0", - "lodash.flatten": "^4.4.0", "lodash.isarguments": "^3.1.0", - "p-map": "^2.1.0", - "redis-commands": "1.7.0", "redis-errors": "^1.2.0", "redis-parser": "^3.0.0", "standard-as-callback": "^2.1.0" }, "engines": { - "node": ">=6" + "node": ">=12.22.0" }, "funding": { "type": "opencollective", @@ -11176,11 +11150,6 @@ "resolved": "https://registry.npmjs.org/lodash.defaults/-/lodash.defaults-4.2.0.tgz", "integrity": "sha1-0JF4cW/+pN3p5ft7N/bwgCJ0WAw=" }, - "node_modules/lodash.flatten": { - "version": "4.4.0", - "resolved": "https://registry.npmjs.org/lodash.flatten/-/lodash.flatten-4.4.0.tgz", - "integrity": "sha1-8xwiIlqWMtK7+OSt2+8kCqdlph8=" - }, "node_modules/lodash.isarguments": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/lodash.isarguments/-/lodash.isarguments-3.1.0.tgz", @@ -12404,14 +12373,6 @@ "node": ">=8" } }, - "node_modules/p-map": { - "version": "2.1.0", - "resolved": "https://registry.npmjs.org/p-map/-/p-map-2.1.0.tgz", - "integrity": "sha512-y3b8Kpd8OAN444hxfBbFfj1FY/RjtTd8tzYwhUqNYXx0fXx2iX4maP4Qr6qhIKbQXI02wTLAda4fYUbDagTUFw==", - "engines": { - "node": ">=6" - } - }, "node_modules/p-try": { "version": "2.2.0", "resolved": "https://registry.npmjs.org/p-try/-/p-try-2.2.0.tgz", @@ -13176,11 +13137,6 @@ "node": ">=8" } }, - "node_modules/redis-commands": { - "version": "1.7.0", - "resolved": "https://registry.npmjs.org/redis-commands/-/redis-commands-1.7.0.tgz", - "integrity": "sha512-nJWqw3bTFy21hX/CPKHth6sfhZbdiHP6bTawSgQBlKOVRG7EZkfHbbHwQJnrE4vsQf0CMNE+3gJ4Fmm16vdVlQ==" - }, "node_modules/redis-errors": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/redis-errors/-/redis-errors-1.2.0.tgz", @@ -13200,17 +13156,6 @@ "node": ">=4" } }, - "node_modules/redlock": { - "version": "4.2.0", - "resolved": "https://registry.npmjs.org/redlock/-/redlock-4.2.0.tgz", - "integrity": "sha512-j+oQlG+dOwcetUt2WJWttu4CZVeRzUrcVcISFmEmfyuwCVSJ93rDT7YSgg7H7rnxwoRyk/jU46kycVka5tW7jA==", - "dependencies": { - "bluebird": "^3.7.2" - }, - "engines": { - "node": ">=8.0.0" - } - }, "node_modules/regexp-tree": { "version": "0.1.24", "resolved": "https://registry.npmjs.org/regexp-tree/-/regexp-tree-0.1.24.tgz", @@ -18016,6 +17961,11 @@ "integrity": "sha512-LcImhJqqPsNl/OlULzEEK2rYevty0eh1zaOLVz3lnydEU1DQkeaJ8fKBxKdp5/QjCtnIYcaDjh5U11PGh29Dgg==", "dev": true }, + "@ioredis/commands": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/@ioredis/commands/-/commands-1.1.1.tgz", + "integrity": "sha512-fsR4P/ROllzf/7lXYyElUJCheWdTJVJvOTps8v9IWKFATxR61ANOlnoPqhH099xYLrJGpc2ZQ28B3rMeUt5VQg==" + }, "@istanbuljs/load-nyc-config": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/@istanbuljs/load-nyc-config/-/load-nyc-config-1.1.0.tgz", @@ -18474,11 +18424,6 @@ "@types/node": "*" } }, - "@types/bluebird": { - "version": "3.5.33", - "resolved": "https://registry.npmjs.org/@types/bluebird/-/bluebird-3.5.33.tgz", - "integrity": "sha512-ndEo1xvnYeHxm7I/5sF6tBvnsA4Tdi3zj1keRKRs12SP+2ye2A27NDJ1B6PqkfMbGAcT+mqQVqbZRIrhfOp5PQ==" - }, "@types/body-parser": { "version": "1.19.0", "resolved": "https://registry.npmjs.org/@types/body-parser/-/body-parser-1.19.0.tgz", @@ -18618,14 +18563,6 @@ "@types/node": "*" } }, - "@types/ioredis": { - "version": "4.28.10", - "resolved": "https://registry.npmjs.org/@types/ioredis/-/ioredis-4.28.10.tgz", - "integrity": "sha512-69LyhUgrXdgcNDv7ogs1qXZomnfOEnSmrmMFqKgt1XMJxmoOSG/u3wYy13yACIfKuMJ8IhKgHafDO3sx19zVQQ==", - "requires": { - "@types/node": "*" - } - }, "@types/istanbul-lib-coverage": { "version": "2.0.3", "resolved": "https://registry.npmjs.org/@types/istanbul-lib-coverage/-/istanbul-lib-coverage-2.0.3.tgz", @@ -18848,14 +18785,6 @@ "safe-buffer": "*" } }, - "@types/redlock": { - "version": "4.0.1", - "resolved": "https://registry.npmjs.org/@types/redlock/-/redlock-4.0.1.tgz", - "integrity": "sha512-YrPEPHOgLlWtsg/Ocv/gthGDQpx3HXFPhBvCNuBKBKUoMBzvCjCTC95dmJ0WLZgO+fgTxGQFyk6ZO/+/zG5mRg==", - "requires": { - "@types/bluebird": "*" - } - }, "@types/responselike": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/@types/responselike/-/responselike-1.0.0.tgz", @@ -19613,11 +19542,6 @@ "integrity": "sha512-jDctJ/IVQbZoJykoeHbhXpOlNBqGNcwXJKJog42E5HDPUwQTSdjCHdihjj0DlnheQ7blbT6dHOafNAiS8ooQKA==", "dev": true }, - "bluebird": { - "version": "3.7.2", - "resolved": "https://registry.npmjs.org/bluebird/-/bluebird-3.7.2.tgz", - "integrity": "sha512-XpNj6GDQzdfW+r2Wnn7xiSAd7TM3jzkxGXBGTtWKuSXv1xUV+azxAm8jdWZN06QTQk+2N2XB9jRDkvbmQmcRtg==" - }, "boolbase": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/boolbase/-/boolbase-1.0.0.tgz", @@ -20324,9 +20248,9 @@ } }, "debug": { - "version": "4.3.3", - "resolved": "https://registry.npmjs.org/debug/-/debug-4.3.3.tgz", - "integrity": "sha512-/zxw5+vh1Tfv+4Qn7a5nsbcJKPaSvCDhojn6FEl9vupwK2VCSDtEiEtqr8DFtzYFOdz63LBkxec7DYuc2jon6Q==", + "version": "4.3.4", + "resolved": "https://registry.npmjs.org/debug/-/debug-4.3.4.tgz", + "integrity": "sha512-PRWFHuSU3eDtQJPvnNY7Jcket1j0t5OuOsFzPPzsekD52Zl8qUfFIPEiswXqIvHWGVHOgX+7G/vCNNhehwxfkQ==", "requires": { "ms": "2.1.2" }, @@ -20432,9 +20356,9 @@ "integrity": "sha1-hMbhWbgZBP3KWaDvRM2HDTElD5o=" }, "denque": { - "version": "1.5.0", - "resolved": "https://registry.npmjs.org/denque/-/denque-1.5.0.tgz", - "integrity": "sha512-CYiCSgIF1p6EUByQPlGkKnP1M9g0ZV3qMIrqMqZqdwazygIA/YP2vrbcyl1h/WppKJTdl1F85cXIle+394iDAQ==" + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/denque/-/denque-2.0.1.tgz", + "integrity": "sha512-tfiWc6BQLXNLpNiR5iGd0Ocu3P3VpxfzFiqubLgMfhfOw9WyvgJBd46CClNn9k3qfbjvT//0cf7AlYRX/OslMQ==" }, "depd": { "version": "1.1.2", @@ -22449,18 +22373,16 @@ } }, "ioredis": { - "version": "4.28.5", - "resolved": "https://registry.npmjs.org/ioredis/-/ioredis-4.28.5.tgz", - "integrity": "sha512-3GYo0GJtLqgNXj4YhrisLaNNvWSNwSS2wS4OELGfGxH8I69+XfNdnmV1AyN+ZqMh0i7eX+SWjrwFKDBDgfBC1A==", + "version": "5.0.4", + "resolved": "https://registry.npmjs.org/ioredis/-/ioredis-5.0.4.tgz", + "integrity": "sha512-qFJw3MnPNsJF1lcIOP3vztbsasOXK3nDdNAgjQj7t7/Bn/w10PGchTOpqylQNxjzPbLoYDu34LjeJtSWiKBntQ==", "requires": { + "@ioredis/commands": "^1.1.1", "cluster-key-slot": "^1.1.0", - "debug": "^4.3.1", - "denque": "^1.1.0", + "debug": "^4.3.4", + "denque": "^2.0.1", "lodash.defaults": "^4.2.0", - "lodash.flatten": "^4.4.0", "lodash.isarguments": "^3.1.0", - "p-map": "^2.1.0", - "redis-commands": "1.7.0", "redis-errors": "^1.2.0", "redis-parser": "^3.0.0", "standard-as-callback": "^2.1.0" @@ -23731,11 +23653,6 @@ "resolved": "https://registry.npmjs.org/lodash.defaults/-/lodash.defaults-4.2.0.tgz", "integrity": "sha1-0JF4cW/+pN3p5ft7N/bwgCJ0WAw=" }, - "lodash.flatten": { - "version": "4.4.0", - "resolved": "https://registry.npmjs.org/lodash.flatten/-/lodash.flatten-4.4.0.tgz", - "integrity": "sha1-8xwiIlqWMtK7+OSt2+8kCqdlph8=" - }, "lodash.isarguments": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/lodash.isarguments/-/lodash.isarguments-3.1.0.tgz", @@ -24659,11 +24576,6 @@ "p-limit": "^2.2.0" } }, - "p-map": { - "version": "2.1.0", - "resolved": "https://registry.npmjs.org/p-map/-/p-map-2.1.0.tgz", - "integrity": "sha512-y3b8Kpd8OAN444hxfBbFfj1FY/RjtTd8tzYwhUqNYXx0fXx2iX4maP4Qr6qhIKbQXI02wTLAda4fYUbDagTUFw==" - }, "p-try": { "version": "2.2.0", "resolved": "https://registry.npmjs.org/p-try/-/p-try-2.2.0.tgz", @@ -25282,11 +25194,6 @@ "strip-indent": "^3.0.0" } }, - "redis-commands": { - "version": "1.7.0", - "resolved": "https://registry.npmjs.org/redis-commands/-/redis-commands-1.7.0.tgz", - "integrity": "sha512-nJWqw3bTFy21hX/CPKHth6sfhZbdiHP6bTawSgQBlKOVRG7EZkfHbbHwQJnrE4vsQf0CMNE+3gJ4Fmm16vdVlQ==" - }, "redis-errors": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/redis-errors/-/redis-errors-1.2.0.tgz", @@ -25300,14 +25207,6 @@ "redis-errors": "^1.0.0" } }, - "redlock": { - "version": "4.2.0", - "resolved": "https://registry.npmjs.org/redlock/-/redlock-4.2.0.tgz", - "integrity": "sha512-j+oQlG+dOwcetUt2WJWttu4CZVeRzUrcVcISFmEmfyuwCVSJ93rDT7YSgg7H7rnxwoRyk/jU46kycVka5tW7jA==", - "requires": { - "bluebird": "^3.7.2" - } - }, "regexp-tree": { "version": "0.1.24", "resolved": "https://registry.npmjs.org/regexp-tree/-/regexp-tree-0.1.24.tgz", diff --git a/package.json b/package.json index 993f8cf23..254f3b7b4 100644 --- a/package.json +++ b/package.json @@ -82,7 +82,6 @@ "@types/cors": "^2.8.12", "@types/end-of-stream": "^1.4.1", "@types/fs-extra": "^9.0.13", - "@types/ioredis": "^4.28.10", "@types/lodash.orderby": "^4.6.6", "@types/marked": "^4.0.2", "@types/mime-types": "^2.1.1", @@ -92,7 +91,6 @@ "@types/oidc-provider": "^7.8.1", "@types/pump": "^1.1.1", "@types/punycode": "^2.1.0", - "@types/redlock": "^4.0.1", "@types/sparqljs": "^3.1.3", "@types/url-join": "^4.0.1", "@types/uuid": "^8.3.4", @@ -110,7 +108,7 @@ "fetch-sparql-endpoint": "^2.4.0", "fs-extra": "^10.0.0", "handlebars": "^4.7.7", - "ioredis": "^4.28.5", + "ioredis": "^5.0.4", "jose": "^4.4.0", "lodash.orderby": "^4.6.0", "marked": "^4.0.12", @@ -124,7 +122,6 @@ "rdf-parse": "^1.9.1", "rdf-serialize": "^1.2.0", "rdf-terms": "^1.7.1", - "redlock": "^4.2.0", "sparqlalgebrajs": "^4.0.2", "sparqljs": "^3.5.1", "url-join": "^4.0.1", diff --git a/src/index.ts b/src/index.ts index 5697205d6..c2a39090a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -419,7 +419,7 @@ export * from './util/locking/ExpiringReadWriteLocker'; export * from './util/locking/EqualReadWriteLocker'; export * from './util/locking/GreedyReadWriteLocker'; export * from './util/locking/ReadWriteLocker'; -export * from './util/locking/RedisResourceLocker'; +export * from './util/locking/RedisLocker'; export * from './util/locking/ResourceLocker'; export * from './util/locking/SingleThreadedResourceLocker'; export * from './util/locking/WrappedExpiringReadWriteLocker'; diff --git a/src/util/locking/RedisLocker.ts b/src/util/locking/RedisLocker.ts new file mode 100644 index 000000000..556f9a64f --- /dev/null +++ b/src/util/locking/RedisLocker.ts @@ -0,0 +1,204 @@ +import Redis from 'ioredis'; +import type { ResourceIdentifier } from '../../http/representation/ResourceIdentifier'; +import type { Finalizable } from '../../init/final/Finalizable'; +import { getLoggerFor } from '../../logging/LogUtil'; +import { InternalServerError } from '../errors/InternalServerError'; +import type { ReadWriteLocker } from './ReadWriteLocker'; +import type { ResourceLocker } from './ResourceLocker'; +import type { RedisResourceLock, RedisReadWriteLock } from './scripts/RedisLuaScripts'; +import { fromResp2ToBool, REDIS_LUA_SCRIPTS } from './scripts/RedisLuaScripts'; + +export interface AttemptSettings { + /** How many times should an operation in Redis be retried. (-1 is indefinitely). */ + retryCount?: number; + /** The how long should the next retry be delayed (+ some retryJitter) (in ms). */ + retryDelay?: number; + /** Add a fraction of jitter to the original delay each attempt (in ms). */ + retryJitter?: number; +} + +const attemptDefaults: Required = { retryCount: -1, retryDelay: 50, retryJitter: 30 }; + +// Internal prefix for Redis keys; +const PREFIX_RW = '__RW__'; +const PREFIX_LOCK = '__L__'; + +/** + * A Redis Locker that can be used as both: + * * a Read Write Locker that uses a (single) Redis server to store the locks and counts. + * * a Resource Locker that uses a (single) Redis server to store the lock. + * This solution should be process-safe. The only references to locks are string keys + * derived from identifier paths. + * + * The Read Write algorithm roughly goes as follows: + * * Acquire a read lock: allowed as long as there is no write lock. On acquiring the read counter goes up. + * * Acquire a write lock: allowed as long as there is no other write lock AND the read counter is 0. + * * Release a read lock: decreases the read counter with 1 + * * Release a write lock: unlocks the write lock + * + * The Resource locking algorithm uses a single mutex/lock. + * + * All operations, such as checking for a write lock AND read count, are executed in a single Lua script. + * These scripts are used by Redis as a single new command. + * Redis executes its operations in a single thread, as such, each such operation can be considered atomic. + * + * * @see [Redis Commands documentation](https://redis.io/commands/) + * * @see [Redis Lua scripting documentation](https://redis.io/docs/manual/programmability/) + * * @see [ioredis Lua scripting API](https://github.com/luin/ioredis#lua-scripting) + */ +export class RedisLocker implements ReadWriteLocker, ResourceLocker, Finalizable { + protected readonly logger = getLoggerFor(this); + + private readonly redis: Redis; + private readonly redisRw: RedisReadWriteLock; + private readonly redisLock: RedisResourceLock; + private readonly attemptSettings: Required; + + public constructor(redisClient = '127.0.0.1:6379', attemptSettings: AttemptSettings = {}) { + this.redis = this.createRedisClient(redisClient); + this.attemptSettings = { ...attemptDefaults, ...attemptSettings }; + + // Register lua scripts + for (const [ name, script ] of Object.entries(REDIS_LUA_SCRIPTS)) { + this.redis.defineCommand(name, { numberOfKeys: 1, lua: script }); + } + + this.redisRw = this.redis as RedisReadWriteLock; + this.redisLock = this.redis as RedisResourceLock; + } + + /** + * Generate and return a RedisClient based on the provided string + * @param redisClientString - A string that contains either a host address and a + * port number like '127.0.0.1:6379' or just a port number like '6379'. + */ + private createRedisClient(redisClientString: string): Redis { + if (redisClientString.length > 0) { + // Check if port number or ip with port number + // Definitely not perfect, but configuring this is only for experienced users + const match = /^(?:([^:]+):)?(\d{4,5})$/u.exec(redisClientString); + if (!match || !match[2]) { + // At least a port number should be provided + throw new Error(`Invalid data provided to create a Redis client: ${redisClientString}\n + Please provide a port number like '6379' or a host address and a port number like '127.0.0.1:6379'`); + } + const port = Number(match[2]); + const host = match[1]; + return new Redis(port, host); + } + throw new Error(`Empty redisClientString provided!\n + Please provide a port number like '6379' or a host address and a port number like '127.0.0.1:6379'`); + } + + /** + * Try a Redis function according to the set {@link AttemptSettings} + * Since the locking strategy is custom-built on Redis and Redis itself does not have a lock concept, + * this function allows us to wait until we acquired a lock. + * + * The AttemptSettings will dictate how many times we should retry the Redis functions + * before giving up and throwing an error. + * + * @param fn - The function to try + * + * @returns Promise that resolves if operation succeeded. Rejects with error otherwise + * + * @see To convert from Redis operation to Promise use {@link fromResp2ToBool} to wrap the function + */ + private async tryRedisFn(fn: () => Promise): Promise { + const settings = this.attemptSettings; + const maxTries = settings.retryCount === -1 ? Number.POSITIVE_INFINITY : settings.retryCount + 1; + function calcTime(): number { + return Math.max(0, settings.retryDelay + Math.floor(Math.random() * settings.retryJitter)); + } + + let tries = 1; + let acquired = await fn(); + // Keep going until either you get a lock/release or maxTries has been reached. + while (!acquired && (tries <= maxTries)) { + await new Promise((resolve): any => setTimeout(resolve, calcTime())); + acquired = await fn(); + tries += 1; + } + + // Max tries was reached + if (tries > maxTries) { + const err = `The operation did not succeed after the set maximum of tries (${maxTries}).`; + this.logger.warn(err); + throw new InternalServerError(err); + } + } + + /** + * Create a scoped Redis key for Read-Write locking. + * @param identifier - The identifier object to create a Redis key for + * @returns A scoped Redis key that allows cleanup afterwards without affecting other keys. + */ + private getReadWriteKey(identifier: ResourceIdentifier): string { + return `${PREFIX_RW}${identifier.path}`; + } + + /** + * Create a scoped Redis key for Resource locking. + * @param identifier - The identifier object to create a Redis key for + * @returns A scoped Redis key that allows cleanup afterwards without affecting other keys. + */ + private getResourceKey(identifier: ResourceIdentifier): string { + return `${PREFIX_LOCK}${identifier.path}`; + } + + /* ReadWriteLocker methods */ + + public async withReadLock(identifier: ResourceIdentifier, whileLocked: () => (Promise | T)): Promise { + const key = this.getReadWriteKey(identifier); + await this.tryRedisFn((): Promise => fromResp2ToBool(this.redisRw.acquireReadLock(key))); + try { + return await whileLocked(); + } finally { + await this.tryRedisFn((): Promise => fromResp2ToBool(this.redisRw.releaseReadLock(key))); + } + } + + public async withWriteLock(identifier: ResourceIdentifier, whileLocked: () => (Promise | T)): Promise { + const key = this.getReadWriteKey(identifier); + await this.tryRedisFn((): Promise => fromResp2ToBool(this.redisRw.acquireWriteLock(key))); + try { + return await whileLocked(); + } finally { + await this.tryRedisFn((): Promise => fromResp2ToBool(this.redisRw.releaseWriteLock(key))); + } + } + + /* ResourceLocker methods */ + + public async acquire(identifier: ResourceIdentifier): Promise { + const key = this.getResourceKey(identifier); + await this.tryRedisFn((): Promise => fromResp2ToBool(this.redisLock.acquireLock(key))); + } + + public async release(identifier: ResourceIdentifier): Promise { + const key = this.getResourceKey(identifier); + await this.tryRedisFn((): Promise => fromResp2ToBool(this.redisLock.releaseLock(key))); + } + + /* Finalizer methods */ + + public async finalize(): Promise { + // This for loop is an extra failsafe, + // this extra code won't slow down anything, this function will only be called to shut down in peace + try { + // Remove any lock still open, since once closed, they should no longer be held. + const keysRw = await this.redisRw.keys(`${PREFIX_RW}*`); + if (keysRw.length > 0) { + await this.redisRw.del(...keysRw); + } + + const keysLock = await this.redisLock.keys(`${PREFIX_LOCK}*`); + if (keysLock.length > 0) { + await this.redisLock.del(...keysLock); + } + } finally { + await this.redis.quit(); + } + } +} + diff --git a/src/util/locking/RedisResourceLocker.ts b/src/util/locking/RedisResourceLocker.ts deleted file mode 100644 index 2a841d576..000000000 --- a/src/util/locking/RedisResourceLocker.ts +++ /dev/null @@ -1,178 +0,0 @@ -import { assert } from 'console'; -import Redis from 'ioredis'; -import type { Redis as RedisType } from 'ioredis'; -import type { Lock } from 'redlock'; -import Redlock from 'redlock'; -import type { ResourceIdentifier } from '../../http/representation/ResourceIdentifier'; -import type { Finalizable } from '../../init/final/Finalizable'; -import { getLoggerFor } from '../../logging/LogUtil'; -import { InternalServerError } from '../errors/InternalServerError'; -import type { ResourceLocker } from './ResourceLocker'; - -// The ttl set on a lock, not really important cause redlock wil not handle expiration -const ttl = 10000; -// The default redlock config -const defaultRedlockConfig = { - // The expected clock drift; for more details - // see http://redis.io/topics/distlock - // Multiplied by lock ttl to determine drift time - driftFactor: 0.01, - // The max number of times Redlock will attempt - // to lock a resource before erroring - retryCount: 1000000, - // The time in ms between attempts - retryDelay: 200, - // The max time in ms randomly added to retries - // to improve performance under high contention - // see https://www.awsarchitectureblog.com/2015/03/backoff.html - retryJitter: 200, -}; -/** - * A locking system that uses a Redis server or any number of - * Redis nodes / clusters - * This solution has issues though: - * - Redlock wants to handle expiration itself, this is against the design of a ResourceLocker. - * The workaround for this is to extend an active lock indefinitely. - * - This solution is not multithreaded! If threadA locks a resource, only threadA can unlock this resource. - * If threadB wont be able to lock a resource if threadA already acquired that lock, - * in that sense it is kind of multithreaded. - * - Redlock does not provide the ability to see which locks have expired - */ -export class RedisResourceLocker implements ResourceLocker, Finalizable { - protected readonly logger = getLoggerFor(this); - - private readonly redlock: Redlock; - private readonly lockMap: Map; - - public constructor(redisClients: string[], redlockOptions?: Record) { - this.lockMap = new Map(); - const clients: RedisType[] = this.createRedisClients(redisClients); - if (clients.length === 0) { - throw new Error('At least 1 client should be provided'); - } - this.redlock = this.createRedlock(clients, redlockOptions); - this.redlock.on('clientError', (err): void => { - this.logger.error(`Redis/Redlock error: ${err}`); - }); - } - - /** - * Generate and return a list of RedisClients based on the provided strings - * @param redisClientsStrings - a list of strings that contain either a host address and a - * port number like '127.0.0.1:6379' or just a port number like '6379' - */ - private createRedisClients(redisClientsStrings: string[]): RedisType[] { - const result: RedisType[] = []; - if (redisClientsStrings && redisClientsStrings.length > 0) { - for (const client of redisClientsStrings) { - // Check if port number or ip with port number - // Definitely not perfect, but configuring this is only for experienced users - const match = new RegExp(/^(?:([^:]+):)?(\d{4,5})$/u, 'u').exec(client); - if (!match || !match[2]) { - // At least a port number should be provided - throw new Error(`Invalid data provided to create a Redis client: ${client}\n - Please provide a port number like '6379' or a host address and a port number like '127.0.0.1:6379'`); - } - const port = Number(match[2]); - const host = match[1]; - const redisclient: RedisType = new Redis(port, host); - result.push(redisclient); - } - } - return result; - } - - /** - * Generate and return a Redlock instance - * @param clients - a list of RedisClients you want to use for the redlock instance - * @param redlockOptions - extra redlock options to overwrite the default config - */ - private createRedlock(clients: RedisType[], redlockOptions: Record = {}): Redlock { - try { - return new Redlock( - clients, - { ...defaultRedlockConfig, ...redlockOptions }, - ); - } catch (error: unknown) { - throw new InternalServerError(`Error initializing Redlock: ${error}`, { cause: error }); - } - } - - public async finalize(): Promise { - // This for loop is an extra failsafe, - // this extra code won't slow down anything, this function will only be called to shut down in peace - try { - for (const [ , { lock }] of this.lockMap.entries()) { - await this.release({ path: lock.resource }); - } - } finally { - await this.redlock.quit(); - } - } - - public async acquire(identifier: ResourceIdentifier): Promise { - const resource = identifier.path; - let lock: Lock | undefined; - try { - lock = await this.redlock.lock(resource, ttl); - assert(lock); - } catch (error: unknown) { - this.logger.debug(`Unable to acquire lock for ${resource}`); - throw new InternalServerError(`Unable to acquire lock for ${resource} (${error})`, { cause: error }); - } - if (this.lockMap.get(resource)) { - throw new InternalServerError(`Acquired duplicate lock on ${resource}`); - } - // Lock acquired - this.logger.debug(`Acquired lock for resource: ${resource}!`); - const interval = this.extendLockIndefinitely(resource); - this.lockMap.set(resource, { lock, interval }); - } - - public async release(identifier: ResourceIdentifier): Promise { - const resource = identifier.path; - const entry = this.lockMap.get(resource); - if (!entry) { - // Lock is invalid - this.logger.warn(`Unexpected release request for non-existent lock on ${resource}`); - throw new InternalServerError(`Trying to unlock resource that is not locked: ${resource}`); - } - try { - await this.redlock.unlock(entry.lock); - clearInterval(entry.interval); - this.lockMap.delete(resource); - // Successfully released lock - this.logger.debug(`Released lock for ${resource}, ${this.getLockCount()} active locks remaining!`); - } catch (error: unknown) { - this.logger.error(`Error releasing lock for ${resource} (${error})`); - throw new InternalServerError(`Unable to release lock for: ${resource}, ${error}`, { cause: error }); - } - } - - /** - * Counts the number of active locks. - */ - private getLockCount(): number { - return this.lockMap.size; - } - - /** - * This function is internally used to keep an acquired lock active, a wrapper class will handle expiration - * @param identifier - Identifier of the lock to be extended - */ - private extendLockIndefinitely(identifier: string): NodeJS.Timeout { - return setInterval(async(): Promise => { - const entry = this.lockMap.get(identifier)!; - try { - const newLock = await this.redlock.extend(entry.lock, ttl); - this.lockMap.set(identifier, { lock: newLock, interval: entry.interval }); - this.logger.debug(`Extended (Redis)lock for resource: ${identifier}`); - } catch (error: unknown) { - // No error should be re-thrown because this means the lock has simply been released - this.logger.error(`Failed to extend this (Redis)lock for resource: ${identifier}, ${error}`); - clearInterval(entry.interval); - this.lockMap.delete(identifier); - } - }, ttl / 2); - } -} diff --git a/src/util/locking/scripts/RedisLuaScripts.ts b/src/util/locking/scripts/RedisLuaScripts.ts new file mode 100644 index 000000000..b2e7d5cc0 --- /dev/null +++ b/src/util/locking/scripts/RedisLuaScripts.ts @@ -0,0 +1,151 @@ +import type { Callback, Redis } from 'ioredis'; +import { InternalServerError } from '../../errors/InternalServerError'; + +const SUFFIX_WLOCK = '.wlock'; +const SUFFIX_LOCK = '.lock'; +const SUFFIX_COUNT = '.count'; +const LOCKED = 'locked'; + +/** + * Lua scripts to be used as Redis operations. + */ +export const REDIS_LUA_SCRIPTS = { + acquireReadLock: ` + -- Return 0 if an entry already exists. + local lockKey = KEYS[1].."${SUFFIX_WLOCK}" + if redis.call("exists", lockKey) == 1 then + return 0 + end + + -- Return true if succeeded (and counter is incremented) + local countKey = KEYS[1].."${SUFFIX_COUNT}" + return redis.call("incr", countKey) > 0 + `, + acquireWriteLock: ` + -- Return 0 if a lock entry already exists or read count is > 0 + local lockKey = KEYS[1].."${SUFFIX_WLOCK}" + local countKey = KEYS[1].."${SUFFIX_COUNT}" + local count = tonumber(redis.call("get", countKey)) + if ((redis.call("exists", lockKey) == 1) or (count ~= nil and count > 0)) then + return 0 + end + + -- Set lock and respond with 'OK' if succeeded (otherwise null) + return redis.call("set", lockKey, "${LOCKED}"); + `, + releaseReadLock: ` + -- Return 1 after decreasing the counter, if counter is < 0 now: return '-ERR' + local countKey = KEYS[1].."${SUFFIX_COUNT}" + local result = redis.call("decr", countKey) + if result >= 0 then + return 1 + else + return redis.error_reply("Error trying to release readlock when read count was 0.") + end + `, + releaseWriteLock: ` + -- Release the lock and reply with 1 if succeeded (otherwise return '-ERR') + local lockKey = KEYS[1].."${SUFFIX_WLOCK}" + local result = redis.call("del", lockKey) + if (result > 0) then + return 1 + else + return redis.error_reply("Error trying to release writelock that did not exist.") + end + `, + acquireLock: ` + -- Return 0 if lock entry already exists, or 'OK' if it succeeds in setting the lock entry. + local key = KEYS[1].."${SUFFIX_LOCK}" + if redis.call("exists", key) == 1 then + return 0 + end + + -- Return 'OK' if succeeded setting entry + return redis.call("set", key, "${LOCKED}"); + `, + releaseLock: ` + -- Release the lock and reply with 1 if succeeded (otherwise return '-ERR') + local key = KEYS[1].."${SUFFIX_LOCK}" + local result = redis.call("del", key) + if result > 0 then + return 1 + else + return redis.error_reply("Error trying to release lock that did not exist.") + end + `, +} as const; + +export type RedisAnswer = 0 | 1 | null | 'OK' | string; + +/** + * Convert a RESP2 response to a boolean. + * @param result - The Promise-wrapped result of a RESP2 Redis function. + * @returns * `1`, `'OK'`: return `true` + * * `0`: returns `false` + * * `-ERR`: throw error + * @throws On `-ERR*` `null` or any other value + */ +export async function fromResp2ToBool(result: Promise): Promise { + const res = await result; + switch (res) { + case 1: + case 'OK': + return true; + case 0: + return false; + case null: + throw new Error('Redis operation error detected (value was null).'); + default: + if (res.toString().startsWith('-ERR')) { + throw new InternalServerError(`Redis error: ${res.toString().slice(5)}`); + } else { + throw new InternalServerError(`Unexpected Redis answer received! (${res})`); + } + } +} + +export interface RedisReadWriteLock extends Redis { + /** + * Try to acquire a readLock on `resourceIdentifierPath`. + * Will succeed if there are no write locks. + * + * @returns 1 if succeeded. 0 if not possible. + */ + acquireReadLock: (resourceIdentifierPath: string, callback?: Callback) => Promise; + + /** + * Try to acquire a writeLock on `resourceIdentifierPath`. + * Only works if no other write lock is present and the read counter is 0. + * + * @returns 'OK' if succeeded, 0 if not possible. + */ + acquireWriteLock: (resourceIdentifierPath: string, callback?: Callback) => Promise; + + /** + * Release readLock. This means decrementing the read counter with 1. + * @returns 1 if succeeded. '-ERR' if read count goes below 0 + */ + releaseReadLock: (resourceIdentifierPath: string, callback?: Callback) => Promise; + + /** + * Release writeLock. This means deleting the write lock. + * @returns 1 if succeeded. '-ERR' if write lock was non-existing. + */ + releaseWriteLock: (resourceIdentifierPath: string, callback?: Callback) => Promise; +} + +export interface RedisResourceLock extends Redis { + /** + * Try to acquire a lock on `resourceIdentifierPath`. + * Only works if no other lock is present. + * + * @returns 'OK' if succeeded, 0 if not possible. + */ + acquireLock: (resourceIdentifierPath: string, callback?: Callback) => Promise; + + /** + * Release lock. This means deleting the lock. + * @returns 1 if succeeded. '-ERR' if lock was non-existing. + */ + releaseLock: (resourceIdentifierPath: string, callback?: Callback) => Promise; +} diff --git a/test/integration/RedisLocker.test.ts b/test/integration/RedisLocker.test.ts new file mode 100644 index 000000000..5d0b77b70 --- /dev/null +++ b/test/integration/RedisLocker.test.ts @@ -0,0 +1,402 @@ +import EventEmitter from 'events'; +import fetch from 'cross-fetch'; +import Redis, { ReplyError } from 'ioredis'; +import type { App } from '../../src'; +import type { RedisLocker } from '../../src/util/locking/RedisLocker'; +import type { RedisReadWriteLock, RedisResourceLock } from '../../src/util/locking/scripts/RedisLuaScripts'; +import { REDIS_LUA_SCRIPTS } from '../../src/util/locking/scripts/RedisLuaScripts'; +import { describeIf, flushPromises, getPort } from '../util/Util'; +import { getDefaultVariables, getTestConfigPath, instantiateFromConfig } from './Config'; +/** + * Test the general functionality of the server using a RedisLocker with Read-Write strategy. + */ +describeIf('docker', 'A server with a RedisLocker', (): void => { + const port = getPort('RedisLocker'); + const baseUrl = `http://localhost:${port}/`; + let app: App; + let locker: RedisLocker; + + beforeAll(async(): Promise => { + const instances = await instantiateFromConfig( + 'urn:solid-server:test:Instances', + getTestConfigPath('server-redis-lock.json'), + getDefaultVariables(port, baseUrl), + ) as Record; + ({ app, locker } = instances); + await app.start(); + }); + + afterAll(async(): Promise => { + await app.stop(); + }); + + describe('has a locker that', (): void => { + it('can add a file to the store, read it and delete it.', async(): Promise => { + // Create file + const fileUrl = `${baseUrl}testfile2.txt`; + const fileData = 'TESTFILE2'; + + let response = await fetch(fileUrl, { + method: 'PUT', + headers: { + 'content-type': 'text/plain', + }, + body: fileData, + }); + expect(response.status).toBe(201); + + // Get file + response = await fetch(fileUrl); + expect(response.status).toBe(200); + const body = await response.text(); + expect(body).toContain('TESTFILE2'); + + // DELETE file + response = await fetch(fileUrl, { method: 'DELETE' }); + expect(response.status).toBe(205); + response = await fetch(fileUrl); + expect(response.status).toBe(404); + }); + + it('can create a folder and delete it.', async(): Promise => { + const containerPath = 'secondfolder/'; + const containerUrl = `${baseUrl}${containerPath}`; + // PUT + let response = await fetch(containerUrl, { + method: 'PUT', + headers: { + 'content-type': 'text/plain', + }, + }); + expect(response.status).toBe(201); + + // GET + response = await fetch(containerUrl); + expect(response.status).toBe(200); + + // DELETE + response = await fetch(containerUrl, { method: 'DELETE' }); + expect(response.status).toBe(205); + response = await fetch(containerUrl); + expect(response.status).toBe(404); + }); + + it('can get a resource multiple times.', async(): Promise => { + const fileUrl = `${baseUrl}image.png`; + const fileData = 'testtesttest'; + + let response = await fetch(fileUrl, { + method: 'PUT', + headers: { + 'content-type': 'text/plain', + }, + body: fileData, + }); + expect(response.status).toBe(201); + + // GET 4 times + for (let i = 0; i < 4; i++) { + const res = await fetch(fileUrl); + expect(res.status).toBe(200); + expect(res.headers.get('content-type')).toBe('text/plain'); + const body = await res.text(); + expect(body).toContain('testtesttest'); + } + + // DELETE + response = await fetch(fileUrl, { method: 'DELETE' }); + expect(response.status).toBe(205); + response = await fetch(fileUrl); + expect(response.status).toBe(404); + }); + }); + + describe('implements ResoureLocker and', (): void => { + const identifier = { path: 'http://test.com/foo' }; + + it('can acquire a resource.', async(): Promise => { + await expect(locker.acquire(identifier)).resolves.toBeUndefined(); + // Clean up lock + await locker.release(identifier); + }); + + it('can release a resource.', async(): Promise => { + await expect(locker.acquire(identifier)).resolves.toBeUndefined(); + await expect(locker.release(identifier)).resolves.toBeUndefined(); + }); + + it('can acquire different locks simultaneously.', async(): Promise => { + const lock1 = locker.acquire({ path: 'path1' }); + const lock2 = locker.acquire({ path: 'path2' }); + const lock3 = locker.acquire({ path: 'path3' }); + const release1 = locker.release({ path: 'path1' }); + const release2 = locker.release({ path: 'path2' }); + const release3 = locker.release({ path: 'path3' }); + + await expect(Promise.all([ lock1, lock2, lock3 ])).resolves.toBeDefined(); + // Clean up locks + await Promise.all([ release1, release2, release3 ]); + }); + + it('cannot acquire the same lock simultaneously.', async(): Promise => { + await expect(locker.acquire(identifier)).resolves.toBeUndefined(); + await expect(locker.acquire(identifier)).rejects + .toThrow(/The operation did not succeed after the set maximum of tries \(\d+\)./u); + await expect(locker.acquire(identifier)).rejects + .toThrow(/The operation did not succeed after the set maximum of tries \(\d+\)./u); + }); + }); + + describe('implements ReadWriteLocker and', (): void => { + const identifier = { path: 'http://test.com/foo' }; + + it('can read a resource.', async(): Promise => { + const testFn = jest.fn(); + await expect(locker.withReadLock(identifier, (): any => testFn())).resolves.toBeUndefined(); + expect(testFn).toHaveBeenCalled(); + }); + + it('can write a resource.', async(): Promise => { + const testFn = jest.fn(); + await expect(locker.withWriteLock(identifier, (): any => testFn())).resolves.toBeUndefined(); + expect(testFn).toHaveBeenCalled(); + }); + + it('can read a resource twice again after it was unlocked.', async(): Promise => { + const testFn = jest.fn(); + await expect(locker.withReadLock(identifier, (): any => testFn())).resolves.toBeUndefined(); + expect(testFn).toHaveBeenCalledTimes(1); + await expect(locker.withReadLock(identifier, (): any => testFn())).resolves.toBeUndefined(); + expect(testFn).toHaveBeenCalledTimes(2); + }); + + it('can acquire different readLocks simultaneously.', async(): Promise => { + const testFn = jest.fn(); + const lock1 = locker.withReadLock({ path: 'path1' }, (): any => testFn()); + const lock2 = locker.withReadLock({ path: 'path2' }, (): any => testFn()); + const lock3 = locker.withReadLock({ path: 'path3' }, (): any => testFn()); + + await expect(Promise.all([ lock1, lock2, lock3 ])).resolves.toBeDefined(); + }); + + it('can acquire different writeLocks simultaneously.', async(): Promise => { + const testFn = jest.fn(); + const lock1 = locker.withWriteLock({ path: 'path1' }, (): any => testFn()); + const lock2 = locker.withWriteLock({ path: 'path2' }, (): any => testFn()); + const lock3 = locker.withWriteLock({ path: 'path3' }, (): any => testFn()); + + await expect(Promise.all([ lock1, lock2, lock3 ])).resolves.toBeDefined(); + }); + + it('can acquire the same readLock simultaneously.', async(): Promise => { + let res = ''; + const emitter = new EventEmitter(); + const unlocks = [ 0, 1, 2 ].map((num): Promise => + new Promise((resolve): any => emitter.on(`release${num}`, resolve))); + const promises = [ 0, 1, 2 ].map((num): Promise => + locker.withReadLock(identifier, async(): Promise => { + res += `l${num}`; + await unlocks[num]; + res += `r${num}`; + return num; + })); + + await flushPromises(); + + emitter.emit('release1'); + await flushPromises(); + await expect(promises[1]).resolves.toBe(1); + emitter.emit('release0'); + await flushPromises(); + await expect(promises[0]).resolves.toBe(0); + emitter.emit('release2'); + await flushPromises(); + await expect(promises[2]).resolves.toBe(2); + + expect(res).toContain('l0l1l2'); + expect(res).toContain('r1r0r2'); + }); + + it('cannot acquire the same writeLock simultaneously.', async(): Promise => { + let res = ''; + const emitter = new EventEmitter(); + const unlocks = [ 0, 1, 2 ].map((num): Promise => + new Promise((resolve): any => emitter.on(`release${num}`, resolve))); + const promises = [ 0, 1, 2 ].map((num): Promise => + locker.withWriteLock(identifier, async(): Promise => num)); + + const l1 = promises[1].then(async(): Promise => { + res += 'l1'; + await unlocks[1]; + res += 'r1'; + }); + const l0 = promises[0].then(async(): Promise => { + res += 'l0'; + await unlocks[0]; + res += 'r0'; + }); + const l2 = promises[2].then(async(): Promise => { + res += 'l2'; + await unlocks[2]; + res += 'r2'; + }); + + emitter.emit('release1'); + emitter.emit('release0'); + emitter.emit('release2'); + await Promise.all([ l0, l1, l2 ]); + + expect(res).toContain('l0r0'); + expect(res).toContain('l1r1'); + expect(res).toContain('l2r2'); + }); + }); + + describe('defines custom Redis lua functions', (): void => { + let redis: Redis & RedisReadWriteLock & RedisResourceLock; + + async function clearRedis(): Promise { + const keys = await redis.keys('*'); + if (keys && keys.length > 0) { + await redis.del(keys); + } + } + + beforeAll(async(): Promise => { + redis = new Redis('127.0.0.1:6379') as Redis & RedisReadWriteLock & RedisResourceLock; + + // Register lua scripts + for (const [ name, script ] of Object.entries(REDIS_LUA_SCRIPTS)) { + redis.defineCommand(name, { numberOfKeys: 1, lua: script }); + } + + await clearRedis(); + }); + + afterAll(async(): Promise => { + await redis.quit(); + }); + + beforeEach(async(): Promise => { + await clearRedis(); + }); + + it('#acquireReadLock.', async(): Promise => { + const key1 = 'key1'; + const writeKey1 = `${key1}.wlock`; + const countKey1 = `${key1}.count`; + // Test fails + await redis.set(writeKey1, 'locked'); + await expect(redis.acquireReadLock(key1)).resolves.toBe(0); + + // Test succeeds + await redis.del(writeKey1); + await expect(redis.acquireReadLock(key1)).resolves.toBe(1); + await expect(redis.get(countKey1)).resolves.toBe('1'); + await expect(redis.acquireReadLock(key1)).resolves.toBe(1); + await expect(redis.get(countKey1)).resolves.toBe('2'); + await expect(redis.acquireReadLock(key1)).resolves.toBe(1); + await expect(redis.get(countKey1)).resolves.toBe('3'); + }); + + it('#acquireWriteLock.', async(): Promise => { + const key1 = 'key1'; + const writeKey1 = `${key1}.wlock`; + const countKey1 = `${key1}.count`; + + // Test fails because count > 0 + await expect(redis.incr(countKey1)).resolves.toBe(1); + await expect(redis.acquireWriteLock(key1)).resolves.toBe(0); + + // Test fails because write lock is present + await clearRedis(); + await redis.set(writeKey1, 'locked'); + await expect(redis.acquireWriteLock(key1)).resolves.toBe(0); + + // Test succeeds + await clearRedis(); + await expect(redis.acquireWriteLock(key1)).resolves.toBe('OK'); + + // Test fails again + await expect(redis.acquireWriteLock(key1)).resolves.toBe(0); + }); + + it('#releaseReadLock.', async(): Promise => { + const key1 = 'key1'; + const countKey1 = `${key1}.count`; + + // Test succeeds + await expect(redis.acquireReadLock(key1)).resolves.toBe(1); + await expect(redis.acquireReadLock(key1)).resolves.toBe(1); + await expect(redis.acquireReadLock(key1)).resolves.toBe(1); + await expect(redis.get(countKey1)).resolves.toBe('3'); + + await expect(redis.releaseReadLock(key1)).resolves.toBe(1); + await expect(redis.releaseReadLock(key1)).resolves.toBe(1); + await expect(redis.releaseReadLock(key1)).resolves.toBe(1); + await expect(redis.get(countKey1)).resolves.toBe('0'); + + // Test fails + await expect(redis.releaseReadLock(key1)).rejects + .toThrow(ReplyError); + await expect(redis.releaseReadLock(key1)).rejects + .toThrow('Error trying to release readlock when read count was 0.'); + }); + + it('#releaseWriteLock.', async(): Promise => { + const key1 = 'key1'; + const writeKey1 = `${key1}.wlock`; + + // Test fails + await expect(redis.releaseWriteLock(key1)).rejects + .toThrow(ReplyError); + await expect(redis.releaseWriteLock(key1)).rejects + .toThrow('Error trying to release writelock that did not exist.'); + + // Test succeeds + await redis.acquireWriteLock(key1); + await expect(redis.exists(writeKey1)).resolves.toBe(1); + await expect(redis.get(writeKey1)).resolves.toBe('locked'); + await expect(redis.releaseWriteLock(key1)).resolves.toBe(1); + await expect(redis.exists(writeKey1)).resolves.toBe(0); + }); + + it('#acquireLock.', async(): Promise => { + const key1 = 'key1'; + const lockKey1 = `${key1}.lock`; + // Test succeeds + await expect(redis.acquireLock(key1)).resolves.toBe('OK'); + await expect(redis.exists(lockKey1)).resolves.toBe(1); + await expect(redis.get(lockKey1)).resolves.toBe('locked'); + + // Test fails + await expect(redis.acquireLock(key1)).resolves.toBe(0); + await expect(redis.exists(lockKey1)).resolves.toBe(1); + await expect(redis.get(lockKey1)).resolves.toBe('locked'); + + // Test succeeds again + await redis.releaseLock(key1); + await expect(redis.exists(lockKey1)).resolves.toBe(0); + await expect(redis.acquireLock(key1)).resolves.toBe('OK'); + }); + + it('#releaseLock.', async(): Promise => { + const key1 = 'key1'; + const lockKey1 = `${key1}.lock`; + + // Test fails + await expect(redis.releaseLock(key1)).rejects + .toThrow(ReplyError); + await expect(redis.releaseLock(key1)).rejects + .toThrow('Error trying to release lock that did not exist.'); + + // Test succeeds + await redis.acquireLock(key1); + await expect(redis.exists(lockKey1)).resolves.toBe(1); + await expect(redis.get(lockKey1)).resolves.toBe('locked'); + await expect(redis.releaseLock(key1)).resolves.toBe(1); + await expect(redis.exists(lockKey1)).resolves.toBe(0); + }); + }); +}); + diff --git a/test/integration/RedisResourceLockerIntegration.test.ts b/test/integration/RedisResourceLockerIntegration.test.ts deleted file mode 100644 index eebbaee8d..000000000 --- a/test/integration/RedisResourceLockerIntegration.test.ts +++ /dev/null @@ -1,164 +0,0 @@ -import fetch from 'cross-fetch'; -import type { App, RedisResourceLocker } from '../../src'; -import { describeIf, flushPromises, getPort } from '../util/Util'; -import { getDefaultVariables, getTestConfigPath, instantiateFromConfig } from './Config'; - -/** - * Test the general functionality of the server using a RedisResourceLocker - */ -describeIf('docker', 'A server with a RedisResourceLocker as ResourceLocker', (): void => { - const port = getPort('RedisResourceLocker'); - const baseUrl = `http://localhost:${port}/`; - let app: App; - let locker: RedisResourceLocker; - - beforeAll(async(): Promise => { - const instances = await instantiateFromConfig( - 'urn:solid-server:test:Instances', - getTestConfigPath('run-with-redlock.json'), - getDefaultVariables(port, baseUrl), - ) as Record; - ({ app, locker } = instances); - await app.start(); - }); - - afterAll(async(): Promise => { - await app.stop(); - }); - - it('can add a file to the store, read it and delete it.', async(): Promise => { - // Create file - const fileUrl = `${baseUrl}testfile2.txt`; - const fileData = 'TESTFILE2'; - - let response = await fetch(fileUrl, { - method: 'PUT', - headers: { - 'content-type': 'text/plain', - }, - body: fileData, - }); - expect(response.status).toBe(201); - - // Get file - response = await fetch(fileUrl); - expect(response.status).toBe(200); - const body = await response.text(); - expect(body).toContain('TESTFILE2'); - - // DELETE file - response = await fetch(fileUrl, { method: 'DELETE' }); - expect(response.status).toBe(205); - response = await fetch(fileUrl); - expect(response.status).toBe(404); - }); - - it('can create a folder and delete it.', async(): Promise => { - const containerPath = 'secondfolder/'; - const containerUrl = `${baseUrl}${containerPath}`; - // PUT - let response = await fetch(containerUrl, { - method: 'PUT', - headers: { - 'content-type': 'text/plain', - }, - }); - expect(response.status).toBe(201); - - // GET - response = await fetch(containerUrl); - expect(response.status).toBe(200); - - // DELETE - response = await fetch(containerUrl, { method: 'DELETE' }); - expect(response.status).toBe(205); - response = await fetch(containerUrl); - expect(response.status).toBe(404); - }); - - it('can get a resource multiple times.', async(): Promise => { - const fileUrl = `${baseUrl}image.png`; - const fileData = 'testtesttest'; - - let response = await fetch(fileUrl, { - method: 'PUT', - headers: { - 'content-type': 'text/plain', - }, - body: fileData, - }); - expect(response.status).toBe(201); - - // GET 4 times - for (let i = 0; i < 4; i++) { - const res = await fetch(fileUrl); - expect(res.status).toBe(200); - expect(res.headers.get('content-type')).toBe('text/plain'); - const body = await res.text(); - expect(body).toContain('testtesttest'); - } - - // DELETE - response = await fetch(fileUrl, { method: 'DELETE' }); - expect(response.status).toBe(205); - response = await fetch(fileUrl); - expect(response.status).toBe(404); - }); - - describe('Test the ResourceLocker itself', (): void => { - const identifier = { path: 'http://test.com/foo' }; - - it('can lock and unlock a resource.', async(): Promise => { - await expect(locker.acquire(identifier)).resolves.toBeUndefined(); - await expect(locker.release(identifier)).resolves.toBeUndefined(); - }); - - it('can lock a resource again after it was unlocked.', async(): Promise => { - await expect(locker.acquire(identifier)).resolves.toBeUndefined(); - await expect(locker.release(identifier)).resolves.toBeUndefined(); - await expect(locker.acquire(identifier)).resolves.toBeUndefined(); - await expect(locker.release(identifier)).resolves.toBeUndefined(); - }); - - it('can acquire different keys simultaneously.', async(): Promise => { - const lock1 = locker.acquire({ path: 'path1' }); - const lock2 = locker.acquire({ path: 'path2' }); - const lock3 = locker.acquire({ path: 'path3' }); - - await expect(Promise.all([ lock1, lock2, lock3 ])).resolves.toBeDefined(); - - await locker.release({ path: 'path1' }); - await locker.release({ path: 'path2' }); - await locker.release({ path: 'path3' }); - }); - - it('Cannot acquire the same lock simultaneously.', async(): Promise => { - let res = ''; - const lock1 = locker.acquire(identifier); - const lock2 = locker.acquire(identifier); - const lock3 = locker.acquire(identifier); - - await flushPromises(); - - const l2 = lock2.then(async(): Promise => { - res += 'l2'; - await locker.release(identifier); - res += 'r2'; - }); - const l1 = lock1.then(async(): Promise => { - res += 'l1'; - await locker.release(identifier); - res += 'r1'; - }); - const l3 = lock3.then(async(): Promise => { - res += 'l3'; - await locker.release(identifier); - res += 'r3'; - }); - await Promise.all([ l1, l2, l3 ]); - expect(res).toContain('l1r1'); - expect(res).toContain('l2r2'); - expect(res).toContain('l3r3'); - }); - }); -}); diff --git a/test/integration/config/run-with-redlock.json b/test/integration/config/server-redis-lock.json similarity index 89% rename from test/integration/config/run-with-redlock.json rename to test/integration/config/server-redis-lock.json index d639e7819..89e6d49a8 100644 --- a/test/integration/config/run-with-redlock.json +++ b/test/integration/config/server-redis-lock.json @@ -40,9 +40,16 @@ }, { "RecordObject:_record_key": "locker", - "RecordObject:_record_value": { "@id": "urn:solid-server:default:RedisResourceLocker" } + "RecordObject:_record_value": { "@id": "urn:solid-server:default:RedisLocker" } } ] + }, + { + "@id": "urn:solid-server:default:RedisLocker", + "@type": "RedisLocker", + "attemptSettings_retryCount": 29, + "attemptSettings_retryDelay": 100, + "attemptSettings_retryJitter": 0 } ] } diff --git a/test/unit/util/locking/RedisLocker.test.ts b/test/unit/util/locking/RedisLocker.test.ts new file mode 100644 index 000000000..8ad73d8fd --- /dev/null +++ b/test/unit/util/locking/RedisLocker.test.ts @@ -0,0 +1,539 @@ +import EventEmitter from 'events'; +import type { Redis } from 'ioredis'; +import type { ReadWriteLocker, ResourceLocker } from '../../../../src'; +import { InternalServerError } from '../../../../src'; +import { RedisLocker } from '../../../../src/util/locking/RedisLocker'; +import type { RedisResourceLock, RedisReadWriteLock } from '../../../../src/util/locking/scripts/RedisLuaScripts'; +import { flushPromises } from '../../../util/Util'; + +interface LockState { + reads: number; + lock: boolean; +} + +const store = { + ensureKey(key: string): void { + if (!(key in this.internal)) { + this.internal[key] = { lock: false, reads: 0 }; + } + }, + internal: {} as Record, + reset(): void { + this.internal = {}; + }, + acquireReadLock(key: string): number { + this.ensureKey(key); + if (this.internal[key].lock) { + return 0; + } + this.internal[key].reads += 1; + return 1; + }, + acquireWriteLock(key: string): number | null | 'OK' { + this.ensureKey(key); + if (this.internal[key].lock || this.internal[key].reads > 0) { + return 0; + } + + this.internal[key].lock = true; + return 'OK'; + }, + releaseReadLock(key: string): number { + this.internal[key].reads -= 1; + return 1; + }, + releaseWriteLock(key: string): number | null { + if (!this.internal[key] || !this.internal[key].lock) { + return null; + } + this.internal[key].lock = false; + return 1; + }, + acquireLock(key: string): number | null | 'OK' { + this.ensureKey(key); + if (this.internal[key].lock) { + return 0; + } + this.internal[key].lock = true; + return 'OK'; + }, + releaseLock(key: string): number | string { + if (!(key in this.internal) || !this.internal[key].lock) { + return '-ERR Can\'t release non-existing lock.\r\n'; + } + this.internal[key].lock = false; + return 1; + }, + +}; + +const redis: jest.Mocked = { + defineCommand: jest.fn(), + quit: jest.fn(), + keys: jest.fn().mockResolvedValue([]), + del: jest.fn(), + acquireReadLock: jest.fn().mockImplementation(async(key: string): Promise => + store.acquireReadLock(key)), + acquireWriteLock: jest.fn().mockImplementation(async(key: string): Promise => + store.acquireWriteLock(key)), + releaseReadLock: jest.fn().mockImplementation(async(key: string): Promise => + store.releaseReadLock(key)), + releaseWriteLock: jest.fn().mockImplementation(async(key: string): Promise => + store.releaseWriteLock(key)), + acquireLock: jest.fn().mockImplementation(async(key: string): Promise => + store.acquireLock(key)), + releaseLock: jest.fn().mockImplementation(async(key: string): Promise => + store.releaseLock(key)), +} as any; + +jest.mock('ioredis', (): any => jest.fn().mockImplementation((): Redis => redis)); + +describe('A RedisLocker', (): void => { + describe('with Read-Write logic', (): void => { + const resource1 = { path: 'http://test.com/resource' }; + const resource2 = { path: 'http://test.com/resource2' }; + let locker: RedisLocker; + + beforeEach(async(): Promise => { + store.reset(); + jest.clearAllMocks(); + locker = new RedisLocker('6379'); + }); + + afterEach(async(): Promise => { + // In case some locks are not released by a test the timers will still be running + jest.clearAllTimers(); + }); + + afterAll(async(): Promise => { + jest.restoreAllMocks(); + }); + + it('will fill in default arguments when constructed with empty arguments.', (): void => { + expect((): ReadWriteLocker => new RedisLocker()).toBeDefined(); + expect((): ReadWriteLocker => new RedisLocker()).not.toThrow(); + }); + + it('errors when instantiated with incorrect arguments.', (): void => { + const arg = 'wrongRedisString'; + expect((): RedisLocker => new RedisLocker(arg)) + .toThrow(`Invalid data provided to create a Redis client: ${arg}`); + expect((): RedisLocker => new RedisLocker('')) + .toThrow(`Empty redisClientString provided!`); + }); + + it('errors when instantiated with empty arguments.', (): void => { + expect((): RedisLocker => new RedisLocker('')) + .toThrow(`Empty redisClientString provided!`); + }); + + it('does not block single read operations.', async(): Promise => { + await expect(locker.withReadLock(resource1, (): any => 5)).resolves.toBe(5); + }); + + it('does not block single write operations.', async(): Promise => { + await expect(locker.withWriteLock(resource1, (): any => 5)).resolves.toBe(5); + }); + + it('does not block multiple read operations.', async(): Promise => { + const order: string[] = []; + const emitter = new EventEmitter(); + + const unlocks = [ 0, 1, 2 ].map((num): any => new Promise((resolve): any => + emitter.on(`release${num}`, resolve))); + const promises = [ 0, 1, 2 ].map((num): any => locker.withReadLock(resource1, async(): Promise => { + order.push(`start ${num}`); + await unlocks[num]; + order.push(`finish ${num}`); + return num; + })); + + // Allow time to attach listeners + await flushPromises(); + + emitter.emit('release2'); + await expect(promises[2]).resolves.toBe(2); + emitter.emit('release0'); + await expect(promises[0]).resolves.toBe(0); + emitter.emit('release1'); + await expect(promises[1]).resolves.toBe(1); + + expect(order).toEqual([ 'start 0', 'start 1', 'start 2', 'finish 2', 'finish 0', 'finish 1' ]); + }); + + it('blocks multiple write operations without guaranteed order (fairness).', async(): Promise => { + const order: string[] = []; + const emitter = new EventEmitter(); + + const unlocks = [ 0, 1, 2 ].map((num): any => new Promise((resolve): any => + emitter.on(`release${num}`, resolve))); + const promises = [ 0, 1, 2 ].map((num): any => locker.withWriteLock(resource1, async(): Promise => { + order.push(`start ${num}`); + await unlocks[num]; + order.push(`finish ${num}`); + return num; + })); + + // Allow time to attach listeners + await flushPromises(); + + emitter.emit('release2'); + + // Allow time to finish write 2 + await flushPromises(); + + emitter.emit('release0'); + emitter.emit('release1'); + await Promise.all([ promises[2], promises[0], promises[1] ]); + expect(order).toHaveLength(6); + expect(order.slice(0, 2)).toEqual([ 'start 0', 'finish 0' ]); + expect(order.slice(2) + .map((el): boolean => [ 'start 1', 'finish 1', 'start 2', 'finish 2' ].includes(el))).toBeTruthy(); + }); + + it('allows multiple write operations on different resources.', async(): Promise => { + const order: string[] = []; + const emitter = new EventEmitter(); + + const resources = [ resource1, resource2 ]; + const unlocks = [ 0, 1 ].map((num): any => new Promise((resolve): any => emitter.on(`release${num}`, resolve))); + const promises = [ 0, 1 ].map((num): any => locker.withWriteLock(resources[num], async(): Promise => { + order.push(`start ${num}`); + await unlocks[num]; + order.push(`finish ${num}`); + return num; + })); + + // Allow time to attach listeners + await flushPromises(); + + emitter.emit('release1'); + await expect(promises[1]).resolves.toBe(1); + emitter.emit('release0'); + await expect(promises[0]).resolves.toBe(0); + + expect(order).toEqual([ 'start 0', 'start 1', 'finish 1', 'finish 0' ]); + }); + + it('blocks write operations during read operations.', async(): Promise => { + const order: string[] = []; + const emitter = new EventEmitter(); + + const promRead = new Promise((resolve): any => { + emitter.on('releaseRead', resolve); + }); + + // We want to make sure the write operation only starts while the read operation is busy + // Otherwise the internal write lock might not be acquired yet + const delayedLockWrite = new Promise((resolve): void => { + emitter.on('readStarted', (): void => { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + locker.withWriteLock(resource1, (): any => { + order.push('write'); + resolve(); + }); + }); + }); + + const lockRead = locker.withReadLock(resource1, async(): Promise => { + emitter.emit('readStarted'); + order.push('read start'); + await promRead; + order.push('read finish'); + }); + + // Allow time to attach listeners + await flushPromises(); + + const promAll = Promise.all([ delayedLockWrite, lockRead ]); + + emitter.emit('releaseRead'); + await promAll; + expect(order).toEqual([ 'read start', 'read finish', 'write' ]); + }); + + it('allows write operations on different resources during read operations.', async(): Promise => { + const order: string[] = []; + const emitter = new EventEmitter(); + + const promRead = new Promise((resolve): any => { + emitter.on('releaseRead', resolve); + }); + + const delayedLockWrite = new Promise((resolve): void => { + emitter.on('readStarted', (): void => { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + locker.withWriteLock(resource2, (): any => { + order.push('write'); + resolve(); + }); + }); + }); + + const lockRead = locker.withReadLock(resource1, async(): Promise => { + emitter.emit('readStarted'); + order.push('read start'); + await promRead; + order.push('read finish'); + }); + + // Allow time to attach listeners + await flushPromises(); + + const promAll = Promise.all([ delayedLockWrite, lockRead ]); + + emitter.emit('releaseRead'); + await promAll; + expect(order).toEqual([ 'read start', 'write', 'read finish' ]); + }); + + it('prioritizes read operations when a read operation is waiting.', async(): Promise => { + // This test is very similar to the previous ones but adds an extra read lock + const order: string[] = []; + const emitter = new EventEmitter(); + + const promRead1 = new Promise((resolve): any => emitter.on('releaseRead1', resolve)); + const promRead2 = new Promise((resolve): any => emitter.on('releaseRead2', resolve)); + + const delayedLockWrite = new Promise((resolve): void => { + emitter.on('readStarted', (): void => { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + locker.withWriteLock(resource1, (): any => { + order.push('write'); + resolve(); + }); + }); + }); + + const delayedLockRead2 = new Promise((resolve): void => { + emitter.on('readStarted', (): void => { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + locker.withReadLock(resource1, async(): Promise => { + order.push('read 2 start'); + await promRead2; + order.push('read 2 finish'); + resolve(); + }); + }); + }); + + const lockRead = locker.withReadLock(resource1, async(): Promise => { + emitter.emit('readStarted'); + order.push('read 1 start'); + await promRead1; + order.push('read 1 finish'); + }); + + // Allow time to attach listeners + await flushPromises(); + + const promAll = Promise.all([ delayedLockWrite, lockRead, delayedLockRead2 ]); + + emitter.emit('releaseRead1'); + + // Allow time to finish read 1 + await flushPromises(); + + emitter.emit('releaseRead2'); + await promAll; + expect(order).toEqual([ 'read 1 start', 'read 2 start', 'read 1 finish', 'read 2 finish', 'write' ]); + }); + + it('blocks read operations during write operations.', async(): Promise => { + // Again similar but with read and write order switched + const order: string[] = []; + const emitter = new EventEmitter(); + + const promWrite = new Promise((resolve): any => { + emitter.on('releaseWrite', resolve); + }); + + // We want to make sure the read operation only starts while the write operation is busy + const delayedLockRead = new Promise((resolve): void => { + emitter.on('writeStarted', (): void => { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + locker.withReadLock(resource1, (): any => { + order.push('read'); + resolve(); + }); + }); + }); + + const lockWrite = locker.withWriteLock(resource1, async(): Promise => { + emitter.emit('writeStarted'); + order.push('write start'); + await promWrite; + order.push('write finish'); + }); + + // Allow time to attach listeners + await flushPromises(); + + const promAll = Promise.all([ delayedLockRead, lockWrite ]); + + emitter.emit('releaseWrite'); + await promAll; + expect(order).toEqual([ 'write start', 'write finish', 'read' ]); + }); + + it('throws error if Redis answers with null.', async(): Promise => { + const emitter = new EventEmitter(); + const promise = locker.withWriteLock(resource1, (): any => + new Promise((resolve): any => emitter.on('release', resolve))); + await redis.releaseWriteLock(`__RW__${resource1.path}`); + await flushPromises(); + emitter.emit('release'); + await expect(promise).rejects.toThrow('Redis operation error detected (value was null).'); + }); + + it('errors when a readLock is not possible.', async(): Promise => { + const locker2 = new RedisLocker('localhost:6379', { retryCount: 0 }); + redis.acquireReadLock.mockResolvedValueOnce(0); + await expect(locker2.withReadLock(resource1, (): any => 5)).rejects + .toThrow(/The operation did not succeed after the set maximum of tries \(\d+\)./u); + }); + + it('errors when a writeLock is not possible.', async(): Promise => { + const locker2 = new RedisLocker('localhost:6379', { retryCount: 0 }); + redis.acquireWriteLock.mockResolvedValueOnce(0); + await expect(locker2.withWriteLock(resource1, (): any => 5)).rejects + .toThrow(/The operation did not succeed after the set maximum of tries \(\d+\)./u); + }); + + it('throws error if Redis answers unexpectedly.', async(): Promise => { + redis.acquireWriteLock.mockResolvedValueOnce('unexpected' as any); + const promise = locker.withWriteLock(resource1, (): any => ({})); + await expect(promise).rejects.toThrow('Unexpected Redis answer received! (unexpected)'); + }); + + describe('finalize()', (): void => { + it('should quit when there are no more keys when finalize() is called.', async(): Promise => { + // This works since the Redis is simply a mock and quit should have cleared the internal store + await locker.withWriteLock(resource1, async(): Promise => { + await locker.finalize(); + expect(redis.quit).toHaveBeenCalledTimes(1); + }); + }); + it('should clear all lock keys when finalize() is called.', async(): Promise => { + redis.keys.mockResolvedValueOnce([ '__L__k1', '__L__k2' ]); + // This works since the Redis is simply a mock and quit should have cleared the internal store + await locker.withWriteLock(resource1, async(): Promise => { + await locker.finalize(); + expect(redis.quit).toHaveBeenCalledTimes(1); + }); + }); + + it('should clear all rw keys when finalize() is called.', async(): Promise => { + redis.keys.mockResolvedValueOnce([ '__RW__k1', '__RW__k2' ]); + // This works since the Redis is simply a mock and quit should have cleared the internal store + await locker.withWriteLock(resource1, async(): Promise => { + await locker.finalize(); + expect(redis.quit).toHaveBeenCalledTimes(1); + }); + }); + }); + }); + + describe('with resource lock logic', (): void => { + let locker: RedisLocker; + const identifier = { path: 'http://test.com/foo' }; + + beforeEach(async(): Promise => { + jest.clearAllMocks(); + locker = new RedisLocker('6379', { retryCount: 5 }); + }); + + afterEach(async(): Promise => { + // In case some locks are not released by a test the timers will still be running + jest.clearAllTimers(); + }); + + afterAll(async(): Promise => { + jest.restoreAllMocks(); + }); + + it('will fill in default arguments when constructed with empty arguments.', (): void => { + expect((): ResourceLocker => new RedisLocker()).toBeDefined(); + expect((): ResourceLocker => new RedisLocker()).not.toThrow(); + }); + + it('can lock and unlock a resource.', async(): Promise => { + await expect(locker.acquire(identifier)).resolves.toBeUndefined(); + await expect(locker.release(identifier)).resolves.toBeUndefined(); + expect(redis.acquireLock).toHaveBeenCalledTimes(1); + expect(redis.releaseLock).toHaveBeenCalledTimes(1); + }); + + it('can lock a resource again after it was unlocked.', async(): Promise => { + await expect(locker.acquire(identifier)).resolves.toBeUndefined(); + await expect(locker.release(identifier)).resolves.toBeUndefined(); + await expect(locker.acquire(identifier)).resolves.toBeUndefined(); + expect(redis.acquireLock).toHaveBeenCalledTimes(2); + expect(redis.releaseLock).toHaveBeenCalledTimes(1); + await expect(locker.release(identifier)).resolves.toBeUndefined(); + }); + + it('errors when unlocking a resource that was not locked.', async(): Promise => { + await expect(locker.acquire(identifier)).resolves.toBeUndefined(); + await expect(locker.release(identifier)).resolves.toBeUndefined(); + await expect(locker.release(identifier)).rejects.toThrow(InternalServerError); + expect(redis.acquireLock).toHaveBeenCalledTimes(1); + expect(redis.releaseLock).toHaveBeenCalledTimes(2); + }); + + it('errors when Redis.acquireLock throws an error.', async(): Promise => { + redis.acquireLock.mockResolvedValueOnce('-ERR random Error\r\n'); + const prom = locker.acquire(identifier); + await expect(prom).rejects.toThrow(InternalServerError); + await expect(prom).rejects.toThrow('Redis error: random Error'); + expect(redis.acquireLock).toHaveBeenCalledTimes(1); + }); + + it('errors when Redis.releaseLock throws an error.', async(): Promise => { + await locker.acquire(identifier); + redis.releaseLock.mockResolvedValueOnce('-ERR random Error\r\n'); + const prom = locker.release(identifier); + await expect(prom).rejects.toThrow(InternalServerError); + await expect(prom).rejects.toThrow('Redis error: random Error'); + expect(redis.releaseLock).toHaveBeenCalledTimes(1); + await expect(locker.release(identifier)).resolves.toBeUndefined(); + }); + + it('can acquire different keys simultaneously.', async(): Promise => { + const lock1 = locker.acquire({ path: 'path1' }); + const lock2 = locker.acquire({ path: 'path2' }); + const lock3 = locker.acquire({ path: 'path3' }); + + await expect(Promise.all([ lock1, lock2, lock3 ])).resolves.toBeDefined(); + + await locker.release({ path: 'path1' }); + await locker.release({ path: 'path2' }); + await locker.release({ path: 'path3' }); + }); + + describe('createRedisClients', (): void => { + it('errors when invalid string is passed.', async(): Promise => { + // Reset calls done in `beforeEach` + jest.clearAllMocks(); + const clientString = 'noHostOrPort'; + expect((): any => new RedisLocker(clientString)) + .toThrow('Invalid data provided to create a Redis client: noHostOrPort'); + }); + }); + + describe('finalize()', (): void => { + it('should clear all locks (even when empty) when finalize() is called.', async(): Promise => { + await locker.finalize(); + expect(redis.quit).toHaveBeenCalledTimes(1); + }); + + it('should clear all locks when finalize() is called.', async(): Promise => { + redis.keys + .mockResolvedValueOnce([ '__L__k1', '__L__k2' ]) + .mockResolvedValueOnce([ '__L__k1', '__L__k2' ]); + await locker.finalize(); + expect(redis.quit).toHaveBeenCalledTimes(1); + }); + }); + }); +}); diff --git a/test/unit/util/locking/RedisResourceLocker.test.ts b/test/unit/util/locking/RedisResourceLocker.test.ts deleted file mode 100644 index 0623a8604..000000000 --- a/test/unit/util/locking/RedisResourceLocker.test.ts +++ /dev/null @@ -1,205 +0,0 @@ -import { EventEmitter } from 'events'; -import Redis from 'ioredis'; -import Redlock from 'redlock'; -import type { Lock } from 'redlock'; -import * as LogUtil from '../../../../src/logging/LogUtil'; -import { InternalServerError } from '../../../../src/util/errors/InternalServerError'; -import { RedisResourceLocker } from '../../../../src/util/locking/RedisResourceLocker'; - -const redlock: jest.Mocked = Object.assign(new EventEmitter(), { - lock: jest.fn().mockImplementation(async(resource: string, ttl: number): Promise => - ({ resource, expiration: Date.now() + ttl } as Lock)), - unlock: jest.fn(), - extend: jest.fn().mockImplementation( - async(lock: Lock, ttl: number): Promise => { - lock.expiration += ttl; - return lock; - }, - ), - quit: jest.fn(), -}) as any; - -jest.mock('redlock', (): any => jest.fn().mockImplementation((): Redlock => redlock)); -jest.mock('ioredis', (): any => jest.fn()); - -jest.useFakeTimers(); - -describe('A RedisResourceLocker', (): void => { - let locker: RedisResourceLocker; - const identifier = { path: 'http://test.com/foo' }; - - beforeEach(async(): Promise => { - jest.clearAllMocks(); - redlock.removeAllListeners(); - - locker = new RedisResourceLocker([ '6379' ]); - }); - - afterEach(async(): Promise => { - // In case some locks are not released by a test the timers will still be running - jest.clearAllTimers(); - }); - - afterAll(async(): Promise => { - jest.restoreAllMocks(); - }); - - it('can lock and unlock a resource.', async(): Promise => { - await expect(locker.acquire(identifier)).resolves.toBeUndefined(); - await expect(locker.release(identifier)).resolves.toBeUndefined(); - expect(redlock.lock).toHaveBeenCalledTimes(1); - expect(redlock.unlock).toHaveBeenCalledTimes(1); - }); - - it('can lock a resource again after it was unlocked.', async(): Promise => { - await expect(locker.acquire(identifier)).resolves.toBeUndefined(); - await expect(locker.release(identifier)).resolves.toBeUndefined(); - await expect(locker.acquire(identifier)).resolves.toBeUndefined(); - expect(redlock.lock).toHaveBeenCalledTimes(2); - expect(redlock.unlock).toHaveBeenCalledTimes(1); - await expect(locker.release(identifier)).resolves.toBeUndefined(); - }); - - it('errors when unlocking a resource that was not locked.', async(): Promise => { - await expect(locker.acquire(identifier)).resolves.toBeUndefined(); - await expect(locker.release(identifier)).resolves.toBeUndefined(); - await expect(locker.release(identifier)).rejects.toThrow(InternalServerError); - expect(redlock.lock).toHaveBeenCalledTimes(1); - expect(redlock.unlock).toHaveBeenCalledTimes(1); - }); - - it('errors when redlock.lock throws an error.', async(): Promise => { - redlock.lock.mockRejectedValueOnce(new Error('random Error')); - const prom = locker.acquire(identifier); - await expect(prom).rejects.toThrow(InternalServerError); - await expect(prom).rejects.toThrow('Unable to acquire lock for '); - await expect(prom).rejects.toThrow('Error: random Error'); - expect(redlock.lock).toHaveBeenCalledTimes(1); - }); - - it('errors if redlock.lock resolves but a lock is already stored.', async(): Promise => { - await expect(locker.acquire(identifier)).resolves.toBeUndefined(); - // Works since redlock.lock is mocked to always succeed - const prom = locker.acquire(identifier); - await expect(prom).rejects.toThrow(InternalServerError); - await expect(prom).rejects.toThrow(`Acquired duplicate lock on ${identifier.path}`); - }); - - it('errors when redlock.unlock throws an error.', async(): Promise => { - await locker.acquire(identifier); - redlock.unlock.mockRejectedValueOnce(new Error('random Error')); - const prom = locker.release(identifier); - await expect(prom).rejects.toThrow(InternalServerError); - await expect(prom).rejects.toThrow('Unable to release lock for: '); - await expect(prom).rejects.toThrow('Error: random Error'); - expect(redlock.unlock).toHaveBeenCalledTimes(1); - await expect(locker.release(identifier)).resolves.toBeUndefined(); - }); - - it('does not extend when there are no locks to extend.', async(): Promise => { - await locker.acquire(identifier); - await locker.release(identifier); - jest.advanceTimersByTime(20000); - expect(redlock.extend).toHaveBeenCalledTimes(0); - }); - - it('cleans up if lock extension failed.', async(): Promise => { - // This should never happen though - redlock.extend.mockImplementationOnce((): any => { - throw new Error('random error'); - }); - await locker.acquire(identifier); - jest.advanceTimersByTime(20000); - expect(redlock.extend).toHaveBeenCalledTimes(1); - // Will throw since we removed the lock entry - await expect(locker.release(identifier)).rejects.toThrow(InternalServerError); - }); - - it('can acquire different keys simultaneously.', async(): Promise => { - const lock1 = locker.acquire({ path: 'path1' }); - const lock2 = locker.acquire({ path: 'path2' }); - const lock3 = locker.acquire({ path: 'path3' }); - - await expect(Promise.all([ lock1, lock2, lock3 ])).resolves.toBeDefined(); - - await locker.release({ path: 'path1' }); - await locker.release({ path: 'path2' }); - await locker.release({ path: 'path3' }); - }); - - it('extends a lock indefinitely.', async(): Promise => { - await locker.acquire(identifier); - jest.advanceTimersByTime(20000); - await expect(locker.release(identifier)).resolves.toBeUndefined(); - }); - - it('uses users redlockOptions if passed to constructor.', async(): Promise => { - // Reset calls done in `beforeEach` - jest.clearAllMocks(); - const clients = [ '6379' ]; - const options = { - driftFactor: 0.2, - retryCount: 20, - retryDelay: 2000, - retryJitter: 2000, - }; - locker = new RedisResourceLocker(clients, options); - expect(Redlock).toHaveBeenCalledTimes(1); - expect(Redlock).toHaveBeenLastCalledWith(expect.any(Array), options); - }); - - it('errors on creation when no redis servers are passed to the constructor.', async(): Promise => { - expect((): any => new RedisResourceLocker([])).toThrow('At least 1 client should be provided'); - }); - - it('errors if there is an issue creating the Redlock.', async(): Promise => { - (Redlock as unknown as jest.Mock).mockImplementationOnce((): never => { - throw new Error('redlock error!'); - }); - expect((): any => new RedisResourceLocker([ '1234' ])) - .toThrow('Error initializing Redlock: Error: redlock error!'); - }); - - it('logs redis client errors.', async(): Promise => { - const logger = { error: jest.fn() }; - const mock = jest.spyOn(LogUtil, 'getLoggerFor'); - mock.mockReturnValueOnce(logger as any); - locker = new RedisResourceLocker([ '6379' ]); - expect(logger.error).toHaveBeenCalledTimes(0); - redlock.emit('clientError', 'problem!'); - expect(logger.error).toHaveBeenCalledTimes(1); - expect(logger.error).toHaveBeenLastCalledWith('Redis/Redlock error: problem!'); - }); - - describe('createRedisClients', (): void => { - it('should create and return the right amount of redisClients.', async(): Promise => { - // Reset calls done in `beforeEach` - jest.clearAllMocks(); - const clientStrings = [ '6379', '127.0.0.1:6378' ]; - locker = new RedisResourceLocker(clientStrings); - expect(Redis).toHaveBeenCalledTimes(2); - expect(Redis).toHaveBeenCalledWith(6379, undefined); - expect(Redis).toHaveBeenCalledWith(6378, '127.0.0.1'); - }); - - it('errors when invalid string is passed.', async(): Promise => { - // Reset calls done in `beforeEach` - jest.clearAllMocks(); - const clientStrings = [ 'noHostOrPort' ]; - expect((): any => new RedisResourceLocker(clientStrings)) - .toThrow('Invalid data provided to create a Redis client: noHostOrPort'); - expect(Redis).toHaveBeenCalledTimes(0); - }); - }); - - describe('finalize()', (): void => { - it('should clear all locks and intervals when finalize() is called.', async(): Promise => { - await locker.acquire(identifier); - await locker.finalize(); - expect(redlock.quit).toHaveBeenCalledTimes(1); - - // This works since the Redlock is simply a mock and quit should have cleared the lockMap - await expect(locker.acquire(identifier)).resolves.toBeUndefined(); - }); - }); -}); diff --git a/test/util/Util.ts b/test/util/Util.ts index 2e5098503..1410d38cb 100644 --- a/test/util/Util.ts +++ b/test/util/Util.ts @@ -17,7 +17,7 @@ const portNames = [ 'PermissionTable', 'PodCreation', 'PodQuota', - 'RedisResourceLocker', + 'RedisLocker', 'RestrictedIdentity', 'SeedingPods', 'ServerFetch',