Wide variety of improvements to performance, making some group operations synchronous to avoid re-fetching groups repeatedly.

This commit is contained in:
mike12345567 2025-02-28 17:11:42 +00:00
parent 48dafb42bc
commit f75cfc1db6
6 changed files with 77 additions and 45 deletions

View File

@ -26,8 +26,9 @@ import {
import { import {
getAccountHolderFromUsers, getAccountHolderFromUsers,
isAdmin, isAdmin,
isCreator, creatorsInList,
validateUniqueUser, validateUniqueUser,
isCreatorAsync,
} from "./utils" } from "./utils"
import { import {
getFirstPlatformUser, getFirstPlatformUser,
@ -261,8 +262,16 @@ export class UserDB {
} }
const change = dbUser ? 0 : 1 // no change if there is existing user const change = dbUser ? 0 : 1 // no change if there is existing user
const creatorsChange =
(await isCreator(dbUser)) !== (await isCreator(user)) ? 1 : 0 let creatorsChange = 0
if (dbUser) {
const [isDbUserCreator, isUserCreator] = await creatorsInList([
dbUser,
user,
])
creatorsChange = isDbUserCreator !== isUserCreator ? 1 : 0
}
return UserDB.quotas.addUsers(change, creatorsChange, async () => { return UserDB.quotas.addUsers(change, creatorsChange, async () => {
if (!opts.isAccountHolder) { if (!opts.isAccountHolder) {
await validateUniqueUser(email, tenantId) await validateUniqueUser(email, tenantId)
@ -353,7 +362,7 @@ export class UserDB {
} }
newUser.userGroups = groups || [] newUser.userGroups = groups || []
newUsers.push(newUser) newUsers.push(newUser)
if (await isCreator(newUser)) { if (await isCreatorAsync(newUser)) {
newCreators.push(newUser) newCreators.push(newUser)
} }
} }
@ -453,10 +462,8 @@ export class UserDB {
})) }))
const dbResponse = await usersCore.bulkUpdateGlobalUsers(toDelete) const dbResponse = await usersCore.bulkUpdateGlobalUsers(toDelete)
const creatorsEval = await Promise.all(usersToDelete.map(isCreator)) const creatorsEval = await creatorsInList(usersToDelete)
const creatorsToDeleteCount = creatorsEval.filter( const creatorsToDeleteCount = creatorsEval.filter(creator => creator).length
creator => !!creator
).length
const ssoUsersToDelete: AnyDocument[] = [] const ssoUsersToDelete: AnyDocument[] = []
for (let user of usersToDelete) { for (let user of usersToDelete) {
@ -533,7 +540,7 @@ export class UserDB {
await db.remove(userId, dbUser._rev!) await db.remove(userId, dbUser._rev!)
const creatorsToDelete = (await isCreator(dbUser)) ? 1 : 0 const creatorsToDelete = (await isCreatorAsync(dbUser)) ? 1 : 0
await UserDB.quotas.removeUsers(1, creatorsToDelete) await UserDB.quotas.removeUsers(1, creatorsToDelete)
await eventHelpers.handleDeleteEvents(dbUser) await eventHelpers.handleDeleteEvents(dbUser)
await cache.user.invalidateUser(userId) await cache.user.invalidateUser(userId)

View File

@ -2,39 +2,39 @@ import { User, UserGroup } from "@budibase/types"
import { generator, structures } from "../../../tests" import { generator, structures } from "../../../tests"
import { DBTestConfiguration } from "../../../tests/extra" import { DBTestConfiguration } from "../../../tests/extra"
import { getGlobalDB } from "../../context" import { getGlobalDB } from "../../context"
import { isCreator } from "../utils" import { isCreatorSync, creatorsInList } from "../utils"
const config = new DBTestConfiguration() const config = new DBTestConfiguration()
describe("Users", () => { describe("Users", () => {
it("User is a creator if it is configured as a global builder", async () => { it("User is a creator if it is configured as a global builder", () => {
const user: User = structures.users.user({ builder: { global: true } }) const user: User = structures.users.user({ builder: { global: true } })
expect(await isCreator(user)).toBe(true) expect(isCreatorSync(user, [])).toBe(true)
}) })
it("User is a creator if it is configured as a global admin", async () => { it("User is a creator if it is configured as a global admin", () => {
const user: User = structures.users.user({ admin: { global: true } }) const user: User = structures.users.user({ admin: { global: true } })
expect(await isCreator(user)).toBe(true) expect(isCreatorSync(user, [])).toBe(true)
}) })
it("User is a creator if it is configured with creator permission", async () => { it("User is a creator if it is configured with creator permission", () => {
const user: User = structures.users.user({ builder: { creator: true } }) const user: User = structures.users.user({ builder: { creator: true } })
expect(await isCreator(user)).toBe(true) expect(isCreatorSync(user, [])).toBe(true)
}) })
it("User is a creator if it is a builder in some application", async () => { it("User is a creator if it is a builder in some application", () => {
const user: User = structures.users.user({ builder: { apps: ["app1"] } }) const user: User = structures.users.user({ builder: { apps: ["app1"] } })
expect(await isCreator(user)).toBe(true) expect(isCreatorSync(user, [])).toBe(true)
}) })
it("User is a creator if it has CREATOR permission in some application", async () => { it("User is a creator if it has CREATOR permission in some application", () => {
const user: User = structures.users.user({ roles: { app1: "CREATOR" } }) const user: User = structures.users.user({ roles: { app1: "CREATOR" } })
expect(await isCreator(user)).toBe(true) expect(isCreatorSync(user, [])).toBe(true)
}) })
it("User is a creator if it has ADMIN permission in some application", async () => { it("User is a creator if it has ADMIN permission in some application", () => {
const user: User = structures.users.user({ roles: { app1: "ADMIN" } }) const user: User = structures.users.user({ roles: { app1: "ADMIN" } })
expect(await isCreator(user)).toBe(true) expect(isCreatorSync(user, [])).toBe(true)
}) })
it("User is a creator if it remains to a group with ADMIN permissions", async () => { it("User is a creator if it remains to a group with ADMIN permissions", async () => {
@ -59,7 +59,7 @@ describe("Users", () => {
await db.put(group) await db.put(group)
for (let user of users) { for (let user of users) {
await db.put(user) await db.put(user)
const creator = await isCreator(user) const creator = (await creatorsInList([user]))[0]
expect(creator).toBe(true) expect(creator).toBe(true)
} }
}) })

View File

@ -22,7 +22,7 @@ import {
} from "@budibase/types" } from "@budibase/types"
import * as context from "../context" import * as context from "../context"
import { getGlobalDB } from "../context" import { getGlobalDB } from "../context"
import { isCreator } from "./utils" import { creatorsInList } from "./utils"
import { UserDB } from "./db" import { UserDB } from "./db"
import { dataFilters } from "@budibase/shared-core" import { dataFilters } from "@budibase/shared-core"
@ -305,8 +305,8 @@ export async function getCreatorCount() {
let creators = 0 let creators = 0
async function iterate(startPage?: string) { async function iterate(startPage?: string) {
const page = await paginatedUsers({ bookmark: startPage }) const page = await paginatedUsers({ bookmark: startPage })
const creatorsEval = await Promise.all(page.data.map(isCreator)) const creatorsEval = await creatorsInList(page.data)
creators += creatorsEval.filter(creator => !!creator).length creators += creatorsEval.filter(creator => creator).length
if (page.hasNextPage) { if (page.hasNextPage) {
await iterate(page.nextPage) await iterate(page.nextPage)
} }

View File

@ -16,30 +16,55 @@ export const hasAdminPermissions = sdk.users.hasAdminPermissions
export const hasBuilderPermissions = sdk.users.hasBuilderPermissions export const hasBuilderPermissions = sdk.users.hasBuilderPermissions
export const hasAppBuilderPermissions = sdk.users.hasAppBuilderPermissions export const hasAppBuilderPermissions = sdk.users.hasAppBuilderPermissions
export async function isCreator(user?: User | ContextUser) { async function getGroups(groupIds: string[]) {
if (groupIds.length) {
const db = context.getGlobalDB()
return await db.getMultiple<UserGroup>(groupIds)
}
return []
}
export async function creatorsInList(
users: (User | ContextUser)[],
groups?: UserGroup[]
) {
if (!groups) {
const groupIds = users.flatMap(user => user.userGroups || [])
if (groupIds.length) {
groups = await getGroups(groupIds)
}
}
return users.map(user => isCreatorSync(user, groups || []))
}
// fetches groups if no provided, but is async and shouldn't be looped with
export async function isCreatorAsync(
user: User | ContextUser,
groups?: UserGroup[]
) {
if (!groups) {
groups = await getGroups(user.userGroups || [])
}
return isCreatorSync(user, groups)
}
export function isCreatorSync(user: User | ContextUser, groups: UserGroup[]) {
const isCreatorByUserDefinition = sdk.users.isCreator(user) const isCreatorByUserDefinition = sdk.users.isCreator(user)
if (!isCreatorByUserDefinition && user) { if (!isCreatorByUserDefinition && user) {
return await isCreatorByGroupMembership(user) return isCreatorByGroupMembership(user, groups)
} }
return isCreatorByUserDefinition return isCreatorByUserDefinition
} }
async function isCreatorByGroupMembership(user?: User | ContextUser) { function isCreatorByGroupMembership(
const userGroups = user?.userGroups || [] user: User | ContextUser,
groups: UserGroup[]
) {
const userGroups = groups.filter(
group => user.userGroups?.indexOf(group._id!) !== -1
)
if (userGroups.length > 0) { if (userGroups.length > 0) {
const db = context.getGlobalDB() return userGroups.some(group =>
const groups: UserGroup[] = []
for (let groupId of userGroups) {
try {
const group = await db.get<UserGroup>(groupId)
groups.push(group)
} catch (e: any) {
if (e.error !== "not_found") {
throw e
}
}
}
return groups.some(group =>
Object.values(group.roles || {}).includes(BUILTIN_ROLE_IDS.ADMIN) Object.values(group.roles || {}).includes(BUILTIN_ROLE_IDS.ADMIN)
) )
} }

@ -1 +1 @@
Subproject commit e3843dd4eaced68ae063355b77df200dbc789c98 Subproject commit a1903c9b8c5985f72ddb33b40b4a37521aaf8fa0

View File

@ -34,7 +34,7 @@ const checkAuthorized = async (
const isCreatorApi = permType === PermissionType.CREATOR const isCreatorApi = permType === PermissionType.CREATOR
const isBuilderApi = permType === PermissionType.BUILDER const isBuilderApi = permType === PermissionType.BUILDER
const isGlobalBuilder = users.isGlobalBuilder(ctx.user) const isGlobalBuilder = users.isGlobalBuilder(ctx.user)
const isCreator = await users.isCreator(ctx.user) const isCreator = await users.isCreatorAsync(ctx.user)
const isBuilder = appId const isBuilder = appId
? users.isBuilder(ctx.user, appId) ? users.isBuilder(ctx.user, appId)
: users.hasBuilderPermissions(ctx.user) : users.hasBuilderPermissions(ctx.user)