From fe846f86a5506ab7c3d0231053b5a2c579347c35 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 21 Apr 2022 14:56:14 +0100 Subject: [PATCH] Fixing issues with test cases - a lot of test cases didn't setup tenancy in any format, which now means that the API endpoints they call would not have access to a Global DB instance. Also had to disable the closing of the database in test as this was removing the database from memory, meaning future calls would find an empty database when they opened it. --- packages/backend-core/src/context/index.js | 5 +- packages/backend-core/src/db/index.js | 3 +- .../server/src/api/controllers/application.js | 2 +- .../src/api/controllers/row/internal.js | 3 +- .../server/src/api/controllers/webhook.js | 4 +- .../src/automations/tests/deleteRow.spec.js | 2 +- .../src/integrations/tests/airtable.spec.js | 4 +- .../src/integrations/tests/couchdb.spec.js | 5 +- .../src/middleware/tests/usageQuota.spec.js | 26 +++-------- .../functions/tests/appUrls.spec.js | 2 +- .../tests/userEmailViewCasing.spec.js | 23 ++++++---- .../usageQuotas/tests/syncApps.spec.js | 46 ++++++++++--------- .../usageQuotas/tests/syncRows.spec.js | 44 +++++++++--------- .../src/tests/utilities/TestConfiguration.js | 33 ++++++++----- .../server/src/utilities/usageQuota/rows.js | 2 +- 15 files changed, 108 insertions(+), 96 deletions(-) diff --git a/packages/backend-core/src/context/index.js b/packages/backend-core/src/context/index.js index 2a49353770..906ae9cce5 100644 --- a/packages/backend-core/src/context/index.js +++ b/packages/backend-core/src/context/index.js @@ -169,12 +169,11 @@ exports.updateTenantId = tenantId => { cls.setOnContext(ContextKeys.TENANT_ID, tenantId) } -exports.updateAppId = appId => { +exports.updateAppId = async appId => { try { // have to close first, before removing the databases from context - const promise = closeAppDBs() + await closeAppDBs() cls.setOnContext(ContextKeys.APP_ID, appId) - return promise } catch (err) { if (env.isTest()) { TEST_APP_ID = appId diff --git a/packages/backend-core/src/db/index.js b/packages/backend-core/src/db/index.js index ab4acfccbf..1a29c8c2b0 100644 --- a/packages/backend-core/src/db/index.js +++ b/packages/backend-core/src/db/index.js @@ -1,4 +1,5 @@ const pouch = require("./pouch") +const env = require("../environment") let PouchDB let initialised = false @@ -36,7 +37,7 @@ exports.dangerousGetDB = (dbName, opts) => { // use this function if you have called dangerousGetDB - close // the databases you've opened once finished exports.closeDB = async db => { - if (!db) { + if (!db || env.isTest()) { return } try { diff --git a/packages/server/src/api/controllers/application.js b/packages/server/src/api/controllers/application.js index 0e950cd502..ae1071124d 100644 --- a/packages/server/src/api/controllers/application.js +++ b/packages/server/src/api/controllers/application.js @@ -123,7 +123,7 @@ async function createInstance(template) { const tenantId = isMultiTenant() ? getTenantId() : null const baseAppId = generateAppID(tenantId) const appId = generateDevAppID(baseAppId) - updateAppId(appId) + await updateAppId(appId) const db = getAppDB() await db.put({ diff --git a/packages/server/src/api/controllers/row/internal.js b/packages/server/src/api/controllers/row/internal.js index 68ff4ca88d..8083b10584 100644 --- a/packages/server/src/api/controllers/row/internal.js +++ b/packages/server/src/api/controllers/row/internal.js @@ -6,6 +6,7 @@ const { DocumentTypes, InternalTables, } = require("../../../db/utils") +const { dangerousGetDB } = require("@budibase/backend-core/db") const userController = require("../user") const { inputProcessing, @@ -250,7 +251,7 @@ exports.fetch = async ctx => { } exports.find = async ctx => { - const db = getAppDB() + const db = dangerousGetDB(ctx.appId) const table = await db.get(ctx.params.tableId) let row = await findRow(ctx, ctx.params.tableId, ctx.params.rowId) row = await outputProcessing(table, row) diff --git a/packages/server/src/api/controllers/webhook.js b/packages/server/src/api/controllers/webhook.js index 67427e5710..1698775ab4 100644 --- a/packages/server/src/api/controllers/webhook.js +++ b/packages/server/src/api/controllers/webhook.js @@ -54,7 +54,7 @@ exports.destroy = async ctx => { } exports.buildSchema = async ctx => { - updateAppId(ctx.params.instance) + await updateAppId(ctx.params.instance) const db = getAppDB() const webhook = await db.get(ctx.params.id) webhook.bodySchema = toJsonSchema(ctx.request.body) @@ -80,7 +80,7 @@ exports.buildSchema = async ctx => { exports.trigger = async ctx => { const prodAppId = getProdAppID(ctx.params.instance) - updateAppId(prodAppId) + await updateAppId(prodAppId) try { const db = getAppDB() const webhook = await db.get(ctx.params.id) diff --git a/packages/server/src/automations/tests/deleteRow.spec.js b/packages/server/src/automations/tests/deleteRow.spec.js index 21246f22d0..a88de65871 100644 --- a/packages/server/src/automations/tests/deleteRow.spec.js +++ b/packages/server/src/automations/tests/deleteRow.spec.js @@ -27,7 +27,7 @@ describe("test the delete row action", () => { expect(res.row._id).toEqual(row._id) let error try { - await config.getRow(table._id, res.id) + await config.getRow(table._id, res.row._id) } catch (err) { error = err } diff --git a/packages/server/src/integrations/tests/airtable.spec.js b/packages/server/src/integrations/tests/airtable.spec.js index e6654f6f71..4769430ded 100644 --- a/packages/server/src/integrations/tests/airtable.spec.js +++ b/packages/server/src/integrations/tests/airtable.spec.js @@ -7,7 +7,9 @@ class TestConfiguration { this.integration = new AirtableIntegration.integration(config) this.client = { create: jest.fn(), - select: jest.fn(), + select: jest.fn(() => ({ + firstPage: jest.fn(() => []), + })), update: jest.fn(), destroy: jest.fn(), } diff --git a/packages/server/src/integrations/tests/couchdb.spec.js b/packages/server/src/integrations/tests/couchdb.spec.js index e21b67fa1c..65c9c83ad2 100644 --- a/packages/server/src/integrations/tests/couchdb.spec.js +++ b/packages/server/src/integrations/tests/couchdb.spec.js @@ -2,8 +2,10 @@ jest.mock("pouchdb", () => function CouchDBMock() { this.post = jest.fn() this.allDocs = jest.fn(() => ({ rows: [] })) this.put = jest.fn() + this.get = jest.fn() this.remove = jest.fn() this.plugin = jest.fn() + this.close = jest.fn() }) const CouchDBIntegration = require("../couchdb") @@ -62,6 +64,7 @@ describe("CouchDB Integration", () => { it("calls the delete method with the correct params", async () => { const id = "1234" const response = await config.integration.delete({ id }) - expect(config.integration.client.remove).toHaveBeenCalledWith(id) + expect(config.integration.client.get).toHaveBeenCalledWith(id) + expect(config.integration.client.remove).toHaveBeenCalled() }) }) \ No newline at end of file diff --git a/packages/server/src/middleware/tests/usageQuota.spec.js b/packages/server/src/middleware/tests/usageQuota.spec.js index 1282615a50..09e31a9053 100644 --- a/packages/server/src/middleware/tests/usageQuota.spec.js +++ b/packages/server/src/middleware/tests/usageQuota.spec.js @@ -6,7 +6,10 @@ jest.mock("@budibase/backend-core/tenancy", () => ({ const usageQuotaMiddleware = require("../usageQuota") const usageQuota = require("../../utilities/usageQuota") -const CouchDB = require("../../db") + +jest.mock("@budibase/backend-core/db") +const { dangerousGetDB } = require("@budibase/backend-core/db") + const env = require("../../environment") class TestConfiguration { @@ -80,7 +83,7 @@ describe("usageQuota middleware", () => { _rev: "test", }) - CouchDB.mockImplementationOnce(() => ({ + dangerousGetDB.mockImplementationOnce(() => ({ get: async () => true })) @@ -96,7 +99,7 @@ describe("usageQuota middleware", () => { }) config.setProd(true) - CouchDB.mockImplementationOnce(() => ({ + dangerousGetDB.mockImplementationOnce(() => ({ get: async () => { throw new Error() } @@ -114,21 +117,4 @@ describe("usageQuota middleware", () => { expect(usageQuota.update).toHaveBeenCalledWith("rows", 1) expect(config.next).toHaveBeenCalled() }) - - // it("calculates the correct file size from a file upload call and adds it to quota", async () => { - // config.setUrl("/upload") - // config.setProd(true) - // config.setFiles([ - // { - // size: 100 - // }, - // { - // size: 10000 - // }, - // ]) - // await config.executeMiddleware() - - // expect(usageQuota.update).toHaveBeenCalledWith("storage", 10100) - // expect(config.next).toHaveBeenCalled() - // }) }) \ No newline at end of file diff --git a/packages/server/src/migrations/functions/tests/appUrls.spec.js b/packages/server/src/migrations/functions/tests/appUrls.spec.js index 476cdeb07e..fe5b9aeeae 100644 --- a/packages/server/src/migrations/functions/tests/appUrls.spec.js +++ b/packages/server/src/migrations/functions/tests/appUrls.spec.js @@ -14,7 +14,7 @@ describe("run", () => { it("runs successfully", async () => { const app = await config.createApp("testApp") - const metadata = doWithDB(app.appId, async db => { + const metadata = await doWithDB(app.appId, async db => { const metadataDoc = await db.get(DocumentTypes.APP_METADATA) delete metadataDoc.url await db.put(metadataDoc) diff --git a/packages/server/src/migrations/functions/tests/userEmailViewCasing.spec.js b/packages/server/src/migrations/functions/tests/userEmailViewCasing.spec.js index c0d7823cbf..9db59caa4a 100644 --- a/packages/server/src/migrations/functions/tests/userEmailViewCasing.spec.js +++ b/packages/server/src/migrations/functions/tests/userEmailViewCasing.spec.js @@ -1,5 +1,6 @@ const TestConfig = require("../../../tests/utilities/TestConfiguration") -const { getGlobalDB } = require("@budibase/backend-core/tenancy") +const { TENANT_ID } = require("../../../tests/utilities/structures") +const { getGlobalDB, doInTenant } = require("@budibase/backend-core/tenancy") // mock email view creation const coreDb = require("@budibase/backend-core/db") @@ -9,17 +10,19 @@ coreDb.createUserEmailView = createUserEmailView const migration = require("../userEmailViewCasing") describe("run", () => { - let config = new TestConfig(false) - const globalDb = getGlobalDB() + doInTenant(TENANT_ID, () => { + let config = new TestConfig(false) + const globalDb = getGlobalDB() - beforeEach(async () => { - await config.init() - }) + beforeEach(async () => { + await config.init() + }) - afterAll(config.end) + afterAll(config.end) - it("runs successfully", async () => { - await migration.run(globalDb) - expect(createUserEmailView).toHaveBeenCalledTimes(1) + it("runs successfully", async () => { + await migration.run(globalDb) + expect(createUserEmailView).toHaveBeenCalledTimes(1) + }) }) }) diff --git a/packages/server/src/migrations/functions/usageQuotas/tests/syncApps.spec.js b/packages/server/src/migrations/functions/usageQuotas/tests/syncApps.spec.js index 7c74cbfe9a..d12bd8a3ad 100644 --- a/packages/server/src/migrations/functions/usageQuotas/tests/syncApps.spec.js +++ b/packages/server/src/migrations/functions/usageQuotas/tests/syncApps.spec.js @@ -1,37 +1,41 @@ -const { getGlobalDB } = require("@budibase/backend-core/tenancy") +const { doInTenant, getGlobalDB } = require("@budibase/backend-core/tenancy") const TestConfig = require("../../../../tests/utilities/TestConfiguration") +const { TENANT_ID } = require("../../../../tests/utilities/structures") const { getUsageQuotaDoc, update, Properties } = require("../../../../utilities/usageQuota") const syncApps = require("../syncApps") const env = require("../../../../environment") describe("syncApps", () => { + let config = new TestConfig(false) - beforeEach(async () => { - await config.init() - env._set("USE_QUOTAS", 1) - }) + beforeEach(async () => { + await config.init() + env._set("USE_QUOTAS", 1) + }) - afterAll(config.end) + afterAll(config.end) - it("runs successfully", async () => { - // create the usage quota doc and mock usages - const db = getGlobalDB() - await getUsageQuotaDoc(db) - await update(Properties.APPS, 3) + it("runs successfully", async () => { + await doInTenant(TENANT_ID, async () => { + const db = getGlobalDB() + // create the usage quota doc and mock usages + await getUsageQuotaDoc(db) + await update(Properties.APPS, 3) - let usageDoc = await getUsageQuotaDoc(db) - expect(usageDoc.usageQuota.apps).toEqual(3) + let usageDoc = await getUsageQuotaDoc(db) + expect(usageDoc.usageQuota.apps).toEqual(3) - // create an extra app to test the migration - await config.createApp("quota-test") + // create an extra app to test the migration + await config.createApp("quota-test") - // migrate - await syncApps.run() + // migrate + await syncApps.run() - // assert the migration worked - usageDoc = await getUsageQuotaDoc(db) - expect(usageDoc.usageQuota.apps).toEqual(2) - }) + // assert the migration worked + usageDoc = await getUsageQuotaDoc(db) + expect(usageDoc.usageQuota.apps).toEqual(2) + }) + }) }) diff --git a/packages/server/src/migrations/functions/usageQuotas/tests/syncRows.spec.js b/packages/server/src/migrations/functions/usageQuotas/tests/syncRows.spec.js index 034d0eb067..aa5caa588f 100644 --- a/packages/server/src/migrations/functions/usageQuotas/tests/syncRows.spec.js +++ b/packages/server/src/migrations/functions/usageQuotas/tests/syncRows.spec.js @@ -1,5 +1,6 @@ -const { getGlobalDB } = require("@budibase/backend-core/tenancy") +const { doInTenant, getGlobalDB } = require("@budibase/backend-core/tenancy") const TestConfig = require("../../../../tests/utilities/TestConfiguration") +const { TENANT_ID } = require("../../../../tests/utilities/structures") const { getUsageQuotaDoc, update, Properties } = require("../../../../utilities/usageQuota") const syncRows = require("../syncRows") const env = require("../../../../environment") @@ -15,29 +16,30 @@ describe("syncRows", () => { afterAll(config.end) it("runs successfully", async () => { - // create the usage quota doc and mock usages - const db = getGlobalDB() - await getUsageQuotaDoc(db) - await update(Properties.ROW, 300) + await doInTenant(TENANT_ID, async () => { + const db = getGlobalDB() + await getUsageQuotaDoc(db) + await update(Properties.ROW, 300) - let usageDoc = await getUsageQuotaDoc(db) - expect(usageDoc.usageQuota.rows).toEqual(300) + let usageDoc = await getUsageQuotaDoc(db) + expect(usageDoc.usageQuota.rows).toEqual(300) - // app 1 - await config.createTable() - await config.createRow() - // app 2 - await config.createApp("second-app") - await config.createTable() - await config.createRow() - await config.createRow() - - // migrate - await syncRows.run() + // app 1 + await config.createTable() + await config.createRow() + // app 2 + await config.createApp("second-app") + await config.createTable() + await config.createRow() + await config.createRow() - // assert the migration worked - usageDoc = await getUsageQuotaDoc(db) - expect(usageDoc.usageQuota.rows).toEqual(3) + // migrate + await syncRows.run() + + // assert the migration worked + usageDoc = await getUsageQuotaDoc(db) + expect(usageDoc.usageQuota.rows).toEqual(3) + }) }) }) diff --git a/packages/server/src/tests/utilities/TestConfiguration.js b/packages/server/src/tests/utilities/TestConfiguration.js index ca8c7ec06b..fbf0f17abd 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.js +++ b/packages/server/src/tests/utilities/TestConfiguration.js @@ -18,7 +18,7 @@ const supertest = require("supertest") const { cleanup } = require("../../utilities/fileSystem") const { Cookies, Headers } = require("@budibase/backend-core/constants") const { jwt } = require("@budibase/backend-core/auth") -const { doWithGlobalDB } = require("@budibase/backend-core/tenancy") +const { doInTenant, doWithGlobalDB } = require("@budibase/backend-core/tenancy") const { createASession } = require("@budibase/backend-core/sessions") const { user: userCache } = require("@budibase/backend-core/cache") const newid = require("../../db/newid") @@ -55,6 +55,22 @@ class TestConfiguration { return this.appId } + async doInContext(appId, task) { + if (!appId) { + appId = this.appId + } + return doInTenant(TENANT_ID, () => { + // check if already in a context + if (context.getAppId() == null && appId !== null) { + return context.doInAppContext(appId, async () => { + return task() + }) + } else { + return task() + } + }) + } + async _req(config, params, controlFunc) { const request = {} // fake cookies, we don't need them @@ -66,21 +82,13 @@ class TestConfiguration { request.request = { body: config, } - async function run() { + return this.doInContext(this.appId, async () => { if (params) { request.params = params } await controlFunc(request) return request.body - } - // check if already in a context - if (context.getAppId() == null && this.appId !== null) { - return context.doInAppContext(this.appId, async () => { - return run() - }) - } else { - return run() - } + }) } async generateApiKey(userId = GLOBAL_USER_ID) { @@ -201,6 +209,9 @@ class TestConfiguration { async createApp(appName) { // create dev app + // clear any old app + this.appId = null + await context.updateAppId(null) this.app = await this._req({ name: appName }, null, controllers.app.create) this.appId = this.app.appId await context.updateAppId(this.appId) diff --git a/packages/server/src/utilities/usageQuota/rows.js b/packages/server/src/utilities/usageQuota/rows.js index 7261ee2302..78879cc166 100644 --- a/packages/server/src/utilities/usageQuota/rows.js +++ b/packages/server/src/utilities/usageQuota/rows.js @@ -62,7 +62,7 @@ exports.getUniqueRows = async appIds => { continue } try { - appRows.push(await getAppRows(appId)) + appRows = appRows.concat(await getAppRows(appId)) } catch (e) { console.error(e) // don't error out if we can't count the app rows, just continue