Merge pull request #11823 from Budibase/BUDI-7455/make_usage_of_bulk_cache

Make use of bulk cache for BB references

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<User>(userIds)
   const missingUsersFromCache = userIds.filter(uid => !usersFromCache[uid])
   const users = Object.values(usersFromCache)
   let notFoundIds

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 {}
@@ -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: Record<string, any> = {}
+      let final: Record<string, T> = {}
       let count = 0
       for (let result of response) {
         if (result) {

packages/backend-core/tests/extra/DBTestConfiguration.ts

@@ -18,7 +18,7 @@ class DBTestConfiguration {
 
   // TENANCY
 
-  doInTenant(task: any) {
+  doInTenant<T>(task: () => Promise<T>) {
     return context.doInTenant(this.tenantId, () => {
       return task()
     })

packages/backend-core/tests/index.ts

@@ -1 +1,2 @@
 export * from "./core/utilities"
+export * from "./extra"

packages/server/src/sdk/app/applications/applications.ts

@@ -35,7 +35,7 @@ export async function fetch(status: AppStatus, user: ContextUser) {
     for (let app of apps) {
       const lock = locks[app.appId]
       if (lock) {
-        app.lockedBy = lock
+        app.lockedBy = lock as any
       } else {
         // make sure its definitely not present
         delete app.lockedBy

packages/server/src/utilities/rowProcessor/bbReferenceProcessor.ts

@@ -19,19 +19,12 @@ export async function processInputBBReferences(
         result.push(...value.split(",").map((id: string) => id.trim()))
       }
 
-      for (const id of result) {
+      const { notFoundIds } = await cache.user.getUsers(result)
-        try {
+
-          const user = await cache.user.getUser(id)
+      if (notFoundIds?.length) {
-          if (!user) {
+        throw new InvalidBBRefError(notFoundIds[0], FieldSubtype.USER)
-            throw new InvalidBBRefError(id, FieldSubtype.USER)
-          }
-        } catch (err: any) {
-          if (err != null && err.status === 404 && err.error === "not_found") {
-            throw new InvalidBBRefError(id, FieldSubtype.USER)
-          }
-          throw err
-        }
       }
+
       break
     default:
       throw utils.unreachable(subtype)
@@ -49,26 +42,14 @@ export async function processOutputBBReferences(
     return value
   }
 
-  const result = []
+  const ids = value.split(",").filter(id => !!id)
-
-  const validIds = value.split(",").filter(id => !!id)
 
   switch (subtype) {
     case FieldSubtype.USER:
-      for (const id of validIds) {
+      const { users } = await cache.user.getUsers(ids)
-        try {
+      return users
-          const user = await cache.user.getUser(id)
+
-          if (user) {
-            result.push(user)
-          }
-        } catch {
-          // If user cannot be found, we just strip it
-        }
-      }
-      break
     default:
       throw utils.unreachable(subtype)
   }
-
-  return result
 }

packages/server/src/utilities/rowProcessor/tests/bbReferenceProcessor.spec.ts

@@ -1,133 +1,142 @@
+import _ from "lodash"
 import * as backendCore from "@budibase/backend-core"
 import { FieldSubtype, User } from "@budibase/types"
 import {
   processInputBBReferences,
   processOutputBBReferences,
 } from "../bbReferenceProcessor"
-import { generator, structures } from "@budibase/backend-core/tests"
+import {
+  DBTestConfiguration,
+  generator,
+  structures,
+} from "@budibase/backend-core/tests"
 import { InvalidBBRefError } from "../errors"
 
 jest.mock("@budibase/backend-core", (): typeof backendCore => {
-  const actual = jest.requireActual("@budibase/backend-core")
+  const actual: typeof backendCore = jest.requireActual(
+    "@budibase/backend-core"
+  )
   return {
     ...actual,
     cache: {
       ...actual.cache,
       user: {
-        getUser: jest.fn(),
+        ...actual.cache.user,
-        invalidateUser: jest.fn(),
+        getUsers: jest.fn(actual.cache.user.getUsers),
       },
     },
   }
 })
 
+const config = new DBTestConfiguration()
+
 describe("bbReferenceProcessor", () => {
-  const mockedCacheGetUser = backendCore.cache.user.getUser as jest.Mock
+  const cacheGetUsersSpy = backendCore.cache.user
+    .getUsers as jest.MockedFunction<typeof backendCore.cache.user.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 = backendCore.context.getGlobalDB()
+      for (const userId of userIds) {
+        const user = structures.users.user({ _id: userId })
+        await db.put(user)
+        users.push(user)
+      }
+    })
+  })
 
   beforeEach(() => {
-    jest.resetAllMocks()
+    jest.clearAllMocks()
   })
 
   describe("processInputBBReferences", () => {
     describe("subtype user", () => {
       it("validate valid string id", async () => {
-        const userId = generator.guid()
+        const user = _.sample(users)
+        const userId = user!._id!
 
-        const userFromCache = structures.users.user()
+        const result = await config.doInTenant(() =>
-        mockedCacheGetUser.mockResolvedValueOnce(userFromCache)
+          processInputBBReferences(userId, FieldSubtype.USER)
-
+        )
-        const result = await processInputBBReferences(userId, FieldSubtype.USER)
 
         expect(result).toEqual(userId)
-        expect(mockedCacheGetUser).toBeCalledTimes(1)
+        expect(cacheGetUsersSpy).toBeCalledTimes(1)
-        expect(mockedCacheGetUser).toBeCalledWith(userId)
+        expect(cacheGetUsersSpy).toBeCalledWith([userId])
       })
 
       it("throws an error given an invalid id", async () => {
         const userId = generator.guid()
 
-        mockedCacheGetUser.mockRejectedValue({
-          status: 404,
-          error: "not_found",
-        })
-
         await expect(
-          processInputBBReferences(userId, FieldSubtype.USER)
+          config.doInTenant(() =>
+            processInputBBReferences(userId, FieldSubtype.USER)
+          )
         ).rejects.toThrowError(new InvalidBBRefError(userId, FieldSubtype.USER))
+        expect(cacheGetUsersSpy).toBeCalledTimes(1)
+        expect(cacheGetUsersSpy).toBeCalledWith([userId])
       })
 
       it("validates valid user ids as csv", async () => {
-        const userIds: string[] = []
+        const userIds = _.sampleSize(users, 5).map(x => x._id!)
-        for (let i = 0; i < 5; i++) {
-          const userId = generator.guid()
-          const user = structures.users.user({ _id: userId })
-          mockedCacheGetUser.mockResolvedValueOnce(user)
-          userIds.push(userId)
-        }
 
         const userIdCsv = userIds.join(" ,  ")
-        const result = await processInputBBReferences(
+        const result = await config.doInTenant(() =>
-          userIdCsv,
+          processInputBBReferences(userIdCsv, FieldSubtype.USER)
-          FieldSubtype.USER
         )
 
         expect(result).toEqual(userIds.join(","))
-        expect(mockedCacheGetUser).toBeCalledTimes(5)
+        expect(cacheGetUsersSpy).toBeCalledTimes(1)
-        userIds.forEach(userId => {
+        expect(cacheGetUsersSpy).toBeCalledWith(userIds)
-          expect(mockedCacheGetUser).toBeCalledWith(userId)
-        })
       })
 
       it("throws an error given an invalid id in a csv", async () => {
-        const userId1 = generator.guid()
+        const expectedUserIds = _.sampleSize(users, 2).map(x => x._id!)
-        const userId2 = generator.guid()
+        const wrongId = generator.guid()
-        const userId3 = generator.guid()
-        mockedCacheGetUser.mockResolvedValueOnce(
-          structures.users.user({ _id: userId1 })
-        )
-        mockedCacheGetUser.mockResolvedValueOnce(
-          structures.users.user({ _id: userId2 })
-        )
 
-        const userIdCsv = [userId1, userId2, userId3].join(" ,  ")
+        const userIdCsv = [
+          expectedUserIds[0],
+          wrongId,
+          expectedUserIds[1],
+        ].join(" ,  ")
 
         await expect(
-          processInputBBReferences(userIdCsv, FieldSubtype.USER)
+          config.doInTenant(() =>
+            processInputBBReferences(userIdCsv, FieldSubtype.USER)
+          )
         ).rejects.toThrowError(
-          new InvalidBBRefError(userId3, FieldSubtype.USER)
+          new InvalidBBRefError(wrongId, FieldSubtype.USER)
         )
       })
 
       it("validate valid user object", async () => {
-        const userId = generator.guid()
+        const userId = _.sample(users)!._id!
 
-        const userFromCache = structures.users.user()
+        const result = await config.doInTenant(() =>
-        mockedCacheGetUser.mockResolvedValueOnce(userFromCache)
+          processInputBBReferences({ _id: userId }, FieldSubtype.USER)
-
-        const result = await processInputBBReferences(
-          { _id: userId },
-          FieldSubtype.USER
         )
 
         expect(result).toEqual(userId)
-        expect(mockedCacheGetUser).toBeCalledTimes(1)
+        expect(cacheGetUsersSpy).toBeCalledTimes(1)
-        expect(mockedCacheGetUser).toBeCalledWith(userId)
+        expect(cacheGetUsersSpy).toBeCalledWith([userId])
       })
 
       it("validate valid user object array", async () => {
-        const users = Array.from({ length: 3 }, () => ({
+        const userIds = _.sampleSize(users, 3).map(x => x._id!)
-          _id: generator.guid(),
-        }))
 
-        mockedCacheGetUser.mockResolvedValue(structures.users.user())
+        const result = await config.doInTenant(() =>
+          processInputBBReferences(userIds, FieldSubtype.USER)
+        )
 
-        const result = await processInputBBReferences(users, FieldSubtype.USER)
+        expect(result).toEqual(userIds.join(","))
-
+        expect(cacheGetUsersSpy).toBeCalledTimes(1)
-        expect(result).toEqual(users.map(x => x._id).join(","))
+        expect(cacheGetUsersSpy).toBeCalledWith(userIds)
-        expect(mockedCacheGetUser).toBeCalledTimes(3)
-        for (const user of users) {
-          expect(mockedCacheGetUser).toBeCalledWith(user._id)
-        }
       })
     })
   })
@@ -135,39 +144,45 @@ describe("bbReferenceProcessor", () => {
   describe("processOutputBBReferences", () => {
     describe("subtype user", () => {
       it("fetches user given a valid string id", async () => {
-        const userId = generator.guid()
+        const user = _.sample(users)!
+        const userId = user._id!
 
-        const userFromCache = structures.users.user()
+        const result = await config.doInTenant(() =>
-        mockedCacheGetUser.mockResolvedValueOnce(userFromCache)
+          processOutputBBReferences(userId, FieldSubtype.USER)
-
-        const result = await processOutputBBReferences(
-          userId,
-          FieldSubtype.USER
         )
 
-        expect(result).toEqual([userFromCache])
+        expect(result).toEqual([
-        expect(mockedCacheGetUser).toBeCalledTimes(1)
+          {
-        expect(mockedCacheGetUser).toBeCalledWith(userId)
+            ...user,
+            budibaseAccess: true,
+            _rev: expect.any(String),
+          },
+        ])
+        expect(cacheGetUsersSpy).toBeCalledTimes(1)
+        expect(cacheGetUsersSpy).toBeCalledWith([userId])
       })
 
       it("fetches user given a valid string id csv", async () => {
-        const userId1 = generator.guid()
+        const [user1, user2] = _.sampleSize(users, 2)
-        const userId2 = generator.guid()
+        const userId1 = user1._id!
+        const userId2 = user2._id!
 
-        const userFromCache1 = structures.users.user({ _id: userId1 })
+        const result = await config.doInTenant(() =>
-        const userFromCache2 = structures.users.user({ _id: userId2 })
+          processOutputBBReferences(
-        mockedCacheGetUser.mockResolvedValueOnce(userFromCache1)
+            [userId1, userId2].join(","),
-        mockedCacheGetUser.mockResolvedValueOnce(userFromCache2)
+            FieldSubtype.USER
-
+          )
-        const result = await processOutputBBReferences(
-          [userId1, userId2].join(","),
-          FieldSubtype.USER
         )
 
-        expect(result).toEqual([userFromCache1, userFromCache2])
+        expect(result).toEqual(
-        expect(mockedCacheGetUser).toBeCalledTimes(2)
+          [user1, user2].map(u => ({
-        expect(mockedCacheGetUser).toBeCalledWith(userId1)
+            ...u,
-        expect(mockedCacheGetUser).toBeCalledWith(userId2)
+            budibaseAccess: true,
+            _rev: expect.any(String),
+          }))
+        )
+        expect(cacheGetUsersSpy).toBeCalledTimes(1)
+        expect(cacheGetUsersSpy).toBeCalledWith([userId1, userId2])
       })
     })
   })

packages/server/src/websockets/websocket.ts

@@ -150,7 +150,7 @@ export class BaseSocket {
     if (room) {
       const sessionIds = await this.getRoomSessionIds(room)
       const keys = sessionIds.map(this.getSessionKey.bind(this))
-      const sessions = await this.redisClient?.bulkGet(keys)
+      const sessions = await this.redisClient?.bulkGet<SocketSession>(keys)
       return Object.values(sessions || {})
     } else {
       return []