From b86640772ba07a16d077d93973810a9c5f365aa4 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 29 Nov 2023 18:45:48 +0000 Subject: [PATCH 01/32] Fix for saving relationships that have the same field name used on both sides, previously this could cause a relationship to be cleared depending on how the relationship schema was configured. There is a chance when saving that this won't happen as which side of the relationship is denoted by doc1 and doc2 is random, so when this happens is random. Using the table to pick the correct side is safer than just using the field name. --- .../server/src/api/controllers/static/index.ts | 6 ++++-- .../server/src/db/linkedRows/LinkController.ts | 16 +++++++++++++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/packages/server/src/api/controllers/static/index.ts b/packages/server/src/api/controllers/static/index.ts index 4c5415a6c6..2963546e7f 100644 --- a/packages/server/src/api/controllers/static/index.ts +++ b/packages/server/src/api/controllers/static/index.ts @@ -24,7 +24,7 @@ import AWS from "aws-sdk" import fs from "fs" import sdk from "../../../sdk" import * as pro from "@budibase/pro" -import { App, Ctx, ProcessAttachmentResponse, Upload } from "@budibase/types" +import { App, Ctx, ProcessAttachmentResponse } from "@budibase/types" const send = require("koa-send") @@ -212,7 +212,9 @@ export const serveBuilderPreview = async function (ctx: Ctx) { if (!env.isJest()) { let appId = context.getAppId() - const previewHbs = loadHandlebarsFile(`${__dirname}/preview.hbs`) + const templateLoc = join(__dirname, "templates") + const previewLoc = fs.existsSync(templateLoc) ? templateLoc : __dirname + const previewHbs = loadHandlebarsFile(join(previewLoc, "preview.hbs")) ctx.body = await processString(previewHbs, { clientLibPath: objectStore.clientLibraryUrl(appId!, appInfo.version), }) diff --git a/packages/server/src/db/linkedRows/LinkController.ts b/packages/server/src/db/linkedRows/LinkController.ts index c4eed1169a..f52694465f 100644 --- a/packages/server/src/db/linkedRows/LinkController.ts +++ b/packages/server/src/db/linkedRows/LinkController.ts @@ -251,9 +251,19 @@ class LinkController { // find the docs that need to be deleted let toDeleteDocs = thisFieldLinkDocs .filter(doc => { - let correctDoc = - doc.doc1.fieldName === fieldName ? doc.doc2 : doc.doc1 - return rowField.indexOf(correctDoc.rowId) === -1 + let correctDoc + if ( + doc.doc1.tableId === table._id! && + doc.doc1.fieldName === fieldName + ) { + correctDoc = doc.doc2 + } else if ( + doc.doc2.tableId === table._id! && + doc.doc2.fieldName === fieldName + ) { + correctDoc = doc.doc1 + } + return correctDoc && rowField.indexOf(correctDoc.rowId) === -1 }) .map(doc => { return { ...doc, _deleted: true } From 160fbf21258823fe0fd06941e9061fc4719699b7 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 29 Nov 2023 19:53:56 +0000 Subject: [PATCH 02/32] Adding test case and fixing issue that it revealed with external tables as well. --- .../server/src/api/routes/tests/row.spec.ts | 35 +++++++++++++++++++ .../src/sdk/app/tables/external/utils.ts | 12 ++++--- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index de49441f3a..ba80f36b1b 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -517,9 +517,22 @@ describe.each([ }) describe("patch", () => { + let otherTable: Table + beforeAll(async () => { const tableConfig = generateTableConfig() table = await createTable(tableConfig) + const otherTableConfig = generateTableConfig() + // need a short name of table here - for relationship tests + otherTableConfig.name = "a" + otherTableConfig.schema.relationship = { + name: "relationship", + type: FieldType.LINK, + fieldName: "relationship", + tableId: table._id!, + relationshipType: RelationshipType.ONE_TO_MANY, + } + otherTable = await createTable(otherTableConfig) }) it("should update only the fields that are supplied", async () => { @@ -615,6 +628,28 @@ describe.each([ expect(getResp.body.user1[0]._id).toEqual(user2._id) expect(getResp.body.user2[0]._id).toEqual(user2._id) }) + + it("should be able to update relationships when both columns are same name", async () => { + let row = await config.api.row.save(table._id!, { + name: "test", + description: "test", + }) + let row2 = await config.api.row.save(otherTable._id!, { + name: "test", + description: "test", + relationship: [row._id], + }) + row = (await config.api.row.get(table._id!, row._id!)).body + expect(row.relationship.length).toBe(1) + const resp = await config.api.row.patch(table._id!, { + _id: row._id!, + _rev: row._rev!, + tableId: row.tableId!, + name: "test2", + relationship: [row2._id], + }) + expect(resp.relationship.length).toBe(1) + }) }) describe("destroy", () => { diff --git a/packages/server/src/sdk/app/tables/external/utils.ts b/packages/server/src/sdk/app/tables/external/utils.ts index bde812dd3d..50ea98eb39 100644 --- a/packages/server/src/sdk/app/tables/external/utils.ts +++ b/packages/server/src/sdk/app/tables/external/utils.ts @@ -1,5 +1,6 @@ import { Datasource, + FieldType, ManyToManyRelationshipFieldMetadata, ManyToOneRelationshipFieldMetadata, OneToManyRelationshipFieldMetadata, @@ -42,10 +43,13 @@ export function cleanupRelationships( for (let [relatedKey, relatedSchema] of Object.entries( relatedTable.schema )) { - if ( - relatedSchema.type === FieldTypes.LINK && - relatedSchema.fieldName === foreignKey - ) { + if (relatedSchema.type !== FieldType.LINK) { + continue + } + // if they both have the same field name it will appear as if it needs to be removed, + // don't cleanup in this scenario + const sameFieldNameForBoth = relatedSchema.name === schema.name + if (relatedSchema.fieldName === foreignKey && !sameFieldNameForBoth) { delete relatedTable.schema[relatedKey] } } From ae9061f97882bcc465803854bf30aeac672d45e3 Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Thu, 30 Nov 2023 11:27:57 +0000 Subject: [PATCH 03/32] Bump version to 2.13.24 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 491de0dd97..022fbb055a 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.13.23", + "version": "2.13.24", "npmClient": "yarn", "packages": [ "packages/*" From 71a4e96d589281aacd57f1e0ee54346ce50bf9e6 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 30 Nov 2023 11:34:11 +0000 Subject: [PATCH 04/32] Move CODEOWNERS to the root to see if that fixes it. --- CODEOWNERS | 3 +++ packages/backend-core/CODEOWNERS | 1 - packages/server/CODEOWNERS | 1 - packages/worker/CODEOWNERS | 1 - 4 files changed, 3 insertions(+), 3 deletions(-) create mode 100644 CODEOWNERS delete mode 100644 packages/backend-core/CODEOWNERS delete mode 100644 packages/server/CODEOWNERS delete mode 100644 packages/worker/CODEOWNERS diff --git a/CODEOWNERS b/CODEOWNERS new file mode 100644 index 0000000000..69d69ab7d0 --- /dev/null +++ b/CODEOWNERS @@ -0,0 +1,3 @@ +/packages/server @Budibase/backend +/packages/worker @Budibase/backend +/packages/backend-core @Budibase/backend diff --git a/packages/backend-core/CODEOWNERS b/packages/backend-core/CODEOWNERS deleted file mode 100644 index 84313fb9cf..0000000000 --- a/packages/backend-core/CODEOWNERS +++ /dev/null @@ -1 +0,0 @@ -* @Budibase/backend \ No newline at end of file diff --git a/packages/server/CODEOWNERS b/packages/server/CODEOWNERS deleted file mode 100644 index 84313fb9cf..0000000000 --- a/packages/server/CODEOWNERS +++ /dev/null @@ -1 +0,0 @@ -* @Budibase/backend \ No newline at end of file diff --git a/packages/worker/CODEOWNERS b/packages/worker/CODEOWNERS deleted file mode 100644 index 84313fb9cf..0000000000 --- a/packages/worker/CODEOWNERS +++ /dev/null @@ -1 +0,0 @@ -* @Budibase/backend \ No newline at end of file From 58736e02c7dd0d835792edad2d1a3649b68efd32 Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Thu, 30 Nov 2023 12:43:09 +0000 Subject: [PATCH 05/32] Bump version to 2.13.25 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 022fbb055a..6569facc59 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.13.24", + "version": "2.13.25", "npmClient": "yarn", "packages": [ "packages/*" From 3131798d1d86ebf5c9bc949dea8c9f48bb835d60 Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Thu, 30 Nov 2023 12:52:40 +0000 Subject: [PATCH 06/32] Bump version to 2.13.26 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 6569facc59..794a077423 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.13.25", + "version": "2.13.26", "npmClient": "yarn", "packages": [ "packages/*" From 6315806bea201486e5f2aaa52c24d1cfdf7db3a0 Mon Sep 17 00:00:00 2001 From: melohagan <101575380+melohagan@users.noreply.github.com> Date: Thu, 30 Nov 2023 14:42:38 +0000 Subject: [PATCH 07/32] Update originalIndex on select (#12480) --- .../controls/ButtonActionEditor/ButtonActionDrawer.svelte | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/builder/src/components/design/settings/controls/ButtonActionEditor/ButtonActionDrawer.svelte b/packages/builder/src/components/design/settings/controls/ButtonActionEditor/ButtonActionDrawer.svelte index 109f9f62a2..236bba58dc 100644 --- a/packages/builder/src/components/design/settings/controls/ButtonActionEditor/ButtonActionDrawer.svelte +++ b/packages/builder/src/components/design/settings/controls/ButtonActionEditor/ButtonActionDrawer.svelte @@ -147,6 +147,7 @@ const selectAction = action => () => { selectedAction = action + originalActionIndex = actions.findIndex(item => item.id === action.id) } const onAddAction = actionType => { From 37e065df21bba6ea7e7f2926493cfe7ee87beec3 Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Thu, 30 Nov 2023 14:44:12 +0000 Subject: [PATCH 08/32] Bump version to 2.13.27 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 794a077423..3c442524fa 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.13.26", + "version": "2.13.27", "npmClient": "yarn", "packages": [ "packages/*" From 02fefa55296a24e860a41e7d00b4b0cccfcfdb41 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 30 Nov 2023 15:09:01 +0000 Subject: [PATCH 09/32] Fixes for postgres test case, there was an issue with creating tables with relationships during the creation phase. --- .../server/src/api/routes/tests/row.spec.ts | 8 +++++--- .../src/integration-test/postgres.spec.ts | 18 ++++++++++++++++++ .../src/sdk/app/tables/external/index.ts | 2 ++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index ba80f36b1b..5b3c69b87a 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -527,12 +527,14 @@ describe.each([ otherTableConfig.name = "a" otherTableConfig.schema.relationship = { name: "relationship", - type: FieldType.LINK, - fieldName: "relationship", - tableId: table._id!, relationshipType: RelationshipType.ONE_TO_MANY, + type: FieldType.LINK, + tableId: table._id!, + fieldName: "relationship", } otherTable = await createTable(otherTableConfig) + // need to set the config back to the original table + config.table = table }) it("should update only the fields that are supplied", async () => { diff --git a/packages/server/src/integration-test/postgres.spec.ts b/packages/server/src/integration-test/postgres.spec.ts index 8dc49a9489..67e4fee81c 100644 --- a/packages/server/src/integration-test/postgres.spec.ts +++ b/packages/server/src/integration-test/postgres.spec.ts @@ -934,25 +934,43 @@ describe("postgres integrations", () => { }, ], }) + const m2oRel = { + [m2oFieldName]: [ + { + _id: row._id, + }, + ], + } expect(res.body[m2oFieldName]).toEqual([ { + ...m2oRel, ...foreignRowsByType[RelationshipType.MANY_TO_ONE][0].row, [`fk_${manyToOneRelationshipInfo.table.name}_${manyToOneRelationshipInfo.fieldName}`]: row.id, }, { + ...m2oRel, ...foreignRowsByType[RelationshipType.MANY_TO_ONE][1].row, [`fk_${manyToOneRelationshipInfo.table.name}_${manyToOneRelationshipInfo.fieldName}`]: row.id, }, { + ...m2oRel, ...foreignRowsByType[RelationshipType.MANY_TO_ONE][2].row, [`fk_${manyToOneRelationshipInfo.table.name}_${manyToOneRelationshipInfo.fieldName}`]: row.id, }, ]) + const o2mRel = { + [o2mFieldName]: [ + { + _id: row._id, + }, + ], + } expect(res.body[o2mFieldName]).toEqual([ { + ...o2mRel, ...foreignRowsByType[RelationshipType.ONE_TO_MANY][0].row, _id: expect.any(String), _rev: expect.any(String), diff --git a/packages/server/src/sdk/app/tables/external/index.ts b/packages/server/src/sdk/app/tables/external/index.ts index f445fcaf08..157a683ad0 100644 --- a/packages/server/src/sdk/app/tables/external/index.ts +++ b/packages/server/src/sdk/app/tables/external/index.ts @@ -136,6 +136,8 @@ export async function save( schema.main = true } + // add in the new table for relationship purposes + tables[tableToSave.name] = tableToSave cleanupRelationships(tableToSave, tables, oldTable) const operation = tableId ? Operation.UPDATE_TABLE : Operation.CREATE_TABLE From 4326ee17d9811269a8bf25badba1c2c67e44d637 Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Thu, 30 Nov 2023 17:33:41 +0000 Subject: [PATCH 10/32] Bump version to 2.13.28 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 3c442524fa..87b7c50bf7 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.13.27", + "version": "2.13.28", "npmClient": "yarn", "packages": [ "packages/*" From f71e1ac03a784e89105efe75c93a14448b5caf55 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 29 Nov 2023 13:56:25 +0100 Subject: [PATCH 11/32] Allow locks without TTL --- packages/backend-core/src/redis/redlockImpl.ts | 17 +++++++++++++++-- packages/types/src/sdk/locks.ts | 4 ++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/packages/backend-core/src/redis/redlockImpl.ts b/packages/backend-core/src/redis/redlockImpl.ts index 266f1fe989..cccb981399 100644 --- a/packages/backend-core/src/redis/redlockImpl.ts +++ b/packages/backend-core/src/redis/redlockImpl.ts @@ -4,6 +4,7 @@ import { LockOptions, LockType } from "@budibase/types" import * as context from "../context" import env from "../environment" import { logWarn } from "../logging" +import { Duration } from "../utils" async function getClient( type: LockType, @@ -105,12 +106,21 @@ export async function doWithLock( task: () => Promise ): Promise> { const redlock = await getClient(opts.type, opts.customOptions) - let lock + let lock: Redlock.Lock | undefined + let interval try { const name = getLockName(opts) + let ttl = opts.ttl || Duration.fromSeconds(15).toMs() + // create the lock - lock = await redlock.lock(name, opts.ttl) + lock = await redlock.lock(name, ttl) + + if (!opts.ttl) { + interval = setInterval(() => { + lock!.extend(ttl) + }, ttl / 2) + } // perform locked task // need to await to ensure completion before unlocking @@ -134,5 +144,8 @@ export async function doWithLock( if (lock) { await lock.unlock() } + if (interval) { + clearInterval(interval) + } } } diff --git a/packages/types/src/sdk/locks.ts b/packages/types/src/sdk/locks.ts index a35e7b379b..080574b735 100644 --- a/packages/types/src/sdk/locks.ts +++ b/packages/types/src/sdk/locks.ts @@ -36,9 +36,9 @@ export interface LockOptions { */ name: LockName /** - * The ttl to auto-expire the lock if not unlocked manually + * The ttl to auto-expire the lock if not unlocked manually. If undefined, the lock will be autoextending while the process is running. */ - ttl: number + ttl?: number /** * The individual resource to lock. This is useful for locking around very specific identifiers, e.g. a document that is prone to conflicts */ From 3073397800b5704194ac570a5bd4707cb93e94be Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 29 Nov 2023 18:20:11 +0100 Subject: [PATCH 12/32] Fix ttl --- packages/backend-core/src/redis/redlockImpl.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/backend-core/src/redis/redlockImpl.ts b/packages/backend-core/src/redis/redlockImpl.ts index cccb981399..048d17d080 100644 --- a/packages/backend-core/src/redis/redlockImpl.ts +++ b/packages/backend-core/src/redis/redlockImpl.ts @@ -111,14 +111,15 @@ export async function doWithLock( try { const name = getLockName(opts) - let ttl = opts.ttl || Duration.fromSeconds(15).toMs() + let ttl = opts.ttl || Duration.fromSeconds(30).toMs() // create the lock lock = await redlock.lock(name, ttl) if (!opts.ttl) { + // No TTL is provided, so we keep extending the lock while the task is running interval = setInterval(() => { - lock!.extend(ttl) + lock?.extend(ttl / 2) }, ttl / 2) } From 5a7dbb00764131b4b312531b064b794041bf2665 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 29 Nov 2023 18:20:54 +0100 Subject: [PATCH 13/32] Async --- packages/backend-core/src/redis/redlockImpl.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend-core/src/redis/redlockImpl.ts b/packages/backend-core/src/redis/redlockImpl.ts index 048d17d080..9a6b200c81 100644 --- a/packages/backend-core/src/redis/redlockImpl.ts +++ b/packages/backend-core/src/redis/redlockImpl.ts @@ -118,8 +118,8 @@ export async function doWithLock( if (!opts.ttl) { // No TTL is provided, so we keep extending the lock while the task is running - interval = setInterval(() => { - lock?.extend(ttl / 2) + interval = setInterval(async () => { + await lock?.extend(ttl / 2) }, ttl / 2) } From 26a77298acf508b73a7f9d4df404d00a027d099a Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 29 Nov 2023 18:31:53 +0100 Subject: [PATCH 14/32] Use timers --- packages/backend-core/src/redis/redlockImpl.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/backend-core/src/redis/redlockImpl.ts b/packages/backend-core/src/redis/redlockImpl.ts index 9a6b200c81..d57385d8fe 100644 --- a/packages/backend-core/src/redis/redlockImpl.ts +++ b/packages/backend-core/src/redis/redlockImpl.ts @@ -5,6 +5,7 @@ import * as context from "../context" import env from "../environment" import { logWarn } from "../logging" import { Duration } from "../utils" +import { timers } from ".." async function getClient( type: LockType, @@ -118,7 +119,7 @@ export async function doWithLock( if (!opts.ttl) { // No TTL is provided, so we keep extending the lock while the task is running - interval = setInterval(async () => { + interval = timers.set(async () => { await lock?.extend(ttl / 2) }, ttl / 2) } @@ -146,7 +147,7 @@ export async function doWithLock( await lock.unlock() } if (interval) { - clearInterval(interval) + timers.clear(interval) } } } From a32582eb8aee59ef39a892907c8eb8e442058bd0 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 29 Nov 2023 20:50:50 +0100 Subject: [PATCH 15/32] Use autoextend as locktype --- .../backend-core/src/redis/redlockImpl.ts | 60 +++++++++++++------ packages/types/src/sdk/locks.ts | 5 +- 2 files changed, 46 insertions(+), 19 deletions(-) diff --git a/packages/backend-core/src/redis/redlockImpl.ts b/packages/backend-core/src/redis/redlockImpl.ts index d57385d8fe..3383dbff13 100644 --- a/packages/backend-core/src/redis/redlockImpl.ts +++ b/packages/backend-core/src/redis/redlockImpl.ts @@ -4,8 +4,7 @@ import { LockOptions, LockType } from "@budibase/types" import * as context from "../context" import env from "../environment" import { logWarn } from "../logging" -import { Duration } from "../utils" -import { timers } from ".." +import { utils } from "@budibase/shared-core" async function getClient( type: LockType, @@ -14,7 +13,11 @@ async function getClient( if (type === LockType.CUSTOM) { return newRedlock(opts) } - if (env.isTest() && type !== LockType.TRY_ONCE) { + if ( + env.isTest() && + type !== LockType.TRY_ONCE && + type !== LockType.AUTO_EXTEND + ) { return newRedlock(OPTIONS.TEST) } switch (type) { @@ -30,13 +33,16 @@ async function getClient( case LockType.DELAY_500: { return newRedlock(OPTIONS.DELAY_500) } + case LockType.AUTO_EXTEND: { + return newRedlock(OPTIONS.AUTO_EXTEND) + } default: { - throw new Error(`Could not get redlock client: ${type}`) + throw utils.unreachable(type) } } } -const OPTIONS = { +const OPTIONS: Record = { TRY_ONCE: { // immediately throws an error if the lock is already held retryCount: 0, @@ -69,10 +75,14 @@ const OPTIONS = { DELAY_500: { retryDelay: 500, }, + CUSTOM: {}, + AUTO_EXTEND: { + retryCount: -1, + }, } export async function newRedlock(opts: Redlock.Options = {}) { - let options = { ...OPTIONS.DEFAULT, ...opts } + let options = { ...OPTIONS, ...opts } const redisWrapper = await getLockClient() const client = redisWrapper.getClient() return new Redlock([client], options) @@ -108,20 +118,36 @@ export async function doWithLock( ): Promise> { const redlock = await getClient(opts.type, opts.customOptions) let lock: Redlock.Lock | undefined - let interval + let timeout try { const name = getLockName(opts) - let ttl = opts.ttl || Duration.fromSeconds(30).toMs() - // create the lock - lock = await redlock.lock(name, ttl) + lock = await redlock.lock(name, opts.ttl) - if (!opts.ttl) { + if (opts.type === LockType.AUTO_EXTEND) { // No TTL is provided, so we keep extending the lock while the task is running - interval = timers.set(async () => { - await lock?.extend(ttl / 2) - }, ttl / 2) + const extendInIntervals = (): void => { + timeout = setTimeout(async () => { + let isExpired = false + try { + lock = await lock!.extend(1000) + } catch (err: any) { + isExpired = err.message.includes("Cannot extend lock on resource") + if (isExpired) { + console.error("The lock expired", { name }) + } else { + throw err + } + } + + if (!isExpired) { + extendInIntervals() + } + }, opts.ttl / 2) + } + + extendInIntervals() } // perform locked task @@ -143,11 +169,11 @@ export async function doWithLock( throw e } } finally { + if (timeout) { + clearTimeout(timeout) + } if (lock) { await lock.unlock() } - if (interval) { - timers.clear(interval) - } } } diff --git a/packages/types/src/sdk/locks.ts b/packages/types/src/sdk/locks.ts index 080574b735..2a2e74c4cc 100644 --- a/packages/types/src/sdk/locks.ts +++ b/packages/types/src/sdk/locks.ts @@ -10,6 +10,7 @@ export enum LockType { DEFAULT = "default", DELAY_500 = "delay_500", CUSTOM = "custom", + AUTO_EXTEND = "auto_extend", } export enum LockName { @@ -36,9 +37,9 @@ export interface LockOptions { */ name: LockName /** - * The ttl to auto-expire the lock if not unlocked manually. If undefined, the lock will be autoextending while the process is running. + * The ttl to auto-expire the lock if not unlocked manually. */ - ttl?: number + ttl: number /** * The individual resource to lock. This is useful for locking around very specific identifiers, e.g. a document that is prone to conflicts */ From 4cd76ea0fc0abea383ca42baa817816d9fbfe068 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 29 Nov 2023 21:03:36 +0100 Subject: [PATCH 16/32] Add tests --- .../src/redis/tests/redlockImpl.spec.ts | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 packages/backend-core/src/redis/tests/redlockImpl.spec.ts diff --git a/packages/backend-core/src/redis/tests/redlockImpl.spec.ts b/packages/backend-core/src/redis/tests/redlockImpl.spec.ts new file mode 100644 index 0000000000..567c160db2 --- /dev/null +++ b/packages/backend-core/src/redis/tests/redlockImpl.spec.ts @@ -0,0 +1,44 @@ +import { LockName, LockType } from "@budibase/types" +import { doWithLock } from "../redlockImpl" +import { DBTestConfiguration } from "../../../tests" +import { Duration } from "../../utils" + +describe("redlockImpl", () => { + beforeEach(() => { + jest.useFakeTimers() + }) + + afterEach(() => { + jest.runOnlyPendingTimers() + jest.useRealTimers() + }) + + describe("doWithLock", () => { + it("should execute the task and return the result", async () => { + const mockTask = jest.fn().mockResolvedValue("mockResult") + + // Define test options + const testOpts = { + name: LockName.PERSIST_WRITETHROUGH, + type: LockType.AUTO_EXTEND, + ttl: 30000, + } + + // Call the function with the mock lock and task + const config = new DBTestConfiguration() + const result = await config.doInTenant(() => + doWithLock(testOpts, async () => { + jest.advanceTimersByTime(Duration.fromSeconds(10).toMs()) + jest.advanceTimersByTime(Duration.fromSeconds(10).toMs()) + jest.advanceTimersByTime(Duration.fromSeconds(10).toMs()) + return mockTask() + }) + ) + + // Assert the result and verify function calls + expect(result.executed).toBe(true) + expect(result.executed && result.result).toBe("mockResult") + expect(mockTask).toHaveBeenCalledTimes(1) + }) + }) +}) From f1fafc07f27400add94347fd2ad54ca906084625 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 29 Nov 2023 21:03:49 +0100 Subject: [PATCH 17/32] Update ioredis-mock --- packages/backend-core/package.json | 2 +- yarn.lock | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/backend-core/package.json b/packages/backend-core/package.json index dc8d71b52c..306aabfe6a 100644 --- a/packages/backend-core/package.json +++ b/packages/backend-core/package.json @@ -72,7 +72,7 @@ "@types/tar-fs": "2.0.1", "@types/uuid": "8.3.4", "chance": "1.1.8", - "ioredis-mock": "8.7.0", + "ioredis-mock": "8.9.0", "jest": "29.6.2", "jest-environment-node": "29.6.2", "jest-serial-runner": "1.2.1", diff --git a/yarn.lock b/yarn.lock index 700c3f9456..a09ae20de6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -12667,16 +12667,16 @@ invert-kv@^2.0.0: resolved "https://registry.yarnpkg.com/invert-kv/-/invert-kv-2.0.0.tgz#7393f5afa59ec9ff5f67a27620d11c226e3eec02" integrity sha512-wPVv/y/QQ/Uiirj/vh3oP+1Ww+AWehmi1g5fFWGPF6IpCBCDVrhgHRMvrLfdYcwDh3QJbGXDW4JAuzxElLSqKA== -ioredis-mock@8.7.0: - version "8.7.0" - resolved "https://registry.yarnpkg.com/ioredis-mock/-/ioredis-mock-8.7.0.tgz#9877a85e0d233e1b49123d1c6e320df01e9a1d36" - integrity sha512-BJcSjkR3sIMKbH93fpFzwlWi/jl1kd5I3vLvGQxnJ/W/6bD2ksrxnyQN186ljAp3Foz4p1ivViDE3rZeKEAluA== +ioredis-mock@8.9.0: + version "8.9.0" + resolved "https://registry.yarnpkg.com/ioredis-mock/-/ioredis-mock-8.9.0.tgz#5d694c4b81d3835e4291e0b527f947e260981779" + integrity sha512-yIglcCkI1lvhwJVoMsR51fotZVsPsSk07ecTCgRTRlicG0Vq3lke6aAaHklyjmRNRsdYAgswqC2A0bPtQK4LSw== dependencies: "@ioredis/as-callback" "^3.0.0" "@ioredis/commands" "^1.2.0" fengari "^0.1.4" fengari-interop "^0.1.3" - semver "^7.3.8" + semver "^7.5.4" ioredis@5.3.2: version "5.3.2" From 73fd1f66c58736ce8963226e0135d10b85e72441 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 29 Nov 2023 22:01:25 +0100 Subject: [PATCH 18/32] Fix --- packages/backend-core/src/redis/redlockImpl.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend-core/src/redis/redlockImpl.ts b/packages/backend-core/src/redis/redlockImpl.ts index 3383dbff13..8368696137 100644 --- a/packages/backend-core/src/redis/redlockImpl.ts +++ b/packages/backend-core/src/redis/redlockImpl.ts @@ -131,7 +131,7 @@ export async function doWithLock( timeout = setTimeout(async () => { let isExpired = false try { - lock = await lock!.extend(1000) + lock = await lock!.extend(opts.ttl) } catch (err: any) { isExpired = err.message.includes("Cannot extend lock on resource") if (isExpired) { From 9c12c5b62ef5e760add8e5b7c1783190dfa89c25 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 29 Nov 2023 22:01:49 +0100 Subject: [PATCH 19/32] Fix comments --- packages/backend-core/src/redis/redlockImpl.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend-core/src/redis/redlockImpl.ts b/packages/backend-core/src/redis/redlockImpl.ts index 8368696137..841da5fcd1 100644 --- a/packages/backend-core/src/redis/redlockImpl.ts +++ b/packages/backend-core/src/redis/redlockImpl.ts @@ -126,7 +126,7 @@ export async function doWithLock( lock = await redlock.lock(name, opts.ttl) if (opts.type === LockType.AUTO_EXTEND) { - // No TTL is provided, so we keep extending the lock while the task is running + // We keep extending the lock while the task is running const extendInIntervals = (): void => { timeout = setTimeout(async () => { let isExpired = false From 078384941ae68a6741ab5c0794d1b82337167d31 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 29 Nov 2023 22:09:09 +0100 Subject: [PATCH 20/32] Clean --- packages/backend-core/src/redis/redlockImpl.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/backend-core/src/redis/redlockImpl.ts b/packages/backend-core/src/redis/redlockImpl.ts index 841da5fcd1..dec8aad032 100644 --- a/packages/backend-core/src/redis/redlockImpl.ts +++ b/packages/backend-core/src/redis/redlockImpl.ts @@ -169,11 +169,7 @@ export async function doWithLock( throw e } } finally { - if (timeout) { - clearTimeout(timeout) - } - if (lock) { - await lock.unlock() - } + clearTimeout(timeout) + await lock?.unlock() } } From db6517bc0c21cd69b09e1ebcc939cf51a24549b3 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 29 Nov 2023 22:42:29 +0100 Subject: [PATCH 21/32] Use real sleeps --- .../src/redis/tests/redlockImpl.spec.ts | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/packages/backend-core/src/redis/tests/redlockImpl.spec.ts b/packages/backend-core/src/redis/tests/redlockImpl.spec.ts index 567c160db2..0661b8f7cc 100644 --- a/packages/backend-core/src/redis/tests/redlockImpl.spec.ts +++ b/packages/backend-core/src/redis/tests/redlockImpl.spec.ts @@ -1,18 +1,8 @@ import { LockName, LockType } from "@budibase/types" import { doWithLock } from "../redlockImpl" import { DBTestConfiguration } from "../../../tests" -import { Duration } from "../../utils" describe("redlockImpl", () => { - beforeEach(() => { - jest.useFakeTimers() - }) - - afterEach(() => { - jest.runOnlyPendingTimers() - jest.useRealTimers() - }) - describe("doWithLock", () => { it("should execute the task and return the result", async () => { const mockTask = jest.fn().mockResolvedValue("mockResult") @@ -21,16 +11,14 @@ describe("redlockImpl", () => { const testOpts = { name: LockName.PERSIST_WRITETHROUGH, type: LockType.AUTO_EXTEND, - ttl: 30000, + ttl: 5, } // Call the function with the mock lock and task const config = new DBTestConfiguration() const result = await config.doInTenant(() => doWithLock(testOpts, async () => { - jest.advanceTimersByTime(Duration.fromSeconds(10).toMs()) - jest.advanceTimersByTime(Duration.fromSeconds(10).toMs()) - jest.advanceTimersByTime(Duration.fromSeconds(10).toMs()) + await new Promise(r => setTimeout(() => r(), 10)) return mockTask() }) ) From c86d94968068cf6f6e0c9f4c65100dc55ef0e1cc Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 30 Nov 2023 10:33:39 +0100 Subject: [PATCH 22/32] Add and dry tests --- .../src/redis/tests/redlockImpl.spec.ts | 104 ++++++++++++++---- 1 file changed, 85 insertions(+), 19 deletions(-) diff --git a/packages/backend-core/src/redis/tests/redlockImpl.spec.ts b/packages/backend-core/src/redis/tests/redlockImpl.spec.ts index 0661b8f7cc..b3dd572ba9 100644 --- a/packages/backend-core/src/redis/tests/redlockImpl.spec.ts +++ b/packages/backend-core/src/redis/tests/redlockImpl.spec.ts @@ -1,32 +1,98 @@ -import { LockName, LockType } from "@budibase/types" +import { LockName, LockType, LockOptions } from "@budibase/types" +import tk from "timekeeper" import { doWithLock } from "../redlockImpl" -import { DBTestConfiguration } from "../../../tests" +import { DBTestConfiguration, generator } from "../../../tests" + +tk.reset() describe("redlockImpl", () => { describe("doWithLock", () => { - it("should execute the task and return the result", async () => { - const mockTask = jest.fn().mockResolvedValue("mockResult") + const config = new DBTestConfiguration() + const lockTtl = 30 - // Define test options - const testOpts = { - name: LockName.PERSIST_WRITETHROUGH, - type: LockType.AUTO_EXTEND, - ttl: 5, - } - - // Call the function with the mock lock and task - const config = new DBTestConfiguration() - const result = await config.doInTenant(() => - doWithLock(testOpts, async () => { - await new Promise(r => setTimeout(() => r(), 10)) - return mockTask() + function runLockWithExecutionTime({ + opts, + task, + executionTimeMs, + }: { + opts: LockOptions + task: () => Promise + executionTimeMs: number + }) { + return config.doInTenant(() => + doWithLock(opts, async () => { + await new Promise(r => setTimeout(() => r(), executionTimeMs)) + return task() }) ) + } + + it.each(Object.values(LockType))( + "should return the task value", + async (lockType: LockType) => { + const expectedResult = generator.guid() + const mockTask = jest.fn().mockResolvedValue(expectedResult) + + const opts = { + name: LockName.PERSIST_WRITETHROUGH, + type: lockType, + ttl: lockTtl, + } + + const result = await runLockWithExecutionTime({ + opts, + task: mockTask, + executionTimeMs: 0, + }) + + expect(result.executed).toBe(true) + expect(result.executed && result.result).toBe(expectedResult) + expect(mockTask).toHaveBeenCalledTimes(1) + } + ) + + it("should extend when type is autoextend", async () => { + const expectedResult = generator.guid() + const mockTask = jest.fn().mockResolvedValue(expectedResult) + + const opts = { + name: LockName.PERSIST_WRITETHROUGH, + type: LockType.AUTO_EXTEND, + ttl: lockTtl, + } + + const result = await runLockWithExecutionTime({ + opts, + task: mockTask, + executionTimeMs: lockTtl * 2, + }) - // Assert the result and verify function calls expect(result.executed).toBe(true) - expect(result.executed && result.result).toBe("mockResult") + expect(result.executed && result.result).toBe(expectedResult) expect(mockTask).toHaveBeenCalledTimes(1) }) + + it.each(Object.values(LockType).filter(t => t !== LockType.AUTO_EXTEND))( + "should timeout when type is %s", + async (lockType: LockType) => { + const mockTask = jest.fn().mockResolvedValue("mockResult") + + const opts = { + name: LockName.PERSIST_WRITETHROUGH, + type: lockType, + ttl: lockTtl, + } + + await expect( + runLockWithExecutionTime({ + opts, + task: mockTask, + executionTimeMs: lockTtl * 2, + }) + ).rejects.toThrowError( + `Unable to fully release the lock on resource \"lock:${config.tenantId}_persist_writethrough\".` + ) + } + ) }) }) From dcb6933eaac9ac5ddcb7e103bd2ffd36f971634e Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 30 Nov 2023 11:20:43 +0100 Subject: [PATCH 23/32] Clean --- packages/backend-core/src/redis/tests/redlockImpl.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/backend-core/src/redis/tests/redlockImpl.spec.ts b/packages/backend-core/src/redis/tests/redlockImpl.spec.ts index b3dd572ba9..b9d11c20a0 100644 --- a/packages/backend-core/src/redis/tests/redlockImpl.spec.ts +++ b/packages/backend-core/src/redis/tests/redlockImpl.spec.ts @@ -8,7 +8,7 @@ tk.reset() describe("redlockImpl", () => { describe("doWithLock", () => { const config = new DBTestConfiguration() - const lockTtl = 30 + const lockTtl = 25 function runLockWithExecutionTime({ opts, @@ -28,7 +28,7 @@ describe("redlockImpl", () => { } it.each(Object.values(LockType))( - "should return the task value", + "should return the task value and release the lock", async (lockType: LockType) => { const expectedResult = generator.guid() const mockTask = jest.fn().mockResolvedValue(expectedResult) @@ -64,7 +64,7 @@ describe("redlockImpl", () => { const result = await runLockWithExecutionTime({ opts, task: mockTask, - executionTimeMs: lockTtl * 2, + executionTimeMs: lockTtl * 3, }) expect(result.executed).toBe(true) From bbe41e04a2aa2ae704700b11e111e7df0d4809cc Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 30 Nov 2023 11:56:04 +0100 Subject: [PATCH 24/32] Update openapi specs --- packages/server/specs/openapi.json | 2 +- packages/server/specs/openapi.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/server/specs/openapi.json b/packages/server/specs/openapi.json index a6900adea7..da532802ab 100644 --- a/packages/server/specs/openapi.json +++ b/packages/server/specs/openapi.json @@ -2152,7 +2152,7 @@ "/applications/{appId}/publish": { "post": { "operationId": "appPublish", - "summary": "Unpublish an application", + "summary": "Publish an application", "tags": [ "applications" ], diff --git a/packages/server/specs/openapi.yaml b/packages/server/specs/openapi.yaml index ad02a3cd9c..7543641ba6 100644 --- a/packages/server/specs/openapi.yaml +++ b/packages/server/specs/openapi.yaml @@ -1761,7 +1761,7 @@ paths: "/applications/{appId}/publish": post: operationId: appPublish - summary: Unpublish an application + summary: Publish an application tags: - applications parameters: From 57b4c08731f0562264bc64c89574379d061be966 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 30 Nov 2023 11:57:29 +0100 Subject: [PATCH 25/32] Increase timeouts --- packages/backend-core/src/redis/tests/redlockImpl.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend-core/src/redis/tests/redlockImpl.spec.ts b/packages/backend-core/src/redis/tests/redlockImpl.spec.ts index b9d11c20a0..9d312721b1 100644 --- a/packages/backend-core/src/redis/tests/redlockImpl.spec.ts +++ b/packages/backend-core/src/redis/tests/redlockImpl.spec.ts @@ -8,7 +8,7 @@ tk.reset() describe("redlockImpl", () => { describe("doWithLock", () => { const config = new DBTestConfiguration() - const lockTtl = 25 + const lockTtl = 50 function runLockWithExecutionTime({ opts, From 25099ee28ef8bb004d8bf5508fecb901c983dfb5 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 30 Nov 2023 12:02:47 +0100 Subject: [PATCH 26/32] Clean redlock test settings --- packages/backend-core/src/redis/redlockImpl.ts | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/packages/backend-core/src/redis/redlockImpl.ts b/packages/backend-core/src/redis/redlockImpl.ts index dec8aad032..a4586a4495 100644 --- a/packages/backend-core/src/redis/redlockImpl.ts +++ b/packages/backend-core/src/redis/redlockImpl.ts @@ -13,13 +13,7 @@ async function getClient( if (type === LockType.CUSTOM) { return newRedlock(opts) } - if ( - env.isTest() && - type !== LockType.TRY_ONCE && - type !== LockType.AUTO_EXTEND - ) { - return newRedlock(OPTIONS.TEST) - } + switch (type) { case LockType.TRY_ONCE: { return newRedlock(OPTIONS.TRY_ONCE) @@ -42,7 +36,7 @@ async function getClient( } } -const OPTIONS: Record = { +const OPTIONS: Record = { TRY_ONCE: { // immediately throws an error if the lock is already held retryCount: 0, @@ -50,11 +44,6 @@ const OPTIONS: Record = { TRY_TWICE: { retryCount: 1, }, - TEST: { - // higher retry count in unit tests - // due to high contention. - retryCount: 100, - }, DEFAULT: { // the expected clock drift; for more details // see http://redis.io/topics/distlock From e81e37b613d7fa765a2a6ff57b46f05b456db0b2 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 30 Nov 2023 12:07:14 +0100 Subject: [PATCH 27/32] Clean comments --- packages/types/src/sdk/locks.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/types/src/sdk/locks.ts b/packages/types/src/sdk/locks.ts index 2a2e74c4cc..a04b8238a9 100644 --- a/packages/types/src/sdk/locks.ts +++ b/packages/types/src/sdk/locks.ts @@ -37,7 +37,7 @@ export interface LockOptions { */ name: LockName /** - * The ttl to auto-expire the lock if not unlocked manually. + * The ttl to auto-expire the lock if not unlocked manually */ ttl: number /** From a8ac4eed6dc072d8f903c2f8e17e734a827b84b9 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 30 Nov 2023 13:54:42 +0100 Subject: [PATCH 28/32] Autoextend without ttl --- .../backend-core/src/redis/redlockImpl.ts | 13 +++++++++---- packages/types/src/sdk/locks.ts | 19 +++++++++++++------ 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/packages/backend-core/src/redis/redlockImpl.ts b/packages/backend-core/src/redis/redlockImpl.ts index a4586a4495..2cabeccf1d 100644 --- a/packages/backend-core/src/redis/redlockImpl.ts +++ b/packages/backend-core/src/redis/redlockImpl.ts @@ -2,9 +2,9 @@ import Redlock from "redlock" import { getLockClient } from "./init" import { LockOptions, LockType } from "@budibase/types" import * as context from "../context" -import env from "../environment" import { logWarn } from "../logging" import { utils } from "@budibase/shared-core" +import { Duration } from "../utils" async function getClient( type: LockType, @@ -111,8 +111,13 @@ export async function doWithLock( try { const name = getLockName(opts) + const ttl = + opts.type === LockType.AUTO_EXTEND + ? Duration.fromSeconds(10).toMs() + : opts.ttl + // create the lock - lock = await redlock.lock(name, opts.ttl) + lock = await redlock.lock(name, ttl) if (opts.type === LockType.AUTO_EXTEND) { // We keep extending the lock while the task is running @@ -120,7 +125,7 @@ export async function doWithLock( timeout = setTimeout(async () => { let isExpired = false try { - lock = await lock!.extend(opts.ttl) + lock = await lock!.extend(ttl) } catch (err: any) { isExpired = err.message.includes("Cannot extend lock on resource") if (isExpired) { @@ -133,7 +138,7 @@ export async function doWithLock( if (!isExpired) { extendInIntervals() } - }, opts.ttl / 2) + }, ttl / 2) } extendInIntervals() diff --git a/packages/types/src/sdk/locks.ts b/packages/types/src/sdk/locks.ts index a04b8238a9..b9470402c1 100644 --- a/packages/types/src/sdk/locks.ts +++ b/packages/types/src/sdk/locks.ts @@ -22,7 +22,7 @@ export enum LockName { QUOTA_USAGE_EVENT = "quota_usage_event", } -export interface LockOptions { +export type LockOptions = { /** * The lock type determines which client to use */ @@ -36,10 +36,6 @@ export interface LockOptions { * The name for the lock */ name: LockName - /** - * The ttl to auto-expire the lock if not unlocked manually - */ - ttl: number /** * The individual resource to lock. This is useful for locking around very specific identifiers, e.g. a document that is prone to conflicts */ @@ -48,4 +44,15 @@ export interface LockOptions { * This is a system-wide lock - don't use tenancy in lock key */ systemLock?: boolean -} +} & ( + | { + /** + * The ttl to auto-expire the lock if not unlocked manually + */ + ttl: number + type: Exclude + } + | { + type: LockType.AUTO_EXTEND + } +) From bd89633e61c1720b6e19d2686f80faf17d306b29 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 30 Nov 2023 18:23:35 +0100 Subject: [PATCH 29/32] Fix wrong commited code --- packages/backend-core/src/redis/redlockImpl.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend-core/src/redis/redlockImpl.ts b/packages/backend-core/src/redis/redlockImpl.ts index 2cabeccf1d..36504ea916 100644 --- a/packages/backend-core/src/redis/redlockImpl.ts +++ b/packages/backend-core/src/redis/redlockImpl.ts @@ -71,7 +71,7 @@ const OPTIONS: Record = { } export async function newRedlock(opts: Redlock.Options = {}) { - let options = { ...OPTIONS, ...opts } + const options = { ...OPTIONS.DEFAULT, ...opts } const redisWrapper = await getLockClient() const client = redisWrapper.getClient() return new Redlock([client], options) From fb72b77ac19491a9f06994014eed0c9145661a70 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 30 Nov 2023 18:34:56 +0100 Subject: [PATCH 30/32] Use jest.useFakeTimers --- .../backend-core/src/redis/redlockImpl.ts | 24 ++++------------ .../src/redis/tests/redlockImpl.spec.ts | 28 +++++++++++-------- packages/types/src/sdk/locks.ts | 1 + 3 files changed, 24 insertions(+), 29 deletions(-) diff --git a/packages/backend-core/src/redis/redlockImpl.ts b/packages/backend-core/src/redis/redlockImpl.ts index 36504ea916..a65dcd0c3f 100644 --- a/packages/backend-core/src/redis/redlockImpl.ts +++ b/packages/backend-core/src/redis/redlockImpl.ts @@ -101,20 +101,20 @@ function getLockName(opts: LockOptions) { return name } +export const autoExtendPollingMs = Duration.fromSeconds(10).toMs() + export async function doWithLock( opts: LockOptions, task: () => Promise ): Promise> { const redlock = await getClient(opts.type, opts.customOptions) let lock: Redlock.Lock | undefined - let timeout + let timeout: NodeJS.Timeout | undefined try { const name = getLockName(opts) const ttl = - opts.type === LockType.AUTO_EXTEND - ? Duration.fromSeconds(10).toMs() - : opts.ttl + opts.type === LockType.AUTO_EXTEND ? autoExtendPollingMs : opts.ttl // create the lock lock = await redlock.lock(name, ttl) @@ -123,21 +123,9 @@ export async function doWithLock( // We keep extending the lock while the task is running const extendInIntervals = (): void => { timeout = setTimeout(async () => { - let isExpired = false - try { - lock = await lock!.extend(ttl) - } catch (err: any) { - isExpired = err.message.includes("Cannot extend lock on resource") - if (isExpired) { - console.error("The lock expired", { name }) - } else { - throw err - } - } + lock = await lock!.extend(ttl, () => opts.onExtend && opts.onExtend()) - if (!isExpired) { - extendInIntervals() - } + extendInIntervals() }, ttl / 2) } diff --git a/packages/backend-core/src/redis/tests/redlockImpl.spec.ts b/packages/backend-core/src/redis/tests/redlockImpl.spec.ts index 9d312721b1..6f894c2951 100644 --- a/packages/backend-core/src/redis/tests/redlockImpl.spec.ts +++ b/packages/backend-core/src/redis/tests/redlockImpl.spec.ts @@ -1,14 +1,15 @@ import { LockName, LockType, LockOptions } from "@budibase/types" -import tk from "timekeeper" -import { doWithLock } from "../redlockImpl" +import { autoExtendPollingMs, doWithLock } from "../redlockImpl" import { DBTestConfiguration, generator } from "../../../tests" -tk.reset() - describe("redlockImpl", () => { + beforeEach(() => { + jest.useFakeTimers() + }) + describe("doWithLock", () => { const config = new DBTestConfiguration() - const lockTtl = 50 + const lockTtl = autoExtendPollingMs function runLockWithExecutionTime({ opts, @@ -21,7 +22,10 @@ describe("redlockImpl", () => { }) { return config.doInTenant(() => doWithLock(opts, async () => { - await new Promise(r => setTimeout(() => r(), executionTimeMs)) + const interval = lockTtl / 10 + for (let i = executionTimeMs; i > 0; i -= interval) { + await jest.advanceTimersByTimeAsync(interval) + } return task() }) ) @@ -33,7 +37,7 @@ describe("redlockImpl", () => { const expectedResult = generator.guid() const mockTask = jest.fn().mockResolvedValue(expectedResult) - const opts = { + const opts: LockOptions = { name: LockName.PERSIST_WRITETHROUGH, type: lockType, ttl: lockTtl, @@ -54,22 +58,24 @@ describe("redlockImpl", () => { it("should extend when type is autoextend", async () => { const expectedResult = generator.guid() const mockTask = jest.fn().mockResolvedValue(expectedResult) + const mockOnExtend = jest.fn() - const opts = { + const opts: LockOptions = { name: LockName.PERSIST_WRITETHROUGH, type: LockType.AUTO_EXTEND, - ttl: lockTtl, + onExtend: mockOnExtend, } const result = await runLockWithExecutionTime({ opts, task: mockTask, - executionTimeMs: lockTtl * 3, + executionTimeMs: lockTtl * 2.5, }) expect(result.executed).toBe(true) expect(result.executed && result.result).toBe(expectedResult) expect(mockTask).toHaveBeenCalledTimes(1) + expect(mockOnExtend).toHaveBeenCalledTimes(5) }) it.each(Object.values(LockType).filter(t => t !== LockType.AUTO_EXTEND))( @@ -77,7 +83,7 @@ describe("redlockImpl", () => { async (lockType: LockType) => { const mockTask = jest.fn().mockResolvedValue("mockResult") - const opts = { + const opts: LockOptions = { name: LockName.PERSIST_WRITETHROUGH, type: lockType, ttl: lockTtl, diff --git a/packages/types/src/sdk/locks.ts b/packages/types/src/sdk/locks.ts index b9470402c1..6ff91d4315 100644 --- a/packages/types/src/sdk/locks.ts +++ b/packages/types/src/sdk/locks.ts @@ -54,5 +54,6 @@ export type LockOptions = { } | { type: LockType.AUTO_EXTEND + onExtend?: () => void } ) From 670853a0ea074635c49d086dd2def605a8753255 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 30 Nov 2023 18:36:36 +0100 Subject: [PATCH 31/32] Renames --- packages/backend-core/src/redis/redlockImpl.ts | 4 ++-- packages/backend-core/src/redis/tests/redlockImpl.spec.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/backend-core/src/redis/redlockImpl.ts b/packages/backend-core/src/redis/redlockImpl.ts index a65dcd0c3f..a7b2e2b4c6 100644 --- a/packages/backend-core/src/redis/redlockImpl.ts +++ b/packages/backend-core/src/redis/redlockImpl.ts @@ -101,7 +101,7 @@ function getLockName(opts: LockOptions) { return name } -export const autoExtendPollingMs = Duration.fromSeconds(10).toMs() +export const AUTO_EXTEND_POLLING_MS = Duration.fromSeconds(10).toMs() export async function doWithLock( opts: LockOptions, @@ -114,7 +114,7 @@ export async function doWithLock( const name = getLockName(opts) const ttl = - opts.type === LockType.AUTO_EXTEND ? autoExtendPollingMs : opts.ttl + opts.type === LockType.AUTO_EXTEND ? AUTO_EXTEND_POLLING_MS : opts.ttl // create the lock lock = await redlock.lock(name, ttl) diff --git a/packages/backend-core/src/redis/tests/redlockImpl.spec.ts b/packages/backend-core/src/redis/tests/redlockImpl.spec.ts index 6f894c2951..70920366ee 100644 --- a/packages/backend-core/src/redis/tests/redlockImpl.spec.ts +++ b/packages/backend-core/src/redis/tests/redlockImpl.spec.ts @@ -1,5 +1,5 @@ import { LockName, LockType, LockOptions } from "@budibase/types" -import { autoExtendPollingMs, doWithLock } from "../redlockImpl" +import { AUTO_EXTEND_POLLING_MS, doWithLock } from "../redlockImpl" import { DBTestConfiguration, generator } from "../../../tests" describe("redlockImpl", () => { @@ -9,7 +9,7 @@ describe("redlockImpl", () => { describe("doWithLock", () => { const config = new DBTestConfiguration() - const lockTtl = autoExtendPollingMs + const lockTtl = AUTO_EXTEND_POLLING_MS function runLockWithExecutionTime({ opts, From 12015c79aec963229de50ec31aa4242419a08d10 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 30 Nov 2023 18:37:34 +0100 Subject: [PATCH 32/32] Add comments --- packages/backend-core/src/redis/tests/redlockImpl.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/backend-core/src/redis/tests/redlockImpl.spec.ts b/packages/backend-core/src/redis/tests/redlockImpl.spec.ts index 70920366ee..a1e83d8e6c 100644 --- a/packages/backend-core/src/redis/tests/redlockImpl.spec.ts +++ b/packages/backend-core/src/redis/tests/redlockImpl.spec.ts @@ -22,6 +22,7 @@ describe("redlockImpl", () => { }) { return config.doInTenant(() => doWithLock(opts, async () => { + // Run in multiple intervals until hitting the expected time const interval = lockTtl / 10 for (let i = executionTimeMs; i > 0; i -= interval) { await jest.advanceTimersByTimeAsync(interval)