~rockorager/offmap

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
2 2

[PATCH offmap v2] email: don't keep files open after downloading

Details
Message ID
<20221130163824.102652-1-tim@timculverhouse.com>
DKIM signature
missing
Download raw message
Patch: +38 -32
Store the absolute filename to the downloaded email instead of keeping
an open file descriptor.

Use os.Rename when creating maildir emails to reduce I/O.

Fix tests.

Reported-by: inwit <inwit@sindominio.net>
Signed-off-by: Tim Culverhouse <tim@timculverhouse.com>
---
v2: read the entire file into a buffer for reading

 email.go               | 28 +++++++++++++++++++---------
 imap/imap.go           |  8 +-------
 maildir/maildir.go     |  9 ++-------
 maildir/update.go      | 10 ++++++----
 maildir/update_test.go | 15 ++++++++++-----
 5 files changed, 38 insertions(+), 32 deletions(-)

diff --git a/email.go b/email.go
index c11fd82bca8c..7d9fdf50627d 100644
--- a/email.go
+++ b/email.go
@@ -1,8 +1,10 @@
package offmap

import (
	"bytes"
	"fmt"
	"io"
	"os"
	"time"

	"github.com/emersion/go-maildir"
@@ -23,8 +25,9 @@ type Email struct {
	// Date is the best known time for the email creation
	Date time.Time `json:"-"`

	// Body is the RFC822 body of the email (ie, the entire email)
	body io.ReadCloser
	// filename is the absolute path of the email on disk
	filename string
	body     io.Reader
	// size is the size of email in bytes
	size int
}
@@ -40,17 +43,28 @@ func NewEmail(mbox *Mailbox, uid uint32, key string, flags []maildir.Flag) *Emai
	}
}

