test: Start work on test suite for #82

- smtp: Tidy up []byte/buffer/string use in delivery #69

pkg/server/smtp/handler.go

@@ -352,7 +352,6 @@ func (ss *Session) mailHandler(cmd string, arg string) {
 func (ss *Session) dataHandler() {
 	recipients := make([]recipientDetails, 0, ss.recipients.Len())
 	// Get a Mailbox and a new Message for each recipient
-	msgSize := 0
 	if ss.server.storeMessages {
 		for e := ss.recipients.Front(); e != nil; e = e.Next() {
 			recip := e.Value.(string)
@@ -373,11 +372,9 @@ func (ss *Session) dataHandler() {
 	}
 
 	ss.send("354 Start mail input; end with <CRLF>.<CRLF>")
-	var lineBuf bytes.Buffer
+	msgBuf := &bytes.Buffer{}
-	msgBuf := make([][]byte, 0, 1024)
 	for {
-		lineBuf.Reset()
+		lineBuf, err := ss.readByteLine()
-		err := ss.readByteLine(&lineBuf)
 		if err != nil {
 			if netErr, ok := err.(net.Error); ok {
 				if netErr.Timeout() {
@@ -388,9 +385,7 @@ func (ss *Session) dataHandler() {
 			ss.enterState(QUIT)
 			return
 		}
-		line := lineBuf.Bytes()
+		if bytes.Equal(lineBuf, []byte(".\r\n")) || bytes.Equal(lineBuf, []byte(".\n")) {
-		// ss.logTrace("DATA: %q", line)
-		if string(line) == ".\r\n" || string(line) == ".\n" {
 			// Mail data complete
 			if ss.server.storeMessages {
 				// Create a message for each valid recipient
@@ -405,7 +400,7 @@ func (ss *Session) dataHandler() {
 						return
 					}
 					mu.Lock()
-					ok := ss.deliverMessage(r, msgBuf)
+					ok := ss.deliverMessage(r, msgBuf.Bytes())
 					mu.Unlock()
 					if ok {
 						expReceivedTotal.Add(1)
@@ -420,47 +415,34 @@ func (ss *Session) dataHandler() {
 				expReceivedTotal.Add(1)
 			}
 			ss.send("250 Mail accepted for delivery")
-			ss.logInfo("Message size %v bytes", msgSize)
+			ss.logInfo("Message size %v bytes", msgBuf.Len())
 			ss.reset()
 			return
 		}
 		// SMTP RFC says remove leading periods from input
-		if len(line) > 0 && line[0] == '.' {
+		if len(lineBuf) > 0 && lineBuf[0] == '.' {
-			line = line[1:]
+			lineBuf = lineBuf[1:]
 		}
-		// Second append copies line/lineBuf so we can reuse it
+		msgBuf.Write(lineBuf)
-		msgBuf = append(msgBuf, append([]byte{}, line...))
+		if msgBuf.Len() > ss.server.maxMessageBytes {
-		msgSize += len(line)
-		if msgSize > ss.server.maxMessageBytes {
 			// Max message size exceeded
 			ss.send("552 Maximum message size exceeded")
 			ss.logWarn("Max message size exceeded while in DATA")
 			ss.reset()
-			// Should really cleanup the crap on filesystem (after issue #23)
 			return
 		}
 	} // end for
 }
 
 // deliverMessage creates and populates a new Message for the specified recipient
-func (ss *Session) deliverMessage(r recipientDetails, msgBuf [][]byte) (ok bool) {
+func (ss *Session) deliverMessage(r recipientDetails, content []byte) (ok bool) {
 	name, err := stringutil.ParseMailboxName(r.localPart)
 	if err != nil {
 		// This parse already succeeded when MailboxFor was called, shouldn't fail here.
 		return false
 	}
-	buf := bytes.Buffer{}
-	// Generate Received header
-	stamp := time.Now().Format(timeStampFormat)
-	recd := fmt.Sprintf("Received: from %s ([%s]) by %s\r\n  for <%s>; %s\r\n",
-		ss.remoteDomain, ss.remoteHost, ss.server.domain, r.address, stamp)
-	buf.WriteString(recd)
-	// Append lines from msgBuf
-	for _, line := range msgBuf {
-		buf.Write(line)
-	}
 	// TODO replace with something that only reads header?
-	env, err := enmime.ReadEnvelope(bytes.NewReader(buf.Bytes()))
+	env, err := enmime.ReadEnvelope(bytes.NewReader(content))
 	if err != nil {
 		ss.logError("Failed to parse message for %q: %v", r.localPart, err)
 		return false
@@ -475,6 +457,10 @@ func (ss *Session) deliverMessage(r recipientDetails, msgBuf [][]byte) (ok bool)
 		ss.logError("Failed to get To addresses: %v", err)
 		return false
 	}
+	// Generate Received header.
+	stamp := time.Now().Format(timeStampFormat)
+	recd := strings.NewReader(fmt.Sprintf("Received: from %s ([%s]) by %s\r\n  for <%s>; %s\r\n",
+		ss.remoteDomain, ss.remoteHost, ss.server.domain, r.address, stamp))
 	delivery := &message.Delivery{
 		Meta: message.Metadata{
 			Mailbox: name,
@@ -483,14 +469,14 @@ func (ss *Session) deliverMessage(r recipientDetails, msgBuf [][]byte) (ok bool)
 			Date:    time.Now(),
 			Subject: env.GetHeader("Subject"),
 		},
-		Reader: bytes.NewReader(buf.Bytes()),
+		Reader: io.MultiReader(recd, bytes.NewReader(content)),
 	}
 	id, err := ss.server.dataStore.AddMessage(delivery)
 	if err != nil {
 		ss.logError("Failed to store message for %q: %s", r.localPart, err)
 		return false
 	}
-	// Broadcast message information
+	// Broadcast message information.
 	broadcast := msghub.Message{
 		Mailbox: name,
 		ID:      id,
@@ -501,7 +487,6 @@ func (ss *Session) deliverMessage(r recipientDetails, msgBuf [][]byte) (ok bool)
 		Size:    delivery.Size(),
 	}
 	ss.server.msgHub.Dispatch(broadcast)
-
 	return true
 }
 
@@ -535,16 +520,11 @@ func (ss *Session) send(msg string) {
 
 // readByteLine reads a line of input into the provided buffer. Does
 // not reset the Buffer - please do so prior to calling.
-func (ss *Session) readByteLine(buf io.Writer) error {
+func (ss *Session) readByteLine() ([]byte, error) {
 	if err := ss.conn.SetReadDeadline(ss.nextDeadline()); err != nil {
-		return err
+		return nil, err
 	}
-	line, err := ss.reader.ReadBytes('\n')
+	return ss.reader.ReadBytes('\n')
-	if err != nil {
-		return err
-	}
-	_, err = buf.Write(line)
-	return err
 }
 
 // Reads a line of input

pkg/storage/file/fstore.go

@@ -121,6 +121,7 @@ func (fs *Store) AddMessage(m storage.StoreMessage) (id string, err error) {
 	// Update the index.
 	fm.Fdate = m.Date()
 	fm.Ffrom = m.From()
+	fm.Fto = m.To()
 	fm.Fsize = size
 	fm.Fsubject = m.Subject()
 	mb.messages = append(mb.messages, fm)

pkg/storage/file/fstore_test.go

@@ -16,9 +16,23 @@ import (
 	"github.com/jhillyerd/inbucket/pkg/config"
 	"github.com/jhillyerd/inbucket/pkg/message"
 	"github.com/jhillyerd/inbucket/pkg/storage"
+	"github.com/jhillyerd/inbucket/pkg/test"
 	"github.com/stretchr/testify/assert"
 )
 
+// TestSuite runs storage package test suite on file store.
+func TestSuite(t *testing.T) {
+	ds, logbuf := setupDataStore(config.DataStoreConfig{})
+	defer teardownDataStore(ds)
+	test.StoreSuite(t, ds)
+	if t.Failed() {
+		// Wait for handler to finish logging.
+		time.Sleep(2 * time.Second)
+		// Dump buffered log data if there was a failure.
+		_, _ = io.Copy(os.Stderr, logbuf)
+	}
+}
+
 // Test directory structure created by filestore
 func TestFSDirStructure(t *testing.T) {
 	ds, logbuf := setupDataStore(config.DataStoreConfig{})

pkg/storage/storage.go

@@ -28,8 +28,6 @@ type Store interface {
 	VisitMailboxes(f func([]StoreMessage) (cont bool)) error
 	// LockFor is a temporary hack to fix #77 until Datastore revamp
 	LockFor(emailAddress string) (*sync.RWMutex, error)
-	// NewMessage is temproary until #69 MessageData refactor
-	NewMessage(mailbox string) (StoreMessage, error)
 }
 
 // StoreMessage represents a message to be stored, or returned from a storage implementation.

pkg/test/storage_suite.go

@@ -0,0 +1,89 @@
+package test
+
+import (
+	"net/mail"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/jhillyerd/inbucket/pkg/message"
+	"github.com/jhillyerd/inbucket/pkg/storage"
+)
+
+// StoreSuite runs a set of general tests on the provided Store
+func StoreSuite(t *testing.T, store storage.Store) {
+	testCases := []struct {
+		name string
+		test func(*testing.T, storage.Store)
+	}{
+		{"metadata", testMetadata},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			tc.test(t, store)
+		})
+	}
+}
+
+func testMetadata(t *testing.T, ds storage.Store) {
+	// Store a message
+	mailbox := "testmailbox"
+	from := &mail.Address{Name: "From Person", Address: "from@person.com"}
+	to := []*mail.Address{
+		{Name: "One Person", Address: "one@a.person.com"},
+		{Name: "Two Person", Address: "two@b.person.com"},
+	}
+	date := time.Now()
+	subject := "fantastic test subject line"
+	content := "doesn't matter"
+	delivery := &message.Delivery{
+		Meta: message.Metadata{
+			// ID and Size will be determined by the Store
+			Mailbox: mailbox,
+			From:    from,
+			To:      to,
+			Date:    date,
+			Subject: subject,
+		},
+		Reader: strings.NewReader(content),
+	}
+	id, err := ds.AddMessage(delivery)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if id == "" {
+		t.Fatal("Expected AddMessage() to return non-empty ID string")
+	}
+	// Retrieve and validate the message
+	sm, err := ds.GetMessage(mailbox, id)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if sm.Mailbox() != mailbox {
+		t.Errorf("got mailbox %q, want: %q", sm.Mailbox(), mailbox)
+	}
+	if sm.ID() != id {
+		t.Errorf("got id %q, want: %q", sm.ID(), id)
+	}
+	if *sm.From() != *from {
+		t.Errorf("got from %v, want: %v", sm.From(), from)
+	}
+	if len(sm.To()) != len(to) {
+		t.Errorf("got len(to) = %v, want: %v", len(sm.To()), len(to))
+	} else {
+		for i, got := range sm.To() {
+			if *to[i] != *got {
+				t.Errorf("got to[%v] %v, want: %v", i, got, to[i])
+			}
+		}
+	}
+	if !sm.Date().Equal(date) {
+		t.Errorf("got date %v, want: %v", sm.Date(), date)
+	}
+	if sm.Subject() != subject {
+		t.Errorf("got subject %q, want: %q", sm.Subject(), subject)
+	}
+	if sm.Size() != int64(len(content)) {
+		t.Errorf("got size %v, want: %v", sm.Size(), len(content))
+	}
+}