Skip to content

Commit

Permalink
Include service broker fields when listing plans
Browse files Browse the repository at this point in the history
fixes #3280

Co-authored-by: Georgi Sabev <[email protected]>
  • Loading branch information
danail-branekov and georgethebeatle committed Aug 12, 2024
1 parent 32bb2ff commit d184f73
Show file tree
Hide file tree
Showing 5 changed files with 148 additions and 19 deletions.
99 changes: 83 additions & 16 deletions api/handlers/service_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package handlers

import (
"context"
"fmt"
"net/http"
"net/url"
"slices"
Expand Down Expand Up @@ -37,19 +38,22 @@ type ServicePlan struct {
requestValidator RequestValidator
servicePlanRepo CFServicePlanRepository
serviceOfferingRepo CFServiceOfferingRepository
serviceBrokerRepo CFServiceBrokerRepository
}

func NewServicePlan(
serverURL url.URL,
requestValidator RequestValidator,
servicePlanRepo CFServicePlanRepository,
serviceOfferingRepo CFServiceOfferingRepository,
serviceBrokerRepo CFServiceBrokerRepository,
) *ServicePlan {
return &ServicePlan{
serverURL: serverURL,
requestValidator: requestValidator,
servicePlanRepo: servicePlanRepo,
serviceOfferingRepo: serviceOfferingRepo,
serviceBrokerRepo: serviceBrokerRepo,
}
}

Expand All @@ -62,47 +66,110 @@ func (h *ServicePlan) list(r *http.Request) (*routing.Response, error) {
return nil, apierrors.LogAndReturn(logger, err, "failed to decode json payload")
}

servicePlanList, err := h.servicePlanRepo.ListPlans(r.Context(), authInfo, payload.ToMessage())
servicePlans, err := h.servicePlanRepo.ListPlans(r.Context(), authInfo, payload.ToMessage())
if err != nil {
return nil, apierrors.LogAndReturn(logger, err, "failed to list service plans")
}

includedResources, err := h.getIncludedResources(r.Context(), authInfo, payload, servicePlans)
if err != nil {
return nil, apierrors.LogAndReturn(logger, err, "failed to build included resources")
}

return routing.NewResponse(http.StatusOK).WithBody(presenter.ForList(presenter.ForServicePlan, servicePlans, h.serverURL, *r.URL, includedResources...)), nil
}

func (h *ServicePlan) getIncludedResources(
ctx context.Context,
authInfo authorization.Info,
payload payloads.ServicePlanList,
servicePlans []repositories.ServicePlanRecord,
) ([]model.IncludedResource, error) {
if len(payload.IncludeResources) == 0 && len(payload.IncludeBrokerFields) == 0 {
return nil, nil
}

includedResources := []model.IncludedResource{}

offerings, err := h.listOfferings(ctx, authInfo, servicePlans)
if err != nil {
return nil, fmt.Errorf("failed to list offerings for plans: %w", err)
}

if slices.Contains(payload.IncludeResources, "service_offering") {
includedOfferings, err := h.getIncludedServiceOfferings(r.Context(), authInfo, servicePlanList, h.serverURL)
includedResources = append(includedResources, iter.Map(iter.Lift(offerings), func(o repositories.ServiceOfferingRecord) model.IncludedResource {
return model.IncludedResource{
Type: "service_offerings",
Resource: presenter.ForServiceOffering(o, h.serverURL),
}
}).Collect()...)
}

if len(payload.IncludeBrokerFields) != 0 {
brokers, err := h.listBrokers(ctx, authInfo, offerings)
if err != nil {
return nil, fmt.Errorf("failed to list brokers for offerings of plans: %w", err)
}

includedBrokerFields, err := h.getIncludedBrokerFields(brokers, payload.IncludeBrokerFields)
if err != nil {
return nil, apierrors.LogAndReturn(logger, err, "failed to get included service offerings")
return nil, fmt.Errorf("failed to get included broker fields: %w", err)
}
includedResources = append(includedResources, includedOfferings...)
includedResources = append(includedResources, includedBrokerFields...)
}

return routing.NewResponse(http.StatusOK).WithBody(presenter.ForList(presenter.ForServicePlan, servicePlanList, h.serverURL, *r.URL, includedResources...)), nil
return includedResources, nil
}

