Resolve comments

cache/cache.go

@@ -4,6 +4,7 @@ import (
 	"sync"
 
 	"github.com/aserto-dev/azm/model"
+	"github.com/aserto-dev/azm/model/diff"
 )
 
 type Cache struct {
@@ -27,12 +28,11 @@ func (c *Cache) UpdateModel(m *model.Model) error {
 	return nil
 }
 
-// Returns a copy of the current model.
-func (c *Cache) GetModel() model.Model {
+// Returns a diff struct resulted between the old and the new model.
+func (c *Cache) Diff(other *model.Model) *diff.Diff {
 	c.mtx.Lock()
 	defer c.mtx.Unlock()
-	m := *c.model
-	return m
+	return c.model.Diff(other)
 }
 
 // ObjectExists, checks if given object type name exists in the model cache.

go.mod

@@ -20,6 +20,8 @@ require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/golang/protobuf v1.5.3 // indirect
 	github.com/grpc-ecosystem/grpc-gateway/v2 v2.18.0 // indirect
+	github.com/hashicorp/errwrap v1.0.0 // indirect
+	github.com/hashicorp/go-multierror v1.1.1 // indirect
 	github.com/mattn/go-colorable v0.1.13 // indirect
 	github.com/mattn/go-isatty v0.0.19 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect

go.sum

@@ -14,6 +14,10 @@ github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
 github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
 github.com/grpc-ecosystem/grpc-gateway/v2 v2.18.0 h1:RtRsiaGvWxcwd8y3BiRZxsylPT8hLWZ5SPcfI+3IDNk=
 github.com/grpc-ecosystem/grpc-gateway/v2 v2.18.0/go.mod h1:TzP6duP4Py2pHLVPPQp42aoYI92+PCrVotyR5e8Vqlk=
+github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA=
+github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
+github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo=
+github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM=
 github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/magefile/mage v1.15.0 h1:BvGheCMAsG3bWUDbZ8AyXXpCNwU9u5CB6sM+HNb9HYg=

model/diff/diff.go

@@ -4,10 +4,11 @@ import (
 	"context"
 
 	"github.com/aserto-dev/go-directory/pkg/derr"
+	"github.com/hashicorp/go-multierror"
 	"github.com/pkg/errors"
 )
 
-//go:generate go run github.com/golang/mock/mockgen -destination=mock_directory_validator.go  -package=diff github.com/aserto-dev/azm/model/diff DirectoryValidator
+//go:generate go run github.com/golang/mock/mockgen -destination=mock_instances.go  -package=diff github.com/aserto-dev/azm/model/diff Instances
 
 type Diff struct {
 	Added   Changes
@@ -19,47 +20,52 @@ type Changes struct {
 	Relations map[string][]string
 }
 
-type DirectoryValidator interface {
-	HasObjectInstances(ctx context.Context, objectType string) (bool, error)
-	HasRelationInstances(ctx context.Context, objectType, relationName string) (bool, error)
+type Instances interface {
+	ObjectsExist(ctx context.Context, objectType string) (bool, error)
+	RelationsExist(ctx context.Context, objectType, relationName string) (bool, error)
 }
 
-func (d *Diff) Validate(ctx context.Context, dv DirectoryValidator) error {
+func (d *Diff) Validate(ctx context.Context, dv Instances) error {
+	var errs error
 	if err := d.validateObjectTypes(ctx, dv); err != nil {
-		return err
+		errs = multierror.Append(errs, err)
 	}
 
 	if err := d.validateRelationsTypes(ctx, dv); err != nil {
-		return err
+		errs = multierror.Append(errs, err)
 	}
 
-	return nil
+	return errs
 }
 
-func (d *Diff) validateObjectTypes(ctx context.Context, dv DirectoryValidator) error {
+func (d *Diff) validateObjectTypes(ctx context.Context, dv Instances) error {
+	var errs error
 	for _, objType := range d.Removed.Objects {
-		hasInstance, err := dv.HasObjectInstances(ctx, objType)
+		hasInstance, err := dv.ObjectsExist(ctx, objType)
 		if err != nil {
-			return err
+			errs = multierror.Append(errs, err)
+			continue
 		}
 		if hasInstance {
-			return errors.Wrapf(derr.ErrObjectTypeInUse, "object type: %s", objType)
+			errs = multierror.Append(errs, errors.Wrapf(derr.ErrObjectTypeInUse, "object type: %s", objType))
 		}
 	}
-	return nil
+	return errs
 }
 
-func (d *Diff) validateRelationsTypes(ctx context.Context, dv DirectoryValidator) error {
+func (d *Diff) validateRelationsTypes(ctx context.Context, dv Instances) error {
+	var errs error
 	for objType, rels := range d.Removed.Relations {
 		for _, rel := range rels {
-			hasInstance, err := dv.HasRelationInstances(ctx, objType, rel)
+			hasInstance, err := dv.RelationsExist(ctx, objType, rel)
 			if err != nil {
-				return err
+				errs = multierror.Append(errs, err)
+				continue
 			}
 			if hasInstance {
-				return errors.Wrapf(derr.ErrRelationTypeInUse, "relation type: %s; object type: %s", rel, objType)
+				errs = multierror.Append(errs, errors.Wrapf(derr.ErrRelationTypeInUse, "relation type: %s; object type: %s", rel, objType))
 			}
 		}
 	}
-	return nil
+	return errs
 }

model/diff/diff_test.go

@@ -2,6 +2,7 @@ package diff_test
 
 import (
 	"context"
+	"errors"
 	"testing"
 
 	"github.com/aserto-dev/azm/model/diff"
@@ -12,7 +13,7 @@ import (
 
 func TestValidateDiffNoDeletion(t *testing.T) {
 	ctrl := gomock.NewController(t)
-	mockDirectoryValidator := diff.NewMockDirectoryValidator(ctrl)
+	mockDirectoryValidator := diff.NewMockInstances(ctrl)
 
 	dif := diff.Diff{Removed: diff.Changes{}, Added: diff.Changes{}}
 	err := dif.Validate(context.Background(), mockDirectoryValidator)
@@ -22,28 +23,28 @@ func TestValidateDiffNoDeletion(t *testing.T) {
 
 func TestValidateDiffWithObjectTypeDeletion(t *testing.T) {
 	ctrl := gomock.NewController(t)
-	mockDirectoryValidator := diff.NewMockDirectoryValidator(ctrl)
+	mockDirectoryValidator := diff.NewMockInstances(ctrl)
 	objType := "user"
 	bCtx := context.Background()
 
 	dif := diff.Diff{Removed: diff.Changes{Objects: []string{objType}}, Added: diff.Changes{}}
 
-	mockDirectoryValidator.EXPECT().HasObjectInstances(bCtx, objType).Return(false, nil)
+	mockDirectoryValidator.EXPECT().ObjectsExist(bCtx, objType).Return(false, nil)
 	err := dif.Validate(bCtx, mockDirectoryValidator)
 
 	require.NoError(t, err)
 }
 
 func TestValidateDiffWith2ObjectTypeDeletion(t *testing.T) {
 	ctrl := gomock.NewController(t)
-	mockDirectoryValidator := diff.NewMockDirectoryValidator(ctrl)
+	mockDirectoryValidator := diff.NewMockInstances(ctrl)
 	objTypes := []string{"user", "member"}
 	bCtx := context.Background()
 
 	dif := diff.Diff{Removed: diff.Changes{Objects: objTypes}, Added: diff.Changes{}}
 
-	mockDirectoryValidator.EXPECT().HasObjectInstances(bCtx, objTypes[0]).Return(false, nil)
-	mockDirectoryValidator.EXPECT().HasObjectInstances(bCtx, objTypes[1]).Return(true, nil)
+	mockDirectoryValidator.EXPECT().ObjectsExist(bCtx, objTypes[0]).Return(false, nil)
+	mockDirectoryValidator.EXPECT().ObjectsExist(bCtx, objTypes[1]).Return(true, nil)
 	err := dif.Validate(bCtx, mockDirectoryValidator)
 
 	require.Error(t, err)
@@ -52,18 +53,38 @@ func TestValidateDiffWith2ObjectTypeDeletion(t *testing.T) {
 
 func TestValidateDiffWithRelationTypeDeletion(t *testing.T) {
 	ctrl := gomock.NewController(t)
-	mockDirectoryValidator := diff.NewMockDirectoryValidator(ctrl)
+	mockDirectoryValidator := diff.NewMockInstances(ctrl)
 	objTypes := []string{"user", "member"}
 	relationTypes := map[string][]string{"folder": {"parent_folder"}}
 	bCtx := context.Background()
 
 	dif := diff.Diff{Removed: diff.Changes{Objects: objTypes, Relations: relationTypes}, Added: diff.Changes{}}
 
-	mockDirectoryValidator.EXPECT().HasObjectInstances(bCtx, objTypes[0]).Return(false, nil)
-	mockDirectoryValidator.EXPECT().HasObjectInstances(bCtx, objTypes[1]).Return(false, nil)
-	mockDirectoryValidator.EXPECT().HasRelationInstances(bCtx, "folder", relationTypes["folder"][0]).Return(true, nil)
+	mockDirectoryValidator.EXPECT().ObjectsExist(bCtx, objTypes[0]).Return(false, nil)
+	mockDirectoryValidator.EXPECT().ObjectsExist(bCtx, objTypes[1]).Return(false, nil)
+	mockDirectoryValidator.EXPECT().RelationsExist(bCtx, "folder", relationTypes["folder"][0]).Return(true, nil)
 	err := dif.Validate(bCtx, mockDirectoryValidator)
 
 	require.Error(t, err)
 	require.Contains(t, err.Error(), derr.ErrRelationTypeInUse.Message)
 }
+
+func TestValidateDiffWithObjectInstances(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	mockDirectoryValidator := diff.NewMockInstances(ctrl)
+	objTypes := []string{"user", "member"}
+	relationTypes := map[string][]string{"folder": {"parent_folder"}}
+	bCtx := context.Background()
+
+	dif := diff.Diff{Removed: diff.Changes{Objects: objTypes, Relations: relationTypes}, Added: diff.Changes{}}
+
+	mockDirectoryValidator.EXPECT().ObjectsExist(bCtx, objTypes[0]).Return(false, errors.New("Boom!"))
+	mockDirectoryValidator.EXPECT().ObjectsExist(bCtx, objTypes[1]).Return(true, nil)
+	mockDirectoryValidator.EXPECT().RelationsExist(bCtx, "folder", relationTypes["folder"][0]).Return(true, nil)
+	err := dif.Validate(bCtx, mockDirectoryValidator)
+
+	require.Error(t, err)
+	require.Contains(t, err.Error(), derr.ErrRelationTypeInUse.Message)
+	require.Contains(t, err.Error(), derr.ErrObjectTypeInUse.Message)
+	require.Contains(t, err.Error(), "Boom!")
+}