From a1227c58190f7e227290f966450c6bd155cd51b0 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 13 Apr 2021 17:11:55 +0100 Subject: [PATCH] Removing the lookup of _id in usage quota when in dev/self host for performance reasons as part of usage quota, re-writing some bits of fetch self for cleaner implementation, fixing some issues with updating/saving users from within app. --- packages/server/src/api/controllers/auth.js | 32 +++++++++++-------- packages/server/src/api/controllers/row.js | 2 +- packages/server/src/api/controllers/user.js | 3 ++ .../src/middleware/tests/usageQuota.spec.js | 9 ++++-- packages/server/src/middleware/usageQuota.js | 14 ++++---- .../server/src/utilities/workerRequests.js | 4 ++- packages/worker/src/api/routes/admin/index.js | 2 +- 7 files changed, 39 insertions(+), 27 deletions(-) diff --git a/packages/server/src/api/controllers/auth.js b/packages/server/src/api/controllers/auth.js index 80005813ea..a500598305 100644 --- a/packages/server/src/api/controllers/auth.js +++ b/packages/server/src/api/controllers/auth.js @@ -79,20 +79,24 @@ exports.fetchSelf = async ctx => { return } - if (!appId) { - const db = new CouchDB(StaticDatabases.USER.name) - const user = await db.get(userId) - delete user.password - ctx.body = { user } - return - } - - const db = new CouchDB(appId) const user = await getFullUser({ ctx, userId: userId }) - const userTable = await db.get(InternalTables.USER_METADATA) - if (user) { - delete user.password + + if (appId) { + const db = new CouchDB(appId) + // remove the full roles structure + delete user.roles + try { + const userTable = await db.get(InternalTables.USER_METADATA) + const metadata = await db.get(userId) + // specifically needs to make sure is enriched + ctx.body = await outputProcessing(appId, userTable, { + ...user, + ...metadata, + }) + } catch (err) { + ctx.body = user + } + } else { + ctx.body = user } - // specifically needs to make sure is enriched - ctx.body = await outputProcessing(appId, userTable, user) } diff --git a/packages/server/src/api/controllers/row.js b/packages/server/src/api/controllers/row.js index 9f59eb46f3..6ce6aa3e28 100644 --- a/packages/server/src/api/controllers/row.js +++ b/packages/server/src/api/controllers/row.js @@ -130,7 +130,7 @@ exports.save = async function(ctx) { } // if the row obj had an _id then it will have been retrieved - const existingRow = ctx.preExisting + const existingRow = await db.get(inputs._id) if (existingRow) { ctx.params.rowId = inputs._id await exports.patch(ctx) diff --git a/packages/server/src/api/controllers/user.js b/packages/server/src/api/controllers/user.js index 2e2fa2c6f0..f800a54ace 100644 --- a/packages/server/src/api/controllers/user.js +++ b/packages/server/src/api/controllers/user.js @@ -26,6 +26,9 @@ exports.fetchMetadata = async function(ctx) { const users = [] for (let user of global) { const info = metadata.find(meta => meta._id.includes(user.email)) + // remove these props, not for the correct DB + delete user._id + delete user._rev users.push({ ...user, ...info, diff --git a/packages/server/src/middleware/tests/usageQuota.spec.js b/packages/server/src/middleware/tests/usageQuota.spec.js index 646f492329..01407a670b 100644 --- a/packages/server/src/middleware/tests/usageQuota.spec.js +++ b/packages/server/src/middleware/tests/usageQuota.spec.js @@ -76,8 +76,10 @@ describe("usageQuota middleware", () => { }) it("passes through to next middleware if document already exists", async () => { + config.setProd(true) config.setBody({ - _id: "test" + _id: "test", + _rev: "test", }) CouchDB.mockImplementationOnce(() => ({ @@ -87,13 +89,14 @@ describe("usageQuota middleware", () => { await config.executeMiddleware() expect(config.next).toHaveBeenCalled() - expect(config.ctx.preExisting).toBe(true) }) it("throws if request has _id, but the document no longer exists", async () => { config.setBody({ - _id: "123" + _id: "123", + _rev: "test", }) + config.setProd(true) CouchDB.mockImplementationOnce(() => ({ get: async () => { diff --git a/packages/server/src/middleware/usageQuota.js b/packages/server/src/middleware/usageQuota.js index a37f089a65..ac8336e769 100644 --- a/packages/server/src/middleware/usageQuota.js +++ b/packages/server/src/middleware/usageQuota.js @@ -27,6 +27,11 @@ function getProperty(url) { } module.exports = async (ctx, next) => { + // if in development or a self hosted cloud usage quotas should not be executed + if (env.isDev() || env.SELF_HOSTED) { + return next() + } + const db = new CouchDB(ctx.appId) let usage = METHOD_MAP[ctx.req.method] const property = getProperty(ctx.req.url) @@ -34,20 +39,15 @@ module.exports = async (ctx, next) => { return next() } // post request could be a save of a pre-existing entry - if (ctx.request.body && ctx.request.body._id) { + if (ctx.request.body && ctx.request.body._id && ctx.request.body._rev) { try { - ctx.preExisting = await db.get(ctx.request.body._id) + await db.get(ctx.request.body._id) return next() } catch (err) { ctx.throw(404, `${ctx.request.body._id} does not exist`) - return } } - // if in development or a self hosted cloud usage quotas should not be executed - if (env.isDev() || env.SELF_HOSTED) { - return next() - } // update usage for uploads to be the total size if (property === usageQuota.Properties.UPLOAD) { const files = diff --git a/packages/server/src/utilities/workerRequests.js b/packages/server/src/utilities/workerRequests.js index 65d013c935..b04b3698f4 100644 --- a/packages/server/src/utilities/workerRequests.js +++ b/packages/server/src/utilities/workerRequests.js @@ -96,7 +96,7 @@ exports.saveGlobalUser = async (ctx, appId, email, body) => { body: { ...globalUser, email, - password: body.password, + password: body.password || undefined, status: body.status, roles, }, @@ -114,5 +114,7 @@ exports.saveGlobalUser = async (ctx, appId, email, body) => { delete body.password delete body.roleId delete body.status + delete body.roles + delete body.builder return body } diff --git a/packages/worker/src/api/routes/admin/index.js b/packages/worker/src/api/routes/admin/index.js index de19a277ef..88a1809fc5 100644 --- a/packages/worker/src/api/routes/admin/index.js +++ b/packages/worker/src/api/routes/admin/index.js @@ -12,7 +12,7 @@ function buildUserSaveValidation() { _id: Joi.string(), _rev: Joi.string(), email: Joi.string(), - password: Joi.string(), + password: Joi.string().allow(null, ""), // maps appId -> roleId for the user roles: Joi.object() .pattern(/.*/, Joi.string())