From e4934f75605c920bf071194f5e1e2046a1f7aeaa Mon Sep 17 00:00:00 2001 From: Kiril Kabakchiev Date: Fri, 6 Dec 2019 12:53:53 +0200 Subject: [PATCH] Add composite error to async jobs (#82) --- Gopkg.lock | 12 +++++----- Gopkg.toml | 2 +- cf/service_access.go | 22 +++++------------- cf/service_access_test.go | 2 +- cf/service_visibilities.go | 47 +++++++++++++++----------------------- 5 files changed, 32 insertions(+), 53 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 823de48..1bd4ed2 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -40,7 +40,8 @@ version = "v1.5.0" [[projects]] - digest = "1:f70b6a8ee0ec108ee71d59d92c4015eb0b908ece2396d28fa99253aed28d6814" + branch = "master" + digest = "1:b3a24d07d84fb45352e3b8f53a0a4850f9a052245a9cc0cb57d2705649141cb5" name = "github.com/Peripli/service-broker-proxy" packages = [ "pkg/authn", @@ -53,8 +54,7 @@ "pkg/sm", ] pruneopts = "UT" - revision = "f19e624be497dd0510b9f00436d67f24a776d4ba" - version = "v0.7.0" + revision = "7942aea817dd6b404db17bdc0474c43a10baaba2" [[projects]] digest = "1:127aa936d6291ec7ebb8678fef00779fc3574a616cb2b938dbaa407af7221471" @@ -428,11 +428,11 @@ "pbkdf2", ] pruneopts = "UT" - revision = "86a70503ff7e82ffc18c7b0de83db35da4791e6a" + revision = "e7c4368fe9ddd156b5f1463283cb51c5b400c373" [[projects]] branch = "master" - digest = "1:54b4b9bc4be8560c3c1849569a2e75a0fe30c7c77b45f2e96f76df6dbbf04bb6" + digest = "1:6d181de04481fe0fca984916930cdaaa0fbad5121c08b7255eb78be76c5ebada" name = "golang.org/x/net" packages = [ "context", @@ -442,7 +442,7 @@ "html/charset", ] pruneopts = "UT" - revision = "5ee1b9f4859acd2e99987ef94ec7a58427c53bef" + revision = "1ddd1de85cb0337b623b740a609d35817d516a8d" [[projects]] branch = "master" diff --git a/Gopkg.toml b/Gopkg.toml index d3732b5..544489b 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -35,7 +35,7 @@ [[constraint]] name = "github.com/Peripli/service-broker-proxy" - version = "=0.7.0" + branch = "master" [[constraint]] name = "github.com/cloudfoundry-community/go-cfenv" diff --git a/cf/service_access.go b/cf/service_access.go index 48ac8d2..8503949 100644 --- a/cf/service_access.go +++ b/cf/service_access.go @@ -7,7 +7,8 @@ import ( "fmt" "net/http" "net/url" - "strings" + + "github.com/Peripli/service-broker-proxy/pkg/sbproxy/reconcile" "github.com/Peripli/service-broker-proxy/pkg/platform" @@ -33,19 +34,7 @@ func (pc *PlatformClient) DisableAccessForPlan(ctx context.Context, request *pla return pc.updateAccessForPlan(ctx, request, false) } -type compositeError []error - -func (ce compositeError) Error() string { - errs := make([]string, 0, len(ce)) - for _, e := range ce { - errs = append(errs, fmt.Sprintln(e.Error())) - } - - return strings.Join(errs, "") -} - func (pc *PlatformClient) updateAccessForPlan(ctx context.Context, request *platform.ModifyPlanAccessRequest, isEnabled bool) error { - var compositeErr compositeError if request == nil { return errors.Errorf("modify plan access request cannot be nil") @@ -56,14 +45,15 @@ func (pc *PlatformClient) updateAccessForPlan(ctx context.Context, request *plat return err } + compositeErr := &reconcile.CompositeError{} if orgGUIDs, ok := request.Labels[OrgLabelKey]; ok && len(orgGUIDs) != 0 { for _, orgGUID := range orgGUIDs { if err := pc.updateOrgVisibilityForPlan(ctx, plan, isEnabled, orgGUID); err != nil { - compositeErr = append(compositeErr, err) + compositeErr.Add(err) } } - if compositeErr != nil { - return errors.Wrapf(compositeErr, "error while updating access for catalog plan with id %s; %d errors occurred: %s", request.CatalogPlanID, len(compositeErr), compositeErr) + if compositeErr.Len() != 0 { + return errors.Wrapf(compositeErr, "error while updating access for catalog plan with id %s; %d errors occurred: %s", request.CatalogPlanID, compositeErr.Len(), compositeErr) } } else { if err := pc.updatePlan(plan, isEnabled); err != nil { diff --git a/cf/service_access_test.go b/cf/service_access_test.go index d2c7b34..eadec22 100644 --- a/cf/service_access_test.go +++ b/cf/service_access_test.go @@ -477,7 +477,7 @@ var _ = Describe("Client Service Plan Access", func() { getPlansRoute.reaction.Code = ccResponseErrCode }) - It("returns an error", assertFunc(&orgData, &planGUID, &brokerGUID, &ccResponseErrBody)) + It("returns an error", assertFunc(&orgData, &planGUID, &brokerGUID, fmt.Errorf(ccResponseErrBody.Description))) }) Context("when no plan is found", func() { diff --git a/cf/service_visibilities.go b/cf/service_visibilities.go index 787c2b9..0426c9f 100644 --- a/cf/service_visibilities.go +++ b/cf/service_visibilities.go @@ -7,6 +7,8 @@ import ( "strings" "sync" + "github.com/Peripli/service-broker-proxy/pkg/sbproxy/reconcile" + "github.com/Peripli/service-manager/pkg/log" "github.com/pkg/errors" @@ -113,7 +115,7 @@ func (pc *PlatformClient) GetVisibilitiesByBrokers(ctx context.Context, brokerNa } func (pc *PlatformClient) getBrokersByName(ctx context.Context, names []string) ([]cfclient.ServiceBroker, error) { - var errorOccured error + errorOccured := &reconcile.CompositeError{} var mutex sync.Mutex var wg sync.WaitGroup wgLimitChannel := make(chan struct{}, pc.settings.Reconcile.MaxParallelRequests) @@ -138,27 +140,24 @@ func (pc *PlatformClient) getBrokersByName(ctx context.Context, names []string) query := queryBuilder{} query.set("name", brokerNames) brokers, err := pc.client.ListServiceBrokersByQuery(query.build(ctx)) - mutex.Lock() defer mutex.Unlock() if err != nil { - if errorOccured == nil { - errorOccured = err - } - } else if errorOccured == nil { + errorOccured.Add(err) + } else if errorOccured.Len() == 0 { result = append(result, brokers...) } }(chunk) } wg.Wait() - if errorOccured != nil { + if errorOccured.Len() != 0 { return nil, errorOccured } return result, nil } func (pc *PlatformClient) getServicesByBrokers(ctx context.Context, brokers []cfclient.ServiceBroker) ([]cfclient.Service, error) { - var errorOccured error + errorOccured := &reconcile.CompositeError{} var mutex sync.Mutex var wg sync.WaitGroup wgLimitChannel := make(chan struct{}, pc.settings.Reconcile.MaxParallelRequests) @@ -183,20 +182,17 @@ func (pc *PlatformClient) getServicesByBrokers(ctx context.Context, brokers []cf brokerGUIDs = append(brokerGUIDs, broker.Guid) } brokers, err := pc.getServicesByBrokerGUIDs(ctx, brokerGUIDs) - mutex.Lock() defer mutex.Unlock() if err != nil { - if errorOccured == nil { - errorOccured = err - } - } else if errorOccured == nil { + errorOccured.Add(err) + } else if errorOccured.Len() == 0 { result = append(result, brokers...) } }(chunk) } wg.Wait() - if errorOccured != nil { + if errorOccured.Len() != 0 { return nil, errorOccured } return result, nil @@ -209,7 +205,7 @@ func (pc *PlatformClient) getServicesByBrokerGUIDs(ctx context.Context, brokerGU } func (pc *PlatformClient) getPlansByServices(ctx context.Context, services []cfclient.Service) ([]cfclient.ServicePlan, error) { - var errorOccured error + errorOccured := &reconcile.CompositeError{} var mutex sync.Mutex var wg sync.WaitGroup wgLimitChannel := make(chan struct{}, pc.settings.Reconcile.MaxParallelRequests) @@ -234,20 +230,17 @@ func (pc *PlatformClient) getPlansByServices(ctx context.Context, services []cfc serviceGUIDs = append(serviceGUIDs, service.Guid) } plans, err := pc.getPlansByServiceGUIDs(ctx, serviceGUIDs) - mutex.Lock() defer mutex.Unlock() if err != nil { - if errorOccured == nil { - errorOccured = err - } - } else if errorOccured == nil { + errorOccured.Add(err) + } else if errorOccured.Len() == 0 { result = append(result, plans...) } }(chunk) } wg.Wait() - if errorOccured != nil { + if errorOccured.Len() != 0 { return nil, errorOccured } return result, nil @@ -261,7 +254,7 @@ func (pc *PlatformClient) getPlansByServiceGUIDs(ctx context.Context, serviceGUI func (pc *PlatformClient) getPlansVisibilities(ctx context.Context, plans []cfclient.ServicePlan) ([]cfclient.ServicePlanVisibility, error) { var result []cfclient.ServicePlanVisibility - var errorOccured error + errorOccured := &reconcile.CompositeError{} var wg sync.WaitGroup var mutex sync.Mutex wgLimitChannel := make(chan struct{}, pc.settings.Reconcile.MaxParallelRequests) @@ -286,21 +279,17 @@ func (pc *PlatformClient) getPlansVisibilities(ctx context.Context, plans []cfcl plansGUID = append(plansGUID, p.Guid) } visibilities, err := pc.getPlanVisibilitiesByPlanGUID(ctx, plansGUID) - mutex.Lock() defer mutex.Unlock() - if err != nil { - if errorOccured == nil { - errorOccured = err - } - } else if errorOccured == nil { + errorOccured.Add(err) + } else if errorOccured.Len() == 0 { result = append(result, visibilities...) } }(chunk) } wg.Wait() - if errorOccured != nil { + if errorOccured.Len() != 0 { return nil, errorOccured } return result, nil