From b42ea130eac7b7928ca1a0d249f67c3e2acaf6c8 Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Sat, 24 Mar 2018 14:30:53 -0700 Subject: [PATCH] storage/mem: implement message cap for #88 - Move message cap tests into storage test suite. - Update change log. --- CHANGELOG.md | 5 +++ pkg/storage/file/fstore_test.go | 76 +-------------------------------- pkg/storage/mem/store.go | 24 ++++++++--- pkg/storage/mem/store_test.go | 4 +- pkg/test/storage_suite.go | 59 +++++++++++++++++++++---- 5 files changed, 76 insertions(+), 92 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fb1ecc..2c226d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ This project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] ### Added +- Inbucket is now configured using environment variables instead of a config + file. +- In-memory storage option, best for small installations and desktops. Will be + used by default. +- Storage type is now displayed on Status page. - Store size is now calculated during retention scan and displayed on the Status page. diff --git a/pkg/storage/file/fstore_test.go b/pkg/storage/file/fstore_test.go index 10326b0..7023706 100644 --- a/pkg/storage/file/fstore_test.go +++ b/pkg/storage/file/fstore_test.go @@ -22,8 +22,8 @@ import ( // TestSuite runs storage package test suite on file store. func TestSuite(t *testing.T) { - test.StoreSuite(t, func() (storage.Store, func(), error) { - ds, _ := setupDataStore(config.Storage{}) + test.StoreSuite(t, func(conf config.Storage) (storage.Store, func(), error) { + ds, _ := setupDataStore(conf) destroy := func() { teardownDataStore(ds) } @@ -144,78 +144,6 @@ 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.Storage{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 - msgs, err := ds.GetMessages(mbName) - 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.Storage{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 - msgs, err := ds.GetMessages(mbName) - 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) - } -} - // Test Get the latest message func TestGetLatestMessage(t *testing.T) { ds, logbuf := setupDataStore(config.Storage{}) diff --git a/pkg/storage/mem/store.go b/pkg/storage/mem/store.go index cbf78b5..0559a5b 100644 --- a/pkg/storage/mem/store.go +++ b/pkg/storage/mem/store.go @@ -14,12 +14,14 @@ import ( type Store struct { sync.Mutex boxes map[string]*mbox + cap int } type mbox struct { sync.RWMutex name string - counter int + last int + first int messages map[string]*Message } @@ -29,6 +31,7 @@ var _ storage.Store = &Store{} func New(cfg config.Storage) (storage.Store, error) { return &Store{ boxes: make(map[string]*mbox), + cap: cfg.MailboxMsgCap, }, nil } @@ -46,10 +49,10 @@ func (s *Store) AddMessage(message storage.Message) (id string, err error) { return } // Generate message ID. - mb.counter++ - id = strconv.Itoa(mb.counter) + mb.last++ + id = strconv.Itoa(mb.last) m := &Message{ - index: mb.counter, + index: mb.last, mailbox: message.Mailbox(), id: id, from: message.From(), @@ -59,6 +62,13 @@ func (s *Store) AddMessage(message storage.Message) (id string, err error) { source: source, } mb.messages[id] = m + if s.cap > 0 { + // Enforce cap. + for len(mb.messages) > s.cap { + delete(mb.messages, strconv.Itoa(mb.first)) + mb.first++ + } + } }) return id, err } @@ -121,7 +131,7 @@ func (s *Store) VisitMailboxes(f func([]storage.Message) (cont bool)) error { } // withMailbox gets or creates a mailbox, locks it, then calls f. -func (s *Store) withMailbox(mailbox string, rw bool, f func(mb *mbox)) { +func (s *Store) withMailbox(mailbox string, writeLock bool, f func(mb *mbox)) { s.Lock() mb, ok := s.boxes[mailbox] if !ok { @@ -133,13 +143,13 @@ func (s *Store) withMailbox(mailbox string, rw bool, f func(mb *mbox)) { s.boxes[mailbox] = mb } s.Unlock() - if rw { + if writeLock { mb.Lock() } else { mb.RLock() } defer func() { - if rw { + if writeLock { mb.Unlock() } else { mb.RUnlock() diff --git a/pkg/storage/mem/store_test.go b/pkg/storage/mem/store_test.go index 53f986d..8a45ec1 100644 --- a/pkg/storage/mem/store_test.go +++ b/pkg/storage/mem/store_test.go @@ -10,8 +10,8 @@ import ( // TestSuite runs storage package test suite on file store. func TestSuite(t *testing.T) { - test.StoreSuite(t, func() (storage.Store, func(), error) { - s, _ := New(config.Storage{}) + test.StoreSuite(t, func(conf config.Storage) (storage.Store, func(), error) { + s, _ := New(conf) destroy := func() {} return s, destroy, nil }) diff --git a/pkg/test/storage_suite.go b/pkg/test/storage_suite.go index 3d167b5..1038dac 100644 --- a/pkg/test/storage_suite.go +++ b/pkg/test/storage_suite.go @@ -9,30 +9,34 @@ import ( "testing" "time" + "github.com/jhillyerd/inbucket/pkg/config" "github.com/jhillyerd/inbucket/pkg/message" "github.com/jhillyerd/inbucket/pkg/storage" ) // StoreFactory returns a new store for the test suite. -type StoreFactory func() (store storage.Store, destroy func(), err error) +type StoreFactory func(config.Storage) (store storage.Store, destroy func(), err error) // StoreSuite runs a set of general tests on the provided Store. func StoreSuite(t *testing.T, factory StoreFactory) { testCases := []struct { name string test func(*testing.T, storage.Store) + conf config.Storage }{ - {"metadata", testMetadata}, - {"content", testContent}, - {"delivery order", testDeliveryOrder}, - {"size", testSize}, - {"delete", testDelete}, - {"purge", testPurge}, - {"visit mailboxes", testVisitMailboxes}, + {"metadata", testMetadata, config.Storage{}}, + {"content", testContent, config.Storage{}}, + {"delivery order", testDeliveryOrder, config.Storage{}}, + {"size", testSize, config.Storage{}}, + {"delete", testDelete, config.Storage{}}, + {"purge", testPurge, config.Storage{}}, + {"cap=10", testMsgCap, config.Storage{MailboxMsgCap: 10}}, + {"cap=0", testNoMsgCap, config.Storage{MailboxMsgCap: 0}}, + {"visit mailboxes", testVisitMailboxes, config.Storage{}}, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - store, destroy, err := factory() + store, destroy, err := factory(tc.conf) if err != nil { t.Fatal(err) } @@ -260,6 +264,43 @@ func testPurge(t *testing.T, store storage.Store) { getAndCountMessages(t, store, mailbox, 0) } +// testMsgCap verifies the message cap is enforced. +func testMsgCap(t *testing.T, store storage.Store) { + mbCap := 10 + mailbox := "captain" + for i := 0; i < 20; i++ { + subj := fmt.Sprintf("subject %v", i) + deliverMessage(t, store, mailbox, subj, time.Now()) + msgs, err := store.GetMessages(mailbox) + if err != nil { + t.Fatalf("Failed to GetMessages for %q: %v", mailbox, err) + } + if len(msgs) > mbCap { + t.Errorf("Mailbox has %v messages, should be capped at %v", len(msgs), mbCap) + break + } + // 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("Got subject %q, wanted first subject: %q", msgs[0].Subject(), firstSubj) + } + } +} + +// testNoMsgCap verfies a cap of 0 is not enforced. +func testNoMsgCap(t *testing.T, store storage.Store) { + mailbox := "captain" + for i := 0; i < 20; i++ { + subj := fmt.Sprintf("subject %v", i) + deliverMessage(t, store, mailbox, subj, time.Now()) + getAndCountMessages(t, store, mailbox, i+1) + } +} + // testVisitMailboxes creates some mailboxes and confirms the VisitMailboxes method visits all of // them. func testVisitMailboxes(t *testing.T, ds storage.Store) {