Make use of UserDB

This commit is contained in:
Adria Navarro 2023-09-19 13:01:16 +02:00
parent 9e1ccc35ee
commit 1d63b219b8
2 changed files with 38 additions and 69 deletions

View File

@ -1,24 +1,15 @@
import { User } from "@budibase/types" import { User } from "@budibase/types"
import { cache, tenancy } from "../.."
import { generator, structures } from "../../../tests" import { generator, structures } from "../../../tests"
import { DBTestConfiguration } from "../../../tests/extra" import { DBTestConfiguration } from "../../../tests/extra"
import { getUsers } from "../user" import { getUsers } from "../user"
import { getGlobalDB, getGlobalDBName } from "../../context" import { getGlobalDB } from "../../context"
import _ from "lodash" import _ from "lodash"
import { getDB } from "../../db"
import type * as TenancyType from "../../tenancy"
import * as redis from "../../redis/init" import * as redis from "../../redis/init"
import { UserDB } from "../../users"
const config = new DBTestConfiguration() 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("user cache", () => {
describe("getUsers", () => { describe("getUsers", () => {
const users: User[] = [] const users: User[] = []
@ -51,9 +42,9 @@ describe("user cache", () => {
const userIdsToRequest = usersToRequest.map(x => x._id!) 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.users).toHaveLength(5)
expect(results).toEqual({ expect(results).toEqual({
@ -64,14 +55,8 @@ describe("user cache", () => {
})), })),
}) })
expect(tenancy.getTenantDB).toBeCalledTimes(1) expect(UserDB.bulkGet).toBeCalledTimes(1)
expect(tenancy.getTenantDB).toBeCalledWith(config.tenantId) expect(UserDB.bulkGet).toBeCalledWith(userIdsToRequest)
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 () => { 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!) const userIdsToRequest = usersToRequest.map(x => x._id!)
jest.spyOn(staticDb, "allDocs") jest.spyOn(UserDB, "bulkGet")
await getUsers(userIdsToRequest, config.tenantId) await config.doInTenant(() => getUsers(userIdsToRequest))
const resultsFromCache = await getUsers(userIdsToRequest, config.tenantId) const resultsFromCache = await config.doInTenant(() =>
getUsers(userIdsToRequest)
)
expect(resultsFromCache.users).toHaveLength(5) expect(resultsFromCache.users).toHaveLength(5)
expect(resultsFromCache).toEqual({ 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 () => { 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!) const userIdsToRequest = usersToRequest.map(x => x._id!)
jest.spyOn(staticDb, "allDocs") jest.spyOn(UserDB, "bulkGet")
await getUsers( await config.doInTenant(() =>
[userIdsToRequest[0], userIdsToRequest[3]], getUsers([userIdsToRequest[0], userIdsToRequest[3]])
config.tenantId
) )
;(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.users).toHaveLength(5)
expect(results).toEqual({ expect(results).toEqual({
@ -124,12 +110,12 @@ describe("user cache", () => {
), ),
}) })
expect(staticDb.allDocs).toBeCalledTimes(1) expect(UserDB.bulkGet).toBeCalledTimes(1)
expect(staticDb.allDocs).toBeCalledWith({ expect(UserDB.bulkGet).toBeCalledWith([
keys: [userIdsToRequest[1], userIdsToRequest[2], userIdsToRequest[4]], userIdsToRequest[1],
include_docs: true, userIdsToRequest[2],
limit: 3, userIdsToRequest[4],
}) ])
}) })
it("requesting existing and unexisting ids will return found ones", async () => { it("requesting existing and unexisting ids will return found ones", async () => {
@ -141,7 +127,7 @@ describe("user cache", () => {
...usersToRequest.map(x => x._id!), ...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.users).toHaveLength(3)
expect(results).toEqual({ expect(results).toEqual({

View File

@ -29,30 +29,15 @@ async function populateFromDB(userId: string, tenantId: string) {
} }
async function populateUsersFromDB( async function populateUsersFromDB(
userIds: string[], userIds: string[]
tenantId: string
): Promise<{ users: User[]; notFoundIds?: string[] }> { ): Promise<{ users: User[]; notFoundIds?: string[] }> {
const db = tenancy.getTenantDB(tenantId) const getUsersResponse = await UserDB.bulkGet(userIds)
const allDocsResponse = await db.allDocs<any>({
keys: userIds, // Handle missed user ids
include_docs: true, const notFoundIds = userIds.filter((uid, i) => !getUsersResponse[i])
limit: userIds.length,
}) 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( await Promise.all(
users.map(async (user: any) => { users.map(async (user: any) => {
user.budibaseAccess = true 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 * @returns
*/ */
export async function getUsers( export async function getUsers(
userIds: string[], userIds: string[]
tenantId?: string
): Promise<{ users: User[]; notFoundIds?: string[] }> { ): Promise<{ users: User[]; notFoundIds?: string[] }> {
const client = await redis.getUserClient() const client = await redis.getUserClient()
// try cache // try cache
@ -139,11 +126,7 @@ export async function getUsers(
let notFoundIds let notFoundIds
if (missingUsersFromCache.length) { if (missingUsersFromCache.length) {
tenantId ??= context.getTenantId() const usersFromDb = await populateUsersFromDB(missingUsersFromCache)
const usersFromDb = await populateUsersFromDB(
missingUsersFromCache,
tenantId
)
notFoundIds = usersFromDb.notFoundIds notFoundIds = usersFromDb.notFoundIds
for (const userToCache of usersFromDb.users) { for (const userToCache of usersFromDb.users) {