From 59e6557cc5ee64dc8ca29b1c14e1143943cfa577 Mon Sep 17 00:00:00 2001 From: Pascal Bleser Date: Wed, 26 Nov 2025 15:26:18 +0100 Subject: [PATCH] groupware: fix email summaries and allow negative offsets * fix a bug in how email summaries are flattened across multiple accounts, which was previous resulting in empty email objects * allow negative offset in email pagination * make all /emails endpoints return emails without bodies --- pkg/jmap/jmap_api_email.go | 43 +-- pkg/jmap/jmap_model.go | 4 +- .../pkg/groupware/groupware_api_emails.go | 298 ++++++------------ .../pkg/groupware/groupware_route.go | 2 - 4 files changed, 114 insertions(+), 233 deletions(-) diff --git a/pkg/jmap/jmap_api_email.go b/pkg/jmap/jmap_api_email.go index ec882a9016..bdcab1a855 100644 --- a/pkg/jmap/jmap_api_email.go +++ b/pkg/jmap/jmap_api_email.go @@ -107,9 +107,9 @@ func (j *Client) GetEmailBlobId(accountId string, session *Session, ctx context. } // Retrieve all the Emails in a given Mailbox by its id. -func (j *Client) GetAllEmailsInMailbox(accountId string, session *Session, ctx context.Context, logger *log.Logger, acceptLanguage string, mailboxId string, offset uint, limit uint, collapseThreads bool, fetchBodies bool, maxBodyValueBytes uint, withThreads bool) (Emails, SessionState, State, Language, Error) { +func (j *Client) GetAllEmailsInMailbox(accountId string, session *Session, ctx context.Context, logger *log.Logger, acceptLanguage string, mailboxId string, offset int, limit uint, collapseThreads bool, fetchBodies bool, maxBodyValueBytes uint, withThreads bool) (Emails, SessionState, State, Language, Error) { logger = j.loggerParams("GetAllEmailsInMailbox", session, logger, func(z zerolog.Context) zerolog.Context { - return z.Bool(logFetchBodies, fetchBodies).Uint(logOffset, offset).Uint(logLimit, limit) + return z.Bool(logFetchBodies, fetchBodies).Int(logOffset, offset).Uint(logLimit, limit) }) query := EmailQueryCommand{ @@ -123,7 +123,7 @@ func (j *Client) GetAllEmailsInMailbox(accountId string, session *Session, ctx c query.Position = offset } if limit > 0 { - query.Limit = limit + query.Limit = &limit } get := EmailGetRefCommand{ @@ -273,9 +273,9 @@ type EmailSnippetQueryResult struct { QueryState State `json:"queryState"` } -func (j *Client) QueryEmailSnippets(accountIds []string, filter EmailFilterElement, session *Session, ctx context.Context, logger *log.Logger, acceptLanguage string, offset uint, limit uint) (map[string]EmailSnippetQueryResult, SessionState, State, Language, Error) { +func (j *Client) QueryEmailSnippets(accountIds []string, filter EmailFilterElement, session *Session, ctx context.Context, logger *log.Logger, acceptLanguage string, offset int, limit uint) (map[string]EmailSnippetQueryResult, SessionState, State, Language, Error) { logger = j.loggerParams("QueryEmailSnippets", session, logger, func(z zerolog.Context) zerolog.Context { - return z.Uint(logLimit, limit).Uint(logOffset, offset) + return z.Uint(logLimit, limit).Int(logOffset, offset) }) uniqueAccountIds := structs.Uniq(accountIds) @@ -292,7 +292,7 @@ func (j *Client) QueryEmailSnippets(accountIds []string, filter EmailFilterEleme query.Position = offset } if limit > 0 { - query.Limit = limit + query.Limit = &limit } mails := EmailGetRefCommand{ @@ -389,7 +389,7 @@ type EmailQueryResult struct { QueryState State `json:"queryState"` } -func (j *Client) QueryEmails(accountIds []string, filter EmailFilterElement, session *Session, ctx context.Context, logger *log.Logger, acceptLanguage string, offset uint, limit uint, fetchBodies bool, maxBodyValueBytes uint) (map[string]EmailQueryResult, SessionState, State, Language, Error) { +func (j *Client) QueryEmails(accountIds []string, filter EmailFilterElement, session *Session, ctx context.Context, logger *log.Logger, acceptLanguage string, offset int, limit uint, fetchBodies bool, maxBodyValueBytes uint) (map[string]EmailQueryResult, SessionState, State, Language, Error) { logger = j.loggerParams("QueryEmails", session, logger, func(z zerolog.Context) zerolog.Context { return z.Bool(logFetchBodies, fetchBodies) }) @@ -408,7 +408,7 @@ func (j *Client) QueryEmails(accountIds []string, filter EmailFilterElement, ses query.Position = offset } if limit > 0 { - query.Limit = limit + query.Limit = &limit } mails := EmailGetRefCommand{ @@ -471,7 +471,7 @@ type EmailQueryWithSnippetsResult struct { QueryState State `json:"queryState"` } -func (j *Client) QueryEmailsWithSnippets(accountIds []string, filter EmailFilterElement, session *Session, ctx context.Context, logger *log.Logger, acceptLanguage string, offset uint, limit uint, fetchBodies bool, maxBodyValueBytes uint) (map[string]EmailQueryWithSnippetsResult, SessionState, State, Language, Error) { +func (j *Client) QueryEmailsWithSnippets(accountIds []string, filter EmailFilterElement, session *Session, ctx context.Context, logger *log.Logger, acceptLanguage string, offset int, limit uint, fetchBodies bool, maxBodyValueBytes uint) (map[string]EmailQueryWithSnippetsResult, SessionState, State, Language, Error) { logger = j.loggerParams("QueryEmailsWithSnippets", session, logger, func(z zerolog.Context) zerolog.Context { return z.Bool(logFetchBodies, fetchBodies) }) @@ -490,7 +490,7 @@ func (j *Client) QueryEmailsWithSnippets(accountIds []string, filter EmailFilter query.Position = offset } if limit > 0 { - query.Limit = limit + query.Limit = &limit } snippet := SearchSnippetGetRefCommand{ @@ -920,9 +920,9 @@ func (j *Client) EmailsInThread(accountId string, threadId string, session *Sess type EmailsSummary struct { Emails []Email `json:"emails"` - Total int `json:"total"` - Limit int `json:"limit"` - Offset int `json:"offset"` + Total uint `json:"total"` + Limit uint `json:"limit"` + Offset uint `json:"offset"` State State `json:"state"` } @@ -957,13 +957,16 @@ func (j *Client) QueryEmailSummaries(accountIds []string, session *Session, ctx invocations := make([]Invocation, len(uniqueAccountIds)*factor) for i, accountId := range uniqueAccountIds { - invocations[i*factor+0] = invocation(CommandEmailQuery, EmailQueryCommand{ + get := EmailQueryCommand{ AccountId: accountId, Filter: filter, Sort: []EmailComparator{{Property: EmailPropertyReceivedAt, IsAscending: false}}, - Limit: limit, - //CalculateTotal: false, - }, mcid(accountId, "0")) + } + if limit > 0 { + get.Limit = &limit + } + invocations[i*factor+0] = invocation(CommandEmailQuery, get, mcid(accountId, "0")) + invocations[i*factor+1] = invocation(CommandEmailGet, EmailGetRefCommand{ AccountId: accountId, IdsRef: &ResultReference{ @@ -1017,9 +1020,9 @@ func (j *Client) QueryEmailSummaries(accountIds []string, session *Session, ctx resp[accountId] = EmailsSummary{ Emails: response.List, - Total: int(queryResponse.Total), - Limit: int(queryResponse.Limit), - Offset: int(queryResponse.Position), + Total: queryResponse.Total, + Limit: queryResponse.Limit, + Offset: queryResponse.Position, State: response.State, } } diff --git a/pkg/jmap/jmap_model.go b/pkg/jmap/jmap_model.go index c09d59eb82..dd8b8a14c2 100644 --- a/pkg/jmap/jmap_model.go +++ b/pkg/jmap/jmap_model.go @@ -1677,7 +1677,7 @@ type EmailQueryCommand struct { // // If the index is greater than or equal to the total number of objects in the results // list, then the ids array in the response will be empty, but this is not an error. - Position uint `json:"position,omitempty"` + Position int `json:"position,omitempty"` // An Email id. // @@ -1705,7 +1705,7 @@ type EmailQueryCommand struct { // to the maximum; the new limit is returned with the response so the client is aware. // // If a negative value is given, the call MUST be rejected with an invalidArguments error. - Limit uint `json:"limit,omitempty"` + Limit *uint `json:"limit,omitempty"` // Does the client wish to know the total number of results in the query? // diff --git a/services/groupware/pkg/groupware/groupware_api_emails.go b/services/groupware/pkg/groupware/groupware_api_emails.go index a5499ee785..551c3229f2 100644 --- a/services/groupware/pkg/groupware/groupware_api_emails.go +++ b/services/groupware/pkg/groupware/groupware_api_emails.go @@ -17,7 +17,6 @@ import ( "github.com/opencloud-eu/opencloud/pkg/jmap" "github.com/opencloud-eu/opencloud/pkg/log" - "github.com/opencloud-eu/opencloud/pkg/structs" "github.com/opencloud-eu/opencloud/services/groupware/pkg/metrics" ) @@ -100,12 +99,12 @@ func (g *Groupware) GetAllEmailsInMailbox(w http.ResponseWriter, r *http.Request return req.parameterErrorResponse(accountId, UriParamMailboxId, fmt.Sprintf("Missing required mailbox ID path parameter '%v'", UriParamMailboxId)) } - offset, ok, err := req.parseUIntParam(QueryParamOffset, 0) + offset, ok, err := req.parseIntParam(QueryParamOffset, 0) if err != nil { return errorResponse(accountId, err) } if ok { - l = l.Uint(QueryParamOffset, offset) + l = l.Int(QueryParamOffset, offset) } limit, ok, err := req.parseUIntParam(QueryParamLimit, g.defaultEmailLimit) @@ -434,7 +433,7 @@ type EmailSearchResults struct { QueryState jmap.State `json:"queryState,omitempty"` } -func (g *Groupware) buildFilter(req Request) (bool, jmap.EmailFilterElement, bool, uint, uint, *log.Logger, *Error) { +func (g *Groupware) buildFilter(req Request) (bool, jmap.EmailFilterElement, bool, int, uint, *log.Logger, *Error) { q := req.r.URL.Query() mailboxId := q.Get(QueryParamMailboxId) notInMailboxIds := q[QueryParamNotInMailboxId] @@ -452,12 +451,12 @@ func (g *Groupware) buildFilter(req Request) (bool, jmap.EmailFilterElement, boo l := req.logger.With() - offset, ok, err := req.parseUIntParam(QueryParamOffset, 0) + offset, ok, err := req.parseIntParam(QueryParamOffset, 0) if err != nil { return false, nil, snippets, 0, 0, nil, err } if ok { - l = l.Uint(QueryParamOffset, offset) + l = l.Int(QueryParamOffset, offset) } limit, ok, err := req.parseUIntParam(QueryParamLimit, g.defaultEmailLimit) @@ -582,39 +581,34 @@ func (g *Groupware) buildFilter(req Request) (bool, jmap.EmailFilterElement, boo return true, filter, snippets, offset, limit, logger, nil } -func (g *Groupware) searchEmails(w http.ResponseWriter, r *http.Request) { - g.respond(w, r, func(req Request) Response { - accountId, err := req.GetAccountIdForMail() - if err != nil { - return errorResponse(accountId, err) - } - - ok, filter, makesSnippets, offset, limit, logger, err := g.buildFilter(req) - if !ok { - return errorResponse(accountId, err) - } - logger = log.From(req.logger.With().Str(logAccountId, log.SafeString(accountId))) - - if !filter.IsNotEmpty() { - filter = nil - } - - fetchEmails, ok, err := req.parseBoolParam(QueryParamSearchFetchEmails, false) - if err != nil { - return errorResponse(accountId, err) - } - if ok { - logger = log.From(logger.With().Bool(QueryParamSearchFetchEmails, fetchEmails)) - } - - if fetchEmails { - fetchBodies, ok, err := req.parseBoolParam(QueryParamSearchFetchBodies, false) +func (g *Groupware) GetEmails(w http.ResponseWriter, r *http.Request) { + q := r.URL.Query() + since := q.Get(QueryParamSince) + if since == "" { + since = r.Header.Get(HeaderSince) + } + if since != "" { + // get email changes since a given state + g.getEmailsSince(w, r, since) + } else { + // do a search + g.respond(w, r, func(req Request) Response { + accountId, err := req.GetAccountIdForMail() if err != nil { return errorResponse(accountId, err) } - if ok { - logger = log.From(logger.With().Bool(QueryParamSearchFetchBodies, fetchBodies)) + + ok, filter, makesSnippets, offset, limit, logger, err := g.buildFilter(req) + if !ok { + return errorResponse(accountId, err) } + logger = log.From(req.logger.With().Str(logAccountId, log.SafeString(accountId))) + + if !filter.IsNotEmpty() { + filter = nil + } + + fetchBodies := false resultsByAccount, sessionState, state, lang, jerr := g.jmap.QueryEmailsWithSnippets([]string{accountId}, filter, req.session, req.ctx, logger, req.language(), offset, limit, fetchBodies, g.maxBodyValueBytes) if jerr != nil { @@ -655,38 +649,7 @@ func (g *Groupware) searchEmails(w http.ResponseWriter, r *http.Request) { } else { return notFoundResponse(accountId, sessionState) } - } else { - resultsByAccountId, sessionState, state, lang, jerr := g.jmap.QueryEmailSnippets([]string{accountId}, filter, req.session, req.ctx, logger, req.language(), offset, limit) - if jerr != nil { - return req.errorResponseFromJmap(accountId, jerr) - } - - if results, ok := resultsByAccountId[accountId]; ok { - return etagResponse(accountId, EmailSearchSnippetsResults{ - Results: structs.Map(results.Snippets, func(s jmap.SearchSnippetWithMeta) Snippet { return Snippet{SearchSnippetWithMeta: s} }), - Total: results.Total, - Limit: results.Limit, - QueryState: results.QueryState, - }, sessionState, EmailResponseObjectType, state, lang) - } else { - return notFoundResponse(accountId, sessionState) - } - } - }) -} - -func (g *Groupware) GetEmails(w http.ResponseWriter, r *http.Request) { - q := r.URL.Query() - since := q.Get(QueryParamSince) - if since == "" { - since = r.Header.Get(HeaderSince) - } - if since != "" { - // get email changes since a given state - g.getEmailsSince(w, r, since) - } else { - // do a search - g.searchEmails(w, r) + }) } } @@ -704,162 +667,79 @@ func (g *Groupware) GetEmailsForAllAccounts(w http.ResponseWriter, r *http.Reque filter = nil } - fetchEmails, ok, err := req.parseBoolParam(QueryParamSearchFetchEmails, false) - if err != nil { - return errorResponse(joinAccountIds(allAccountIds), err) - } - if ok { - logger = log.From(logger.With().Bool(QueryParamSearchFetchEmails, fetchEmails)) - } - - if fetchEmails { - fetchBodies, ok, err := req.parseBoolParam(QueryParamSearchFetchBodies, false) - if err != nil { - return errorResponse(joinAccountIds(allAccountIds), err) - } - if ok { - logger = log.From(logger.With().Bool(QueryParamSearchFetchBodies, fetchBodies)) + if makesSnippets { + resultsByAccountId, sessionState, state, lang, jerr := g.jmap.QueryEmailSnippets(allAccountIds, filter, req.session, req.ctx, logger, req.language(), offset, limit) + if jerr != nil { + return req.errorResponseFromJmap(joinAccountIds(allAccountIds), jerr) } - if makesSnippets { - resultsByAccountId, sessionState, state, lang, jerr := g.jmap.QueryEmailsWithSnippets(allAccountIds, filter, req.session, req.ctx, logger, req.language(), offset, limit, fetchBodies, g.maxBodyValueBytes) - if jerr != nil { - return req.errorResponseFromJmap(joinAccountIds(allAccountIds), jerr) - } + var totalOverAllAccounts uint = 0 + total := 0 + for _, results := range resultsByAccountId { + totalOverAllAccounts += results.Total + total += len(results.Snippets) + } - flattenedByAccountId := make(map[string][]EmailWithSnippets, len(resultsByAccountId)) - total := 0 - var totalOverAllAccounts uint = 0 + flattened := make([]Snippet, total) + { + i := 0 for accountId, results := range resultsByAccountId { - totalOverAllAccounts += results.Total - flattened := make([]EmailWithSnippets, len(results.Results)) - for i, result := range results.Results { - snippets := structs.MapN(result.Snippets, func(s jmap.SearchSnippet) *SnippetWithoutEmailId { - if s.Subject != "" || s.Preview != "" { - return &SnippetWithoutEmailId{ - Subject: s.Subject, - Preview: s.Preview, - } - } else { - return nil - } - }) - - sanitized, err := req.sanitizeEmail(result.Email) - if err != nil { - return errorResponseWithSessionState(accountId, err, sessionState) - } - flattened[i] = EmailWithSnippets{ - AccountId: accountId, - Email: sanitized, - Snippets: snippets, - } - } - flattenedByAccountId[accountId] = flattened - total += len(flattened) - } - - flattened := make([]EmailWithSnippets, total) - { - i := 0 - for _, list := range flattenedByAccountId { - for _, e := range list { - flattened[i] = e - i++ + for _, result := range results.Snippets { + flattened[i] = Snippet{ + AccountId: accountId, + SearchSnippetWithMeta: result, } } } - - slices.SortFunc(flattened, func(a, b EmailWithSnippets) int { return a.ReceivedAt.Compare(b.ReceivedAt) }) - - // TODO offset and limit over the aggregated results by account - - return etagResponse(joinAccountIds(allAccountIds), EmailWithSnippetsSearchResults{ - Results: flattened, - Total: totalOverAllAccounts, - Limit: limit, - QueryState: state, - }, sessionState, EmailResponseObjectType, state, lang) - } else { - resultsByAccountId, sessionState, state, lang, jerr := g.jmap.QueryEmails(allAccountIds, filter, req.session, req.ctx, logger, req.language(), offset, limit, fetchBodies, g.maxBodyValueBytes) - if jerr != nil { - return req.errorResponseFromJmap(joinAccountIds(allAccountIds), jerr) - } - - total := 0 - var totalOverAllAccounts uint = 0 - for _, results := range resultsByAccountId { - totalOverAllAccounts += results.Total - total += len(results.Emails) - } - - flattened := make([]jmap.Email, total) - { - i := 0 - for _, list := range resultsByAccountId { - for _, e := range list.Emails { - sanitized, err := req.sanitizeEmail(e) - if err != nil { - return errorResponseWithSessionState(joinAccountIds(allAccountIds), err, sessionState) - } - flattened[i] = sanitized - i++ - } - } - } - - slices.SortFunc(flattened, func(a, b jmap.Email) int { return a.ReceivedAt.Compare(b.ReceivedAt) }) - - // TODO offset and limit over the aggregated results by account - - return etagResponse(joinAccountIds(allAccountIds), EmailSearchResults{ - Results: flattened, - Total: totalOverAllAccounts, - Limit: limit, - QueryState: state, - }, sessionState, EmailResponseObjectType, state, lang) } + + slices.SortFunc(flattened, func(a, b Snippet) int { return a.ReceivedAt.Compare(b.ReceivedAt) }) + + // TODO offset and limit over the aggregated results by account + + return etagResponse(joinAccountIds(allAccountIds), EmailSearchSnippetsResults{ + Results: flattened, + Total: totalOverAllAccounts, + Limit: limit, + QueryState: state, + }, sessionState, EmailResponseObjectType, state, lang) } else { - if makesSnippets { - resultsByAccountId, sessionState, state, lang, jerr := g.jmap.QueryEmailSnippets(allAccountIds, filter, req.session, req.ctx, logger, req.language(), offset, limit) - if jerr != nil { - return req.errorResponseFromJmap(joinAccountIds(allAccountIds), jerr) - } + withThreads := true - var totalOverAllAccounts uint = 0 - total := 0 - for _, results := range resultsByAccountId { - totalOverAllAccounts += results.Total - total += len(results.Snippets) - } + resultsByAccountId, sessionState, state, lang, jerr := g.jmap.QueryEmailSummaries(allAccountIds, req.session, req.ctx, logger, req.language(), filter, limit, withThreads) + if jerr != nil { + return req.errorResponseFromJmap(joinAccountIds(allAccountIds), jerr) + } - flattened := make([]Snippet, total) - { - i := 0 - for accountId, results := range resultsByAccountId { - for _, result := range results.Snippets { - flattened[i] = Snippet{ - AccountId: accountId, - SearchSnippetWithMeta: result, - } - } + var totalAcrossAllAccounts uint = 0 + total := 0 + for _, results := range resultsByAccountId { + totalAcrossAllAccounts += results.Total + total += len(results.Emails) + } + + flattened := make([]jmap.Email, total) + { + i := 0 + for accountId, results := range resultsByAccountId { + for _, result := range results.Emails { + result.AccountId = accountId + flattened[i] = result + i++ } } - - slices.SortFunc(flattened, func(a, b Snippet) int { return a.ReceivedAt.Compare(b.ReceivedAt) }) - - // TODO offset and limit over the aggregated results by account - - return etagResponse(joinAccountIds(allAccountIds), EmailSearchSnippetsResults{ - Results: flattened, - Total: totalOverAllAccounts, - Limit: limit, - QueryState: state, - }, sessionState, EmailResponseObjectType, state, lang) - } else { - // TODO implement search without email bodies (only retrieve a few chosen properties?) + without snippets - return notImplementesResponse() } + + slices.SortFunc(flattened, func(a, b jmap.Email) int { return a.ReceivedAt.Compare(b.ReceivedAt) }) + + // TODO offset and limit over the aggregated results by account + + return etagResponse(joinAccountIds(allAccountIds), EmailSearchResults{ + Results: flattened, + Total: totalAcrossAllAccounts, + Limit: limit, + QueryState: state, + }, sessionState, EmailResponseObjectType, state, lang) } }) } diff --git a/services/groupware/pkg/groupware/groupware_route.go b/services/groupware/pkg/groupware/groupware_route.go index d31a63f68d..9dcf755851 100644 --- a/services/groupware/pkg/groupware/groupware_route.go +++ b/services/groupware/pkg/groupware/groupware_route.go @@ -48,8 +48,6 @@ const ( QueryParamSearchMaxSize = "maxsize" QueryParamSearchKeyword = "keyword" QueryParamSearchMessageId = "messageId" - QueryParamSearchFetchBodies = "fetchbodies" - QueryParamSearchFetchEmails = "fetchemails" QueryParamOffset = "offset" QueryParamLimit = "limit" QueryParamDays = "days"