Implement PATCH /v3/service_plans/{guid}/visibility

Also
- Add visibility_type to the service plan rest resource
- Implement filtering service offerings by broke name
- Implement filtering service plans by name

The above improvements enable certain options of the `cf service-acces` and
the `cf enable-service-access` commands

fixes #3276

Co-authored-by: Georgi Sabev <georgethebeatle@gmail.com>

api/handlers/service_offering_test.go

@@ -41,13 +41,7 @@ var _ = Describe("ServiceOffering", func() {
 				CFResource: model.CFResource{
 					GUID: "offering-guid",
 				},
-				Relationships: repositories.ServiceOfferingRelationships{
-					ServiceBroker: model.ToOneRelationship{
-						Data: model.Relationship{
-							GUID: "broker-guid",
-						},
-					},
-				},
+				ServiceBrokerGUID: "broker-guid",
 			}}, nil)
 		})
 

api/handlers/service_plan.go

@@ -25,6 +25,7 @@ type CFServicePlanRepository interface {
 	GetPlan(context.Context, authorization.Info, string) (repositories.ServicePlanRecord, error)
 	ListPlans(context.Context, authorization.Info, repositories.ListServicePlanMessage) ([]repositories.ServicePlanRecord, error)
 	ApplyPlanVisibility(context.Context, authorization.Info, repositories.ApplyServicePlanVisibilityMessage) (repositories.ServicePlanRecord, error)
+	UpdatePlanVisibility(context.Context, authorization.Info, repositories.UpdateServicePlanVisibilityMessage) (repositories.ServicePlanRecord, error)
 }
 
 type ServicePlan struct {
@@ -79,7 +80,7 @@ func (h *ServicePlan) getPlanVisibility(r *http.Request) (*routing.Response, err
 
 func (h *ServicePlan) applyPlanVisibility(r *http.Request) (*routing.Response, error) {
 	authInfo, _ := authorization.InfoFromContext(r.Context())
-	logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.service-plan.get-visibility")
+	logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.service-plan.apply-visibility")
 
 	planGUID := routing.URLParam(r, "guid")
 	logger = logger.WithValues("guid", planGUID)
@@ -89,14 +90,34 @@ func (h *ServicePlan) applyPlanVisibility(r *http.Request) (*routing.Response, e
 		return nil, apierrors.LogAndReturn(logger, err, "failed to decode json payload")
 	}
 
-	visibility, err := h.servicePlanRepo.ApplyPlanVisibility(r.Context(), authInfo, payload.ToMessage(planGUID))
+	visibility, err := h.servicePlanRepo.ApplyPlanVisibility(r.Context(), authInfo, payload.ToApplyMessage(planGUID))
 	if err != nil {
 		return nil, apierrors.LogAndReturn(logger, err, "failed to apply plan visibility")
 	}
 
 	return routing.NewResponse(http.StatusOK).WithBody(presenter.ForServicePlanVisibility(visibility, h.serverURL)), nil
 }
 
+func (h *ServicePlan) updatePlanVisibility(r *http.Request) (*routing.Response, error) {
+	authInfo, _ := authorization.InfoFromContext(r.Context())
+	logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.service-plan.update-visibility")
+
+	planGUID := routing.URLParam(r, "guid")
+	logger = logger.WithValues("guid", planGUID)
+
+	payload := payloads.ServicePlanVisibility{}
+	if err := h.requestValidator.DecodeAndValidateJSONPayload(r, &payload); err != nil {
+		return nil, apierrors.LogAndReturn(logger, err, "failed to decode json payload")
+	}
+
+	visibility, err := h.servicePlanRepo.UpdatePlanVisibility(r.Context(), authInfo, payload.ToUpdateMessage(planGUID))
+	if err != nil {
+		return nil, apierrors.LogAndReturn(logger, err, "failed to update plan visibility")
+	}
+
+	return routing.NewResponse(http.StatusOK).WithBody(presenter.ForServicePlanVisibility(visibility, h.serverURL)), nil
+}
+
 func (h *ServicePlan) UnauthenticatedRoutes() []routing.Route {
 	return nil
 }
@@ -106,5 +127,6 @@ func (h *ServicePlan) AuthenticatedRoutes() []routing.Route {
 		{Method: "GET", Pattern: ServicePlansPath, Handler: h.list},
 		{Method: "GET", Pattern: ServicePlanVisivilityPath, Handler: h.getPlanVisibility},
 		{Method: "POST", Pattern: ServicePlanVisivilityPath, Handler: h.applyPlanVisibility},
+		{Method: "PATCH", Pattern: ServicePlanVisivilityPath, Handler: h.updatePlanVisibility},
 	}
 }

api/handlers/service_plan_test.go

@@ -41,13 +41,7 @@ var _ = Describe("ServicePlan", func() {
 				CFResource: model.CFResource{
 					GUID: "plan-guid",
 				},
-				Relationships: repositories.ServicePlanRelationships{
-					ServiceOffering: model.ToOneRelationship{
-						Data: model.Relationship{
-							GUID: "service-offering-guid",
-						},
-					},
-				},
+				ServiceOfferingGUID: "service-offering-guid",
 			}}, nil)
 		})
 
