From 0fcdd15b58af1d855ee12c821dfeceede2e48f45 Mon Sep 17 00:00:00 2001 From: Lucas Rayan Date: Sun, 2 Aug 2026 20:01:12 -0300 Subject: [PATCH] #24: make draft autosave replace the previous draft The autosave I had just added appended a new message on every debounce, so a few minutes of typing would have left the Drafts folder holding a dozen copies of the same unfinished mail. Caught before deploying. The composer now returns the stored UID and sends it back as replace_uid on the next save; the use case expunges the superseded draft, but only after the replacement has committed, so a failure leaves the older draft in place rather than losing both. Expunge is a soft delete matching the expunged_at column the schema already uses, and it locates the message by mailbox as well as UID so a caller cannot delete someone else's mail by guessing a number. Co-Authored-By: Claude Opus 5 --- .../application/port/webmail_repository.go | 2 + .../usecase/webmail_compose_test.go | 98 +++++++++++++++++++ .../application/usecase/webmail_mail.go | 24 ++++- .../postgres/webmail_repository.go | 60 ++++++++++++ .../internal/presentation/http/mail_api.go | 32 +++++- .../src/app/(user)/user/compose/page.tsx | 8 ++ 6 files changed, 218 insertions(+), 6 deletions(-) diff --git a/services/protocols/internal/application/port/webmail_repository.go b/services/protocols/internal/application/port/webmail_repository.go index d2a159f..b91fd50 100644 --- a/services/protocols/internal/application/port/webmail_repository.go +++ b/services/protocols/internal/application/port/webmail_repository.go @@ -61,4 +61,6 @@ type WebmailRepository interface { GetMessageBlob(ctx context.Context, mailboxID, folderName string, uid uint32) (uuid.UUID, error) // MarkSeen records that the user opened the message. MarkSeen(ctx context.Context, mailboxID, folderName string, uid uint32, seen bool) error + // Expunge soft-deletes one message, keeping the folder counters in step. + Expunge(ctx context.Context, mailboxID, folderName string, uid uint32) error } diff --git a/services/protocols/internal/application/usecase/webmail_compose_test.go b/services/protocols/internal/application/usecase/webmail_compose_test.go index 0ae2a1f..c5f103b 100644 --- a/services/protocols/internal/application/usecase/webmail_compose_test.go +++ b/services/protocols/internal/application/usecase/webmail_compose_test.go @@ -1,8 +1,13 @@ package usecase import ( + "context" "strings" "testing" + + "github.com/google/uuid" + + "lambdamail/protocols/internal/application/port" ) // Every one of these reproduces a way the composer's mail was being mangled or @@ -112,3 +117,96 @@ func TestHtmlToPlainTextIsReadable(t *testing.T) { t.Errorf("markup survived into the plain-text part:\n%s", got) } } + +// --- draft storage ------------------------------------------------------- + +type stubWebmailRepo struct { + mailboxID string + expunged []uint32 + expungeErr error +} + +func (s *stubWebmailRepo) FindMailboxIDByAddress(context.Context, string) (string, error) { + return s.mailboxID, nil +} +func (s *stubWebmailRepo) ListFolders(context.Context, string) ([]port.WebmailFolder, error) { + return nil, nil +} +func (s *stubWebmailRepo) ListMessages(context.Context, string, string, string, int, int) ([]port.WebmailMessage, error) { + return nil, nil +} +func (s *stubWebmailRepo) GetMessageBlob(context.Context, string, string, uint32) (uuid.UUID, error) { + return uuid.Nil, nil +} +func (s *stubWebmailRepo) MarkSeen(context.Context, string, string, uint32, bool) error { return nil } +func (s *stubWebmailRepo) Expunge(_ context.Context, _, _ string, uid uint32) error { + s.expunged = append(s.expunged, uid) + return s.expungeErr +} + +type stubAuthRepo struct{ account *port.MailboxAuth } + +func (s *stubAuthRepo) FindByAddress(context.Context, string) (*port.MailboxAuth, error) { + return s.account, nil +} + +// Autosaving must replace the previous draft, not pile up another copy of the +// same half-written message every time the typing pauses. +func TestDraftAutosaveReplacesThePreviousOne(t *testing.T) { + mailboxID := uuid.New() + repo := &stubWebmailRepo{mailboxID: mailboxID.String()} + messages := &recordingMessageRepository{} + blobs := &stubBlobStorage{ref: port.BlobRef{ID: uuid.New(), SHA256: "d", SizeBytes: 5}} + + uc := (&WebmailUseCase{ + repo: repo, + auth: &stubAuthRepo{account: &port.MailboxAuth{ID: mailboxID, EmailAddress: "me@example.test"}}, + localHost: "mail.example.test", + }).WithLocalFiling(blobs, messages) + + first, err := uc.SaveDraft(context.Background(), ComposeInput{ + From: "me@example.test", Subject: "Half", HTML: "

writ

