improve training of junk filter

before, we used heuristics to decide when to train/untrain a message as junk or
nonjunk: the message had to be seen, be in certain mailboxes. then if a message
was marked as junk, it was junk. and otherwise it was nonjunk. this wasn't good
enough: you may want to keep some messages around as neither junk or nonjunk.
and that wasn't possible.

ideally, we would just look at the imap $Junk and $NotJunk flags. the problem
is that mail clients don't set these flags, or don't make it easy. thunderbird
can set the flags based on its own bayesian filter. it has a shortcut for
marking Junk and moving it to the junk folder (good), but the counterpart of
notjunk only marks a message as notjunk without showing in the UI that it was
marked as notjunk. there is also no "move and mark as notjunk" mechanism. e.g.
"archive" does not mark a message as notjunk. ios mail and mutt don't appear to
have any way to see or change the $Junk and $NotJunk flags.

what email clients do have is the ability to move messages to other
mailboxes/folders. so mox now has a mechanism that allows you to configure
mailboxes that automatically set $Junk or $NotJunk (or clear both) when a
message is moved/copied/delivered to that folder. e.g. a mailbox called junk or
spam or rejects marks its messags as junk. inbox, postmaster, dmarc, tlsrpt,
neutral* mark their messages as neither junk or notjunk. other folders mark
their messages as notjunk. e.g. list/*, archive. this functionality is
optional, but enabled with the quickstart and for new accounts.

also, mox now keeps track of the previous training of a message and will only
untrain/train if needed. before, there probably have been duplicate or missing
(un)trainings.

this also includes a new subcommand "retrain" to recreate the junkfilter for an
account. you should run it after updating to this version. and you should
probably also modify your account config to include the AutomaticJunkFlags.

config/config.go

@@ -198,10 +198,19 @@ type Account struct {
 	SubjectPass  struct {
 		Period time.Duration `sconf-doc:"How long unique values are accepted after generating, e.g. 12h."` // todo: have a reasonable default for this?
 	} `sconf:"optional" sconf-doc:"If configured, messages classified as weakly spam are rejected with instructions to retry delivery, but this time with a signed token added to the subject. During the next delivery attempt, the signed token will bypass the spam filter. Messages with a clear spam signal, such as a known bad reputation, are rejected/delayed without a signed token."`
-	RejectsMailbox string      `sconf:"optional" sconf-doc:"Mail that looks like spam will be rejected, but a copy can be stored temporarily in a mailbox, e.g. Rejects. If mail isn't coming in when you expect, you can look there. The mail still isn't accepted, so the remote mail server may retry (hopefully, if legitimate), or give up (hopefully, if indeed a spammer)."`
+	RejectsMailbox     string `sconf:"optional" sconf-doc:"Mail that looks like spam will be rejected, but a copy can be stored temporarily in a mailbox, e.g. Rejects. If mail isn't coming in when you expect, you can look there. The mail still isn't accepted, so the remote mail server may retry (hopefully, if legitimate), or give up (hopefully, if indeed a spammer). Messages are automatically removed from this mailbox, so do not set it to a mailbox that has messages you want to keep."`
-	JunkFilter     *JunkFilter `sconf:"optional" sconf-doc:"Content-based filtering, using the junk-status of individual messages to rank words in such messages as spam or ham. It is recommended you always set the applicable (non)-junk status on messages, and that you do not empty your Trash because those messages contain valuable ham/spam training information."` // todo: sane defaults for junkfilter
+	AutomaticJunkFlags struct {
+		Enabled              bool   `sconf-doc:"If enabled, flags will be set automatically if they match a regular expression below. When two lists are set, the empty list will match all remaining messages. Messages are matched in the order specified and the search stops on the first match. Mailboxes are lowercased before matching."`
+		JunkMailboxRegexp    string `sconf:"optional" sconf-doc:"Example: ^(junk|spam|rejects)."`
+		NeutralMailboxRegexp string `sconf:"optional" sconf-doc:"Example: ^(inbox|neutral|postmaster|dmarc|tlsrpt), and you may wish to add trash depending on how you use it, or leave this empty."`
+		NotJunkMailboxRegexp string `sconf:"optional" sconf-doc:"Example: .* or an empty string."`
+	} `sconf:"optional" sconf-doc:"Automatically set $Junk and $NotJunk flags based on mailbox messages are delivered/moved/copied to. Email clients typically have too limited functionality to conveniently set these flags, especially $NonJunk, but they can all move messages to a different mailbox, so this helps them."`
+	JunkFilter *JunkFilter `sconf:"optional" sconf-doc:"Content-based filtering, using the junk-status of individual messages to rank words in such messages as spam or ham. It is recommended you always set the applicable (non)-junk status on messages, and that you do not empty your Trash because those messages contain valuable ham/spam training information."` // todo: sane defaults for junkfilter
 
-	DNSDomain dns.Domain `sconf:"-"` // Parsed form of Domain.
+	DNSDomain      dns.Domain     `sconf:"-"` // Parsed form of Domain.
+	JunkMailbox    *regexp.Regexp `sconf:"-" json:"-"`
+	NeutralMailbox *regexp.Regexp `sconf:"-" json:"-"`
+	NotJunkMailbox *regexp.Regexp `sconf:"-" json:"-"`
 }
 
 type JunkFilter struct {

config/doc.go

@@ -424,9 +424,32 @@ describe-static" and "mox config describe-domains":
 			# in a mailbox, e.g. Rejects. If mail isn't coming in when you expect, you can
 			# look there. The mail still isn't accepted, so the remote mail server may retry
 			# (hopefully, if legitimate), or give up (hopefully, if indeed a spammer).
-			# (optional)
+			# Messages are automatically removed from this mailbox, so do not set it to a
+			# mailbox that has messages you want to keep. (optional)
 			RejectsMailbox:
 
+			# Automatically set $Junk and $NotJunk flags based on mailbox messages are
+			# delivered/moved/copied to. Email clients typically have too limited
+			# functionality to conveniently set these flags, especially $NonJunk, but they can
+			# all move messages to a different mailbox, so this helps them. (optional)
+			AutomaticJunkFlags:
+
+				# If enabled, flags will be set automatically if they match a regular expression
+				# below. When two lists are set, the empty list will match all remaining messages.
+				# Messages are matched in the order specified and the search stops on the first
+				# match. Mailboxes are lowercased before matching.
+				Enabled: false
+
+				# Example: ^(junk|spam|rejects). (optional)
+				JunkMailboxRegexp:
+
+				# Example: ^(inbox|neutral|postmaster|dmarc|tlsrpt), and you may wish to add trash
+				# depending on how you use it, or leave this empty. (optional)
+				NeutralMailboxRegexp:
+
+				# Example: .* or an empty string. (optional)
+				NotJunkMailboxRegexp:
+
 			# Content-based filtering, using the junk-status of individual messages to rank
 			# words in such messages as spam or ham. It is recommended you always set the
 			# applicable (non)-junk status on messages, and that you do not empty your Trash

ctl.go

@@ -8,6 +8,7 @@ import (
 	"log"
 	"net"
 	"os"
+	"path/filepath"
 	"runtime"
 	"runtime/debug"
 	"sort"
@@ -16,6 +17,8 @@ import (
 	"syscall"
 	"time"
 
+	"github.com/mjl-/bstore"
+
 	"github.com/mjl-/mox/dns"
 	"github.com/mjl-/mox/message"
 	"github.com/mjl-/mox/metrics"
@@ -629,6 +632,67 @@ func servectlcmd(ctx context.Context, log *mlog.Log, ctl *ctl, xcmd *string, shu
 		}
 		ctl.xwriteok()
 
+	case "retrain":
+		/* protocol:
+		> "retrain"
+		> account
+		< "ok" or error
+		*/
+		account := ctl.xread()
+		acc, err := store.OpenAccount(account)
+		ctl.xcheck(err, "open account")
+
+		acc.WithWLock(func() {
+			conf, _ := acc.Conf()
+			if conf.JunkFilter == nil {
+				ctl.xcheck(store.ErrNoJunkFilter, "looking for junk filter")
+			}
+
+			// Remove existing junk filter files.
+			basePath := mox.DataDirPath("accounts")
+			dbPath := filepath.Join(basePath, acc.Name, "junkfilter.db")
+			bloomPath := filepath.Join(basePath, acc.Name, "junkfilter.bloom")
+			if err := os.Remove(dbPath); err != nil {
+				log.Errorx("removing old junkfilter database file", err, mlog.Field("path", dbPath))
+			}
+			if err := os.Remove(bloomPath); err != nil {
+				log.Errorx("removing old junkfilter bloom filter file", err, mlog.Field("path", bloomPath))
+			}
+
+			// Open junk filter, this creates new files.
+			jf, _, err := acc.OpenJunkFilter(ctl.log)
+			ctl.xcheck(err, "open new junk filter")
+			defer func() {
+				if jf == nil {
+					return
+				}
+				if err := jf.Close(); err != nil {
+					log.Errorx("closing junk filter during cleanup", err)
+				}
+			}()
+
+			// Read through messages with junk or nonjunk flag set, and train them.
+			var total, trained int
+			q := bstore.QueryDB[store.Message](acc.DB)
+			err = q.ForEach(func(m store.Message) error {
+				total++
+				ok, err := acc.TrainMessage(ctl.log, jf, m)
+				if ok {
+					trained++
+				}
+				return err
+			})
+			ctl.xcheck(err, "training messages")
+			ctl.log.Info("retrained messages", mlog.Field("total", total), mlog.Field("trained", trained))
+
+			// Close junk filter, marking success.
+			err = jf.Close()
+			jf = nil
+			ctl.xcheck(err, "closing junk filter")
+		})
+
+		ctl.xwriteok()
+
 	default:
 		log.Info("unrecognized command", mlog.Field("cmd", cmd))
 		ctl.xwrite("unrecognized command")

