diff --git a/internal/imap/imap.go b/internal/imap/imap.go index fa27d22..cd1465e 100644 --- a/internal/imap/imap.go +++ b/internal/imap/imap.go @@ -135,6 +135,21 @@ func (c *Client) Since(uid uint32, limit int) ([]Message, uint32, error) { } uids := found.AllUIDs() + + // Anything at or below the marker is dropped here rather than trusted to the + // range in the query. A server that reads `1272:*` as `0:1272` — and one does, + // which is how the same mail became a task every ten minutes with the marker + // standing still at 1271 — hands back the whole mailbox. The marker is ours; + // enforcing it here costs one pass over a list of at most a few hundred + // numbers and does not depend on anybody else's reading of the spec. + kept := make([]imap.UID, 0, len(uids)) + for _, u := range uids { + if uint32(u) > uid { + kept = append(kept, u) + } + } + uids = kept + if len(uids) == 0 { return nil, uid, nil } diff --git a/internal/imap/roundtrip_test.go b/internal/imap/roundtrip_test.go index d96046a..5eebe48 100644 --- a/internal/imap/roundtrip_test.go +++ b/internal/imap/roundtrip_test.go @@ -262,3 +262,57 @@ func TestAMultipartMailReadsAsItsText(t *testing.T) { t.Errorf("the snippet does not start with the text of the mail: %q", got) } } + +// The marker is ours, not the server's. +// +// A mailbox read with a marker above everything in it must come back empty. The +// query says so, but a server is free to read the range differently — one reads +// `1272:*` as `0:1272` and hands back the lot, which is how the same mail became +// a task every ten minutes while the marker stood still at 1271. The filter is +// on this side now, so the invariant holds whatever comes back. +func TestNothingAtOrBelowTheMarkerComesBack(t *testing.T) { + addr, pool := serveMailbox(t, []mail{ + {subject: "En", from: "a@example.dk", body: "en", flagged: true}, + {subject: "To", from: "b@example.dk", body: "to", flagged: true}, + {subject: "Tre", from: "c@example.dk", body: "tre", flagged: true}, + }) + client, err := Dial(Account{Host: addr, Username: "kw", Password: "hemmelig", RootCAs: pool}) + if err != nil { + t.Fatal(err) + } + defer client.Close() + + all, highest, err := client.Since(0, 25) + if err != nil { + t.Fatal(err) + } + if len(all) != 3 { + t.Fatalf("got %d messages on the first read, want 3", len(all)) + } + + // Read again from the marker the first run would have written. + rest, again, err := client.Since(highest, 25) + if err != nil { + t.Fatal(err) + } + if len(rest) != 0 { + t.Errorf("%d message(s) came back at or below the marker: %v", len(rest), rest[0].Subject) + } + // And the marker must not go backwards, or the next run reads them all again. + if again < highest { + t.Errorf("the marker moved back from %d to %d", highest, again) + } + + // Every message, individually: none of them may reappear once passed. + for _, m := range all { + later, _, err := client.Since(m.UID, 25) + if err != nil { + t.Fatal(err) + } + for _, l := range later { + if l.UID <= m.UID { + t.Errorf("reading from %d gave %d back", m.UID, l.UID) + } + } + } +} diff --git a/internal/store/savemailbox_test.go b/internal/store/savemailbox_test.go new file mode 100644 index 0000000..2447f8d --- /dev/null +++ b/internal/store/savemailbox_test.go @@ -0,0 +1,40 @@ +package store + +import ( + "context" + "testing" +) + +// Saving a mailbox that is already there must update it, not fail. +// +// The row is written again on every token refresh and every time Gmail's seen +// list changes. If that write errors, the tasks a sync made are kept and the note +// of having made them is not — so the same mail becomes a task again on the next +// sweep, once every ten minutes, forever. +func TestSavingAnExistingMailboxUpdatesIt(t *testing.T) { + db, userID := sealedStore(t) + ctx := context.Background() + + m := &Mailbox{UserID: userID, Kind: "gmail", Username: "kw@nolimit.dk", + RefreshToken: "r", Seen: []string{"a"}} + if err := db.SaveMailbox(ctx, m); err != nil { + t.Fatal(err) + } + + m.Seen = []string{"a", "b"} + m.AccessToken = "fresh" + if err := db.SaveMailbox(ctx, m); err != nil { + t.Fatalf("saving it a second time failed: %v", err) + } + + back, err := db.MailboxOfKind(ctx, userID, "gmail") + if err != nil { + t.Fatal(err) + } + if len(back.Seen) != 2 { + t.Errorf("the seen list came back as %v", back.Seen) + } + if back.AccessToken != "fresh" { + t.Errorf("the token came back as %q", back.AccessToken) + } +}