From cea5f695cb1b2a7cdf79dccbd5deca348ef5bf55 Mon Sep 17 00:00:00 2001 From: Danail Branekov Date: Fri, 9 Aug 2024 12:35:20 +0000 Subject: [PATCH] Generalise including resources' fields `includes` and `fields` are quite similar concepts. The difference is that `include` inludes the complete resource, while `fields` select the fields of the resource to be included. With this commit we abstract that difference away. `IncludedResource` now has a `SelectJSONFields` method that allows picking the fields one want to see in the response. Under the hood `SelectJSONFields` uses json marshal/unmarshal, tehrefore fields selectors work on the resource json representation. --- api/handlers/service_offering.go | 51 +++++++++++++-------- model/included.go | 34 ++++++++++++++ model/included_test.go | 78 ++++++++++++++++++++++++++++++++ model/model_suite_test.go | 13 ++++++ 4 files changed, 156 insertions(+), 20 deletions(-) create mode 100644 model/included_test.go create mode 100644 model/model_suite_test.go diff --git a/api/handlers/service_offering.go b/api/handlers/service_offering.go index a4cde23d5..1b94646c1 100644 --- a/api/handlers/service_offering.go +++ b/api/handlers/service_offering.go @@ -5,7 +5,6 @@ import ( "context" "net/http" "net/url" - "slices" "code.cloudfoundry.org/korifi/api/authorization" apierrors "code.cloudfoundry.org/korifi/api/errors" @@ -63,7 +62,7 @@ func (h *ServiceOffering) list(r *http.Request) (*routing.Response, error) { return nil, apierrors.LogAndReturn(logger, err, "failed to list service offerings") } - brokerIncludes, err := h.getBrokerIncludes(r.Context(), authInfo, serviceOfferingList, payload.IncludeBrokerFields) + brokerIncludes, err := h.getBrokerIncludes(r.Context(), authInfo, serviceOfferingList, payload.IncludeBrokerFields, h.serverURL) if err != nil { return nil, apierrors.LogAndReturn(logger, err, "failed to get broker includes") } @@ -71,41 +70,53 @@ func (h *ServiceOffering) list(r *http.Request) (*routing.Response, error) { return routing.NewResponse(http.StatusOK).WithBody(presenter.ForList(presenter.ForServiceOffering, serviceOfferingList, h.serverURL, *r.URL, brokerIncludes...)), nil } +func (h *ServiceOffering) listBrokersForOfferings( + ctx context.Context, + authInfo authorization.Info, + serviceOfferings []repositories.ServiceOfferingRecord, +) ([]repositories.ServiceBrokerRecord, error) { + brokerGUIDs := iter.Map(iter.Lift(serviceOfferings), func(o repositories.ServiceOfferingRecord) string { + return o.ServiceBrokerGUID + }).Collect() + + return h.serviceBrokerRepo.ListServiceBrokers(ctx, authInfo, repositories.ListServiceBrokerMessage{ + GUIDs: tools.Uniq(brokerGUIDs), + }) +} + func (h *ServiceOffering) getBrokerIncludes( ctx context.Context, authInfo authorization.Info, serviceOfferings []repositories.ServiceOfferingRecord, brokerFields []string, + baseURL url.URL, ) ([]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), - }) + brokers, err := h.listBrokersForOfferings(ctx, authInfo, serviceOfferings) 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 - } - + brokerIncludes := iter.Map(iter.Lift(brokers), func(b repositories.ServiceBrokerRecord) model.IncludedResource { return model.IncludedResource{ Type: "service_brokers", - Resource: resource, + Resource: presenter.ForServiceBroker(b, baseURL), + } + }).Collect() + + brokerIncludesFielded := []model.IncludedResource{} + for _, brokerInclude := range brokerIncludes { + fieldedInclude, err := brokerInclude.SelectJSONFields(brokerFields...) + if err != nil { + return nil, err } - }).Collect(), nil + brokerIncludesFielded = append(brokerIncludesFielded, fieldedInclude) + } + + return brokerIncludesFielded, nil } func (h *ServiceOffering) UnauthenticatedRoutes() []routing.Route { diff --git a/model/included.go b/model/included.go index a7f9b94a4..62a97e890 100644 --- a/model/included.go +++ b/model/included.go @@ -1,6 +1,40 @@ package model +import ( + "encoding/json" + "fmt" +) + type IncludedResource struct { Type string Resource any } + +func (r IncludedResource) SelectJSONFields(fields ...string) (IncludedResource, error) { + resourceBytes, err := json.Marshal(r.Resource) + if err != nil { + return IncludedResource{}, fmt.Errorf("failed to marshal resource: %w", err) + } + + resourceMap := map[string]any{} + if err := json.Unmarshal(resourceBytes, &resourceMap); err != nil { + return IncludedResource{}, fmt.Errorf("failed to unmarshal resource: %w", err) + } + + if len(fields) == 0 { + return IncludedResource{ + Type: r.Type, + Resource: resourceMap, + }, nil + } + + resourceFromFields := map[string]any{} + for _, field := range fields { + resourceFromFields[field] = resourceMap[field] + } + + return IncludedResource{ + Type: r.Type, + Resource: resourceFromFields, + }, nil +} diff --git a/model/included_test.go b/model/included_test.go new file mode 100644 index 000000000..1a4eedd55 --- /dev/null +++ b/model/included_test.go @@ -0,0 +1,78 @@ +package model_test + +import ( + "code.cloudfoundry.org/korifi/model" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "github.com/onsi/gomega/gstruct" +) + +var _ = Describe("IncludedResource", func() { + var resource model.IncludedResource + + BeforeEach(func() { + resource = model.IncludedResource{ + Type: "my-resource-type", + Resource: struct { + StringField string `json:"string_field"` + IntField int `json:"int_field"` + StructField struct { + Foo string `json:"foo"` + } `json:"struct_field"` + }{ + StringField: "my_string", + IntField: 5, + StructField: struct { + Foo string `json:"foo"` + }{ + Foo: "bar", + }, + }, + } + }) + + Describe("SelectJSONFields", func() { + var ( + resourceWithFields model.IncludedResource + fields []string + ) + + BeforeEach(func() { + fields = []string{} + }) + + JustBeforeEach(func() { + var err error + resourceWithFields, err = resource.SelectJSONFields(fields...) + Expect(err).NotTo(HaveOccurred()) + }) + + It("returns a resource with all fields", func() { + Expect(resourceWithFields).To(MatchAllFields(Fields{ + "Type": Equal("my-resource-type"), + "Resource": MatchAllKeys(Keys{ + "string_field": Equal("my_string"), + "int_field": BeEquivalentTo(5), + "struct_field": MatchAllKeys(Keys{ + "foo": Equal("bar"), + }), + }), + })) + }) + + When("fields are selected", func() { + BeforeEach(func() { + fields = []string{"int_field"} + }) + + It("returns a resource with selected fields only", func() { + Expect(resourceWithFields).To(MatchAllFields(Fields{ + "Type": Equal("my-resource-type"), + "Resource": MatchAllKeys(Keys{ + "int_field": BeEquivalentTo(5), + }), + })) + }) + }) + }) +}) diff --git a/model/model_suite_test.go b/model/model_suite_test.go new file mode 100644 index 000000000..64ce08a9d --- /dev/null +++ b/model/model_suite_test.go @@ -0,0 +1,13 @@ +package model_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestModel(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Model Suite") +}