From f13257bebe5fbfc699e535f300a468d26b46366e Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 15 Nov 2021 17:40:45 +0000 Subject: [PATCH 1/3] Updating the getAllApps function to use a cached version of the app metadata, rather than retrieving it individually everytime. Also invalidating the results everytime they are updated (at least in the important locations). --- packages/auth/cache.js | 1 + packages/auth/src/cache/appMetadata.js | 35 +++++++++++++++++++ packages/auth/src/db/index.js | 4 +-- packages/auth/src/db/utils.js | 3 +- packages/auth/src/redis/authRedis.js | 10 +++++- packages/auth/src/redis/utils.js | 1 + .../server/src/api/controllers/application.js | 7 +++- .../src/api/controllers/deploy/index.js | 2 ++ packages/server/src/api/controllers/dev.js | 2 ++ packages/server/src/middleware/builder.js | 2 ++ packages/server/src/utilities/index.js | 1 + 11 files changed, 63 insertions(+), 5 deletions(-) create mode 100644 packages/auth/src/cache/appMetadata.js diff --git a/packages/auth/cache.js b/packages/auth/cache.js index 48563a16f3..02344586a9 100644 --- a/packages/auth/cache.js +++ b/packages/auth/cache.js @@ -1,3 +1,4 @@ module.exports = { user: require("./src/cache/user"), + app: require("./src/cache/appMetadata"), } diff --git a/packages/auth/src/cache/appMetadata.js b/packages/auth/src/cache/appMetadata.js new file mode 100644 index 0000000000..e30a58770f --- /dev/null +++ b/packages/auth/src/cache/appMetadata.js @@ -0,0 +1,35 @@ +const redis = require("../redis/authRedis") +const { getDB } = require("../db") +const { DocumentTypes } = require("../db/constants") + +const EXPIRY_SECONDS = 3600 + +/** + * The default populate app metadata function + */ +const populateFromDB = async appId => { + return getDB(appId, { skip_setup: true }).get(DocumentTypes.APP_METADATA) +} + +/** + * Get the requested app metadata by id. + * Use redis cache to first read the app metadata. + * If not present fallback to loading the app metadata directly and re-caching. + * @param {*} appId the id of the app to get metadata from. + * @returns {object} the app metadata. + */ +exports.getAppMetadata = async appId => { + const client = await redis.getAppClient() + // try cache + let metadata = await client.get(appId) + if (!metadata) { + metadata = await populateFromDB(appId) + client.store(appId, metadata, EXPIRY_SECONDS) + } + return metadata +} + +exports.invalidateAppMetadata = async appId => { + const client = await redis.getAppClient() + await client.delete(appId) +} diff --git a/packages/auth/src/db/index.js b/packages/auth/src/db/index.js index 163364dbf3..94f5513f19 100644 --- a/packages/auth/src/db/index.js +++ b/packages/auth/src/db/index.js @@ -4,8 +4,8 @@ module.exports.setDB = pouch => { Pouch = pouch } -module.exports.getDB = dbName => { - return new Pouch(dbName) +module.exports.getDB = (dbName, opts = {}) => { + return new Pouch(dbName, opts) } module.exports.getCouch = () => { diff --git a/packages/auth/src/db/utils.js b/packages/auth/src/db/utils.js index b956089660..f8f36b17b3 100644 --- a/packages/auth/src/db/utils.js +++ b/packages/auth/src/db/utils.js @@ -6,6 +6,7 @@ const { StaticDatabases, SEPARATOR, DocumentTypes } = require("./constants") const { getTenantId, getTenantIDFromAppID } = require("../tenancy") const fetch = require("node-fetch") const { getCouch } = require("./index") +const { getAppMetadata } = require("../cache/appMetadata") const UNICODE_MAX = "\ufff0" @@ -234,7 +235,7 @@ exports.getAllApps = async (CouchDB, { dev, all, idsOnly } = {}) => { } const appPromises = appDbNames.map(db => // skip setup otherwise databases could be re-created - new CouchDB(db, { skip_setup: true }).get(DocumentTypes.APP_METADATA) + getAppMetadata(db) ) if (appPromises.length === 0) { return [] diff --git a/packages/auth/src/redis/authRedis.js b/packages/auth/src/redis/authRedis.js index decce6763b..ca5c9bae37 100644 --- a/packages/auth/src/redis/authRedis.js +++ b/packages/auth/src/redis/authRedis.js @@ -1,16 +1,18 @@ const Client = require("./index") const utils = require("./utils") -let userClient, sessionClient +let userClient, sessionClient, appClient async function init() { userClient = await new Client(utils.Databases.USER_CACHE).init() sessionClient = await new Client(utils.Databases.SESSIONS).init() + appClient = await new Client(utils.Databases.APP_METADATA).init() } process.on("exit", async () => { if (userClient) await userClient.finish() if (sessionClient) await sessionClient.finish() + if (appClient) await appClient.finish() }) module.exports = { @@ -26,4 +28,10 @@ module.exports = { } return sessionClient }, + getAppClient: async () => { + if (!appClient) { + await init() + } + return appClient + }, } diff --git a/packages/auth/src/redis/utils.js b/packages/auth/src/redis/utils.js index 6befecd9ba..466b117e96 100644 --- a/packages/auth/src/redis/utils.js +++ b/packages/auth/src/redis/utils.js @@ -15,6 +15,7 @@ exports.Databases = { SESSIONS: "session", USER_CACHE: "users", FLAGS: "flags", + APP_METADATA: "appMetadata", } exports.SEPARATOR = SEPARATOR diff --git a/packages/server/src/api/controllers/application.js b/packages/server/src/api/controllers/application.js index 99c16a975c..d38312af2f 100644 --- a/packages/server/src/api/controllers/application.js +++ b/packages/server/src/api/controllers/application.js @@ -45,6 +45,7 @@ const { } = require("../../utilities/fileSystem/clientLibrary") const { getTenantId, isMultiTenant } = require("@budibase/auth/tenancy") const { syncGlobalUsers } = require("./user") +const { app: appCache } = require("@budibase/auth/cache") const URL_REGEX_SLASH = /\/|\\/g @@ -319,6 +320,7 @@ exports.delete = async ctx => { } // make sure the app/role doesn't stick around after the app has been deleted await removeAppFromUserRoles(ctx, ctx.params.appId) + await appCache.invalidateAppMetadata(ctx.params.appId) ctx.status = 200 ctx.body = result @@ -387,7 +389,10 @@ const updateAppPackage = async (ctx, appPackage, appId) => { // Redis, shouldn't ever store it delete newAppPackage.lockedBy - return await db.put(newAppPackage) + const response = await db.put(newAppPackage) + // remove any cached metadata, so that it will be updated + await appCache.invalidateAppMetadata(appId) + return response } const createEmptyAppPackage = async (ctx, app) => { diff --git a/packages/server/src/api/controllers/deploy/index.js b/packages/server/src/api/controllers/deploy/index.js index 13002476fc..c1138f4b03 100644 --- a/packages/server/src/api/controllers/deploy/index.js +++ b/packages/server/src/api/controllers/deploy/index.js @@ -6,6 +6,7 @@ const { disableAllCrons, enableCronTrigger, } = require("../../../automations/utils") +const { app: appCache } = require("@budibase/auth/cache") // the max time we can wait for an invalidation to complete before considering it failed const MAX_PENDING_TIME_MS = 30 * 60000 @@ -103,6 +104,7 @@ async function deployApp(deployment) { appDoc.appId = productionAppId appDoc.instance._id = productionAppId await db.put(appDoc) + await appCache.invalidateAppMetadata(productionAppId) console.log("New app doc written successfully.") await initDeployedApp(productionAppId) console.log("Deployed app initialised, setting deployment to successful") diff --git a/packages/server/src/api/controllers/dev.js b/packages/server/src/api/controllers/dev.js index ed58b8048b..dbea82b06b 100644 --- a/packages/server/src/api/controllers/dev.js +++ b/packages/server/src/api/controllers/dev.js @@ -6,6 +6,7 @@ const { request } = require("../../utilities/workerRequests") const { clearLock } = require("../../utilities/redis") const { Replication } = require("@budibase/auth").db const { DocumentTypes } = require("../../db/utils") +const { app: appCache } = require("@budibase/auth/cache") async function redirect(ctx, method, path = "global") { const { devPath } = ctx.params @@ -106,6 +107,7 @@ exports.revert = async ctx => { appDoc.appId = appId appDoc.instance._id = appId await db.put(appDoc) + await appCache.invalidateAppMetadata(appId) ctx.body = { message: "Reverted changes successfully.", } diff --git a/packages/server/src/middleware/builder.js b/packages/server/src/middleware/builder.js index 8ea49a3b48..427e54287a 100644 --- a/packages/server/src/middleware/builder.js +++ b/packages/server/src/middleware/builder.js @@ -8,6 +8,7 @@ const { const CouchDB = require("../db") const { DocumentTypes } = require("../db/utils") const { PermissionTypes } = require("@budibase/auth/permissions") +const { app: appCache } = require("@budibase/auth/cache") const DEBOUNCE_TIME_SEC = 30 @@ -51,6 +52,7 @@ async function updateAppUpdatedAt(ctx) { const metadata = await db.get(DocumentTypes.APP_METADATA) metadata.updatedAt = new Date().toISOString() await db.put(metadata) + await appCache.invalidateAppMetadata(appId) // set a new debounce record with a short TTL await setDebounce(appId, DEBOUNCE_TIME_SEC) } diff --git a/packages/server/src/utilities/index.js b/packages/server/src/utilities/index.js index c2ebbcd9f1..eacf9708e2 100644 --- a/packages/server/src/utilities/index.js +++ b/packages/server/src/utilities/index.js @@ -48,6 +48,7 @@ exports.objectStoreUrl = () => { * via a specific endpoint (under /api/assets/client). * @param {string} appId In production we need the appId to look up the correct bucket, as the * version of the client lib may differ between apps. + * @param {string} version The version to retrieve. * @return {string} The URL to be inserted into appPackage response or server rendered * app index file. */ From 5470b77fb3ca42af756ae84e12c48bb27158829c Mon Sep 17 00:00:00 2001 From: Michael Drury Date: Mon, 15 Nov 2021 19:34:08 +0000 Subject: [PATCH 2/3] Fixing issue presented by test, passing Couch instance around for when it is being used in memory. --- packages/auth/src/cache/appMetadata.js | 14 +++++++++----- packages/auth/src/db/index.js | 4 ++-- packages/auth/src/db/utils.js | 9 ++++++--- .../worker/src/api/controllers/global/users.js | 6 +----- 4 files changed, 18 insertions(+), 15 deletions(-) diff --git a/packages/auth/src/cache/appMetadata.js b/packages/auth/src/cache/appMetadata.js index e30a58770f..e3758b349a 100644 --- a/packages/auth/src/cache/appMetadata.js +++ b/packages/auth/src/cache/appMetadata.js @@ -1,5 +1,5 @@ const redis = require("../redis/authRedis") -const { getDB } = require("../db") +const { getCouch } = require("../db") const { DocumentTypes } = require("../db/constants") const EXPIRY_SECONDS = 3600 @@ -7,8 +7,12 @@ const EXPIRY_SECONDS = 3600 /** * The default populate app metadata function */ -const populateFromDB = async appId => { - return getDB(appId, { skip_setup: true }).get(DocumentTypes.APP_METADATA) +const populateFromDB = async (appId, CouchDB = null) => { + if (!CouchDB) { + CouchDB = getCouch() + } + const db = new CouchDB(appId, { skip_setup: true }) + return db.get(DocumentTypes.APP_METADATA) } /** @@ -18,12 +22,12 @@ const populateFromDB = async appId => { * @param {*} appId the id of the app to get metadata from. * @returns {object} the app metadata. */ -exports.getAppMetadata = async appId => { +exports.getAppMetadata = async (appId, CouchDB = null) => { const client = await redis.getAppClient() // try cache let metadata = await client.get(appId) if (!metadata) { - metadata = await populateFromDB(appId) + metadata = await populateFromDB(appId, CouchDB) client.store(appId, metadata, EXPIRY_SECONDS) } return metadata diff --git a/packages/auth/src/db/index.js b/packages/auth/src/db/index.js index 94f5513f19..163364dbf3 100644 --- a/packages/auth/src/db/index.js +++ b/packages/auth/src/db/index.js @@ -4,8 +4,8 @@ module.exports.setDB = pouch => { Pouch = pouch } -module.exports.getDB = (dbName, opts = {}) => { - return new Pouch(dbName, opts) +module.exports.getDB = dbName => { + return new Pouch(dbName) } module.exports.getCouch = () => { diff --git a/packages/auth/src/db/utils.js b/packages/auth/src/db/utils.js index f8f36b17b3..11ce51fae9 100644 --- a/packages/auth/src/db/utils.js +++ b/packages/auth/src/db/utils.js @@ -54,6 +54,9 @@ exports.isProdAppID = appId => { } function isDevApp(app) { + if (!app) { + return false + } return exports.isDevAppID(app.appId) } @@ -233,16 +236,16 @@ exports.getAllApps = async (CouchDB, { dev, all, idsOnly } = {}) => { if (idsOnly) { return appDbNames } - const appPromises = appDbNames.map(db => + const appPromises = appDbNames.map(app => // skip setup otherwise databases could be re-created - getAppMetadata(db) + getAppMetadata(app, CouchDB) ) if (appPromises.length === 0) { return [] } else { const response = await Promise.allSettled(appPromises) const apps = response - .filter(result => result.status === "fulfilled") + .filter(result => result.status === "fulfilled" && result.value != null) .map(({ value }) => value) if (!all) { return apps.filter(app => { diff --git a/packages/worker/src/api/controllers/global/users.js b/packages/worker/src/api/controllers/global/users.js index 42166faad7..87194a7ceb 100644 --- a/packages/worker/src/api/controllers/global/users.js +++ b/packages/worker/src/api/controllers/global/users.js @@ -43,11 +43,7 @@ exports.save = async ctx => { } const parseBooleanParam = param => { - if (param && param === "false") { - return false - } else { - return true - } + return !(param && param === "false") } exports.adminUser = async ctx => { From 289c1325f87d10564d662c0d181d045637337606 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 16 Nov 2021 10:30:37 +0000 Subject: [PATCH 3/3] Adding specific error cases to all app ID checking functions - three cases, is dev/prod, isn't and no app/ID provided. --- packages/auth/src/db/utils.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/auth/src/db/utils.js b/packages/auth/src/db/utils.js index 11ce51fae9..14e80df735 100644 --- a/packages/auth/src/db/utils.js +++ b/packages/auth/src/db/utils.js @@ -8,6 +8,8 @@ const fetch = require("node-fetch") const { getCouch } = require("./index") const { getAppMetadata } = require("../cache/appMetadata") +const NO_APP_ERROR = "No app provided" + const UNICODE_MAX = "\ufff0" exports.ViewNames = { @@ -46,16 +48,22 @@ function getDocParams(docType, docId = null, otherProps = {}) { } exports.isDevAppID = appId => { + if (!appId) { + throw NO_APP_ERROR + } return appId.startsWith(exports.APP_DEV_PREFIX) } exports.isProdAppID = appId => { + if (!appId) { + throw NO_APP_ERROR + } return appId.startsWith(exports.APP_PREFIX) && !exports.isDevAppID(appId) } function isDevApp(app) { if (!app) { - return false + throw NO_APP_ERROR } return exports.isDevAppID(app.appId) }