Push role sorting logic to repo layer

api/handlers/role.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"net/http"
 	"net/url"
-	"sort"
 
 	"code.cloudfoundry.org/korifi/api/authorization"
 	apierrors "code.cloudfoundry.org/korifi/api/errors"
@@ -25,7 +24,7 @@ const (
 
 type CFRoleRepository interface {
 	CreateRole(context.Context, authorization.Info, repositories.CreateRoleMessage) (repositories.RoleRecord, error)
-	ListRoles(context.Context, authorization.Info) ([]repositories.RoleRecord, error)
+	ListRoles(context.Context, authorization.Info, repositories.ListRolesMessage) ([]repositories.RoleRecord, error)
 	GetRole(context.Context, authorization.Info, string) (repositories.RoleRecord, error)
 	DeleteRole(context.Context, authorization.Info, repositories.DeleteRoleMessage) error
 }
@@ -68,19 +67,18 @@ func (h *Role) list(r *http.Request) (*routing.Response, error) {
 	authInfo, _ := authorization.InfoFromContext(r.Context())
 	logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.role.list")
 
-	roleListFilter := new(payloads.RoleList)
-	err := h.requestValidator.DecodeAndValidateURLValues(r, roleListFilter)
+	payload := new(payloads.RoleList)
+	err := h.requestValidator.DecodeAndValidateURLValues(r, payload)
 	if err != nil {
 		return nil, apierrors.LogAndReturn(logger, err, "Unable to decode request query parameters")
 	}
 
-	roles, err := h.roleRepo.ListRoles(r.Context(), authInfo)
+	roles, err := h.roleRepo.ListRoles(r.Context(), authInfo, payload.ToMessage())
 	if err != nil {
 		return nil, apierrors.LogAndReturn(logger, err, "failed to list roles")
 	}
 
-	filteredRoles := filterRoles(roleListFilter, roles)
-	h.sortList(filteredRoles, roleListFilter.OrderBy)
+	filteredRoles := filterRoles(payload, roles)
 
 	return routing.NewResponse(http.StatusOK).WithBody(presenter.ForList(presenter.ForRole, filteredRoles, h.apiBaseURL, *r.URL)), nil
 }
@@ -103,20 +101,6 @@ func match(allowedValues map[string]bool, val string) bool {
 	return len(allowedValues) == 0 || allowedValues[val]
 }
 
-func (h *Role) sortList(roles []repositories.RoleRecord, order string) {
-	switch order {
-	case "":
-	case "created_at":
-		sort.Slice(roles, func(i, j int) bool { return timePtrAfter(&roles[j].CreatedAt, &roles[i].CreatedAt) })
-	case "-created_at":
-		sort.Slice(roles, func(i, j int) bool { return timePtrAfter(&roles[i].CreatedAt, &roles[j].CreatedAt) })
-	case "updated_at":
-		sort.Slice(roles, func(i, j int) bool { return timePtrAfter(roles[j].UpdatedAt, roles[i].UpdatedAt) })
-	case "-updated_at":
-		sort.Slice(roles, func(i, j int) bool { return timePtrAfter(roles[i].UpdatedAt, roles[j].UpdatedAt) })
-	}
-}
-
 func (h *Role) delete(r *http.Request) (*routing.Response, error) {
 	authInfo, _ := authorization.InfoFromContext(r.Context())
 	logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.role.delete")

api/handlers/role_test.go

@@ -179,7 +179,7 @@ var _ = Describe("Role", func() {
 			Expect(req.URL.String()).To(HaveSuffix(rolesBase + "?foo=bar"))
 
 			Expect(roleRepo.ListRolesCallCount()).To(Equal(1))
-			_, actualAuthInfo := roleRepo.ListRolesArgsForCall(0)
+			_, actualAuthInfo, _ := roleRepo.ListRolesArgsForCall(0)
 			Expect(actualAuthInfo).To(Equal(authInfo))
 
 			Expect(rr).To(HaveHTTPStatus(http.StatusOK))
@@ -194,7 +194,7 @@ var _ = Describe("Role", func() {
 			)))
 		})
 
-		Describe("filtering and ordering", func() {
+		Describe("filtering", func() {
 			BeforeEach(func() {
 				roleRepo.ListRolesReturns([]repositories.RoleRecord{
 					{GUID: "1", CreatedAt: time.UnixMilli(5000), UpdatedAt: tools.PtrTo(time.UnixMilli(4000)), Type: "a", Space: "space1", Org: "", User: "user1"},
@@ -227,23 +227,6 @@ var _ = Describe("Role", func() {
 				Entry("user_guids1", payloads.RoleList{UserGUIDs: map[string]bool{"user1": true}}, "1", "2", "3"),
 				Entry("user_guids2", payloads.RoleList{UserGUIDs: map[string]bool{"user1": true, "user2": true}}, "1", "2", "3", "4"),
 			)
-
-			DescribeTable("ordering", func(order string, expectedGUIDs ...any) {
-				req, err := http.NewRequestWithContext(ctx, "GET", rolesBase+"?order_by=not-used-in-test", nil)
-				Expect(err).NotTo(HaveOccurred())
-
-				requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(&payloads.RoleList{OrderBy: order})
-				rr = httptest.NewRecorder()
-				routerBuilder.Build().ServeHTTP(rr, req)
-
-				Expect(rr).To(HaveHTTPStatus(http.StatusOK))
-				Expect(rr).To(HaveHTTPBody(MatchJSONPath("$.resources[*].guid", expectedGUIDs)))
-			},
-				Entry("created_at ASC", "created_at", "4", "3", "1", "2"),
-				Entry("created_at DESC", "-created_at", "2", "1", "3", "4"),
-				Entry("updated_at ASC", "updated_at", "2", "1", "4", "3"),
-				Entry("updated_at DESC", "-updated_at", "3", "4", "1", "2"),
-			)
 		})
 
 		When("decoding the url values fails", func() {

api/main.go

@@ -219,6 +219,7 @@ func main() {
 		cfg.RootNamespace,
 		cfg.RoleMappings,
 		namespaceRetriever,
+		repositories.NewRoleSorter(),
 	)
 	imageClient := image.NewClient(privilegedK8sClient)
 	imageRepo := repositories.NewImageRepository(

api/payloads/role.go

@@ -129,6 +129,12 @@ type RoleList struct {
 	OrderBy    string
 }
 
+func (r RoleList) ToMessage() repositories.ListRolesMessage {
+	return repositories.ListRolesMessage{
+		OrderBy: r.OrderBy,
+	}
+}
+
 func (r RoleList) SupportedKeys() []string {
 	return []string{"guids", "types", "space_guids", "organization_guids", "user_guids", "order_by", "include", "per_page", "page"}
 }

api/payloads/role_test.go

@@ -9,6 +9,7 @@ import (
 
 	"code.cloudfoundry.org/korifi/api/errors"
 	"code.cloudfoundry.org/korifi/api/payloads"
+	"code.cloudfoundry.org/korifi/api/repositories"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -206,4 +207,14 @@ var _ = Describe("role list", func() {
 		},
 		Entry("invalid order_by", "order_by=foo", "value must be one of"),
 	)
+
+	DescribeTable("ToMessage",
+		func(roleList payloads.RoleList, expectedListRolesMessage repositories.ListRolesMessage) {
+			actualListRolesMessage := roleList.ToMessage()
+
+			Expect(actualListRolesMessage).To(Equal(expectedListRolesMessage))
+		},
+		Entry("created_at", payloads.RoleList{OrderBy: "created_at"}, repositories.ListRolesMessage{OrderBy: "created_at"}),
+	)
+
 })