When retraining ham/spam messages, don't make existence of the messages optional.

If messages that should exist don't, that's a real error we don't want to hide.
Part of larger changes.
This commit is contained in:
Mechiel Lukkien 2025-03-01 10:48:36 +01:00
parent 3b731b7afe
commit 5ba51adb14
No known key found for this signature in database
7 changed files with 39 additions and 33 deletions

View File

@ -283,7 +283,7 @@ func (c *conn) cmdxReplace(isUID bool, tag, cmd string, p *parser) {
// Undo any junk filter training for the old message.
om.Junk = false
om.Notjunk = false
err = c.account.RetrainMessages(context.TODO(), c.log, tx, []store.Message{om}, false)
err = c.account.RetrainMessages(context.TODO(), c.log, tx, []store.Message{om})
xcheckf(err, "untraining expunged messages")
// Mark old message expunged.

View File

@ -3842,7 +3842,7 @@ func (c *conn) xexpunge(uidSet *numSet, missingMailboxOK bool) (removed []store.
removed[i].Junk = false
removed[i].Notjunk = false
}
err = c.account.RetrainMessages(context.TODO(), c.log, tx, removed, true)
err = c.account.RetrainMessages(context.TODO(), c.log, tx, removed)
xcheckf(err, "untraining expunged messages")
})
@ -4210,7 +4210,7 @@ func (c *conn) cmdxCopy(isUID bool, tag, cmd string, p *parser) {
xcheckf(err, "sync directory")
}
err = c.account.RetrainMessages(context.TODO(), c.log, tx, nmsgs, false)
err = c.account.RetrainMessages(context.TODO(), c.log, tx, nmsgs)
xcheckf(err, "train copied messages")
})
@ -4358,7 +4358,7 @@ func (c *conn) cmdxMove(isUID bool, tag, cmd string, p *parser) {
err = tx.Update(&mbDst)
xcheckf(err, "updating destination mailbox for uids, keywords and counts")
err = c.account.RetrainMessages(context.TODO(), c.log, tx, msgs, false)
err = c.account.RetrainMessages(context.TODO(), c.log, tx, msgs)
xcheckf(err, "retraining messages after move")
// Prepare broadcast changes to other connections.
@ -4578,7 +4578,7 @@ func (c *conn) cmdxStore(isUID bool, tag, cmd string, p *parser) {
changes = append(changes, mb.ChangeKeywords())
}
err = c.account.RetrainMessages(context.TODO(), c.log, tx, updated, false)
err = c.account.RetrainMessages(context.TODO(), c.log, tx, updated)
xcheckf(err, "training messages")
})

View File

