From 1d63b219b814d518db995a05710f461659d4349d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 19 Sep 2023 13:01:16 +0200 Subject: [PATCH] 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) {