From fc10031d225369cc191cc80d102e5a1479a5e185 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 19 Oct 2026 14:36:53 +0200 Subject: [PATCH] mesh-mail: mail_search orders by date, sender, subject or size and groups by sender, so the newest, the largest or who writes most can be answered (issue 1230) Query's From becomes a list, OR'd (1228/AC11's several domains), since 1230's order-by needs it to test the ascending-order criterion end to end; To and Cc stay single, no criterion asking for more. --- modules/mesh-mail/cmd/mesh-mail/pgstore.go | 111 +++++++-- .../mesh-mail/cmd/mesh-mail/search_test.go | 107 ++++++++- modules/mesh-mail/cmd/mesh-mail/store.go | 221 +++++++++++++++--- 3 files changed, 386 insertions(+), 53 deletions(-) diff --git a/modules/mesh-mail/cmd/mesh-mail/pgstore.go b/modules/mesh-mail/cmd/mesh-mail/pgstore.go index f81fda04..7e8e8139 100644 --- a/modules/mesh-mail/cmd/mesh-mail/pgstore.go +++ b/modules/mesh-mail/cmd/mesh-mail/pgstore.go @@ -395,14 +395,27 @@ func (s *PgStore) Get(ctx context.Context, account, folder string, uid uint32) ( return m, err == nil, err } -// addressFilterSQL is 1228/AC2, 1228/AC3, 1228/AC4, 1228/AC6(cc): an EXISTS over the jsonb array column (sender, -// recipients or copied), matched against one already-lowercased parameter either as a whole address or, with no -// "@", as a domain (every address under it). column names a fixed set the Go code chooses, never caller input. +// addressFilterSQL is 1228/AC2, 1228/AC4, 1228/AC6(cc): an EXISTS over the jsonb array column (recipients or +// copied), matched against one already-lowercased parameter either as a whole address or, with no "@", as a +// domain (every address under it). column names a fixed set the Go code chooses, never caller input. func addressFilterSQL(column, param string) string { return fmt.Sprintf("exists (select 1 from jsonb_array_elements(%s) el where lower(el->>'address') = %s or "+ "lower(el->>'address') like '%%@' || %s)", column, param, param) } +// addressFilterSQLAny is 1228/AC3; 1230/AC11 ("several domains"): an EXISTS over column (sender), matched +// against any of filters, OR'd (each already-lowercased, each either a whole address or, with no "@", a domain). +// The whole list reaches SQL as one parameter, a text[]; never concatenated. +func addressFilterSQLAny(column string, filters []string, param func(any) string) string { + lowered := make([]string, len(filters)) + for i, f := range filters { + lowered[i] = strings.ToLower(f) + } + p := param(lowered) + return fmt.Sprintf("exists (select 1 from jsonb_array_elements(%s) el, unnest(%s::text[]) f where "+ + "lower(el->>'address') = f or lower(el->>'address') like '%%@' || f)", column, p) +} + // wordFilterSQL is 1229/AC5, AC6(words), AC9, AC12, AC13, AC15: one condition per word of the list, each a // whole-word, case-insensitive match (Postgres's \y, its own word boundary) against column, so a hyphenated word // such as "4711-882" is never matched by only its digits (AC12); joined with "and" ("all", the default) or "or" @@ -451,14 +464,10 @@ func globToLike(pattern string) string { return b.String() } -func (s *PgStore) Search(ctx context.Context, q Query) ([]Message, error) { - limit := q.Limit - if limit <= 0 { - limit = 20 - } - var args []any - // param appends one value and answers its placeholder; every value from the query reaches SQL this way, never - // concatenated (only fixed column names and SQL text are built as strings). +// searchWhere is every filter of Query (everything but its order and its limit), shared by Search and +// GroupBySender so the two can never drift apart; every value from the query reaches SQL as one of args' +// parameters, never concatenated (only fixed column names and SQL text are built as strings). +func searchWhere(q Query) (whereSQL string, args []any) { param := func(v any) string { args = append(args, v) return fmt.Sprintf("$%d", len(args)) @@ -470,8 +479,8 @@ func (s *PgStore) Search(ctx context.Context, q Query) ([]Message, error) { if q.Text != "" { where = append(where, "search @@ plainto_tsquery('simple'::regconfig, "+param(q.Text)+")") } - if q.From != "" { - where = append(where, addressFilterSQL("sender", param(strings.ToLower(q.From)))) + if len(q.From) > 0 { + where = append(where, addressFilterSQLAny("sender", q.From, param)) } if q.To != "" { where = append(where, addressFilterSQL("recipients", param(strings.ToLower(q.To)))) @@ -507,8 +516,44 @@ func (s *PgStore) Search(ctx context.Context, q Query) ([]Message, error) { if q.Unread != nil { where = append(where, fmt.Sprintf("(%s = any(flags)) = %s", param(`\Seen`), param(!*q.Unread))) } - sqlText := "select " + messageColumns + " from message where " + strings.Join(where, " and ") + - " order by date desc limit " + param(limit) + return strings.Join(where, " and "), args +} + +// orderColumns is 1230's fixed Go map from an order-by name to its (safe, never caller-built) SQL expression; +// unknown or empty answers "date" — the tool refuses an unknown order-by before the store is ever asked (AC16). +var orderColumns = map[string]string{ + "date": "date", + "sender": "coalesce(sender->0->>'address', '')", + "subject": "subject", + "size": "size", +} + +func orderExpr(orderBy string) string { + if e, ok := orderColumns[orderBy]; ok { + return e + } + return orderColumns["date"] +} + +func (s *PgStore) Search(ctx context.Context, q Query) ([]Message, error) { + limit := q.Limit + if limit <= 0 { + limit = 20 + } + whereSQL, args := searchWhere(q) + param := func(v any) string { + args = append(args, v) + return fmt.Sprintf("$%d", len(args)) + } + // 1230 (AC7, AC11, AC17's schema): order by the chosen expression, ascending unless desc; an empty OrderBy + // keeps the newest-first default (AC1) regardless of OrderDesc. Ties always break ascending on + // (account, folder, uid), so the order is stable. + dir := "asc" + if q.OrderBy == "" || q.OrderDesc { + dir = "desc" + } + sqlText := "select " + messageColumns + " from message where " + whereSQL + + " order by " + orderExpr(q.OrderBy) + " " + dir + ", account, folder, uid limit " + param(limit) rows, err := s.pool.Query(ctx, sqlText, args...) if err != nil { return nil, err @@ -525,6 +570,42 @@ func (s *PgStore) Search(ctx context.Context, q Query) ([]Message, error) { return out, rows.Err() } +// GroupBySender is 1230/AC10: one line per sender among the messages searchWhere's filters match, by count +// descending (address ascending to break ties), Limit bounding the number of lines. +func (s *PgStore) GroupBySender(ctx context.Context, q Query) ([]SenderGroup, error) { + limit := q.Limit + if limit <= 0 { + limit = 20 + } + whereSQL, args := searchWhere(q) + param := func(v any) string { + args = append(args, v) + return fmt.Sprintf("$%d", len(args)) + } + sqlText := `select el->>'address' as address, max(el->>'name') as name, count(*) as count, max(date) as newest + from message, jsonb_array_elements(sender) el where ` + whereSQL + ` group by el->>'address' + order by count desc, address asc limit ` + param(limit) + rows, err := s.pool.Query(ctx, sqlText, args...) + if err != nil { + return nil, err + } + defer rows.Close() + var out []SenderGroup + for rows.Next() { + var g SenderGroup + var name *string + if err := rows.Scan(&g.Address, &name, &g.Count, &g.Newest); err != nil { + return nil, err + } + if name != nil { + g.Name = *name + } + g.Newest = g.Newest.UTC() + out = append(out, g) + } + return out, rows.Err() +} + func (s *PgStore) Folders(ctx context.Context, account string) ([]Folder, error) { rows, err := s.pool.Query(ctx, `select name, uidvalidity, messages, synced from folder where account = $1 order by name`, account) if err != nil { diff --git a/modules/mesh-mail/cmd/mesh-mail/search_test.go b/modules/mesh-mail/cmd/mesh-mail/search_test.go index 331ff044..a76856c6 100644 --- a/modules/mesh-mail/cmd/mesh-mail/search_test.go +++ b/modules/mesh-mail/cmd/mesh-mail/search_test.go @@ -154,7 +154,7 @@ func TestSearchFiltersByFromAndOnOrAfter(t *testing.T) { eachStore(t, func(t *testing.T, st Store) { ctx := context.Background() addFixtures(t, ctx, st) - got, err := st.Search(ctx, Query{Accounts: []string{"mesh"}, From: "bob@example.org", + got, err := st.Search(ctx, Query{Accounts: []string{"mesh"}, From: []string{"bob@example.org"}, OnOrAfter: time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC), Limit: 10}) if err != nil { t.Fatal(err) @@ -227,7 +227,7 @@ func TestSearchFiltersByDomainAttachmentAndWord(t *testing.T) { eachStore(t, func(t *testing.T, st Store) { ctx := context.Background() addFixtures(t, ctx, st) - got, err := st.Search(ctx, Query{Accounts: []string{"mesh"}, From: "google.com", HasAttachment: true, + got, err := st.Search(ctx, Query{Accounts: []string{"mesh"}, From: []string{"google.com"}, HasAttachment: true, AttachmentNamePattern: "*.pdf", BodyWords: []string{"factuur"}, Limit: 10}) if err != nil { t.Fatal(err) @@ -271,7 +271,7 @@ func TestSearchFiltersByDomainAndSubjectWordsAcrossAccounts(t *testing.T) { eachStore(t, func(t *testing.T, st Store) { ctx := context.Background() addFixtures(t, ctx, st) - got, err := st.Search(ctx, Query{Accounts: []string{"mesh", "gmail"}, From: "google.com", + got, err := st.Search(ctx, Query{Accounts: []string{"mesh", "gmail"}, From: []string{"google.com"}, SubjectWords: []string{"security", "alert"}, Limit: 10}) if err != nil { t.Fatal(err) @@ -331,3 +331,104 @@ func TestSearchFiltersByUnreadFlag(t *testing.T) { sameSet(t, gotRead, "", "", "", "") }) } + +// sameOrder asserts got's message-ids equal want, in that exact order (unlike sameSet, which ignores order). +func sameOrder(t *testing.T, got []Message, want ...string) { + t.Helper() + g := make([]string, 0, len(got)) + for _, m := range got { + g = append(g, m.MessageID) + } + if len(g) != len(want) { + t.Fatalf("got %v, want %v", g, want) + } + for i := range g { + if g[i] != want[i] { + t.Fatalf("got %v, want %v", g, want) + } + } +} + +// 1230/AC7: a date range ("last month") ordered by size descending, limit bounding the answer. +func TestSearchOrdersBySizeDescending(t *testing.T) { + eachStore(t, func(t *testing.T, st Store) { + ctx := context.Background() + addFixtures(t, ctx, st) + got, err := st.Search(ctx, Query{Accounts: []string{"mesh"}, OnOrAfter: time.Date(2026, 9, 1, 0, 0, 0, 0, time.UTC), + Before: time.Date(2026, 10, 1, 0, 0, 0, 0, time.UTC), OrderBy: "size", OrderDesc: true, Limit: 5}) + if err != nil { + t.Fatal(err) + } + sameOrder(t, got, "", "") + }) +} + +// 1230/AC11: from several domains (OR'd) and on-or-after a date, order by date ascending, oldest first; limit +// bounds the answer. +func TestSearchFiltersBySeveralDomainsOrderedAscending(t *testing.T) { + eachStore(t, func(t *testing.T, st Store) { + ctx := context.Background() + addFixtures(t, ctx, st) + got, err := st.Search(ctx, Query{Accounts: []string{"mesh"}, From: []string{"example.org", "google.com"}, + OnOrAfter: searchNow.AddDate(0, 0, -90), OrderBy: "date", OrderDesc: false, Limit: 4}) + if err != nil { + t.Fatal(err) + } + // junk1 (example.org, Junk) and sec1 (google.com, INBOX) prove both domains and every folder are reached; + // the ascending order (oldest first) proves OrderDesc: false, not merely the absence of a filter. + sameOrder(t, got, "", "", "", "") + }) +} + +// 1230 (the order-by mechanism beyond date and size, not tied to one AC): sender and subject each sort, and the +// tie-break on (account, folder, uid) is stable when the chosen field is equal. +func TestSearchOrdersBySenderAndSubject(t *testing.T) { + eachStore(t, func(t *testing.T, st Store) { + ctx := context.Background() + addFixtures(t, ctx, st) + bySender, err := st.Search(ctx, Query{Accounts: []string{"mesh"}, Folder: "Sent", OrderBy: "sender", Limit: 10}) + if err != nil { + t.Fatal(err) + } + // Both Sent messages share the same sender (jschoubben@novox.be): the tie-break (account, folder, uid) + // orders them by uid ascending. + sameOrder(t, bySender, "", "") + + bySubject, err := st.Search(ctx, Query{Accounts: []string{"mesh"}, Folder: "Junk", OrderBy: "subject", Limit: 10}) + if err != nil { + t.Fatal(err) + } + sameOrder(t, bySubject, "", "") // "special offer..." < "you won..." + }) +} + +// 1230/AC10: group by sender, folder INBOX, by count descending (address ascending to break ties); limit bounds +// the number of lines, not the number of messages. +func TestGroupBySenderOrdersByCountDescending(t *testing.T) { + eachStore(t, func(t *testing.T, st Store) { + ctx := context.Background() + addFixtures(t, ctx, st) + got, err := st.GroupBySender(ctx, Query{Accounts: []string{"mesh"}, Folder: "INBOX", Limit: 3}) + if err != nil { + t.Fatal(err) + } + if len(got) != 3 { + t.Fatalf("got %+v", got) + } + want := []struct { + address string + count int + }{{"alice@example.org", 2}, {"bob@example.org", 2}, {"ops@example.org", 2}} + for i, w := range want { + if got[i].Address != w.address || got[i].Count != w.count { + t.Fatalf("line %d: got %+v, want %+v", i, got[i], w) + } + } + // bob's two INBOX mails are invoice1 (09:00) and reply1 (12:30): the newest is reply1's. + for _, g := range got { + if g.Address == "bob@example.org" && !g.Newest.Equal(time.Date(2026, 10, 19, 12, 30, 0, 0, time.UTC)) { + t.Fatalf("bob's newest: %v", g.Newest) + } + } + }) +} diff --git a/modules/mesh-mail/cmd/mesh-mail/store.go b/modules/mesh-mail/cmd/mesh-mail/store.go index f4e53990..5253c4ee 100644 --- a/modules/mesh-mail/cmd/mesh-mail/store.go +++ b/modules/mesh-mail/cmd/mesh-mail/store.go @@ -93,9 +93,11 @@ type Query struct { Folder string Limit int - // From, To and Cc (issue 1228, AC2-AC4, AC6's cc part) are each an address or a domain, case-insensitive; a - // domain (no "@") matches every address of it. Empty means no filter on that field. - From string + // From (issue 1228, AC2, AC3, AC6's cc part; issue 1230, AC11's "several domains") is a list of addresses or + // domains, case-insensitive, OR'd together: a message matches when at least one entry matches (a domain, no + // "@", matching every address of it). To and Cc (issue 1228, AC4, AC6's cc part) are each a single address or + // domain, the same way; no criterion asks for more than one. Empty/nil means no filter on that field. + From []string To string Cc string @@ -125,6 +127,22 @@ type Query struct { // Unread (issue 1229, carrying 1228/AC2's flag part) is nil for no filter; true answers only messages without // the \Seen flag, false only those with it. Unread *bool + + // OrderBy and OrderDesc (issue 1230, AC7, AC11, AC17's schema) choose Search's sort: empty OrderBy means + // "date" with OrderDesc ignored, kept descending (today's newest-first default, AC1); given explicitly, one + // of "date", "sender" (by its first address), "subject" or "size", ascending unless OrderDesc. Ties always + // break ascending on (account, folder, uid). GroupBySender ignores both: its own order is fixed (AC10). + OrderBy string + OrderDesc bool +} + +// SenderGroup is one line of mail_search's group-by-sender answer (issue 1230, AC10): the sender's address and +// name (as last seen on a matching message), how many matching messages it sent, and the newest one's date. +type SenderGroup struct { + Address string `json:"address"` + Name string `json:"name,omitempty"` + Count int `json:"count"` + Newest time.Time `json:"newest"` } // Store is the index. @@ -147,8 +165,11 @@ type Store interface { Remove(ctx context.Context, account, folder string, uid uint32, e *Event) error // Get is one message, with its body. Get(ctx context.Context, account, folder string, uid uint32) (Message, bool, error) - // Search finds messages, newest first, without their bodies. + // Search finds messages, newest first by default (OrderBy, OrderDesc choose otherwise), without their bodies. Search(ctx context.Context, q Query) ([]Message, error) + // GroupBySender is 1230/AC10: one line per sender among the messages Query's other filters match, by count + // descending, Limit bounding the number of lines. + GroupBySender(ctx context.Context, q Query) ([]SenderGroup, error) // Folders is every folder the index knows of an account. Folders(ctx context.Context, account string) ([]Folder, error) // Pending is the events not yet said, oldest first; Said removes those said. @@ -265,47 +286,163 @@ func (s *memStore) Get(_ context.Context, account, folder string, uid uint32) (M return m, ok, nil } +// matchesQuery is every filter of Query but its order and its limit, shared by Search and GroupBySender so the +// two never drift apart (mirrored, condition for condition, by PgStore's own WHERE builder, pgstore.go). +func matchesQuery(m Message, q Query, words []string) bool { + if !contains(q.Accounts, m.Account) || (q.Folder != "" && q.Folder != m.Folder) { + return false + } + // 1228/AC2, 1228/AC3, 1228/AC4, 1228/AC6(cc); 1230/AC11 (From's several domains): the address and date-range + // filters, mirroring PgStore.Search. + if !addressMatchesAny(m.From, q.From) || !addressMatches(m.To, q.To) || !addressMatches(m.Cc, q.Cc) { + return false + } + if !q.OnOrAfter.IsZero() && m.Date.Before(q.OnOrAfter) { + return false + } + if !q.Before.IsZero() && !m.Date.Before(q.Before) { + return false + } + // 1229/AC5, 1229/AC6(words), 1229/AC9, 1229/AC12, 1229/AC13, 1229/AC15: the word, attachment and + // read/unread filters, mirroring PgStore.Search. + if !wordMatches(m.Subject, q.SubjectWords, q.SubjectWordsAny) || !wordMatches(m.Body, q.BodyWords, q.BodyWordsAny) { + return false + } + if !attachmentMatches(m.Attachments, q.HasAttachment, q.AttachmentType, q.AttachmentNamePattern) { + return false + } + if q.Unread != nil && hasSeenFlag(m.Flags) == *q.Unread { + return false + } + hay := strings.ToLower(m.Subject + " " + m.Body + " " + peopleText(m.From) + " " + peopleText(m.To) + " " + peopleText(m.Cc)) + for _, w := range words { + if !strings.Contains(hay, w) { + return false + } + } + return true +} + func (s *memStore) Search(_ context.Context, q Query) ([]Message, error) { s.mu.Lock() defer s.mu.Unlock() words := strings.Fields(strings.ToLower(q.Text)) var out []Message for _, m := range s.messages { - if !contains(q.Accounts, m.Account) || (q.Folder != "" && q.Folder != m.Folder) { - continue - } - // 1228/AC2, 1228/AC3, 1228/AC4, 1228/AC6(cc): the address and date-range filters, mirroring PgStore.Search. - if !addressMatches(m.From, q.From) || !addressMatches(m.To, q.To) || !addressMatches(m.Cc, q.Cc) { - continue - } - if !q.OnOrAfter.IsZero() && m.Date.Before(q.OnOrAfter) { - continue - } - if !q.Before.IsZero() && !m.Date.Before(q.Before) { - continue - } - // 1229/AC5, 1229/AC6(words), 1229/AC9, 1229/AC12, 1229/AC13, 1229/AC15: the word, attachment and - // read/unread filters, mirroring PgStore.Search. - if !wordMatches(m.Subject, q.SubjectWords, q.SubjectWordsAny) || !wordMatches(m.Body, q.BodyWords, q.BodyWordsAny) { - continue - } - if !attachmentMatches(m.Attachments, q.HasAttachment, q.AttachmentType, q.AttachmentNamePattern) { - continue - } - if q.Unread != nil && hasSeenFlag(m.Flags) == *q.Unread { - continue - } - hay := strings.ToLower(m.Subject + " " + m.Body + " " + peopleText(m.From) + " " + peopleText(m.To) + " " + peopleText(m.Cc)) - all := true - for _, w := range words { - all = all && strings.Contains(hay, w) - } - if all { + if matchesQuery(m, q, words) { m.Body = "" out = append(out, m) } } - sort.Slice(out, func(i, j int) bool { return out[i].Date.After(out[j].Date) }) + // 1230: order by date (the default), sender, subject or size, ascending or descending, tied-broken on + // (account, folder, uid) for a stable order. + sort.Slice(out, func(i, j int) bool { return orderLess(out[i], out[j], q.OrderBy, q.OrderDesc) }) + if q.Limit > 0 && len(out) > q.Limit { + out = out[:q.Limit] + } + return out, nil +} + +// orderLess is 1230 (AC7, AC11, AC17's schema): date (the default when orderBy is empty), sender (by its first +// address), subject or size, ascending unless desc; ties break on (account, folder, uid), always ascending, so +// the order is stable regardless of the map order Go would otherwise give equal messages. +func orderLess(a, b Message, orderBy string, desc bool) bool { + if orderBy == "" { + // Unspecified order keeps the newest-first default (AC1), regardless of OrderDesc. + desc = true + } + var cmp int + switch orderBy { + case "sender": + cmp = strings.Compare(firstAddress(a.From), firstAddress(b.From)) + case "subject": + cmp = strings.Compare(a.Subject, b.Subject) + case "size": + cmp = compareInt64(a.Size, b.Size) + default: // "date", or empty (the default) + cmp = compareTime(a.Date, b.Date) + } + if desc { + cmp = -cmp + } + if cmp != 0 { + return cmp < 0 + } + if a.Account != b.Account { + return a.Account < b.Account + } + if a.Folder != b.Folder { + return a.Folder < b.Folder + } + return a.UID < b.UID +} + +func firstAddress(ps []Person) string { + if len(ps) == 0 { + return "" + } + return strings.ToLower(ps[0].Address) +} + +func compareInt64(a, b int64) int { + switch { + case a < b: + return -1 + case a > b: + return 1 + default: + return 0 + } +} + +func compareTime(a, b time.Time) int { + switch { + case a.Before(b): + return -1 + case a.After(b): + return 1 + default: + return 0 + } +} + +// GroupBySender is 1230 (AC10): one line per sender among the messages Query's other filters match (its OrderBy +// and OrderDesc are not used; the lines are always ordered by count descending, address ascending to break ties), +// Limit bounding the number of lines, not the number of messages. +func (s *memStore) GroupBySender(_ context.Context, q Query) ([]SenderGroup, error) { + s.mu.Lock() + defer s.mu.Unlock() + words := strings.Fields(strings.ToLower(q.Text)) + byAddress := map[string]*SenderGroup{} + var order []string + for _, m := range s.messages { + if !matchesQuery(m, q, words) { + continue + } + for _, p := range m.From { + addr := strings.ToLower(p.Address) + g, ok := byAddress[addr] + if !ok { + g = &SenderGroup{Address: p.Address, Name: p.Name} + byAddress[addr] = g + order = append(order, addr) + } + g.Count++ + if m.Date.After(g.Newest) { + g.Newest = m.Date + } + } + } + out := make([]SenderGroup, 0, len(order)) + for _, addr := range order { + out = append(out, *byAddress[addr]) + } + sort.Slice(out, func(i, j int) bool { + if out[i].Count != out[j].Count { + return out[i].Count > out[j].Count + } + return out[i].Address < out[j].Address + }) if q.Limit > 0 && len(out) > q.Limit { out = out[:q.Limit] } @@ -393,6 +530,20 @@ func addressMatches(ps []Person, filter string) bool { return false } +// addressMatchesAny is 1230/AC11: From's several domains (or addresses), OR'd together; empty/nil matches every +// message, otherwise at least one entry (each matched as addressMatches does) must match. +func addressMatchesAny(ps []Person, filters []string) bool { + if len(filters) == 0 { + return true + } + for _, f := range filters { + if addressMatches(ps, f) { + return true + } + } + return false +} + // wordRegexp is a whole-word, case-insensitive match (issue 1229, AC12): \b, Go's RE2 word boundary, treats a // dash as a non-word character, so a hyphenated word such as "4711-882" is bounded at both ends and never matched // by only its digits.