@ -654,11 +654,17 @@ func (m Message) LoadPart(r io.ReaderAt) (message.Part, error) {
// 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
trainJunk := m.Junk
return untrain != train || untrain && train && untrainJunk != trainJunk
needs, _, _, _, _ := m.needsTraining()
return needs
}
func (m Message) needsTraining() (needs, untrain, untrainJunk, train, trainJunk bool) {
untrain = m.TrainedJunk != nil
untrainJunk = untrain && *m.TrainedJunk
train = m.Junk != m.Notjunk
trainJunk = m.Junk
needs = untrain != train || untrain && train && untrainJunk != trainJunk
return
}
// JunkFlagsForMailbox sets Junk and Notjunk flags based on mailbox name if configured. Often
@ -1850,7 +1856,7 @@ func (a *Account) DeliverMessage(log mlog.Log, tx *bstore.Tx, m *Message, msgFil
if !notrain && m.NeedsTraining() {
l := []Message{*m}
if err := a.RetrainMessages(context.TODO(), log, tx, l, false); err != nil {
if err := a.RetrainMessages(context.TODO(), log, tx, l); err != nil {
return fmt.Errorf("training junkfilter: %w", err)
}
*m = l[0]
@ -2422,7 +2428,7 @@ func (a *Account) rejectsRemoveMessages(ctx context.Context, log mlog.Log, tx *b
expunged[i].Junk = false
expunged[i].Notjunk = false
}
if err := a.RetrainMessages(ctx, log, tx, expunged, true); err != nil {
if err := a.RetrainMessages(ctx, log, tx, expunged); err != nil {
return nil, fmt.Errorf("retraining expunged messages: %w", err)
}
@ -3134,7 +3140,7 @@ func (a *Account) MailboxDelete(ctx context.Context, log mlog.Log, tx *bstore.Tx
}
}
remove = remove[:n]
if err := a.RetrainMessages(ctx, log, tx, remove, true); err != nil {
if err := a.RetrainMessages(ctx, log, tx, remove); err != nil {
return nil, nil, false, fmt.Errorf("untraining deleted messages: %v", err)
}
}

View File

@ -127,7 +127,7 @@ func TestMailbox(t *testing.T) {
err = acc.DB.Write(ctxbg, func(tx *bstore.Tx) error {
m.Junk = true
l := []Message{m}
err = acc.RetrainMessages(ctxbg, log, tx, l, false)
err = acc.RetrainMessages(ctxbg, log, tx, l)
tcheck(t, err, "train as junk")
m = l[0]
return nil
@ -140,7 +140,7 @@ func TestMailbox(t *testing.T) {
jf, _, err := acc.OpenJunkFilter(ctxbg, log)
tcheck(t, err, "open junk filter")
err = acc.DB.Write(ctxbg, func(tx *bstore.Tx) error {
return acc.RetrainMessage(ctxbg, log, tx, jf, &m, false)
return acc.RetrainMessage(ctxbg, log, tx, jf, &m)
})
tcheck(t, err, "retraining as non-junk")
err = jf.Close()
@ -148,7 +148,7 @@ func TestMailbox(t *testing.T) {
m.Notjunk = false
err = acc.DB.Write(ctxbg, func(tx *bstore.Tx) error {
return acc.RetrainMessages(ctxbg, log, tx, []Message{m}, false)
return acc.RetrainMessages(ctxbg, log, tx, []Message{m})
})
tcheck(t, err, "untraining non-junk")

View File

@ -19,6 +19,11 @@ import (
// ErrNoJunkFilter indicates user did not configure/enable a junk filter.
var ErrNoJunkFilter = errors.New("junkfilter: not configured")
func (a *Account) HasJunkFilter() bool {
conf, _ := a.Conf()
return conf.JunkFilter != nil
}
// OpenJunkFilter returns an opened junk filter for the account.
// If the account does not have a junk filter enabled, ErrNotConfigured is returned.
// Do not forget to save the filter after modifying, and to always close the filter when done.
@ -47,7 +52,7 @@ func (a *Account) OpenJunkFilter(ctx context.Context, log mlog.Log) (*junk.Filte
// RetrainMessages (un)trains messages, if relevant given their flags. Updates
// m.TrainedJunk after retraining.
func (a *Account) RetrainMessages(ctx context.Context, log mlog.Log, tx *bstore.Tx, msgs []Message, absentOK bool) (rerr error) {
func (a *Account) RetrainMessages(ctx context.Context, log mlog.Log, tx *bstore.Tx, msgs []Message) (rerr error) {
if len(msgs) == 0 {
return nil
}
@ -78,7 +83,7 @@ func (a *Account) RetrainMessages(ctx context.Context, log mlog.Log, tx *bstore.
}
}()
}
if err := a.RetrainMessage(ctx, log, tx, jf, &msgs[i], absentOK); err != nil {
if err := a.RetrainMessage(ctx, log, tx, jf, &msgs[i]); err != nil {
return err
}
}
@ -87,16 +92,11 @@ func (a *Account) RetrainMessages(ctx context.Context, log mlog.Log, tx *bstore.
// RetrainMessage untrains and/or trains a message, if relevant given m.TrainedJunk
// and m.Junk/m.Notjunk. Updates m.TrainedJunk after retraining.
func (a *Account) RetrainMessage(ctx context.Context, log mlog.Log, tx *bstore.Tx, jf *junk.Filter, m *Message, absentOK bool) error {
untrain := m.TrainedJunk != nil
untrainJunk := untrain && *m.TrainedJunk
train := m.Junk != m.Notjunk
trainJunk := m.Junk
if !untrain && !train || (untrain && train && untrainJunk == trainJunk) {
func (a *Account) RetrainMessage(ctx context.Context, log mlog.Log, tx *bstore.Tx, jf *junk.Filter, m *Message) error {
need, untrain, untrainJunk, train, trainJunk := m.needsTraining()
if !need {
return nil
}
log.Debug("updating junk filter",
slog.Bool("untrain", untrain),
slog.Bool("untrainjunk", untrainJunk),
@ -135,7 +135,7 @@ func (a *Account) RetrainMessage(ctx context.Context, log mlog.Log, tx *bstore.T
}
m.TrainedJunk = &trainJunk
}
if err := tx.Update(m); err != nil && (!absentOK || err != bstore.ErrAbsent) {
if err := tx.Update(m); err != nil {
return err
}
return nil

View File

@ -1063,7 +1063,7 @@ func (w Webmail) MessageSubmit(ctx context.Context, m SubmitMessage) {
err = tx.Update(&rmb)
xcheckf(ctx, err, "update modseqo of mailbox of replied/forwarded message")
err = acc.RetrainMessages(ctx, log, tx, []store.Message{rm}, false)
err = acc.RetrainMessages(ctx, log, tx, []store.Message{rm})
xcheckf(ctx, err, "retraining messages after reply/forward")
}
@ -1337,7 +1337,7 @@ func (Webmail) MailboxEmpty(ctx context.Context, mailboxID int64) {
err = acc.AddMessageSize(log, tx, -totalSize)
xcheckf(ctx, err, "updating disk usage")
err = acc.RetrainMessages(ctx, log, tx, expunged, true)
err = acc.RetrainMessages(ctx, log, tx, expunged)
xcheckf(ctx, err, "retraining expunged messages")
chremove := store.ChangeRemoveUIDs{MailboxID: mb.ID, UIDs: uids, ModSeq: modseq}

View File

@ -139,7 +139,7 @@ func (x XOps) MessageDeleteTx(ctx context.Context, log mlog.Log, tx *bstore.Tx,
remove[i].Junk = false
remove[i].Notjunk = false
}
err = acc.RetrainMessages(ctx, log, tx, remove, true)
err = acc.RetrainMessages(ctx, log, tx, remove)
x.Checkf(ctx, err, "untraining deleted messages")
for _, ch := range removeChanges {
@ -220,7 +220,7 @@ func (x XOps) MessageFlagsAdd(ctx context.Context, log mlog.Log, acc *store.Acco
}
}
err = acc.RetrainMessages(ctx, log, tx, retrain, false)
err = acc.RetrainMessages(ctx, log, tx, retrain)
x.Checkf(ctx, err, "retraining messages")
})
@ -291,7 +291,7 @@ func (x XOps) MessageFlagsClear(ctx context.Context, log mlog.Log, acc *store.Ac
// note: cannot remove keywords from mailbox by removing keywords from message.
}
err = acc.RetrainMessages(ctx, log, tx, retrain, false)
err = acc.RetrainMessages(ctx, log, tx, retrain)
x.Checkf(ctx, err, "retraining messages")
})
@ -487,7 +487,7 @@ func (x XOps) MessageMoveTx(ctx context.Context, log mlog.Log, acc *store.Accoun
err = tx.Update(&mbDst)
x.Checkf(ctx, err, "updating destination mailbox with uidnext and modseq")
err = acc.RetrainMessages(ctx, log, tx, retrain, false)
err = acc.RetrainMessages(ctx, log, tx, retrain)
x.Checkf(ctx, err, "retraining messages after move")
// Ensure UIDs of the removed message are in increasing order. It is quite common