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

@@ -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()) {

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)
+  try {
+    return await cb(db)
+  } finally {
     await exports.closeDB(db)
-  return resp
+  }
 }
 
 exports.allDbs = () => {

packages/server/src/api/controllers/deploy/index.js

@@ -94,15 +94,15 @@ async function initDeployedApp(prodAppId) {
 }
 
 async function deployApp(deployment) {
+  let replication
+  try {
     const appId = getAppId()
     const devAppId = getDevelopmentAppID(appId)
     const productionAppId = getProdAppID(appId)
-  const replication = new Replication({
+    replication = new Replication({
       source: devAppId,
       target: productionAppId,
     })
-
-  try {
     console.log("Replication object created")
     await replication.replicate()
     console.log("replication complete.. replacing app meta doc")

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,6 +341,7 @@ describe("/rows", () => {
 
   describe("fetchEnrichedRows", () => {
     it("should allow enriching some linked rows", async () => {
+      const { table, firstRow, secondRow } = await doInTenant(setup.structures.TENANT_ID, async () => {
         const table = await config.createLinkedTable()
         const firstRow = await config.createRow({
           name: "Test Contact",
@@ -352,6 +354,8 @@ describe("/rows", () => {
           link: [{_id: firstRow._id}],
           tableId: table._id,
         })
+        return { table, firstRow, secondRow }
+      })
 
       // test basic enrichment
       const resBasic = await request

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()