From 4717248959bbb9bc7e45acd79d95011a4cdf62df Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Thu, 5 Jan 2023 17:12:51 +0100 Subject: [PATCH] Simplify sort code a bit Switch to sort.Slice() instead of sort.Sort(), which requires less boilerplate. --- services/graph/pkg/service/v0/drives.go | 16 +- .../graph/pkg/service/v0/educationschools.go | 15 +- .../graph/pkg/service/v0/educationuser.go | 22 ++- services/graph/pkg/service/v0/groups.go | 13 +- services/graph/pkg/service/v0/ordering.go | 151 ++---------------- services/graph/pkg/service/v0/users.go | 20 ++- 6 files changed, 71 insertions(+), 166 deletions(-) diff --git a/services/graph/pkg/service/v0/drives.go b/services/graph/pkg/service/v0/drives.go index d286531dee..15379c68d1 100644 --- a/services/graph/pkg/service/v0/drives.go +++ b/services/graph/pkg/service/v0/drives.go @@ -927,23 +927,29 @@ func (g Graph) DeleteDrive(w http.ResponseWriter, r *http.Request) { } func sortSpaces(req *godata.GoDataRequest, spaces []*libregraph.Drive) ([]*libregraph.Drive, error) { - var sorter sort.Interface if req.Query.OrderBy == nil || len(req.Query.OrderBy.OrderByItems) != 1 { return spaces, nil } + var less func(i, j int) bool + switch req.Query.OrderBy.OrderByItems[0].Field.Value { case "name": - sorter = spacesByName{spaces} + less = func(i, j int) bool { + return strings.ToLower(spaces[i].GetName()) < strings.ToLower(spaces[j].GetName()) + } case "lastModifiedDateTime": - sorter = spacesByLastModifiedDateTime{spaces} + less = func(i, j int) bool { + return lessSpacesByLastModifiedDateTime(spaces[i], spaces[j]) + } default: return nil, errors.Errorf("we do not support <%s> as a order parameter", req.Query.OrderBy.OrderByItems[0].Field.Value) } if req.Query.OrderBy.OrderByItems[0].Order == _sortDescending { - sorter = sort.Reverse(sorter) + sort.Slice(spaces, reverse(less)) + } else { + sort.Slice(spaces, less) } - sort.Sort(sorter) return spaces, nil } diff --git a/services/graph/pkg/service/v0/educationschools.go b/services/graph/pkg/service/v0/educationschools.go index b3e9fcb20e..c6b8b8c206 100644 --- a/services/graph/pkg/service/v0/educationschools.go +++ b/services/graph/pkg/service/v0/educationschools.go @@ -417,20 +417,25 @@ func (g Graph) DeleteEducationSchoolUser(w http.ResponseWriter, r *http.Request) } func sortEducationSchools(req *godata.GoDataRequest, schools []*libregraph.EducationSchool) ([]*libregraph.EducationSchool, error) { - var sorter sort.Interface if req.Query.OrderBy == nil || len(req.Query.OrderBy.OrderByItems) != 1 { return schools, nil } + var less func(i, j int) bool + switch req.Query.OrderBy.OrderByItems[0].Field.Value { case displayNameAttr: - sorter = schoolsByDisplayName{schools} + less = func(i, j int) bool { + return strings.ToLower(schools[i].GetDisplayName()) < strings.ToLower(schools[j].GetDisplayName()) + } default: return nil, fmt.Errorf("we do not support <%s> as a order parameter", req.Query.OrderBy.OrderByItems[0].Field.Value) } - if req.Query.OrderBy.OrderByItems[0].Order == "desc" { - sorter = sort.Reverse(sorter) + if req.Query.OrderBy.OrderByItems[0].Order == _sortDescending { + sort.Slice(schools, reverse(less)) + } else { + sort.Slice(schools, less) } - sort.Sort(sorter) + return schools, nil } diff --git a/services/graph/pkg/service/v0/educationuser.go b/services/graph/pkg/service/v0/educationuser.go index 41922ed0af..611a3d3893 100644 --- a/services/graph/pkg/service/v0/educationuser.go +++ b/services/graph/pkg/service/v0/educationuser.go @@ -458,24 +458,32 @@ func (g Graph) PatchEducationUser(w http.ResponseWriter, r *http.Request) { } func sortEducationUsers(req *godata.GoDataRequest, users []*libregraph.EducationUser) ([]*libregraph.EducationUser, error) { - var sorter sort.Interface if req.Query.OrderBy == nil || len(req.Query.OrderBy.OrderByItems) != 1 { return users, nil } + var less func(i, j int) bool + switch req.Query.OrderBy.OrderByItems[0].Field.Value { case displayNameAttr: - sorter = educationUsersByDisplayName{users} + less = func(i, j int) bool { + return strings.ToLower(users[i].GetDisplayName()) < strings.ToLower(users[j].GetDisplayName()) + } case "mail": - sorter = educationUsersByMail{users} + less = func(i, j int) bool { + return strings.ToLower(users[i].GetMail()) < strings.ToLower(users[j].GetMail()) + } case "onPremisesSamAccountName": - sorter = educationUsersByOnPremisesSamAccountName{users} + less = func(i, j int) bool { + return strings.ToLower(users[i].GetOnPremisesSamAccountName()) < strings.ToLower(users[j].GetOnPremisesSamAccountName()) + } default: return nil, fmt.Errorf("we do not support <%s> as a order parameter", req.Query.OrderBy.OrderByItems[0].Field.Value) } - if req.Query.OrderBy.OrderByItems[0].Order == "desc" { - sorter = sort.Reverse(sorter) + if req.Query.OrderBy.OrderByItems[0].Order == _sortDescending { + sort.Slice(users, reverse(less)) + } else { + sort.Slice(users, less) } - sort.Sort(sorter) return users, nil } diff --git a/services/graph/pkg/service/v0/groups.go b/services/graph/pkg/service/v0/groups.go index 760105f1de..2df0cc3966 100644 --- a/services/graph/pkg/service/v0/groups.go +++ b/services/graph/pkg/service/v0/groups.go @@ -404,20 +404,25 @@ func (g Graph) DeleteGroupMember(w http.ResponseWriter, r *http.Request) { } func sortGroups(req *godata.GoDataRequest, groups []*libregraph.Group) ([]*libregraph.Group, error) { - var sorter sort.Interface if req.Query.OrderBy == nil || len(req.Query.OrderBy.OrderByItems) != 1 { return groups, nil } + var less func(i, j int) bool + switch req.Query.OrderBy.OrderByItems[0].Field.Value { case displayNameAttr: - sorter = groupsByDisplayName{groups} + less = func(i, j int) bool { + return strings.ToLower(groups[i].GetDisplayName()) < strings.ToLower(groups[j].GetDisplayName()) + } default: return nil, fmt.Errorf("we do not support <%s> as a order parameter", req.Query.OrderBy.OrderByItems[0].Field.Value) } if req.Query.OrderBy.OrderByItems[0].Order == _sortDescending { - sorter = sort.Reverse(sorter) + sort.Slice(groups, reverse(less)) + } else { + sort.Slice(groups, less) } - sort.Sort(sorter) + return groups, nil } diff --git a/services/graph/pkg/service/v0/ordering.go b/services/graph/pkg/service/v0/ordering.go index b636e4dacf..4160fa87fc 100644 --- a/services/graph/pkg/service/v0/ordering.go +++ b/services/graph/pkg/service/v0/ordering.go @@ -6,154 +6,27 @@ import ( libregraph "github.com/owncloud/libre-graph-api-go" ) -type spacesSlice []*libregraph.Drive - -// Len is the number of elements in the collection. -func (d spacesSlice) Len() int { return len(d) } - -// Swap swaps the elements with indexes i and j. -func (d spacesSlice) Swap(i, j int) { d[i], d[j] = d[j], d[i] } - -type spacesByName struct { - spacesSlice -} -type spacesByLastModifiedDateTime struct { - spacesSlice -} - -// Less reports whether the element with index i -// must sort before the element with index j. -func (s spacesByName) Less(i, j int) bool { - return strings.ToLower(s.spacesSlice[i].Name) < strings.ToLower(s.spacesSlice[j].Name) -} - -// Less reports whether the element with index i -// must sort before the element with index j. -func (s spacesByLastModifiedDateTime) Less(i, j int) bool { +// lessSpacesByLastModifiedDateTime reports whether the element i +// must sort before the element j. +func lessSpacesByLastModifiedDateTime(i, j *libregraph.Drive) bool { // compare the items when both dates are set - if s.spacesSlice[i].LastModifiedDateTime != nil && s.spacesSlice[j].LastModifiedDateTime != nil { - return s.spacesSlice[i].LastModifiedDateTime.Before(*s.spacesSlice[j].LastModifiedDateTime) + if i.LastModifiedDateTime != nil && j.LastModifiedDateTime != nil { + return i.LastModifiedDateTime.Before(*j.LastModifiedDateTime) } // an item without a timestamp is considered "less than" an item with a timestamp - if s.spacesSlice[i].LastModifiedDateTime == nil && s.spacesSlice[j].LastModifiedDateTime != nil { + if i.LastModifiedDateTime == nil && j.LastModifiedDateTime != nil { return true } // an item without a timestamp is considered "less than" an item with a timestamp - if s.spacesSlice[i].LastModifiedDateTime != nil && s.spacesSlice[j].LastModifiedDateTime == nil { + if i.LastModifiedDateTime != nil && j.LastModifiedDateTime == nil { return false } // fallback to name if no dateTime is set on both items - return strings.ToLower(s.spacesSlice[i].Name) < strings.ToLower(s.spacesSlice[j].Name) + return strings.ToLower(i.Name) < strings.ToLower(j.Name) } -type userSlice []*libregraph.User - -// Len is the number of elements in the collection. -func (d userSlice) Len() int { return len(d) } - -// Swap swaps the elements with indexes i and j. -func (d userSlice) Swap(i, j int) { d[i], d[j] = d[j], d[i] } - -type usersByDisplayName struct { - userSlice -} - -type usersByMail struct { - userSlice -} - -type usersByOnPremisesSamAccountName struct { - userSlice -} - -// Less reports whether the element with index i -// must sort before the element with index j. -func (u usersByDisplayName) Less(i, j int) bool { - return strings.ToLower(u.userSlice[i].GetDisplayName()) < strings.ToLower(u.userSlice[j].GetDisplayName()) -} - -// Less reports whether the element with index i -// must sort before the element with index j. -func (u usersByMail) Less(i, j int) bool { - return strings.ToLower(u.userSlice[i].GetMail()) < strings.ToLower(u.userSlice[j].GetMail()) -} - -// Less reports whether the element with index i -// must sort before the element with index j. -func (u usersByOnPremisesSamAccountName) Less(i, j int) bool { - return strings.ToLower(u.userSlice[i].GetOnPremisesSamAccountName()) < strings.ToLower(u.userSlice[j].GetOnPremisesSamAccountName()) -} - -type groupSlice []*libregraph.Group - -// Len is the number of elements in the collection. -func (d groupSlice) Len() int { return len(d) } - -// Swap swaps the elements with indexes i and j. -func (d groupSlice) Swap(i, j int) { d[i], d[j] = d[j], d[i] } - -type groupsByDisplayName struct { - groupSlice -} - -// Less reports whether the element with index i -// must sort before the element with index j. -func (g groupsByDisplayName) Less(i, j int) bool { - return strings.ToLower(g.groupSlice[i].GetDisplayName()) < strings.ToLower(g.groupSlice[j].GetDisplayName()) -} - -type schoolSlice []*libregraph.EducationSchool - -// Len is the number of elements in the collection. -func (d schoolSlice) Len() int { return len(d) } - -// Swap swaps the elements with indexes i and j. -func (d schoolSlice) Swap(i, j int) { d[i], d[j] = d[j], d[i] } - -type schoolsByDisplayName struct { - schoolSlice -} - -// Less reports whether the element with index i -// must sort before the element with index j. -func (g schoolsByDisplayName) Less(i, j int) bool { - return strings.ToLower(g.schoolSlice[i].GetDisplayName()) < strings.ToLower(g.schoolSlice[j].GetDisplayName()) -} - -type educationUserSlice []*libregraph.EducationUser - -// Len is the number of elements in the collection. -func (d educationUserSlice) Len() int { return len(d) } - -// Swap swaps the elements with indexes i and j. -func (d educationUserSlice) Swap(i, j int) { d[i], d[j] = d[j], d[i] } - -type educationUsersByDisplayName struct { - educationUserSlice -} - -type educationUsersByMail struct { - educationUserSlice -} - -type educationUsersByOnPremisesSamAccountName struct { - educationUserSlice -} - -// Less reports whether the element with index i -// must sort before the element with index j. -func (u educationUsersByDisplayName) Less(i, j int) bool { - return strings.ToLower(u.educationUserSlice[i].GetDisplayName()) < strings.ToLower(u.educationUserSlice[j].GetDisplayName()) -} - -// Less reports whether the element with index i -// must sort before the element with index j. -func (u educationUsersByMail) Less(i, j int) bool { - return strings.ToLower(u.educationUserSlice[i].GetMail()) < strings.ToLower(u.educationUserSlice[j].GetMail()) -} - -// Less reports whether the element with index i -// must sort before the element with index j. -func (u educationUsersByOnPremisesSamAccountName) Less(i, j int) bool { - return strings.ToLower(u.educationUserSlice[i].GetOnPremisesSamAccountName()) < strings.ToLower(u.educationUserSlice[j].GetOnPremisesSamAccountName()) +func reverse(less func(i, j int) bool) func(i, j int) bool { + return func(i, j int) bool { + return !less(i, j) + } } diff --git a/services/graph/pkg/service/v0/users.go b/services/graph/pkg/service/v0/users.go index 56238e719f..31c390c87e 100644 --- a/services/graph/pkg/service/v0/users.go +++ b/services/graph/pkg/service/v0/users.go @@ -486,24 +486,32 @@ func isValidEmail(e string) bool { } func sortUsers(req *godata.GoDataRequest, users []*libregraph.User) ([]*libregraph.User, error) { - var sorter sort.Interface if req.Query.OrderBy == nil || len(req.Query.OrderBy.OrderByItems) != 1 { return users, nil } + var less func(i, j int) bool + switch req.Query.OrderBy.OrderByItems[0].Field.Value { case displayNameAttr: - sorter = usersByDisplayName{users} + less = func(i, j int) bool { + return strings.ToLower(users[i].GetDisplayName()) < strings.ToLower(users[j].GetDisplayName()) + } case "mail": - sorter = usersByMail{users} + less = func(i, j int) bool { + return strings.ToLower(users[i].GetMail()) < strings.ToLower(users[j].GetMail()) + } case "onPremisesSamAccountName": - sorter = usersByOnPremisesSamAccountName{users} + less = func(i, j int) bool { + return strings.ToLower(users[i].GetOnPremisesSamAccountName()) < strings.ToLower(users[j].GetOnPremisesSamAccountName()) + } default: return nil, fmt.Errorf("we do not support <%s> as a order parameter", req.Query.OrderBy.OrderByItems[0].Field.Value) } if req.Query.OrderBy.OrderByItems[0].Order == _sortDescending { - sorter = sort.Reverse(sorter) + sort.Slice(users, reverse(less)) + } else { + sort.Slice(users, less) } - sort.Sort(sorter) return users, nil }