Skip to content

Commit c951947

Browse files
l-withl-with
authored
Merge pull request #152 from schoren/fix-nilchecks
Fix nilchecks
2 parents 31afea8 + f744ac5 commit c951947

File tree

8 files changed

+201
-26
lines changed

8 files changed

+201
-26
lines changed

‎.gitignore‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,6 @@ terraform.tfstate*
1313

1414
# terraform provider
1515
bin
16-
16+
terraform-provider-*
1717
# try
1818
try

‎mailcow/data_source_dkim_test.go‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,10 @@ package mailcow
22

33
import (
44
"fmt"
5-
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
65
"regexp"
7-
)
8-
9-
import (
106
"testing"
7+
8+
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
119
)
1210

1311
func TestAccDataSourceDkim(t *testing.T) {
@@ -51,9 +49,9 @@ data "mailcow_dkim" "demo" {
5149
}
5250

5351
func testAccDataSourceDkimError() string {
54-
return fmt.Sprintf(`
52+
return `
5553
data "mailcow_dkim" "error" {
5654
domain = "xyzzy"
5755
}
58-
`)
56+
`
5957
}

‎mailcow/data_source_domain_test.go‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,10 @@ package mailcow
22

33
import (
44
"fmt"
5-
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
65
"regexp"
7-
)
8-
9-
import (
106
"testing"
7+
8+
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
119
)
1210

1311
func TestAccDataSourceDomain(t *testing.T) {
@@ -44,9 +42,9 @@ data "mailcow_domain" "demo" {
4442
}
4543

4644
func testAccDataSourceDomainError() string {
47-
return fmt.Sprintf(`
45+
return `
4846
data "mailcow_domain" "error" {
4947
domain = "xyzzy"
5048
}
51-
`)
49+
`
5250
}

‎mailcow/data_source_mailbox_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,9 @@ data "mailcow_mailbox" "mailbox" {
6464
}
6565

6666
func testAccDataSourceMailboxError() string {
67-
return fmt.Sprintf(`
67+
return `
6868
data "mailcow_mailbox" "mailbox" {
6969
address = "xyzzy"
7070
}
71-
`)
71+
`
7272
}

‎mailcow/resource_mailbox.go‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -190,17 +190,23 @@ func resourceMailboxRead(ctx context.Context, d *schema.ResourceData, m interfac
190190
}
191191
mailbox["address"] = id
192192
mailbox["full_name"] = mailbox["name"]
193-
mailbox["quota"] = int(mailbox["quota"].(float64)) / (1024 * 1024)
193+
if mailbox["quota"] != nil {
194+
mailbox["quota"] = int(mailbox["quota"].(float64)) / (1024 * 1024)
195+
} else {
196+
mailbox["quota"] = 0
197+
}
194198

195199
excludeAndAttributes := append(exclude, mailboxAttributes...)
196200
err = setResourceData(resourceMailbox(), d, &mailbox, &excludeAndAttributes, nil)
197201
if err != nil {
198202
return diag.FromErr(err)
199203
}
200-
attributes := mailbox["attributes"].(map[string]interface{})
201-
err = setResourceData(resourceMailbox(), d, &attributes, &exclude, &mailboxAttributes)
202-
if err != nil {
203-
return diag.FromErr(err)
204+
if mailbox["attributes"] != nil {
205+
attributes := mailbox["attributes"].(map[string]interface{})
206+
err = setResourceData(resourceMailbox(), d, &attributes, &exclude, &mailboxAttributes)
207+
if err != nil {
208+
return diag.FromErr(err)
209+
}
204210
}
205211

206212
d.SetId(id)

‎mailcow/resource_mailbox_unit_test.go‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
package mailcow
2+
3+
import (
4+
"testing"
5+
)
6+
7+
// TestMailboxQuotaNilHandling tests that nil quota values are handled correctly
8+
func TestMailboxQuotaNilHandling(t *testing.T) {
9+
testCases := []struct {
10+
name string
11+
quotaValue interface{}
12+
expectedQuota int
13+
shouldPanic bool
14+
}{
15+
{
16+
name: "nil quota should return 0",
17+
quotaValue: nil,
18+
expectedQuota: 0,
19+
shouldPanic: false,
20+
},
21+
{
22+
name: "valid quota in bytes should convert to MB",
23+
quotaValue: float64(5368709120), // 5GB in bytes
24+
expectedQuota: 5120, // 5GB in MB
25+
shouldPanic: false,
26+
},
27+
{
28+
name: "zero quota should return 0",
29+
quotaValue: float64(0),
30+
expectedQuota: 0,
31+
shouldPanic: false,
32+
},
33+
{
34+
name: "small quota should round down",
35+
quotaValue: float64(1048575), // Just under 1MB
36+
expectedQuota: 0,
37+
shouldPanic: false,
38+
},
39+
}
40+
41+
for _, tc := range testCases {
42+
t.Run(tc.name, func(t *testing.T) {
43+
mailbox := map[string]interface{}{
44+
"quota": tc.quotaValue,
45+
}
46+
47+
// Simulate the quota conversion logic
48+
var quota int
49+
if mailbox["quota"] != nil {
50+
quota = int(mailbox["quota"].(float64)) / (1024 * 1024)
51+
} else {
52+
quota = 0
53+
}
54+
55+
if quota != tc.expectedQuota {
56+
t.Errorf("Expected quota %d, got %d", tc.expectedQuota, quota)
57+
}
58+
})
59+
}
60+
}
61+
62+
// TestMailboxAttributesNilHandling tests that nil attributes are handled correctly
63+
func TestMailboxAttributesNilHandling(t *testing.T) {
64+
testCases := []struct {
65+
name string
66+
attributesValue interface{}
67+
shouldProcess bool
68+
}{
69+
{
70+
name: "nil attributes should be skipped",
71+
attributesValue: nil,
72+
shouldProcess: false,
73+
},
74+
{
75+
name: "valid attributes should be processed",
76+
attributesValue: map[string]interface{}{
77+
"force_pw_update": true,
78+
"sogo_access": true,
79+
},
80+
shouldProcess: true,
81+
},
82+
{
83+
name: "empty attributes map should be processed",
84+
attributesValue: map[string]interface{}{},
85+
shouldProcess: true,
86+
},
87+
}
88+
89+
for _, tc := range testCases {
90+
t.Run(tc.name, func(t *testing.T) {
91+
mailbox := map[string]interface{}{
92+
"attributes": tc.attributesValue,
93+
}
94+
95+
// Simulate the attributes processing logic
96+
processed := false
97+
if mailbox["attributes"] != nil {
98+
_ = mailbox["attributes"].(map[string]interface{})
99+
processed = true
100+
}
101+
102+
if processed != tc.shouldProcess {
103+
t.Errorf("Expected processed=%v, got %v", tc.shouldProcess, processed)
104+
}
105+
})
106+
}
107+
}
108+
109+
// TestMailboxDataIntegrity tests that mailbox data is correctly transformed
110+
func TestMailboxDataIntegrity(t *testing.T) {
111+
testCases := []struct {
112+
name string
113+
mailbox map[string]interface{}
114+
expected map[string]interface{}
115+
}{
116+
{
117+
name: "complete mailbox data",
118+
mailbox: map[string]interface{}{
119+
"name": "John Doe",
120+
"quota": float64(10737418240), // 10GB in bytes
121+
"attributes": map[string]interface{}{
122+
"force_pw_update": true,
123+
"sogo_access": true,
124+
},
125+
},
126+
expected: map[string]interface{}{
127+
"full_name": "John Doe",
128+
"quota": 10240, // 10GB in MB
129+
},
130+
},
131+
{
132+
name: "mailbox with nil quota and attributes",
133+
mailbox: map[string]interface{}{
134+
"name": "Jane Doe",
135+
"quota": nil,
136+
"attributes": nil,
137+
},
138+
expected: map[string]interface{}{
139+
"full_name": "Jane Doe",
140+
"quota": 0,
141+
},
142+
},
143+
{
144+
name: "mailbox with only name",
145+
mailbox: map[string]interface{}{
146+
"name": "Test User",
147+
},
148+
expected: map[string]interface{}{
149+
"full_name": "Test User",
150+
"quota": 0,
151+
},
152+
},
153+
}
154+
155+
for _, tc := range testCases {
156+
t.Run(tc.name, func(t *testing.T) {
157+
// Simulate the transformation logic
158+
tc.mailbox["full_name"] = tc.mailbox["name"]
159+
if tc.mailbox["quota"] != nil {
160+
tc.mailbox["quota"] = int(tc.mailbox["quota"].(float64)) / (1024 * 1024)
161+
} else {
162+
tc.mailbox["quota"] = 0
163+
}
164+
165+
if tc.mailbox["full_name"] != tc.expected["full_name"] {
166+
t.Errorf("Expected full_name %v, got %v", tc.expected["full_name"], tc.mailbox["full_name"])
167+
}
168+
169+
if tc.mailbox["quota"] != tc.expected["quota"] {
170+
t.Errorf("Expected quota %v, got %v", tc.expected["quota"], tc.mailbox["quota"])
171+
}
172+
})
173+
}
174+
}

‎mailcow/resource_syncjob.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,12 @@ package mailcow
33
import (
44
"context"
55
"encoding/json"
6-
"errors"
76
"fmt"
7+
"log"
8+
89
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
910
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
1011
"github.com/l-with/terraform-provider-mailcow/api"
11-
"log"
1212
)
1313

1414
func resourceSyncjob() *schema.Resource {
@@ -281,7 +281,7 @@ func getSyncJob(ctx context.Context, c *APIClient, emailAddress string, attribut
281281
}
282282
}
283283
if syncJob == nil {
284-
return nil, errors.New(fmt.Sprintf("syncjob user2=%s and %s=%s not found", emailAddress, attributeKey, attributeValue))
284+
return nil, fmt.Errorf("syncjob user2=%s and %s=%s not found", emailAddress, attributeKey, attributeValue)
285285
}
286286
return syncJob, nil
287287
}

‎mailcow/verbs.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package mailcow
33
import (
44
"context"
55
"encoding/json"
6-
"errors"
76
"fmt"
87
"log"
98

@@ -16,13 +15,13 @@ func checkResponse(response api.MailcowResponseArray, resourceName, info string)
1615
t := *response.GetFinalType()
1716
log.Print("[TRACE] checkResponse t: ", t)
1817
if t != "success" {
19-
return errors.New(fmt.Sprintf(
18+
return fmt.Errorf(
2019
"%s '%s': %s (%s)",
2120
resourceName,
2221
info,
2322
t,
2423
*response.GetFinalMsgs(),
25-
))
24+
)
2625
}
2726
return nil
2827
}

0 commit comments

Comments
 (0)