Handle missing users

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.users).toHaveLength(5)
-      expect(results).toEqual(
+      expect(results).toEqual({
-        usersToRequest.map(u => ({
+        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.users).toHaveLength(5)
-      expect(resultsFromCache).toEqual(
+      expect(resultsFromCache).toEqual({
-        expect.arrayContaining(
+        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.users).toHaveLength(5)
-      expect(results).toEqual(
+      expect(results).toEqual({
-        expect.arrayContaining(
+        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),
+      })
+    })
   })
 })

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<any>({
     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<User>(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) {

packages/backend-core/src/redis/redis.ts

@@ -242,7 +242,7 @@ class RedisWrapper {
     }
   }
 
-  async bulkGet(keys: string[]) {
+  async bulkGet<T>(keys: string[]) {
     const db = this._db
     if (keys.length === 0) {
       return {}