From cdffb15004b5ffd2ecf8dde2d3e93ce0f1f6fae6 Mon Sep 17 00:00:00 2001 From: Steve Cliff Date: Tue, 23 Jun 2026 20:16:15 +0100 Subject: [PATCH] feat(store): add account from_address field + v2 migration Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/store/account.go | 39 ++++++++++++-------- internal/store/account_test.go | 27 ++++++++++++++ internal/store/schema.go | 5 +-- internal/store/store.go | 30 +++++++++++++--- internal/store/store_test.go | 65 ++++++++++++++++++++++++++++++++-- 5 files changed, 143 insertions(+), 23 deletions(-) diff --git a/internal/store/account.go b/internal/store/account.go index 14988e2..9f6bd1d 100644 --- a/internal/store/account.go +++ b/internal/store/account.go @@ -23,6 +23,7 @@ type Account struct { SMTPSecurity string // tls | starttls AuthType string // password | oauth2 Username string + FromAddress string // send-as identity; blank ⇒ fall back to Username Password string // decrypted; empty in ListAccounts WhitelistInEnabled bool WhitelistOutEnabled bool @@ -30,6 +31,15 @@ type Account struct { ProcessBacklog bool } +// SendFrom returns the From identity for outgoing mail, falling back to the +// login username when no explicit from-address is configured. +func (a Account) SendFrom() string { + if a.FromAddress != "" { + return a.FromAddress + } + return a.Username +} + func (s *Store) AddAccount(a Account) (int64, error) { var encPw []byte if a.Password != "" { @@ -42,12 +52,12 @@ func (s *Store) AddAccount(a Account) (int64, error) { res, err := s.db.Exec(` INSERT INTO accounts (name,mode,imap_host,imap_port,imap_security,smtp_host,smtp_port,smtp_security, - auth_type,username, + auth_type,username,from_address, enc_password,whitelist_in_enabled,whitelist_out_enabled,subject_regex,process_backlog) - VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)`, + VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)`, a.Name, a.Mode, a.IMAPHost, a.IMAPPort, a.IMAPSecurity, nullStr(a.SMTPHost), nullInt(a.SMTPPort), nullStr(a.SMTPSecurity), - a.AuthType, a.Username, + a.AuthType, a.Username, nullStr(a.FromAddress), encPw, b2i(a.WhitelistInEnabled), b2i(a.WhitelistOutEnabled), nullStr(a.SubjectRegex), b2i(a.ProcessBacklog)) if err != nil { @@ -59,7 +69,7 @@ func (s *Store) AddAccount(a Account) (int64, error) { func (s *Store) GetAccount(name string) (Account, error) { row := s.db.QueryRow(` SELECT id,name,mode,imap_host,imap_port,imap_security,smtp_host,smtp_port,smtp_security, - auth_type,username, + auth_type,username,from_address, enc_password,whitelist_in_enabled,whitelist_out_enabled,subject_regex,process_backlog FROM accounts WHERE name = ?`, name) a, encPw, err := scanAccount(row) @@ -82,7 +92,7 @@ func (s *Store) GetAccount(name string) (Account, error) { func (s *Store) ListAccounts() ([]Account, error) { rows, err := s.db.Query(` SELECT id,name,mode,imap_host,imap_port,imap_security,smtp_host,smtp_port,smtp_security, - auth_type,username, + auth_type,username,from_address, enc_password,whitelist_in_enabled,whitelist_out_enabled,subject_regex,process_backlog FROM accounts ORDER BY name`) if err != nil { @@ -108,12 +118,12 @@ func (s *Store) UpdateAccount(a Account) error { // Build the SET clause, conditionally including secret columns. set := `mode=?, imap_host=?, imap_port=?, imap_security=?, smtp_host=?, smtp_port=?, smtp_security=?, - auth_type=?, username=?, + auth_type=?, username=?, from_address=?, whitelist_in_enabled=?, whitelist_out_enabled=?, subject_regex=?, process_backlog=?` args := []any{ a.Mode, a.IMAPHost, a.IMAPPort, a.IMAPSecurity, nullStr(a.SMTPHost), nullInt(a.SMTPPort), nullStr(a.SMTPSecurity), - a.AuthType, a.Username, + a.AuthType, a.Username, nullStr(a.FromAddress), b2i(a.WhitelistInEnabled), b2i(a.WhitelistOutEnabled), nullStr(a.SubjectRegex), b2i(a.ProcessBacklog), } @@ -152,16 +162,16 @@ type scanner interface{ Scan(dest ...any) error } func scanAccount(sc scanner) (Account, []byte, error) { var ( - a Account - encPw []byte - subj, smtpHost, smtpSec sql.NullString - smtpPort sql.NullInt64 - wlIn, wlOut int - backlog int + a Account + encPw []byte + subj, smtpHost, smtpSec, fromAddr sql.NullString + smtpPort sql.NullInt64 + wlIn, wlOut int + backlog int ) err := sc.Scan(&a.ID, &a.Name, &a.Mode, &a.IMAPHost, &a.IMAPPort, &a.IMAPSecurity, &smtpHost, &smtpPort, &smtpSec, - &a.AuthType, &a.Username, &encPw, &wlIn, &wlOut, &subj, &backlog) + &a.AuthType, &a.Username, &fromAddr, &encPw, &wlIn, &wlOut, &subj, &backlog) if err != nil { return Account{}, nil, err } @@ -172,6 +182,7 @@ func scanAccount(sc scanner) (Account, []byte, error) { a.WhitelistOutEnabled = wlOut != 0 a.ProcessBacklog = backlog != 0 a.SubjectRegex = subj.String + a.FromAddress = fromAddr.String return a, encPw, nil } diff --git a/internal/store/account_test.go b/internal/store/account_test.go index 67e874b..befeb10 100644 --- a/internal/store/account_test.go +++ b/internal/store/account_test.go @@ -84,3 +84,30 @@ func TestListAccountsOmitsSecrets(t *testing.T) { t.Fatal("ListAccounts must not return secrets") } } + +func TestSendFromFallsBackToUsername(t *testing.T) { + a := Account{Username: "login@example.com"} + if got := a.SendFrom(); got != "login@example.com" { + t.Fatalf("blank from-address should fall back to username, got %q", got) + } + a.FromAddress = "Steve Cliff " + if got := a.SendFrom(); got != "Steve Cliff " { + t.Fatalf("set from-address should win, got %q", got) + } +} + +func TestAddGetAccountRoundTripsFromAddress(t *testing.T) { + s := openTemp(t) + a := sampleAccount() + a.FromAddress = "Steve Cliff " + if _, err := s.AddAccount(a); err != nil { + t.Fatalf("AddAccount: %v", err) + } + got, err := s.GetAccount("work") + if err != nil { + t.Fatalf("GetAccount: %v", err) + } + if got.FromAddress != "Steve Cliff " { + t.Fatalf("FromAddress not round-tripped: %q", got.FromAddress) + } +} diff --git a/internal/store/schema.go b/internal/store/schema.go index e0262e0..81cc87f 100644 --- a/internal/store/schema.go +++ b/internal/store/schema.go @@ -1,8 +1,8 @@ package store -const schemaVersion = 1 +const schemaVersion = 2 -// schemaSQL is the full v1 schema. All statements are idempotent via IF NOT EXISTS. +// schemaSQL is the full current schema. All statements are idempotent via IF NOT EXISTS. const schemaSQL = ` CREATE TABLE IF NOT EXISTS settings ( key TEXT PRIMARY KEY, @@ -21,6 +21,7 @@ CREATE TABLE IF NOT EXISTS accounts ( smtp_security TEXT, auth_type TEXT NOT NULL CHECK (auth_type IN ('password','oauth2')), username TEXT NOT NULL, + from_address TEXT, enc_password BLOB, enc_oauth_client_id BLOB, enc_oauth_client_secret BLOB, diff --git a/internal/store/store.go b/internal/store/store.go index 5fd37f5..4abb192 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -42,15 +42,35 @@ func Open(path string) (*Store, error) { return nil, fmt.Errorf("apply schema: %w", err) } s := &Store{db: db} - if _, err := s.GetSetting("schema_version"); err != nil { - if err := s.SetSetting("schema_version", strconv.Itoa(schemaVersion)); err != nil { - db.Close() - return nil, err - } + if err := s.migrate(); err != nil { + db.Close() + return nil, err } return s, nil } +// migrate brings an existing database up to the current schemaVersion. A brand- +// new database (no schema_version yet) already has every column from schemaSQL, +// so it is simply stamped at the current version. Each older version runs its +// forward step. The version gate makes every step idempotent across reopens. +func (s *Store) migrate() error { + v, err := s.GetSetting("schema_version") + if err != nil { + // Fresh database: schemaSQL created all columns already. + return s.SetSetting("schema_version", strconv.Itoa(schemaVersion)) + } + ver, _ := strconv.Atoi(v) + if ver < 2 { + if _, err := s.db.Exec(`ALTER TABLE accounts ADD COLUMN from_address TEXT`); err != nil { + return fmt.Errorf("migrate to v2: %w", err) + } + if err := s.SetSetting("schema_version", "2"); err != nil { + return err + } + } + return nil +} + func (s *Store) Close() error { return s.db.Close() } // DefaultDBPath resolves EMCLI_DB or the per-OS default location. diff --git a/internal/store/store_test.go b/internal/store/store_test.go index 5479f24..28e32c0 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -1,6 +1,7 @@ package store import ( + "database/sql" "path/filepath" "testing" ) @@ -28,7 +29,7 @@ func TestOpenCreatesSchemaAndIsIdempotent(t *testing.T) { t.Fatalf("first Open: %v", err) } v, err := s.GetSetting("schema_version") - if err != nil || v != "1" { + if err != nil || v != "2" { t.Fatalf("schema_version: %q err=%v", v, err) } s.Close() @@ -39,7 +40,7 @@ func TestOpenCreatesSchemaAndIsIdempotent(t *testing.T) { t.Fatalf("second Open: %v", err) } defer s2.Close() - if v, _ := s2.GetSetting("schema_version"); v != "1" { + if v, _ := s2.GetSetting("schema_version"); v != "2" { t.Fatalf("schema_version after reopen: %q", v) } } @@ -104,3 +105,63 @@ func TestForeignKeyCascade(t *testing.T) { t.Fatalf("whitelist_in row not cascade-deleted: count=%d err=%v", count, err) } } + +func TestOpenMigratesV1AddsFromAddress(t *testing.T) { + p := filepath.Join(t.TempDir(), "emcli.db") + + // Hand-build a v1 database: accounts table WITHOUT from_address, a settings + // table pinned at schema_version=1, and one pre-existing account row. + raw, err := sql.Open("sqlite", p) + if err != nil { + t.Fatalf("sql.Open: %v", err) + } + const v1Schema = ` +CREATE TABLE settings (key TEXT PRIMARY KEY, value TEXT NOT NULL); +CREATE TABLE accounts ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + name TEXT UNIQUE NOT NULL, + mode TEXT NOT NULL, + imap_host TEXT NOT NULL, + imap_port INTEGER NOT NULL, + imap_security TEXT NOT NULL, + smtp_host TEXT, smtp_port INTEGER, smtp_security TEXT, + auth_type TEXT NOT NULL, + username TEXT NOT NULL, + enc_password BLOB, + enc_oauth_client_id BLOB, enc_oauth_client_secret BLOB, enc_oauth_refresh_token BLOB, + whitelist_in_enabled INTEGER NOT NULL DEFAULT 0, + whitelist_out_enabled INTEGER NOT NULL DEFAULT 0, + subject_regex TEXT, + process_backlog INTEGER NOT NULL DEFAULT 0 +); +INSERT INTO settings(key,value) VALUES ('schema_version','1'); +INSERT INTO accounts(name,mode,imap_host,imap_port,imap_security,auth_type,username) + VALUES ('legacy','RO','imap.example.com',993,'tls','password','login@example.com'); +` + if _, err := raw.Exec(v1Schema); err != nil { + t.Fatalf("seed v1 schema: %v", err) + } + raw.Close() + + // Open via the store: the migration must add from_address and bump to v2. + s, err := Open(p) + if err != nil { + t.Fatalf("Open (migrate): %v", err) + } + defer s.Close() + + if v, _ := s.GetSetting("schema_version"); v != "2" { + t.Fatalf("schema_version after migrate: %q, want 2", v) + } + // ListAccounts SELECTs from_address; it would error if the column were missing. + accs, err := s.ListAccounts() + if err != nil { + t.Fatalf("ListAccounts after migrate: %v", err) + } + if len(accs) != 1 || accs[0].FromAddress != "" { + t.Fatalf("legacy account wrong after migrate: %+v", accs) + } + if got := accs[0].SendFrom(); got != "login@example.com" { + t.Fatalf("legacy account should send from username, got %q", got) + } +}