From ef50f4abf0fa4ab2f91be53926af51076f492604 Mon Sep 17 00:00:00 2001 From: Mechiel Lukkien Date: Wed, 1 Nov 2023 18:57:38 +0100 Subject: [PATCH] refactor common pattern of close & remove temporary file into calling the new store.CloseRemoveTempFile --- ctl.go | 8 +------- import.go | 8 +------- queue/dsn.go | 8 +------- serve_unix.go | 9 ++------- smtpserver/dsn.go | 11 ++--------- smtpserver/server.go | 8 +------- store/export.go | 6 +----- store/import.go | 6 +----- webaccount/account.go | 6 +----- webaccount/import.go | 18 +++--------------- webmail/api.go | 8 +------- 11 files changed, 15 insertions(+), 81 deletions(-) diff --git a/ctl.go b/ctl.go index 08e0cd4..e62b299 100644 --- a/ctl.go +++ b/ctl.go @@ -319,13 +319,7 @@ func servectlcmd(ctx context.Context, ctl *ctl, shutdown func()) { msgFile, err := store.CreateMessageTemp("ctl-deliver") ctl.xcheck(err, "creating temporary message file") - defer func() { - name := msgFile.Name() - err := msgFile.Close() - log.Check(err, "closing temporary message file") - err = os.Remove(name) - log.Check(err, "removing temporary message file", mlog.Field("path", name)) - }() + defer store.CloseRemoveTempFile(log, msgFile, "deliver message") mw := message.NewWriter(msgFile) ctl.xwriteok() diff --git a/import.go b/import.go index a281284..79c855d 100644 --- a/import.go +++ b/import.go @@ -311,13 +311,7 @@ func importctl(ctx context.Context, ctl *ctl, mbox bool) { conf, _ := a.Conf() process := func(m *store.Message, msgf *os.File, origPath string) { - defer func() { - name := msgf.Name() - err := msgf.Close() - ctl.log.Check(err, "closing temporary message after failing to import") - err = os.Remove(name) - ctl.log.Check(err, "removing temporary message after failing to import", mlog.Field("path", name)) - }() + defer store.CloseRemoveTempFile(ctl.log, msgf, "message to import") for _, kw := range m.Keywords { mailboxKeywords[kw] = true diff --git a/queue/dsn.go b/queue/dsn.go index 06c1e83..73269c3 100644 --- a/queue/dsn.go +++ b/queue/dsn.go @@ -175,13 +175,7 @@ func deliverDSN(log *mlog.Log, m Msg, remoteMTA dsn.NameIP, secodeOpt, errmsg st qlog("creating temporary message file", err) return } - defer func() { - name := msgFile.Name() - err := msgFile.Close() - log.Check(err, "closing message file") - err = os.Remove(name) - log.Check(err, "removing message file", mlog.Field("path", name)) - }() + defer store.CloseRemoveTempFile(log, msgFile, "dsn message") msgWriter := message.NewWriter(msgFile) if _, err := msgWriter.Write(msgData); err != nil { diff --git a/serve_unix.go b/serve_unix.go index 79414cd..bb66ff8 100644 --- a/serve_unix.go +++ b/serve_unix.go @@ -290,13 +290,8 @@ Only implemented on unix systems, not Windows. log.Infox("making temporary message file for changelog delivery", err) return next } - defer func() { - name := f.Name() - err = f.Close() - log.Check(err, "closing temp changelog file") - err := os.Remove(name) - log.Check(err, "removing temp changelog file", mlog.Field("path", name)) - }() + defer store.CloseRemoveTempFile(log, f, "message for changelog delivery") + m := &store.Message{ Received: time.Now(), Flags: store.Flags{Flagged: true}, diff --git a/smtpserver/dsn.go b/smtpserver/dsn.go index 2a5d6e7..56e8396 100644 --- a/smtpserver/dsn.go +++ b/smtpserver/dsn.go @@ -3,10 +3,8 @@ package smtpserver import ( "context" "fmt" - "os" "github.com/mjl-/mox/dsn" - "github.com/mjl-/mox/mlog" "github.com/mjl-/mox/queue" "github.com/mjl-/mox/smtp" "github.com/mjl-/mox/store" @@ -30,13 +28,8 @@ func queueDSN(ctx context.Context, c *conn, rcptTo smtp.Path, m dsn.Message, req if err != nil { return fmt.Errorf("creating temp file: %w", err) } - defer func() { - name := f.Name() - err = f.Close() - c.log.Check(err, "closing temporary dsn message file") - err := os.Remove(name) - c.log.Check(err, "removing temporary dsn message file", mlog.Field("path", name)) - }() + defer store.CloseRemoveTempFile(c.log, f, "smtpserver dsn message") + if _, err := f.Write([]byte(buf)); err != nil { return fmt.Errorf("writing dsn file: %w", err) } diff --git a/smtpserver/server.go b/smtpserver/server.go index 7a24257..c0e265e 100644 --- a/smtpserver/server.go +++ b/smtpserver/server.go @@ -1532,13 +1532,7 @@ func (c *conn) cmdData(p *parser) { if err != nil { xsmtpServerErrorf(errCodes(smtp.C451LocalErr, smtp.SeSys3Other0, err), "creating temporary file for message: %s", err) } - defer func() { - name := dataFile.Name() - err := dataFile.Close() - c.log.Check(err, "removing temporary message file") - err = os.Remove(name) - c.log.Check(err, "removing temporary message file", mlog.Field("path", name)) - }() + defer store.CloseRemoveTempFile(c.log, dataFile, "smtpserver delivered message") msgWriter := message.NewWriter(dataFile) dr := smtp.NewDataReader(c.r) n, err := io.Copy(&limitWriter{maxSize: c.maxMessageSize, w: msgWriter}, dr) diff --git a/store/export.go b/store/export.go index 829a3ec..fe4f15f 100644 --- a/store/export.go +++ b/store/export.go @@ -214,11 +214,7 @@ func ExportMessages(ctx context.Context, log *mlog.Log, db *bstore.DB, accountDi var mboxwriter *bufio.Writer defer func() { if mboxtmp != nil { - name := mboxtmp.Name() - err := mboxtmp.Close() - log.Check(err, "closing mbox temp file") - err = os.Remove(name) - log.Check(err, "removing mbox temp file", mlog.Field("name", name)) + CloseRemoveTempFile(log, mboxtmp, "mbox") } }() diff --git a/store/import.go b/store/import.go index 004b3a2..dedc55e 100644 --- a/store/import.go +++ b/store/import.go @@ -84,11 +84,7 @@ func (mr *MboxReader) Next() (*Message, *os.File, string, error) { } defer func() { if f != nil { - name := f.Name() - err := f.Close() - mr.log.Check(err, "closing temporary message file after mbox read error") - err = os.Remove(name) - mr.log.Check(err, "removing temporary message file after mbox read error", mlog.Field("path", name)) + CloseRemoveTempFile(mr.log, f, "message after mbox read error") } }() diff --git a/webaccount/account.go b/webaccount/account.go index 6b32814..511c3e4 100644 --- a/webaccount/account.go +++ b/webaccount/account.go @@ -299,11 +299,7 @@ func Handle(w http.ResponseWriter, r *http.Request) { } defer func() { if tmpf != nil { - name := tmpf.Name() - err := tmpf.Close() - log.Check(err, "closing uploaded file") - err = os.Remove(name) - log.Check(err, "removing temporary file", mlog.Field("path", name)) + store.CloseRemoveTempFile(log, tmpf, "upload") } }() if _, err := io.Copy(tmpf, f); err != nil { diff --git a/webaccount/import.go b/webaccount/import.go index 1cbcf6b..2f2e14f 100644 --- a/webaccount/import.go +++ b/webaccount/import.go @@ -202,11 +202,7 @@ type importStep struct { func importStart(log *mlog.Log, accName string, f *os.File, skipMailboxPrefix string) (string, error) { defer func() { if f != nil { - name := f.Name() - err := f.Close() - log.Check(err, "closing uploaded file") - err = os.Remove(name) - log.Check(err, "removing uploaded file", mlog.Field("name", name)) + store.CloseRemoveTempFile(log, f, "upload for import") } }() @@ -316,11 +312,7 @@ func importMessages(ctx context.Context, log *mlog.Log, token string, acc *store } defer func() { - name := f.Name() - err := f.Close() - log.Check(err, "closing uploaded messages file") - err = os.Remove(name) - log.Check(err, "removing uploaded messages file", mlog.Field("path", name)) + store.CloseRemoveTempFile(log, f, "uploaded messages") for _, id := range deliveredIDs { p := acc.MessagePath(id) @@ -594,11 +586,7 @@ func importMessages(ctx context.Context, log *mlog.Log, token string, acc *store ximportcheckf(err, "creating temp message") defer func() { if f != nil { - name := f.Name() - err = f.Close() - log.Check(err, "closing temporary file for delivery") - err := os.Remove(name) - log.Check(err, "removing temporary file for delivery", mlog.Field("path", name)) + store.CloseRemoveTempFile(log, f, "message to import") } }() diff --git a/webmail/api.go b/webmail/api.go index c828ba7..f72539b 100644 --- a/webmail/api.go +++ b/webmail/api.go @@ -359,13 +359,7 @@ func (w Webmail) MessageSubmit(ctx context.Context, m SubmitMessage) { // Create file to compose message into. dataFile, err := store.CreateMessageTemp("webmail-submit") xcheckf(ctx, err, "creating temporary file for message") - defer func() { - name := dataFile.Name() - err := dataFile.Close() - log.Check(err, "closing submit message file") - err = os.Remove(name) - log.Check(err, "removing temporary submit message file", mlog.Field("name", name)) - }() + defer store.CloseRemoveTempFile(log, dataFile, "message to submit") // If writing to the message file fails, we abort immediately. xmsgw := &xerrWriter{ctx, bufio.NewWriter(dataFile), 0, w.maxMessageSize}