Merge pull request #137 from emersion/updates-chan

Replace various update channels with one unique channel
emersion · Aug 16, 2017 · 1cb024e · 1cb024e
2 parents dc58f30 + 930b158
commit 1cb024e
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 75 deletions.
diff --git a/client/client.go b/client/client.go
@@ -22,6 +22,26 @@ var errClosed = fmt.Errorf("imap: connection closed")
 // errUnregisterHandler is returned by a response handler to unregister itself.
 var errUnregisterHandler = fmt.Errorf("imap: unregister handler")
 
+// StatusUpdate is delivered when a status update is received.
+type StatusUpdate struct {
+	Status *imap.StatusResp
+}
+
+// MailboxUpdate is delivered when a mailbox status changes.
+type MailboxUpdate struct {
+	Mailbox *imap.MailboxStatus
+}
+
+// ExpungeUpdate is delivered when a message is deleted.
+type ExpungeUpdate struct {
+	SeqNum uint32
+}
+
+// MessageUpdate is delivered when a message attribute changes.
+type MessageUpdate struct {
+	Message *imap.Message
+}
+
 // Client is an IMAP client.
 type Client struct {
 	conn  *imap.Conn
@@ -43,20 +63,12 @@ type Client struct {
 	// access.
 	locker sync.Mutex
 
-	// A channel where info messages from the server will be sent.
-	Infos chan *imap.StatusResp
-	// A channel where warning messages from the server will be sent.
-	Warnings chan *imap.StatusResp
-	// A channel where error messages from the server will be sent.
-	Errors chan *imap.StatusResp
-	// A channel where bye messages from the server will be sent.
-	Byes chan *imap.StatusResp
-	// A channel where mailbox updates from the server will be sent.
-	MailboxUpdates chan *imap.MailboxStatus
-	// A channel where deleted message IDs will be sent.
-	Expunges chan uint32
-	// A channel where messages updates from the server will be sent.
-	MessageUpdates chan *imap.Message
+	// A channel to which unilateral updates from the server will be sent. An
+	// update can be one of: *StatusUpdate, *MailboxUpdate, *MessageUpdate,
+	// *ExpungeUpdate. Note that blocking this channel blocks the whole client,
+	// so it's recommended to use a separate goroutine and a buffered channel to
+	// prevent deadlocks.
+	Updates chan<- interface{}
 
 	// ErrorLog specifies an optional logger for errors accepting
 	// connections and unexpected behavior from handlers.
@@ -291,23 +303,9 @@ func (c *Client) handleUnilateral() {
 			}
 
 			switch resp.Type {
-			case imap.StatusOk:
-				if c.Infos != nil {
-					go func() {
-						c.Infos <- resp
-					}()
-				}
-			case imap.StatusNo:
-				if c.Warnings != nil {
-					go func() {
-						c.Warnings <- resp
-					}()
-				}
-			case imap.StatusBad:
-				if c.Errors != nil {
-					go func() {
-						c.Errors <- resp
-					}()
+			case imap.StatusOk, imap.StatusNo, imap.StatusBad:
+				if c.Updates != nil {
+					c.Updates <- &StatusUpdate{resp}
 				}
 			case imap.StatusBye:
 				c.locker.Lock()
@@ -317,10 +315,8 @@ func (c *Client) handleUnilateral() {
 
 				c.conn.Close()
 
-				if c.Byes != nil {
-					go func() {
-						c.Byes <- resp
-					}()
+				if c.Updates != nil {
+					c.Updates <- &StatusUpdate{resp}
 				}
 			default:
 				return responses.ErrUnhandled
@@ -349,10 +345,8 @@ func (c *Client) handleUnilateral() {
 					c.mailbox.ItemsLocker.Unlock()
 				}
 
-				if c.MailboxUpdates != nil {
-					go func() {
-						c.MailboxUpdates <- c.Mailbox()
-					}()
+				if c.Updates != nil {
+					c.Updates <- &MailboxUpdate{c.Mailbox()}
 				}
 			case "RECENT":
 				if c.Mailbox() == nil {
@@ -369,16 +363,14 @@ func (c *Client) handleUnilateral() {
 					c.mailbox.ItemsLocker.Unlock()
 				}
 
-				if c.MailboxUpdates != nil {
-					go func() {
-						c.MailboxUpdates <- c.Mailbox()
-					}()
+				if c.Updates != nil {
+					c.Updates <- &MailboxUpdate{c.Mailbox()}
 				}
 			case "EXPUNGE":
 				seqNum, _ := imap.ParseNumber(fields[0])
 
-				if c.Expunges != nil {
-					c.Expunges <- seqNum
+				if c.Updates != nil {
+					c.Updates <- &ExpungeUpdate{seqNum}
 				}
 			case "FETCH":
 				seqNum, _ := imap.ParseNumber(fields[0])
@@ -389,10 +381,8 @@ func (c *Client) handleUnilateral() {
 					break
 				}
 
-				if c.MessageUpdates != nil {
-					go func() {
-						c.MessageUpdates <- msg
-					}()
+				if c.Updates != nil {
+					c.Updates <- &MessageUpdate{msg}
 				}
 			default:
 				return responses.ErrUnhandled

diff --git a/client/client_test.go b/client/client_test.go
@@ -133,51 +133,41 @@ func TestClient_unilateral(t *testing.T) {
 
 	setClientState(c, imap.SelectedState, imap.NewMailboxStatus("INBOX", nil))
 
-	statuses := make(chan *imap.MailboxStatus, 1)
-	c.MailboxUpdates = statuses
-	expunges := make(chan uint32, 1)
-	c.Expunges = expunges
-	messages := make(chan *imap.Message, 1)
-	c.MessageUpdates = messages
-	infos := make(chan *imap.StatusResp, 1)
-	c.Infos = infos
-	warns := make(chan *imap.StatusResp, 1)
-	c.Warnings = warns
-	errors := make(chan *imap.StatusResp, 1)
-	c.Errors = errors
+	updates := make(chan interface{}, 1)
+	c.Updates = updates
 
 	s.WriteString("* 42 EXISTS\r\n")
-	if status := <-statuses; status.Messages != 42 {
-		t.Errorf("Invalid messages count: expected %v but got %v", 42, status.Messages)
+	if update, ok := (<-updates).(*MailboxUpdate); !ok || update.Mailbox.Messages != 42 {
+		t.Errorf("Invalid messages count: expected %v but got %v", 42, update.Mailbox.Messages)
 	}
 
 	s.WriteString("* 587 RECENT\r\n")
-	if status := <-statuses; status.Recent != 587 {
-		t.Errorf("Invalid recent count: expected %v but got %v", 587, status.Recent)
+	if update, ok := (<-updates).(* MailboxUpdate); !ok || update.Mailbox.Recent != 587 {
+		t.Errorf("Invalid recent count: expected %v but got %v", 587, update.Mailbox.Recent)
 	}
 
 	s.WriteString("* 65535 EXPUNGE\r\n")
-	if seqNum := <-expunges; seqNum != 65535 {
-		t.Errorf("Invalid expunged sequence number: expected %v but got %v", 65535, seqNum)
+	if update, ok := (<-updates).(*ExpungeUpdate); !ok || update.SeqNum != 65535 {
+		t.Errorf("Invalid expunged sequence number: expected %v but got %v", 65535, update.SeqNum)
 	}
 
 	s.WriteString("* 431 FETCH (FLAGS (\\Seen))\r\n")
-	if msg := <-messages; msg.SeqNum != 431 {
-		t.Errorf("Invalid expunged sequence number: expected %v but got %v", 431, msg.SeqNum)
+	if update, ok := (<-updates).(*MessageUpdate); !ok || update.Message.SeqNum != 431 {
+		t.Errorf("Invalid expunged sequence number: expected %v but got %v", 431, update.Message.SeqNum)
 	}
 
 	s.WriteString("* OK Reticulating splines...\r\n")
-	if status := <-infos; status.Info != "Reticulating splines..." {
-		t.Errorf("Invalid info: got %v", status.Info)
+	if update, ok := (<-updates).(*StatusUpdate); !ok || update.Status.Info != "Reticulating splines..." {
+		t.Errorf("Invalid info: got %v", update.Status.Info)
 	}
 
 	s.WriteString("* NO Kansai band competition is in 30 seconds !\r\n")
-	if status := <-warns; status.Info != "Kansai band competition is in 30 seconds !" {
-		t.Errorf("Invalid warning: got %v", status.Info)
+	if update, ok := (<-updates).(*StatusUpdate); !ok || update.Status.Info != "Kansai band competition is in 30 seconds !" {
+		t.Errorf("Invalid warning: got %v", update.Status.Info)
 	}
 
 	s.WriteString("* BAD Battery level too low, shutting down.\r\n")
-	if status := <-errors; status.Info != "Battery level too low, shutting down." {
-		t.Errorf("Invalid error: got %v", status.Info)
+	if update, ok := (<-updates).(*StatusUpdate); !ok || update.Status.Info != "Battery level too low, shutting down." {
+		t.Errorf("Invalid error: got %v", update.Status.Info)
 	}
 }