Skip to content

Commit

Permalink
chore: Split existing alter operations (#2156)
Browse files Browse the repository at this point in the history 
* Extract set and unset tags

* Fix warehouse validation

* Fix after review

* Fix warehouses
  • Loading branch information
@sfc-gh-asawicki
sfc-gh-asawicki committed Oct 30, 2023
1 parent 82c3c13 commit dbb7c91
Show file tree
Hide file tree
Showing 24 changed files with 212 additions and 305 deletions.
8 changes: 2 additions & 6 deletions pkg/resources/schema.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,7 @@ func UpdateSchema(d *schema.ResourceData, meta interface{}) error {
unsetTags[i] = sdk.NewDatabaseObjectIdentifier(t.database, t.name)
}
err := client.Schemas.Alter(ctx, id, &sdk.AlterSchemaOptions{
Unset: &sdk.SchemaUnset{
Tag: unsetTags,
},
UnsetTag: unsetTags,
})
if err != nil {
return fmt.Errorf("error dropping tags on %v", d.Id())
Expand All @@ -259,9 +257,7 @@ func UpdateSchema(d *schema.ResourceData, meta interface{}) error {
}
}
err = client.Schemas.Alter(ctx, id, &sdk.AlterSchemaOptions{
Set: &sdk.SchemaSet{
Tag: setTags,
},
SetTag: setTags,
})
if err != nil {
return fmt.Errorf("error setting tags on %v", d.Id())
Expand Down
24 changes: 12 additions & 12 deletions pkg/sdk/accounts.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,19 +90,21 @@ type AlterAccountOptions struct {
alter bool `ddl:"static" sql:"ALTER"`
account bool `ddl:"static" sql:"ACCOUNT"`

Set *AccountSet `ddl:"keyword" sql:"SET"`
Unset *AccountUnset `ddl:"list,no_parentheses" sql:"UNSET"`
Rename *AccountRename `ddl:"-"`
Drop *AccountDrop `ddl:"-"`
Set *AccountSet `ddl:"keyword" sql:"SET"`
Unset *AccountUnset `ddl:"list,no_parentheses" sql:"UNSET"`
SetTag []TagAssociation `ddl:"keyword" sql:"SET TAG"`
UnsetTag []ObjectIdentifier `ddl:"keyword" sql:"UNSET TAG"`
Rename *AccountRename `ddl:"-"`
Drop *AccountDrop `ddl:"-"`
}

func (opts *AlterAccountOptions) validate() error {
if opts == nil {
return errors.Join(ErrNilOptions)
}
var errs []error
if !exactlyOneValueSet(opts.Set, opts.Unset, opts.Drop, opts.Rename) {
errs = append(errs, errExactlyOneOf("CreateAccountOptions", "Set", "Unset", "Drop", "Rename"))
if !exactlyOneValueSet(opts.Set, opts.Unset, opts.SetTag, opts.UnsetTag, opts.Drop, opts.Rename) {
errs = append(errs, errExactlyOneOf("CreateAccountOptions", "Set", "Unset", "SetTag", "UnsetTag", "Drop", "Rename"))
}
if valueSet(opts.Set) {
if err := opts.Set.validate(); err != nil {
Expand Down Expand Up @@ -164,13 +166,12 @@ type AccountSet struct {
ResourceMonitor AccountObjectIdentifier `ddl:"identifier,equals" sql:"RESOURCE_MONITOR"`
PasswordPolicy SchemaObjectIdentifier `ddl:"identifier" sql:"PASSWORD POLICY"`
SessionPolicy SchemaObjectIdentifier `ddl:"identifier" sql:"SESSION POLICY"`
Tag []TagAssociation `ddl:"keyword" sql:"TAG"`
}

func (opts *AccountSet) validate() error {
var errs []error
if !exactlyOneValueSet(opts.Parameters, opts.ResourceMonitor, opts.PasswordPolicy, opts.SessionPolicy, opts.Tag) {
errs = append(errs, errExactlyOneOf("AccountSet", "Parameters", "ResourceMonitor", "PasswordPolicy", "SessionPolicy", "Tag"))
if !exactlyOneValueSet(opts.Parameters, opts.ResourceMonitor, opts.PasswordPolicy, opts.SessionPolicy) {
errs = append(errs, errExactlyOneOf("AccountSet", "Parameters", "ResourceMonitor", "PasswordPolicy", "SessionPolicy"))
}
if valueSet(opts.Parameters) {
if err := opts.Parameters.validate(); err != nil {
Expand Down Expand Up @@ -198,13 +199,12 @@ type AccountUnset struct {
Parameters *AccountLevelParametersUnset `ddl:"list,no_parentheses"`
PasswordPolicy *bool `ddl:"keyword" sql:"PASSWORD POLICY"`
SessionPolicy *bool `ddl:"keyword" sql:"SESSION POLICY"`
Tag []ObjectIdentifier `ddl:"keyword" sql:"TAG"`
}

func (opts *AccountUnset) validate() error {
var errs []error
if !exactlyOneValueSet(opts.Parameters, opts.PasswordPolicy, opts.SessionPolicy, opts.Tag) {
errs = append(errs, errExactlyOneOf("AccountUnset", "Parameters", "PasswordPolicy", "SessionPolicy", "Tag"))
if !exactlyOneValueSet(opts.Parameters, opts.PasswordPolicy, opts.SessionPolicy) {
errs = append(errs, errExactlyOneOf("AccountUnset", "Parameters", "PasswordPolicy", "SessionPolicy"))
}
if valueSet(opts.Parameters) {
if err := opts.Parameters.validate(); err != nil {
Expand Down
24 changes: 10 additions & 14 deletions pkg/sdk/accounts_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,16 +140,14 @@ func TestAccountAlter(t *testing.T) {

t.Run("with set tag", func(t *testing.T) {
opts := &AlterAccountOptions{
Set: &AccountSet{
Tag: []TagAssociation{
{
Name: NewSchemaObjectIdentifier("db", "schema", "tag1"),
Value: "v1",
},
{
Name: NewSchemaObjectIdentifier("db", "schema", "tag2"),
Value: "v2",
},
SetTag: []TagAssociation{
{
Name: NewSchemaObjectIdentifier("db", "schema", "tag1"),
Value: "v1",
},
{
Name: NewSchemaObjectIdentifier("db", "schema", "tag2"),
Value: "v2",
},
},
}
Expand All @@ -158,10 +156,8 @@ func TestAccountAlter(t *testing.T) {

t.Run("with unset tag", func(t *testing.T) {
opts := &AlterAccountOptions{
Unset: &AccountUnset{
Tag: []ObjectIdentifier{
NewSchemaObjectIdentifier("db", "schema", "tag1"),
},
UnsetTag: []ObjectIdentifier{
NewSchemaObjectIdentifier("db", "schema", "tag1"),
},
}
assertOptsValidAndSQLEquals(t, opts, `ALTER ACCOUNT UNSET TAG "db"."schema"."tag1"`)
Expand Down
2 changes: 1 addition & 1 deletion pkg/sdk/errors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func errNotSet(structName string, fieldNames ...string) error {
}

func errExactlyOneOf(structName string, fieldNames ...string) error {
return newError(fmt.Sprintf("exactly one of %s fileds %v must be set", structName, fieldNames), 2)
return newError(fmt.Sprintf("exactly one of %s fields %v must be set", structName, fieldNames), 2)
}

func errAtLeastOneOf(structName string, fieldNames ...string) error {
Expand Down
22 changes: 11 additions & 11 deletions pkg/sdk/masking_policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ type AlterMaskingPolicyOptions struct {
NewName *SchemaObjectIdentifier `ddl:"identifier" sql:"RENAME TO"`
Set *MaskingPolicySet `ddl:"keyword" sql:"SET"`
Unset *MaskingPolicyUnset `ddl:"keyword" sql:"UNSET"`
SetTag []TagAssociation `ddl:"keyword" sql:"SET TAG"`
UnsetTag []ObjectIdentifier `ddl:"keyword" sql:"UNSET TAG"`
}

func (opts *AlterMaskingPolicyOptions) validate() error {
Expand All @@ -112,8 +114,8 @@ func (opts *AlterMaskingPolicyOptions) validate() error {
if opts.NewName != nil && !ValidObjectIdentifier(opts.NewName) {
errs = append(errs, errInvalidIdentifier("AlterMaskingPolicyOptions", "NewName"))
}
if !exactlyOneValueSet(opts.NewName, opts.Set, opts.Unset) {
errs = append(errs, errExactlyOneOf("AlterMaskingPolicyOptions", "NewName", "Set", "Unset"))
if !exactlyOneValueSet(opts.Set, opts.Unset, opts.SetTag, opts.UnsetTag, opts.NewName) {
errs = append(errs, errExactlyOneOf("AlterMaskingPolicyOptions", "Set", "Unset", "SetTag", "UnsetTag", "NewName"))
}
if valueSet(opts.Set) {
if err := opts.Set.validate(); err != nil {
Expand All @@ -129,26 +131,24 @@ func (opts *AlterMaskingPolicyOptions) validate() error {
}

type MaskingPolicySet struct {
Body *string `ddl:"parameter,no_equals" sql:"BODY ->"`
Tag []TagAssociation `ddl:"keyword" sql:"TAG"`
Comment *string `ddl:"parameter,single_quotes" sql:"COMMENT"`
Body *string `ddl:"parameter,no_equals" sql:"BODY ->"`
Comment *string `ddl:"parameter,single_quotes" sql:"COMMENT"`
}

func (v *MaskingPolicySet) validate() error {
if !exactlyOneValueSet(v.Body, v.Tag, v.Comment) {
return errExactlyOneOf("MaskingPolicySet", "Body", "Tag", "Comment")
if !exactlyOneValueSet(v.Body, v.Comment) {
return errExactlyOneOf("MaskingPolicySet", "Body", "Comment")
}
return nil
}

type MaskingPolicyUnset struct {
Tag []ObjectIdentifier `ddl:"keyword" sql:"TAG"`
Comment *bool `ddl:"keyword" sql:"COMMENT"`
Comment *bool `ddl:"keyword" sql:"COMMENT"`
}

func (v *MaskingPolicyUnset) validate() error {
if !exactlyOneValueSet(v.Tag, v.Comment) {
return errExactlyOneOf("MaskingPolicyUnset", "Tag", "Comment")
if !exactlyOneValueSet(v.Comment) {
return errExactlyOneOf("MaskingPolicyUnset", "Comment")
}
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sdk/masking_policy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func TestMaskingPolicyAlter(t *testing.T) {
opts := &AlterMaskingPolicyOptions{
name: id,
}
assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("AlterMaskingPolicyOptions", "NewName", "Set", "Unset"))
assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("AlterMaskingPolicyOptions", "Set", "Unset", "SetTag", "UnsetTag", "NewName"))
})

t.Run("with set", func(t *testing.T) {
Expand Down
18 changes: 5 additions & 13 deletions pkg/sdk/pipes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ type AlterPipeOptions struct {
name SchemaObjectIdentifier `ddl:"identifier"`

// One of
Set *PipeSet `ddl:"list,no_parentheses" sql:"SET"`
Unset *PipeUnset `ddl:"list,no_parentheses" sql:"UNSET"`
SetTags *PipeSetTags `ddl:"list,no_parentheses" sql:"SET TAG"`
UnsetTags *PipeUnsetTags `ddl:"list,no_parentheses" sql:"UNSET TAG"`
Refresh *PipeRefresh `ddl:"keyword" sql:"REFRESH"`
Set *PipeSet `ddl:"list,no_parentheses" sql:"SET"`
Unset *PipeUnset `ddl:"list,no_parentheses" sql:"UNSET"`
SetTag []TagAssociation `ddl:"keyword" sql:"SET TAG"`
UnsetTag []ObjectIdentifier `ddl:"keyword" sql:"UNSET TAG"`
Refresh *PipeRefresh `ddl:"keyword" sql:"REFRESH"`
}

type PipeSet struct {
Expand All @@ -60,14 +60,6 @@ type PipeUnset struct {
Comment *bool `ddl:"keyword" sql:"COMMENT"`
}

type PipeSetTags struct {
Tag []TagAssociation `ddl:"keyword"`
}

type PipeUnsetTags struct {
Tag []ObjectIdentifier `ddl:"keyword"`
}

type PipeRefresh struct {
Prefix *string `ddl:"parameter,single_quotes" sql:"PREFIX"`
ModifiedAfter *string `ddl:"parameter,single_quotes" sql:"MODIFIED_AFTER"`
Expand Down
62 changes: 19 additions & 43 deletions pkg/sdk/pipes_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func TestPipesAlter(t *testing.T) {

t.Run("validation: no alter action", func(t *testing.T) {
opts := defaultOpts()
assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("AlterPipeOptions", "Set", "Unset", "SetTags", "UnsetTags", "Refresh"))
assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("AlterPipeOptions", "Set", "Unset", "SetTag", "UnsetTag", "Refresh"))
})

t.Run("validation: multiple alter actions", func(t *testing.T) {
Expand All @@ -81,7 +81,7 @@ func TestPipesAlter(t *testing.T) {
opts.Unset = &PipeUnset{
Comment: Bool(true),
}
assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("AlterPipeOptions", "Set", "Unset", "SetTags", "UnsetTags", "Refresh"))
assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("AlterPipeOptions", "Set", "Unset", "SetTag", "UnsetTag", "Refresh"))
})

t.Run("validation: no property to set", func(t *testing.T) {
Expand All @@ -90,53 +90,33 @@ func TestPipesAlter(t *testing.T) {
assertOptsInvalidJoinedErrors(t, opts, errAtLeastOneOf("AlterPipeOptions.Set", "ErrorIntegration", "PipeExecutionPaused", "Comment"))
})

t.Run("validation: empty tags slice for set", func(t *testing.T) {
opts := defaultOpts()
opts.SetTags = &PipeSetTags{
Tag: []TagAssociation{},
}
assertOptsInvalidJoinedErrors(t, opts, errNotSet("AlterPipeOptions.SetTags", "Tag"))
})

t.Run("validation: no property to unset", func(t *testing.T) {
opts := defaultOpts()
opts.Unset = &PipeUnset{}
assertOptsInvalidJoinedErrors(t, opts, errAtLeastOneOf("AlterPipeOptions.Unset", "PipeExecutionPaused", "Comment"))
})

t.Run("validation: empty tags slice for unset", func(t *testing.T) {
opts := defaultOpts()
opts.UnsetTags = &PipeUnsetTags{
Tag: []ObjectIdentifier{},
}
assertOptsInvalidJoinedErrors(t, opts, errNotSet("AlterPipeOptions.UnsetTags", "Tag"))
})

t.Run("set tag: single", func(t *testing.T) {
opts := defaultOpts()
opts.SetTags = &PipeSetTags{
Tag: []TagAssociation{
{
Name: NewAccountObjectIdentifier("tag_name1"),
Value: "v1",
},
opts.SetTag = []TagAssociation{
{
Name: NewAccountObjectIdentifier("tag_name1"),
Value: "v1",
},
}
assertOptsValidAndSQLEquals(t, opts, `ALTER PIPE %s SET TAG "tag_name1" = 'v1'`, id.FullyQualifiedName())
})

t.Run("set tag: multiple", func(t *testing.T) {
opts := defaultOpts()
opts.SetTags = &PipeSetTags{
Tag: []TagAssociation{
{
Name: NewAccountObjectIdentifier("tag_name1"),
Value: "v1",
},
{
Name: NewAccountObjectIdentifier("tag_name2"),
Value: "v2",
},
opts.SetTag = []TagAssociation{
{
Name: NewAccountObjectIdentifier("tag_name1"),
Value: "v1",
},
{
Name: NewAccountObjectIdentifier("tag_name2"),
Value: "v2",
},
}
assertOptsValidAndSQLEquals(t, opts, `ALTER PIPE %s SET TAG "tag_name1" = 'v1', "tag_name2" = 'v2'`, id.FullyQualifiedName())
Expand All @@ -155,21 +135,17 @@ func TestPipesAlter(t *testing.T) {

t.Run("unset tag: single", func(t *testing.T) {
opts := defaultOpts()
opts.UnsetTags = &PipeUnsetTags{
Tag: []ObjectIdentifier{
NewAccountObjectIdentifier("tag_name1"),
},
opts.UnsetTag = []ObjectIdentifier{
NewAccountObjectIdentifier("tag_name1"),
}
assertOptsValidAndSQLEquals(t, opts, `ALTER PIPE %s UNSET TAG "tag_name1"`, id.FullyQualifiedName())
})

t.Run("unset tag: multi", func(t *testing.T) {
opts := defaultOpts()
opts.UnsetTags = &PipeUnsetTags{
Tag: []ObjectIdentifier{
NewAccountObjectIdentifier("tag_name1"),
NewAccountObjectIdentifier("tag_name2"),
},
opts.UnsetTag = []ObjectIdentifier{
NewAccountObjectIdentifier("tag_name1"),
NewAccountObjectIdentifier("tag_name2"),
}
assertOptsValidAndSQLEquals(t, opts, `ALTER PIPE %s UNSET TAG "tag_name1", "tag_name2"`, id.FullyQualifiedName())
})
Expand Down
20 changes: 8 additions & 12 deletions pkg/sdk/pipes_validations.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,14 @@ func (opts *AlterPipeOptions) validate() error {
if !ValidObjectIdentifier(opts.name) {
errs = append(errs, ErrInvalidObjectIdentifier)
}
if ok := exactlyOneValueSet(opts.Set, opts.Unset, opts.SetTags, opts.UnsetTags, opts.Refresh); !ok {
errs = append(errs, errExactlyOneOf("AlterPipeOptions", "Set", "Unset", "SetTags", "UnsetTags", "Refresh"))
if ok := exactlyOneValueSet(
opts.Set,
opts.Unset,
opts.SetTag,
opts.UnsetTag,
opts.Refresh,
); !ok {
errs = append(errs, errExactlyOneOf("AlterPipeOptions", "Set", "Unset", "SetTag", "UnsetTag", "Refresh"))
}
if set := opts.Set; valueSet(set) {
if !anyValueSet(set.ErrorIntegration, set.PipeExecutionPaused, set.Comment) {
Expand All @@ -47,16 +53,6 @@ func (opts *AlterPipeOptions) validate() error {
errs = append(errs, errAtLeastOneOf("AlterPipeOptions.Unset", "PipeExecutionPaused", "Comment"))
}
}
if setTags := opts.SetTags; valueSet(setTags) {
if !valueSet(setTags.Tag) {
errs = append(errs, errNotSet("AlterPipeOptions.SetTags", "Tag"))
}
}
if unsetTags := opts.UnsetTags; valueSet(unsetTags) {
if !valueSet(unsetTags.Tag) {
errs = append(errs, errNotSet("AlterPipeOptions.UnsetTags", "Tag"))
}
}
return errors.Join(errs...)
}

Expand Down

0 comments on commit dbb7c91

Please sign in to comment.