diff --git a/api/handlers/fake/cfservice_plan_repository.go b/api/handlers/fake/cfservice_plan_repository.go index c73e5d1cb..9e94d6d44 100644 --- a/api/handlers/fake/cfservice_plan_repository.go +++ b/api/handlers/fake/cfservice_plan_repository.go @@ -11,11 +11,12 @@ import ( ) type CFServicePlanRepository struct { - ListPlansStub func(context.Context, authorization.Info) ([]repositories.ServicePlanRecord, error) + ListPlansStub func(context.Context, authorization.Info, repositories.ListServicePlanMessage) ([]repositories.ServicePlanRecord, error) listPlansMutex sync.RWMutex listPlansArgsForCall []struct { arg1 context.Context arg2 authorization.Info + arg3 repositories.ListServicePlanMessage } listPlansReturns struct { result1 []repositories.ServicePlanRecord @@ -29,19 +30,20 @@ type CFServicePlanRepository struct { invocationsMutex sync.RWMutex } -func (fake *CFServicePlanRepository) ListPlans(arg1 context.Context, arg2 authorization.Info) ([]repositories.ServicePlanRecord, error) { +func (fake *CFServicePlanRepository) ListPlans(arg1 context.Context, arg2 authorization.Info, arg3 repositories.ListServicePlanMessage) ([]repositories.ServicePlanRecord, error) { fake.listPlansMutex.Lock() ret, specificReturn := fake.listPlansReturnsOnCall[len(fake.listPlansArgsForCall)] fake.listPlansArgsForCall = append(fake.listPlansArgsForCall, struct { arg1 context.Context arg2 authorization.Info - }{arg1, arg2}) + arg3 repositories.ListServicePlanMessage + }{arg1, arg2, arg3}) stub := fake.ListPlansStub fakeReturns := fake.listPlansReturns - fake.recordInvocation("ListPlans", []interface{}{arg1, arg2}) + fake.recordInvocation("ListPlans", []interface{}{arg1, arg2, arg3}) fake.listPlansMutex.Unlock() if stub != nil { - return stub(arg1, arg2) + return stub(arg1, arg2, arg3) } if specificReturn { return ret.result1, ret.result2 @@ -55,17 +57,17 @@ func (fake *CFServicePlanRepository) ListPlansCallCount() int { return len(fake.listPlansArgsForCall) } -func (fake *CFServicePlanRepository) ListPlansCalls(stub func(context.Context, authorization.Info) ([]repositories.ServicePlanRecord, error)) { +func (fake *CFServicePlanRepository) ListPlansCalls(stub func(context.Context, authorization.Info, repositories.ListServicePlanMessage) ([]repositories.ServicePlanRecord, error)) { fake.listPlansMutex.Lock() defer fake.listPlansMutex.Unlock() fake.ListPlansStub = stub } -func (fake *CFServicePlanRepository) ListPlansArgsForCall(i int) (context.Context, authorization.Info) { +func (fake *CFServicePlanRepository) ListPlansArgsForCall(i int) (context.Context, authorization.Info, repositories.ListServicePlanMessage) { fake.listPlansMutex.RLock() defer fake.listPlansMutex.RUnlock() argsForCall := fake.listPlansArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2 + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 } func (fake *CFServicePlanRepository) ListPlansReturns(result1 []repositories.ServicePlanRecord, result2 error) { diff --git a/api/handlers/service_plan.go b/api/handlers/service_plan.go index df017af92..297b9b62c 100644 --- a/api/handlers/service_plan.go +++ b/api/handlers/service_plan.go @@ -8,6 +8,7 @@ import ( "code.cloudfoundry.org/korifi/api/authorization" apierrors "code.cloudfoundry.org/korifi/api/errors" + "code.cloudfoundry.org/korifi/api/payloads" "code.cloudfoundry.org/korifi/api/presenter" "code.cloudfoundry.org/korifi/api/repositories" "code.cloudfoundry.org/korifi/api/routing" @@ -20,21 +21,24 @@ const ( //counterfeiter:generate -o fake -fake-name CFServicePlanRepository . CFServicePlanRepository type CFServicePlanRepository interface { - ListPlans(context.Context, authorization.Info) ([]repositories.ServicePlanRecord, error) + ListPlans(context.Context, authorization.Info, repositories.ListServicePlanMessage) ([]repositories.ServicePlanRecord, error) } type ServicePlan struct { - serverURL url.URL - servicePlanRepo CFServicePlanRepository + serverURL url.URL + requestValidator RequestValidator + servicePlanRepo CFServicePlanRepository } func NewServicePlan( serverURL url.URL, + requestValidator RequestValidator, servicePlanRepo CFServicePlanRepository, ) *ServicePlan { return &ServicePlan{ - serverURL: serverURL, - servicePlanRepo: servicePlanRepo, + serverURL: serverURL, + requestValidator: requestValidator, + servicePlanRepo: servicePlanRepo, } } @@ -42,7 +46,12 @@ func (h *ServicePlan) list(r *http.Request) (*routing.Response, error) { authInfo, _ := authorization.InfoFromContext(r.Context()) logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.service-plan.list") - servicePlanList, err := h.servicePlanRepo.ListPlans(r.Context(), authInfo) + var payload payloads.ServicePlanList + if err := h.requestValidator.DecodeAndValidateURLValues(r, &payload); err != nil { + return nil, apierrors.LogAndReturn(logger, err, "failed to decode json payload") + } + + servicePlanList, err := h.servicePlanRepo.ListPlans(r.Context(), authInfo, payload.ToMessage()) if err != nil { return nil, apierrors.LogAndReturn(logger, err, "Failed to list service plans") } diff --git a/api/handlers/service_plan_test.go b/api/handlers/service_plan_test.go index 181bd0aa4..28591e5b4 100644 --- a/api/handlers/service_plan_test.go +++ b/api/handlers/service_plan_test.go @@ -6,6 +6,7 @@ import ( . "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/model" . "code.cloudfoundry.org/korifi/tests/matchers" @@ -15,13 +16,18 @@ import ( ) var _ = Describe("ServicePlan", func() { - var servicePlanRepo *fake.CFServicePlanRepository + var ( + servicePlanRepo *fake.CFServicePlanRepository + requestValidator *fake.RequestValidator + ) BeforeEach(func() { + requestValidator = new(fake.RequestValidator) servicePlanRepo = new(fake.CFServicePlanRepository) apiHandler := NewServicePlan( *serverURL, + requestValidator, servicePlanRepo, ) routerBuilder.LoadRoutes(apiHandler) @@ -52,7 +58,7 @@ var _ = Describe("ServicePlan", func() { It("lists the service plans", func() { Expect(servicePlanRepo.ListPlansCallCount()).To(Equal(1)) - _, actualAuthInfo := servicePlanRepo.ListPlansArgsForCall(0) + _, actualAuthInfo, _ := servicePlanRepo.ListPlansArgsForCall(0) Expect(actualAuthInfo).To(Equal(authInfo)) Expect(rr).Should(HaveHTTPStatus(http.StatusOK)) @@ -66,6 +72,30 @@ var _ = Describe("ServicePlan", func() { ))) }) + When("filtering query params are provided", func() { + BeforeEach(func() { + requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(&payloads.ServicePlanList{ + ServiceOfferingGUIDs: "a1,a2", + }) + }) + + It("passes them to the repository", func() { + Expect(servicePlanRepo.ListPlansCallCount()).To(Equal(1)) + _, _, message := servicePlanRepo.ListPlansArgsForCall(0) + Expect(message.ServiceOfferingGUIDs).To(ConsistOf("a1", "a2")) + }) + }) + + When("the request is invalid", func() { + BeforeEach(func() { + requestValidator.DecodeAndValidateURLValuesReturns(errors.New("invalid-request")) + }) + + It("returns an error", func() { + expectUnknownError() + }) + }) + When("listing the plans fails", func() { BeforeEach(func() { servicePlanRepo.ListPlansReturns(nil, errors.New("list-err")) diff --git a/api/main.go b/api/main.go index 2c7f2d433..6211ae704 100644 --- a/api/main.go +++ b/api/main.go @@ -428,6 +428,7 @@ func main() { ), handlers.NewServicePlan( *serverURL, + requestValidator, servicePlanRepo, ), } diff --git a/api/payloads/service_plan.go b/api/payloads/service_plan.go new file mode 100644 index 000000000..4672063d2 --- /dev/null +++ b/api/payloads/service_plan.go @@ -0,0 +1,32 @@ +package payloads + +import ( + "net/url" + "regexp" + + "code.cloudfoundry.org/korifi/api/payloads/parse" + "code.cloudfoundry.org/korifi/api/repositories" +) + +type ServicePlanList struct { + ServiceOfferingGUIDs string +} + +func (l *ServicePlanList) ToMessage() repositories.ListServicePlanMessage { + return repositories.ListServicePlanMessage{ + ServiceOfferingGUIDs: parse.ArrayParam(l.ServiceOfferingGUIDs), + } +} + +func (l *ServicePlanList) SupportedKeys() []string { + return []string{"service_offering_guids", "page", "per_page"} +} + +func (l *ServicePlanList) IgnoredKeys() []*regexp.Regexp { + return []*regexp.Regexp{regexp.MustCompile(`fields\[.+\]`)} +} + +func (l *ServicePlanList) DecodeFromURLValues(values url.Values) error { + l.ServiceOfferingGUIDs = values.Get("service_offering_guids") + return nil +} diff --git a/api/payloads/service_plan_test.go b/api/payloads/service_plan_test.go new file mode 100644 index 000000000..79815d375 --- /dev/null +++ b/api/payloads/service_plan_test.go @@ -0,0 +1,30 @@ +package payloads_test + +import ( + "code.cloudfoundry.org/korifi/api/payloads" + "code.cloudfoundry.org/korifi/api/repositories" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("ServicePlan", func() { + DescribeTable("valid query", + func(query string, expectedServicePlanList payloads.ServicePlanList) { + actualServicePlanList, decodeErr := decodeQuery[payloads.ServicePlanList](query) + + Expect(decodeErr).NotTo(HaveOccurred()) + Expect(*actualServicePlanList).To(Equal(expectedServicePlanList)) + }, + Entry("names", "names=b1,b2", payloads.ServicePlanList{ServiceOfferingGUIDs: "b1,b2"}), + ) + + Describe("ToMessage", func() { + It("converts payload to repository message", func() { + payload := &payloads.ServicePlanList{ServiceOfferingGUIDs: "b1,b2"} + + Expect(payload.ToMessage()).To(Equal(repositories.ListServicePlanMessage{ + ServiceOfferingGUIDs: []string{"b1", "b2"}, + })) + }) + }) +}) diff --git a/api/repositories/service_offering_repository_test.go b/api/repositories/service_offering_repository_test.go index 920cda1eb..1f07b0b87 100644 --- a/api/repositories/service_offering_repository_test.go +++ b/api/repositories/service_offering_repository_test.go @@ -135,6 +135,7 @@ var _ = Describe("ServiceOfferingRepo", func() { }) It("returns the matching offerings", func() { + Expect(listErr).NotTo(HaveOccurred()) Expect(listedOfferings).To(ConsistOf(MatchFields(IgnoreExtras, Fields{ "ServiceOffering": MatchFields(IgnoreExtras, Fields{ "Name": Equal("my-offering"), diff --git a/api/repositories/service_plan_repository.go b/api/repositories/service_plan_repository.go index c91681222..9db1f28ce 100644 --- a/api/repositories/service_plan_repository.go +++ b/api/repositories/service_plan_repository.go @@ -40,7 +40,15 @@ func NewServicePlanRepo( } } -func (r *ServicePlanRepo) ListPlans(ctx context.Context, authInfo authorization.Info) ([]ServicePlanRecord, error) { +type ListServicePlanMessage struct { + ServiceOfferingGUIDs []string +} + +func (m *ListServicePlanMessage) matches(cfServicePlan korifiv1alpha1.CFServicePlan) bool { + return emptyOrContains(m.ServiceOfferingGUIDs, cfServicePlan.Labels[korifiv1alpha1.RelServiceOfferingLabel]) +} + +func (r *ServicePlanRepo) ListPlans(ctx context.Context, authInfo authorization.Info, message ListServicePlanMessage) ([]ServicePlanRecord, error) { userClient, err := r.userClientFactory.BuildClient(authInfo) if err != nil { return nil, fmt.Errorf("failed to build user client: %w", err) @@ -51,7 +59,7 @@ func (r *ServicePlanRepo) ListPlans(ctx context.Context, authInfo authorization. return nil, apierrors.FromK8sError(err, ServicePlanResourceType) } - return iter.Map(iter.Lift(cfServicePlans.Items), planToRecord).Collect(), nil + return iter.Map(iter.Lift(cfServicePlans.Items).Filter(message.matches), planToRecord).Collect(), nil } func planToRecord(plan korifiv1alpha1.CFServicePlan) ServicePlanRecord { diff --git a/api/repositories/service_plan_repository_test.go b/api/repositories/service_plan_repository_test.go index c2fc668f9..6c0b33d1f 100644 --- a/api/repositories/service_plan_repository_test.go +++ b/api/repositories/service_plan_repository_test.go @@ -25,6 +25,7 @@ var _ = Describe("ServicePlanRepo", func() { var ( planGUID string listedPlans []repositories.ServicePlanRecord + message repositories.ListServicePlanMessage listErr error ) @@ -82,10 +83,12 @@ var _ = Describe("ServicePlanRepo", func() { }, }, })).To(Succeed()) + + message = repositories.ListServicePlanMessage{} }) JustBeforeEach(func() { - listedPlans, listErr = repo.ListPlans(ctx, authInfo) + listedPlans, listErr = repo.ListPlans(ctx, authInfo, message) }) It("lists service offerings", func() { @@ -148,5 +151,34 @@ var _ = Describe("ServicePlanRepo", func() { }), }))) }) + + When("filtering by service_offering_guid", func() { + BeforeEach(func() { + Expect(k8sClient.Create(ctx, &korifiv1alpha1.CFServicePlan{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: uuid.NewString(), + Labels: map[string]string{ + korifiv1alpha1.RelServiceOfferingLabel: "other-offering-guid", + }, + }, + })).To(Succeed()) + + message.ServiceOfferingGUIDs = []string{"other-offering-guid"} + }) + + It("returns matching service plans", func() { + Expect(listErr).NotTo(HaveOccurred()) + Expect(listedPlans).To(ConsistOf(MatchFields(IgnoreExtras, Fields{ + "Relationships": Equal(repositories.ServicePlanRelationships{ + ServiceOffering: model.ToOneRelationship{ + Data: model.Relationship{ + GUID: "other-offering-guid", + }, + }, + }), + }))) + }) + }) }) })