From b0bc6bd63215d6f2a45a852e3c0912e9b8983611 Mon Sep 17 00:00:00 2001 From: Jim Date: Wed, 24 Aug 2022 14:11:49 -0400 Subject: [PATCH] refactor: eliminate gorm references in favor of go-dbw (#2386) --- go.mod | 2 +- internal/auth/oidc/repository_account.go | 4 +- .../oidc/repository_auth_method_update.go | 4 +- .../auth/oidc/repository_managed_group.go | 4 +- internal/auth/password/repository_account.go | 4 +- .../auth/password/repository_authmethod.go | 4 +- .../static/repository_credential.go | 6 +- .../static/repository_credential_store.go | 4 +- .../vault/repository_credential_library.go | 4 +- .../vault/repository_credential_store.go | 6 +- internal/db/common/update.go | 173 ----- internal/db/common/update_test.go | 642 ------------------ internal/host/static/repository_host.go | 4 +- internal/host/static/repository_host_set.go | 4 +- internal/iam/repository_group.go | 4 +- internal/iam/repository_role.go | 4 +- internal/iam/repository_scope.go | 4 +- internal/iam/repository_user.go | 4 +- internal/oplog/queue.go | 3 +- internal/server/repository_worker.go | 4 +- internal/target/repository.go | 4 +- 21 files changed, 38 insertions(+), 854 deletions(-) delete mode 100644 internal/db/common/update.go delete mode 100644 internal/db/common/update_test.go diff --git a/go.mod b/go.mod index f7dc7ebbe..ddc256423 100644 --- a/go.mod +++ b/go.mod @@ -82,7 +82,7 @@ require ( google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.1.0 google.golang.org/protobuf v1.28.1 gorm.io/driver/postgres v1.3.8 - gorm.io/gorm v1.23.8 + gorm.io/gorm v1.23.8 // indirect mvdan.cc/gofumpt v0.3.1 nhooyr.io/websocket v1.8.7 ) diff --git a/internal/auth/oidc/repository_account.go b/internal/auth/oidc/repository_account.go index 1f6ed7ffa..2d68acf8f 100644 --- a/internal/auth/oidc/repository_account.go +++ b/internal/auth/oidc/repository_account.go @@ -6,10 +6,10 @@ import ( "strings" "github.com/hashicorp/boundary/internal/db" - dbcommon "github.com/hashicorp/boundary/internal/db/common" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/oplog" + "github.com/hashicorp/go-dbw" ) // CreateAccount inserts an Account, a, into the repository and returns a @@ -231,7 +231,7 @@ func (r *Repository) UpdateAccount(ctx context.Context, scopeId string, a *Accou } } var dbMask, nullFields []string - dbMask, nullFields = dbcommon.BuildUpdatePaths( + dbMask, nullFields = dbw.BuildUpdatePaths( map[string]interface{}{ NameField: a.Name, DescriptionField: a.Description, diff --git a/internal/auth/oidc/repository_auth_method_update.go b/internal/auth/oidc/repository_auth_method_update.go index 032262474..39a0728b2 100644 --- a/internal/auth/oidc/repository_auth_method_update.go +++ b/internal/auth/oidc/repository_auth_method_update.go @@ -7,10 +7,10 @@ import ( "strings" "github.com/hashicorp/boundary/internal/db" - dbcommon "github.com/hashicorp/boundary/internal/db/common" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/oplog" + "github.com/hashicorp/go-dbw" "github.com/hashicorp/go-multierror" "github.com/hashicorp/go-secure-stdlib/strutil" ) @@ -91,7 +91,7 @@ func (r *Repository) UpdateAuthMethod(ctx context.Context, am *AuthMethod, versi return nil, db.NoRowsAffected, errors.Wrap(ctx, err, op) } - dbMask, nullFields := dbcommon.BuildUpdatePaths( + dbMask, nullFields := dbw.BuildUpdatePaths( map[string]interface{}{ NameField: am.Name, DescriptionField: am.Description, diff --git a/internal/auth/oidc/repository_managed_group.go b/internal/auth/oidc/repository_managed_group.go index 606b5ed0c..1e64f9833 100644 --- a/internal/auth/oidc/repository_managed_group.go +++ b/internal/auth/oidc/repository_managed_group.go @@ -6,10 +6,10 @@ import ( "strings" "github.com/hashicorp/boundary/internal/db" - dbcommon "github.com/hashicorp/boundary/internal/db/common" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/oplog" + "github.com/hashicorp/go-dbw" ) // CreateManagedGroup inserts an ManagedGroup, mg, into the repository and @@ -197,7 +197,7 @@ func (r *Repository) UpdateManagedGroup(ctx context.Context, scopeId string, mg } } var dbMask, nullFields []string - dbMask, nullFields = dbcommon.BuildUpdatePaths( + dbMask, nullFields = dbw.BuildUpdatePaths( map[string]interface{}{ NameField: mg.Name, DescriptionField: mg.Description, diff --git a/internal/auth/password/repository_account.go b/internal/auth/password/repository_account.go index 4bd0beb07..5e5c7f7cc 100644 --- a/internal/auth/password/repository_account.go +++ b/internal/auth/password/repository_account.go @@ -7,11 +7,11 @@ import ( "strings" "github.com/hashicorp/boundary/internal/db" - dbcommon "github.com/hashicorp/boundary/internal/db/common" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/intglobals" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/oplog" + "github.com/hashicorp/go-dbw" ) // CreateAccount inserts a into the repository and returns a new Account @@ -262,7 +262,7 @@ func (r *Repository) UpdateAccount(ctx context.Context, scopeId string, a *Accou } } var dbMask, nullFields []string - dbMask, nullFields = dbcommon.BuildUpdatePaths( + dbMask, nullFields = dbw.BuildUpdatePaths( map[string]interface{}{ "Name": a.Name, "Description": a.Description, diff --git a/internal/auth/password/repository_authmethod.go b/internal/auth/password/repository_authmethod.go index 2433f2b29..1bf617758 100644 --- a/internal/auth/password/repository_authmethod.go +++ b/internal/auth/password/repository_authmethod.go @@ -6,10 +6,10 @@ import ( "strings" "github.com/hashicorp/boundary/internal/db" - dbcommon "github.com/hashicorp/boundary/internal/db/common" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/oplog" + "github.com/hashicorp/go-dbw" ) // CreateAuthMethod inserts m into the repository and returns a new @@ -198,7 +198,7 @@ func (r *Repository) UpdateAuthMethod(ctx context.Context, authMethod *AuthMetho } } var dbMask, nullFields []string - dbMask, nullFields = dbcommon.BuildUpdatePaths( + dbMask, nullFields = dbw.BuildUpdatePaths( map[string]interface{}{ "Name": authMethod.Name, "Description": authMethod.Description, diff --git a/internal/credential/static/repository_credential.go b/internal/credential/static/repository_credential.go index 9d836a3b7..74e0bebca 100644 --- a/internal/credential/static/repository_credential.go +++ b/internal/credential/static/repository_credential.go @@ -7,11 +7,11 @@ import ( "github.com/hashicorp/boundary/internal/credential" "github.com/hashicorp/boundary/internal/db" - dbcommon "github.com/hashicorp/boundary/internal/db/common" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/oplog" "github.com/hashicorp/boundary/internal/types/subtypes" + "github.com/hashicorp/go-dbw" ) // CreateUsernamePasswordCredential inserts c into the repository and returns a new @@ -287,7 +287,7 @@ func (r *Repository) UpdateUsernamePasswordCredential(ctx context.Context, return nil, db.NoRowsAffected, errors.New(ctx, errors.InvalidFieldMask, op, f) } } - dbMask, nullFields := dbcommon.BuildUpdatePaths( + dbMask, nullFields := dbw.BuildUpdatePaths( map[string]interface{}{ nameField: c.Name, descriptionField: c.Description, @@ -403,7 +403,7 @@ func (r *Repository) UpdateSshPrivateKeyCredential(ctx context.Context, return nil, db.NoRowsAffected, errors.New(ctx, errors.InvalidFieldMask, op, f) } } - dbMask, nullFields := dbcommon.BuildUpdatePaths( + dbMask, nullFields := dbw.BuildUpdatePaths( map[string]interface{}{ nameField: c.Name, descriptionField: c.Description, diff --git a/internal/credential/static/repository_credential_store.go b/internal/credential/static/repository_credential_store.go index 7ff5037d5..11e0ca9c5 100644 --- a/internal/credential/static/repository_credential_store.go +++ b/internal/credential/static/repository_credential_store.go @@ -6,10 +6,10 @@ import ( "strings" "github.com/hashicorp/boundary/internal/db" - dbcommon "github.com/hashicorp/boundary/internal/db/common" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/oplog" + "github.com/hashicorp/go-dbw" ) // CreateCredentialStore inserts cs into the repository and returns a new @@ -124,7 +124,7 @@ func (r *Repository) UpdateCredentialStore(ctx context.Context, cs *CredentialSt return nil, db.NoRowsAffected, errors.New(ctx, errors.InvalidFieldMask, op, f) } } - dbMask, nullFields := dbcommon.BuildUpdatePaths( + dbMask, nullFields := dbw.BuildUpdatePaths( map[string]interface{}{ nameField: cs.Name, descriptionField: cs.Description, diff --git a/internal/credential/vault/repository_credential_library.go b/internal/credential/vault/repository_credential_library.go index c27659d13..d35ca547a 100644 --- a/internal/credential/vault/repository_credential_library.go +++ b/internal/credential/vault/repository_credential_library.go @@ -7,11 +7,11 @@ import ( "github.com/hashicorp/boundary/internal/credential" "github.com/hashicorp/boundary/internal/db" - dbcommon "github.com/hashicorp/boundary/internal/db/common" "github.com/hashicorp/boundary/internal/db/timestamp" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/oplog" + "github.com/hashicorp/go-dbw" "github.com/hashicorp/go-secure-stdlib/strutil" ) @@ -157,7 +157,7 @@ func (r *Repository) UpdateCredentialLibrary(ctx context.Context, projectId stri } } var dbMask, nullFields []string - dbMask, nullFields = dbcommon.BuildUpdatePaths( + dbMask, nullFields = dbw.BuildUpdatePaths( map[string]interface{}{ nameField: l.Name, descriptionField: l.Description, diff --git a/internal/credential/vault/repository_credential_store.go b/internal/credential/vault/repository_credential_store.go index 5f50eb830..f66773544 100644 --- a/internal/credential/vault/repository_credential_store.go +++ b/internal/credential/vault/repository_credential_store.go @@ -6,12 +6,12 @@ import ( "strings" "github.com/hashicorp/boundary/internal/db" - dbcommon "github.com/hashicorp/boundary/internal/db/common" "github.com/hashicorp/boundary/internal/db/timestamp" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/oplog" "github.com/hashicorp/boundary/internal/scheduler" + "github.com/hashicorp/go-dbw" "github.com/hashicorp/go-secure-stdlib/parseutil" vault "github.com/hashicorp/vault/api" ) @@ -385,7 +385,7 @@ func (r *Repository) UpdateCredentialStore(ctx context.Context, cs *CredentialSt return nil, db.NoRowsAffected, errors.New(ctx, errors.InvalidFieldMask, op, f) } } - dbMask, nullFields := dbcommon.BuildUpdatePaths( + dbMask, nullFields := dbw.BuildUpdatePaths( map[string]interface{}{ nameField: cs.Name, descriptionField: cs.Description, @@ -407,7 +407,7 @@ func (r *Repository) UpdateCredentialStore(ctx context.Context, cs *CredentialSt clientCert = cs.ClientCertificate().GetCertificate() clientCertKey = cs.ClientCertificate().GetCertificateKey() } - certDbMask, certNullFields := dbcommon.BuildUpdatePaths( + certDbMask, certNullFields := dbw.BuildUpdatePaths( map[string]interface{}{ certificateField: clientCert, certificateKeyField: clientCertKey, diff --git a/internal/db/common/update.go b/internal/db/common/update.go deleted file mode 100644 index 2a76a6f59..000000000 --- a/internal/db/common/update.go +++ /dev/null @@ -1,173 +0,0 @@ -// common package contains functions from internal/db which need to be shared -// commonly with other packages that have a cyclic dependency on internal/db -// like internal/oplog. -package common - -import ( - "fmt" - "reflect" - "strings" - - "github.com/hashicorp/boundary/internal/errors" - "gorm.io/gorm" -) - -// UpdateFields will create a map[string]interface of the update values to be -// sent to the db. The map keys will be the field names for the fields to be -// updated. The caller provided fieldMaskPaths and setToNullPaths must not -// intersect. fieldMaskPaths and setToNullPaths cannot both be zero len. -func UpdateFields(i interface{}, fieldMaskPaths []string, setToNullPaths []string) (map[string]interface{}, error) { - const op = "common.UpdateFields" - if i == nil { - return nil, errors.NewDeprecated(errors.InvalidParameter, op, "interface is missing") - } - if fieldMaskPaths == nil { - fieldMaskPaths = []string{} - } - if setToNullPaths == nil { - setToNullPaths = []string{} - } - if len(fieldMaskPaths) == 0 && len(setToNullPaths) == 0 { - return nil, errors.NewDeprecated(errors.InvalidParameter, op, "both fieldMaskPaths and setToNullPaths are zero len") - } - - inter, maskPaths, nullPaths, err := Intersection(fieldMaskPaths, setToNullPaths) - if err != nil { - return nil, errors.WrapDeprecated(err, op) - } - if len(inter) != 0 { - return nil, errors.NewDeprecated(errors.InvalidParameter, op, "fieldMashPaths and setToNullPaths cannot intersect") - } - - updateFields := map[string]interface{}{} // case sensitive update fields to values - - found := map[string]struct{}{} // we need something to keep track of found fields (case insensitive) - - val := reflect.Indirect(reflect.ValueOf(i)) - structTyp := val.Type() - for i := 0; i < structTyp.NumField(); i++ { - if f, ok := maskPaths[strings.ToUpper(structTyp.Field(i).Name)]; ok { - updateFields[f] = val.Field(i).Interface() - found[strings.ToUpper(f)] = struct{}{} - continue - } - if f, ok := nullPaths[strings.ToUpper(structTyp.Field(i).Name)]; ok { - updateFields[f] = gorm.Expr("NULL") - found[strings.ToUpper(f)] = struct{}{} - continue - } - kind := structTyp.Field(i).Type.Kind() - if kind == reflect.Struct || kind == reflect.Ptr { - embType := structTyp.Field(i).Type - // check if the embedded field is exported via CanInterface() - if val.Field(i).CanInterface() { - embVal := reflect.Indirect(reflect.ValueOf(val.Field(i).Interface())) - // if it's a ptr to a struct, then we need a few more bits before proceeding. - if kind == reflect.Ptr { - embVal = val.Field(i).Elem() - if !embVal.IsValid() { - continue - } - embType = embVal.Type() - if embType.Kind() != reflect.Struct { - continue - } - } - for embFieldNum := 0; embFieldNum < embType.NumField(); embFieldNum++ { - if f, ok := maskPaths[strings.ToUpper(embType.Field(embFieldNum).Name)]; ok { - updateFields[f] = embVal.Field(embFieldNum).Interface() - found[strings.ToUpper(f)] = struct{}{} - } - if f, ok := nullPaths[strings.ToUpper(embType.Field(embFieldNum).Name)]; ok { - updateFields[f] = gorm.Expr("NULL") - found[strings.ToUpper(f)] = struct{}{} - } - } - continue - } - } - } - - if missing := findMissingPaths(setToNullPaths, found); len(missing) != 0 { - return nil, errors.NewDeprecated(errors.InvalidParameter, op, fmt.Sprintf("null paths not found in resource: %s", missing)) - } - - if missing := findMissingPaths(fieldMaskPaths, found); len(missing) != 0 { - return nil, errors.NewDeprecated(errors.InvalidParameter, op, fmt.Sprintf("field mask paths not found in resource: %s", missing)) - } - - return updateFields, nil -} - -func findMissingPaths(paths []string, foundPaths map[string]struct{}) []string { - notFound := []string{} - for _, f := range paths { - if _, ok := foundPaths[strings.ToUpper(f)]; !ok { - notFound = append(notFound, f) - } - } - return notFound -} - -// Intersection is a case-insensitive search for intersecting values. Returns -// []string of the Intersection with values in lowercase, and map[string]string -// of the original av and bv, with the key set to uppercase and value set to the -// original -func Intersection(av, bv []string) ([]string, map[string]string, map[string]string, error) { - const op = "common.Intersection" - if av == nil { - return nil, nil, nil, errors.NewDeprecated(errors.InvalidParameter, op, "av is missing") - } - if bv == nil { - return nil, nil, nil, errors.NewDeprecated(errors.InvalidParameter, op, "bv is missing") - } - if len(av) == 0 && len(bv) == 0 { - return []string{}, map[string]string{}, map[string]string{}, nil - } - s := []string{} - ah := map[string]string{} - bh := map[string]string{} - - for i := 0; i < len(av); i++ { - ah[strings.ToUpper(av[i])] = av[i] - } - for i := 0; i < len(bv); i++ { - k := strings.ToUpper(bv[i]) - bh[k] = bv[i] - if _, found := ah[k]; found { - s = append(s, strings.ToLower(bh[k])) - } - } - return s, ah, bh, nil -} - -// BuildUpdatePaths takes a map of field names to field values, field masks, -// fields allowed to be zero value, and returns both a list of field names to -// update and a list of field names that should be set to null. -func BuildUpdatePaths(fieldValues map[string]interface{}, fieldMask []string, allowZeroFields []string) (masks []string, nulls []string) { - for f, v := range fieldValues { - if !contains(fieldMask, f) { - continue - } - switch { - case isZero(v) && !contains(allowZeroFields, f): - nulls = append(nulls, f) - default: - masks = append(masks, f) - } - } - return masks, nulls -} - -func contains(ss []string, t string) bool { - for _, s := range ss { - if strings.EqualFold(s, t) { - return true - } - } - return false -} - -func isZero(i interface{}) bool { - return i == nil || reflect.DeepEqual(i, reflect.Zero(reflect.TypeOf(i)).Interface()) -} diff --git a/internal/db/common/update_test.go b/internal/db/common/update_test.go deleted file mode 100644 index 22893f06e..000000000 --- a/internal/db/common/update_test.go +++ /dev/null @@ -1,642 +0,0 @@ -package common - -import ( - "testing" - - "github.com/hashicorp/boundary/internal/db/db_test" - "github.com/hashicorp/boundary/internal/db/timestamp" - "github.com/hashicorp/go-secure-stdlib/base62" - "github.com/hashicorp/go-uuid" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "google.golang.org/protobuf/proto" - "google.golang.org/protobuf/types/known/timestamppb" - "gorm.io/gorm" -) - -func Test_intersection(t *testing.T) { - type args struct { - av []string - bv []string - } - tests := []struct { - name string - args args - want []string - want1 map[string]string - want2 map[string]string - wantErr bool - wantErrMsg string - }{ - { - name: "intersect", - args: args{ - av: []string{"alice"}, - bv: []string{"alice", "bob"}, - }, - want: []string{"alice"}, - want1: map[string]string{ - "ALICE": "alice", - }, - want2: map[string]string{ - "ALICE": "alice", - "BOB": "bob", - }, - }, - { - name: "intersect-2", - args: args{ - av: []string{"alice", "bob", "jane", "doe"}, - bv: []string{"alice", "doe", "bert", "ernie", "bigbird"}, - }, - want: []string{"alice", "doe"}, - want1: map[string]string{ - "ALICE": "alice", - "BOB": "bob", - "JANE": "jane", - "DOE": "doe", - }, - want2: map[string]string{ - "ALICE": "alice", - "DOE": "doe", - "BERT": "bert", - "ERNIE": "ernie", - "BIGBIRD": "bigbird", - }, - }, - { - name: "intersect-mixed-case", - args: args{ - av: []string{"AlicE"}, - bv: []string{"alICe", "Bob"}, - }, - want: []string{"alice"}, - want1: map[string]string{ - "ALICE": "AlicE", - }, - want2: map[string]string{ - "ALICE": "alICe", - "BOB": "Bob", - }, - }, - { - name: "no-intersect-mixed-case", - args: args{ - av: []string{"AliCe", "BOb", "jaNe", "DOE"}, - bv: []string{"beRt", "ERnie", "bigBIRD"}, - }, - want: []string{}, - want1: map[string]string{ - "ALICE": "AliCe", - "BOB": "BOb", - "JANE": "jaNe", - "DOE": "DOE", - }, - want2: map[string]string{ - "BERT": "beRt", - "ERNIE": "ERnie", - "BIGBIRD": "bigBIRD", - }, - }, - { - name: "no-intersect-1", - args: args{ - av: []string{"alice", "bob", "jane", "doe"}, - bv: []string{"bert", "ernie", "bigbird"}, - }, - want: []string{}, - want1: map[string]string{ - "ALICE": "alice", - "BOB": "bob", - "JANE": "jane", - "DOE": "doe", - }, - want2: map[string]string{ - "BERT": "bert", - "ERNIE": "ernie", - "BIGBIRD": "bigbird", - }, - }, - { - name: "empty-av", - args: args{ - av: []string{}, - bv: []string{"bert", "ernie", "bigbird"}, - }, - want: []string{}, - want1: map[string]string{}, - want2: map[string]string{ - "BERT": "bert", - "ERNIE": "ernie", - "BIGBIRD": "bigbird", - }, - }, - { - name: "empty-av-and-bv", - args: args{ - av: []string{}, - bv: []string{}, - }, - want: []string{}, - want1: map[string]string{}, - want2: map[string]string{}, - }, - { - name: "nil-av", - args: args{ - av: nil, - bv: []string{"bert", "ernie", "bigbird"}, - }, - want: nil, - want1: nil, - want2: nil, - wantErr: true, - wantErrMsg: "common.Intersection: av is missing: parameter violation: error #100", - }, - { - name: "nil-bv", - args: args{ - av: []string{}, - bv: nil, - }, - want: nil, - want1: nil, - want2: nil, - wantErr: true, - wantErrMsg: "common.Intersection: bv is missing: parameter violation: error #100", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert := assert.New(t) - got, got1, got2, err := Intersection(tt.args.av, tt.args.bv) - if err == nil && tt.wantErr { - assert.Error(err) - } - if tt.wantErr { - assert.Error(err) - assert.Equal(tt.wantErrMsg, err.Error()) - } - assert.Equal(tt.want, got) - assert.Equal(tt.want1, got1) - assert.Equal(tt.want2, got2) - }) - } -} - -func TestUpdateFields(t *testing.T) { - a := assert.New(t) - id, err := uuid.GenerateUUID() - a.NoError(err) - - type args struct { - i interface{} - fieldMaskPaths []string - setToNullPaths []string - } - tests := []struct { - name string - args args - want map[string]interface{} - wantErr bool - wantErrMsg string - }{ - { - name: "missing interface", - args: args{ - i: nil, - fieldMaskPaths: []string{}, - setToNullPaths: []string{}, - }, - want: nil, - wantErr: true, - wantErrMsg: "common.UpdateFields: interface is missing: parameter violation: error #100", - }, - { - name: "missing fieldmasks", - args: args{ - i: testUser(t, id, id), - fieldMaskPaths: nil, - setToNullPaths: []string{}, - }, - want: nil, - wantErr: true, - wantErrMsg: "common.UpdateFields: both fieldMaskPaths and setToNullPaths are zero len: parameter violation: error #100", - }, - { - name: "missing null fields", - args: args{ - i: testUser(t, id, id), - fieldMaskPaths: []string{"Name"}, - setToNullPaths: nil, - }, - want: map[string]interface{}{ - "Name": id, - }, - wantErr: false, - }, - { - name: "all zero len", - args: args{ - i: testUser(t, id, id), - fieldMaskPaths: []string{}, - setToNullPaths: nil, - }, - wantErr: true, - wantErrMsg: "common.UpdateFields: both fieldMaskPaths and setToNullPaths are zero len: parameter violation: error #100", - }, - { - name: "not found masks", - args: args{ - i: testUser(t, id, id), - fieldMaskPaths: []string{"invalidFieldName"}, - setToNullPaths: []string{}, - }, - want: nil, - wantErr: true, - wantErrMsg: "common.UpdateFields: field mask paths not found in resource: [invalidFieldName]: parameter violation: error #100", - }, - { - name: "not found null paths", - args: args{ - i: testUser(t, id, id), - fieldMaskPaths: []string{"name"}, - setToNullPaths: []string{"invalidFieldName"}, - }, - want: nil, - wantErr: true, - wantErrMsg: "common.UpdateFields: null paths not found in resource: [invalidFieldName]: parameter violation: error #100", - }, - { - name: "intersection", - args: args{ - i: testUser(t, id, id), - fieldMaskPaths: []string{"name"}, - setToNullPaths: []string{"name"}, - }, - want: nil, - wantErr: true, - wantErrMsg: "common.UpdateFields: fieldMashPaths and setToNullPaths cannot intersect: parameter violation: error #100", - }, - { - name: "valid", - args: args{ - i: testUser(t, id, id), - fieldMaskPaths: []string{"name"}, - setToNullPaths: []string{"email"}, - }, - want: map[string]interface{}{ - "name": id, - "email": gorm.Expr("NULL"), - }, - wantErr: false, - wantErrMsg: "", - }, - { - name: "valid-just-masks", - args: args{ - i: testUser(t, id, id), - fieldMaskPaths: []string{"name", "email"}, - setToNullPaths: []string{}, - }, - want: map[string]interface{}{ - "name": id, - "email": id, - }, - wantErr: false, - wantErrMsg: "", - }, - { - name: "valid-just-nulls", - args: args{ - i: testUser(t, id, id), - fieldMaskPaths: []string{}, - setToNullPaths: []string{"name", "email"}, - }, - want: map[string]interface{}{ - "name": gorm.Expr("NULL"), - "email": gorm.Expr("NULL"), - }, - wantErr: false, - wantErrMsg: "", - }, - { - name: "valid-not-embedded", - args: args{ - i: db_test.StoreTestUser{ - PublicId: testPublicId(t), - Name: id, - Email: "", - }, - fieldMaskPaths: []string{"name"}, - setToNullPaths: []string{"email"}, - }, - want: map[string]interface{}{ - "name": id, - "email": gorm.Expr("NULL"), - }, - wantErr: false, - wantErrMsg: "", - }, - { - name: "valid-not-embedded-just-masks", - args: args{ - i: db_test.StoreTestUser{ - PublicId: testPublicId(t), - Name: id, - Email: "", - }, - fieldMaskPaths: []string{"name"}, - setToNullPaths: nil, - }, - want: map[string]interface{}{ - "name": id, - }, - wantErr: false, - wantErrMsg: "", - }, - { - name: "valid-not-embedded-just-nulls", - args: args{ - i: db_test.StoreTestUser{ - PublicId: testPublicId(t), - Name: id, - Email: "", - }, - fieldMaskPaths: nil, - setToNullPaths: []string{"email"}, - }, - want: map[string]interface{}{ - "email": gorm.Expr("NULL"), - }, - wantErr: false, - wantErrMsg: "", - }, - { - name: "not found null paths - not embedded", - args: args{ - i: db_test.StoreTestUser{ - PublicId: testPublicId(t), - Name: id, - Email: "", - }, - fieldMaskPaths: []string{"name"}, - setToNullPaths: []string{"invalidFieldName"}, - }, - want: nil, - wantErr: true, - wantErrMsg: "common.UpdateFields: null paths not found in resource: [invalidFieldName]: parameter violation: error #100", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert := assert.New(t) - got, err := UpdateFields(tt.args.i, tt.args.fieldMaskPaths, tt.args.setToNullPaths) - if err == nil && tt.wantErr { - assert.Error(err) - } - if tt.wantErr { - assert.Error(err) - assert.Equal(tt.wantErrMsg, err.Error()) - } - assert.Equal(tt.want, got) - }) - } - t.Run("valid-embedded-timestamp", func(t *testing.T) { - assert, require := assert.New(t), require.New(t) - wantTs := ×tamp.Timestamp{ - Timestamp: ×tamppb.Timestamp{ - Seconds: 1, - Nanos: 1, - }, - } - u := testUser(t, "", "") - u.UpdateTime = wantTs - got, err := UpdateFields(u, []string{"UpdateTime"}, nil) - require.NoError(err) - assert.True(proto.Equal(wantTs, got["UpdateTime"].(*timestamp.Timestamp))) - }) -} - -func testUser(t *testing.T, name, email string) *db_test.TestUser { - t.Helper() - return &db_test.TestUser{ - StoreTestUser: &db_test.StoreTestUser{ - PublicId: testPublicId(t), - Name: name, - Email: email, - }, - } -} - -func testPublicId(t *testing.T) string { - t.Helper() - publicId, err := base62.Random(20) - assert.NoError(t, err) - return publicId -} - -func TestBuildUpdatePaths(t *testing.T) { - type args struct { - fieldValues map[string]interface{} - fieldMask []string - allowZeroFields []string - } - tests := []struct { - name string - args args - wantMasks []string - wantNulls []string - }{ - { - name: "empty-inputs", - args: args{ - fieldValues: map[string]interface{}{}, - fieldMask: []string{}, - allowZeroFields: []string{}, - }, - wantMasks: []string{}, - wantNulls: []string{}, - }, - { - name: "no-changes", - args: args{ - fieldValues: map[string]interface{}{ - "Boolean": true, - "Int": 100, - "String": "hello", - "Float": 1.1, - "Complex": complex(1.1, 1.1), - "ByteSlice": []byte("byte slice"), - "ZeroBoolean": false, - "ZeroInt": 0, - "ZeroString": "", - "ZeroFloat": 0.0, - "ZeroComplex": complex(0.0, 0.0), - "ZeroByteSlice": nil, - }, - fieldMask: []string{}, - allowZeroFields: []string{}, - }, - wantMasks: []string{}, - wantNulls: []string{}, - }, - { - name: "empty-field-mask-allow-all-zero-fields", - args: args{ - fieldValues: map[string]interface{}{ - "Boolean": true, - "Int": 100, - "String": "hello", - "Float": 1.1, - "Complex": complex(1.1, 1.1), - "ByteSlice": []byte("byte slice"), - "ZeroBoolean": false, - "ZeroInt": 0, - "ZeroString": "", - "ZeroFloat": 0.0, - "ZeroComplex": complex(0.0, 0.0), - "ZeroByteSlice": nil, - }, - fieldMask: []string{}, - allowZeroFields: []string{ - "Boolean", "Int", "String", "Float", "Complex", "ByteSlice", - "ZeroBoolean", "ZeroInt", "ZeroString", "ZeroFloat", "ZeroComplex", "ZeroByteSlice", - }, - }, - wantMasks: []string{}, - wantNulls: []string{}, - }, - { - name: "zero-fields-are-nulls", - args: args{ - fieldValues: map[string]interface{}{ - "Boolean": true, - "Int": 100, - "String": "hello", - "Float": 1.1, - "Complex": complex(1.1, 1.1), - "ByteSlice": []byte("byte slice"), - "ZeroBoolean": false, - "ZeroInt": 0, - "ZeroString": "", - "ZeroFloat": 0.0, - "ZeroComplex": complex(0.0, 0.0), - "ZeroByteSlice": nil, - }, - fieldMask: []string{ - "Boolean", "Int", "String", "Float", "Complex", "ByteSlice", - "ZeroBoolean", "ZeroInt", "ZeroString", "ZeroFloat", "ZeroComplex", "ZeroByteSlice", - }, - allowZeroFields: []string{}, - }, - wantMasks: []string{ - "Boolean", "Int", "String", "Float", "Complex", "ByteSlice", - }, - wantNulls: []string{ - "ZeroBoolean", "ZeroInt", "ZeroString", "ZeroFloat", "ZeroComplex", "ZeroByteSlice", - }, - }, - { - name: "all-zero-fields-allowed-no-nulls", - args: args{ - fieldValues: map[string]interface{}{ - "Boolean": true, - "Int": 100, - "String": "hello", - "Float": 1.1, - "Complex": complex(1.1, 1.1), - "ByteSlice": []byte("byte slice"), - "ZeroBoolean": false, - "ZeroInt": 0, - "ZeroString": "", - "ZeroFloat": 0.0, - "ZeroComplex": complex(0.0, 0.0), - "ZeroByteSlice": nil, - }, - fieldMask: []string{ - "Boolean", "Int", "String", "Float", "Complex", "ByteSlice", - "ZeroBoolean", "ZeroInt", "ZeroString", "ZeroFloat", "ZeroComplex", "ZeroByteSlice", - }, - allowZeroFields: []string{ - "ZeroBoolean", "ZeroInt", "ZeroString", "ZeroFloat", "ZeroComplex", "ZeroByteSlice", - }, - }, - wantMasks: []string{ - "Boolean", "Int", "String", "Float", "Complex", "ByteSlice", - "ZeroBoolean", "ZeroInt", "ZeroString", "ZeroFloat", "ZeroComplex", "ZeroByteSlice", - }, - wantNulls: []string{}, - }, - { - name: "non-zeros-allowed-as-zero-fields", - args: args{ - fieldValues: map[string]interface{}{ - "Boolean": true, - "Int": 100, - "String": "hello", - "Float": 1.1, - "Complex": complex(1.1, 1.1), - "ByteSlice": []byte("byte slice"), - "ZeroBoolean": false, - "ZeroInt": 0, - "ZeroString": "", - "ZeroFloat": 0.0, - "ZeroComplex": complex(0.0, 0.0), - "ZeroByteSlice": nil, - }, - fieldMask: []string{ - "Boolean", "Int", "String", "Float", "Complex", "ByteSlice", - "ZeroBoolean", "ZeroInt", "ZeroString", "ZeroFloat", "ZeroComplex", "ZeroByteSlice", - }, - allowZeroFields: []string{ - "Boolean", "Int", "String", "Float", "Complex", "ByteSlice", - }, - }, - wantMasks: []string{ - "Boolean", "Int", "String", "Float", "Complex", "ByteSlice", - }, - wantNulls: []string{ - "ZeroBoolean", "ZeroInt", "ZeroString", "ZeroFloat", "ZeroComplex", "ZeroByteSlice", - }, - }, - { - name: "only-zero-fields-in-fieldmask", - args: args{ - fieldValues: map[string]interface{}{ - "Boolean": true, - "Int": 100, - "String": "hello", - "Float": 1.1, - "Complex": complex(1.1, 1.1), - "ByteSlice": []byte("byte slice"), - "ZeroBoolean": false, - "ZeroInt": 0, - "ZeroString": "", - "ZeroFloat": 0.0, - "ZeroComplex": complex(0.0, 0.0), - "ZeroByteSlice": nil, - }, - fieldMask: []string{ - "ZeroBoolean", "ZeroInt", "ZeroString", "ZeroFloat", "ZeroComplex", "ZeroByteSlice", - }, - allowZeroFields: []string{ - "Boolean", "Int", "String", "Float", "Complex", "ByteSlice", - }, - }, - wantMasks: []string{}, - wantNulls: []string{ - "ZeroBoolean", "ZeroInt", "ZeroString", "ZeroFloat", "ZeroComplex", "ZeroByteSlice", - }, - }, - } - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - assert := assert.New(t) - gotMasks, gotNulls := BuildUpdatePaths(tt.args.fieldValues, tt.args.fieldMask, tt.args.allowZeroFields) - assert.ElementsMatch(tt.wantMasks, gotMasks, "masks") - assert.ElementsMatch(tt.wantNulls, gotNulls, "nulls") - }) - } -} diff --git a/internal/host/static/repository_host.go b/internal/host/static/repository_host.go index d06838b9f..174ed17aa 100644 --- a/internal/host/static/repository_host.go +++ b/internal/host/static/repository_host.go @@ -6,10 +6,10 @@ import ( "strings" "github.com/hashicorp/boundary/internal/db" - dbcommon "github.com/hashicorp/boundary/internal/db/common" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/oplog" + "github.com/hashicorp/go-dbw" ) // CreateHost inserts h into the repository and returns a new Host @@ -141,7 +141,7 @@ func (r *Repository) UpdateHost(ctx context.Context, projectId string, h *Host, } } var dbMask, nullFields []string - dbMask, nullFields = dbcommon.BuildUpdatePaths( + dbMask, nullFields = dbw.BuildUpdatePaths( map[string]interface{}{ "Name": h.Name, "Description": h.Description, diff --git a/internal/host/static/repository_host_set.go b/internal/host/static/repository_host_set.go index 87a39ab9d..ff2c0d99d 100644 --- a/internal/host/static/repository_host_set.go +++ b/internal/host/static/repository_host_set.go @@ -6,11 +6,11 @@ import ( "strings" "github.com/hashicorp/boundary/internal/db" - dbcommon "github.com/hashicorp/boundary/internal/db/common" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/host" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/oplog" + "github.com/hashicorp/go-dbw" ) // CreateSet inserts s into the repository and returns a new HostSet @@ -125,7 +125,7 @@ func (r *Repository) UpdateSet(ctx context.Context, projectId string, s *HostSet } } var dbMask, nullFields []string - dbMask, nullFields = dbcommon.BuildUpdatePaths( + dbMask, nullFields = dbw.BuildUpdatePaths( map[string]interface{}{ "Name": s.Name, "Description": s.Description, diff --git a/internal/iam/repository_group.go b/internal/iam/repository_group.go index 9077b841c..437ede0ce 100644 --- a/internal/iam/repository_group.go +++ b/internal/iam/repository_group.go @@ -6,10 +6,10 @@ import ( "strings" "github.com/hashicorp/boundary/internal/db" - dbcommon "github.com/hashicorp/boundary/internal/db/common" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/oplog" + "github.com/hashicorp/go-dbw" ) // CreateGroup will create a group in the repository and return the written @@ -69,7 +69,7 @@ func (r *Repository) UpdateGroup(ctx context.Context, group *Group, version uint } } var dbMask, nullFields []string - dbMask, nullFields = dbcommon.BuildUpdatePaths( + dbMask, nullFields = dbw.BuildUpdatePaths( map[string]interface{}{ "name": group.Name, "description": group.Description, diff --git a/internal/iam/repository_role.go b/internal/iam/repository_role.go index 48dbede54..5b17675c7 100644 --- a/internal/iam/repository_role.go +++ b/internal/iam/repository_role.go @@ -6,8 +6,8 @@ import ( "strings" "github.com/hashicorp/boundary/internal/db" - dbcommon "github.com/hashicorp/boundary/internal/db/common" "github.com/hashicorp/boundary/internal/errors" + "github.com/hashicorp/go-dbw" ) // CreateRole will create a role in the repository and return the written @@ -69,7 +69,7 @@ func (r *Repository) UpdateRole(ctx context.Context, role *Role, version uint32, } } var dbMask, nullFields []string - dbMask, nullFields = dbcommon.BuildUpdatePaths( + dbMask, nullFields = dbw.BuildUpdatePaths( map[string]interface{}{ "name": role.Name, "description": role.Description, diff --git a/internal/iam/repository_scope.go b/internal/iam/repository_scope.go index 103196700..733cd1643 100644 --- a/internal/iam/repository_scope.go +++ b/internal/iam/repository_scope.go @@ -7,12 +7,12 @@ import ( "strings" "github.com/hashicorp/boundary/internal/db" - dbcommon "github.com/hashicorp/boundary/internal/db/common" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/oplog" "github.com/hashicorp/boundary/internal/types/resource" "github.com/hashicorp/boundary/internal/types/scope" + "github.com/hashicorp/go-dbw" wrapping "github.com/hashicorp/go-kms-wrapping/v2" ) @@ -394,7 +394,7 @@ func (r *Repository) UpdateScope(ctx context.Context, scope *Scope, version uint return nil, db.NoRowsAffected, errors.New(ctx, errors.InvalidFieldMask, op, "you cannot change a scope's parent") } var dbMask, nullFields []string - dbMask, nullFields = dbcommon.BuildUpdatePaths( + dbMask, nullFields = dbw.BuildUpdatePaths( map[string]interface{}{ "name": scope.Name, "description": scope.Description, diff --git a/internal/iam/repository_user.go b/internal/iam/repository_user.go index 284810e52..c743edb76 100644 --- a/internal/iam/repository_user.go +++ b/internal/iam/repository_user.go @@ -6,11 +6,11 @@ import ( "strings" "github.com/hashicorp/boundary/internal/db" - dbcommon "github.com/hashicorp/boundary/internal/db/common" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/oplog" "github.com/hashicorp/boundary/internal/types/scope" + "github.com/hashicorp/go-dbw" ) // CreateUser will create a user in the repository and return the written user @@ -82,7 +82,7 @@ func (r *Repository) UpdateUser(ctx context.Context, user *User, version uint32, } } var dbMask, nullFields []string - dbMask, nullFields = dbcommon.BuildUpdatePaths( + dbMask, nullFields = dbw.BuildUpdatePaths( map[string]interface{}{ "name": user.Name, "description": user.Description, diff --git a/internal/oplog/queue.go b/internal/oplog/queue.go index 0d30623bc..b14ebf316 100644 --- a/internal/oplog/queue.go +++ b/internal/oplog/queue.go @@ -8,7 +8,6 @@ import ( "io" "sync" - "github.com/hashicorp/boundary/internal/db/common" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/go-dbw" "google.golang.org/genproto/protobuf/field_mask" @@ -59,7 +58,7 @@ func (q *Queue) add(ctx context.Context, m proto.Message, typeName string, t OpT if nullPaths == nil { nullPaths = []string{} } - i, _, _, err := common.Intersection(fMasks, nullPaths) + i, _, _, err := dbw.Intersection(fMasks, nullPaths) if err != nil { return errors.Wrap(ctx, err, op) } diff --git a/internal/server/repository_worker.go b/internal/server/repository_worker.go index 8872c6f9a..ea11a6cb3 100644 --- a/internal/server/repository_worker.go +++ b/internal/server/repository_worker.go @@ -6,11 +6,11 @@ import ( "strings" "github.com/hashicorp/boundary/internal/db" - dbcommon "github.com/hashicorp/boundary/internal/db/common" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/server/store" "github.com/hashicorp/boundary/internal/types/scope" + "github.com/hashicorp/go-dbw" wrapping "github.com/hashicorp/go-kms-wrapping/v2" "github.com/hashicorp/nodeenrollment" "github.com/hashicorp/nodeenrollment/registration" @@ -434,7 +434,7 @@ func (r *Repository) UpdateWorker(ctx context.Context, worker *Worker, version u } var dbMask, nullFields []string - dbMask, nullFields = dbcommon.BuildUpdatePaths( + dbMask, nullFields = dbw.BuildUpdatePaths( map[string]interface{}{ nameField: worker.Name, descField: worker.Description, diff --git a/internal/target/repository.go b/internal/target/repository.go index a9640c5d6..2f65552f5 100644 --- a/internal/target/repository.go +++ b/internal/target/repository.go @@ -8,11 +8,11 @@ import ( "github.com/hashicorp/boundary/internal/boundary" "github.com/hashicorp/boundary/internal/db" - dbcommon "github.com/hashicorp/boundary/internal/db/common" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/oplog" "github.com/hashicorp/boundary/internal/types/scope" + "github.com/hashicorp/go-dbw" ) // Cloneable provides a cloning interface @@ -518,7 +518,7 @@ func (r *Repository) UpdateTarget(ctx context.Context, target Target, version ui } } var dbMask, nullFields []string - dbMask, nullFields = dbcommon.BuildUpdatePaths( + dbMask, nullFields = dbw.BuildUpdatePaths( map[string]interface{}{ "Name": target.GetName(), "Description": target.GetDescription(),