storage: Eliminate storage.Mailbox interface for #69

storage/file Mailbox has been renamed mbox, and is now just an
implementation detail.

pkg/server/pop3/handler.go

@@ -65,7 +65,6 @@ type Session struct {
 	state      State             // Current session state
 	reader     *bufio.Reader     // Buffered reader for our net conn
 	user       string            // Mailbox name
-	mailbox    storage.Mailbox   // Mailbox instance
 	messages   []storage.Message // Slice of messages in mailbox
 	retain     []bool            // Messages to retain upon UPDATE (true=retain)
 	msgCount   int               // Number of undeleted messages
@@ -195,14 +194,6 @@ func (ses *Session) authorizationHandler(cmd string, args []string) {
 		if ses.user == "" {
 			ses.ooSeq(cmd)
 		} else {
-			var err error
-			ses.mailbox, err = ses.server.dataStore.MailboxFor(ses.user)
-			if err != nil {
-				ses.logError("Failed to open mailbox for %v", ses.user)
-				ses.send(fmt.Sprintf("-ERR Failed to open mailbox for %v", ses.user))
-				ses.enterState(QUIT)
-				return
-			}
 			ses.loadMailbox()
 			ses.send(fmt.Sprintf("+OK Found %v messages for %v", ses.msgCount, ses.user))
 			ses.enterState(TRANSACTION)
@@ -214,14 +205,6 @@ func (ses *Session) authorizationHandler(cmd string, args []string) {
 			return
 		}
 		ses.user = args[0]
-		var err error
-		ses.mailbox, err = ses.server.dataStore.MailboxFor(ses.user)
-		if err != nil {
-			ses.logError("Failed to open mailbox for %v", ses.user)
-			ses.send(fmt.Sprintf("-ERR Failed to open mailbox for %v", ses.user))
-			ses.enterState(QUIT)
-			return
-		}
 		ses.loadMailbox()
 		ses.send(fmt.Sprintf("+OK Found %v messages for %v", ses.msgCount, ses.user))
 		ses.enterState(TRANSACTION)

pkg/server/smtp/handler.go

@@ -14,7 +14,6 @@ import (
 
 	"github.com/jhillyerd/inbucket/pkg/log"
 	"github.com/jhillyerd/inbucket/pkg/msghub"
-	"github.com/jhillyerd/inbucket/pkg/storage"
 	"github.com/jhillyerd/inbucket/pkg/stringutil"
 )
 
@@ -73,7 +72,6 @@ var commands = map[string]bool{
 // recipientDetails for message delivery
 type recipientDetails struct {
 	address, localPart, domainPart string
-	mailbox                        storage.Mailbox
 }
 
 // Session holds the state of an SMTP session
@@ -365,14 +363,7 @@ func (ss *Session) dataHandler() {
 			}
 			if strings.ToLower(domain) != ss.server.domainNoStore {
 				// Not our "no store" domain, so store the message
-				mb, err := ss.server.dataStore.MailboxFor(local)
+				recipients = append(recipients, recipientDetails{recip, local, domain})
-				if err != nil {
-					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
-				}
-				recipients = append(recipients, recipientDetails{recip, local, domain, mb})
 			} else {
 				log.Tracef("Not storing message for %q", recip)
 			}
@@ -469,13 +460,13 @@ func (ss *Session) deliverMessage(r recipientDetails, msgBuf [][]byte) (ok bool)
 	// Append lines from msgBuf
 	for _, line := range msgBuf {
 		if err := msg.Append(line); err != nil {
-			ss.logError("Failed to append to mailbox %v: %v", r.mailbox, err)
+			ss.logError("Failed to append to mailbox %v: %v", r.localPart, err)
 			// Should really cleanup the crap on filesystem
 			return false
 		}
 	}
 	if err := msg.Close(); err != nil {
-		ss.logError("Error while closing message for %v: %v", r.mailbox, err)
+		ss.logError("Error while closing message for %v: %v", r.localPart, err)
 		return false
 	}
 	name, err := stringutil.ParseMailboxName(r.localPart)

pkg/server/smtp/handler_test.go

@@ -145,11 +145,8 @@ func TestReadyState(t *testing.T) {
 func TestMailState(t *testing.T) {
 	// Setup mock objects
 	mds := &storage.MockDataStore{}
-	mb1 := &storage.MockMailbox{}
 	msg1 := &storage.MockMessage{}
-	mds.On("MailboxFor", "u1").Return(mb1, nil)
 	mds.On("NewMessage", "u1").Return(msg1, nil)
-	mb1.On("Name").Return("u1")
 	msg1.On("ID").Return("")
 	msg1.On("From").Return("")
 	msg1.On("To").Return(make([]string, 0))
@@ -260,11 +257,8 @@ func TestMailState(t *testing.T) {
 func TestDataState(t *testing.T) {
 	// Setup mock objects
 	mds := &storage.MockDataStore{}
-	mb1 := &storage.MockMailbox{}
 	msg1 := &storage.MockMessage{}
-	mds.On("MailboxFor", "u1").Return(mb1, nil)
 	mds.On("NewMessage", "u1").Return(msg1, nil)
-	mb1.On("Name").Return("u1")
 	msg1.On("ID").Return("")
 	msg1.On("From").Return("")
 	msg1.On("To").Return(make([]string, 0))

pkg/storage/file/fmessage.go

@@ -18,7 +18,7 @@ import (
 // Message implements Message and contains a little bit of data about a
 // particular email message, and methods to retrieve the rest of it from disk.
 type Message struct {
-	mailbox *Mailbox
+	mailbox *mbox
 	// Stored in GOB
 	Fid      string
 	Fdate    time.Time
@@ -32,16 +32,15 @@ type Message struct {
 	writer     *bufio.Writer
 }
 
-// NewMessage creates a new FileMessage object and sets the Date and Id fields.
+// newMessage creates a new FileMessage object and sets the Date and ID fields.
 // It will also delete messages over messageCap if configured.
-func (mb *Mailbox) NewMessage() (storage.Message, error) {
+func (mb *mbox) newMessage() (storage.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 {
@@ -51,7 +50,6 @@ func (mb *Mailbox) NewMessage() (storage.Message, error) {
 			}
 		}
 	}
-
 	date := time.Now()
 	id := generateID(date)
 	return &Message{mailbox: mb, Fid: id, Fdate: date, writable: true}, nil

pkg/storage/file/fstore.go

@@ -76,46 +76,29 @@ func New(cfg config.DataStoreConfig) storage.Store {
 
 // GetMessage returns the messages in the named mailbox, or an error.
 func (fs *Store) GetMessage(mailbox, id string) (storage.Message, error) {
-	mb, err := fs.MailboxFor(mailbox)
+	mb, err := fs.mbox(mailbox)
 	if err != nil {
 		return nil, err
 	}
-	return mb.(*Mailbox).GetMessage(id)
+	return mb.getMessage(id)
 }
 
 // GetMessages returns the messages in the named mailbox, or an error.
 func (fs *Store) GetMessages(mailbox string) ([]storage.Message, error) {
-	mb, err := fs.MailboxFor(mailbox)
+	mb, err := fs.mbox(mailbox)
 	if err != nil {
 		return nil, err
 	}
-	return mb.(*Mailbox).GetMessages()
+	return mb.getMessages()
 }
 
 // PurgeMessages deletes all messages in the named mailbox, or returns an error.
-func (fs *Store) PurgeMessages(name string) error {
+func (fs *Store) PurgeMessages(mailbox string) error {
-	mb, err := fs.MailboxFor(name)
+	mb, err := fs.mbox(mailbox)
 	if err != nil {
 		return err
 	}
-	return mb.(*Mailbox).Purge()
+	return mb.purge()
-}
-
-// MailboxFor retrieves the Mailbox object for a specified email address, if the mailbox
-// does not exist, it will attempt to create it.
-func (fs *Store) MailboxFor(emailAddress string) (storage.Mailbox, error) {
-	name, err := stringutil.ParseMailboxName(emailAddress)
-	if err != nil {
-		return nil, err
-	}
-	dir := stringutil.HashMailboxName(name)
-	s1 := dir[0:3]
-	s2 := dir[0:6]
-	path := filepath.Join(fs.mailPath, s1, s2, dir)
-	indexPath := filepath.Join(path, indexFileName)
-
-	return &Mailbox{store: fs, name: name, dirName: dir, path: path,
-		indexPath: indexPath}, nil
 }
 
 // VisitMailboxes accepts a function that will be called with the messages in each mailbox while it
@@ -147,9 +130,9 @@ func (fs *Store) VisitMailboxes(f func([]storage.Message) (cont bool)) error {
 							mbdir := inf3.Name()
 							mbpath := filepath.Join(fs.mailPath, l1, l2, mbdir)
 							idx := filepath.Join(mbpath, indexFileName)
-							mb := &Mailbox{store: fs, dirName: mbdir, path: mbpath,
+							mb := &mbox{store: fs, dirName: mbdir, path: mbpath,
 								indexPath: idx}
-							msgs, err := mb.GetMessages()
+							msgs, err := mb.getMessages()
 							if err != nil {
 								return err
 							}
@@ -177,16 +160,31 @@ func (fs *Store) LockFor(emailAddress string) (*sync.RWMutex, error) {
 
 // NewMessage is temproary until #69 MessageData refactor
 func (fs *Store) NewMessage(mailbox string) (storage.Message, error) {
-	mb, err := fs.MailboxFor(mailbox)
+	mb, err := fs.mbox(mailbox)
 	if err != nil {
 		return nil, err
 	}
-	return mb.(*Mailbox).NewMessage()
+	return mb.newMessage()
 }
 
-// Mailbox implements Mailbox, manages the mail for a specific user and
+// mbox returns the named mailbox.
-// correlates to a particular directory on disk.
+func (fs *Store) mbox(mailbox string) (*mbox, error) {
-type Mailbox struct {
+	name, err := stringutil.ParseMailboxName(mailbox)
+	if err != nil {
+		return nil, err
+	}
+	dir := stringutil.HashMailboxName(name)
+	s1 := dir[0:3]
+	s2 := dir[0:6]
+	path := filepath.Join(fs.mailPath, s1, s2, dir)
+	indexPath := filepath.Join(path, indexFileName)
+
+	return &mbox{store: fs, name: name, dirName: dir, path: path,
+		indexPath: indexPath}, nil
+}
+
+// mbox manages the mail for a specific user and correlates to a particular directory on disk.
+type mbox struct {
 	store       *Store
 	name        string
 	dirName     string
@@ -196,25 +194,14 @@ type Mailbox struct {
 	messages    []*Message
 }
 
-// Name of the mailbox
+// getMessages scans the mailbox directory for .gob files and decodes them into
-func (mb *Mailbox) Name() string {
-	return mb.name
-}
-
-// String renders the name and directory path of the mailbox
-func (mb *Mailbox) String() string {
-	return mb.name + "[" + mb.dirName + "]"
-}
-
-// GetMessages scans the mailbox directory for .gob files and decodes them into
 // a slice of Message objects.
-func (mb *Mailbox) GetMessages() ([]storage.Message, error) {
+func (mb *mbox) getMessages() ([]storage.Message, error) {
 	if !mb.indexLoaded {
 		if err := mb.readIndex(); err != nil {
 			return nil, err
 		}
 	}
-
 	messages := make([]storage.Message, len(mb.messages))
 	for i, m := range mb.messages {
 		messages[i] = m
@@ -222,35 +209,32 @@ func (mb *Mailbox) GetMessages() ([]storage.Message, error) {
 	return messages, nil
 }
 
-// GetMessage decodes a single message by Id and returns a Message object
+// getMessage decodes a single message by ID and returns a Message object.
-func (mb *Mailbox) GetMessage(id string) (storage.Message, error) {
+func (mb *mbox) getMessage(id string) (storage.Message, error) {
 	if !mb.indexLoaded {
 		if err := mb.readIndex(); err != nil {
 			return nil, err
 		}
 	}
-
 	if id == "latest" && len(mb.messages) != 0 {
 		return mb.messages[len(mb.messages)-1], nil
 	}
-
 	for _, m := range mb.messages {
 		if m.Fid == id {
 			return m, nil
 		}
 	}
-
 	return nil, storage.ErrNotExist
 }
 
-// Purge deletes all messages in this mailbox
+// purge deletes all messages in this mailbox.
-func (mb *Mailbox) Purge() error {
+func (mb *mbox) purge() error {
 	mb.messages = mb.messages[:0]
 	return mb.writeIndex()
 }
 
 // readIndex loads the mailbox index data from disk
-func (mb *Mailbox) readIndex() error {
+func (mb *mbox) readIndex() error {
 	// Clear message slice, open index
 	mb.messages = mb.messages[:0]
 	// Lock for reading
@@ -293,7 +277,7 @@ func (mb *Mailbox) readIndex() error {
 }
 
 // writeIndex overwrites the index on disk with the current mailbox data
-func (mb *Mailbox) writeIndex() error {
+func (mb *mbox) writeIndex() error {
 	// Lock for writing
 	indexMx.Lock()
 	defer indexMx.Unlock()
@@ -335,7 +319,7 @@ func (mb *Mailbox) writeIndex() error {
 }
 
 // createDir checks for the presence of the path for this mailbox, creates it if needed
-func (mb *Mailbox) createDir() error {
+func (mb *mbox) createDir() error {
 	dirMx.Lock()
 	defer dirMx.Unlock()
 	if _, err := os.Stat(mb.path); err != nil {
@@ -348,7 +332,7 @@ func (mb *Mailbox) createDir() error {
 }
 
 // removeDir removes the mailbox, plus empty higher level directories
-func (mb *Mailbox) removeDir() error {
+func (mb *mbox) removeDir() error {
 	dirMx.Lock()
 	defer dirMx.Unlock()
 	// remove mailbox dir, including index file

pkg/storage/storage.go

@@ -25,19 +25,12 @@ type Store interface {
 	GetMessages(mailbox string) ([]Message, error)
 	PurgeMessages(mailbox string) error
 	VisitMailboxes(f func([]Message) (cont bool)) error
-	MailboxFor(emailAddress string) (Mailbox, 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) (Message, error)
 }
 
-// Mailbox is an interface to get and manipulate messages in a DataStore
-type Mailbox interface {
-	GetMessages() ([]Message, error)
-	String() string
-}
-
 // Message is an interface for a single message in a Mailbox
 type Message interface {
 	ID() string

pkg/storage/testing.go

@@ -33,12 +33,6 @@ func (m *MockDataStore) PurgeMessages(name string) error {
 	return args.Error(0)
 }
 
-// MailboxFor mock function
-func (m *MockDataStore) MailboxFor(name string) (Mailbox, error) {
-	args := m.Called(name)
-	return args.Get(0).(Mailbox), args.Error(1)
-}
-
 // LockFor mock function returns a new RWMutex, never errors.
 func (m *MockDataStore) LockFor(name string) (*sync.RWMutex, error) {
 	return &sync.RWMutex{}, nil
@@ -56,41 +50,6 @@ func (m *MockDataStore) VisitMailboxes(f func([]Message) (cont bool)) error {
 	return nil
 }
 
-// MockMailbox is a shared mock for unit testing
-type MockMailbox struct {
-	mock.Mock
-}
-
-// GetMessages mock function
-func (m *MockMailbox) GetMessages() ([]Message, error) {
-	args := m.Called()
-	return args.Get(0).([]Message), args.Error(1)
-}
-
-// GetMessage mock function
-func (m *MockMailbox) GetMessage(id string) (Message, error) {
-	args := m.Called(id)
-	return args.Get(0).(Message), args.Error(1)
-}
-
-// Purge mock function
-func (m *MockMailbox) Purge() error {
-	args := m.Called()
-	return args.Error(0)
-}
-
-// NewMessage mock function
-func (m *MockMailbox) NewMessage() (Message, error) {
-	args := m.Called()
-	return args.Get(0).(Message), args.Error(1)
-}
-
-// String mock function
-func (m *MockMailbox) String() string {
-	args := m.Called()
-	return args.String(0)
-}
-
 // MockMessage is a shared mock for unit testing
 type MockMessage struct {
 	mock.Mock