Push service instance sorting logic to repo layer

api/handlers/app.go

@@ -7,7 +7,6 @@ import (
 	"net/http"
 	"net/url"
 	"strconv"
-	"time"
 
 	"code.cloudfoundry.org/korifi/api/authorization"
 	apierrors "code.cloudfoundry.org/korifi/api/errors"
@@ -173,14 +172,6 @@ func (h *App) list(r *http.Request) (*routing.Response, error) { //nolint:dupl
 	return routing.NewResponse(http.StatusOK).WithBody(presenter.ForList(presenter.ForApp, appList, h.serverURL, *r.URL)), nil
 }
 
-func timePtrAfter(t1, t2 *time.Time) bool {
-	if t1 == nil || t2 == nil {
-		return false
-	}
-
-	return (*t1).After(*t2)
-}
-
 func (h *App) setCurrentDroplet(r *http.Request) (*routing.Response, error) {
 	authInfo, _ := authorization.InfoFromContext(r.Context())
 	logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.app.set-current-droplet")

api/handlers/service_instance.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"net/http"
 	"net/url"
-	"sort"
 
 	apierrors "code.cloudfoundry.org/korifi/api/errors"
 	"code.cloudfoundry.org/korifi/api/handlers/include"
@@ -160,8 +159,6 @@ func (h *ServiceInstance) list(r *http.Request) (*routing.Response, error) {
 		return nil, apierrors.LogAndReturn(logger, err, "Failed to list service instance")
 	}
 
-	h.sortList(serviceInstances, payload.OrderBy)
-
 	includedResources, err := h.includeResolver.ResolveIncludes(r.Context(), authInfo, serviceInstances, payload.IncludeResourceRules)
 	if err != nil {
 		return nil, apierrors.LogAndReturn(logger, err, "failed to build included resources")
@@ -170,25 +167,6 @@ func (h *ServiceInstance) list(r *http.Request) (*routing.Response, error) {
 	return routing.NewResponse(http.StatusOK).WithBody(presenter.ForList(presenter.ForServiceInstance, serviceInstances, h.serverURL, *r.URL, includedResources...)), nil
 }
 
-// nolint:dupl
-func (h *ServiceInstance) sortList(siList []repositories.ServiceInstanceRecord, order string) {
-	switch order {
-	case "":
-	case "created_at":
-		sort.Slice(siList, func(i, j int) bool { return timePtrAfter(&siList[j].CreatedAt, &siList[i].CreatedAt) })
-	case "-created_at":
-		sort.Slice(siList, func(i, j int) bool { return timePtrAfter(&siList[i].CreatedAt, &siList[j].CreatedAt) })
-	case "updated_at":
-		sort.Slice(siList, func(i, j int) bool { return timePtrAfter(siList[j].UpdatedAt, siList[i].UpdatedAt) })
-	case "-updated_at":
-		sort.Slice(siList, func(i, j int) bool { return timePtrAfter(siList[i].UpdatedAt, siList[j].UpdatedAt) })
-	case "name":
-		sort.Slice(siList, func(i, j int) bool { return siList[i].Name < siList[j].Name })
-	case "-name":
-		sort.Slice(siList, func(i, j int) bool { return siList[i].Name > siList[j].Name })
-	}
-}
-
 func (h *ServiceInstance) delete(r *http.Request) (*routing.Response, error) {
 	authInfo, _ := authorization.InfoFromContext(r.Context())
 	logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.service-instance.delete")

api/handlers/service_instance_test.go

@@ -3,9 +3,7 @@ package handlers_test
 import (
 	"errors"
 	"net/http"
-	"net/http/httptest"
 	"strings"
-	"time"
 
 	apierrors "code.cloudfoundry.org/korifi/api/errors"
 	. "code.cloudfoundry.org/korifi/api/handlers"
@@ -388,46 +386,6 @@ var _ = Describe("ServiceInstance", func() {
 			})
 		})
 
-		Describe("Order results", func() {
-			BeforeEach(func() {
-				serviceInstanceRepo.ListServiceInstancesReturns([]repositories.ServiceInstanceRecord{
-					{
-						GUID:      "1",
-						Name:      "first-test-si",
-						CreatedAt: time.UnixMilli(3000),
-						UpdatedAt: tools.PtrTo(time.UnixMilli(4000)),
-					},
-					{
-						GUID:      "2",
-						Name:      "second-test-si",
-						CreatedAt: time.UnixMilli(2000),
-						UpdatedAt: tools.PtrTo(time.UnixMilli(5000)),
-					},
-					{
-						GUID:      "3",
-						Name:      "third-test-si",
-						CreatedAt: time.UnixMilli(1000),
-						UpdatedAt: tools.PtrTo(time.UnixMilli(6000)),
-					},
-				}, nil)
-			})
-
-			DescribeTable("ordering results", func(orderBy string, expectedOrder ...any) {
-				requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(&payloads.ServiceInstanceList{OrderBy: orderBy})
-				req := createHttpRequest("GET", "/v3/service_instances?order_by=not-used", nil)
-				rr = httptest.NewRecorder()
-				routerBuilder.Build().ServeHTTP(rr, req)
-				Expect(rr).To(HaveHTTPBody(MatchJSONPath("$.resources[*].guid", expectedOrder)))
-			},
-				Entry("created_at ASC", "created_at", "3", "2", "1"),
-				Entry("created_at DESC", "-created_at", "1", "2", "3"),
-				Entry("updated_at ASC", "updated_at", "1", "2", "3"),
-				Entry("updated_at DESC", "-updated_at", "3", "2", "1"),
-				Entry("name ASC", "name", "1", "2", "3"),
-				Entry("name DESC", "-name", "3", "2", "1"),
-			)
-		})
-
 		When("there is an error fetching service instances", func() {
 			BeforeEach(func() {
 				serviceInstanceRepo.ListServiceInstancesReturns([]repositories.ServiceInstanceRecord{}, errors.New("unknown!"))

api/main.go

@@ -200,6 +200,7 @@ func main() {
 		userClientFactory,
 		nsPermissions,
 		conditions.NewConditionAwaiter[*korifiv1alpha1.CFServiceInstance, korifiv1alpha1.CFServiceInstance, korifiv1alpha1.CFServiceInstanceList](conditionTimeout),
+		repositories.NewServiceInstanceSorter(),
 	)
 	serviceBindingRepo := repositories.NewServiceBindingRepo(
 		namespaceRetriever,

api/payloads/service_instance.go

@@ -210,6 +210,7 @@ func (l *ServiceInstanceList) ToMessage() repositories.ListServiceInstanceMessag
 		Names:         parse.ArrayParam(l.Names),
 		SpaceGUIDs:    parse.ArrayParam(l.SpaceGUIDs),
 		GUIDs:         parse.ArrayParam(l.GUIDs),
+		OrderBy:       l.OrderBy,
 		LabelSelector: l.LabelSelector,
 	}
 }

api/payloads/service_instance_test.go

@@ -89,6 +89,7 @@ var _ = Describe("ServiceInstanceList", func() {
 				Names:         []string{"n1", "n2"},
 				SpaceGUIDs:    []string{"sg1", "sg2"},
 				GUIDs:         []string{"g1", "g2"},
+				OrderBy:       "order",
 				LabelSelector: "foo=bar",
 			}))
 		})

api/repositories/service_instance_repository.go

@@ -6,10 +6,12 @@ import (
 	"errors"
 	"fmt"
 	"slices"
+	"strings"
 	"time"
 
 	"code.cloudfoundry.org/korifi/api/authorization"
 	apierrors "code.cloudfoundry.org/korifi/api/errors"
+	"code.cloudfoundry.org/korifi/api/repositories/compare"
 	korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
 	"code.cloudfoundry.org/korifi/model"
 	"code.cloudfoundry.org/korifi/tools"
@@ -43,19 +45,61 @@ type ServiceInstanceRepo struct {
 	userClientFactory    authorization.UserK8sClientFactory
 	namespacePermissions *authorization.NamespacePermissions
 	awaiter              Awaiter[*korifiv1alpha1.CFServiceInstance]
+	sorter               ServiceInstanceSorter
+}
+
+//counterfeiter:generate -o fake -fake-name ServiceInstanceSorter . ServiceInstanceSorter
+type ServiceInstanceSorter interface {
+	Sort(records []ServiceInstanceRecord, order string) []ServiceInstanceRecord
+}
+
+type serviceInstanceSorter struct {
+	sorter *compare.Sorter[ServiceInstanceRecord]
+}
+
+func NewServiceInstanceSorter() *serviceInstanceSorter {
+	return &serviceInstanceSorter{
+		sorter: compare.NewSorter(ServiceInstanceComparator),
+	}
+}
+
+func (s *serviceInstanceSorter) Sort(records []ServiceInstanceRecord, order string) []ServiceInstanceRecord {
+	return s.sorter.Sort(records, order)
+}
+
+func ServiceInstanceComparator(fieldName string) func(ServiceInstanceRecord, ServiceInstanceRecord) int {
+	return func(s1, s2 ServiceInstanceRecord) int {
+		switch fieldName {
+		case "created_at":
+			return tools.CompareTimePtr(&s1.CreatedAt, &s2.CreatedAt)
+		case "-created_at":
+			return tools.CompareTimePtr(&s2.CreatedAt, &s1.CreatedAt)
+		case "updated_at":
+			return tools.CompareTimePtr(s1.UpdatedAt, s2.UpdatedAt)
+		case "-updated_at":
+			return tools.CompareTimePtr(s2.UpdatedAt, s1.UpdatedAt)
+		case "name":
+			return strings.Compare(s1.Name, s2.Name)
+		case "-name":
+			return strings.Compare(s2.Name, s1.Name)
+		}
+		return 0
+	}
 }
 
 func NewServiceInstanceRepo(
 	namespaceRetriever NamespaceRetriever,
 	userClientFactory authorization.UserK8sClientFactory,
 	namespacePermissions *authorization.NamespacePermissions,
 	awaiter Awaiter[*korifiv1alpha1.CFServiceInstance],
+	sorter ServiceInstanceSorter,
 ) *ServiceInstanceRepo {
 	return &ServiceInstanceRepo{
 		namespaceRetriever:   namespaceRetriever,
 		userClientFactory:    userClientFactory,
 		namespacePermissions: namespacePermissions,
 		awaiter:              awaiter,
+		sorter:               sorter,
 	}
 }
 
@@ -102,6 +146,7 @@ type ListServiceInstanceMessage struct {
 	SpaceGUIDs    []string
 	GUIDs         []string
 	LabelSelector string
+	OrderBy       string
 }
 
 func (m *ListServiceInstanceMessage) matches(serviceInstance korifiv1alpha1.CFServiceInstance) bool {
@@ -347,7 +392,7 @@ func (r *ServiceInstanceRepo) ListServiceInstances(ctx context.Context, authInfo
 	}
 
 	filteredServiceInstances := itx.FromSlice(serviceInstances).Filter(message.matches)
-	return slices.Collect(it.Map(filteredServiceInstances, cfServiceInstanceToRecord)), nil
+	return r.sorter.Sort(slices.Collect(it.Map(filteredServiceInstances, cfServiceInstanceToRecord)), message.OrderBy), nil
 }
 
 func (r *ServiceInstanceRepo) GetServiceInstance(ctx context.Context, authInfo authorization.Info, guid string) (ServiceInstanceRecord, error) {