Skip to content

Commit 5733b9a

Browse files
tosuketosuke
authored
Merge pull request #257 from mackerelio-labs/fixnil
Fix inconsistency between SDK and framework by nil relaxing
2 parents 63a1611 + bb6a122 commit 5733b9a

13 files changed

+385
-244
lines changed

‎internal/mackerel/alert_group_setting.go‎

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func (ag *AlertGroupSettingModel) Read(ctx context.Context, client *Client) erro
4242
return err
4343
}
4444

45-
ag.merge(newAg)
45+
*ag = newAg
4646
return nil
4747
}
4848

@@ -66,9 +66,9 @@ func newAlertGroupSetting(ag mackerel.AlertGroupSetting) AlertGroupSettingModel
6666
ID: types.StringValue(ag.ID),
6767
Name: types.StringValue(ag.Name),
6868
Memo: types.StringValue(ag.Memo),
69-
ServiceScopes: ag.ServiceScopes,
69+
ServiceScopes: nilAsEmptySlice(ag.ServiceScopes),
7070
RoleScopes: normalizeScopes(ag.RoleScopes),
71-
MonitorScopes: ag.MonitorScopes,
71+
MonitorScopes: nilAsEmptySlice(ag.MonitorScopes),
7272
NotificationInterval: types.Int64Value(int64(ag.NotificationInterval)),
7373
}
7474
}
@@ -84,17 +84,3 @@ func (ag AlertGroupSettingModel) mackerelAlertGroupSetting() mackerel.AlertGroup
8484
NotificationInterval: uint64(ag.NotificationInterval.ValueInt64()),
8585
}
8686
}
87-
88-
func (ag *AlertGroupSettingModel) merge(newAg AlertGroupSettingModel) {
89-
// Distinct null and [] by preserving old state
90-
if len(ag.ServiceScopes) == 0 && len(newAg.ServiceScopes) == 0 {
91-
newAg.ServiceScopes = ag.ServiceScopes
92-
}
93-
if len(ag.RoleScopes) == 0 && len(newAg.RoleScopes) == 0 {
94-
newAg.RoleScopes = ag.RoleScopes
95-
}
96-
if len(ag.MonitorScopes) == 0 && len(newAg.MonitorScopes) == 0 {
97-
newAg.MonitorScopes = ag.MonitorScopes
98-
}
99-
*ag = newAg
100-
}

‎internal/mackerel/alert_group_setting_test.go‎

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -74,42 +74,3 @@ func Test_AlertGroupSetting_conv(t *testing.T) {
7474
})
7575
}
7676
}
77-
78-
func Test_AlertGroupSetting_merge(t *testing.T) {
79-
t.Parallel()
80-
81-
// lhs <- rhs = wants
82-
cases := map[string]struct {
83-
lhs AlertGroupSettingModel
84-
rhs AlertGroupSettingModel
85-
wants AlertGroupSettingModel
86-
}{
87-
"nil preserving": {
88-
lhs: AlertGroupSettingModel{
89-
Name: types.StringValue("before"),
90-
},
91-
rhs: AlertGroupSettingModel{
92-
Name: types.StringValue("after"),
93-
ServiceScopes: []string{},
94-
RoleScopes: []string{},
95-
MonitorScopes: []string{},
96-
},
97-
wants: AlertGroupSettingModel{
98-
Name: types.StringValue("after"),
99-
},
100-
},
101-
}
102-
103-
for name, tt := range cases {
104-
t.Run(name, func(t *testing.T) {
105-
t.Parallel()
106-
107-
m := tt.lhs
108-
m.merge(tt.rhs)
109-
if diff := cmp.Diff(m, tt.wants); diff != "" {
110-
t.Error(diff)
111-
}
112-
113-
})
114-
}
115-
}

‎internal/mackerel/aws_integration.go‎

Lines changed: 2 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ func (m *AWSIntegrationModel) Read(_ context.Context, client *Client) error {
116116
return err
117117
}
118118

119-
m.merge(*integration)
119+
*m = *integration
120120
return nil
121121
}
122122

