#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 <noreply@anthropic.com>
This commit is contained in:
1 parent
707e6ae80e
commit
0fcdd15b58
6 files changed
+218
-6
No files matched your search
@@ -61,4 +61,6 @@ type WebmailRepository interface {
|
|||||||
GetMessageBlob(ctx context.Context, mailboxID, folderName string, uid uint32) (uuid.UUID, error)
|
GetMessageBlob(ctx context.Context, mailboxID, folderName string, uid uint32) (uuid.UUID, error)
|
||||||
// MarkSeen records that the user opened the message.
|
// MarkSeen records that the user opened the message.
|
||||||
MarkSeen(ctx context.Context, mailboxID, folderName string, uid uint32, seen bool) error
|
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
|
||||||
}
|
}
|
||||||
@@ -1,8 +1,13 @@
|
|||||||
package usecase
|
package usecase
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"context"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"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
|
// 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)
|
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: "<p>writ</p>",
|
||||||
|
}, 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: "<p>written</p>",
|
||||||
|
}, 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)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -125,9 +125,9 @@ func (uc *WebmailUseCase) SetSeen(ctx context.Context, address, folder string, u
|
|||||||
|
|
||||||
// ComposeInput is one message written in the webmail.
|
// ComposeInput is one message written in the webmail.
|
||||||
type ComposeInput struct {
|
type ComposeInput struct {
|
||||||
From string
|
From string
|
||||||
To []string
|
To []string
|
||||||
Cc []string
|
Cc []string
|
||||||
// Bcc reaches the envelope but never the headers - that is the whole
|
// 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
|
// 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.
|
// 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
|
// 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
|
// message. Drafts are whole RFC 5322 messages like any other, so they are
|
||||||
// readable over IMAP too.
|
// 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)
|
account, err := uc.auth.FindByAddress(ctx, input.From)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return 0, err
|
return 0, err
|
||||||
@@ -261,6 +265,18 @@ func (uc *WebmailUseCase) SaveDraft(ctx context.Context, input ComposeInput) (ui
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return 0, err
|
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 {
|
if len(uids) == 0 {
|
||||||
return 0, nil
|
return 0, nil
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -200,3 +200,63 @@ func (r *WebmailRepository) MarkSeen(
|
|||||||
|
|
||||||
return tx.Commit(ctx)
|
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)
|
||||||
|
}
|
||||||
@@ -4,6 +4,7 @@ import (
|
|||||||
"bytes"
|
"bytes"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"errors"
|
"errors"
|
||||||
|
"io"
|
||||||
"mime"
|
"mime"
|
||||||
"net/http"
|
"net/http"
|
||||||
"strconv"
|
"strconv"
|
||||||
@@ -264,12 +265,13 @@ func (m *mailAPI) handleSaveDraft(w http.ResponseWriter, r *http.Request) {
|
|||||||
if !ok {
|
if !ok {
|
||||||
return
|
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 {
|
if !ok {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
uid, err := m.useCase.SaveDraft(r.Context(), input)
|
uid, err := m.useCase.SaveDraft(r.Context(), input, replaceUID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
m.fail(w, err)
|
m.fail(w, err)
|
||||||
return
|
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")
|
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
|
||||||
|
}
|
||||||
@@ -54,6 +54,9 @@ export default function ComposePage({
|
|||||||
const [undoSeconds, setUndoSeconds] = useState<number | null>(null);
|
const [undoSeconds, setUndoSeconds] = useState<number | null>(null);
|
||||||
const [draftStatus, setDraftStatus] = useState<string>("");
|
const [draftStatus, setDraftStatus] = useState<string>("");
|
||||||
const [sendError, setSendError] = useState<string | null>(null);
|
const [sendError, setSendError] = useState<string | null>(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<number>(0);
|
||||||
|
|
||||||
const editorRef = useRef<HTMLDivElement>(null);
|
const editorRef = useRef<HTMLDivElement>(null);
|
||||||
|
|
||||||
@@ -81,8 +84,13 @@ export default function ComposePage({
|
|||||||
bcc: splitAddresses(bcc),
|
bcc: splitAddresses(bcc),
|
||||||
subject,
|
subject,
|
||||||
html,
|
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"));
|
setDraftStatus(res.ok ? t("mail.draftSaved") : t("mail.draftSaveFailed"));
|
||||||
} catch {
|
} catch {
|
||||||
setDraftStatus(t("mail.draftSaveFailed"));
|
setDraftStatus(t("mail.draftSaveFailed"));
|
||||||
|
|||||||
Reference in new issue
Block a user