From 960ea66fd7e638165a0bdbfbae8639de75c3ead1 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 16 Nov 2021 14:15:13 +0000 Subject: [PATCH 1/4] Quick update to the app caching to improve performance even further, cache when an app doesn't have metadata/is invalid, meaning we don't need to poll the database everytime to see if the metadata doc exists. --- packages/auth/src/cache/appMetadata.js | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/packages/auth/src/cache/appMetadata.js b/packages/auth/src/cache/appMetadata.js index e3758b349a..598a4b7b74 100644 --- a/packages/auth/src/cache/appMetadata.js +++ b/packages/auth/src/cache/appMetadata.js @@ -2,6 +2,9 @@ const redis = require("../redis/authRedis") const { getCouch } = require("../db") const { DocumentTypes } = require("../db/constants") +const AppState = { + INVALID: "invalid", +} const EXPIRY_SECONDS = 3600 /** @@ -27,8 +30,25 @@ exports.getAppMetadata = async (appId, CouchDB = null) => { // try cache let metadata = await client.get(appId) if (!metadata) { - metadata = await populateFromDB(appId, CouchDB) - client.store(appId, metadata, EXPIRY_SECONDS) + let expiry = EXPIRY_SECONDS + try { + metadata = await populateFromDB(appId, CouchDB) + } catch (err) { + // app DB left around, but no metadata, it is invalid + if (err && err.status === 404) { + metadata = { state: AppState.INVALID } + // don't expire the reference to an invalid app, it'll only be + // updated if a metadata doc actually gets stored (app is remade/reverted) + expiry = undefined + } else { + throw err + } + } + client.store(appId, metadata, expiry) + } + // we've stored in the cache an object to tell us that it is currently invalid + if (!metadata || metadata.state === AppState.INVALID) { + throw { status: 404, message: "No app metadata found" } } return metadata } From a9c6395d72215c5347b6ff7017f6b1f749bed259 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 16 Nov 2021 15:23:02 +0000 Subject: [PATCH 2/4] Fixing issue discovered by cypress test. --- packages/auth/src/cache/appMetadata.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/auth/src/cache/appMetadata.js b/packages/auth/src/cache/appMetadata.js index 598a4b7b74..400714987b 100644 --- a/packages/auth/src/cache/appMetadata.js +++ b/packages/auth/src/cache/appMetadata.js @@ -44,7 +44,7 @@ exports.getAppMetadata = async (appId, CouchDB = null) => { throw err } } - client.store(appId, metadata, expiry) + await client.store(appId, metadata, expiry) } // we've stored in the cache an object to tell us that it is currently invalid if (!metadata || metadata.state === AppState.INVALID) { From 8d56fe1339276cb4be530c7c369a0d0f53d50320 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 16 Nov 2021 17:40:31 +0000 Subject: [PATCH 3/4] Adding a script to be able to debug backend like cypress runs, without needing to build everytime - fixing an issue that appears to only occur occasionally in the cypress environment. --- packages/auth/src/cache/appMetadata.js | 18 ++++++++++++-- packages/builder/cypress/setup.js | 1 + packages/server/scripts/likeCypress.ts | 33 ++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 packages/server/scripts/likeCypress.ts diff --git a/packages/auth/src/cache/appMetadata.js b/packages/auth/src/cache/appMetadata.js index 400714987b..982f95dbe5 100644 --- a/packages/auth/src/cache/appMetadata.js +++ b/packages/auth/src/cache/appMetadata.js @@ -18,11 +18,16 @@ const populateFromDB = async (appId, CouchDB = null) => { return db.get(DocumentTypes.APP_METADATA) } +const isInvalid = metadata => { + return !metadata || metadata.state === AppState.INVALID +} + /** * 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. + * @param {string} appId the id of the app to get metadata from. + * @param {object} CouchDB the database being passed * @returns {object} the app metadata. */ exports.getAppMetadata = async (appId, CouchDB = null) => { @@ -44,10 +49,19 @@ exports.getAppMetadata = async (appId, CouchDB = null) => { throw err } } + // needed for cypress/some scenarios where the caching happens + // so quickly the requests can get slightly out of sync + // might store its invalid just before it stores its valid + if (isInvalid(metadata)) { + const temp = await client.get(appId) + if (temp) { + metadata = temp + } + } await client.store(appId, metadata, expiry) } // we've stored in the cache an object to tell us that it is currently invalid - if (!metadata || metadata.state === AppState.INVALID) { + if (isInvalid(metadata)) { throw { status: 404, message: "No app metadata found" } } return metadata diff --git a/packages/builder/cypress/setup.js b/packages/builder/cypress/setup.js index 1a6f1d5b2b..7657303853 100644 --- a/packages/builder/cypress/setup.js +++ b/packages/builder/cypress/setup.js @@ -17,6 +17,7 @@ process.env.JWT_SECRET = cypressConfig.env.JWT_SECRET process.env.COUCH_URL = `leveldb://${tmpdir}/.data/` process.env.SELF_HOSTED = 1 process.env.WORKER_URL = "http://localhost:10002/" +process.env.APPS_URL = `http://localhost:${MAIN_PORT}/` process.env.MINIO_URL = `http://localhost:${MAIN_PORT}/` process.env.MINIO_ACCESS_KEY = "budibase" process.env.MINIO_SECRET_KEY = "budibase" diff --git a/packages/server/scripts/likeCypress.ts b/packages/server/scripts/likeCypress.ts new file mode 100644 index 0000000000..505dbef523 --- /dev/null +++ b/packages/server/scripts/likeCypress.ts @@ -0,0 +1,33 @@ +/****************************************************** + * This script just makes it easy to re-create * + * a cypress like environment for testing the backend * + ******************************************************/ +const path = require("path") +const tmpdir = path.join(require("os").tmpdir(), ".budibase") + +const MAIN_PORT = "4002" +const WORKER_PORT = "4001" + +// @ts-ignore +process.env.PORT = "4001" +process.env.BUDIBASE_API_KEY = "6BE826CB-6B30-4AEC-8777-2E90464633DE" +process.env.NODE_ENV = "cypress" +process.env.ENABLE_ANALYTICS = "false" +process.env.JWT_SECRET = "budibase" +process.env.COUCH_URL = `leveldb://${tmpdir}/.data/` +process.env.SELF_HOSTED = "1" +process.env.WORKER_URL = "http://localhost:4002/" +process.env.MINIO_URL = `http://localhost:4001/` +process.env.MINIO_ACCESS_KEY = "budibase" +process.env.MINIO_SECRET_KEY = "budibase" +process.env.COUCH_DB_USER = "budibase" +process.env.COUCH_DB_PASSWORD = "budibase" +process.env.INTERNAL_API_KEY = "budibase" +process.env.ALLOW_DEV_AUTOMATIONS = "1" + +// don't make this a variable or top level require +// it will cause environment module to be loaded prematurely +const server = require("../src/app") +process.env.PORT = "4002" +const worker = require("../../worker/src/index") +process.env.PORT = "4001" \ No newline at end of file From 3b9f3d66905c3b9d1b547b0cfd80ed76d35aa09a Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 16 Nov 2021 20:56:24 +0000 Subject: [PATCH 4/4] Fix for cypress test issues, when metadata is updated rapidly it could get into a bad state - this should resolve it. --- packages/auth/src/cache/appMetadata.js | 14 +++++++++++++- packages/auth/src/middleware/authenticated.js | 4 ++++ packages/server/scripts/likeCypress.ts | 14 +++++++------- packages/server/src/api/controllers/application.js | 1 + packages/server/src/api/controllers/dev.js | 3 ++- packages/server/src/middleware/builder.js | 5 +++-- 6 files changed, 30 insertions(+), 11 deletions(-) diff --git a/packages/auth/src/cache/appMetadata.js b/packages/auth/src/cache/appMetadata.js index 982f95dbe5..9e2317ae37 100644 --- a/packages/auth/src/cache/appMetadata.js +++ b/packages/auth/src/cache/appMetadata.js @@ -67,7 +67,19 @@ exports.getAppMetadata = async (appId, CouchDB = null) => { return metadata } -exports.invalidateAppMetadata = async appId => { +/** + * Invalidate/reset the cached metadata when a change occurs in the db. + * @param appId {string} the cache key to bust/update. + * @param newMetadata {object|undefined} optional - can simply provide the new metadata to update with. + * @return {Promise} will respond with success when cache is updated. + */ +exports.invalidateAppMetadata = async (appId, newMetadata = null) => { + if (!appId) { + throw "Cannot invalidate if no app ID provided." + } const client = await redis.getAppClient() await client.delete(appId) + if (newMetadata) { + await client.store(appId, newMetadata, EXPIRY_SECONDS) + } } diff --git a/packages/auth/src/middleware/authenticated.js b/packages/auth/src/middleware/authenticated.js index 4223e7e395..f0fb6e21c5 100644 --- a/packages/auth/src/middleware/authenticated.js +++ b/packages/auth/src/middleware/authenticated.js @@ -92,6 +92,10 @@ module.exports = ( finalise(ctx, { authenticated, user, internal, version, publicEndpoint }) return next() } catch (err) { + // invalid token, clear the cookie + if (err && err.name === "JsonWebTokenError") { + clearCookie(ctx, Cookies.Auth) + } // allow configuring for public access if ((opts && opts.publicAllowed) || publicEndpoint) { finalise(ctx, { authenticated: false, version, publicEndpoint }) diff --git a/packages/server/scripts/likeCypress.ts b/packages/server/scripts/likeCypress.ts index 505dbef523..625b4386e1 100644 --- a/packages/server/scripts/likeCypress.ts +++ b/packages/server/scripts/likeCypress.ts @@ -5,19 +5,19 @@ const path = require("path") const tmpdir = path.join(require("os").tmpdir(), ".budibase") -const MAIN_PORT = "4002" -const WORKER_PORT = "4001" +const MAIN_PORT = "10001" +const WORKER_PORT = "10002" // @ts-ignore -process.env.PORT = "4001" +process.env.PORT = MAIN_PORT process.env.BUDIBASE_API_KEY = "6BE826CB-6B30-4AEC-8777-2E90464633DE" process.env.NODE_ENV = "cypress" process.env.ENABLE_ANALYTICS = "false" process.env.JWT_SECRET = "budibase" process.env.COUCH_URL = `leveldb://${tmpdir}/.data/` process.env.SELF_HOSTED = "1" -process.env.WORKER_URL = "http://localhost:4002/" -process.env.MINIO_URL = `http://localhost:4001/` +process.env.WORKER_URL = `http://localhost:${WORKER_PORT}/` +process.env.MINIO_URL = `http://localhost:${MAIN_PORT}/` process.env.MINIO_ACCESS_KEY = "budibase" process.env.MINIO_SECRET_KEY = "budibase" process.env.COUCH_DB_USER = "budibase" @@ -28,6 +28,6 @@ process.env.ALLOW_DEV_AUTOMATIONS = "1" // don't make this a variable or top level require // it will cause environment module to be loaded prematurely const server = require("../src/app") -process.env.PORT = "4002" +process.env.PORT = WORKER_PORT const worker = require("../../worker/src/index") -process.env.PORT = "4001" \ No newline at end of file +process.env.PORT = MAIN_PORT \ No newline at end of file diff --git a/packages/server/src/api/controllers/application.js b/packages/server/src/api/controllers/application.js index d38312af2f..cd14313b47 100644 --- a/packages/server/src/api/controllers/application.js +++ b/packages/server/src/api/controllers/application.js @@ -255,6 +255,7 @@ exports.create = async ctx => { await createApp(appId) } + await appCache.invalidateAppMetadata(appId, newApplication) ctx.status = 200 ctx.body = newApplication } diff --git a/packages/server/src/api/controllers/dev.js b/packages/server/src/api/controllers/dev.js index dbea82b06b..559ffcbcfd 100644 --- a/packages/server/src/api/controllers/dev.js +++ b/packages/server/src/api/controllers/dev.js @@ -25,7 +25,8 @@ async function redirect(ctx, method, path = "global") { ) ) if (response.status !== 200) { - ctx.throw(response.status, response.statusText) + const err = await response.text() + ctx.throw(400, err) } const cookie = response.headers.get("set-cookie") if (cookie) { diff --git a/packages/server/src/middleware/builder.js b/packages/server/src/middleware/builder.js index 427e54287a..f8a84c4c25 100644 --- a/packages/server/src/middleware/builder.js +++ b/packages/server/src/middleware/builder.js @@ -51,8 +51,9 @@ async function updateAppUpdatedAt(ctx) { const db = new CouchDB(appId) const metadata = await db.get(DocumentTypes.APP_METADATA) metadata.updatedAt = new Date().toISOString() - await db.put(metadata) - await appCache.invalidateAppMetadata(appId) + const response = await db.put(metadata) + metadata._rev = response.rev + await appCache.invalidateAppMetadata(appId, metadata) // set a new debounce record with a short TTL await setDebounce(appId, DEBOUNCE_TIME_SEC) }