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 {}