From 151ed604f81d9cc221033ac27cafbf86f3b1a015 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 20 Apr 2022 23:10:39 +0100 Subject: [PATCH] Fixing some issues detected by the test cases, making the in-use mechanism for context more clear to complete avoid stack up of contexts (leading to loss of knowledge around previous databases. --- packages/backend-core/src/context/index.js | 39 +++++++++++++++---- packages/backend-core/src/db/index.js | 8 ++-- .../src/api/controllers/deploy/index.js | 16 ++++---- .../server/src/api/routes/tests/row.spec.js | 26 +++++++------ .../src/tests/utilities/TestConfiguration.js | 2 +- 5 files changed, 60 insertions(+), 31 deletions(-) diff --git a/packages/backend-core/src/context/index.js b/packages/backend-core/src/context/index.js index d56fdebd5e..0857715bad 100644 --- a/packages/backend-core/src/context/index.js +++ b/packages/backend-core/src/context/index.js @@ -23,6 +23,8 @@ const ContextKeys = { // get the dev app DB from the request DEV_DB: "devDb", DB_OPTS: "dbOpts", + // check if something else is using the context, don't close DB + IN_USE: "inUse", } exports.DEFAULT_TENANT_ID = DEFAULT_TENANT_ID @@ -41,6 +43,15 @@ async function closeAppDBs() { continue } await closeDB(db) + // clear the DB from context, incase someone tries to use it again + cls.setOnContext(dbKey, null) + } + // clear the app ID now that the databases are closed + if (cls.getFromContext(ContextKeys.APP_ID)) { + cls.setOnContext(ContextKeys.APP_ID, null) + } + if (cls.getFromContext(ContextKeys.DB_OPTS)) { + cls.setOnContext(ContextKeys.DB_OPTS, null) } } @@ -67,15 +78,24 @@ exports.doInTenant = (tenantId, task) => { // invoke the task return await task() } finally { - if (!opts.existing) { + const using = cls.getFromContext(ContextKeys.IN_USE) + if (!using || using <= 1) { await closeDB(exports.getGlobalDB()) + // clear from context now that database is closed/task is finished + cls.setOnContext(ContextKeys.TENANT_ID, null) + cls.setOnContext(ContextKeys.GLOBAL_DB, null) + } else { + cls.setOnContext(using - 1) } } } - if (cls.getFromContext(ContextKeys.TENANT_ID) === tenantId) { + const using = cls.getFromContext(ContextKeys.IN_USE) + if (using) { + cls.setOnContext(ContextKeys.IN_USE, using + 1) return internal({ existing: true }) } else { return cls.run(async () => { + cls.setOnContext(ContextKeys.IN_USE, 1) return internal() }) } @@ -111,6 +131,7 @@ exports.doInAppContext = (appId, task) => { if (!appId) { throw new Error("appId is required") } + // the internal function is so that we can re-use an existing // context - don't want to close DB on a parent context async function internal(opts = { existing: false }) { @@ -124,15 +145,21 @@ exports.doInAppContext = (appId, task) => { // invoke the task return await task() } finally { - if (!opts.existing) { + const using = cls.getFromContext(ContextKeys.IN_USE) + if (!using || using <= 1) { await closeAppDBs() + } else { + cls.setOnContext(using - 1) } } } - if (appId === cls.getFromContext(ContextKeys.APP_ID)) { + const using = cls.getFromContext(ContextKeys.IN_USE) + if (using) { + cls.setOnContext(ContextKeys.IN_USE, using + 1) return internal({ existing: true }) } else { return cls.run(async () => { + cls.setOnContext(ContextKeys.IN_USE, 1) return internal() }) } @@ -147,10 +174,6 @@ exports.updateAppId = appId => { // have to close first, before removing the databases from context const promise = closeAppDBs() cls.setOnContext(ContextKeys.APP_ID, appId) - cls.setOnContext(ContextKeys.PROD_DB, null) - cls.setOnContext(ContextKeys.DEV_DB, null) - cls.setOnContext(ContextKeys.CURRENT_DB, null) - cls.setOnContext(ContextKeys.DB_OPTS, null) return promise } catch (err) { if (env.isTest()) { diff --git a/packages/backend-core/src/db/index.js b/packages/backend-core/src/db/index.js index 1669ea09bb..ab4acfccbf 100644 --- a/packages/backend-core/src/db/index.js +++ b/packages/backend-core/src/db/index.js @@ -53,9 +53,11 @@ exports.doWithDB = async (dbName, cb, opts) => { const db = exports.dangerousGetDB(dbName, opts) // need this to be async so that we can correctly close DB after all // async operations have been completed - const resp = await cb(db) - await exports.closeDB(db) - return resp + try { + return await cb(db) + } finally { + await exports.closeDB(db) + } } exports.allDbs = () => { diff --git a/packages/server/src/api/controllers/deploy/index.js b/packages/server/src/api/controllers/deploy/index.js index c9dd5e7e8a..791d04e8c4 100644 --- a/packages/server/src/api/controllers/deploy/index.js +++ b/packages/server/src/api/controllers/deploy/index.js @@ -94,15 +94,15 @@ async function initDeployedApp(prodAppId) { } async function deployApp(deployment) { - const appId = getAppId() - const devAppId = getDevelopmentAppID(appId) - const productionAppId = getProdAppID(appId) - const replication = new Replication({ - source: devAppId, - target: productionAppId, - }) - + let replication try { + const appId = getAppId() + const devAppId = getDevelopmentAppID(appId) + const productionAppId = getProdAppID(appId) + replication = new Replication({ + source: devAppId, + target: productionAppId, + }) console.log("Replication object created") await replication.replicate() console.log("replication complete.. replacing app meta doc") diff --git a/packages/server/src/api/routes/tests/row.spec.js b/packages/server/src/api/routes/tests/row.spec.js index 8354f01ad7..d7ec995edb 100644 --- a/packages/server/src/api/routes/tests/row.spec.js +++ b/packages/server/src/api/routes/tests/row.spec.js @@ -2,6 +2,7 @@ const { outputProcessing } = require("../../../utilities/rowProcessor") const setup = require("./utilities") const { basicRow } = setup.structures const { doInAppContext } = require("@budibase/backend-core/context") +const { doInTenant } = require("@budibase/backend-core/tenancy") // mock the fetch for the search system jest.mock("node-fetch") @@ -340,17 +341,20 @@ describe("/rows", () => { describe("fetchEnrichedRows", () => { it("should allow enriching some linked rows", async () => { - const table = await config.createLinkedTable() - const firstRow = await config.createRow({ - name: "Test Contact", - description: "original description", - tableId: table._id - }) - const secondRow = await config.createRow({ - name: "Test 2", - description: "og desc", - link: [{_id: firstRow._id}], - tableId: table._id, + const { table, firstRow, secondRow } = await doInTenant(setup.structures.TENANT_ID, async () => { + const table = await config.createLinkedTable() + const firstRow = await config.createRow({ + name: "Test Contact", + description: "original description", + tableId: table._id + }) + const secondRow = await config.createRow({ + name: "Test 2", + description: "og desc", + link: [{_id: firstRow._id}], + tableId: table._id, + }) + return { table, firstRow, secondRow } }) // test basic enrichment diff --git a/packages/server/src/tests/utilities/TestConfiguration.js b/packages/server/src/tests/utilities/TestConfiguration.js index 063806b144..ca8c7ec06b 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.js +++ b/packages/server/src/tests/utilities/TestConfiguration.js @@ -203,7 +203,7 @@ class TestConfiguration { // create dev app this.app = await this._req({ name: appName }, null, controllers.app.create) this.appId = this.app.appId - context.updateAppId(this.appId) + await context.updateAppId(this.appId) // create production app this.prodApp = await this.deploy()