Add configurable mailbox message cap

- Add new configuration option [datastore]mailbox.message.cap
- Modify filestore to enforce message cap if value > 0
- Filestore unit tests for message cap when enabled & disabled
- Change to DataStore.Mailbox.NewMessage() interface to allow error
  return

config/config.go

@@ -42,6 +42,7 @@ type DataStoreConfig struct {
 	Path             string
 	RetentionMinutes int
 	RetentionSleep   int
+	MailboxMsgCap    int
 }
 
 var (
@@ -121,6 +122,7 @@ func LoadConfig(filename string) error {
 	requireOption(messages, "datastore", "path")
 	requireOption(messages, "datastore", "retention.minutes")
 	requireOption(messages, "datastore", "retention.sleep.millis")
+	requireOption(messages, "datastore", "mailbox.message.cap")
 
 	// Return error if validations failed
 	if messages.Len() > 0 {
@@ -361,6 +363,11 @@ func parseDataStoreConfig() error {
 	if err != nil {
 		return fmt.Errorf("Failed to parse [%v]%v: '%v'", section, option, err)
 	}
+	option = "mailbox.message.cap"
+	dataStoreConfig.MailboxMsgCap, err = Config.Int(section, option)
+	if err != nil {
+		return fmt.Errorf("Failed to parse [%v]%v: '%v'", section, option, err)
+	}
 
 	return nil
 }

etc/devel.conf

@@ -100,3 +100,8 @@ retention.minutes=0
 # This should help reduce disk I/O when there are a large number of messages
 # to purge.
 retention.sleep.millis=100
+
+# Maximum number of messages we will store in a single mailbox. If this
+# number is exceeded, the oldest message in the box will be deleted each
+# time a new message is received for it.
+mailbox.message.cap=100

etc/inbucket.conf

@@ -100,3 +100,13 @@ retention.minutes=240
 # This should help reduce disk I/O when there are a large number of messages
 # to purge.
 retention.sleep.millis=100
+
+# Maximum number of messages we will store in a single mailbox. If this
+# number is exceeded, the oldest message in the box will be deleted each
+# time a new message is received for it.
+mailbox.message.cap=100
+
+# Maximum number of messages we will store in a single mailbox. If this
+# number is exceeded, the oldest message in the box will be deleted each
+# time a new message is received for it.
+mailbox.message.cap=500

etc/unix-sample.conf

@@ -100,3 +100,8 @@ retention.minutes=240
 # This should help reduce disk I/O when there are a large number of messages
 # to purge.
 retention.sleep.millis=100
+
+# Maximum number of messages we will store in a single mailbox. If this
+# number is exceeded, the oldest message in the box will be deleted each
+# time a new message is received for it.
+mailbox.message.cap=500

etc/win-sample.conf

@@ -100,3 +100,8 @@ retention.minutes=240
 # This should help reduce disk I/O when there are a large number of messages
 # to purge.
 retention.sleep.millis=100
+
+# Maximum number of messages we will store in a single mailbox. If this
+# number is exceeded, the oldest message in the box will be deleted each
+# time a new message is received for it.
+mailbox.message.cap=500

smtpd/datastore.go

@@ -16,7 +16,7 @@ type Mailbox interface {
 	GetMessages() ([]Message, error)
 	GetMessage(id string) (Message, error)
 	Purge() error
-	NewMessage() Message
+	NewMessage() (Message, error)
 	String() string
 }
 

smtpd/filestore.go

@@ -44,33 +44,31 @@ func countGenerator(c chan int) {
 // A DataStore is the root of the mail storage hiearchy.  It provides access to
 // Mailbox objects
 type FileDataStore struct {
-	path     string
+	path       string
-	mailPath string
+	mailPath   string
+	messageCap int
 }
 
 // NewFileDataStore creates a new DataStore object using the specified path
-func NewFileDataStore(path string) DataStore {
+func NewFileDataStore(cfg config.DataStoreConfig) DataStore {
+	path := cfg.Path
+	if path == "" {
+		log.LogError("No value configured for datastore path")
+		return nil
+	}
 	mailPath := filepath.Join(path, "mail")
 	if _, err := os.Stat(mailPath); err != nil {
 		// Mail datastore does not yet exist
 		os.MkdirAll(mailPath, 0770)
 	}
-	return &FileDataStore{path: path, mailPath: mailPath}
+	return &FileDataStore{path: path, mailPath: mailPath, messageCap: cfg.MailboxMsgCap}
 }
 
 // DefaultFileDataStore creates a new DataStore object.  It uses the inbucket.Config object to
 // construct it's path.
 func DefaultFileDataStore() DataStore {
-	path, err := config.Config.String("datastore", "path")
+	cfg := config.GetDataStoreConfig()
-	if err != nil {
+	return NewFileDataStore(cfg)
-		log.LogError("Error getting datastore path: %v", err)
-		return nil
-	}
-	if path == "" {
-		log.LogError("No value configured for datastore path")
-		return nil
-	}
-	return NewFileDataStore(path)
 }
 
 // Retrieves the Mailbox object for a specified email address, if the mailbox
@@ -292,11 +290,28 @@ type FileMessage struct {
 }
 
 // NewMessage creates a new Message object and sets the Date and Id fields.
-func (mb *FileMailbox) NewMessage() Message {
+// It will also delete messages over messageCap if configured.
+func (mb *FileMailbox) NewMessage() (Message, error) {
+	// Load index
+	if !mb.indexLoaded {
+		if err := mb.readIndex(); err != nil {
+			return nil, err
+		}
+	}
+
+	// Delete old messages over messageCap
+	if mb.store.messageCap > 0 {
+		for len(mb.messages) >= mb.store.messageCap {
+			log.LogInfo("Mailbox %q over configured message cap", mb.name)
+			if err := mb.messages[0].Delete(); err != nil {
+				return nil, err
+			}
+		}
+	}
+
 	date := time.Now()
 	id := generateId(date)
-
+	return &FileMessage{mailbox: mb, Fid: id, Fdate: date, writable: true}, nil
-	return &FileMessage{mailbox: mb, Fid: id, Fdate: date, writable: true}
 }
 
 func (m *FileMessage) Id() string {

smtpd/filestore_test.go

@@ -3,6 +3,7 @@ package smtpd
 import (
 	"bytes"
 	"fmt"
+	"github.com/jhillyerd/inbucket/config"
 	"github.com/stretchr/testify/assert"
 	"io"
 	"io/ioutil"
@@ -15,7 +16,7 @@ import (
 
 // Test directory structure created by filestore
 func TestFSDirStructure(t *testing.T) {
-	ds, logbuf := setupDataStore()
+	ds, logbuf := setupDataStore(config.DataStoreConfig{})
 	defer teardownDataStore(ds)
 	root := ds.path
 
@@ -99,7 +100,7 @@ func TestFSDirStructure(t *testing.T) {
 
 // Test FileDataStore.AllMailboxes()
 func TestFSAllMailboxes(t *testing.T) {
-	ds, logbuf := setupDataStore()
+	ds, logbuf := setupDataStore(config.DataStoreConfig{})
 	defer teardownDataStore(ds)
 
 	for _, name := range []string{"abby", "bill", "christa", "donald", "evelyn"} {
@@ -127,7 +128,7 @@ func TestFSAllMailboxes(t *testing.T) {
 // Test delivering several messages to the same mailbox, meanwhile querying its
 // contents with a new mailbox object each time
 func TestFSDeliverMany(t *testing.T) {
-	ds, logbuf := setupDataStore()
+	ds, logbuf := setupDataStore(config.DataStoreConfig{})
 	defer teardownDataStore(ds)
 
 	mbName := "fred"
@@ -176,7 +177,7 @@ func TestFSDeliverMany(t *testing.T) {
 
 // Test deleting messages
 func TestFSDelete(t *testing.T) {
-	ds, logbuf := setupDataStore()
+	ds, logbuf := setupDataStore(config.DataStoreConfig{})
 	defer teardownDataStore(ds)
 
 	mbName := "fred"
@@ -250,7 +251,7 @@ func TestFSDelete(t *testing.T) {
 
 // Test purging a mailbox
 func TestFSPurge(t *testing.T) {
-	ds, logbuf := setupDataStore()
+	ds, logbuf := setupDataStore(config.DataStoreConfig{})
 	defer teardownDataStore(ds)
 
 	mbName := "fred"
@@ -298,7 +299,7 @@ func TestFSPurge(t *testing.T) {
 
 // Test message size calculation
 func TestFSSize(t *testing.T) {
-	ds, logbuf := setupDataStore()
+	ds, logbuf := setupDataStore(config.DataStoreConfig{})
 	defer teardownDataStore(ds)
 
 	mbName := "fred"
@@ -336,7 +337,7 @@ func TestFSSize(t *testing.T) {
 
 // Test missing files
 func TestFSMissing(t *testing.T) {
-	ds, logbuf := setupDataStore()
+	ds, logbuf := setupDataStore(config.DataStoreConfig{})
 	defer teardownDataStore(ds)
 
 	mbName := "fred"
@@ -376,8 +377,88 @@ func TestFSMissing(t *testing.T) {
 	}
 }
 
+// Test delivering several messages to the same mailbox, see if message cap works
+func TestFSMessageCap(t *testing.T) {
+	mbCap := 10
+	ds, logbuf := setupDataStore(config.DataStoreConfig{MailboxMsgCap: mbCap})
+	defer teardownDataStore(ds)
+
+	mbName := "captain"
+	for i := 0; i < 20; i++ {
+		// Add a message
+		subj := fmt.Sprintf("subject %v", i)
+		deliverMessage(ds, mbName, subj, time.Now())
+		t.Logf("Delivered %q", subj)
+
+		// Check number of messages
+		mb, err := ds.MailboxFor(mbName)
+		if err != nil {
+			t.Fatalf("Failed to MailboxFor(%q): %v", mbName, err)
+		}
+		msgs, err := mb.GetMessages()
+		if err != nil {
+			t.Fatalf("Failed to GetMessages for %q: %v", mbName, err)
+		}
+		if len(msgs) > mbCap {
+			t.Errorf("Mailbox should be capped at %v messages, but has %v", mbCap, len(msgs))
+		}
+
+		// Check that the first message is correct
+		first := i - mbCap + 1
+		if first < 0 {
+			first = 0
+		}
+		firstSubj := fmt.Sprintf("subject %v", first)
+		if firstSubj != msgs[0].Subject() {
+			t.Errorf("Expected first subject to be %q, got %q", firstSubj, msgs[0].Subject())
+		}
+	}
+
+	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 delivering several messages to the same mailbox, see if no message cap works
+func TestFSNoMessageCap(t *testing.T) {
+	mbCap := 0
+	ds, logbuf := setupDataStore(config.DataStoreConfig{MailboxMsgCap: mbCap})
+	defer teardownDataStore(ds)
+
+	mbName := "captain"
+	for i := 0; i < 20; i++ {
+		// Add a message
+		subj := fmt.Sprintf("subject %v", i)
+		deliverMessage(ds, mbName, subj, time.Now())
+		t.Logf("Delivered %q", subj)
+
+		// Check number of messages
+		mb, err := ds.MailboxFor(mbName)
+		if err != nil {
+			t.Fatalf("Failed to MailboxFor(%q): %v", mbName, err)
+		}
+		msgs, err := mb.GetMessages()
+		if err != nil {
+			t.Fatalf("Failed to GetMessages for %q: %v", mbName, err)
+		}
+		if len(msgs) != i+1 {
+			t.Errorf("Expected %v messages, got %v", i+1, len(msgs))
+		}
+	}
+
+	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)
+	}
+}
+
 // setupDataStore creates a new FileDataStore in a temporary directory
-func setupDataStore() (*FileDataStore, *bytes.Buffer) {
+func setupDataStore(cfg config.DataStoreConfig) (*FileDataStore, *bytes.Buffer) {
 	path, err := ioutil.TempDir("", "inbucket")
 	if err != nil {
 		panic(err)
@@ -387,12 +468,14 @@ func setupDataStore() (*FileDataStore, *bytes.Buffer) {
 	buf := new(bytes.Buffer)
 	log.SetOutput(buf)
 
-	return NewFileDataStore(path).(*FileDataStore), buf
+	cfg.Path = path
+	return NewFileDataStore(cfg).(*FileDataStore), buf
 }
 
 // deliverMessage creates and delivers a message to the specific mailbox, returning
 // the size of the generated message.
-func deliverMessage(ds *FileDataStore, mbName string, subject string, date time.Time) (id string, size int) {
+func deliverMessage(ds *FileDataStore, mbName string, subject string,
+	date time.Time) (id string, size int) {
 	// Build fake SMTP message for delivery
 	testMsg := make([]byte, 0, 300)
 	testMsg = append(testMsg, []byte("To: somebody@host\r\n")...)
@@ -407,12 +490,13 @@ func deliverMessage(ds *FileDataStore, mbName string, subject string, date time.
 	}
 	// Create message object
 	id = generateId(date)
-	msg := &FileMessage{
+	msg, err := mb.NewMessage()
-		mailbox:  mb.(*FileMailbox),
+	if err != nil {
-		writable: true,
+		panic(err)
-		Fdate:    date,
-		Fid:      id,
 	}
+	fmsg := msg.(*FileMessage)
+	fmsg.Fdate = date
+	fmsg.Fid = id
 	msg.Append(testMsg)
 	if err = msg.Close(); err != nil {
 		panic(err)

smtpd/handler.go

@@ -352,13 +352,18 @@ func (ss *Session) dataHandler() {
 				// Not our "no store" domain, so store the message
 				mb, err := ss.server.dataStore.MailboxFor(local)
 				if err != nil {
-					ss.logError("Failed to open mailbox for %q", local)
+					ss.logError("Failed to open mailbox for %q: %s", local, err)
 					ss.send(fmt.Sprintf("451 Failed to open mailbox for %v", local))
 					ss.reset()
 					return
 				}
 				mailboxes[i] = mb
-				messages[i] = mb.NewMessage()
+				if messages[i], err = mb.NewMessage(); err != nil {
+					ss.logError("Failed to create message for %q: %s", local, err)
+					ss.send(fmt.Sprintf("451 Failed to create message for %v", local))
+					ss.reset()
+					return
+				}
 
 				// Generate Received header
 				recd := fmt.Sprintf("Received: from %s ([%s]) by %s\r\n  for <%s>; %s\r\n",

smtpd/handler_test.go

@@ -152,7 +152,7 @@ func TestMailState(t *testing.T) {
 	mb1 := &MockMailbox{}
 	msg1 := &MockMessage{}
 	mds.On("MailboxFor").Return(mb1, nil)
-	mb1.On("NewMessage").Return(msg1)
+	mb1.On("NewMessage").Return(msg1, nil)
 	msg1.On("Close").Return(nil)
 
 	server, logbuf := setupSmtpServer(mds)
@@ -262,7 +262,7 @@ func TestDataState(t *testing.T) {
 	mb1 := &MockMailbox{}
 	msg1 := &MockMessage{}
 	mds.On("MailboxFor").Return(mb1, nil)
-	mb1.On("NewMessage").Return(msg1)
+	mb1.On("NewMessage").Return(msg1, nil)
 	msg1.On("Close").Return(nil)
 
 	server, logbuf := setupSmtpServer(mds)

smtpd/retention_test.go

@@ -98,9 +98,9 @@ func (m *MockMailbox) Purge() error {
 	return args.Error(0)
 }
 
-func (m *MockMailbox) NewMessage() Message {
+func (m *MockMailbox) NewMessage() (Message, error) {
 	args := m.Called()
-	return args.Get(0).(Message)
+	return args.Get(0).(Message), args.Error(1)
 }
 
 func (m *MockMailbox) String() string {

web/rest_test.go

@@ -114,9 +114,9 @@ func (m *MockMailbox) Purge() error {
 	return args.Error(0)
 }
 
-func (m *MockMailbox) NewMessage() smtpd.Message {
+func (m *MockMailbox) NewMessage() (smtpd.Message, error) {
 	args := m.Called()
-	return args.Get(0).(smtpd.Message)
+	return args.Get(0).(smtpd.Message), args.Error(1)
 }
 
 func (m *MockMailbox) String() string {