@@ -112,7 +106,9 @@ var _ = Describe("ServicePlan", func() {
 	Describe("GET /v3/service_plans/{guid}/visibility", func() {
 		BeforeEach(func() {
 			servicePlanRepo.GetPlanReturns(repositories.ServicePlanRecord{
-				VisibilityType: korifiv1alpha1.AdminServicePlanVisibilityType,
+				Visibility: repositories.PlanVisibility{
+					Type: korifiv1alpha1.AdminServicePlanVisibilityType,
+				},
 			}, nil)
 		})
 
@@ -155,7 +151,9 @@ var _ = Describe("ServicePlan", func() {
 			})
 
 			servicePlanRepo.ApplyPlanVisibilityReturns(repositories.ServicePlanRecord{
-				VisibilityType: korifiv1alpha1.PublicServicePlanVisibilityType,
+				Visibility: repositories.PlanVisibility{
+					Type: korifiv1alpha1.PublicServicePlanVisibilityType,
+				},
 			}, nil)
 		})
 
@@ -177,8 +175,9 @@ var _ = Describe("ServicePlan", func() {
 			_, actualAuthInfo, actualMessage := servicePlanRepo.ApplyPlanVisibilityArgsForCall(0)
 			Expect(actualAuthInfo).To(Equal(authInfo))
 			Expect(actualMessage).To(Equal(repositories.ApplyServicePlanVisibilityMessage{
-				PlanGUID: "my-service-plan",
-				Type:     korifiv1alpha1.PublicServicePlanVisibilityType,
+				PlanGUID:      "my-service-plan",
+				Type:          korifiv1alpha1.PublicServicePlanVisibilityType,
+				Organizations: []string{},
 			}))
 
 			Expect(rr).To(HaveHTTPStatus(http.StatusOK))
@@ -209,4 +208,69 @@ var _ = Describe("ServicePlan", func() {
 			})
 		})
 	})
+
+	Describe("PATCH /v3/service_plans/{guid}/visibility", func() {
+		BeforeEach(func() {
+			requestValidator.DecodeAndValidateJSONPayloadStub = decodeAndValidatePayloadStub(&payloads.ServicePlanVisibility{
+				Type: korifiv1alpha1.PublicServicePlanVisibilityType,
+			})
+
+			servicePlanRepo.UpdatePlanVisibilityReturns(repositories.ServicePlanRecord{
+				Visibility: repositories.PlanVisibility{
+					Type: korifiv1alpha1.PublicServicePlanVisibilityType,
+				},
+			}, nil)
+		})
+
+		JustBeforeEach(func() {
+			req, err := http.NewRequestWithContext(ctx, "PATCH", "/v3/service_plans/my-service-plan/visibility", strings.NewReader("the-payload"))
+			Expect(err).NotTo(HaveOccurred())
+
+			routerBuilder.Build().ServeHTTP(rr, req)
+		})
+
+		It("validates the payload", func() {
+			Expect(requestValidator.DecodeAndValidateJSONPayloadCallCount()).To(Equal(1))
+			actualReq, _ := requestValidator.DecodeAndValidateJSONPayloadArgsForCall(0)
+			Expect(bodyString(actualReq)).To(Equal("the-payload"))
+		})
+
+		It("updates the plan visibility", func() {
+			Expect(servicePlanRepo.UpdatePlanVisibilityCallCount()).To(Equal(1))
+			_, actualAuthInfo, actualMessage := servicePlanRepo.UpdatePlanVisibilityArgsForCall(0)
+			Expect(actualAuthInfo).To(Equal(authInfo))
+			Expect(actualMessage).To(Equal(repositories.UpdateServicePlanVisibilityMessage{
+				PlanGUID:      "my-service-plan",
+				Type:          korifiv1alpha1.PublicServicePlanVisibilityType,
+				Organizations: []string{},
+			}))
+
+			Expect(rr).To(HaveHTTPStatus(http.StatusOK))
+			Expect(rr).To(HaveHTTPHeaderWithValue("Content-Type", "application/json"))
+
+			Expect(rr).To(HaveHTTPBody(SatisfyAll(
+				MatchJSONPath("$.type", korifiv1alpha1.PublicServicePlanVisibilityType),
+			)))
+		})
+
+		When("the payload is invalid", func() {
+			BeforeEach(func() {
+				requestValidator.DecodeAndValidateJSONPayloadReturns(errors.New("invalid-payload"))
+			})
+
+			It("returns an error", func() {
+				expectUnknownError()
+			})
+		})
+
+		When("updating the visibility fails", func() {
+			BeforeEach(func() {
+				servicePlanRepo.UpdatePlanVisibilityReturns(repositories.ServicePlanRecord{}, errors.New("visibility-err"))
+			})
+
+			It("returns an error", func() {
+				expectUnknownError()
+			})
+		})
+	})
 })