doc.go

@@ -55,6 +55,7 @@ low-maintenance self-hosted email.
 	mox dnsbl check zone ip
 	mox dnsbl checkhealth zone
 	mox mtasts lookup domain
+	mox retrain accountname
 	mox sendmail [-Fname] [ignoredflags] [-t] [<message]
 	mox spf check domain ip
 	mox spf lookup domain
@@ -542,6 +543,15 @@ should be used, and how long the policy can be cached.
 
 	usage: mox mtasts lookup domain
 
+# mox retrain
+
+Recreate and retrain the junk filter for the account.
+
+Useful after having made changes to the junk filter configuration, or if the
+implementation has changed.
+
+	usage: mox retrain accountname
+
 # mox sendmail
 
 Sendmail is a drop-in replacement for /usr/sbin/sendmail to deliver emails sent by unix processes like cron.

go.mod

@@ -3,7 +3,7 @@ module github.com/mjl-/mox
 go 1.18
 
 require (
-	github.com/mjl-/bstore v0.0.0-20230114150735-9d9c0a2dcc79
+	github.com/mjl-/bstore v0.0.0-20230211204415-a9899ef6e782
 	github.com/mjl-/sconf v0.0.4
 	github.com/mjl-/sherpa v0.6.5
 	github.com/mjl-/sherpadoc v0.0.10

go.sum

@@ -145,8 +145,8 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/matttproud/golang_protobuf_extensions v1.0.1 h1:4hp9jkHxhMHkqkrB3Ix0jegS5sx/RkqARlsWZ6pIwiU=
 github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=
-github.com/mjl-/bstore v0.0.0-20230114150735-9d9c0a2dcc79 h1:bptDsTAvgtmIOrhKjMVrUm4JBkF0jekpVmsZdkgALPM=
+github.com/mjl-/bstore v0.0.0-20230211204415-a9899ef6e782 h1:dVwJA/wXzXXUROM9oM3Stg3cmqixiFh4Zi1Xumvtj74=
-github.com/mjl-/bstore v0.0.0-20230114150735-9d9c0a2dcc79/go.mod h1:/cD25FNBaDfvL/plFRxI3Ba3E+wcB0XVOS8nJDqndg0=
+github.com/mjl-/bstore v0.0.0-20230211204415-a9899ef6e782/go.mod h1:/cD25FNBaDfvL/plFRxI3Ba3E+wcB0XVOS8nJDqndg0=
 github.com/mjl-/sconf v0.0.4 h1:uyfn4vv5qOULSgiwQsPbbgkiONKnMFMsSOhsHfAiYwI=
 github.com/mjl-/sconf v0.0.4/go.mod h1:ezf7YOn7gtClo8y71SqgZKaEkyMQ5Te7vkv4PmTTfwM=
 github.com/mjl-/sherpa v0.6.5 h1:d90uG/j8fw+2M+ohCTAcVwTSUURGm8ktYDScJO1nKog=

http/account.html

@@ -138,6 +138,7 @@ const index = async () => {
 	const page = document.getElementById('page')
 	dom._kids(page,
 		crumbs('Mox Account'),
+		dom.p('NOTE: Not all account settings can be configured through these pages yet. See the configuration file for more options.'),
 		dom.div(
 			'Default domain: ',
 			domain.ASCII ? domainString(domain) : '(none)',

imapserver/copy_test.go

@@ -43,11 +43,19 @@ func TestCopy(t *testing.T) {
 	ptr := func(v uint32) *uint32 { return &v }
 	tc.xcodeArg(imapclient.CodeCopyUID{DestUIDValidity: 1, From: []imapclient.NumRange{{First: 3, Last: ptr(4)}}, To: []imapclient.NumRange{{First: 1, Last: ptr(2)}}})
 	tc2.transactf("ok", "noop")
-	tc2.xuntagged(imapclient.UntaggedExists(2), imapclient.UntaggedFetch{Seq: 1, Attrs: []imapclient.FetchAttr{imapclient.FetchUID(1), imapclient.FetchFlags(nil)}}, imapclient.UntaggedFetch{Seq: 2, Attrs: []imapclient.FetchAttr{imapclient.FetchUID(2), imapclient.FetchFlags(nil)}})
+	tc2.xuntagged(
+		imapclient.UntaggedExists(2),
+		imapclient.UntaggedFetch{Seq: 1, Attrs: []imapclient.FetchAttr{imapclient.FetchUID(1), imapclient.FetchFlags(nil)}},
+		imapclient.UntaggedFetch{Seq: 2, Attrs: []imapclient.FetchAttr{imapclient.FetchUID(2), imapclient.FetchFlags(nil)}},
+	)
 
 	tc.transactf("no", "uid copy 1,2 Trash") // No match.
 	tc.transactf("ok", "uid copy 4,3 Trash")
 	tc.xcodeArg(imapclient.CodeCopyUID{DestUIDValidity: 1, From: []imapclient.NumRange{{First: 3, Last: ptr(4)}}, To: []imapclient.NumRange{{First: 3, Last: ptr(4)}}})
 	tc2.transactf("ok", "noop")
-	tc2.xuntagged(imapclient.UntaggedExists(4), imapclient.UntaggedFetch{Seq: 3, Attrs: []imapclient.FetchAttr{imapclient.FetchUID(3), imapclient.FetchFlags(nil)}}, imapclient.UntaggedFetch{Seq: 4, Attrs: []imapclient.FetchAttr{imapclient.FetchUID(4), imapclient.FetchFlags(nil)}})
+	tc2.xuntagged(
+		imapclient.UntaggedExists(4),
+		imapclient.UntaggedFetch{Seq: 3, Attrs: []imapclient.FetchAttr{imapclient.FetchUID(3), imapclient.FetchFlags(nil)}},
+		imapclient.UntaggedFetch{Seq: 4, Attrs: []imapclient.FetchAttr{imapclient.FetchUID(4), imapclient.FetchFlags(nil)}},
+	)
 }

imapserver/server.go

@@ -1951,11 +1951,13 @@ func (c *conn) cmdDelete(tag, cmd string, p *parser) {
 				_, err = qm.Delete()
 				xcheckf(err, "removing messages")
 
-				conf, _ := c.account.Conf()
+				// Mark messages as not needing training. Then retrain them, so that are untrained if they were.
-				if name != conf.RejectsMailbox {
+				for i := range remove {
-					err = c.account.Untrain(c.log, remove)
+					remove[i].Junk = false
-					xcheckf(err, "untraining deleted messages")
+					remove[i].Notjunk = false
 				}
+				err = c.account.RetrainMessages(c.log, tx, remove, true)
+				xcheckf(err, "untraining deleted messages")
 			}
 
 			err = tx.Delete(&store.Mailbox{ID: mb.ID})
@@ -2519,7 +2521,7 @@ func (c *conn) cmdAppend(tag, cmd string, p *parser) {
 				MsgPrefix:     msgPrefix,
 			}
 			isSent := name == "Sent"
-			c.account.DeliverX(c.log, tx, &msg, msgFile, true, isSent, true, true)
+			c.account.DeliverX(c.log, tx, &msg, msgFile, true, isSent, true)
 		})
 
 		// Fetch pending changes, possibly with new UIDs, so we can apply them before adding our own new UID.
@@ -2684,11 +2686,14 @@ func (c *conn) xexpunge(uidSet *numSet, missingMailboxOK bool) []store.Message {
 			_, err = qm.Delete()
 			xcheckf(err, "removing messages marked for deletion")
 
-			conf, _ := c.account.Conf()
+			// Mark removed messages as not needing training, then retrain them, so if they
-			if mb.Name != conf.RejectsMailbox {
+			// were trained, they get untrained.
-				err = c.account.Untrain(c.log, remove)
+			for i := range remove {
-				xcheckf(err, "untraining deleted messages")
+				remove[i].Junk = false
+				remove[i].Notjunk = false
 			}
+			err = c.account.RetrainMessages(c.log, tx, remove, true)
+			xcheckf(err, "untraining deleted messages")
 		})
 
 		// Broadcast changes to other connections. We may not have actually removed any
@@ -2920,6 +2925,9 @@ func (c *conn) cmdxCopy(isUID bool, tag, cmd string, p *parser) {
 			for _, m := range xmsgs {
 				msgs[m.UID] = m
 			}
+			nmsgs := make([]store.Message, len(xmsgs))
+
+			conf, _ := c.account.Conf()
 
 			// Insert new messages into database.
 			var origMsgIDs, newMsgIDs []int64
@@ -2934,9 +2942,12 @@ func (c *conn) cmdxCopy(isUID bool, tag, cmd string, p *parser) {
 				m.UID = uidFirst + store.UID(i)
 				m.MailboxID = mbDst.ID
 				m.MailboxOrigID = mbSrc.ID
+				m.TrainedJunk = nil
+				m.JunkFlagsForMailbox(mbDst.Name, conf)
 				err := tx.Insert(&m)
 				xcheckf(err, "inserting message")
 				msgs[uid] = m
+				nmsgs[i] = m
 				origUIDs = append(origUIDs, uid)
 				newUIDs = append(newUIDs, m.UID)
 				newMsgIDs = append(newMsgIDs, m.ID)
@@ -2964,11 +2975,8 @@ func (c *conn) cmdxCopy(isUID bool, tag, cmd string, p *parser) {
 				createdIDs = append(createdIDs, newMsgIDs[i])
 			}
 
-			conf, _ := c.account.Conf()
+			err = c.account.RetrainMessages(c.log, tx, nmsgs, false)
-			if mbDst.Name != conf.RejectsMailbox {
+			xcheckf(err, "train copied messages")
-				err = c.account.Train(c.log, xmsgs)
-				xcheckf(err, "train copied messages")
-			}
 		})
 
 		// Broadcast changes to other connections.
@@ -3087,6 +3095,7 @@ func (c *conn) cmdxMove(isUID bool, tag, cmd string, p *parser) {
 				xserverErrorf("uid and message mismatch")
 			}
 
+			conf, _ := c.account.Conf()
 			for i := range msgs {
 				m := &msgs[i]
 				if m.UID != uids[i] {
@@ -3094,11 +3103,15 @@ func (c *conn) cmdxMove(isUID bool, tag, cmd string, p *parser) {
 				}
 				m.MailboxID = mbDst.ID
 				m.UID = uidnext
+				m.JunkFlagsForMailbox(mbDst.Name, conf)
 				uidnext++
 				err := tx.Update(m)
 				xcheckf(err, "updating moved message in database")
 			}
 
+			err = c.account.RetrainMessages(c.log, tx, msgs, false)
+			xcheckf(err, "retraining messages after move")
+
 			// Prepare broadcast changes to other connections.
 			changes = make([]store.Change, 0, 1+len(msgs))
 			changes = append(changes, store.ChangeRemoveUIDs{MailboxID: c.mailboxID, UIDs: uids})
@@ -3172,12 +3185,10 @@ func (c *conn) cmdxStore(isUID bool, tag, cmd string, p *parser) {
 	updates := store.FlagsQuerySet(mask, flags)
 
 	var updated []store.Message
-	var oflags []store.Flags
 
 	c.account.WithWLock(func() {
-		var mb store.Mailbox
 		c.xdbwrite(func(tx *bstore.Tx) {
-			mb = c.xmailboxID(tx, c.mailboxID) // Validate.
+			c.xmailboxID(tx, c.mailboxID) // Validate.
 
 			uidargs := c.xnumSetCondition(isUID, nums)
 
@@ -3188,11 +3199,6 @@ func (c *conn) cmdxStore(isUID bool, tag, cmd string, p *parser) {
 			q := bstore.QueryTx[store.Message](tx)
 			q.FilterNonzero(store.Message{MailboxID: c.mailboxID})
 			q.FilterEqual("UID", uidargs...)
-			q.FilterFn(func(m store.Message) bool {
-				// We use this filter just to get the pre-update flags...
-				oflags = append(oflags, m.Flags)
-				return true
-			})
 			if len(updates) == 0 {
 				var err error
 				updated, err = q.List()
@@ -3202,29 +3208,10 @@ func (c *conn) cmdxStore(isUID bool, tag, cmd string, p *parser) {
 				_, err := q.UpdateFields(updates)
 				xcheckf(err, "updating flags")
 			}
-		})
 
-		conf, _ := c.account.Conf()
+			err := c.account.RetrainMessages(c.log, tx, updated, false)
-		if mb.Name != conf.RejectsMailbox {
+			xcheckf(err, "training messages")
-			jf, _, err := c.account.OpenJunkFilter(c.log)
+		})
-			if err == nil {
-				defer func() {
-					if jf != nil {
-						err := jf.Close()
-						c.xsanity(err, "closing junkfilter")
-					}
-				}()
-				for i, m := range updated {
-					err := c.account.Retrain(c.log, jf, oflags[i], m)
-					xcheckf(err, "retraining message")
-				}
-				err = jf.Close()
-				jf = nil
-				xcheckf(err, "closing junkfilter")
-			} else if !errors.Is(err, store.ErrNoJunkFilter) {
-				xcheckf(err, "open junk filter for retraining")
-			}
-		}
 
 		// Broadcast changes to other connections.
 		changes := make([]store.Change, len(updated))

import.go

@@ -216,7 +216,10 @@ func importctl(ctl *ctl, mbox bool) {
 
 	// Messages don't always have a junk flag set. We'll assume anything in a mailbox
 	// starting with junk or spam is junk mail.
-	isjunk := strings.HasPrefix(strings.ToLower(mailbox), "junk") || strings.HasPrefix(strings.ToLower(mailbox), "spam")
+
+	var msgJunkFlags store.Message
+	conf, _ := a.Conf()
+	msgJunkFlags.JunkFlagsForMailbox(mailbox, conf)
 
 	// First check if we can access the mbox/maildir.
 	// Mox needs to be able to access those files, the user running the import command
@@ -224,13 +227,13 @@ func importctl(ctl *ctl, mbox bool) {
 	if mbox {
 		mboxf, err = os.Open(src)
 		ctl.xcheck(err, "open mbox file")
-		msgreader = newMboxReader(isjunk, store.CreateMessageTemp, mboxf, ctl.log)
+		msgreader = newMboxReader(msgJunkFlags.Junk, msgJunkFlags.Notjunk, store.CreateMessageTemp, mboxf, ctl.log)
 	} else {
 		mdnewf, err = os.Open(filepath.Join(src, "new"))
 		ctl.xcheck(err, "open subdir new of maildir")
 		mdcurf, err = os.Open(filepath.Join(src, "cur"))
 		ctl.xcheck(err, "open subdir cur of maildir")
-		msgreader = newMaildirReader(isjunk, store.CreateMessageTemp, mdnewf, mdcurf, ctl.log)
+		msgreader = newMaildirReader(msgJunkFlags.Junk, msgJunkFlags.Notjunk, store.CreateMessageTemp, mdnewf, mdcurf, ctl.log)
 	}
 
 	tx, err := a.DB.Begin(true)
@@ -276,8 +279,7 @@ func importctl(ctl *ctl, mbox bool) {
 		const consumeFile = true
 		isSent := mailbox == "Sent"
 		const sync = false
-		const train = false
+		a.DeliverX(ctl.log, tx, m, mf, consumeFile, isSent, sync)
-		a.DeliverX(ctl.log, tx, m, mf, consumeFile, isSent, sync, train)
 		deliveredIDs = append(deliveredIDs, m.ID)
 		ctl.log.Debug("delivered message", mlog.Field("id", m.ID))
 		changes = append(changes, store.ChangeAddUID{MailboxID: m.MailboxID, UID: m.UID, Flags: m.Flags})
@@ -337,12 +339,13 @@ func importctl(ctl *ctl, mbox bool) {
 				}
 			}
 
-			if jf != nil && (m.Seen || m.Junk) {
+			if jf != nil && m.NeedsTraining() {
 				if words, err := jf.ParseMessage(p); err != nil {
 					ctl.log.Infox("parsing message for updating junk filter", err, mlog.Field("parse", ""), mlog.Field("path", origPath))
 				} else {
 					err = jf.Train(!m.Junk, words)
 					ctl.xcheck(err, "training junk filter")
+					m.TrainedJunk = &m.Junk
 				}
 			}
 
@@ -402,10 +405,11 @@ type mboxReader struct {
 	log        *mlog.Log
 	eof        bool
 	junk       bool
+	notjunk    bool
 }
 
-func newMboxReader(isjunk bool, createTemp func(pattern string) (*os.File, error), f *os.File, log *mlog.Log) *mboxReader {
+func newMboxReader(isjunk, isnotjunk bool, createTemp func(pattern string) (*os.File, error), f *os.File, log *mlog.Log) *mboxReader {
-	return &mboxReader{createTemp: createTemp, path: f.Name(), line: 1, r: bufio.NewReader(f), log: log, junk: isjunk}
+	return &mboxReader{createTemp: createTemp, path: f.Name(), line: 1, r: bufio.NewReader(f), log: log, junk: isjunk, notjunk: isnotjunk}
 }
 
 func (mr *mboxReader) position() string {
@@ -486,7 +490,7 @@ func (mr *mboxReader) Next() (*store.Message, *os.File, string, error) {
 
 	// todo: look at Status or X-Status header in message?
 	// todo: take Received from the "From " line if present?
-	flags := store.Flags{Seen: true, Junk: mr.junk}
+	flags := store.Flags{Seen: true, Junk: mr.junk, Notjunk: mr.notjunk}
 	m := &store.Message{Flags: flags, Size: size}
 
 	// Prevent cleanup by defer.
@@ -505,9 +509,10 @@ type maildirReader struct {
 	dovecotKeywords []string
 	log             *mlog.Log
 	junk            bool
+	notjunk         bool
 }
 
-func newMaildirReader(isjunk bool, createTemp func(pattern string) (*os.File, error), newf, curf *os.File, log *mlog.Log) *maildirReader {
+func newMaildirReader(isjunk, isnotjunk bool, createTemp func(pattern string) (*os.File, error), newf, curf *os.File, log *mlog.Log) *maildirReader {
 	mr := &maildirReader{createTemp: createTemp, newf: newf, curf: curf, f: newf, log: log, junk: isjunk}
 
 	// Best-effort parsing of dovecot keywords.
@@ -642,6 +647,9 @@ func (mr *maildirReader) Next() (*store.Message, *os.File, string, error) {
 	if mr.junk {
 		flags.Junk = true
 	}
+	if mr.notjunk {
+		flags.Notjunk = true
+	}
 
 	m := &store.Message{Received: received, Flags: flags, Size: size}
 

import_test.go

@@ -18,7 +18,7 @@ func TestMboxReader(t *testing.T) {
 	}
 	defer mboxf.Close()
 
-	mr := newMboxReader(false, createTemp, mboxf, mlog.New("mboxreader"))
+	mr := newMboxReader(false, false, createTemp, mboxf, mlog.New("mboxreader"))
 	_, mf0, _, err := mr.Next()
 	if err != nil {
 		t.Fatalf("next mbox message: %v", err)
@@ -56,7 +56,7 @@ func TestMaildirReader(t *testing.T) {
 	}
 	defer curf.Close()
 
-	mr := newMaildirReader(false, createTemp, newf, curf, mlog.New("maildirreader"))
+	mr := newMaildirReader(false, false, createTemp, newf, curf, mlog.New("maildirreader"))
 	_, mf0, _, err := mr.Next()
 	if err != nil {
 		t.Fatalf("next maildir message: %v", err)

main.go

@@ -121,6 +121,7 @@ var commands = []struct {
 	{"dnsbl check", cmdDNSBLCheck},
 	{"dnsbl checkhealth", cmdDNSBLCheckhealth},
 	{"mtasts lookup", cmdMTASTSLookup},
+	{"retrain", cmdRetrain},
 	{"sendmail", cmdSendmail},
 	{"spf check", cmdSPFCheck},
 	{"spf lookup", cmdSPFLookup},
@@ -1513,6 +1514,25 @@ should be used, and how long the policy can be cached.
 	}
 }
 
+func cmdRetrain(c *cmd) {
+	c.params = "accountname"
+	c.help = `Recreate and retrain the junk filter for the account.
+
+Useful after having made changes to the junk filter configuration, or if the
+implementation has changed.
+`
+	args := c.Parse()
+	if len(args) != 1 {
+		c.Usage()
+	}
+
+	mustLoadConfig()
+	ctl := xctl()
+	ctl.xwrite("retrain")
+	ctl.xwrite(args[0])
+	ctl.xreadok()
+}
+
 func cmdTLSRPTDBAddReport(c *cmd) {
 	c.unlisted = true
 	c.params = "< message"

mox-/admin.go

@@ -129,6 +129,9 @@ func MakeAccountConfig(addr smtp.Address) config.Account {
 			},
 		},
 	}
+	account.AutomaticJunkFlags.Enabled = true
+	account.AutomaticJunkFlags.JunkMailboxRegexp = "^(junk|spam|rejects)"
+	account.AutomaticJunkFlags.NeutralMailboxRegexp = "^(inbox|neutral|postmaster|dmarc|tlsrpt)"
 	account.SubjectPass.Period = 12 * time.Hour
 	return account
 }

mox-/config.go

@@ -732,12 +732,35 @@ func prepareDynamicConfig(ctx context.Context, dynamicPath string, static config
 		if err != nil {
 			addErrorf("parsing domain %q for account %q: %s", acc.Domain, accName, err)
 		}
-		c.Accounts[accName] = acc
 
 		if strings.EqualFold(acc.RejectsMailbox, "Inbox") {
-			addErrorf("account %q: cannot set RejectsMailbox to inbox", accName)
+			addErrorf("account %q: cannot set RejectsMailbox to inbox, messages will be removed automatically from the rejects mailbox", accName)
 		}
 		checkMailboxNormf(acc.RejectsMailbox, "account %q", accName)
+
+		if acc.AutomaticJunkFlags.JunkMailboxRegexp != "" {
+			r, err := regexp.Compile(acc.AutomaticJunkFlags.JunkMailboxRegexp)
+			if err != nil {
+				addErrorf("invalid JunkMailboxRegexp regular expression: %v", err)
+			}
+			acc.JunkMailbox = r
+		}
+		if acc.AutomaticJunkFlags.NeutralMailboxRegexp != "" {
+			r, err := regexp.Compile(acc.AutomaticJunkFlags.NeutralMailboxRegexp)
+			if err != nil {
+				addErrorf("invalid NeutralMailboxRegexp regular expression: %v", err)
+			}
+			acc.NeutralMailbox = r
+		}
+		if acc.AutomaticJunkFlags.NotJunkMailboxRegexp != "" {
+			r, err := regexp.Compile(acc.AutomaticJunkFlags.NotJunkMailboxRegexp)
+			if err != nil {
+				addErrorf("invalid NotJunkMailboxRegexp regular expression: %v", err)
+			}
+			acc.NotJunkMailbox = r
+		}
+		c.Accounts[accName] = acc
+
 		for addrName, dest := range acc.Destinations {
 			checkMailboxNormf(dest.Mailbox, "account %q, destination %q", accName, addrName)
 

smtpserver/reputation.go

@@ -122,7 +122,9 @@ func reputation(tx *bstore.Tx, log *mlog.Log, m *store.Message) (rjunk *bool, rc
 	messageQuery := func(fm *store.Message, maxAge time.Duration, maxCount int) *bstore.Query[store.Message] {
 		q := bstore.QueryTx[store.Message](tx)
 		q.FilterEqual("MailboxOrigID", m.MailboxID)
-		q.FilterEqual("Seen", true)
+		q.FilterFn(func(m store.Message) bool {
+			return m.Junk || m.Notjunk
+		})
 		if fm != nil {
 			q.FilterNonzero(*fm)
 		}
@@ -167,10 +169,9 @@ func reputation(tx *bstore.Tx, log *mlog.Log, m *store.Message) (rjunk *bool, rc
 		q.FilterEqual("MsgFromValidated", m.MsgFromValidated)
 		msgs := xmessageList(q, "mgsfromfull")
 		if len(msgs) > 0 {
-			ham := !msgs[0].Junk || len(msgs) > 1 && !msgs[1].Junk
-			conclusive := m.MsgFromValidated
 			// todo: we may want to look at dkim/spf in this case.
-			spam := !ham
+			spam := msgs[0].Junk && (len(msgs) == 1 || msgs[1].Junk)
+			conclusive := m.MsgFromValidated
 			return &spam, conclusive, methodMsgfromFull, nil
 		}
 		if !m.MsgFromValidated {
@@ -180,8 +181,8 @@ func reputation(tx *bstore.Tx, log *mlog.Log, m *store.Message) (rjunk *bool, rc
 			q := messageQuery(&store.Message{MsgFromLocalpart: m.MsgFromLocalpart, MsgFromDomain: m.MsgFromDomain, MsgFromValidated: true}, 3*year, 2)
 			msgs = xmessageList(q, "msgfromfull-validated")
 			if len(msgs) > 0 {
-				ham := !msgs[0].Junk || len(msgs) > 1 && !msgs[1].Junk
+				spam := msgs[0].Junk && (len(msgs) == 1 || msgs[1].Junk)
-				return xtrue, !ham, methodMsgfromFull, nil
+				return xtrue, spam, methodMsgfromFull, nil
 			}
 		}
 
@@ -213,16 +214,16 @@ func reputation(tx *bstore.Tx, log *mlog.Log, m *store.Message) (rjunk *bool, rc
 			q.FilterEqual("MsgFromValidated", m.MsgFromValidated)
 			msgs := xmessageList(q, descr)
 			if len(msgs) > 0 {
-				nham := 0
+				nonjunk := 0
 				for _, m := range msgs {
 					if !m.Junk {
-						nham++
+						nonjunk++
 					}
 				}
-				if 100*nham/len(msgs) > 80 {
+				if 100*nonjunk/len(msgs) > 80 {
 					return xfalse, true, method, nil
 				}
-				if nham == 0 {
+				if nonjunk == 0 {
 					// Only conclusive with at least 3 different localparts.
 					localparts := map[smtp.Localpart]struct{}{}
 					for _, m := range msgs {
@@ -244,8 +245,8 @@ func reputation(tx *bstore.Tx, log *mlog.Log, m *store.Message) (rjunk *bool, rc
 				q.FilterEqual("MsgFromValidated", true)
 				msgs = xmessageList(q, descr+"-validated")
 				if len(msgs) > 0 {
-					ham := !msgs[0].Junk || len(msgs) > 1 && !msgs[1].Junk
+					spam := msgs[0].Junk && (len(msgs) == 1 || msgs[1].Junk)
-					return xtrue, !ham, method, nil
+					return xtrue, spam, method, nil
 				}
 			}
 

smtpserver/reputation_test.go

@@ -100,8 +100,8 @@ func TestReputation(t *testing.T) {
 			DKIMDomains: dkimDomains,
 
 			Flags: store.Flags{
-				Junk: junk,
+				Junk:    junk,
-				Seen: true,
+				Notjunk: !junk,
 			},
 		}
 		return m

smtpserver/server.go

@@ -2150,11 +2150,12 @@ func (c *conn) deliver(ctx context.Context, recvHdrFor func(string) string, msgW
 				if err != nil {
 					log.Errorx("checking whether reject is already present", err)
 				} else if !present {
-					m.Flags.Seen = true // We don't want to draw attention.
+					m.Seen = true // We don't want to draw attention.
+					m.Junk = true // This is junk, also train as such.
 					m.MessageID = messageid
 					m.MessageHash = messagehash
 					acc.WithWLock(func() {
-						if hasSpace, err := acc.TidyRejectsMailbox(conf.RejectsMailbox); err != nil {
+						if hasSpace, err := acc.TidyRejectsMailbox(c.log, conf.RejectsMailbox); err != nil {
 							log.Errorx("tidying rejects mailbox", err)
 						} else if hasSpace {
 							if err := acc.DeliverMailbox(log, conf.RejectsMailbox, m, dataFile, false); err != nil {

smtpserver/server_test.go

@@ -322,7 +322,6 @@ func tretrain(t *testing.T, acc *store.Account) {
 
 	// Fetch messags to retrain on.
 	q := bstore.QueryDB[store.Message](acc.DB)
-	q.FilterEqual("Seen", true)
 	q.FilterFn(func(m store.Message) bool {
 		return m.Flags.Junk || m.Flags.Notjunk
 	})

store/account.go

@@ -276,8 +276,9 @@ type Message struct {
 
 	MessageHash []byte // Hash of message. For rejects delivery, so optional like MessageID.
 	Flags
-	Size      int64
+	Size        int64
-	MsgPrefix []byte // Typically holds received headers and/or header separator.
+	TrainedJunk *bool  // If nil, no training done yet. Otherwise, true is trained as junk, false trained as nonjunk.
+	MsgPrefix   []byte // Typically holds received headers and/or header separator.
 
 	// ParsedBuf message structure. Currently saved as JSON of message.Part because bstore
 	// cannot yet store recursive types. Created when first needed, and saved in the
@@ -299,6 +300,48 @@ func (m Message) LoadPart(r io.ReaderAt) (message.Part, error) {
 	return p, nil
 }
 
+// NeedsTraining returns whether message needs a training update, based on
+// TrainedJunk (current training status) and new Junk/Notjunk flags.
+func (m Message) NeedsTraining() bool {
+	untrain := m.TrainedJunk != nil
+	untrainJunk := untrain && *m.TrainedJunk
+	train := m.Junk || m.Notjunk && !(m.Junk && m.Notjunk)
+	trainJunk := m.Junk
+	return untrain != train || untrain && train && untrainJunk != trainJunk
+}
+
+// JunkFlagsForMailbox sets Junk and Notjunk flags based on mailbox name if configured. Often
+// used when delivering/moving/copying messages to a mailbox. Mail clients are not
+// very helpful with setting junk/notjunk flags. But clients can move/copy messages
+// to other mailboxes. So we set flags when clients move a message.
+func (m *Message) JunkFlagsForMailbox(mailbox string, conf config.Account) {
+	if !conf.AutomaticJunkFlags.Enabled {
+		return
+	}
+
+	lmailbox := strings.ToLower(mailbox)
+
+	if conf.JunkMailbox != nil && conf.JunkMailbox.MatchString(lmailbox) {
+		m.Junk = true
+		m.Notjunk = false
+	} else if conf.NeutralMailbox != nil && conf.NeutralMailbox.MatchString(lmailbox) {
+		m.Junk = false
+		m.Notjunk = false
+	} else if conf.NotJunkMailbox != nil && conf.NotJunkMailbox.MatchString(lmailbox) {
+		m.Junk = false
+		m.Notjunk = true
+	} else if conf.JunkMailbox == nil && conf.NeutralMailbox != nil && conf.NotJunkMailbox != nil {
+		m.Junk = true
+		m.Notjunk = false
+	} else if conf.JunkMailbox != nil && conf.NeutralMailbox == nil && conf.NotJunkMailbox != nil {
+		m.Junk = false
+		m.Notjunk = false
+	} else if conf.JunkMailbox != nil && conf.NeutralMailbox != nil && conf.NotJunkMailbox == nil {
+		m.Junk = false
+		m.Notjunk = true
+	}
+}
+
 // Recipient represents the recipient of a message. It is tracked to allow
 // first-time incoming replies from users this account has sent messages to. On
 // IMAP append to Sent, the message is parsed and recipients are inserted as
@@ -528,14 +571,10 @@ func (a *Account) WithRLock(fn func()) {
 // If sync is true, the message file and its directory are synced. Should be true
 // for regular mail delivery, but can be false when importing many messages.
 //
-// if train is true, the junkfilter (if configured) is trained with the message.
-// Should be used for regular mail delivery, but can be false when importing many
-// messages.
-//
 // Must be called with account rlock or wlock.
 //
 // Caller must broadcast new message.
-func (a *Account) DeliverX(log *mlog.Log, tx *bstore.Tx, m *Message, msgFile *os.File, consumeFile, isSent, sync, train bool) {
+func (a *Account) DeliverX(log *mlog.Log, tx *bstore.Tx, m *Message, msgFile *os.File, consumeFile, isSent, sync bool) {
 	mb := Mailbox{ID: m.MailboxID}
 	err := tx.Get(&mb)
 	xcheckf(err, "get mailbox")
@@ -544,6 +583,9 @@ func (a *Account) DeliverX(log *mlog.Log, tx *bstore.Tx, m *Message, msgFile *os
 	err = tx.Update(&mb)
 	xcheckf(err, "updating mailbox nextuid")
 
+	conf, _ := a.Conf()
+	m.JunkFlagsForMailbox(mb.Name, conf)
+
 	var part *message.Part
 	if m.ParsedBuf == nil {
 		mr := FileMsgReader(m.MsgPrefix, msgFile) // We don't close, it would close the msgFile.
@@ -629,13 +671,10 @@ func (a *Account) DeliverX(log *mlog.Log, tx *bstore.Tx, m *Message, msgFile *os
 		xcheckf(err, "sync directory")
 	}
 
-	if train {
+	l := []Message{*m}
-		conf, _ := a.Conf()
+	err = a.RetrainMessages(log, tx, l, false)
-		if mb.Name != conf.RejectsMailbox {
+	xcheckf(err, "training junkfilter")
-			err := a.Train(log, []Message{*m})
+	*m = l[0]
-			xcheckf(err, "train junkfilter with new message")
-		}
-	}
 }
 
 // write contents of r to new file dst, for delivering a message.
@@ -969,7 +1008,7 @@ func (a *Account) DeliverMailbox(log *mlog.Log, mailbox string, m *Message, msgF
 		m.MailboxOrigID = mb.ID
 		changes = append(changes, chl...)
 
-		a.DeliverX(log, tx, m, msgFile, consumeFile, mb.Sent, true, true)
+		a.DeliverX(log, tx, m, msgFile, consumeFile, mb.Sent, true)
 		return nil
 	})
 	// todo: if rename succeeded but transaction failed, we should remove the file.
@@ -988,7 +1027,7 @@ func (a *Account) DeliverMailbox(log *mlog.Log, mailbox string, m *Message, msgF
 //
 // Caller most hold account wlock.
 // Changes are broadcasted.
-func (a *Account) TidyRejectsMailbox(rejectsMailbox string) (hasSpace bool, rerr error) {
+func (a *Account) TidyRejectsMailbox(log *mlog.Log, rejectsMailbox string) (hasSpace bool, rerr error) {
 	var changes []Change
 
 	err := extransact(a.DB, true, func(tx *bstore.Tx) error {
@@ -1007,7 +1046,7 @@ func (a *Account) TidyRejectsMailbox(rejectsMailbox string) (hasSpace bool, rerr
 		remove, err := qdel.List()
 		xcheckf(err, "listing old messages")
 
-		changes = a.xremoveMessages(tx, mb, remove)
+		changes = a.xremoveMessages(log, tx, mb, remove)
 
 		// We allow up to n messages.
 		qcount := bstore.QueryTx[Message](tx)
@@ -1027,7 +1066,7 @@ func (a *Account) TidyRejectsMailbox(rejectsMailbox string) (hasSpace bool, rerr
 	return hasSpace, err
 }
 
-func (a *Account) xremoveMessages(tx *bstore.Tx, mb *Mailbox, l []Message) []Change {
+func (a *Account) xremoveMessages(log *mlog.Log, tx *bstore.Tx, mb *Mailbox, l []Message) []Change {
 	if len(l) == 0 {
 		return nil
 	}
@@ -1048,8 +1087,18 @@ func (a *Account) xremoveMessages(tx *bstore.Tx, mb *Mailbox, l []Message) []Cha
 	// Actually remove the messages.
 	qdm := bstore.QueryTx[Message](tx)
 	qdm.FilterIDs(ids)
+	var deleted []Message
+	qdm.Gather(&deleted)
 	_, err = qdm.Delete()
-	xcheckf(err, "deleting from message recipient")
+	xcheckf(err, "deleting from messages")
+
+	// Mark as neutral and train so junk filter gets untrained with these (junk) messages.
+	for i := range deleted {
+		deleted[i].Junk = false
+		deleted[i].Notjunk = false
+	}
+	err = a.RetrainMessages(log, tx, deleted, true)
+	xcheckf(err, "training deleted messages")
 
 	changes := make([]Change, len(l))
 	for i, m := range l {
@@ -1077,7 +1126,7 @@ func (a *Account) RejectsRemove(log *mlog.Log, rejectsMailbox, messageID string)
 		remove, err := q.List()
 		xcheckf(err, "listing messages to remove")
 
-		changes = a.xremoveMessages(tx, mb, remove)
+		changes = a.xremoveMessages(log, tx, mb, remove)
 
 		return err
 	})

store/account_test.go

@@ -72,13 +72,13 @@ func TestMailbox(t *testing.T) {
 			tcheck(t, err, "sent mailbox")
 			msent.MailboxID = mbsent.ID
 			msent.MailboxOrigID = mbsent.ID
-			acc.DeliverX(xlog, tx, &msent, msgFile, false, true, true, true)
+			acc.DeliverX(xlog, tx, &msent, msgFile, false, true, true)
 
 			err = tx.Insert(&mbrejects)
 			tcheck(t, err, "insert rejects mailbox")
 			mreject.MailboxID = mbrejects.ID
 			mreject.MailboxOrigID = mbrejects.ID
-			acc.DeliverX(xlog, tx, &mreject, msgFile, false, false, true, true)
+			acc.DeliverX(xlog, tx, &mreject, msgFile, false, false, true)
 
 			return nil
 		})
@@ -86,25 +86,34 @@ func TestMailbox(t *testing.T) {
 
 		err = acc.Deliver(xlog, conf.Destinations["mjl"], &mconsumed, msgFile, true)
 		tcheck(t, err, "deliver with consume")
+
+		err = acc.DB.Write(func(tx *bstore.Tx) error {
+			m.Junk = true
+			l := []Message{m}
+			err = acc.RetrainMessages(log, tx, l, false)
+			tcheck(t, err, "train as junk")
+			m = l[0]
+			return nil
+		})
+		tcheck(t, err, "train messages")
 	})
 
-	m.Junk = true
-	err = acc.Train(log, []Message{m})
-	tcheck(t, err, "train as junk")
-
-	flags := m.Flags
-
-	m.Seen = true
 	m.Junk = false
+	m.Notjunk = true
 	jf, _, err := acc.OpenJunkFilter(log)
 	tcheck(t, err, "open junk filter")
-	err = acc.Retrain(log, jf, flags, m)
+	err = acc.DB.Write(func(tx *bstore.Tx) error {
-	tcheck(t, err, "retrain as non-junk")
+		return acc.RetrainMessage(log, tx, jf, &m, false)
+	})
+	tcheck(t, err, "retraining as non-junk")
 	err = jf.Close()
 	tcheck(t, err, "close junk filter")
 
-	err = acc.Untrain(log, []Message{m})
+	m.Notjunk = false
-	tcheck(t, err, "untrain non-junk")
+	err = acc.DB.Write(func(tx *bstore.Tx) error {
+		return acc.RetrainMessages(log, tx, []Message{m}, false)
+	})
+	tcheck(t, err, "untraining non-junk")
 
 	err = acc.SetPassword("testtest")
 	tcheck(t, err, "set password")
@@ -171,7 +180,7 @@ func TestMailbox(t *testing.T) {
 		tcheck(t, err, "write tx")
 
 		// todo: check that messages are removed and changes sent.
-		hasSpace, err := acc.TidyRejectsMailbox("Rejects")
+		hasSpace, err := acc.TidyRejectsMailbox(log, "Rejects")
 		tcheck(t, err, "tidy rejects mailbox")
 		if !hasSpace {
 			t.Fatalf("no space for more rejects")

store/train.go

@@ -5,6 +5,8 @@ import (
 	"os"
 	"path/filepath"
 
+	"github.com/mjl-/bstore"
+
 	"github.com/mjl-/mox/config"
 	"github.com/mjl-/mox/junk"
 	"github.com/mjl-/mox/mlog"
@@ -40,28 +42,20 @@ func (a *Account) OpenJunkFilter(log *mlog.Log) (*junk.Filter, *config.JunkFilte
 	return f, jf, err
 }
 
-// Train new messages, if relevant given their flags.
+// RetrainMessages (un)trains messages, if relevant given their flags. Updates
-func (a *Account) Train(log *mlog.Log, msgs []Message) error {
+// m.TrainedJunk after retraining.
-	return a.xtrain(log, msgs, false, true)
+func (a *Account) RetrainMessages(log *mlog.Log, tx *bstore.Tx, msgs []Message, absentOK bool) (rerr error) {
-}
-
-// Untrain removed messages, if relevant given their flags.
-func (a *Account) Untrain(log *mlog.Log, msgs []Message) error {
-	return a.xtrain(log, msgs, true, false)
-}
-
-// train or untrain messages, if relevant given their flags.
-func (a *Account) xtrain(log *mlog.Log, msgs []Message, untrain, train bool) (rerr error) {
 	if len(msgs) == 0 {
 		return nil
 	}
 
 	var jf *junk.Filter
 
-	for _, m := range msgs {
+	for i := range msgs {
-		if !m.Seen && !m.Junk {
+		if !msgs[i].NeedsTraining() {
 			continue
 		}
+
 		// Lazy open the junk filter.
 		if jf == nil {
 			var err error
@@ -79,33 +73,28 @@ func (a *Account) xtrain(log *mlog.Log, msgs []Message, untrain, train bool) (re
 				}
 			}()
 		}
-		ham := !m.Junk
+		if err := a.RetrainMessage(log, tx, jf, &msgs[i], absentOK); err != nil {
-		err := xtrainMessage(log, a, jf, m, untrain, ham, train, ham)
-		if err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
-// Retrain message, if relevant given old flags and the new flags in m.
+// RetrainMessage untrains and/or trains a message, if relevant given m.TrainedJunk
-func (a *Account) Retrain(log *mlog.Log, jf *junk.Filter, old Flags, m Message) error {
+// and m.Junk/m.Notjunk. Updates m.TrainedJunk after retraining.
-	untrain := old.Seen || old.Junk
+func (a *Account) RetrainMessage(log *mlog.Log, tx *bstore.Tx, jf *junk.Filter, m *Message, absentOK bool) error {
-	train := m.Seen || m.Junk
+	untrain := m.TrainedJunk != nil
-	untrainHam := !old.Junk
+	untrainJunk := untrain && *m.TrainedJunk
-	trainHam := !m.Junk
+	train := m.Junk || m.Notjunk && !(m.Junk && m.Notjunk)
+	trainJunk := m.Junk
 
-	if !untrain && !train || (untrain && train && trainHam == untrainHam) {
+	if !untrain && !train || (untrain && train && untrainJunk == trainJunk) {
 		return nil
 	}
 
-	return xtrainMessage(log, a, jf, m, untrain, untrainHam, train, trainHam)
+	log.Info("updating junk filter", mlog.Field("untrain", untrain), mlog.Field("untrainJunk", untrainJunk), mlog.Field("train", train), mlog.Field("trainJunk", trainJunk))
-}
 
-func xtrainMessage(log *mlog.Log, a *Account, jf *junk.Filter, m Message, untrain, untrainHam, train, trainHam bool) error {
+	mr := a.MessageReader(*m)
-	log.Info("updating junk filter", mlog.Field("untrain", untrain), mlog.Field("untrainHam", untrainHam), mlog.Field("train", train), mlog.Field("trainHam", trainHam))
-
-	mr := a.MessageReader(m)
 	defer mr.Close()
 
 	p, err := m.LoadPart(mr)
@@ -121,16 +110,46 @@ func xtrainMessage(log *mlog.Log, a *Account, jf *junk.Filter, m Message, untrai
 	}
 
 	if untrain {
-		err := jf.Untrain(untrainHam, words)
+		err := jf.Untrain(!untrainJunk, words)
 		if err != nil {
 			return err
 		}
+		m.TrainedJunk = nil
 	}
 	if train {
-		err := jf.Train(trainHam, words)
+		err := jf.Train(!trainJunk, words)
 		if err != nil {
 			return err
 		}
+		m.TrainedJunk = &trainJunk
+	}
+	if err := tx.Update(m); err != nil && (!absentOK || err != bstore.ErrAbsent) {
+		return err
 	}
 	return nil
 }
+
+// TrainMessage trains the junk filter based on the current m.Junk/m.Notjunk flags,
+// disregarding m.TrainedJunk and not updating that field.
+func (a *Account) TrainMessage(log *mlog.Log, jf *junk.Filter, m Message) (bool, error) {
+	if !m.Junk && !m.Notjunk || (m.Junk && m.Notjunk) {
+		return false, nil
+	}
+
+	mr := a.MessageReader(m)
+	defer mr.Close()
+
+	p, err := m.LoadPart(mr)
+	if err != nil {
+		log.Errorx("loading part for message", err)
+		return false, nil
+	}
+
+	words, err := jf.ParseMessage(p)
+	if err != nil {
+		log.Errorx("parsing message for updating junk filter", err, mlog.Field("parse", ""))
+		return false, nil
+	}
+
+	return true, jf.Train(m.Notjunk, words)
+}

vendor/github.com/mjl-/bstore/doc.go

@@ -1,5 +1,5 @@
 /*
-Package bstore is a database library for storing and quering Go struct data.
+Package bstore is a database library for storing and querying Go struct data.
 
 Bstore is designed as a small, pure Go library that still provides most of
 the common data consistency requirements for modest database use cases. Bstore
@@ -102,7 +102,7 @@ track of a data/schema version.
 
 As a special case, you can switch field types between pointer and non-pointer
 types. With one exception: changing from pointer to non-pointer where the type
-has a field that must be nonzer is not allowed. The on-disk encoding will not be
+has a field that must be nonzero is not allowed. The on-disk encoding will not be
 changed, and nil pointers will turn into zero values, and zero values into nil
 pointers. Also see section Limitations about pointer types.
 

vendor/github.com/mjl-/bstore/export.go

@@ -262,7 +262,11 @@ func (ft fieldType) parseValue(p *parser) any {
 		// We don't have the type available, so we just return the binary data.
 		return p.TakeBytes(false)
 	case kindBool:
-		return true
+		if !ft.Ptr {
+			return true
+		}
+		buf := p.Take(1)
+		return buf[0] != 0
 	case kindInt8:
 		return int8(p.Varint())
 	case kindInt16:

vendor/github.com/mjl-/bstore/pack.go

@@ -180,8 +180,15 @@ func (ft fieldType) pack(p *packer, rv reflect.Value) {
 		}
 		p.AddBytes(buf)
 	case kindBool:
-		// No value needed. If false, it would be zero, handled above,
+		if ft.Ptr {
-		// with a 0 in the fieldmap.
+			var b byte = 0
+			if rv.Bool() {
+				b = 1
+			}
+			p.Write([]byte{b})
+		}
+		// If not pointer, no value is needed. If false, we would not get here, there would
+		// be a 0 in the fieldmap.
 	case kindInt:
 		v := rv.Int()
 		if v < math.MinInt32 || v > math.MaxInt32 {

vendor/github.com/mjl-/bstore/parse.go

@@ -199,7 +199,12 @@ func (ft fieldType) parse(p *parser, rv reflect.Value) {
 			rv.Set(v.Elem())
 		}
 	case kindBool:
-		rv.SetBool(true)
+		if ft.Ptr {
+			buf := p.Take(1)
+			rv.SetBool(buf[0] != 0)
+		} else {
+			rv.SetBool(true)
+		}
 	case kindInt:
 		v := p.Varint()
 		if v < math.MinInt32 || v > math.MaxInt32 {
@@ -283,6 +288,9 @@ func (ft fieldType) skip(p *parser) {
 	case kindBytes, kindBinaryMarshal, kindString:
 		p.TakeBytes(false)
 	case kindBool:
+		if ft.Ptr {
+			p.Take(1)
+		}
 	case kindInt8, kindInt16, kindInt32, kindInt, kindInt64:
 		p.Varint()
 	case kindUint8, kindUint16, kindUint32, kindUint, kindUint64, kindFloat32, kindFloat64:

vendor/modules.txt

@@ -11,7 +11,7 @@ github.com/golang/protobuf/ptypes/timestamp
 # github.com/matttproud/golang_protobuf_extensions v1.0.1
 ## explicit
 github.com/matttproud/golang_protobuf_extensions/pbutil
-# github.com/mjl-/bstore v0.0.0-20230114150735-9d9c0a2dcc79
+# github.com/mjl-/bstore v0.0.0-20230211204415-a9899ef6e782
 ## explicit; go 1.19
 github.com/mjl-/bstore
 # github.com/mjl-/sconf v0.0.4