@@ -157,7 +157,7 @@ func newAWSIntegrationModel(aws mackerel.AWSIntegration) (*AWSIntegrationModel,
157157
continue
158158
}
159159
if len(awsService.IncludedMetrics) != 0 {
160-
return nil, fmt.Errorf("%s: IncludedMetrics is not supported.", name)
160+
return nil, fmt.Errorf("%s: IncludedMetrics is not supported", name)
161161
}
162162

163163
svcs[name] = AWSIntegrationService{
@@ -240,45 +240,6 @@ func (m *AWSIntegrationModel) updateParam() *mackerel.UpdateAWSIntegrationParam
240240
return (*mackerel.UpdateAWSIntegrationParam)(m.createParam())
241241
}
242242

243-
func (m *AWSIntegrationModel) merge(newModel AWSIntegrationModel) {
244-
oldServices := make(map[string]AWSIntegrationService)
245-
m.each(func(name string, service *AWSIntegrationService) *AWSIntegrationService {
246-
if service != nil {
247-
oldServices[name] = *service
248-
}
249-
return service
250-
})
251-
252-
newModel.SecretKey = m.SecretKey
253-
newModel.each(func(name string, service *AWSIntegrationService) *AWSIntegrationService {
254-
oldService, ok := oldServices[name]
255-
if !ok {
256-
return service
257-
}
258-
259-
// If new == nil && old == zero, use old one.
260-
if service == nil {
261-
if !oldService.Enable.ValueBool() &&
262-
len(oldService.ExcludedMetrics) == 0 &&
263-
oldService.Role.IsNull() &&
264-
!oldService.RetireAutomatically.ValueBool() {
265-
return &oldService
266-
} else {
267-
return nil
268-
}
269-
}
270-
271-
if service.Role.ValueString() == "" && oldService.Role.ValueString() == "" {
272-
service.Role = oldService.Role
273-
}
274-
if len(service.ExcludedMetrics) == 0 && len(oldService.ExcludedMetrics) == 0 {
275-
service.ExcludedMetrics = oldService.ExcludedMetrics
276-
}
277-
return service
278-
})
279-
*m = newModel
280-
}
281-
282243
type awsServiceEachFunc func(name string, service *AWSIntegrationService) *AWSIntegrationService
283244

284245
// Iterates and updates over services by name

‎internal/mackerel/downtime.go‎

Lines changed: 11 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func (d *DowntimeModel) Read(_ context.Context, client *Client) error {
7070
if err != nil {
7171
return err
7272
}
73-
d.merge(*newModel)
73+
*d = *newModel
7474
return nil
7575
}
7676

