From 6d6b3dd9711df5bb15e769c116f665397e88e7f3 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 18 Sep 2023 13:10:27 +0200 Subject: [PATCH 01/10] Refactor --- .../tests/core/utilities/structures/shared.ts | 19 -------------- .../tests/core/utilities/structures/sso.ts | 7 +++-- .../tests/core/utilities/structures/users.ts | 26 +++++++++++++++++-- 3 files changed, 27 insertions(+), 25 deletions(-) delete mode 100644 packages/backend-core/tests/core/utilities/structures/shared.ts diff --git a/packages/backend-core/tests/core/utilities/structures/shared.ts b/packages/backend-core/tests/core/utilities/structures/shared.ts deleted file mode 100644 index de0e19486c..0000000000 --- a/packages/backend-core/tests/core/utilities/structures/shared.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { User } from "@budibase/types" -import { generator } from "./generator" -import { uuid } from "./common" - -export const newEmail = () => { - return `${uuid()}@test.com` -} - -export const user = (userProps?: any): User => { - return { - email: newEmail(), - password: "test", - roles: { app_test: "admin" }, - firstName: generator.first(), - lastName: generator.last(), - pictureUrl: "http://test.com", - ...userProps, - } -} diff --git a/packages/backend-core/tests/core/utilities/structures/sso.ts b/packages/backend-core/tests/core/utilities/structures/sso.ts index 4d13635f09..2e3af712a9 100644 --- a/packages/backend-core/tests/core/utilities/structures/sso.ts +++ b/packages/backend-core/tests/core/utilities/structures/sso.ts @@ -13,8 +13,7 @@ import { } from "@budibase/types" import { generator } from "./generator" import { email, uuid } from "./common" -import * as shared from "./shared" -import { user } from "./shared" +import * as users from "./users" import sample from "lodash/sample" export function OAuth(): OAuth2 { @@ -26,7 +25,7 @@ export function OAuth(): OAuth2 { export function authDetails(userDoc?: User): SSOAuthDetails { if (!userDoc) { - userDoc = user() + userDoc = users.user() } const userId = userDoc._id || uuid() @@ -52,7 +51,7 @@ export function providerType(): SSOProviderType { export function ssoProfile(user?: User): SSOProfile { if (!user) { - user = shared.user() + user = users.user() } return { id: user._id!, diff --git a/packages/backend-core/tests/core/utilities/structures/users.ts b/packages/backend-core/tests/core/utilities/structures/users.ts index 0a4f2e8b54..420a9fde0e 100644 --- a/packages/backend-core/tests/core/utilities/structures/users.ts +++ b/packages/backend-core/tests/core/utilities/structures/users.ts @@ -4,11 +4,33 @@ import { BuilderUser, SSOAuthDetails, SSOUser, + User, } from "@budibase/types" -import { user } from "./shared" import { authDetails } from "./sso" +import { uuid } from "./common" +import { generator } from "./generator" +import { tenant } from "." +import { generateGlobalUserID } from "../../../../src/docIds" -export { user, newEmail } from "./shared" +export const newEmail = () => { + return `${uuid()}@test.com` +} + +export const user = (userProps?: Partial>): User => { + const userId = userProps?._id || generateGlobalUserID() + return { + _id: userId, + userId, + email: newEmail(), + password: "test", + roles: { app_test: "admin" }, + firstName: generator.first(), + lastName: generator.last(), + pictureUrl: "http://test.com", + tenantId: tenant.id(), + ...userProps, + } +} export const adminUser = (userProps?: any): AdminUser => { return { From 3336433de82b93e36064376815acb0fc97561291 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 18 Sep 2023 13:39:05 +0200 Subject: [PATCH 02/10] User cache, get in bulk --- packages/backend-core/src/cache/user.ts | 46 +++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/packages/backend-core/src/cache/user.ts b/packages/backend-core/src/cache/user.ts index e2af78adfd..f05cdd42ac 100644 --- a/packages/backend-core/src/cache/user.ts +++ b/packages/backend-core/src/cache/user.ts @@ -27,6 +27,31 @@ async function populateFromDB(userId: string, tenantId: string) { return user } +async function populateUsersFromDB(userIds: string[], tenantId: string) { + const db = tenancy.getTenantDB(tenantId) + const allDocsResponse = await db.allDocs({ + keys: userIds, + include_docs: true, + limit: userIds.length, + }) + + const users = allDocsResponse.rows.map(r => r.doc) + await Promise.all( + users.map(async user => { + user.budibaseAccess = true + if (!env.SELF_HOSTED && !env.DISABLE_ACCOUNT_PORTAL) { + const account = await accounts.getAccount(user.email) + if (account) { + user.account = account + user.accountPortalAccess = true + } + } + }) + ) + + return users +} + /** * Get the requested user by id. * Use redis cache to first read the user. @@ -77,6 +102,27 @@ export async function getUser( return user } +/** + * Get the requested users by id. + * Use redis cache to first read the users. + * If not present fallback to loading the users directly and re-caching. + * @param {*} userIds the ids of the user to get + * @param {*} tenantId the tenant of the users to get + * @returns + */ +export async function getUsers(userIds: string[], tenantId: string) { + const client = await redis.getUserClient() + // try cache + let usersFromCache = await client.bulkGet(userIds) + const missingUsersFromCache = userIds.filter(uid => !usersFromCache[uid]) + const usersFromDb = await populateUsersFromDB(missingUsersFromCache, tenantId) + for (const userToCache of usersFromDb) { + await client.store(userToCache._id, userToCache, EXPIRY_SECONDS) + } + const users = [...Object.values(usersFromCache), ...usersFromDb] + return users +} + export async function invalidateUser(userId: string) { const client = await redis.getUserClient() await client.delete(userId) From d3b04ef4de108da40981fc53f8c5395d1579a86f Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 18 Sep 2023 13:46:59 +0200 Subject: [PATCH 03/10] Add tests --- .../backend-core/src/cache/tests/user.spec.ts | 95 +++++++++++++++++++ packages/backend-core/src/cache/user.ts | 15 ++- 2 files changed, 106 insertions(+), 4 deletions(-) create mode 100644 packages/backend-core/src/cache/tests/user.spec.ts diff --git a/packages/backend-core/src/cache/tests/user.spec.ts b/packages/backend-core/src/cache/tests/user.spec.ts new file mode 100644 index 0000000000..3d746fc506 --- /dev/null +++ b/packages/backend-core/src/cache/tests/user.spec.ts @@ -0,0 +1,95 @@ +import { User } from "@budibase/types" +import { tenancy } from "../.." +import { generator, structures } from "../../../tests" +import { DBTestConfiguration } from "../../../tests/extra" +import { getUsers } from "../user" +import { getGlobalDB, getGlobalDBName } from "../../context" +import _ from "lodash" +import { getDB } from "../../db" +import type * as TenancyType from "../../tenancy" + +const config = new DBTestConfiguration() + +// This mock is required to ensure that getTenantDB returns always as a singleton. +// This will allow us to spy on the db +const staticDb = getDB(getGlobalDBName(config.tenantId)) +jest.mock("../../tenancy", (): typeof TenancyType => ({ + ...jest.requireActual("../../tenancy"), + getTenantDB: jest.fn().mockImplementation(() => staticDb), +})) + +describe("user cache", () => { + describe("getUsers", () => { + const users: User[] = [] + beforeAll(async () => { + const userCount = 10 + const userIds = generator.arrayOf(() => generator.guid(), { + min: userCount, + max: userCount, + }) + + await config.doInTenant(async () => { + const db = getGlobalDB() + for (const userId of userIds) { + const user = structures.users.user({ _id: userId }) + await db.put(user) + users.push(user) + } + }) + }) + + beforeEach(() => { + jest.clearAllMocks() + }) + + it("when no user is in cache, all of them are retrieved from db", async () => { + const usersToRequest = _.sampleSize(users, 5) + + const userIdsToRequest = usersToRequest.map(x => x._id!) + + jest.spyOn(staticDb, "allDocs") + + const results = await getUsers(userIdsToRequest, config.tenantId) + + expect(results).toHaveLength(5) + expect(results).toEqual( + usersToRequest.map(u => ({ + ...u, + budibaseAccess: true, + _rev: expect.any(String), + })) + ) + + expect(tenancy.getTenantDB).toBeCalledTimes(1) + expect(tenancy.getTenantDB).toBeCalledWith(config.tenantId) + expect(staticDb.allDocs).toBeCalledTimes(1) + expect(staticDb.allDocs).toBeCalledWith({ + keys: userIdsToRequest, + include_docs: true, + limit: 5, + }) + }) + + it("on a second all, all of them are retrieved from cache", async () => { + const usersToRequest = _.sampleSize(users, 5) + + const userIdsToRequest = usersToRequest.map(x => x._id!) + + jest.spyOn(staticDb, "allDocs") + + await getUsers(userIdsToRequest, config.tenantId) + const resultsFromCache = await getUsers(userIdsToRequest, config.tenantId) + + expect(resultsFromCache).toHaveLength(5) + expect(resultsFromCache).toEqual( + usersToRequest.map(u => ({ + ...u, + budibaseAccess: true, + _rev: expect.any(String), + })) + ) + + expect(staticDb.allDocs).toBeCalledTimes(1) + }) + }) +}) diff --git a/packages/backend-core/src/cache/user.ts b/packages/backend-core/src/cache/user.ts index f05cdd42ac..9742b41b65 100644 --- a/packages/backend-core/src/cache/user.ts +++ b/packages/backend-core/src/cache/user.ts @@ -115,11 +115,18 @@ export async function getUsers(userIds: string[], tenantId: string) { // try cache let usersFromCache = await client.bulkGet(userIds) const missingUsersFromCache = userIds.filter(uid => !usersFromCache[uid]) - const usersFromDb = await populateUsersFromDB(missingUsersFromCache, tenantId) - for (const userToCache of usersFromDb) { - await client.store(userToCache._id, userToCache, EXPIRY_SECONDS) + const users = Object.values(usersFromCache) + + if (missingUsersFromCache.length) { + const usersFromDb = await populateUsersFromDB( + missingUsersFromCache, + tenantId + ) + for (const userToCache of usersFromDb) { + await client.store(userToCache._id, userToCache, EXPIRY_SECONDS) + } + users.push(...usersFromDb) } - const users = [...Object.values(usersFromCache), ...usersFromDb] return users } From 4311d563d2298bbd7fc23b063aa51c52faf14f70 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 18 Sep 2023 13:57:59 +0200 Subject: [PATCH 04/10] Add tests --- .../backend-core/src/cache/tests/user.spec.ts | 54 ++++++++++++++++--- 1 file changed, 47 insertions(+), 7 deletions(-) diff --git a/packages/backend-core/src/cache/tests/user.spec.ts b/packages/backend-core/src/cache/tests/user.spec.ts index 3d746fc506..45537694c7 100644 --- a/packages/backend-core/src/cache/tests/user.spec.ts +++ b/packages/backend-core/src/cache/tests/user.spec.ts @@ -1,5 +1,5 @@ import { User } from "@budibase/types" -import { tenancy } from "../.." +import { cache, tenancy } from "../.." import { generator, structures } from "../../../tests" import { DBTestConfiguration } from "../../../tests/extra" import { getUsers } from "../user" @@ -7,6 +7,7 @@ import { getGlobalDB, getGlobalDBName } from "../../context" import _ from "lodash" import { getDB } from "../../db" import type * as TenancyType from "../../tenancy" +import * as redis from "../../redis/init" const config = new DBTestConfiguration() @@ -38,8 +39,11 @@ describe("user cache", () => { }) }) - beforeEach(() => { + beforeEach(async () => { jest.clearAllMocks() + + const redisClient = await redis.getUserClient() + await redisClient.clear() }) it("when no user is in cache, all of them are retrieved from db", async () => { @@ -82,14 +86,50 @@ describe("user cache", () => { expect(resultsFromCache).toHaveLength(5) expect(resultsFromCache).toEqual( - usersToRequest.map(u => ({ - ...u, - budibaseAccess: true, - _rev: expect.any(String), - })) + expect.arrayContaining( + usersToRequest.map(u => ({ + ...u, + budibaseAccess: true, + _rev: expect.any(String), + })) + ) ) expect(staticDb.allDocs).toBeCalledTimes(1) }) + + it("when some users are cached, only the missing ones are retrieved from db", async () => { + const usersToRequest = _.sampleSize(users, 5) + + const userIdsToRequest = usersToRequest.map(x => x._id!) + + jest.spyOn(staticDb, "allDocs") + + await getUsers( + [userIdsToRequest[0], userIdsToRequest[3]], + config.tenantId + ) + ;(staticDb.allDocs as jest.Mock).mockClear() + + const results = await getUsers(userIdsToRequest, config.tenantId) + + expect(results).toHaveLength(5) + expect(results).toEqual( + expect.arrayContaining( + usersToRequest.map(u => ({ + ...u, + budibaseAccess: true, + _rev: expect.any(String), + })) + ) + ) + + expect(staticDb.allDocs).toBeCalledTimes(1) + expect(staticDb.allDocs).toBeCalledWith({ + keys: [userIdsToRequest[1], userIdsToRequest[2], userIdsToRequest[4]], + include_docs: true, + limit: 3, + }) + }) }) }) From 6f1d0271265627b29d28da9a0dfaae63385eae03 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 18 Sep 2023 14:38:20 +0200 Subject: [PATCH 05/10] Fix test --- .../backend-core/src/middleware/passport/sso/tests/sso.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/backend-core/src/middleware/passport/sso/tests/sso.spec.ts b/packages/backend-core/src/middleware/passport/sso/tests/sso.spec.ts index 484a118cbd..c3ddf220e6 100644 --- a/packages/backend-core/src/middleware/passport/sso/tests/sso.spec.ts +++ b/packages/backend-core/src/middleware/passport/sso/tests/sso.spec.ts @@ -102,6 +102,7 @@ describe("sso", () => { // modified external id to match user format ssoUser._id = "us_" + details.userId + delete ssoUser.userId // new sso user won't have a password delete ssoUser.password From 7b4585ce688a4a2cca2d7a147cd39bb535fb043a Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 19 Sep 2023 12:08:59 +0200 Subject: [PATCH 06/10] Tenantid optional --- packages/backend-core/src/cache/user.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/backend-core/src/cache/user.ts b/packages/backend-core/src/cache/user.ts index 9742b41b65..e21c8a1e43 100644 --- a/packages/backend-core/src/cache/user.ts +++ b/packages/backend-core/src/cache/user.ts @@ -110,7 +110,7 @@ export async function getUser( * @param {*} tenantId the tenant of the users to get * @returns */ -export async function getUsers(userIds: string[], tenantId: string) { +export async function getUsers(userIds: string[], tenantId?: string) { const client = await redis.getUserClient() // try cache let usersFromCache = await client.bulkGet(userIds) @@ -118,6 +118,7 @@ export async function getUsers(userIds: string[], tenantId: string) { const users = Object.values(usersFromCache) if (missingUsersFromCache.length) { + tenantId ??= context.getTenantId() const usersFromDb = await populateUsersFromDB( missingUsersFromCache, tenantId From 9e1ccc35ee92521d8639fa7885b9553663e82eed Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 19 Sep 2023 12:22:25 +0200 Subject: [PATCH 07/10] Handle missing users --- .../backend-core/src/cache/tests/user.spec.ts | 54 +++++++++++++------ packages/backend-core/src/cache/user.ts | 43 +++++++++++---- packages/backend-core/src/redis/redis.ts | 2 +- 3 files changed, 73 insertions(+), 26 deletions(-) diff --git a/packages/backend-core/src/cache/tests/user.spec.ts b/packages/backend-core/src/cache/tests/user.spec.ts index 45537694c7..490d769b92 100644 --- a/packages/backend-core/src/cache/tests/user.spec.ts +++ b/packages/backend-core/src/cache/tests/user.spec.ts @@ -55,14 +55,14 @@ describe("user cache", () => { const results = await getUsers(userIdsToRequest, config.tenantId) - expect(results).toHaveLength(5) - expect(results).toEqual( - usersToRequest.map(u => ({ + expect(results.users).toHaveLength(5) + expect(results).toEqual({ + users: usersToRequest.map(u => ({ ...u, budibaseAccess: true, _rev: expect.any(String), - })) - ) + })), + }) expect(tenancy.getTenantDB).toBeCalledTimes(1) expect(tenancy.getTenantDB).toBeCalledWith(config.tenantId) @@ -84,16 +84,16 @@ describe("user cache", () => { await getUsers(userIdsToRequest, config.tenantId) const resultsFromCache = await getUsers(userIdsToRequest, config.tenantId) - expect(resultsFromCache).toHaveLength(5) - expect(resultsFromCache).toEqual( - expect.arrayContaining( + expect(resultsFromCache.users).toHaveLength(5) + expect(resultsFromCache).toEqual({ + users: expect.arrayContaining( usersToRequest.map(u => ({ ...u, budibaseAccess: true, _rev: expect.any(String), })) - ) - ) + ), + }) expect(staticDb.allDocs).toBeCalledTimes(1) }) @@ -113,16 +113,16 @@ describe("user cache", () => { const results = await getUsers(userIdsToRequest, config.tenantId) - expect(results).toHaveLength(5) - expect(results).toEqual( - expect.arrayContaining( + expect(results.users).toHaveLength(5) + expect(results).toEqual({ + users: expect.arrayContaining( usersToRequest.map(u => ({ ...u, budibaseAccess: true, _rev: expect.any(String), })) - ) - ) + ), + }) expect(staticDb.allDocs).toBeCalledTimes(1) expect(staticDb.allDocs).toBeCalledWith({ @@ -131,5 +131,29 @@ describe("user cache", () => { limit: 3, }) }) + + it("requesting existing and unexisting ids will return found ones", async () => { + const usersToRequest = _.sampleSize(users, 3) + const missingIds = [generator.guid(), generator.guid()] + + const userIdsToRequest = _.shuffle([ + ...missingIds, + ...usersToRequest.map(x => x._id!), + ]) + + const results = await getUsers(userIdsToRequest, config.tenantId) + + expect(results.users).toHaveLength(3) + expect(results).toEqual({ + users: expect.arrayContaining( + usersToRequest.map(u => ({ + ...u, + budibaseAccess: true, + _rev: expect.any(String), + })) + ), + notFoundIds: expect.arrayContaining(missingIds), + }) + }) }) }) diff --git a/packages/backend-core/src/cache/user.ts b/packages/backend-core/src/cache/user.ts index e21c8a1e43..ccd9946504 100644 --- a/packages/backend-core/src/cache/user.ts +++ b/packages/backend-core/src/cache/user.ts @@ -6,6 +6,7 @@ import env from "../environment" import * as accounts from "../accounts" import { UserDB } from "../users" import { sdk } from "@budibase/shared-core" +import { User } from "@budibase/types" const EXPIRY_SECONDS = 3600 @@ -27,7 +28,10 @@ async function populateFromDB(userId: string, tenantId: string) { return user } -async function populateUsersFromDB(userIds: string[], tenantId: string) { +async function populateUsersFromDB( + userIds: string[], + tenantId: string +): Promise<{ users: User[]; notFoundIds?: string[] }> { const db = tenancy.getTenantDB(tenantId) const allDocsResponse = await db.allDocs({ keys: userIds, @@ -35,9 +39,22 @@ async function populateUsersFromDB(userIds: string[], tenantId: string) { limit: userIds.length, }) - const users = allDocsResponse.rows.map(r => r.doc) + const { users, notFoundIds } = allDocsResponse.rows.reduce( + (p, c) => { + if (c.doc) { + p.users.push(c.doc) + } else { + p.notFoundIds ??= [] + p.notFoundIds.push(c.key) + } + return p + }, + { + users: [], + } as { users: User[]; notFoundIds?: string[] } + ) await Promise.all( - users.map(async user => { + users.map(async (user: any) => { user.budibaseAccess = true if (!env.SELF_HOSTED && !env.DISABLE_ACCOUNT_PORTAL) { const account = await accounts.getAccount(user.email) @@ -49,7 +66,7 @@ async function populateUsersFromDB(userIds: string[], tenantId: string) { }) ) - return users + return { users, notFoundIds: notFoundIds } } /** @@ -110,12 +127,16 @@ export async function getUser( * @param {*} tenantId the tenant of the users to get * @returns */ -export async function getUsers(userIds: string[], tenantId?: string) { +export async function getUsers( + userIds: string[], + tenantId?: string +): Promise<{ users: User[]; notFoundIds?: string[] }> { const client = await redis.getUserClient() // try cache - let usersFromCache = await client.bulkGet(userIds) + let usersFromCache = await client.bulkGet(userIds) const missingUsersFromCache = userIds.filter(uid => !usersFromCache[uid]) const users = Object.values(usersFromCache) + let notFoundIds if (missingUsersFromCache.length) { tenantId ??= context.getTenantId() @@ -123,12 +144,14 @@ export async function getUsers(userIds: string[], tenantId?: string) { missingUsersFromCache, tenantId ) - for (const userToCache of usersFromDb) { - await client.store(userToCache._id, userToCache, EXPIRY_SECONDS) + + notFoundIds = usersFromDb.notFoundIds + for (const userToCache of usersFromDb.users) { + await client.store(userToCache._id!, userToCache, EXPIRY_SECONDS) } - users.push(...usersFromDb) + users.push(...usersFromDb.users) } - return users + return { users, notFoundIds: notFoundIds } } export async function invalidateUser(userId: string) { diff --git a/packages/backend-core/src/redis/redis.ts b/packages/backend-core/src/redis/redis.ts index 5056a5d549..5eaa2b4b61 100644 --- a/packages/backend-core/src/redis/redis.ts +++ b/packages/backend-core/src/redis/redis.ts @@ -242,7 +242,7 @@ class RedisWrapper { } } - async bulkGet(keys: string[]) { + async bulkGet(keys: string[]) { const db = this._db if (keys.length === 0) { return {} From 1d63b219b814d518db995a05710f461659d4349d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 19 Sep 2023 13:01:16 +0200 Subject: [PATCH 08/10] Make use of UserDB --- .../backend-core/src/cache/tests/user.spec.ts | 64 ++++++++----------- packages/backend-core/src/cache/user.ts | 43 ++++--------- 2 files changed, 38 insertions(+), 69 deletions(-) diff --git a/packages/backend-core/src/cache/tests/user.spec.ts b/packages/backend-core/src/cache/tests/user.spec.ts index 490d769b92..80e5bc3063 100644 --- a/packages/backend-core/src/cache/tests/user.spec.ts +++ b/packages/backend-core/src/cache/tests/user.spec.ts @@ -1,24 +1,15 @@ import { User } from "@budibase/types" -import { cache, tenancy } from "../.." import { generator, structures } from "../../../tests" import { DBTestConfiguration } from "../../../tests/extra" import { getUsers } from "../user" -import { getGlobalDB, getGlobalDBName } from "../../context" +import { getGlobalDB } from "../../context" import _ from "lodash" -import { getDB } from "../../db" -import type * as TenancyType from "../../tenancy" + import * as redis from "../../redis/init" +import { UserDB } from "../../users" const config = new DBTestConfiguration() -// This mock is required to ensure that getTenantDB returns always as a singleton. -// This will allow us to spy on the db -const staticDb = getDB(getGlobalDBName(config.tenantId)) -jest.mock("../../tenancy", (): typeof TenancyType => ({ - ...jest.requireActual("../../tenancy"), - getTenantDB: jest.fn().mockImplementation(() => staticDb), -})) - describe("user cache", () => { describe("getUsers", () => { const users: User[] = [] @@ -51,9 +42,9 @@ describe("user cache", () => { const userIdsToRequest = usersToRequest.map(x => x._id!) - jest.spyOn(staticDb, "allDocs") + jest.spyOn(UserDB, "bulkGet") - const results = await getUsers(userIdsToRequest, config.tenantId) + const results = await config.doInTenant(() => getUsers(userIdsToRequest)) expect(results.users).toHaveLength(5) expect(results).toEqual({ @@ -64,14 +55,8 @@ describe("user cache", () => { })), }) - expect(tenancy.getTenantDB).toBeCalledTimes(1) - expect(tenancy.getTenantDB).toBeCalledWith(config.tenantId) - expect(staticDb.allDocs).toBeCalledTimes(1) - expect(staticDb.allDocs).toBeCalledWith({ - keys: userIdsToRequest, - include_docs: true, - limit: 5, - }) + expect(UserDB.bulkGet).toBeCalledTimes(1) + expect(UserDB.bulkGet).toBeCalledWith(userIdsToRequest) }) it("on a second all, all of them are retrieved from cache", async () => { @@ -79,10 +64,12 @@ describe("user cache", () => { const userIdsToRequest = usersToRequest.map(x => x._id!) - jest.spyOn(staticDb, "allDocs") + jest.spyOn(UserDB, "bulkGet") - await getUsers(userIdsToRequest, config.tenantId) - const resultsFromCache = await getUsers(userIdsToRequest, config.tenantId) + await config.doInTenant(() => getUsers(userIdsToRequest)) + const resultsFromCache = await config.doInTenant(() => + getUsers(userIdsToRequest) + ) expect(resultsFromCache.users).toHaveLength(5) expect(resultsFromCache).toEqual({ @@ -95,7 +82,7 @@ describe("user cache", () => { ), }) - expect(staticDb.allDocs).toBeCalledTimes(1) + expect(UserDB.bulkGet).toBeCalledTimes(1) }) it("when some users are cached, only the missing ones are retrieved from db", async () => { @@ -103,15 +90,14 @@ describe("user cache", () => { const userIdsToRequest = usersToRequest.map(x => x._id!) - jest.spyOn(staticDb, "allDocs") + jest.spyOn(UserDB, "bulkGet") - await getUsers( - [userIdsToRequest[0], userIdsToRequest[3]], - config.tenantId + await config.doInTenant(() => + getUsers([userIdsToRequest[0], userIdsToRequest[3]]) ) - ;(staticDb.allDocs as jest.Mock).mockClear() + ;(UserDB.bulkGet as jest.Mock).mockClear() - const results = await getUsers(userIdsToRequest, config.tenantId) + const results = await config.doInTenant(() => getUsers(userIdsToRequest)) expect(results.users).toHaveLength(5) expect(results).toEqual({ @@ -124,12 +110,12 @@ describe("user cache", () => { ), }) - expect(staticDb.allDocs).toBeCalledTimes(1) - expect(staticDb.allDocs).toBeCalledWith({ - keys: [userIdsToRequest[1], userIdsToRequest[2], userIdsToRequest[4]], - include_docs: true, - limit: 3, - }) + expect(UserDB.bulkGet).toBeCalledTimes(1) + expect(UserDB.bulkGet).toBeCalledWith([ + userIdsToRequest[1], + userIdsToRequest[2], + userIdsToRequest[4], + ]) }) it("requesting existing and unexisting ids will return found ones", async () => { @@ -141,7 +127,7 @@ describe("user cache", () => { ...usersToRequest.map(x => x._id!), ]) - const results = await getUsers(userIdsToRequest, config.tenantId) + const results = await config.doInTenant(() => getUsers(userIdsToRequest)) expect(results.users).toHaveLength(3) expect(results).toEqual({ diff --git a/packages/backend-core/src/cache/user.ts b/packages/backend-core/src/cache/user.ts index ccd9946504..481d3691e4 100644 --- a/packages/backend-core/src/cache/user.ts +++ b/packages/backend-core/src/cache/user.ts @@ -29,30 +29,15 @@ async function populateFromDB(userId: string, tenantId: string) { } async function populateUsersFromDB( - userIds: string[], - tenantId: string + userIds: string[] ): Promise<{ users: User[]; notFoundIds?: string[] }> { - const db = tenancy.getTenantDB(tenantId) - const allDocsResponse = await db.allDocs({ - keys: userIds, - include_docs: true, - limit: userIds.length, - }) + const getUsersResponse = await UserDB.bulkGet(userIds) + + // Handle missed user ids + const notFoundIds = userIds.filter((uid, i) => !getUsersResponse[i]) + + const users = getUsersResponse.filter(x => x) - const { users, notFoundIds } = allDocsResponse.rows.reduce( - (p, c) => { - if (c.doc) { - p.users.push(c.doc) - } else { - p.notFoundIds ??= [] - p.notFoundIds.push(c.key) - } - return p - }, - { - users: [], - } as { users: User[]; notFoundIds?: string[] } - ) await Promise.all( users.map(async (user: any) => { user.budibaseAccess = true @@ -66,7 +51,10 @@ async function populateUsersFromDB( }) ) - return { users, notFoundIds: notFoundIds } + if (notFoundIds.length) { + return { users, notFoundIds } + } + return { users } } /** @@ -128,8 +116,7 @@ export async function getUser( * @returns */ export async function getUsers( - userIds: string[], - tenantId?: string + userIds: string[] ): Promise<{ users: User[]; notFoundIds?: string[] }> { const client = await redis.getUserClient() // try cache @@ -139,11 +126,7 @@ export async function getUsers( let notFoundIds if (missingUsersFromCache.length) { - tenantId ??= context.getTenantId() - const usersFromDb = await populateUsersFromDB( - missingUsersFromCache, - tenantId - ) + const usersFromDb = await populateUsersFromDB(missingUsersFromCache) notFoundIds = usersFromDb.notFoundIds for (const userToCache of usersFromDb.users) { From e128f1c9216e92bc3fbe2d28202913826a2ca36a Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 19 Sep 2023 13:27:42 +0200 Subject: [PATCH 09/10] Fix types --- packages/backend-core/src/cache/user.ts | 2 +- packages/backend-core/src/redis/redis.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/backend-core/src/cache/user.ts b/packages/backend-core/src/cache/user.ts index 481d3691e4..b3fd7c08cd 100644 --- a/packages/backend-core/src/cache/user.ts +++ b/packages/backend-core/src/cache/user.ts @@ -120,7 +120,7 @@ export async function getUsers( ): Promise<{ users: User[]; notFoundIds?: string[] }> { const client = await redis.getUserClient() // try cache - let usersFromCache = await client.bulkGet(userIds) + let usersFromCache = await client.bulkGet(userIds) const missingUsersFromCache = userIds.filter(uid => !usersFromCache[uid]) const users = Object.values(usersFromCache) let notFoundIds diff --git a/packages/backend-core/src/redis/redis.ts b/packages/backend-core/src/redis/redis.ts index 5eaa2b4b61..78817d0aa0 100644 --- a/packages/backend-core/src/redis/redis.ts +++ b/packages/backend-core/src/redis/redis.ts @@ -242,7 +242,7 @@ class RedisWrapper { } } - async bulkGet(keys: string[]) { + async bulkGet(keys: string[]) { const db = this._db if (keys.length === 0) { return {} @@ -250,7 +250,7 @@ class RedisWrapper { const prefixedKeys = keys.map(key => addDbPrefix(db, key)) let response = await this.getClient().mget(prefixedKeys) if (Array.isArray(response)) { - let final: any = {} + let final: Record = {} let count = 0 for (let result of response) { if (result) { From dade2314849dc6d7d6df8b6a6357f522a9c7840d Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Tue, 19 Sep 2023 12:15:45 +0000 Subject: [PATCH 10/10] Bump version to 2.10.9-alpha.5 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 219091b329..cf21e415f2 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.10.9-alpha.4", + "version": "2.10.9-alpha.5", "npmClient": "yarn", "packages": [ "packages/*"