Skip to content

Commit 62a1f2e

Browse files
[FIX] refactor tests and improve error handling
1 parent 505ee83 commit 62a1f2e

File tree

18 files changed

+80
-70
lines changed

18 files changed

+80
-70
lines changed

apperror/apperror_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ func TestCatchFunction(t *testing.T) {
351351
}, "operation failed")
352352
}
353353

354-
func TestCatchFunctionSuccess(t *testing.T) {
354+
func TestCatchFunctionSuccess(_ *testing.T) {
355355
// Test successful operation (should not panic)
356356
apperror.Catch(func() error {
357357
return nil

cache/redis.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ func (rc *RedisCache) GetMulti(ctx context.Context, keys []string) (map[string]i
251251
rc.updateStats(func(s *Stats) { s.Hits++ })
252252
continue
253253
}
254-
254+
255255
rc.updateStats(func(s *Stats) { s.Misses++ })
256256
}
257257
}

database/config.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ func (c *Config) Validate() error {
4747
return nil
4848
}
4949

50+
// Changed checks if the configuration has changed
5051
func (c *Config) Changed(n *Config) bool {
5152
if c.Driver != n.Driver {
5253
return true

database/database_test.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,10 @@ func TestExecuteWithoutConnection(t *testing.T) {
157157
// Ensure we're in a disconnected state for this test
158158
// Only disconnect if currently connected to avoid hanging
159159
if database.Connected() {
160-
database.Disconnect()
160+
err := database.Disconnect()
161+
if err != nil {
162+
t.Errorf("Disconnect should not return an error: %v", err)
163+
}
161164
time.Sleep(100 * time.Millisecond) // Wait for disconnection
162165
}
163166

@@ -184,7 +187,10 @@ func TestReconnect(t *testing.T) {
184187
func TestAwaitConnectionTimeout(t *testing.T) {
185188
// Ensure we start in a disconnected state for this test
186189
if database.Connected() {
187-
database.Disconnect()
190+
err := database.Disconnect()
191+
if err != nil {
192+
t.Errorf("Disconnect should not return an error: %v", err)
193+
}
188194
time.Sleep(200 * time.Millisecond) // Wait for disconnection
189195
}
190196

@@ -230,7 +236,10 @@ func TestConnectWithInvalidConfig(t *testing.T) {
230236
}
231237

232238
// Clean up
233-
database.Disconnect()
239+
err := database.Disconnect()
240+
if err != nil {
241+
t.Errorf("Disconnect should not return an error: %v", err)
242+
}
234243
}
235244

236245
func TestConnectWithSQLiteConfig(t *testing.T) {

mail/config.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,7 @@ func (c *ClientConfig) TLSConfig() *tls.Config {
209209
}
210210
}
211211

212+
// Validate checks the client configuration for errors
212213
func (c *ClientConfig) Validate() error {
213214
if c.Host == "" {
214215
return apperror.NewError("SMTP host is required")
@@ -240,6 +241,7 @@ func (c *ServerConfig) TLSConfig() *tls.Config {
240241
}
241242
}
242243

244+
// Validate checks the server configuration for errors
243245
func (c *ServerConfig) Validate() error {
244246
if c.Host == "" {
245247
return apperror.NewError("SMTP server host is required")
@@ -256,6 +258,7 @@ func (c *ServerConfig) Validate() error {
256258
return nil
257259
}
258260

261+
// Validate checks the queue configuration for errors
259262
func (c *QueueConfig) Validate() error {
260263
if c.QueueName == "" {
261264
return apperror.NewError("Queue name is required")
@@ -269,13 +272,15 @@ func (c *QueueConfig) Validate() error {
269272
return nil
270273
}
271274

275+
// Validate checks the template configuration for errors
272276
func (c *TemplateConfig) Validate() error {
273277
if c.DefaultTemplate == "" {
274278
return apperror.NewError("Default template is required")
275279
}
276280
return nil
277281
}
278282

283+
// Validate checks the configuration for errors
279284
func (c *Config) Validate() error {
280285
if err := c.Client.Validate(); err != nil {
281286
return apperror.Wrap(err)

mail/config_test.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -555,16 +555,13 @@ func validateSecurityConfig(config mail.SecurityConfig) bool {
555555
return true
556556
}
557557

558-
func validateQueueConfig(config mail.QueueConfig) bool {
558+
func validateQueueConfig(_ mail.QueueConfig) bool {
559559
// Basic queue config is always valid
560560
return true
561561
}
562562

563563
func validateTemplateConfig(config mail.TemplateConfig) bool {
564-
if config.DefaultTemplate == "" {
565-
return false
566-
}
567-
return true
564+
return config.DefaultTemplate != ""
568565
}
569566

570567
func isValidIPOrCIDR(s string) bool {

mail/internal/email/email.go

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
// Package email allows to send email messages.
2+
// The package is used internally by the mail package.
13
package email
24

35
import (
@@ -26,8 +28,10 @@ import (
2628
)
2729

2830
const (
29-
MaxLineLength = 76 // MaxLineLength is the maximum line length per RFC 2045
30-
DefaultContentType = "text/plain; charset=us-ascii" // DefaultContentType is the default Content-Type according to RFC 2045, section 5.2
31+
// MaxLineLength is the maximum line length per RFC 2045
32+
MaxLineLength = 76
33+
// DefaultContentType is the default Content-Type according to RFC 2045, section 5.2
34+
DefaultContentType = "text/plain; charset=us-ascii"
3135
)
3236

3337
var maxBigInt = big.NewInt(math.MaxInt64)
@@ -144,10 +148,10 @@ func NewFromReader(r io.Reader) (*Email, error) {
144148
continue
145149
}
146150
}
147-
switch {
148-
case contentType == "text/plain":
151+
switch contentType {
152+
case "text/plain":
149153
msg.Text = p.body
150-
case contentType == "text/html":
154+
case "text/html":
151155
msg.HTML = p.body
152156
}
153157
}
@@ -174,11 +178,11 @@ func (e *Email) Attach(r io.Reader, filename string, contentType string) (*Attac
174178
// AttachFile is used to attach content to the email.
175179
// It attempts to open the file referenced by filename and, if successful, creates an Attachment.
176180
func (e *Email) AttachFile(filename string) (*Attachment, error) {
177-
f, err := os.Open(filename)
181+
f, err := os.Open(filepath.Clean(filename))
178182
if err != nil {
179183
return nil, apperror.NewError("could not open attachment file").AddError(err)
180184
}
181-
defer f.Close()
185+
defer apperror.Catch(f.Close, "could not close attachment file")
182186

183187
ct := mime.TypeByExtension(filepath.Ext(filename))
184188
basename := filepath.Base(filename)
@@ -301,7 +305,10 @@ func (e *Email) Bytes() ([]byte, error) {
301305
}
302306

303307
if isMixed || isAlternative {
304-
relatedWriter.Close()
308+
err = relatedWriter.Close()
309+
if err != nil {
310+
return nil, apperror.NewError("could not close multipart/related part").AddError(err)
311+
}
305312
}
306313
}
307314
}
@@ -370,7 +377,7 @@ func (e *Email) Send(address string, auth smtp.Auth, helo string) error {
370377
if err != nil {
371378
return apperror.NewError("could not dial SMTP connection").AddError(err)
372379
}
373-
defer conn.Close()
380+
defer apperror.Catch(conn.Close, "could not close SMTP connection")
374381

375382
// Send custom HELO
376383
err = conn.Hello(helo)
@@ -447,12 +454,12 @@ func (e *Email) SendWithTLS(address string, auth smtp.Auth, config *tls.Config,
447454
if err != nil {
448455
return apperror.NewError("could not dial TLS connection").AddError(err)
449456
}
450-
defer conn.Close()
457+
defer apperror.Catch(conn.Close, "could not close TLS connection")
451458
c, err := smtp.NewClient(conn, strings.Split(address, ":")[0])
452459
if err != nil {
453460
return apperror.NewError("could not create SMTP client").AddError(err)
454461
}
455-
defer c.Quit()
462+
defer apperror.Catch(c.Quit, "could not quit SMTP session")
456463

457464
// Send custom HELO if provided (after connection but before auth)
458465
if helo != "" {
@@ -524,7 +531,7 @@ func (e *Email) SendWithStartTLS(address string, auth smtp.Auth, config *tls.Con
524531
if err != nil {
525532
return apperror.NewError("could not dial SMTP connection").AddError(err)
526533
}
527-
defer conn.Close()
534+
defer apperror.Catch(conn.Close, "could not close SMTP connection")
528535

529536
// Send custom HELO if provided (before STARTTLS)
530537
if helo != "" {
@@ -836,13 +843,13 @@ func headerToBytes(buff io.Writer, header textproto.MIMEHeader) error {
836843
if err != nil {
837844
return apperror.NewError("could not write header field separator").AddError(err)
838845
}
839-
switch {
840-
case field == "Content-Type" || field == "Content-Disposition":
846+
switch field {
847+
case "Content-Type", "Content-Disposition":
841848
_, err := buff.Write([]byte(subval))
842849
if err != nil {
843850
return apperror.NewError("could not write header field value").AddError(err)
844851
}
845-
case field == "From" || field == "To" || field == "Cc" || field == "Bcc":
852+
case "From", "To", "Cc", "Bcc":
846853
_, err := buff.Write([]byte(subval))
847854
if err != nil {
848855
return apperror.NewError("could not write header field value").AddError(err)

mail/internal/email/email_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ func TestEmail_AttachFile(t *testing.T) {
5454
tempFile := filepath.Join(tempDir, "test.txt")
5555
content := "test file content"
5656

57-
err := os.WriteFile(tempFile, []byte(content), 0644)
57+
err := os.WriteFile(tempFile, []byte(content), 0600)
5858
if err != nil {
5959
t.Fatalf("Failed to create temp file: %v", err)
6060
}

mail/mail_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -424,11 +424,11 @@ func TestManagerSendAsyncQueueDisabled(t *testing.T) {
424424
}
425425
}
426426

427-
func TestManagerAddNotificationHandler(t *testing.T) {
427+
func TestManagerAddNotificationHandler(_ *testing.T) {
428428
config := mail.DefaultConfig()
429429
manager := mail.NewManager(config, nil)
430430

431-
handler := func(ctx context.Context, from string, to []string, data io.Reader) error {
431+
handler := func(_ context.Context, _ string, to []string, _ io.Reader) error {
432432
return nil
433433
}
434434

mail/security.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -59,17 +59,17 @@ func NewSecurityManager(config SecurityConfig) *SecurityManager {
5959
sm.allowedNetworks = append(sm.allowedNetworks, network)
6060
continue
6161
}
62-
62+
6363
ip := net.ParseIP(ipStr)
6464
if ip == nil {
6565
continue
6666
}
67-
67+
6868
// Single IP address - convert to /32 or /128 network
6969
var network *net.IPNet
70-
_, network, _ = net.ParseCIDR(ipStr + "/128") // Default to IPv6
70+
_, network, _ = net.ParseCIDR(ipStr + "/128") // Default to IPv6
7171
if ip.To4() != nil {
72-
_, network, _ = net.ParseCIDR(ipStr + "/32") // Override for IPv4
72+
_, network, _ = net.ParseCIDR(ipStr + "/32") // Override for IPv4
7373
}
7474
sm.allowedNetworks = append(sm.allowedNetworks, network)
7575
}
@@ -80,17 +80,17 @@ func NewSecurityManager(config SecurityConfig) *SecurityManager {
8080
sm.blockedNetworks = append(sm.blockedNetworks, network)
8181
continue
8282
}
83-
83+
8484
ip := net.ParseIP(ipStr)
8585
if ip == nil {
8686
continue
8787
}
88-
88+
8989
// Single IP address - convert to /32 or /128 network
9090
var network *net.IPNet
91-
_, network, _ = net.ParseCIDR(ipStr + "/128") // Default to IPv6
91+
_, network, _ = net.ParseCIDR(ipStr + "/128") // Default to IPv6
9292
if ip.To4() != nil {
93-
_, network, _ = net.ParseCIDR(ipStr + "/32") // Override for IPv4
93+
_, network, _ = net.ParseCIDR(ipStr + "/32") // Override for IPv4
9494
}
9595
sm.blockedNetworks = append(sm.blockedNetworks, network)
9696
}
@@ -154,7 +154,7 @@ func (sm *SecurityManager) ValidateConnection(remoteAddr string) error {
154154
}
155155
return &SecurityError{Type: "auth_blocked", Message: "IP temporarily blocked due to authentication failures"}
156156
}
157-
157+
158158
// Reset auth failure tracking after window expires
159159
tracker.blocked = false
160160
tracker.failures = 0

0 commit comments

Comments
 (0)