From d008cceaed7ddfd65df6740ac34a0a073e678f03 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Thu, 4 Jan 2024 19:19:39 +0100 Subject: [PATCH] Fix syntax error for resource monitor alter --- .../provider/resource_monitor_resource.go | 2 +- pkg/resources/resource_monitor.go | 26 ++++++++-------- pkg/sdk/resource_monitors.go | 21 +++++++------ pkg/sdk/resource_monitors_test.go | 25 +++++++++++++++- .../resource_monitors_integration_test.go | 30 +++++++++++++++++-- 5 files changed, 77 insertions(+), 27 deletions(-) diff --git a/framework/provider/resource_monitor_resource.go b/framework/provider/resource_monitor_resource.go index 921387a376..588c509e19 100644 --- a/framework/provider/resource_monitor_resource.go +++ b/framework/provider/resource_monitor_resource.go @@ -736,7 +736,7 @@ func (r *ResourceMonitorResource) update(ctx context.Context, plan *resourceMoni for _, e := range elements { notifiedUsers = append(notifiedUsers, sdk.NotifiedUser{Name: e.ValueString()}) } - opts.NotifyUsers = &sdk.NotifyUsers{ + opts.Set.NotifyUsers = &sdk.NotifyUsers{ Users: notifiedUsers, } } diff --git a/pkg/resources/resource_monitor.go b/pkg/resources/resource_monitor.go index eb5159305f..e4ad6a196f 100644 --- a/pkg/resources/resource_monitor.go +++ b/pkg/resources/resource_monitor.go @@ -319,19 +319,6 @@ func UpdateResourceMonitor(d *schema.ResourceData, meta interface{}) error { opts := sdk.AlterResourceMonitorOptions{} - if d.HasChange("notify_users") { - runSetStatement = true - - userNames := expandStringList(d.Get("notify_users").(*schema.Set).List()) - users := []sdk.NotifiedUser{} - for _, name := range userNames { - users = append(users, sdk.NotifiedUser{Name: name}) - } - opts.NotifyUsers = &sdk.NotifyUsers{ - Users: users, - } - } - set := sdk.ResourceMonitorSet{} if d.HasChange("credit_quota") { runSetStatement = true @@ -353,6 +340,19 @@ func UpdateResourceMonitor(d *schema.ResourceData, meta interface{}) error { set.EndTimestamp = sdk.Pointer(d.Get("end_timestamp").(string)) } + if d.HasChange("notify_users") { + runSetStatement = true + + userNames := expandStringList(d.Get("notify_users").(*schema.Set).List()) + users := []sdk.NotifiedUser{} + for _, name := range userNames { + users = append(users, sdk.NotifiedUser{Name: name}) + } + set.NotifyUsers = &sdk.NotifyUsers{ + Users: users, + } + } + if set != (sdk.ResourceMonitorSet{}) { opts.Set = &set } diff --git a/pkg/sdk/resource_monitors.go b/pkg/sdk/resource_monitors.go index 1363f38b1e..16098547c8 100644 --- a/pkg/sdk/resource_monitors.go +++ b/pkg/sdk/resource_monitors.go @@ -288,7 +288,6 @@ type AlterResourceMonitorOptions struct { IfExists *bool `ddl:"keyword" sql:"IF EXISTS"` name AccountObjectIdentifier `ddl:"identifier"` Set *ResourceMonitorSet `ddl:"keyword" sql:"SET"` - NotifyUsers *NotifyUsers `ddl:"parameter,equals" sql:"NOTIFY_USERS"` Triggers []TriggerDefinition `ddl:"keyword,no_comma" sql:"TRIGGERS"` } @@ -300,11 +299,14 @@ func (opts *AlterResourceMonitorOptions) validate() error { if !ValidObjectIdentifier(opts.name) { errs = append(errs, ErrInvalidObjectIdentifier) } - if everyValueNil(opts.Set, opts.NotifyUsers, opts.Triggers) { - errs = append(errs, errAtLeastOneOf("AlterResourceMonitorOptions", "Set", "NotifyUsers", "Triggers")) + if everyValueNil(opts.Set, opts.Triggers) { + errs = append(errs, errAtLeastOneOf("AlterResourceMonitorOptions", "Set", "Triggers")) } - if valueSet(opts.Set) { - if (opts.Set.Frequency != nil && opts.Set.StartTimestamp == nil) || (opts.Set.Frequency == nil && opts.Set.StartTimestamp != nil) { + if set := opts.Set; valueSet(set) { + if everyValueNil(set.CreditQuota, set.Frequency, set.StartTimestamp, set.EndTimestamp, set.NotifyUsers) { + errs = append(errs, errAtLeastOneOf("ResourceMonitorSet", "CreditQuota", "Frequency", "StartTimestamp", "EndTimestamp", "NotifyUsers")) + } + if (set.Frequency != nil && set.StartTimestamp == nil) || (set.Frequency == nil && set.StartTimestamp != nil) { errs = append(errs, errors.New("must specify frequency and start time together")) } } @@ -330,10 +332,11 @@ func (v *resourceMonitors) Alter(ctx context.Context, id AccountObjectIdentifier type ResourceMonitorSet struct { // at least one - CreditQuota *int `ddl:"parameter,equals" sql:"CREDIT_QUOTA"` - Frequency *Frequency `ddl:"parameter,equals" sql:"FREQUENCY"` - StartTimestamp *string `ddl:"parameter,equals,single_quotes" sql:"START_TIMESTAMP"` - EndTimestamp *string `ddl:"parameter,equals,single_quotes" sql:"END_TIMESTAMP"` + CreditQuota *int `ddl:"parameter,equals" sql:"CREDIT_QUOTA"` + Frequency *Frequency `ddl:"parameter,equals" sql:"FREQUENCY"` + StartTimestamp *string `ddl:"parameter,equals,single_quotes" sql:"START_TIMESTAMP"` + EndTimestamp *string `ddl:"parameter,equals,single_quotes" sql:"END_TIMESTAMP"` + NotifyUsers *NotifyUsers `ddl:"parameter,equals" sql:"NOTIFY_USERS"` } // dropResourceMonitorOptions is based on https://docs.snowflake.com/en/sql-reference/sql/drop-resource-monitor. diff --git a/pkg/sdk/resource_monitors_test.go b/pkg/sdk/resource_monitors_test.go index 532820bac1..b44d0ec85b 100644 --- a/pkg/sdk/resource_monitors_test.go +++ b/pkg/sdk/resource_monitors_test.go @@ -62,7 +62,15 @@ func TestResourceMonitorAlter(t *testing.T) { opts := &AlterResourceMonitorOptions{ name: id, } - assertOptsInvalidJoinedErrors(t, opts, errAtLeastOneOf("AlterResourceMonitorOptions", "Set", "NotifyUsers", "Triggers")) + assertOptsInvalidJoinedErrors(t, opts, errAtLeastOneOf("AlterResourceMonitorOptions", "Set", "Triggers")) + }) + + t.Run("validation: no option for set provided", func(t *testing.T) { + opts := &AlterResourceMonitorOptions{ + name: id, + Set: &ResourceMonitorSet{}, + } + assertOptsInvalidJoinedErrors(t, opts, errAtLeastOneOf("ResourceMonitorSet", "CreditQuota", "Frequency", "StartTimestamp", "EndTimestamp", "NotifyUsers")) }) t.Run("with a single set", func(t *testing.T) { @@ -76,6 +84,21 @@ func TestResourceMonitorAlter(t *testing.T) { assertOptsValidAndSQLEquals(t, opts, "ALTER RESOURCE MONITOR %s SET CREDIT_QUOTA = %d", id.FullyQualifiedName(), *newCreditQuota) }) + t.Run("set notify users", func(t *testing.T) { + opts := &AlterResourceMonitorOptions{ + name: id, + Set: &ResourceMonitorSet{ + NotifyUsers: &NotifyUsers{ + Users: []NotifiedUser{ + {Name: "user1"}, + {Name: "user2"}, + }, + }, + }, + } + assertOptsValidAndSQLEquals(t, opts, "ALTER RESOURCE MONITOR %s SET NOTIFY_USERS = (\"user1\", \"user2\")", id.FullyQualifiedName()) + }) + t.Run("with a multitple set", func(t *testing.T) { newCreditQuota := Int(50) newFrequency := FrequencyYearly diff --git a/pkg/sdk/testint/resource_monitors_integration_test.go b/pkg/sdk/testint/resource_monitors_integration_test.go index 0a359dc9cc..106a118f85 100644 --- a/pkg/sdk/testint/resource_monitors_integration_test.go +++ b/pkg/sdk/testint/resource_monitors_integration_test.go @@ -211,6 +211,30 @@ func TestInt_ResourceMonitorAlter(t *testing.T) { assert.Equal(t, creditQuota, int(resourceMonitor.CreditQuota)) }) + t.Run("when changing notify users", func(t *testing.T) { + resourceMonitor, resourceMonitorCleanup := createResourceMonitor(t, client) + t.Cleanup(resourceMonitorCleanup) + alterOptions := &sdk.AlterResourceMonitorOptions{ + Set: &sdk.ResourceMonitorSet{ + NotifyUsers: &sdk.NotifyUsers{ + Users: []sdk.NotifiedUser{{Name: "ARTUR_SAWICKI"}}, + }, + }, + } + err := client.ResourceMonitors.Alter(ctx, resourceMonitor.ID(), alterOptions) + require.NoError(t, err) + resourceMonitors, err := client.ResourceMonitors.Show(ctx, &sdk.ShowResourceMonitorOptions{ + Like: &sdk.Like{ + Pattern: sdk.String(resourceMonitor.Name), + }, + }) + require.NoError(t, err) + assert.Equal(t, 1, len(resourceMonitors)) + resourceMonitor = &resourceMonitors[0] + assert.Len(t, resourceMonitor.NotifyUsers, 1) + assert.Equal(t, "ARTUR_SAWICKI", resourceMonitor.NotifyUsers[0]) + }) + t.Run("when changing scheduling info", func(t *testing.T) { resourceMonitor, resourceMonitorCleanup := createResourceMonitor(t, client) t.Cleanup(resourceMonitorCleanup) @@ -256,11 +280,11 @@ func TestInt_ResourceMonitorAlter(t *testing.T) { alterOptions := &sdk.AlterResourceMonitorOptions{ Set: &sdk.ResourceMonitorSet{ CreditQuota: &creditQuota, + NotifyUsers: &sdk.NotifyUsers{ + Users: []sdk.NotifiedUser{{Name: "ARTUR_SAWICKI"}}, + }, }, Triggers: newTriggers, - NotifyUsers: &sdk.NotifyUsers{ - Users: []sdk.NotifiedUser{{Name: "ARTUR_SAWICKI"}}, - }, } err := client.ResourceMonitors.Alter(ctx, resourceMonitor.ID(), alterOptions) require.NoError(t, err)