api/main.go

@@ -227,8 +227,8 @@ func main() {
 	)
 	metricsRepo := repositories.NewMetricsRepo(userClientFactory)
 	serviceBrokerRepo := repositories.NewServiceBrokerRepo(userClientFactory, cfg.RootNamespace)
-	serviceOfferingRepo := repositories.NewServiceOfferingRepo(userClientFactory, cfg.RootNamespace)
-	servicePlanRepo := repositories.NewServicePlanRepo(userClientFactory, cfg.RootNamespace)
+	serviceOfferingRepo := repositories.NewServiceOfferingRepo(userClientFactory, cfg.RootNamespace, serviceBrokerRepo)
+	servicePlanRepo := repositories.NewServicePlanRepo(userClientFactory, cfg.RootNamespace, orgRepo)
 
 	processStats := actions.NewProcessStats(processRepo, appRepo, metricsRepo)
 	manifest := actions.NewManifest(

api/payloads/service_offering.go

@@ -9,17 +9,19 @@ import (
 )
 
 type ServiceOfferingList struct {
-	Names string
+	Names       string
+	BrokerNames string
 }
 
 func (l *ServiceOfferingList) ToMessage() repositories.ListServiceOfferingMessage {
 	return repositories.ListServiceOfferingMessage{
-		Names: parse.ArrayParam(l.Names),
+		Names:       parse.ArrayParam(l.Names),
+		BrokerNames: parse.ArrayParam(l.BrokerNames),
 	}
 }
 
 func (l *ServiceOfferingList) SupportedKeys() []string {
-	return []string{"names", "page", "per_page"}
+	return []string{"names", "service_broker_names", "page", "per_page"}
 }
 
 func (l *ServiceOfferingList) IgnoredKeys() []*regexp.Regexp {
@@ -28,5 +30,6 @@ func (l *ServiceOfferingList) IgnoredKeys() []*regexp.Regexp {
 
 func (l *ServiceOfferingList) DecodeFromURLValues(values url.Values) error {
 	l.Names = values.Get("names")
+	l.BrokerNames = values.Get("service_broker_names")
 	return nil
 }

api/payloads/service_offering_test.go

@@ -16,14 +16,16 @@ var _ = Describe("ServiceOffering", func() {
 			Expect(*actualServiceOfferingList).To(Equal(expectedServiceOfferingList))
 		},
 		Entry("names", "names=b1,b2", payloads.ServiceOfferingList{Names: "b1,b2"}),
+		Entry("service_broker_names", "service_broker_names=b1,b2", payloads.ServiceOfferingList{BrokerNames: "b1,b2"}),
 	)
 
 	Describe("ToMessage", func() {
 		It("converts payload to repository message", func() {
-			payload := &payloads.ServiceOfferingList{Names: "b1,b2"}
+			payload := &payloads.ServiceOfferingList{Names: "b1,b2", BrokerNames: "br1,br2"}
 
 			Expect(payload.ToMessage()).To(Equal(repositories.ListServiceOfferingMessage{
-				Names: []string{"b1", "b2"},
+				Names:       []string{"b1", "b2"},
+				BrokerNames: []string{"br1", "br2"},
 			}))
 		})
 	})