From e360f9bffa2ecd52b4b56b4219d29598951fa016 Mon Sep 17 00:00:00 2001 From: thibaud-leclere Date: Fri, 10 Apr 2026 11:17:36 +0200 Subject: [PATCH] fix: tighten imap service message contract --- internal/imapclient/service.go | 4 +- internal/imapclient/service_test.go | 173 ++++++++++++++++++++++++++-- internal/imapclient/types.go | 18 +-- 3 files changed, 177 insertions(+), 18 deletions(-) diff --git a/internal/imapclient/service.go b/internal/imapclient/service.go index 0c8edac..7a1a856 100644 --- a/internal/imapclient/service.go +++ b/internal/imapclient/service.go @@ -22,6 +22,6 @@ func (s Service) ListMessages(ctx context.Context, cred secretstore.Credential, return s.backend.ListMessages(ctx, cred, mailbox, limit) } -func (s Service) GetMessage(ctx context.Context, cred secretstore.Credential, mailbox string, id string) (Message, error) { - return s.backend.GetMessage(ctx, cred, mailbox, id) +func (s Service) GetMessage(ctx context.Context, cred secretstore.Credential, mailbox string, uid uint32) (Message, error) { + return s.backend.GetMessage(ctx, cred, mailbox, uid) } diff --git a/internal/imapclient/service_test.go b/internal/imapclient/service_test.go index 21f4ac0..b3a12cc 100644 --- a/internal/imapclient/service_test.go +++ b/internal/imapclient/service_test.go @@ -2,27 +2,47 @@ package imapclient import ( "context" + "errors" "testing" "email-mcp/internal/secretstore" ) -type backendStub struct{} - -func (backendStub) ListMailboxes(context.Context, secretstore.Credential) ([]Mailbox, error) { - return []Mailbox{{Name: "INBOX"}}, nil +type backendStub struct { + listMailboxes func(context.Context, secretstore.Credential) ([]Mailbox, error) + listMessages func(context.Context, secretstore.Credential, string, int) ([]MessageSummary, error) + getMessage func(context.Context, secretstore.Credential, string, uint32) (Message, error) } -func (backendStub) ListMessages(context.Context, secretstore.Credential, string, int) ([]MessageSummary, error) { - return nil, nil +func (b backendStub) ListMailboxes(ctx context.Context, cred secretstore.Credential) ([]Mailbox, error) { + return b.listMailboxes(ctx, cred) } -func (backendStub) GetMessage(context.Context, secretstore.Credential, string, string) (Message, error) { - return Message{}, nil +func (b backendStub) ListMessages(ctx context.Context, cred secretstore.Credential, mailbox string, limit int) ([]MessageSummary, error) { + return b.listMessages(ctx, cred, mailbox, limit) +} + +func (b backendStub) GetMessage(ctx context.Context, cred secretstore.Credential, mailbox string, uid uint32) (Message, error) { + return b.getMessage(ctx, cred, mailbox, uid) } func TestServiceListMailboxesUsesBackend(t *testing.T) { - svc := NewService(backendStub{}) + svc := NewService(backendStub{ + listMailboxes: func(_ context.Context, cred secretstore.Credential) ([]Mailbox, error) { + if cred.Host != "imap.example.com" || cred.Username != "alice" || cred.Password != "secret" { + t.Fatalf("unexpected credential: %#v", cred) + } + return []Mailbox{{Name: "INBOX"}}, nil + }, + listMessages: func(context.Context, secretstore.Credential, string, int) ([]MessageSummary, error) { + t.Fatal("ListMessages should not be called") + return nil, nil + }, + getMessage: func(context.Context, secretstore.Credential, string, uint32) (Message, error) { + t.Fatal("GetMessage should not be called") + return Message{}, nil + }, + }) boxes, err := svc.ListMailboxes(context.Background(), secretstore.Credential{ Host: "imap.example.com", @@ -36,3 +56,138 @@ func TestServiceListMailboxesUsesBackend(t *testing.T) { t.Fatalf("unexpected mailboxes: %#v", boxes) } } + +func TestServiceListMessagesUsesBackend(t *testing.T) { + svc := NewService(backendStub{ + listMailboxes: func(context.Context, secretstore.Credential) ([]Mailbox, error) { + t.Fatal("ListMailboxes should not be called") + return nil, nil + }, + listMessages: func(_ context.Context, cred secretstore.Credential, mailbox string, limit int) ([]MessageSummary, error) { + if cred.Host != "imap.example.com" || mailbox != "INBOX" || limit != 10 { + t.Fatalf("unexpected call: cred=%#v mailbox=%q limit=%d", cred, mailbox, limit) + } + return []MessageSummary{{ + UID: 42, + Subject: "hello", + From: "alice@example.com", + }}, nil + }, + getMessage: func(context.Context, secretstore.Credential, string, uint32) (Message, error) { + t.Fatal("GetMessage should not be called") + return Message{}, nil + }, + }) + + messages, err := svc.ListMessages(context.Background(), secretstore.Credential{ + Host: "imap.example.com", + Username: "alice", + Password: "secret", + }, "INBOX", 10) + if err != nil { + t.Fatalf("ListMessages returned error: %v", err) + } + if len(messages) != 1 || messages[0].UID != 42 { + t.Fatalf("unexpected messages: %#v", messages) + } +} + +func TestServiceGetMessageUsesBackend(t *testing.T) { + svc := NewService(backendStub{ + listMailboxes: func(context.Context, secretstore.Credential) ([]Mailbox, error) { + t.Fatal("ListMailboxes should not be called") + return nil, nil + }, + listMessages: func(context.Context, secretstore.Credential, string, int) ([]MessageSummary, error) { + t.Fatal("ListMessages should not be called") + return nil, nil + }, + getMessage: func(_ context.Context, cred secretstore.Credential, mailbox string, uid uint32) (Message, error) { + if cred.Host != "imap.example.com" || mailbox != "INBOX" || uid != 42 { + t.Fatalf("unexpected call: cred=%#v mailbox=%q uid=%d", cred, mailbox, uid) + } + return Message{ + UID: 42, + Mailbox: "INBOX", + Headers: []Header{ + {Name: "Received", Value: "first"}, + {Name: "Received", Value: "second"}, + {Name: "Subject", Value: "hello"}, + }, + Body: "body", + }, nil + }, + }) + + message, err := svc.GetMessage(context.Background(), secretstore.Credential{ + Host: "imap.example.com", + Username: "alice", + Password: "secret", + }, "INBOX", 42) + if err != nil { + t.Fatalf("GetMessage returned error: %v", err) + } + if message.UID != 42 { + t.Fatalf("unexpected message UID: %#v", message) + } + if len(message.Headers) != 3 { + t.Fatalf("unexpected headers: %#v", message.Headers) + } + if message.Headers[0].Name != "Received" || message.Headers[0].Value != "first" { + t.Fatalf("unexpected first header: %#v", message.Headers[0]) + } + if message.Headers[1].Name != "Received" || message.Headers[1].Value != "second" { + t.Fatalf("unexpected second header: %#v", message.Headers[1]) + } +} + +func TestServicePropagatesBackendErrors(t *testing.T) { + wantErr := errors.New("backend failed") + + tests := []struct { + name string + run func(Service) error + }{ + { + name: "ListMailboxes", + run: func(svc Service) error { + _, err := svc.ListMailboxes(context.Background(), secretstore.Credential{}) + return err + }, + }, + { + name: "ListMessages", + run: func(svc Service) error { + _, err := svc.ListMessages(context.Background(), secretstore.Credential{}, "INBOX", 10) + return err + }, + }, + { + name: "GetMessage", + run: func(svc Service) error { + _, err := svc.GetMessage(context.Background(), secretstore.Credential{}, "INBOX", 42) + return err + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + svc := NewService(backendStub{ + listMailboxes: func(context.Context, secretstore.Credential) ([]Mailbox, error) { + return nil, wantErr + }, + listMessages: func(context.Context, secretstore.Credential, string, int) ([]MessageSummary, error) { + return nil, wantErr + }, + getMessage: func(context.Context, secretstore.Credential, string, uint32) (Message, error) { + return Message{}, wantErr + }, + }) + + if err := tc.run(svc); !errors.Is(err, wantErr) { + t.Fatalf("expected error %v, got %v", wantErr, err) + } + }) + } +} diff --git a/internal/imapclient/types.go b/internal/imapclient/types.go index 7184a4b..85d563f 100644 --- a/internal/imapclient/types.go +++ b/internal/imapclient/types.go @@ -11,21 +11,25 @@ type Mailbox struct { } type MessageSummary struct { - ID string `json:"id"` + UID uint32 `json:"uid"` Subject string `json:"subject"` From string `json:"from"` - UID uint32 `json:"uid"` +} + +type Header struct { + Name string `json:"name"` + Value string `json:"value"` } type Message struct { - ID string `json:"id"` - Mailbox string `json:"mailbox"` - Headers map[string]string `json:"headers"` - Body string `json:"body"` + UID uint32 `json:"uid"` + Mailbox string `json:"mailbox"` + Headers []Header `json:"headers"` + Body string `json:"body"` } type Backend interface { ListMailboxes(context.Context, secretstore.Credential) ([]Mailbox, error) ListMessages(context.Context, secretstore.Credential, string, int) ([]MessageSummary, error) - GetMessage(context.Context, secretstore.Credential, string, string) (Message, error) + GetMessage(context.Context, secretstore.Credential, string, uint32) (Message, error) }