func (h *ServicePlan) getIncludedServiceOfferings(
func (h *ServicePlan) listOfferings(
ctx context.Context,
authInfo authorization.Info,
servicePlans []repositories.ServicePlanRecord,
baseUrl url.URL,
) ([]model.IncludedResource, error) {
) ([]repositories.ServiceOfferingRecord, error) {
offeringGUIDs := iter.Map(iter.Lift(servicePlans), func(o repositories.ServicePlanRecord) string {
return o.ServiceOfferingGUID
}).Collect()

offerings, err := h.serviceOfferingRepo.ListOfferings(ctx, authInfo, repositories.ListServiceOfferingMessage{
return h.serviceOfferingRepo.ListOfferings(ctx, authInfo, repositories.ListServiceOfferingMessage{
GUIDs: tools.Uniq(offeringGUIDs),
})
if err != nil {
return nil, err
}
}

func (h *ServicePlan) listBrokers(
ctx context.Context,
authInfo authorization.Info,
offerings []repositories.ServiceOfferingRecord,
) ([]repositories.ServiceBrokerRecord, error) {
brokerGUIDs := iter.Map(iter.Lift(offerings), func(o repositories.ServiceOfferingRecord) string {
return o.ServiceBrokerGUID
}).Collect()

return h.serviceBrokerRepo.ListServiceBrokers(ctx, authInfo, repositories.ListServiceBrokerMessage{
GUIDs: brokerGUIDs,
})
}

return iter.Map(iter.Lift(offerings), func(o repositories.ServiceOfferingRecord) model.IncludedResource {
func (h *ServicePlan) getIncludedBrokerFields(
brokers []repositories.ServiceBrokerRecord,
brokerFields []string,
) ([]model.IncludedResource, error) {
brokerIncludes := iter.Map(iter.Lift(brokers), func(b repositories.ServiceBrokerRecord) model.IncludedResource {
return model.IncludedResource{
Type: "service_offerings",
Resource: presenter.ForServiceOffering(o, baseUrl),
Type: "service_brokers",
Resource: presenter.ForServiceBroker(b, h.serverURL),
}
}).Collect(), nil
}).Collect()

brokerIncludesFields := []model.IncludedResource{}
for _, brokerInclude := range brokerIncludes {
fields, err := brokerInclude.SelectJSONFields(brokerFields...)
if err != nil {
return nil, err
}
brokerIncludesFields = append(brokerIncludesFields, fields)
}

return brokerIncludesFields, nil
}

func (h *ServicePlan) getPlanVisibility(r *http.Request) (*routing.Response, error) {
Expand Down
58 changes: 57 additions & 1 deletion api/handlers/service_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,22 @@ var _ = Describe("ServicePlan", func() {
var (
servicePlanRepo *fake.CFServicePlanRepository
serviceOfferingRepo *fake.CFServiceOfferingRepository
serviceBrokerRepo *fake.CFServiceBrokerRepository
requestValidator *fake.RequestValidator
)

BeforeEach(func() {
requestValidator = new(fake.RequestValidator)
servicePlanRepo = new(fake.CFServicePlanRepository)
serviceOfferingRepo = new(fake.CFServiceOfferingRepository)
serviceBrokerRepo = new(fake.CFServiceBrokerRepository)

apiHandler := NewServicePlan(
*serverURL,
requestValidator,
servicePlanRepo,
serviceOfferingRepo,
serviceBrokerRepo,
)
routerBuilder.LoadRoutes(apiHandler)
})
Expand Down Expand Up @@ -121,7 +124,7 @@ var _ = Describe("ServicePlan", func() {
})
})

It("includes broker fields in the response", func() {
It("includes service offering in the response", func() {
Expect(rr).Should(HaveHTTPStatus(http.StatusOK))
Expect(rr).To(HaveHTTPBody(SatisfyAll(
MatchJSONPath("$.included.service_offerings[0].guid", "service-offering-guid"),
Expand All @@ -130,6 +133,59 @@ var _ = Describe("ServicePlan", func() {
})
})

Describe("fields service_offering.service_broker", func() {
BeforeEach(func() {
serviceBrokerRepo.ListServiceBrokersReturns([]repositories.ServiceBrokerRecord{{
ServiceBroker: services.ServiceBroker{
Name: "service-broker-name",
},
CFResource: model.CFResource{
GUID: "service-broker-guid",
},
}}, nil)

serviceOfferingRepo.ListOfferingsReturns([]repositories.ServiceOfferingRecord{{
ServiceOffering: services.ServiceOffering{
Name: "service-offering-name",
},
CFResource: model.CFResource{
GUID: "service-offering-guid",
},
ServiceBrokerGUID: "service-broker-guid",
}}, nil)

requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(&payloads.ServicePlanList{
IncludeBrokerFields: []string{"guid", "name"},
})
})

It("lists the brokers", func() {
Expect(serviceBrokerRepo.ListServiceBrokersCallCount()).To(Equal(1))
_, _, actualListMessage := serviceBrokerRepo.ListServiceBrokersArgsForCall(0)
Expect(actualListMessage).To(Equal(repositories.ListServiceBrokerMessage{
GUIDs: []string{"service-broker-guid"},
}))
})

When("listing brokers fails", func() {
BeforeEach(func() {
serviceBrokerRepo.ListServiceBrokersReturns([]repositories.ServiceBrokerRecord{}, errors.New("list-broker-err"))
})

It("returns an error", func() {
expectUnknownError()
})
})

It("includes broker fields in the response", func() {
Expect(rr).Should(HaveHTTPStatus(http.StatusOK))
Expect(rr).To(HaveHTTPBody(SatisfyAll(
MatchJSONPath("$.included.service_brokers[0].guid", "service-broker-guid"),
MatchJSONPath("$.included.service_brokers[0].name", "service-broker-name"),
)))
})
})

When("the request is invalid", func() {
BeforeEach(func() {
requestValidator.DecodeAndValidateURLValuesReturns(errors.New("invalid-request"))
Expand Down
1 change: 1 addition & 0 deletions api/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ func main() {
requestValidator,
servicePlanRepo,
serviceOfferingRepo,
serviceBrokerRepo,
),
}
for _, handler := range apiHandlers {
Expand Down
7 changes: 5 additions & 2 deletions api/payloads/service_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@ type ServicePlanList struct {
Names string
Available *bool
IncludeResources []string
IncludeBrokerFields []string
}

func (l ServicePlanList) Validate() error {
return jellidation.ValidateStruct(&l,
jellidation.Field(&l.IncludeResources, jellidation.Each(validation.OneOf("service_offering"))),
jellidation.Field(&l.IncludeBrokerFields, jellidation.Each(validation.OneOf("guid", "name"))),
)
}

Expand All @@ -38,11 +40,11 @@ func (l *ServicePlanList) ToMessage() repositories.ListServicePlanMessage {
}

func (l *ServicePlanList) SupportedKeys() []string {
return []string{"service_offering_guids", "names", "available", "page", "per_page", "include"}
return []string{"service_offering_guids", "names", "available", "fields[service_offering.service_broker]", "page", "per_page", "include"}
}

func (l *ServicePlanList) IgnoredKeys() []*regexp.Regexp {
return []*regexp.Regexp{regexp.MustCompile(`fields\[.+\]`)}
return nil
}

func (l *ServicePlanList) DecodeFromURLValues(values url.Values) error {
Expand All @@ -55,6 +57,7 @@ func (l *ServicePlanList) DecodeFromURLValues(values url.Values) error {
}
l.Available = available
l.IncludeResources = parse.ArrayParam(values.Get("include"))
l.IncludeBrokerFields = parse.ArrayParam(values.Get("fields[service_offering.service_broker]"))

return nil
}
Expand Down
2 changes: 2 additions & 0 deletions api/payloads/service_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ var _ = Describe("ServicePlan", func() {
Entry("available", "available=true", payloads.ServicePlanList{Available: tools.PtrTo(true)}),
Entry("not available", "available=false", payloads.ServicePlanList{Available: tools.PtrTo(false)}),
Entry("include", "include=service_offering", payloads.ServicePlanList{IncludeResources: []string{"service_offering"}}),
Entry("service broker fields", "fields[service_offering.service_broker]=guid,name", payloads.ServicePlanList{IncludeBrokerFields: []string{"guid", "name"}}),
)

DescribeTable("invalid query",
Expand All @@ -34,6 +35,7 @@ var _ = Describe("ServicePlan", func() {
},
Entry("invalid available", "available=invalid", MatchError(ContainSubstring("failed to parse"))),
Entry("invalid include", "include=foo", MatchError(ContainSubstring("value must be one of: service_offering"))),
Entry("invalid service broker fields", "fields[service_offering.service_broker]=foo", MatchError(ContainSubstring("value must be one of"))),
)

Describe("ToMessage", func() {
Expand Down

0 comments on commit d184f73

Please sign in to comment.