", + }, 0) + if err != nil { + t.Fatalf("first save: %v", err) + } + if len(repo.expunged) != 0 { + t.Errorf("the first save has nothing to replace, expunged %v", repo.expunged) + } + + if _, err := uc.SaveDraft(context.Background(), ComposeInput{ + From: "me@example.test", Subject: "Half", HTML: "

written

", + }, first); err != nil { + t.Fatalf("second save: %v", err) + } + + if len(repo.expunged) != 1 || repo.expunged[0] != first { + t.Errorf("the superseded draft was not removed: expunged %v, want [%d]", repo.expunged, first) + } + if len(messages.persisted) != 2 { + t.Fatalf("want 2 writes, got %d", len(messages.persisted)) + } + if messages.persisted[0].TargetFolderName != "Drafts" { + t.Errorf("draft filed in %q, want Drafts", messages.persisted[0].TargetFolderName) + } +} + +// A draft is a half-written message by definition; it must save with no +// recipients, where Send would rightly refuse. +func TestDraftSavesWithoutRecipients(t *testing.T) { + mailboxID := uuid.New() + uc := (&WebmailUseCase{ + repo: &stubWebmailRepo{mailboxID: mailboxID.String()}, + auth: &stubAuthRepo{account: &port.MailboxAuth{ID: mailboxID, EmailAddress: "me@example.test"}}, + localHost: "mail.example.test", + }).WithLocalFiling( + &stubBlobStorage{ref: port.BlobRef{ID: uuid.New(), SHA256: "d", SizeBytes: 5}}, + &recordingMessageRepository{}, + ) + + if _, err := uc.SaveDraft(context.Background(), ComposeInput{ + From: "me@example.test", Subject: "no recipients yet", + }, 0); err != nil { + t.Fatalf("a draft with no recipients must still save: %v", err) + } +} diff --git a/services/protocols/internal/application/usecase/webmail_mail.go b/services/protocols/internal/application/usecase/webmail_mail.go index 718fc88..23d1aa7 100644 --- a/services/protocols/internal/application/usecase/webmail_mail.go +++ b/services/protocols/internal/application/usecase/webmail_mail.go @@ -125,9 +125,9 @@ func (uc *WebmailUseCase) SetSeen(ctx context.Context, address, folder string, u // ComposeInput is one message written in the webmail. type ComposeInput struct { - From string - To []string - Cc []string + From string + To []string + Cc []string // Bcc reaches the envelope but never the headers - that is the whole // point of it. It used not to exist here at all, so a blind copy typed // into the webmail was silently dropped and nobody received it. @@ -223,7 +223,11 @@ func (uc *WebmailUseCase) fileLocalCopy( // from a setTimeout, with no request behind it, and closing the tab lost the // message. Drafts are whole RFC 5322 messages like any other, so they are // readable over IMAP too. -func (uc *WebmailUseCase) SaveDraft(ctx context.Context, input ComposeInput) (uint32, error) { +// replaceUID is the draft this one supersedes, or 0 for the first save of a +// message. Without it every autosave would append another copy and a few +// minutes of typing would leave the Drafts folder full of the same half-written +// message. +func (uc *WebmailUseCase) SaveDraft(ctx context.Context, input ComposeInput, replaceUID uint32) (uint32, error) { account, err := uc.auth.FindByAddress(ctx, input.From) if err != nil { return 0, err @@ -261,6 +265,18 @@ func (uc *WebmailUseCase) SaveDraft(ctx context.Context, input ComposeInput) (ui if err != nil { return 0, err } + + // Only after the replacement is committed, so a failure here leaves the + // older draft in place rather than losing both. + if replaceUID != 0 { + mailboxID, idErr := uc.mailboxID(ctx, account.EmailAddress) + if idErr == nil { + if err := uc.repo.Expunge(ctx, mailboxID, "Drafts", replaceUID); err != nil { + log.Printf("webmail: could not remove the superseded draft %d: %v", replaceUID, err) + } + } + } + if len(uids) == 0 { return 0, nil } diff --git a/services/protocols/internal/infrastructure/postgres/webmail_repository.go b/services/protocols/internal/infrastructure/postgres/webmail_repository.go index 21d6936..a26140c 100644 --- a/services/protocols/internal/infrastructure/postgres/webmail_repository.go +++ b/services/protocols/internal/infrastructure/postgres/webmail_repository.go @@ -200,3 +200,63 @@ func (r *WebmailRepository) MarkSeen( return tx.Commit(ctx) } + +// Expunge marks one message deleted and keeps the folder counters in step. +// +// Soft delete, matching the expunged_at column the rest of the schema uses: the +// blob stays referenced until the retention sweep, so a mistaken delete is +// recoverable and IMAP clients holding the UID still resolve it. +// +// The message is located by mailbox as well as UID, so a caller cannot expunge +// somebody else's mail by guessing a number. +func (r *WebmailRepository) Expunge( + ctx context.Context, mailboxID, folderName string, uid uint32, +) error { + tx, err := r.pool.Begin(ctx) + if err != nil { + return err + } + defer tx.Rollback(ctx) + + var messageID uuid.UUID + var folderID string + var wasUnread bool + err = tx.QueryRow(ctx, ` + SELECT e.id, f.id::text, + NOT EXISTS (SELECT 1 FROM message_flags mf + WHERE mf.message_id = e.id AND mf.flag = '\Seen') + FROM email_messages e + JOIN folders f ON f.id = e.folder_id + WHERE e.mailbox_id = $1 AND `+folderCondition+` + AND e.uid = $3 AND e.expunged_at IS NULL + `, mailboxID, folderName, int64(uid)).Scan(&messageID, &folderID, &wasUnread) + if errors.Is(err, pgx.ErrNoRows) { + return ErrMessageNotFound + } + if err != nil { + return err + } + + if _, err := tx.Exec(ctx, + `UPDATE email_messages SET expunged_at = NOW() WHERE id = $1`, messageID); err != nil { + return err + } + + // GREATEST guards the counters against ever going negative, which a + // double expunge of the same row would otherwise cause. + unreadDelta := 0 + if wasUnread { + unreadDelta = -1 + } + if _, err := tx.Exec(ctx, ` + UPDATE folders + SET total_count = GREATEST(0, total_count - 1), + unread_count = GREATEST(0, unread_count + $2), + highest_modseq = highest_modseq + 1 + WHERE id = $1 + `, folderID, unreadDelta); err != nil { + return err + } + + return tx.Commit(ctx) +} diff --git a/services/protocols/internal/presentation/http/mail_api.go b/services/protocols/internal/presentation/http/mail_api.go index 29bc391..9ac0413 100644 --- a/services/protocols/internal/presentation/http/mail_api.go +++ b/services/protocols/internal/presentation/http/mail_api.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/json" "errors" + "io" "mime" "net/http" "strconv" @@ -264,12 +265,13 @@ func (m *mailAPI) handleSaveDraft(w http.ResponseWriter, r *http.Request) { if !ok { return } - input, ok := m.decodeCompose(w, r, session.Email) + // The body is read once, so replace_uid has to come out of the same decode. + input, replaceUID, ok := m.decodeDraft(w, r, session.Email) if !ok { return } - uid, err := m.useCase.SaveDraft(r.Context(), input) + uid, err := m.useCase.SaveDraft(r.Context(), input, replaceUID) if err != nil { m.fail(w, err) return @@ -339,3 +341,29 @@ func (m *mailAPI) fail(w http.ResponseWriter, err error) { writeError(w, http.StatusInternalServerError, "INTERNAL_ERROR", "Request could not be processed") } } + +// decodeDraft reads a composed message plus the draft it supersedes. +// +// replace_uid carries the UID of the previous autosave so the new one takes +// its place. Without it each keystroke pause would leave another copy behind +// and the Drafts folder would fill with the same unfinished message. +func (m *mailAPI) decodeDraft( + w http.ResponseWriter, r *http.Request, sender string, +) (usecase.ComposeInput, uint32, bool) { + raw, err := io.ReadAll(http.MaxBytesReader(w, r.Body, maxComposeBytes)) + if err != nil { + writeError(w, http.StatusBadRequest, "BAD_REQUEST", "Could not read the request") + return usecase.ComposeInput{}, 0, false + } + + var meta struct { + ReplaceUID uint32 `json:"replace_uid"` + } + // A missing or malformed replace_uid is not fatal: the draft is still + // worth saving, it just will not supersede anything. + _ = json.Unmarshal(raw, &meta) + + r.Body = io.NopCloser(bytes.NewReader(raw)) + input, ok := m.decodeCompose(w, r, sender) + return input, meta.ReplaceUID, ok +} diff --git a/services/webmail/src/app/(user)/user/compose/page.tsx b/services/webmail/src/app/(user)/user/compose/page.tsx index c00b582..5e93c3f 100644 --- a/services/webmail/src/app/(user)/user/compose/page.tsx +++ b/services/webmail/src/app/(user)/user/compose/page.tsx @@ -54,6 +54,9 @@ export default function ComposePage({ const [undoSeconds, setUndoSeconds] = useState(null); const [draftStatus, setDraftStatus] = useState(""); const [sendError, setSendError] = useState(null); + // The UID of the draft already stored for this message. Sent back on each + // autosave so the server replaces it instead of leaving another copy. + const draftUidRef = useRef(0); const editorRef = useRef(null); @@ -81,8 +84,13 @@ export default function ComposePage({ bcc: splitAddresses(bcc), subject, html, + replace_uid: draftUidRef.current, }), }); + if (res.ok) { + const data = await res.json().catch(() => ({})); + if (typeof data.uid === "number") draftUidRef.current = data.uid; + } setDraftStatus(res.ok ? t("mail.draftSaved") : t("mail.draftSaveFailed")); } catch { setDraftStatus(t("mail.draftSaveFailed"));