Skip to content

Commit

Permalink
Push buildpack sorting logic to repo layer
Browse files Browse the repository at this point in the history
  • Loading branch information
gogolok authored and danail-branekov committed Oct 23, 2024
1 parent 83da842 commit 0498c9a
Show file tree
Hide file tree
Showing 9 changed files with 296 additions and 89 deletions.
30 changes: 4 additions & 26 deletions api/handlers/buildpack.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"net/http"
"net/url"
"sort"

"code.cloudfoundry.org/korifi/api/authorization"
apierrors "code.cloudfoundry.org/korifi/api/errors"
Expand All @@ -22,7 +21,7 @@ const (

//counterfeiter:generate -o fake -fake-name BuildpackRepository . BuildpackRepository
type BuildpackRepository interface {
ListBuildpacks(ctx context.Context, authInfo authorization.Info) ([]repositories.BuildpackRecord, error)
ListBuildpacks(ctx context.Context, authInfo authorization.Info, message repositories.ListBuildpacksMessage) ([]repositories.BuildpackRecord, error)
}

type Buildpack struct {
Expand All @@ -47,40 +46,19 @@ func (h *Buildpack) list(r *http.Request) (*routing.Response, error) {
authInfo, _ := authorization.InfoFromContext(r.Context())
logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.build.list")

buildpackListFilter := new(payloads.BuildpackList)
if err := h.requestValidator.DecodeAndValidateURLValues(r, buildpackListFilter); err != nil {
payload := new(payloads.BuildpackList)
if err := h.requestValidator.DecodeAndValidateURLValues(r, payload); err != nil {
return nil, apierrors.LogAndReturn(logger, err, "Unable to parse request query parameters")
}

buildpacks, err := h.buildpackRepo.ListBuildpacks(r.Context(), authInfo)
buildpacks, err := h.buildpackRepo.ListBuildpacks(r.Context(), authInfo, payload.ToMessage())
if err != nil {
return nil, apierrors.LogAndReturn(logger, err, "Failed to fetch buildpacks from Kubernetes")
}

h.sortList(buildpacks, buildpackListFilter.OrderBy)

return routing.NewResponse(http.StatusOK).WithBody(presenter.ForList(presenter.ForBuildpack, buildpacks, h.serverURL, *r.URL)), nil
}

// nolint:dupl
func (h *Buildpack) sortList(bpList []repositories.BuildpackRecord, order string) {
switch order {
case "":
case "created_at":
sort.Slice(bpList, func(i, j int) bool { return timePtrAfter(&bpList[j].CreatedAt, &bpList[i].CreatedAt) })
case "-created_at":
sort.Slice(bpList, func(i, j int) bool { return timePtrAfter(&bpList[i].CreatedAt, &bpList[j].CreatedAt) })
case "updated_at":
sort.Slice(bpList, func(i, j int) bool { return timePtrAfter(bpList[j].UpdatedAt, bpList[i].UpdatedAt) })
case "-updated_at":
sort.Slice(bpList, func(i, j int) bool { return timePtrAfter(bpList[i].UpdatedAt, bpList[j].UpdatedAt) })
case "position":
sort.Slice(bpList, func(i, j int) bool { return bpList[i].Position < bpList[j].Position })
case "-position":
sort.Slice(bpList, func(i, j int) bool { return bpList[i].Position > bpList[j].Position })
}
}

func (h *Buildpack) UnauthenticatedRoutes() []routing.Route {
return nil
}
Expand Down
43 changes: 1 addition & 42 deletions api/handlers/buildpack_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,10 @@ package handlers_test
import (
"errors"
"net/http"
"net/http/httptest"
"time"

. "code.cloudfoundry.org/korifi/api/handlers"
"code.cloudfoundry.org/korifi/api/handlers/fake"
"code.cloudfoundry.org/korifi/api/payloads"
"code.cloudfoundry.org/korifi/api/repositories"
. "code.cloudfoundry.org/korifi/tests/matchers"
"code.cloudfoundry.org/korifi/tools"
Expand Down Expand Up @@ -62,7 +60,7 @@ var _ = Describe("Buildpack", func() {

It("returns the buildpacks for the default builder", func() {
Expect(buildpackRepo.ListBuildpacksCallCount()).To(Equal(1))
_, actualAuthInfo := buildpackRepo.ListBuildpacksArgsForCall(0)
_, actualAuthInfo, _ := buildpackRepo.ListBuildpacksArgsForCall(0)
Expect(actualAuthInfo).To(Equal(authInfo))

Expect(rr).To(HaveHTTPStatus(http.StatusOK))
Expand All @@ -75,45 +73,6 @@ var _ = Describe("Buildpack", func() {
)))
})

Describe("Order results", func() {
BeforeEach(func() {
buildpackRepo.ListBuildpacksReturns([]repositories.BuildpackRecord{
{
CreatedAt: time.UnixMilli(1000),
UpdatedAt: tools.PtrTo(time.UnixMilli(5000)),
Position: 1,
},
{
CreatedAt: time.UnixMilli(3000),
UpdatedAt: tools.PtrTo(time.UnixMilli(4000)),
Position: 2,
},
{
CreatedAt: time.UnixMilli(2000),
UpdatedAt: tools.PtrTo(time.UnixMilli(6000)),
Position: 3,
},
}, nil)
})

DescribeTable("ordering results", func(orderBy string, expectedOrder ...any) {
req = createHttpRequest("GET", "/v3/buildpacks?order_by=not-used", nil)
requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(&payloads.BuildpackList{
OrderBy: orderBy,
})
rr = httptest.NewRecorder()
routerBuilder.Build().ServeHTTP(rr, req)
Expect(rr).To(HaveHTTPBody(MatchJSONPath("$.resources[*].position", expectedOrder)))
},
Entry("created_at ASC", "created_at", 1.0, 3.0, 2.0),
Entry("created_at DESC", "-created_at", 2.0, 3.0, 1.0),
Entry("updated_at ASC", "updated_at", 2.0, 1.0, 3.0),
Entry("updated_at DESC", "-updated_at", 3.0, 1.0, 2.0),
Entry("position ASC", "position", 1.0, 2.0, 3.0),
Entry("position DESC", "-position", 3.0, 2.0, 1.0),
)
})

When("request is invalid", func() {
BeforeEach(func() {
requestValidator.DecodeAndValidateURLValuesReturns(errors.New("foo"))
Expand Down
18 changes: 10 additions & 8 deletions api/handlers/fake/buildpack_repository.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions api/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ func main() {
buildpackRepo := repositories.NewBuildpackRepository(cfg.BuilderName,
userClientFactory,
cfg.RootNamespace,
repositories.NewBuildpackSorter(),
)
roleRepo := repositories.NewRoleRepo(
userClientFactory,
Expand Down
7 changes: 7 additions & 0 deletions api/payloads/buildpack.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,20 @@ import (
"net/url"

"code.cloudfoundry.org/korifi/api/payloads/validation"
"code.cloudfoundry.org/korifi/api/repositories"
jellidation "github.com/jellydator/validation"
)

type BuildpackList struct {
OrderBy string
}

func (b BuildpackList) ToMessage() repositories.ListBuildpacksMessage {
return repositories.ListBuildpacksMessage{
OrderBy: b.OrderBy,
}
}

func (d BuildpackList) SupportedKeys() []string {
return []string{"order_by", "per_page", "page"}
}
Expand Down
12 changes: 11 additions & 1 deletion api/payloads/buildpack_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@ import (
. "github.com/onsi/gomega"

"code.cloudfoundry.org/korifi/api/payloads"
"code.cloudfoundry.org/korifi/api/repositories"
)

var _ = Describe("Buildpack", func() {
var _ = Describe("BuildpackList", func() {
Describe("Validation", func() {
DescribeTable("valid query",
func(query string, expectedBuildpackList payloads.BuildpackList) {
Expand All @@ -34,4 +35,13 @@ var _ = Describe("Buildpack", func() {
Entry("invalid order_by", "order_by=foo", "value must be one of"),
)
})

DescribeTable("ToMessage",
func(buildpackList payloads.BuildpackList, expectedListBuildpacksMessage repositories.ListBuildpacksMessage) {
actualListBuildpacksMessage := buildpackList.ToMessage()

Expect(actualListBuildpacksMessage).To(Equal(expectedListBuildpacksMessage))
},
Entry("created_at", payloads.BuildpackList{OrderBy: "created_at"}, repositories.ListBuildpacksMessage{OrderBy: "created_at"}),
)
})
58 changes: 55 additions & 3 deletions api/repositories/buildpack_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import (

"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/tools"
"github.com/BooleanCat/go-functional/v2/it"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
Expand All @@ -23,6 +25,7 @@ type BuildpackRepository struct {
builderName string
userClientFactory authorization.UserK8sClientFactory
rootNamespace string
sorter BuildpackSorter
}

type BuildpackRecord struct {
Expand All @@ -34,15 +37,64 @@ type BuildpackRecord struct {
UpdatedAt *time.Time
}

func NewBuildpackRepository(builderName string, userClientFactory authorization.UserK8sClientFactory, rootNamespace string) *BuildpackRepository {
//counterfeiter:generate -o fake -fake-name BuildpackSorter . BuildpackSorter
type BuildpackSorter interface {
Sort(records []BuildpackRecord, order string) []BuildpackRecord
}

type buildpackSorter struct {
sorter *compare.Sorter[BuildpackRecord]
}

func NewBuildpackSorter() *buildpackSorter {
return &buildpackSorter{
sorter: compare.NewSorter(BuildpackComparator),
}
}

func (s *buildpackSorter) Sort(records []BuildpackRecord, order string) []BuildpackRecord {
return s.sorter.Sort(records, order)
}

func BuildpackComparator(fieldName string) func(BuildpackRecord, BuildpackRecord) int {
return func(b1, b2 BuildpackRecord) int {
switch fieldName {
case "created_at":
return tools.CompareTimePtr(&b1.CreatedAt, &b2.CreatedAt)
case "-created_at":
return tools.CompareTimePtr(&b2.CreatedAt, &b1.CreatedAt)
case "updated_at":
return tools.CompareTimePtr(b1.UpdatedAt, b2.UpdatedAt)
case "-updated_at":
return tools.CompareTimePtr(b2.UpdatedAt, b1.UpdatedAt)
case "position":
return b1.Position - b2.Position
case "-position":
return b2.Position - b1.Position
}
return 0
}
}

type ListBuildpacksMessage struct {
OrderBy string
}

func NewBuildpackRepository(
builderName string,
userClientFactory authorization.UserK8sClientFactory,
rootNamespace string,
sorter BuildpackSorter,
) *BuildpackRepository {
return &BuildpackRepository{
builderName: builderName,
userClientFactory: userClientFactory,
rootNamespace: rootNamespace,
sorter: sorter,
}
}

func (r *BuildpackRepository) ListBuildpacks(ctx context.Context, authInfo authorization.Info) ([]BuildpackRecord, error) {
func (r *BuildpackRepository) ListBuildpacks(ctx context.Context, authInfo authorization.Info, message ListBuildpacksMessage) ([]BuildpackRecord, error) {
var builderInfo korifiv1alpha1.BuilderInfo

userClient, err := r.userClientFactory.BuildClient(authInfo)
Expand Down Expand Up @@ -81,7 +133,7 @@ func (r *BuildpackRepository) ListBuildpacks(ctx context.Context, authInfo autho
return nil, apierrors.NewResourceNotReadyError(fmt.Errorf("BuilderInfo %q not ready: %s", r.builderName, conditionNotReadyMessage))
}

return builderInfoToBuildpackRecords(builderInfo), nil
return r.sorter.Sort(builderInfoToBuildpackRecords(builderInfo), message.OrderBy), nil
}

func builderInfoToBuildpackRecords(info korifiv1alpha1.BuilderInfo) []BuildpackRecord {
Expand Down
Loading

0 comments on commit 0498c9a

Please sign in to comment.