@@ -95,12 +95,12 @@ func newDowntime(d mackerel.Downtime) *DowntimeModel {
9595
Memo: types.StringValue(d.Memo),
9696
Start: types.Int64Value(d.Start),
9797
Duration: types.Int64Value(d.Duration),
98-
ServiceScopes: d.ServiceScopes,
99-
ServiceExcludeScopes: d.ServiceExcludeScopes,
100-
RoleScopes: d.RoleScopes,
101-
RoleExcludeScopes: d.RoleExcludeScopes,
102-
MonitorScopes: d.MonitorScopes,
103-
MonitorExcludeScopes: d.MonitorExcludeScopes,
98+
ServiceScopes: nilAsEmptySlice(d.ServiceScopes),
99+
ServiceExcludeScopes: nilAsEmptySlice(d.ServiceExcludeScopes),
100+
RoleScopes: nilAsEmptySlice(d.RoleScopes),
101+
RoleExcludeScopes: nilAsEmptySlice(d.RoleExcludeScopes),
102+
MonitorScopes: nilAsEmptySlice(d.MonitorScopes),
103+
MonitorExcludeScopes: nilAsEmptySlice(d.MonitorExcludeScopes),
104104
}
105105
if d.Recurrence != nil {
106106
recurrence := DowntimeRecurrence{
@@ -178,29 +178,9 @@ func (d *DowntimeModel) mackerelDowntime() *mackerel.Downtime {
178178
return mackerelDowntime
179179
}
180180

181-
func (d *DowntimeModel) merge(newModel DowntimeModel) {
182-
// preserve nil
183-
if len(d.Recurrence) == 1 && len(d.Recurrence[0].Weekdays) == 0 &&
184-
len(newModel.Recurrence) == 1 && len(newModel.Recurrence[0].Weekdays) == 0 {
185-
newModel.Recurrence[0].Weekdays = d.Recurrence[0].Weekdays
186-
}
187-
if len(d.ServiceScopes) == 0 && len(newModel.ServiceScopes) == 0 {
188-
newModel.ServiceScopes = d.ServiceScopes
189-
}
190-
if len(d.ServiceExcludeScopes) == 0 && len(newModel.ServiceExcludeScopes) == 0 {
191-
newModel.ServiceExcludeScopes = d.ServiceExcludeScopes
192-
}
193-
if len(d.RoleScopes) == 0 && len(newModel.RoleScopes) == 0 {
194-
newModel.RoleScopes = d.RoleScopes
195-
}
196-
if len(d.RoleExcludeScopes) == 0 && len(newModel.RoleExcludeScopes) == 0 {
197-
newModel.RoleExcludeScopes = d.RoleExcludeScopes
198-
}
199-
if len(d.MonitorScopes) == 0 && len(newModel.MonitorScopes) == 0 {
200-
newModel.MonitorScopes = d.MonitorScopes
201-
}
202-
if len(d.MonitorExcludeScopes) == 0 && len(newModel.MonitorExcludeScopes) == 0 {
203-
newModel.MonitorExcludeScopes = d.MonitorExcludeScopes
181+
func nilAsEmptySlice[V any](slice []V) []V {
182+
if slice == nil {
183+
return []V{}
204184
}
205-
*d = newModel
185+
return slice
206186
}

‎internal/mackerel/downtime_test.go‎

Lines changed: 32 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,17 @@ func Test_Downtime_ReadDowntime(t *testing.T) {
5757
inID: "5ghjb6vgDFN",
5858

5959
wants: DowntimeModel{
60-
ID: types.StringValue("5ghjb6vgDFN"),
61-
Name: types.StringValue("basic"),
62-
Memo: types.StringValue(""),
63-
Start: types.Int64Value(1735707600),
64-
Duration: types.Int64Value(3600),
60+
ID: types.StringValue("5ghjb6vgDFN"),
61+
Name: types.StringValue("basic"),
62+
Memo: types.StringValue(""),
63+
Start: types.Int64Value(1735707600),
64+
Duration: types.Int64Value(3600),
65+
ServiceScopes: []string{},
66+
ServiceExcludeScopes: []string{},
67+
RoleScopes: []string{},
68+
RoleExcludeScopes: []string{},
69+
MonitorScopes: []string{},
70+
MonitorExcludeScopes: []string{},
6571
},
6672
},
6773
"no downtime": {
@@ -106,17 +112,29 @@ func Test_Downtime_conv(t *testing.T) {
106112
}{
107113
"basic": {
108114
api: mackerel.Downtime{
109-
ID: "5ghjb6vgDFN",
110-
Name: "basic",
111-
Start: 1735707600,
112-
Duration: 3600,
115+
ID: "5ghjb6vgDFN",
116+
Name: "basic",
117+
Start: 1735707600,
118+
Duration: 3600,
119+
ServiceScopes: []string{},
120+
ServiceExcludeScopes: []string{},
121+
RoleScopes: []string{},
122+
RoleExcludeScopes: []string{},
123+
MonitorScopes: []string{},
124+
MonitorExcludeScopes: []string{},
113125
},
114126
model: DowntimeModel{
115-
ID: types.StringValue("5ghjb6vgDFN"),
116-
Name: types.StringValue("basic"),
117-
Memo: types.StringValue(""),
118-
Start: types.Int64Value(1735707600),
119-
Duration: types.Int64Value(3600),
127+
ID: types.StringValue("5ghjb6vgDFN"),
128+
Name: types.StringValue("basic"),
129+
Memo: types.StringValue(""),
130+
Start: types.Int64Value(1735707600),
131+
Duration: types.Int64Value(3600),
132+
ServiceScopes: []string{},
133+
ServiceExcludeScopes: []string{},
134+
RoleScopes: []string{},
135+
RoleExcludeScopes: []string{},
136+
MonitorScopes: []string{},
137+
MonitorExcludeScopes: []string{},
120138
},
121139
},
122140
"full": {
@@ -183,85 +201,3 @@ func Test_Downtime_conv(t *testing.T) {
183201
})
184202
}
185203
}
186-
187-
func Test_Downtime_merge(t *testing.T) {
188-
t.Parallel()
189-
190-
// in <- inNew == wants
191-
cases := map[string]struct {
192-
in DowntimeModel
193-
inNew DowntimeModel
194-
wants DowntimeModel
195-
}{
196-
"basic": {
197-
in: DowntimeModel{
198-
ID: types.StringValue("5ghjb6vgDFN"),
199-
Name: types.StringValue("basic"),
200-
Memo: types.StringValue(""),
201-
Start: types.Int64Value(1735707600),
202-
Duration: types.Int64Value(3600),
203-
Recurrence: []DowntimeRecurrence{{
204-
Type: types.StringValue("weekly"),
205-
Interval: types.Int64Value(2),
206-
Weekdays: nil,
207-
Until: types.Int64Value(1767193199),
208-
}},
209-
ServiceScopes: nil,
210-
ServiceExcludeScopes: nil,
211-
RoleScopes: nil,
212-
RoleExcludeScopes: nil,
213-
MonitorScopes: nil,
214-
MonitorExcludeScopes: []string{},
215-
},
216-
inNew: DowntimeModel{
217-
ID: types.StringValue("5ghjb6vgDFN"),
218-
Name: types.StringValue("basic"),
219-
Memo: types.StringValue("memo"), // changed
220-
Start: types.Int64Value(1735707600),
221-
Duration: types.Int64Value(3600),
222-
Recurrence: []DowntimeRecurrence{{
223-
Type: types.StringValue("weekly"),
224-
Interval: types.Int64Value(2),
225-
Weekdays: []string{},
226-
Until: types.Int64Value(1767193199),
227-
}},
228-
ServiceScopes: []string{},
229-
ServiceExcludeScopes: []string{},
230-
RoleScopes: []string{},
231-
RoleExcludeScopes: []string{},
232-
MonitorScopes: []string{},
233-
MonitorExcludeScopes: nil,
234-
},
235-
wants: DowntimeModel{
236-
ID: types.StringValue("5ghjb6vgDFN"),
237-
Name: types.StringValue("basic"),
238-
Memo: types.StringValue("memo"), // changed
239-
Start: types.Int64Value(1735707600),
240-
Duration: types.Int64Value(3600),
241-
Recurrence: []DowntimeRecurrence{{
242-
Type: types.StringValue("weekly"),
243-
Interval: types.Int64Value(2),
244-
Weekdays: nil,
245-
Until: types.Int64Value(1767193199),
246-
}},
247-
ServiceScopes: nil,
248-
ServiceExcludeScopes: nil,
249-
RoleScopes: nil,
250-
RoleExcludeScopes: nil,
251-
MonitorScopes: nil,
252-
MonitorExcludeScopes: []string{},
253-
},
254-
},
255-
}
256-
257-
for name, tt := range cases {
258-
t.Run(name, func(t *testing.T) {
259-
t.Parallel()
260-
261-
tt.in.merge(tt.inNew)
262-
if diff := cmp.Diff(tt.in, tt.wants); diff != "" {
263-
t.Error(diff)
264-
}
265-
})
266-
}
267-
}

‎internal/planmodifierutil/nil_relaxed.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ func NilRelaxedSet() planmodifier.Set {
1515
return nilRelaxedModifier{}
1616
}
1717

18+
func NilRelaxedList() planmodifier.List {
19+
return nilRelaxedModifier{}
20+
}
21+
1822
type nilRelaxedModifier struct{}
1923

2024
const desctiprion = "For compatibility with the states created by SDK provider, Terraform consider nil and zero values to be same."
@@ -42,3 +46,11 @@ func (_ nilRelaxedModifier) PlanModifySet(ctx context.Context, req planmodifier.
4246
resp.PlanValue = req.StateValue
4347
}
4448
}
49+
50+
func (_ nilRelaxedModifier) PlanModifyList(ctx context.Context, req planmodifier.ListRequest, resp *planmodifier.ListResponse) {
51+
if req.PlanValue.IsUnknown() {
52+
resp.PlanValue = types.ListNull(req.PlanValue.ElementType(ctx))
53+
} else if req.PlanValue.IsNull() && len(req.StateValue.Elements()) == 0 {
54+
resp.PlanValue = req.StateValue
55+
}
56+
}

0 commit comments

Comments
 (0)