graph/users: small optimization for filter by role queries

When filtering by role assignment we now populate the
'appRoleAssignments' property of the returned users when the incoming
request contained and '$expand=appRoleAssignments'. This avoids having
to query the role assignments for all users in the result set a second
time.
The effect of this optimization depends a lot on the actual setup. In
single binary all-in-one installations it is rather small (this it saves
quite a few CPU cycles). In distributed setup it should be larger as it
reduces the number of network round trips to the settings service
significantly.
This commit is contained in:
Ralf Haferkamp
2024-06-06 09:22:52 +02:00
committed by Ralf Haferkamp
parent 4d29f5949c
commit 96e59598d3
3 changed files with 96 additions and 5 deletions
+1 -1
View File
@@ -298,7 +298,7 @@ func (g Graph) GetUsers(w http.ResponseWriter, r *http.Request) {
expandAppRoleAssignments := slices.Contains(exp, "appRoleAssignments")
expandMemberOf := slices.Contains(exp, "memberOf")
for _, u := range users {
if expandAppRoleAssignments {
if expandAppRoleAssignments && u.AppRoleAssignments == nil {
u.AppRoleAssignments, err = g.fetchAppRoleAssignments(r.Context(), u.GetId())
if err != nil {
// TODO I think we should not continue here, see http://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part1-protocol.html#sec_SystemQueryOptionexpand
+21 -4
View File
@@ -156,7 +156,7 @@ func (g Graph) applyFilterLogicalAnd(ctx context.Context, req *godata.GoDataRequ
return []*libregraph.User{}, err
}
logger.Debug().Str("property", property).Str("value", value).Msg("applying approleAssignments filter on result set")
return g.filterUsersByAppRoleID(ctx, value, res2)
return g.filterUsersByAppRoleID(ctx, req, value, res2)
}
// 1st part is no appRoleAssignmentFilter, run the filter query
@@ -172,7 +172,7 @@ func (g Graph) applyFilterLogicalAnd(ctx context.Context, req *godata.GoDataRequ
return users, unsupportedFilterError()
}
logger.Debug().Str("property", property).Str("value", value).Msg("applying approleAssignments filter on result set")
return g.filterUsersByAppRoleID(ctx, value, res1)
return g.filterUsersByAppRoleID(ctx, req, value, res1)
}
// 2nd part is no appRoleAssignmentFilter either
@@ -329,22 +329,39 @@ func (g Graph) applyAppRoleAssignmentEq(ctx context.Context, req *godata.GoDataR
return users, err
}
return g.filterUsersByAppRoleID(ctx, filterValue, users)
return g.filterUsersByAppRoleID(ctx, req, filterValue, users)
}
return users, unsupportedFilterError()
}
func (g Graph) filterUsersByAppRoleID(ctx context.Context, id string, users []*libregraph.User) ([]*libregraph.User, error) {
func (g Graph) filterUsersByAppRoleID(ctx context.Context, req *godata.GoDataRequest, id string, users []*libregraph.User) ([]*libregraph.User, error) {
// We're using a map for the results here, in order to avoid returning
// a user twice. The settings API, still has an issue that causes it to
// duplicate some assignments on restart:
// https://github.com/owncloud/ocis/issues/3432
var expand bool
if exp := req.Query.GetExpand(); exp != nil {
for _, item := range exp.ExpandItems {
if item.Path[0].Value == "appRoleAssignments" {
expand = true
break
}
}
}
resultUsersMap := make(map[string]*libregraph.User, len(users))
for _, user := range users {
assignments, err := g.fetchAppRoleAssignments(ctx, user.GetId())
if err != nil {
return users, err
}
// To avoid re-fetching all assignments in the upper layer handler when
// the $expand query parameter is set, we're adding the assignments to the
// resulting users here already.
if expand {
user.AppRoleAssignments = assignments
}
for _, assignment := range assignments {
if assignment.GetAppRoleId() == id {
if _, ok := resultUsersMap[user.GetId()]; !ok {