Include broker fields when listing offerings

fixes #3413

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

api/handlers/package_test.go

@@ -191,9 +191,7 @@ var _ = Describe("Package", func() {
 			It("calls the package repository with expected arguments", func() {
 				_, _, message := packageRepo.ListPackagesArgsForCall(0)
 				Expect(message).To(Equal(repositories.ListPackagesMessage{
-					GUIDs:    []string{},
 					AppGUIDs: []string{appGUID},
-					States:   []string{},
 				}))
 
 				Expect(rr).To(HaveHTTPStatus(http.StatusOK))
@@ -252,9 +250,7 @@ var _ = Describe("Package", func() {
 			It("calls repository ListPackage with the correct message object", func() {
 				_, _, message := packageRepo.ListPackagesArgsForCall(0)
 				Expect(message).To(Equal(repositories.ListPackagesMessage{
-					GUIDs:    []string{},
-					AppGUIDs: []string{},
-					States:   []string{"READY", "AWAITING_UPLOAD"},
+					States: []string{"READY", "AWAITING_UPLOAD"},
 				}))
 				Expect(rr).To(HaveHTTPStatus(http.StatusOK))
 			})

api/handlers/service_broker_test.go

@@ -14,7 +14,6 @@ import (
 	"code.cloudfoundry.org/korifi/model/services"
 	. "code.cloudfoundry.org/korifi/tests/matchers"
 	"code.cloudfoundry.org/korifi/tools"
-	. "github.com/onsi/gomega/gstruct"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -120,9 +119,7 @@ var _ = Describe("ServiceBroker", func() {
 			Expect(serviceBrokerRepo.ListServiceBrokersCallCount()).To(Equal(1))
 			_, actualAuthInfo, actualListMsg := serviceBrokerRepo.ListServiceBrokersArgsForCall(0)
 			Expect(actualAuthInfo).To(Equal(authInfo))
-			Expect(actualListMsg).To(MatchAllFields(Fields{
-				"Names": BeEmpty(),
-			}))
+			Expect(actualListMsg).To(Equal(repositories.ListServiceBrokerMessage{}))
 
 			Expect(rr).Should(HaveHTTPStatus(http.StatusOK))
 			Expect(rr).To(HaveHTTPHeaderWithValue("Content-Type", "application/json"))

api/handlers/service_offering.go

@@ -5,13 +5,17 @@ import (
 	"context"
 	"net/http"
 	"net/url"
+	"slices"
 
 	"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"
+	"code.cloudfoundry.org/korifi/model"
+	"code.cloudfoundry.org/korifi/tools"
+	"github.com/BooleanCat/go-functional/iter"
 	"github.com/go-logr/logr"
 )
 
@@ -28,17 +32,20 @@ type ServiceOffering struct {
 	serverURL           url.URL
 	requestValidator    RequestValidator
 	serviceOfferingRepo CFServiceOfferingRepository
+	serviceBrokerRepo   CFServiceBrokerRepository
 }
 
 func NewServiceOffering(
 	serverURL url.URL,
 	requestValidator RequestValidator,
 	serviceOfferingRepo CFServiceOfferingRepository,
+	serviceBrokerRepo CFServiceBrokerRepository,
 ) *ServiceOffering {
 	return &ServiceOffering{
 		serverURL:           serverURL,
 		requestValidator:    requestValidator,
 		serviceOfferingRepo: serviceOfferingRepo,
+		serviceBrokerRepo:   serviceBrokerRepo,
 	}
 }
 
@@ -53,10 +60,52 @@ func (h *ServiceOffering) list(r *http.Request) (*routing.Response, error) {
 
 	serviceOfferingList, err := h.serviceOfferingRepo.ListOfferings(r.Context(), authInfo, payload.ToMessage())
 	if err != nil {
-		return nil, apierrors.LogAndReturn(logger, err, "Failed to list service offerings")
+		return nil, apierrors.LogAndReturn(logger, err, "failed to list service offerings")
 	}
 
-	return routing.NewResponse(http.StatusOK).WithBody(presenter.ForList(presenter.ForServiceOffering, serviceOfferingList, h.serverURL, *r.URL)), nil
+	brokerIncludes, err := h.getBrokerIncludes(r.Context(), authInfo, serviceOfferingList, payload.IncludeBrokerFields)
+	if err != nil {
+		return nil, apierrors.LogAndReturn(logger, err, "failed to get broker includes")
+	}
+
+	return routing.NewResponse(http.StatusOK).WithBody(presenter.ForList(presenter.ForServiceOffering, serviceOfferingList, h.serverURL, *r.URL, brokerIncludes...)), nil
+}
+
+func (h *ServiceOffering) getBrokerIncludes(
+	ctx context.Context,
+	authInfo authorization.Info,
+	serviceOfferings []repositories.ServiceOfferingRecord,
+	brokerFields []string,
+) ([]model.IncludedResource, error) {
+	if len(brokerFields) == 0 {
+		return nil, nil
+	}
+
+	brokerGUIDs := iter.Map(iter.Lift(serviceOfferings), func(o repositories.ServiceOfferingRecord) string {
+		return o.ServiceBrokerGUID
+	}).Collect()
+
+	brokers, err := h.serviceBrokerRepo.ListServiceBrokers(ctx, authInfo, repositories.ListServiceBrokerMessage{
+		GUIDs: tools.Uniq(brokerGUIDs),
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	return iter.Map(iter.Lift(brokers), func(b repositories.ServiceBrokerRecord) model.IncludedResource {
+		resource := map[string]string{}
+		if slices.Contains(brokerFields, "guid") {
+			resource["guid"] = b.GUID
+		}
+		if slices.Contains(brokerFields, "name") {
+			resource["name"] = b.Name
+		}
+
+		return model.IncludedResource{
+			Type:     "service_brokers",
+			Resource: resource,
+		}
+	}).Collect(), nil
 }
 
 func (h *ServiceOffering) UnauthenticatedRoutes() []routing.Route {

api/handlers/service_offering_test.go

@@ -20,16 +20,19 @@ var _ = Describe("ServiceOffering", func() {
 	var (
 		requestValidator    *fake.RequestValidator
 		serviceOfferingRepo *fake.CFServiceOfferingRepository
+		serviceBrokerRepo   *fake.CFServiceBrokerRepository
 	)
 
 	BeforeEach(func() {
 		requestValidator = new(fake.RequestValidator)
 		serviceOfferingRepo = new(fake.CFServiceOfferingRepository)
+		serviceBrokerRepo = new(fake.CFServiceBrokerRepository)
 
 		apiHandler := NewServiceOffering(
 			*serverURL,
 			requestValidator,
 			serviceOfferingRepo,
+			serviceBrokerRepo,
 		)
 		routerBuilder.LoadRoutes(apiHandler)
 	})
@@ -83,6 +86,71 @@ var _ = Describe("ServiceOffering", func() {
 			})
 		})
 
+		Describe("include broker fields", func() {
+			BeforeEach(func() {
+				serviceBrokerRepo.ListServiceBrokersReturns([]repositories.ServiceBrokerRecord{{
+					ServiceBroker: services.ServiceBroker{
+						Name: "broker-name",
+					},
+					CFResource: model.CFResource{
+						GUID: "broker-guid",
+					},
+				}}, nil)
+
+				requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(&payloads.ServiceOfferingList{
+					IncludeBrokerFields: []string{"foo"},
+				})
+			})
+
+			It("lists the brokers", func() {
+				Expect(serviceBrokerRepo.ListServiceBrokersCallCount()).To(Equal(1))
+				_, _, actualListMessage := serviceBrokerRepo.ListServiceBrokersArgsForCall(0)
+				Expect(actualListMessage).To(Equal(repositories.ListServiceBrokerMessage{
+					GUIDs: []string{"broker-guid"},
+				}))
+			})
+
+			When("listing brokers fails", func() {
+				BeforeEach(func() {
+					serviceBrokerRepo.ListServiceBrokersReturns([]repositories.ServiceBrokerRecord{}, errors.New("list-broker-err"))
+				})
+
+				It("returns an error", func() {
+					expectUnknownError()
+				})
+			})
+
+			Describe("broker name", func() {
+				BeforeEach(func() {
+					requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(&payloads.ServiceOfferingList{
+						IncludeBrokerFields: []string{"name"},
+					})
+				})
+
+				It("includes broker fields in the response", func() {
+					Expect(rr).Should(HaveHTTPStatus(http.StatusOK))
+					Expect(rr).To(HaveHTTPBody(SatisfyAll(
+						MatchJSONPath("$.included.service_brokers[0].name", "broker-name"),
+					)))
+				})
+			})
+
+			Describe("broker guid", func() {
+				BeforeEach(func() {
+					requestValidator.DecodeAndValidateURLValuesStub = decodeAndValidateURLValuesStub(&payloads.ServiceOfferingList{
+						IncludeBrokerFields: []string{"guid"},
+					})
+				})
+
+				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", "broker-guid"),
+					)))
+				})
+			})
+		})
+
 		When("the request is invalid", func() {
 			BeforeEach(func() {
 				requestValidator.DecodeAndValidateURLValuesReturns(errors.New("invalid-request"))

api/main.go

@@ -425,6 +425,7 @@ func main() {
 			*serverURL,
 			requestValidator,
 			serviceOfferingRepo,
+			serviceBrokerRepo,
 		),
 		handlers.NewServicePlan(
 			*serverURL,

api/payloads/package_test.go

@@ -319,10 +319,10 @@ var _ = Describe("PackageList", func() {
 
 			Expect(actualListPackagesMessage).To(Equal(expectedListPackagesMessage))
 		},
-		Entry("guids", payloads.PackageList{GUIDs: "g1,g2", AppGUIDs: "", States: ""}, repositories.ListPackagesMessage{GUIDs: []string{"g1", "g2"}, AppGUIDs: []string{}, States: []string{}}),
-		Entry("app_guids", payloads.PackageList{GUIDs: "", AppGUIDs: "ag1,ag2", States: ""}, repositories.ListPackagesMessage{GUIDs: []string{}, AppGUIDs: []string{"ag1", "ag2"}, States: []string{}}),
-		Entry("states", payloads.PackageList{GUIDs: "", AppGUIDs: "", States: "s1,s2"}, repositories.ListPackagesMessage{GUIDs: []string{}, AppGUIDs: []string{}, States: []string{"s1", "s2"}}),
-		Entry("empty", payloads.PackageList{}, repositories.ListPackagesMessage{GUIDs: []string{}, AppGUIDs: []string{}, States: []string{}}),
+		Entry("guids", payloads.PackageList{GUIDs: "g1,g2"}, repositories.ListPackagesMessage{GUIDs: []string{"g1", "g2"}}),
+		Entry("app_guids", payloads.PackageList{AppGUIDs: "ag1,ag2"}, repositories.ListPackagesMessage{AppGUIDs: []string{"ag1", "ag2"}}),
+		Entry("states", payloads.PackageList{States: "s1,s2"}, repositories.ListPackagesMessage{States: []string{"s1", "s2"}}),
+		Entry("empty", payloads.PackageList{}, repositories.ListPackagesMessage{}),
 	)
 
 	DescribeTable("invalid query",

api/payloads/parse/arrays.go

@@ -4,7 +4,7 @@ import "strings"
 
 func ArrayParam(arrayParam string) []string {
 	if arrayParam == "" {
-		return []string{}
+		return nil
 	}
 
 	elements := strings.Split(arrayParam, ",")

api/payloads/parse/arrays_test.go

@@ -7,9 +7,9 @@ import (
 )
 
 var _ = Describe("ParseArrayParam", func() {
-	When("a nil value is specified", func() {
-		It("returns an empty array", func() {
-			Expect(ArrayParam("")).To(Equal([]string{}))
+	When("an empty string is specified", func() {
+		It("returns nil", func() {
+			Expect(ArrayParam("")).To(BeNil())
 		})
 	})
 

api/payloads/service_offering.go

@@ -5,12 +5,21 @@ import (
 	"regexp"
 
 	"code.cloudfoundry.org/korifi/api/payloads/parse"
+	"code.cloudfoundry.org/korifi/api/payloads/validation"
 	"code.cloudfoundry.org/korifi/api/repositories"
+	jellidation "github.com/jellydator/validation"
 )
 
 type ServiceOfferingList struct {
-	Names       string
-	BrokerNames string
+	Names               string
+	BrokerNames         string
+	IncludeBrokerFields []string
+}
+
+func (l ServiceOfferingList) Validate() error {
+	return jellidation.ValidateStruct(&l,
+		jellidation.Field(&l.IncludeBrokerFields, jellidation.Each(validation.OneOf("guid", "name"))),
+	)
 }
 
 func (l *ServiceOfferingList) ToMessage() repositories.ListServiceOfferingMessage {
@@ -21,15 +30,16 @@ func (l *ServiceOfferingList) ToMessage() repositories.ListServiceOfferingMessag
 }
 
 func (l *ServiceOfferingList) SupportedKeys() []string {
-	return []string{"names", "service_broker_names", "page", "per_page"}
+	return []string{"names", "service_broker_names", "fields[service_broker]", "page", "per_page"}
 }
 
 func (l *ServiceOfferingList) IgnoredKeys() []*regexp.Regexp {
-	return []*regexp.Regexp{regexp.MustCompile(`fields\[.+\]`)}
+	return nil
 }
 
 func (l *ServiceOfferingList) DecodeFromURLValues(values url.Values) error {
 	l.Names = values.Get("names")
 	l.BrokerNames = values.Get("service_broker_names")
+	l.IncludeBrokerFields = parse.ArrayParam(values.Get("fields[service_broker]"))
 	return nil
 }

api/payloads/service_offering_test.go

@@ -5,6 +5,7 @@ import (
 	"code.cloudfoundry.org/korifi/api/repositories"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"github.com/onsi/gomega/types"
 )
 
 var _ = Describe("ServiceOffering", func() {
@@ -17,6 +18,15 @@ var _ = Describe("ServiceOffering", func() {
 		},
 		Entry("names", "names=b1,b2", payloads.ServiceOfferingList{Names: "b1,b2"}),
 		Entry("service_broker_names", "service_broker_names=b1,b2", payloads.ServiceOfferingList{BrokerNames: "b1,b2"}),
+		Entry("fields[service_broker]", "fields[service_broker]=guid,name", payloads.ServiceOfferingList{IncludeBrokerFields: []string{"guid", "name"}}),
+	)
+
+	DescribeTable("invalid query",
+		func(query string, errMatcher types.GomegaMatcher) {
+			_, decodeErr := decodeQuery[payloads.ServiceOfferingList](query)
+			Expect(decodeErr).To(errMatcher)
+		},
+		Entry("invalid service broker field", "fields[service_broker]=foo", MatchError(ContainSubstring("value must be one of: guid, name"))),
 	)
 
 	Describe("ToMessage", func() {

api/presenter/service_binding.go

@@ -4,6 +4,8 @@ import (
 	"net/url"
 
 	"code.cloudfoundry.org/korifi/api/repositories"
+	"code.cloudfoundry.org/korifi/model"
+	"github.com/BooleanCat/go-functional/iter"
 )
 
 const (
@@ -77,13 +79,12 @@ func ForServiceBinding(record repositories.ServiceBindingRecord, baseURL url.URL
 }
 
 func ForServiceBindingList(serviceBindingRecords []repositories.ServiceBindingRecord, appRecords []repositories.AppRecord, baseURL, requestURL url.URL) ListResponse[ServiceBindingResponse] {
-	ret := ForList(ForServiceBinding, serviceBindingRecords, baseURL, requestURL)
-	if len(appRecords) > 0 {
-		appData := IncludedData{}
-		for _, appRecord := range appRecords {
-			appData.Apps = append(appData.Apps, ForApp(appRecord, baseURL))
+	includedApps := iter.Map(iter.Lift(appRecords), func(app repositories.AppRecord) model.IncludedResource {
+		return model.IncludedResource{
+			Type:     "apps",
+			Resource: ForApp(app, baseURL),
 		}
-		ret.Included = &appData
-	}
-	return ret
+	}).Collect()
+
+	return ForList(ForServiceBinding, serviceBindingRecords, baseURL, requestURL, includedApps...)
 }