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.

This commit is contained in:
mike12345567 2022-04-20 23:10:39 +01:00
parent 6afe3a28ef
commit 151ed604f8
5 changed files with 60 additions and 31 deletions

View File

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

View File

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

View File

@ -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")

View File

@ -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

View File

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