From 3e1a7445f6cce3c90d804d12808f989524ec3d0d Mon Sep 17 00:00:00 2001
From: H0llyW00dzZ <h0llyw00dzz@pm.me>
Date: Fri, 24 Jan 2025 08:10:45 +0000
Subject: [PATCH] Improve [Client] [List Messages] Reduce Complexity (#7)

- [+] feat(list_messages.go): refactor ListMessages method to improve mailbox selection and message fetching
- [+] refactor(list_messages.go): extract address and body processing into separate methods for better readability

Reviewed-on: https://git.b0zal.io/H0llyW00dzZ/imap/pulls/7
Co-authored-by: H0llyW00dzZ <h0llyw00dzz@pm.me>
Co-committed-by: H0llyW00dzZ <h0llyw00dzz@pm.me>
---
 client/list_messages.go | 110 +++++++++++++++++++++++++++++-----------
 1 file changed, 79 insertions(+), 31 deletions(-)

diff --git a/client/list_messages.go b/client/list_messages.go
index ffa9afe..4ab34cc 100644
--- a/client/list_messages.go
+++ b/client/list_messages.go
@@ -29,23 +29,13 @@ func (c *IMAPClient) ListMessages(mailbox string, numMessages uint32, config Mes
 		return nil, fmt.Errorf("client is not connected")
 	}
 
-	mbox, err := c.client.Select(mailbox, false)
+	mbox, err := c.selectMailbox(mailbox)
 	if err != nil {
-		return nil, fmt.Errorf("failed to select %s: %v", mailbox, err)
+		return nil, err
 	}
 
-	from := uint32(1)
-	to := mbox.Messages
-	if mbox.Messages > numMessages {
-		from = mbox.Messages - numMessages + 1
-	}
-	seqset := new(imap.SeqSet)
-	seqset.AddRange(from, to)
-
-	items := []imap.FetchItem{imap.FetchEnvelope}
-	if config.GrabBody {
-		items = append(items, imap.FetchItem("BODY.PEEK[]"))
-	}
+	seqset := c.createSeqSet(mbox.Messages, numMessages)
+	items := c.getFetchItems(config)
 
 	messages := make(chan *imap.Message, numMessages)
 	done := make(chan error, 1)
@@ -53,6 +43,51 @@ func (c *IMAPClient) ListMessages(mailbox string, numMessages uint32, config Mes
 		done <- c.client.Fetch(seqset, items, messages)
 	}()
 
+	results, err := c.processMessages(messages, config)
+	if err != nil {
+		return nil, err
+	}
+
+	if err := <-done; err != nil {
+		return nil, fmt.Errorf("failed to fetch messages: %v", err)
+	}
+
+	return results, nil
+}
+
+// selectMailbox selects the specified mailbox and returns its status.
+// It returns an error if the mailbox cannot be selected.
+func (c *IMAPClient) selectMailbox(mailbox string) (*imap.MailboxStatus, error) {
+	mbox, err := c.client.Select(mailbox, false)
+	if err != nil {
+		return nil, fmt.Errorf("failed to select %s: %v", mailbox, err)
+	}
+	return mbox, nil
+}
+
+// createSeqSet creates a sequence set for fetching messages based on the total and desired number of messages.
+func (c *IMAPClient) createSeqSet(totalMessages, numMessages uint32) *imap.SeqSet {
+	from := uint32(1)
+	to := totalMessages
+	if totalMessages > numMessages {
+		from = totalMessages - numMessages + 1
+	}
+	seqset := new(imap.SeqSet)
+	seqset.AddRange(from, to)
+	return seqset
+}
+
+// getFetchItems determines which parts of the message to fetch based on the MessageConfig.
+func (c *IMAPClient) getFetchItems(config MessageConfig) []imap.FetchItem {
+	items := []imap.FetchItem{imap.FetchEnvelope}
+	if config.GrabBody {
+		items = append(items, imap.FetchItem("BODY.PEEK[]"))
+	}
+	return items
+}
+
+// processMessages processes fetched messages and extracts details based on the MessageConfig.
+func (c *IMAPClient) processMessages(messages chan *imap.Message, config MessageConfig) ([]map[string]any, error) {
 	var results []map[string]any
 	for msg := range messages {
 		details := make(map[string]any)
@@ -61,35 +96,48 @@ func (c *IMAPClient) ListMessages(mailbox string, numMessages uint32, config Mes
 			details[KeyID] = msg.Envelope.MessageId
 		}
 		if config.GrabFrom {
-			var from []string
-			for _, addr := range msg.Envelope.From {
-				from = append(from, addr.Address())
-			}
-			details[KeyFrom] = from
+			details[KeyFrom] = c.extractAddresses(msg.Envelope.From)
 		}
 		if config.GrabSubject && msg.Envelope.Subject != "" {
 			details[KeySubject] = msg.Envelope.Subject
 		}
 		if config.GrabBody {
-			for _, literal := range msg.Body {
-				buf := bytebufferpool.Get()
-				if _, err := buf.ReadFrom(literal); err == nil {
-					details[KeyBody] = buf.String()
-				}
-				buf.Reset() // Reset the buffer before returning it to the pool.
-				bytebufferpool.Put(buf)
+			body, err := c.extractBody(msg.Body)
+			if err != nil {
+				return nil, err
 			}
+			details[KeyBody] = body
 		}
 		results = append(results, details)
 	}
-
-	if err := <-done; err != nil {
-		return nil, fmt.Errorf("failed to fetch messages: %v", err)
-	}
-
 	return results, nil
 }
 
+// extractAddresses extracts email addresses from a list of IMAP addresses.
+func (c *IMAPClient) extractAddresses(addresses []*imap.Address) []string {
+	var from []string
+	for _, addr := range addresses {
+		from = append(from, addr.Address())
+	}
+	return from
+}
+
+// extractBody reads and returns the body content from the message body literals.
+func (c *IMAPClient) extractBody(body map[*imap.BodySectionName]imap.Literal) (string, error) {
+	for _, literal := range body {
+		buf := bytebufferpool.Get()
+		if _, err := buf.ReadFrom(literal); err == nil {
+			result := buf.String()
+			buf.Reset() // Reset the buffer before returning it to the pool.
+			bytebufferpool.Put(buf)
+			return result, nil
+		}
+		bytebufferpool.Put(buf)
+		return "", fmt.Errorf("failed to read body")
+	}
+	return "", nil
+}
+
 // ListUserMessages lists messages for a specific user based on the MessageConfig
 func (m *MultiUserIMAP) ListUserMessages(username, mailbox string, numMessages uint32, config MessageConfig) ([]map[string]any, error) {
 	m.mu.Lock()