func (e *Email) SetBody(body io.ReadCloser) {
	e.body = body
func (e *Email) SetFilename(path string) {
	e.filename = path
}

func (e *Email) Read(p []byte) (int, error) {
	if e.body == nil {
	if e.filename == "" {
		return 0, fmt.Errorf("email: no body")
	}
	if e.body == nil {
		b, err := os.ReadFile(e.filename)
		if err != nil {
			return 0, fmt.Errorf("email: %v", err)
		}
		e.body = bytes.NewBuffer(b)
	}
	return e.body.Read(p)
}

func (e *Email) Filename() string {
	return e.filename
}

func (e *Email) SetSize(i int) {
	e.size = i
}
@@ -58,7 +72,3 @@ func (e *Email) SetSize(i int) {
func (e *Email) Len() int {
	return e.size
}

func (e *Email) Close() error {
	return e.body.Close()
}
diff --git a/imap/imap.go b/imap/imap.go
index 74e3eb55e032..4bf9e918a517 100644
--- a/imap/imap.go
+++ b/imap/imap.go
@@ -220,12 +220,6 @@ func (s *Store) DownloadEmail(emls []*offmap.Email) []*offmap.Email {
					atomic.AddInt32(&dled, 1)
					cur := atomic.LoadInt32(&dled)
					f.Close()
					// Reopen but with read only mode
					f, err = os.Open(filepath)
					if err != nil {
						log.Errorf("imap: %v", err)
						continue
					}
					emlMu.Lock()
					eml, ok := emlMap[filename]
					emlMu.Unlock()
@@ -233,7 +227,7 @@ func (s *Store) DownloadEmail(emls []*offmap.Email) []*offmap.Email {
						log.Errorf("imap: %s: eml not found: %s", status.Name, filename)
						continue
					}
					eml.SetBody(f)
					eml.SetFilename(filepath)
					log.Tracef("imap: (client %d) downloaded %d of %d", client.id, cur, total)
				}
				msgWg.Done()
diff --git a/maildir/maildir.go b/maildir/maildir.go
index 2682066e3fa4..5209332d285d 100644
--- a/maildir/maildir.go
+++ b/maildir/maildir.go
@@ -66,17 +66,12 @@ func (s *Store) DownloadEmail(emls []*offmap.Email) []*offmap.Email {
			log.Errorf("maildir: couldn't find email %d", eml.UID)
			continue
		}
		f, err := os.Open(filename)
		fi, err := os.Stat(filename)
		if err != nil {
			log.Errorf("maildir: couldn't open file %d", eml.UID)
			continue
		}
		fi, err := f.Stat()
		if err != nil {
			log.Errorf("maildir: couldn't open file %d", eml.UID)
			continue
		}
		eml.SetBody(f)
		eml.SetFilename(filename)
		eml.Date = fi.ModTime()
		eml.SetSize(int(fi.Size()))
	}
diff --git a/maildir/update.go b/maildir/update.go
index 2e8a1c56a59c..dc0d6ac6e53b 100644
--- a/maildir/update.go
+++ b/maildir/update.go
@@ -2,7 +2,6 @@ package maildir

import (
	"fmt"
	"io"
	"os"
	"path"

@@ -109,13 +108,16 @@ func (s *Store) renameMaildir(orig string, dest string) error {
func (s *Store) createEmail(mbox string, emls []*offmap.Email) error {
	dir := maildir.Dir(path.Join(s.root, mbox))
	for _, eml := range emls {
		defer eml.Close()
		key, wc, err := dir.Create(eml.Flags)
		if err != nil {
			return fmt.Errorf("maildir: could not create email: %v", err)
		}
		defer wc.Close()
		_, err = io.Copy(wc, eml)
		wc.Close()
		dest, err := dir.Filename(key)
		if err != nil {
			return fmt.Errorf("maildir: could not write email: %v", err)
		}
		err = os.Rename(eml.Filename(), dest)
		if err != nil {
			return fmt.Errorf("maildir: could not write email: %v", err)
		}
diff --git a/maildir/update_test.go b/maildir/update_test.go
index fbbe2f23c742..6faa060d47fb 100644
--- a/maildir/update_test.go
+++ b/maildir/update_test.go
@@ -1,8 +1,8 @@
package maildir

import (
	"bytes"
	"io"
	"os"
	"path"
	"testing"

	"git.sr.ht/~rockorager/offmap"
@@ -75,9 +75,14 @@ func TestCreateEmail(t *testing.T) {
	mailbox := mboxes["Inbox"]
	eml := offmap.NewEmail(mailbox, 0, "key", []maildir.Flag{'S'})

	buf := bytes.NewBufferString("Email body")
	rc := io.NopCloser(buf)
	eml.SetBody(rc)
	tmpEml := path.Join(t.TempDir(), "tmpEmail")
	f, err := os.Create(tmpEml)
	if err != nil {
		t.Fatal(err)
	}
	f.WriteString("email body")
	f.Close()
	eml.SetFilename(tmpEml)

	mboxes, err = store.readCurrentState()
	assert.NoError(t, err)
-- 
2.38.1

[offmap/patches/.build.yml] build success

builds.sr.ht <builds@sr.ht>
Details
Message ID
<COPS2ANO9ON5.23Z1MFLBW6XLV@cirno2>
In-Reply-To
<20221130163824.102652-1-tim@timculverhouse.com> (view parent)
DKIM signature
missing
Download raw message
offmap/patches/.build.yml: SUCCESS in 1m15s

[email: don't keep files open after downloading][0] v2 from [Tim Culverhouse][1]

[0]: https://lists.sr.ht/~rockorager/offmap/patches/37247
[1]: tim@timculverhouse.com

✓ #895050 SUCCESS offmap/patches/.build.yml https://builds.sr.ht/~rockorager/job/895050
Details
Message ID
<COPS51DDI0J2.3SMMJ3X2SILUK@hades.moritz.sh>
In-Reply-To
<20221130163824.102652-1-tim@timculverhouse.com> (view parent)
DKIM signature
missing
Download raw message
Reviewed-by: Moritz Poldrack <moritz@poldrack.dev>
-- 
Moritz Poldrack
https://moritz.sh
Reply to thread Export thread (mbox)