Performance improvements for visibility management flows (#485)

cmd/smgen/storage_type_template.go

@@ -46,7 +46,7 @@ func (*{{.Type}}) TableName() string {
 	return {{.Type}}Table
 }
 
-func (e *{{.Type}}) NewLabel(id, key, value string) storage.Label {
+func (e *{{.Type}}) NewLabel(id, entityID, key, value string) storage.Label {
 	now := pq.NullTime{
 		Time:  time.Now(),
 		Valid: true,
@@ -59,7 +59,7 @@ func (e *{{.Type}}) NewLabel(id, key, value string) storage.Label {
 			CreatedAt: now,
 			UpdatedAt: now,
 		},
-		{{.Type}}ID: sql.NullString{String: e.ID, Valid: e.ID != ""},
+		{{.Type}}ID: sql.NullString{String: entityID, Valid: entityID != ""},
 	}
 }
 

storage/encrypting_repository.go

@@ -201,6 +201,10 @@ func (er *encryptingRepository) Update(ctx context.Context, obj types.Object, la
 	return updatedObj, nil
 }
 
+func (cr *encryptingRepository) UpdateLabels(ctx context.Context, objectType types.ObjectType, objectID string, labelChanges types.LabelChanges, criteria ...query.Criterion) error {
+	return cr.repository.UpdateLabels(ctx, objectType, objectID, labelChanges, criteria...)
+}
+
 func (er *encryptingRepository) DeleteReturning(ctx context.Context, objectType types.ObjectType, criteria ...query.Criterion) (types.ObjectList, error) {
 	objList, err := er.repository.DeleteReturning(ctx, objectType, criteria...)
 	if err != nil {

storage/encrypting_repository_test.go

@@ -364,6 +364,17 @@ var _ = Describe("Encrypting Repository", func() {
 		})
 	})
 
+	Describe("UpdateLabels", func() {
+		It("does not invoke encryption or decryption and invokes the next in chain", func() {
+			delegateUpdateCallsCountBeforeOp := fakeRepository.UpdateLabelsCallCount()
+			err := repository.UpdateLabels(ctx, objWithDecryptedPassword.GetType(), objWithDecryptedPassword.GetID(), types.LabelChanges{})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(fakeEncrypter.EncryptCallCount() - encryptCallsCountBeforeOp).To(Equal(0))
+			Expect(fakeEncrypter.DecryptCallCount() - decryptCallsCountBeforeOp).To(Equal(0))
+			Expect(fakeRepository.UpdateLabelsCallCount() - delegateUpdateCallsCountBeforeOp).To(Equal(1))
+		})
+	})
+
 	Describe("DeleteReturning", func() {
 		Context("when decrypting fails", func() {
 			It("returns an error", func() {

storage/integrity_repository.go

@@ -122,6 +122,10 @@ func (cr *integrityRepository) Update(ctx context.Context, obj types.Object, lab
 	return cr.repository.Update(ctx, obj, labelChanges, criteria...)
 }
 
+func (cr *integrityRepository) UpdateLabels(ctx context.Context, objectType types.ObjectType, objectID string, labelChanges types.LabelChanges, criteria ...query.Criterion) error {
+	return cr.repository.UpdateLabels(ctx, objectType, objectID, labelChanges, criteria...)
+}
+
 func (cr *integrityRepository) Count(ctx context.Context, objectType types.ObjectType, criteria ...query.Criterion) (int, error) {
 	return cr.repository.Count(ctx, objectType, criteria...)
 }

storage/integrity_repository_test.go

@@ -299,6 +299,16 @@ var _ = Describe("Integrity Repository", func() {
 		})
 	})
 
+	Describe("UpdateLabels", func() {
+		It("does not invoke integrity calculation or validation and invokes the next in chain", func() {
+			delegateUpdateCallsCountBeforeOp := fakeRepository.UpdateLabelsCallCount()
+			err := repository.UpdateLabels(ctx, object.GetType(), object.GetID(), types.LabelChanges{})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(len(fakeIntegrityProcessor.Invocations())).To(Equal(0))
+			Expect(fakeRepository.UpdateLabelsCallCount() - delegateUpdateCallsCountBeforeOp).To(Equal(1))
+		})
+	})
+
 	Describe("DeleteReturning", func() {
 		Context("when delegate call fails", func() {
 			It("returns an error", func() {
@@ -357,6 +367,12 @@ var _ = Describe("Integrity Repository", func() {
 					Expect(returnedObj.(security.IntegralObject).GetIntegrity()).To(Equal(randomIntegrity))
 					Expect(objectArg.(security.IntegralObject).GetIntegrity()).To(Equal(randomIntegrity))
 
+					// verify update labels
+					delegateUpdateLabelsCallsCountBeforeOp := fakeRepository.UpdateLabelsCallCount()
+					err = repository.UpdateLabels(ctx, object.GetType(), object.GetID(), types.LabelChanges{})
+					Expect(err).ToNot(HaveOccurred())
+					Expect(fakeRepository.UpdateLabelsCallCount() - delegateUpdateLabelsCallsCountBeforeOp).To(Equal(1))
+
 					// verify get
 					delegateGetCallsCountBeforeOp := fakeRepository.GetCallCount()
 					byID := query.ByField(query.EqualsOperator, "id", "id")

storage/interceptable_repository.go

@@ -340,6 +340,49 @@ func (ir *queryScopedInterceptableRepository) Update(ctx context.Context, obj ty
 	return updatedObj, nil
 }
 
+func (ir *queryScopedInterceptableRepository) UpdateLabels(ctx context.Context, objectType types.ObjectType, objectID string, labelChanges types.LabelChanges, criteria ...query.Criterion) error {
+	byID := query.ByField(query.EqualsOperator, "id", objectID)
+	obj, err := ir.repositoryInTransaction.Get(ctx, objectType, byID)
+	if err != nil {
+		return err
+	}
+
+	updateObjFunc := func(ctx context.Context, _ Repository, _, _ types.Object, labelChanges ...*types.LabelChange) (types.Object, error) {
+		err := ir.repositoryInTransaction.UpdateLabels(ctx, objectType, objectID, labelChanges, criteria...)
+		if err != nil {
+			return nil, err
+		}
+
+		operation, found := opcontext.Get(ctx)
+		if found && operation.ResourceID != objectID && operation.ID != objectID && objectType != types.OperationType {
+			operation.TransitiveResources = append(operation.TransitiveResources, &types.RelatedType{
+				ID:            objectID,
+				Type:          objectType,
+				OperationType: types.UPDATE,
+			})
+		}
+
+		return obj, nil
+	}
+
+	if updateOnTxFunc, found := ir.updateOnTxFuncs[objectType]; found {
+		delete(ir.updateOnTxFuncs, objectType)
+
+		_, err = updateOnTxFunc(updateObjFunc)(ctx, ir, obj, obj, labelChanges...)
+
+		ir.updateOnTxFuncs[objectType] = updateOnTxFunc
+
+	} else {
+		_, err = updateObjFunc(ctx, ir, obj, obj, labelChanges...)
+	}
+
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
 func (itr *InterceptableTransactionalRepository) InTransaction(ctx context.Context, f func(ctx context.Context, storage Repository) error) error {
 	createOnTxInterceptors, updateOnTxInterceptors, deleteOnTxInterceptors := itr.provideOnTxInterceptors()
 
@@ -645,6 +688,48 @@ func (itr *InterceptableTransactionalRepository) Update(ctx context.Context, obj
 	return obj, nil
 }
 
+func (itr *InterceptableTransactionalRepository) UpdateLabels(ctx context.Context, objectType types.ObjectType, objectID string, labelChanges types.LabelChanges, criteria ...query.Criterion) error {
+	providedCreateInterceptors, providedUpdateInterceptors, providedDeleteInterceptors := itr.provideInterceptors()
+
+	finalInterceptor := func(ctx context.Context, obj types.Object, labelChanges ...*types.LabelChange) (types.Object, error) {
+		var result types.Object
+		var err error
+
+		if err = itr.RawRepository.InTransaction(ctx, func(ctx context.Context, txStorage Repository) error {
+			interceptableRepository := newScopedRepositoryWithInterceptors(txStorage, providedCreateInterceptors, providedUpdateInterceptors, providedDeleteInterceptors)
+			err = interceptableRepository.UpdateLabels(ctx, objectType, objectID, labelChanges, criteria...)
+			if err != nil {
+				return err
+			}
+
+			return nil
+		}); err != nil {
+			return nil, err
+		}
+
+		return result, nil
+	}
+
+	var err error
+	byID := query.ByField(query.EqualsOperator, "id", objectID)
+	obj, err := itr.RawRepository.Get(ctx, objectType, byID)
+	if err != nil {
+		return err
+	}
+
+	if providedUpdateInterceptors[objectType] != nil {
+		_, err = providedUpdateInterceptors[objectType].AroundTxUpdate(finalInterceptor)(ctx, obj, labelChanges...)
+	} else {
+		_, err = finalInterceptor(ctx, obj, labelChanges...)
+	}
+
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
 func (itr *InterceptableTransactionalRepository) validateCreateProviders(objectType types.ObjectType, providerName string, order InterceptorOrder) {
 	var existingProviderNames []string
 	for _, existingProvider := range itr.createAroundTxProviders[objectType] {

storage/interceptable_repository_test.go

@@ -28,7 +28,7 @@ var _ = Describe("Interceptable TransactionalRepository", func() {
 	var fakeCreateInterceptor *storagefakes.FakeCreateInterceptor
 
 	var fakeUpdateAroundTxInterceptorProvider *storagefakes.FakeUpdateAroundTxInterceptorProvider
-	var fakeUpdateAroundTxIntercetptor *storagefakes.FakeUpdateAroundTxInterceptor
+	var fakeUpdateAroundTxInterceptor *storagefakes.FakeUpdateAroundTxInterceptor
 
 	var fakeUpdateOnTxInterceptorProvider *storagefakes.FakeUpdateOnTxInterceptorProvider
 	var fakeUpdateOnTxIntercetptor *storagefakes.FakeUpdateOnTxInterceptor
@@ -77,8 +77,8 @@ var _ = Describe("Interceptable TransactionalRepository", func() {
 			}
 		})
 
-		fakeUpdateAroundTxIntercetptor = &storagefakes.FakeUpdateAroundTxInterceptor{}
-		fakeUpdateAroundTxIntercetptor.AroundTxUpdateCalls(func(next storage.InterceptUpdateAroundTxFunc) storage.InterceptUpdateAroundTxFunc {
+		fakeUpdateAroundTxInterceptor = &storagefakes.FakeUpdateAroundTxInterceptor{}
+		fakeUpdateAroundTxInterceptor.AroundTxUpdateCalls(func(next storage.InterceptUpdateAroundTxFunc) storage.InterceptUpdateAroundTxFunc {
 			return func(ctx context.Context, obj types.Object, labelChanges ...*types.LabelChange) (object types.Object, e error) {
 				return next(ctx, obj, labelChanges...)
 			}
@@ -144,7 +144,7 @@ var _ = Describe("Interceptable TransactionalRepository", func() {
 
 		fakeUpdateAroundTxInterceptorProvider = &storagefakes.FakeUpdateAroundTxInterceptorProvider{}
 		fakeUpdateAroundTxInterceptorProvider.NameReturns("fakeUpdateAroundTxInterceptor")
-		fakeUpdateAroundTxInterceptorProvider.ProvideReturns(fakeUpdateAroundTxIntercetptor)
+		fakeUpdateAroundTxInterceptorProvider.ProvideReturns(fakeUpdateAroundTxInterceptor)
 
 		fakeUpdateOnTxInterceptorProvider = &storagefakes.FakeUpdateOnTxInterceptorProvider{}
 		fakeUpdateOnTxInterceptorProvider.NameReturns("fakeUpdateOnTxInterceptor")
@@ -217,7 +217,7 @@ var _ = Describe("Interceptable TransactionalRepository", func() {
 		Expect(fakeCreateInterceptor.AroundTxCreateCallCount()).To(Equal(0))
 		Expect(fakeCreateInterceptor.OnTxCreateCallCount()).To(Equal(0))
 
-		Expect(fakeUpdateAroundTxIntercetptor.AroundTxUpdateCallCount()).To(Equal(0))
+		Expect(fakeUpdateAroundTxInterceptor.AroundTxUpdateCallCount()).To(Equal(0))
 		Expect(fakeUpdateOnTxIntercetptor.OnTxUpdateCallCount()).To(Equal(0))
 		Expect(fakeUpdateIntercetptor.AroundTxUpdateCallCount()).To(Equal(0))
 		Expect(fakeUpdateIntercetptor.OnTxUpdateCallCount()).To(Equal(0))
@@ -258,7 +258,7 @@ var _ = Describe("Interceptable TransactionalRepository", func() {
 
 			Expect(err).ShouldNot(HaveOccurred())
 
-			Expect(fakeUpdateAroundTxIntercetptor.AroundTxUpdateCallCount()).To(Equal(1))
+			Expect(fakeUpdateAroundTxInterceptor.AroundTxUpdateCallCount()).To(Equal(1))
 			Expect(fakeUpdateOnTxIntercetptor.OnTxUpdateCallCount()).To(Equal(1))
 			Expect(fakeUpdateIntercetptor.AroundTxUpdateCallCount()).To(Equal(1))
 			Expect(fakeUpdateIntercetptor.OnTxUpdateCallCount()).To(Equal(1))
@@ -267,6 +267,20 @@ var _ = Describe("Interceptable TransactionalRepository", func() {
 		})
 	})
 
+	Describe("UpdateLabels", func() {
+		It("invokes all interceptors", func() {
+			err := interceptableRepository.UpdateLabels(ctx, types.ServiceBrokerType, "id", types.LabelChanges{})
+			Expect(err).ShouldNot(HaveOccurred())
+
+			Expect(fakeUpdateAroundTxInterceptor.AroundTxUpdateCallCount()).To(Equal(1))
+			Expect(fakeUpdateOnTxIntercetptor.OnTxUpdateCallCount()).To(Equal(1))
+			Expect(fakeUpdateIntercetptor.AroundTxUpdateCallCount()).To(Equal(1))
+			Expect(fakeUpdateIntercetptor.OnTxUpdateCallCount()).To(Equal(1))
+
+			Expect(fakeStorage.UpdateLabelsCallCount()).To(Equal(1))
+		})
+	})
+
 	Describe("Delete", func() {
 		It("invokes all interceptors", func() {
 			byID := query.ByField(query.EqualsOperator, "id", "id")
@@ -373,7 +387,7 @@ var _ = Describe("Interceptable TransactionalRepository", func() {
 				Expect(fakeDeleteInterceptor.OnTxDeleteCallCount()).To(Equal(0))
 
 				Expect(fakeCreateAroundTxInterceptor.AroundTxCreateCallCount()).To(Equal(0))
-				Expect(fakeUpdateAroundTxIntercetptor.AroundTxUpdateCallCount()).To(Equal(0))
+				Expect(fakeUpdateAroundTxInterceptor.AroundTxUpdateCallCount()).To(Equal(0))
 				Expect(fakeDeleteAroundTxInterceptor.AroundTxDeleteCallCount()).To(Equal(0))
 
 				Expect(fakeStorage.CreateCallCount()).To(Equal(executionsCount))
@@ -445,7 +459,7 @@ var _ = Describe("Interceptable TransactionalRepository", func() {
 			Expect(fakeDeleteInterceptor.OnTxDeleteCallCount()).To(Equal(0))
 
 			Expect(fakeCreateAroundTxInterceptor.AroundTxCreateCallCount()).To(Equal(0))
-			Expect(fakeUpdateAroundTxIntercetptor.AroundTxUpdateCallCount()).To(Equal(0))
+			Expect(fakeUpdateAroundTxInterceptor.AroundTxUpdateCallCount()).To(Equal(0))
 			Expect(fakeDeleteAroundTxInterceptor.AroundTxDeleteCallCount()).To(Equal(0))
 
 			Expect(fakeStorage.CreateCallCount()).To(Equal(3))

storage/interfaces.go

@@ -38,8 +38,7 @@ type Entity interface {
 	GetID() string
 	ToObject() (types.Object, error)
 	FromObject(object types.Object) (Entity, error)
-	BuildLabels(labels types.Labels, newLabel func(id, key, value string) Label) ([]Label, error)
-	NewLabel(id, key, value string) Label
+	NewLabel(id, entityID, key, value string) Label
 }
 
 type Label interface {
@@ -190,6 +189,9 @@ type Repository interface {
 
 	// Update updates an object from SM DB
 	Update(ctx context.Context, obj types.Object, labelChanges types.LabelChanges, criteria ...query.Criterion) (types.Object, error)
+
+	// UpdateLabels updates the object labels in SM DB
+	UpdateLabels(ctx context.Context, objectType types.ObjectType, objectID string, labelChanges types.LabelChanges, _ ...query.Criterion) error
 }
 
 // TransactionalRepository is a storage repository that can initiate a transaction

storage/postgres/abstract.go

@@ -224,13 +224,6 @@ func checkRowsAffected(ctx context.Context, result sql.Result) error {
 	return nil
 }
 
-func checkSQLNoRows(err error) error {
-	if err == sql.ErrNoRows {
-		return util.ErrNotFoundInStorage
-	}
-	return err
-}
-
 func toNullString(s string) sql.NullString {
 	return sql.NullString{
 		String: s,

storage/postgres/base_entity.go

@@ -18,12 +18,8 @@ package postgres
 
 import (
 	"database/sql"
-	"fmt"
 	"time"
 
-	"github.com/Peripli/service-manager/pkg/types"
-	"github.com/Peripli/service-manager/storage"
-	"github.com/gofrs/uuid"
 	"github.com/lib/pq"
 )
 
@@ -39,21 +35,6 @@ func (e *BaseEntity) GetID() string {
 	return e.ID
 }
 
-func (e *BaseEntity) BuildLabels(labels types.Labels, newLabel func(id, key, value string) storage.Label) ([]storage.Label, error) {
-	var result []storage.Label
-	for key, values := range labels {
-		for _, labelValue := range values {
-			UUID, err := uuid.NewV4()
-			if err != nil {
-				return nil, fmt.Errorf("could not generate GUID for broker label: %s", err)
-			}
-			result = append(result, newLabel(UUID.String(), key, labelValue))
-		}
-	}
-
-	return result, nil
-}
-
 type BaseLabelEntity struct {
 	ID        sql.NullString `db:"id"`
 	Key       sql.NullString `db:"key"`

storage/postgres/broker_gen.go

@@ -24,7 +24,7 @@ func (*Broker) TableName() string {
 	return BrokerTable
 }
 
-func (e *Broker) NewLabel(id, key, value string) storage.Label {
+func (e *Broker) NewLabel(id, entityID, key, value string) storage.Label {
 	now := pq.NullTime{
 		Time:  time.Now(),
 		Valid: true,
@@ -37,7 +37,7 @@ func (e *Broker) NewLabel(id, key, value string) storage.Label {
 			CreatedAt: now,
 			UpdatedAt: now,
 		},
-		BrokerID: sql.NullString{String: e.ID, Valid: e.ID != ""},
+		BrokerID: sql.NullString{String: entityID, Valid: entityID != ""},
 	}
 }
 

storage/postgres/brokerplatformcredential_gen.go

@@ -24,7 +24,7 @@ func (*BrokerPlatformCredential) TableName() string {
 	return BrokerPlatformCredentialTable
 }
 
-func (e *BrokerPlatformCredential) NewLabel(id, key, value string) storage.Label {
+func (e *BrokerPlatformCredential) NewLabel(id, entityID, key, value string) storage.Label {
 	now := pq.NullTime{
 		Time:  time.Now(),
 		Valid: true,
@@ -37,7 +37,7 @@ func (e *BrokerPlatformCredential) NewLabel(id, key, value string) storage.Label
 			CreatedAt: now,
 			UpdatedAt: now,
 		},
-		BrokerPlatformCredentialID: sql.NullString{String: e.ID, Valid: e.ID != ""},
+		BrokerPlatformCredentialID: sql.NullString{String: entityID, Valid: entityID != ""},
 	}
 }
 

storage/postgres/interfaces.go

@@ -47,7 +47,7 @@ type EntityLabelRow interface {
 	PostgresLabel
 }
 
-func validateLabels(entities []storage.Label) error {
+func validateLabels(entities []PostgresLabel) error {
 	pairs := make(map[string][]string)
 	for _, bl := range entities {
 		newKey := bl.GetKey()

storage/postgres/keystore.go

@@ -56,11 +56,7 @@ func (s *Safe) FromObject(object types.Object) (storage.Entity, error) {
 	return nil, nil
 }
 
-func (s *Safe) BuildLabels(labels types.Labels, newLabel func(id, key, value string) storage.Label) ([]storage.Label, error) {
-	return nil, nil
-}
-
-func (s *Safe) NewLabel(id, key, value string) storage.Label {
+func (s *Safe) NewLabel(id, entityID, key, value string) storage.Label {
 	return nil
 }