Skip to content

Commit

Permalink
Optimize the entity permission to permit mappings (#1642)
Browse files Browse the repository at this point in the history
* Optimize the entity permission to permit mappings

In some cases, when a large number of entities are processed for
permission to permit mapping, the code would just hang without
indication of what went wrong.  At least 2 `yield` would be needed
to process each entity.  Over the course of 1000+ entities, the 2000
`yield` calls seem to cause problems.

The mapping code has been optimized to allow using a lookup cached
version.  Now only a single `yield` is required.  The change allows
1000+ entities to be processed successfully.

Signed-off-by: Scott J Dickerson <[email protected]>

* Setup `curryEntityPermissionsToUserPermits()`

Move from using `.cached()` to `curryEntityPermissionsToUserPermits()`
to make the form more explicit.

Fixed up the jsdoc on the util functions.

Signed-off-by: Scott J Dickerson <[email protected]>

* Minor changes

  - fix typos in docs
  - remove debug params in various places
  - add curried permit mapping to `entityPermissionsToUserPermits()`
  - use a consistent name of the curried function

Signed-off-by: Scott J Dickerson <[email protected]>

Signed-off-by: Scott J Dickerson <[email protected]>
  • Loading branch information
sjd78 authored Dec 14, 2022
1 parent 7a1a0d3 commit 52b126e
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 24 deletions.
11 changes: 7 additions & 4 deletions src/sagas/base-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {

import {
callExternalAction,
entityPermissionsToUserPermits,
curryEntityPermissionsToUserPermits,
mapCpuOptions,
mapConfigKeyVersion,
} from './utils'
Expand All @@ -35,8 +35,9 @@ export function* fetchAllClusters () {
)

// Calculate permits and 'canUser*'
const _entityPermissionsToUserPermits = yield curryEntityPermissionsToUserPermits()
for (const cluster of clustersInternal) {
cluster.userPermits = yield entityPermissionsToUserPermits(cluster)
cluster.userPermits = _entityPermissionsToUserPermits(cluster)
cluster.canUserUseCluster = canUserUseCluster(cluster.userPermits)
}

Expand Down Expand Up @@ -84,8 +85,9 @@ export function* fetchAllTemplates () {
)

// Calculate permits and 'canUser*'
const _entityPermissionsToUserPermits = yield curryEntityPermissionsToUserPermits()
for (const template of templatesInternal) {
template.userPermits = yield entityPermissionsToUserPermits(template)
template.userPermits = _entityPermissionsToUserPermits(template)
template.canUserUseTemplate = canUserUseTemplate(template.userPermits)
}

Expand Down Expand Up @@ -114,8 +116,9 @@ export function* fetchAllVnicProfiles () {
)

// Calculate permits and 'canUser*'
const _entityPermissionsToUserPermits = yield curryEntityPermissionsToUserPermits()
for (const vnicProfile of vnicProfilesInternal) {
vnicProfile.userPermits = yield entityPermissionsToUserPermits(vnicProfile)
vnicProfile.userPermits = _entityPermissionsToUserPermits(vnicProfile)
vnicProfile.canUserUseProfile = canUserUseVnicProfile(vnicProfile.userPermits)
}

Expand Down
8 changes: 5 additions & 3 deletions src/sagas/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ import {

import {
callExternalAction,
curryEntityPermissionsToUserPermits,
delay,
doCheckTokenExpired,
entityPermissionsToUserPermits,
mapCpuOptions,
} from './utils'

Expand Down Expand Up @@ -88,8 +88,10 @@ const VM_FETCH_ADDITIONAL_SHALLOW = [
export const EVERYONE_GROUP_ID = 'eee00000-0000-0000-0000-123456789eee'

export function* transformAndPermitVm (vm) {
const _entityPermissionsToUserPermits = yield curryEntityPermissionsToUserPermits()

const internalVm = Transforms.VM.toInternal({ vm })
internalVm.userPermits = yield entityPermissionsToUserPermits(internalVm)
internalVm.userPermits = _entityPermissionsToUserPermits(internalVm)

internalVm.canUserChangeCd = canUserChangeCd(internalVm.userPermits)
internalVm.canUserEditVm = canUserEditVm(internalVm.userPermits)
Expand All @@ -107,7 +109,7 @@ export function* transformAndPermitVm (vm) {

// Permit disks fetched and transformed along with the VM
for (const disk of internalVm.disks) {
disk.userPermits = yield entityPermissionsToUserPermits(disk)
disk.userPermits = _entityPermissionsToUserPermits(disk)
disk.canUserEditDisk = canUserEditDisk(disk.userPermits)
}

Expand Down
8 changes: 5 additions & 3 deletions src/sagas/storageDomains.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Api, { Transforms } from '_/ovirtapi'
import { all, call, put, select } from 'redux-saga/effects'

import { callExternalAction, entityPermissionsToUserPermits } from './utils'
import { callExternalAction, curryEntityPermissionsToUserPermits } from './utils'
import { canUserUseStorageDomain, canUserUseIsoImages } from '_/utils'

import {
Expand Down Expand Up @@ -50,8 +50,9 @@ function* fetchDataCenters () {
)

// Calculate permits and 'canUser*'
const _entityPermissionsToUserPermits = yield curryEntityPermissionsToUserPermits()
for (const dataCenter of dataCentersInternal) {
dataCenter.userPermits = yield entityPermissionsToUserPermits(dataCenter)
dataCenter.userPermits = _entityPermissionsToUserPermits(dataCenter)
}

return dataCentersInternal
Expand All @@ -69,8 +70,9 @@ function* fetchDataAndIsoStorageDomains () {
.filter(storageDomain => storageDomain.type === 'data' || storageDomain.type === 'iso')

// Calculate permits and 'canUser*'
const _entityPermissionsToUserPermits = yield curryEntityPermissionsToUserPermits()
for (const storageDomain of storageDomainsInternal) {
storageDomain.userPermits = yield entityPermissionsToUserPermits(storageDomain)
storageDomain.userPermits = _entityPermissionsToUserPermits(storageDomain)
storageDomain.canUserUseDomain = canUserUseStorageDomain(storageDomain.userPermits)
storageDomain.canUserUseIsoImages = canUserUseIsoImages(storageDomain.userPermits)
}
Expand Down
54 changes: 40 additions & 14 deletions src/sagas/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,29 +156,55 @@ export function* delayInMsSteps (count = 20, msMultiplier = 2000) {
}
}

export function* selectorUserAndRoles () {
const userAndRoles = yield select(state => ({
userGroups: state.config.get('userGroups'),
userId: state.config.getIn(['user', 'id']),
roles: state.roles,
}))

return userAndRoles
}

/**
* Lookup users and roles from the redux store and convert an entity's set of permissions
* to a set of permits for the app's current user. Function `mapEntityPermits` is called
* to do the actual mapping.
*
* NOTE: For effiency sake, use the cached lookup version `curryEntityPermissionsToUserPermits`
* in calling code. This helps keep the redux-saga effect queue small and memory use efficient.
*/
export function* entityPermissionsToUserPermits (entity) {
const userAndRoles = yield selectorUserAndRoles()
return mapEntityPermits(entity, userAndRoles)
}

/**
* Curry `mapEntityPermits` with a single lookup of user and roles from the redux store.
*
* NOTE: This version is most useful when processing multiple entities at a time.
*
* @returns A function that will convert an entity's set of permissions to a set of
* permits using a shared set of user and roles data.
*/
export function* curryEntityPermissionsToUserPermits () {
const userAndRoles = yield selectorUserAndRoles()
return (entity) => mapEntityPermits(entity, userAndRoles)
}

/**
* Convert an entity's set of permissions to a set of permits for the app's current
* user by mapping the permissions through their assigned roles to the permits.
*
* NOTE: If the user is an admin user the user's group and id membership must be
* explicitly checked.
*/
export function* entityPermissionsToUserPermits (entity) {
function mapEntityPermits (entity, { userGroups, userId, roles } = {}) {
const permissions = entity.permissions
? Array.isArray(entity.permissions) ? entity.permissions : [entity.permissions]
: []

const {
userGroups,
userId,
roles,
} = yield select(state => ({
userGroups: state.config.get('userGroups'),
userId: state.config.getIn(['user', 'id']),
roles: state.roles,
}))

const permitNames = []
const permitNames = new Set()
for (const permission of permissions) {
if (
(permission.groupId && userGroups.includes(permission.groupId)) ||
Expand All @@ -188,12 +214,12 @@ export function* entityPermissionsToUserPermits (entity) {
if (!role) {
console.info('Could not find role in redux state, roleId:', permission.roleId)
} else {
permitNames.push(...role.permitNames)
role.permitNames.forEach(name => permitNames.add(name))
}
}
}

return Array.from(new Set(permitNames))
return Array.from(permitNames)
}

/**
Expand Down

0 comments on commit 52b126e

Please sign in to comment.