From 42cce92e8fbcfaa233f0d4eef6cb82a38c59d15b Mon Sep 17 00:00:00 2001 From: Pascal Bleser Date: Fri, 5 Dec 2025 10:36:31 +0100 Subject: [PATCH] groupware: add retrieving and adding mailboxIds for drafts and sent if they are missing --- pkg/jmap/jmap_api_email.go | 24 ++-- pkg/jmap/jmap_api_mailbox.go | 40 ++++++ pkg/jmap/jmap_integration_email_test.go | 100 +++++++++++++++ pkg/jmap/jmap_model.go | 8 +- pkg/structs/structs.go | 7 +- .../pkg/groupware/groupware_api_emails.go | 117 +++++++++++++++--- .../pkg/groupware/groupware_error.go | 14 +++ 7 files changed, 276 insertions(+), 34 deletions(-) diff --git a/pkg/jmap/jmap_api_email.go b/pkg/jmap/jmap_api_email.go index b145f7d616..e548b83444 100644 --- a/pkg/jmap/jmap_api_email.go +++ b/pkg/jmap/jmap_api_email.go @@ -18,8 +18,13 @@ type Emails struct { Offset uint `json:"offset,omitzero"` } +type getEmailsResult struct { + emails []Email + notFound []string +} + // Retrieve specific Emails by their id. -func (j *Client) GetEmails(accountId string, session *Session, ctx context.Context, logger *log.Logger, acceptLanguage string, ids []string, fetchBodies bool, maxBodyValueBytes uint, markAsSeen bool, withThreads bool) ([]Email, SessionState, State, Language, Error) { +func (j *Client) GetEmails(accountId string, session *Session, ctx context.Context, logger *log.Logger, acceptLanguage string, ids []string, fetchBodies bool, maxBodyValueBytes uint, markAsSeen bool, withThreads bool) ([]Email, []string, SessionState, State, Language, Error) { logger = j.logger("GetEmails", session, logger) get := EmailGetCommand{AccountId: accountId, Ids: ids, FetchAllBodyValues: fetchBodies} @@ -52,35 +57,36 @@ func (j *Client) GetEmails(accountId string, session *Session, ctx context.Conte cmd, err := j.request(session, logger, methodCalls...) if err != nil { logger.Error().Err(err).Send() - return nil, "", "", "", simpleError(err, JmapErrorInvalidJmapRequestPayload) + return nil, nil, "", "", "", simpleError(err, JmapErrorInvalidJmapRequestPayload) } - return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, acceptLanguage, func(body *Response) ([]Email, State, Error) { + result, sessionState, state, language, gwerr := command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, acceptLanguage, func(body *Response) (getEmailsResult, State, Error) { if markAsSeen { var markResponse EmailSetResponse err = retrieveResponseMatchParameters(logger, body, CommandEmailSet, "0", &markResponse) if err != nil { - return nil, "", err + return getEmailsResult{}, "", err } for _, seterr := range markResponse.NotUpdated { // TODO we don't have a way to compose multiple set errors yet - return nil, "", setErrorError(seterr, EmailType) + return getEmailsResult{}, "", setErrorError(seterr, EmailType) } } var response EmailGetResponse err = retrieveResponseMatchParameters(logger, body, CommandEmailGet, "1", &response) if err != nil { - return nil, "", err + return getEmailsResult{}, "", err } if withThreads { var threads ThreadGetResponse err = retrieveResponseMatchParameters(logger, body, CommandThreadGet, "2", &threads) if err != nil { - return nil, "", err + return getEmailsResult{}, "", err } setThreadSize(&threads, response.List) } - return response.List, response.State, nil + return getEmailsResult{emails: response.List, notFound: response.NotFound}, response.State, nil }) + return result.emails, result.notFound, sessionState, state, language, gwerr } func (j *Client) GetEmailBlobId(accountId string, session *Session, ctx context.Context, logger *log.Logger, acceptLanguage string, id string) (string, SessionState, State, Language, Error) { @@ -890,7 +896,7 @@ func (j *Client) SubmitEmail(accountId string, identityId string, emailId string return EmailSubmission{}, "", err } - if emailId := structs.FirstKey(setResponse.Updated); emailId != nil && len(setResponse.Updated) == 1 { + if len(setResponse.Updated) == 1 { var getResponse EmailSubmissionGetResponse err = retrieveResponseMatchParameters(logger, body, CommandEmailSubmissionGet, "1", &getResponse) if err != nil { diff --git a/pkg/jmap/jmap_api_mailbox.go b/pkg/jmap/jmap_api_mailbox.go index 0f65f8e30a..4b18829e11 100644 --- a/pkg/jmap/jmap_api_mailbox.go +++ b/pkg/jmap/jmap_api_mailbox.go @@ -114,6 +114,46 @@ func (j *Client) SearchMailboxes(accountIds []string, session *Session, ctx cont }) } +func (j *Client) SearchMailboxIdsPerRole(accountIds []string, session *Session, ctx context.Context, logger *log.Logger, acceptLanguage string, roles []string) (map[string]map[string]string, SessionState, State, Language, Error) { + logger = j.logger("SearchMailboxIdsPerRole", session, logger) + + uniqueAccountIds := structs.Uniq(accountIds) + + invocations := make([]Invocation, len(uniqueAccountIds)*len(roles)) + for i, accountId := range uniqueAccountIds { + for j, role := range roles { + invocations[i*len(roles)+j] = invocation(CommandMailboxQuery, MailboxQueryCommand{AccountId: accountId, Filter: MailboxFilterCondition{Role: role}}, mcid(accountId, role)) + } + } + cmd, err := j.request(session, logger, invocations...) + if err != nil { + return nil, "", "", "", err + } + + return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, acceptLanguage, func(body *Response) (map[string]map[string]string, State, Error) { + resp := map[string]map[string]string{} + stateByAccountid := map[string]State{} + for _, accountId := range uniqueAccountIds { + mailboxIdsByRole := map[string]string{} + for _, role := range roles { + var response MailboxQueryResponse + err = retrieveResponseMatchParameters(logger, body, CommandMailboxQuery, mcid(accountId, role), &response) + if err != nil { + return nil, "", err + } + if len(response.Ids) == 1 { + mailboxIdsByRole[role] = response.Ids[0] + } + if _, ok := stateByAccountid[accountId]; !ok { + stateByAccountid[accountId] = response.QueryState + } + } + resp[accountId] = mailboxIdsByRole + } + return resp, squashState(stateByAccountid), nil + }) +} + type MailboxChanges struct { Destroyed []string `json:"destroyed,omitzero"` HasMoreChanges bool `json:"hasMoreChanges,omitzero"` diff --git a/pkg/jmap/jmap_integration_email_test.go b/pkg/jmap/jmap_integration_email_test.go index bb39c75023..2a53261bfb 100644 --- a/pkg/jmap/jmap_integration_email_test.go +++ b/pkg/jmap/jmap_integration_email_test.go @@ -134,6 +134,106 @@ func TestEmails(t *testing.T) { } } +func TestSendingEmails(t *testing.T) { + if skip(t) { + return + } + + require := require.New(t) + + s, err := newStalwartTest(t) + require.NoError(err) + defer s.Close() + + accountId := s.session.PrimaryAccounts.Mail + + var mailboxPerRole map[string]Mailbox + { + mailboxes, _, _, _, err := s.client.GetAllMailboxes([]string{accountId}, s.session, s.ctx, s.logger, "") + require.NoError(err) + mailboxPerRole = structs.Index(mailboxes[accountId], func(m Mailbox) string { return m.Role }) + require.Contains(mailboxPerRole, JmapMailboxRoleInbox) + require.Contains(mailboxPerRole, JmapMailboxRoleDrafts) + require.Contains(mailboxPerRole, JmapMailboxRoleSent) + require.Contains(mailboxPerRole, JmapMailboxRoleTrash) + } + { + roles := []string{JmapMailboxRoleDrafts, JmapMailboxRoleSent, JmapMailboxRoleInbox} + m, _, _, _, err := s.client.SearchMailboxIdsPerRole([]string{accountId}, s.session, s.ctx, s.logger, "", roles) + require.NoError(err) + require.Contains(m, accountId) + a := m[accountId] + for _, role := range roles { + require.Contains(a, role) + } + } + + { + var identity Identity + { + identities, _, _, _, err := s.client.GetAllIdentities(accountId, s.session, s.ctx, s.logger, "") + require.NoError(err) + require.NotEmpty(identities) + identity = identities[0] + } + + create := EmailCreate{ + Keywords: toBoolMapS("test"), + Subject: "testing 123", + MailboxIds: toBoolMapS(mailboxPerRole[JmapMailboxRoleDrafts].Id), + } + created, _, _, _, err := s.client.CreateEmail(accountId, create, "", s.session, s.ctx, s.logger, "") + require.NoError(err) + require.NotEmpty(created.Id) + + { + emails, notFound, _, _, _, err := s.client.GetEmails(accountId, s.session, s.ctx, s.logger, "", []string{created.Id}, true, 0, false, false) + require.NoError(err) + require.Len(emails, 1) + require.Empty(notFound) + email := emails[0] + require.Equal(created.Id, email.Id) + require.Len(email.MailboxIds, 1) + require.Contains(email.MailboxIds, mailboxPerRole[JmapMailboxRoleDrafts].Id) + } + + update := EmailCreate{ + To: []EmailAddress{{Name: identity.Name, Email: identity.Email}}, + Keywords: toBoolMapS("test"), + Subject: "testing 1234", + MailboxIds: toBoolMapS(mailboxPerRole[JmapMailboxRoleDrafts].Id), + } + updated, _, _, _, err := s.client.CreateEmail(accountId, update, created.Id, s.session, s.ctx, s.logger, "") + require.NoError(err) + require.NotEmpty(updated.Id) + require.NotEqual(created.Id, updated.Id) + + var updatedMailboxId string + { + emails, notFound, _, _, _, err := s.client.GetEmails(accountId, s.session, s.ctx, s.logger, "", []string{created.Id, updated.Id}, true, 0, false, false) + require.NoError(err) + require.Len(emails, 1) + require.Len(notFound, 1) + email := emails[0] + require.Equal(updated.Id, email.Id) + require.Len(email.MailboxIds, 1) + require.Contains(email.MailboxIds, mailboxPerRole[JmapMailboxRoleDrafts].Id) + require.Equal(notFound[0], created.Id) + var ok bool + updatedMailboxId, ok = structs.FirstKey(email.MailboxIds) + require.True(ok) + } + + move := MoveMail{ + FromMailboxId: updatedMailboxId, + ToMailboxId: mailboxPerRole[JmapMailboxRoleSent].Id, + } + + sub, _, _, _, err := s.client.SubmitEmail(accountId, identity.Id, updated.Id, &move, s.session, s.ctx, s.logger, "") + fmt.Printf("sub: %v\n", sub) + } +} + func matchEmail(t *testing.T, actual Email, expected filledMail, hasBodies bool) { require := require.New(t) require.Len(actual.MessageId, 1) diff --git a/pkg/jmap/jmap_model.go b/pkg/jmap/jmap_model.go index bf01424cd8..603493b868 100644 --- a/pkg/jmap/jmap_model.go +++ b/pkg/jmap/jmap_model.go @@ -1390,7 +1390,6 @@ type MailboxFilterElement interface { } type MailboxFilterCondition struct { - MailboxFilterElement ParentId string `json:"parentId,omitempty"` Name string `json:"name,omitempty"` Role string `json:"role,omitempty"` @@ -1398,14 +1397,17 @@ type MailboxFilterCondition struct { IsSubscribed *bool `json:"isSubscribed,omitempty"` } +func (c MailboxFilterCondition) _isAMailboxFilterElement() {} + var _ MailboxFilterElement = &MailboxFilterCondition{} type MailboxFilterOperator struct { - MailboxFilterElement Operator FilterOperatorTerm `json:"operator"` Conditions []MailboxFilterElement `json:"conditions,omitempty"` } +func (c MailboxFilterOperator) _isAMailboxFilterElement() {} + var _ MailboxFilterElement = &MailboxFilterOperator{} type MailboxComparator struct { @@ -2840,7 +2842,7 @@ type EmailGetResponse struct { // This array contains the ids passed to the method for records that do not exist. // // The array is empty if all requested ids were found or if the ids argument passed in was either null or an empty array. - NotFound []any `json:"notFound"` + NotFound []string `json:"notFound"` } type EmailChangesResponse struct { diff --git a/pkg/structs/structs.go b/pkg/structs/structs.go index 4eb63e9a2e..80aaf457fe 100644 --- a/pkg/structs/structs.go +++ b/pkg/structs/structs.go @@ -170,11 +170,12 @@ func Missing[E comparable](expected, actual []E) []E { return missing } -func FirstKey[K comparable, V any](m map[K]V) *K { +func FirstKey[K comparable, V any](m map[K]V) (K, bool) { for k := range m { - return &k + return k, true } - return nil + var zero K + return zero, false } func Any[E any](s []E, predicate func(E) bool) bool { diff --git a/services/groupware/pkg/groupware/groupware_api_emails.go b/services/groupware/pkg/groupware/groupware_api_emails.go index 0a319e6a50..5b592fc070 100644 --- a/services/groupware/pkg/groupware/groupware_api_emails.go +++ b/services/groupware/pkg/groupware/groupware_api_emails.go @@ -17,6 +17,7 @@ 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" ) @@ -208,7 +209,7 @@ func (g *Groupware) GetEmailsById(w http.ResponseWriter, r *http.Request) { if len(ids) == 1 { logger := log.From(l.Str("id", log.SafeString(id))) - emails, sessionState, state, lang, jerr := g.jmap.GetEmails(accountId, req.session, req.ctx, logger, req.language(), ids, true, g.maxBodyValueBytes, markAsSeen, true) + emails, _, sessionState, state, lang, jerr := g.jmap.GetEmails(accountId, req.session, req.ctx, logger, req.language(), ids, true, g.maxBodyValueBytes, markAsSeen, true) if jerr != nil { return req.errorResponseFromJmap(single(accountId), jerr) } @@ -224,7 +225,7 @@ func (g *Groupware) GetEmailsById(w http.ResponseWriter, r *http.Request) { } else { logger := log.From(l.Array("ids", log.SafeStringArray(ids))) - emails, sessionState, state, lang, jerr := g.jmap.GetEmails(accountId, req.session, req.ctx, logger, req.language(), ids, true, g.maxBodyValueBytes, markAsSeen, false) + emails, _, sessionState, state, lang, jerr := g.jmap.GetEmails(accountId, req.session, req.ctx, logger, req.language(), ids, true, g.maxBodyValueBytes, markAsSeen, false) if jerr != nil { return req.errorResponseFromJmap(single(accountId), jerr) } @@ -274,7 +275,7 @@ func (g *Groupware) GetEmailAttachments(w http.ResponseWriter, r *http.Request) } l := req.logger.With().Str(logAccountId, log.SafeString(accountId)) logger := log.From(l) - emails, sessionState, state, lang, jerr := g.jmap.GetEmails(accountId, req.session, req.ctx, logger, req.language(), []string{id}, false, 0, false, false) + emails, _, sessionState, state, lang, jerr := g.jmap.GetEmails(accountId, req.session, req.ctx, logger, req.language(), []string{id}, false, 0, false, false) if jerr != nil { return req.errorResponseFromJmap(single(accountId), jerr) } @@ -302,7 +303,7 @@ func (g *Groupware) GetEmailAttachments(w http.ResponseWriter, r *http.Request) l = contextAppender(l) logger := log.From(l) - emails, _, _, lang, jerr := g.jmap.GetEmails(mailAccountId, req.session, req.ctx, logger, req.language(), []string{id}, false, 0, false, false) + emails, _, _, _, lang, jerr := g.jmap.GetEmails(mailAccountId, req.session, req.ctx, logger, req.language(), []string{id}, false, 0, false, false) if jerr != nil { return req.apiErrorFromJmap(req.observeJmapError(jerr)) } @@ -744,6 +745,63 @@ func (g *Groupware) GetEmailsForAllAccounts(w http.ResponseWriter, r *http.Reque }) } +var draftEmailAutoMailboxRolePrecedence = []string{ + jmap.JmapMailboxRoleDrafts, // we want draft emails to be created in the Mailbox with the drafts role + jmap.JmapMailboxRoleInbox, // but if there is none, we will use the Mailbox with the inbox role instead +} + +func findDraftsMailboxId(j *jmap.Client, accountId string, req Request, logger *log.Logger) (string, Response) { + mailboxIdsPerAccountIds, _, _, _, jerr := j.SearchMailboxIdsPerRole(single(accountId), req.session, req.ctx, logger, req.language(), draftEmailAutoMailboxRolePrecedence) + if jerr != nil { + return "", req.errorResponseFromJmap(single(accountId), jerr) + } else { + for _, role := range draftEmailAutoMailboxRolePrecedence { + if mailboxId, ok := mailboxIdsPerAccountIds[accountId][role]; ok { + return mailboxId, Response{} + } + } + // couldn't find a Mailbox with the drafts role for that account, + // we have to return an error... ? + return "", errorResponse(single(accountId), apiError(req.errorId(), ErrorNoMailboxWithDraftRole)) + } +} + +var sentEmailAutoMailboxRolePrecedence = []string{ + jmap.JmapMailboxRoleSent, // we want sent emails to be created in the Mailbox with the sent role + jmap.JmapMailboxRoleInbox, // but if there is none, we will use the Mailbox with the inbox role instead +} + +var draftAndSentMailboxRoles = structs.Uniq(structs.Concat(draftEmailAutoMailboxRolePrecedence, sentEmailAutoMailboxRolePrecedence)) + +func findSentMailboxId(j *jmap.Client, accountId string, req Request, logger *log.Logger) (string, string, Response) { + mailboxIdsPerAccountIds, _, _, _, jerr := j.SearchMailboxIdsPerRole(single(accountId), req.session, req.ctx, logger, req.language(), draftAndSentMailboxRoles) + if jerr != nil { + return "", "", req.errorResponseFromJmap(single(accountId), jerr) + } else { + sentMailboxId := "" + for _, role := range sentEmailAutoMailboxRolePrecedence { + if mailboxId, ok := mailboxIdsPerAccountIds[accountId][role]; ok { + sentMailboxId = mailboxId + break + } + } + if sentMailboxId == "" { + return "", "", errorResponse(single(accountId), apiError(req.errorId(), ErrorNoMailboxWithSentRole)) + } + draftsMailboxId := "" + for _, role := range draftEmailAutoMailboxRolePrecedence { + if mailboxId, ok := mailboxIdsPerAccountIds[accountId][role]; ok { + draftsMailboxId = mailboxId + break + } + } + if draftsMailboxId == "" { + return "", "", errorResponse(single(accountId), apiError(req.errorId(), ErrorNoMailboxWithDraftRole)) + } + return draftsMailboxId, sentMailboxId, Response{} + } +} + func (g *Groupware) CreateEmail(w http.ResponseWriter, r *http.Request) { g.respond(w, r, func(req Request) Response { logger := req.logger @@ -760,6 +818,15 @@ func (g *Groupware) CreateEmail(w http.ResponseWriter, r *http.Request) { return errorResponse(single(accountId), err) } + if len(body.MailboxIds) < 1 { + mailboxId, resp := findDraftsMailboxId(g.jmap, accountId, req, logger) + if mailboxId != "" { + body.MailboxIds[mailboxId] = true + } else { + return resp + } + } + created, sessionState, state, lang, jerr := g.jmap.CreateEmail(accountId, body, "", req.session, req.ctx, logger, req.language()) if jerr != nil { return req.errorResponseFromJmap(single(accountId), jerr) @@ -788,6 +855,15 @@ func (g *Groupware) ReplaceEmail(w http.ResponseWriter, r *http.Request) { return errorResponse(single(accountId), err) } + if len(body.MailboxIds) < 1 { + mailboxId, resp := findDraftsMailboxId(g.jmap, accountId, req, logger) + if mailboxId != "" { + body.MailboxIds[mailboxId] = true + } else { + return resp + } + } + created, sessionState, state, lang, jerr := g.jmap.CreateEmail(accountId, body, replaceId, req.session, req.ctx, logger, req.language()) if jerr != nil { return req.errorResponseFromJmap(single(accountId), jerr) @@ -1174,22 +1250,25 @@ func (g *Groupware) SendEmail(w http.ResponseWriter, r *http.Request) { { moveFromMailboxId, _ := req.getStringParam(QueryParamMoveFromMailboxId, "") moveToMailboxId, _ := req.getStringParam(QueryParamMoveToMailboxId, "") - if moveFromMailboxId != "" && moveToMailboxId != "" { - move = &jmap.MoveMail{FromMailboxId: moveFromMailboxId, ToMailboxId: moveToMailboxId} - l.Str(QueryParamMoveFromMailboxId, log.SafeString(moveFromMailboxId)).Str(QueryParamMoveToMailboxId, log.SafeString(moveFromMailboxId)) - } else if moveFromMailboxId == "" && moveToMailboxId == "" { - // nothing to change - } else { - missing := moveFromMailboxId - if moveFromMailboxId == "" { - missing = moveFromMailboxId + + if moveFromMailboxId == "" || moveToMailboxId == "" { + draftsMailboxId, sentMailboxId, resp := findSentMailboxId(g.jmap, accountId, req, req.logger) + if draftsMailboxId != "" && sentMailboxId != "" { + if moveFromMailboxId == "" { + moveFromMailboxId = draftsMailboxId + } + if moveToMailboxId == "" { + moveToMailboxId = sentMailboxId + } + } else { + return resp } - // only one is set - msg := fmt.Sprintf("Missing required value for query parameter '%v'", missing) - return errorResponse(single(accountId), req.observedParameterError(ErrorMissingMandatoryRequestParameter, - withDetail(msg), - withSource(&ErrorSource{Parameter: missing}))) } + + // TODO some parameter to prevent moving the sent email from one Mailbox to another? + + move = &jmap.MoveMail{FromMailboxId: moveFromMailboxId, ToMailboxId: moveToMailboxId} + l.Str(QueryParamMoveFromMailboxId, log.SafeString(moveFromMailboxId)).Str(QueryParamMoveToMailboxId, log.SafeString(moveFromMailboxId)) } logger := log.From(l) @@ -1285,7 +1364,7 @@ func (g *Groupware) RelatedToEmail(w http.ResponseWriter, r *http.Request) { reqId := req.GetRequestId() getEmailsBefore := time.Now() - emails, sessionState, state, lang, jerr := g.jmap.GetEmails(accountId, req.session, req.ctx, logger, req.language(), []string{id}, true, g.maxBodyValueBytes, false, false) + emails, _, sessionState, state, lang, jerr := g.jmap.GetEmails(accountId, req.session, req.ctx, logger, req.language(), []string{id}, true, g.maxBodyValueBytes, false, false) getEmailsDuration := time.Since(getEmailsBefore) if jerr != nil { return req.errorResponseFromJmap(single(accountId), jerr) diff --git a/services/groupware/pkg/groupware/groupware_error.go b/services/groupware/pkg/groupware/groupware_error.go index 4db41492d1..1c1d177abd 100644 --- a/services/groupware/pkg/groupware/groupware_error.go +++ b/services/groupware/pkg/groupware/groupware_error.go @@ -199,6 +199,8 @@ const ( ErrorCodeFailedToDeleteSomeIdentities = "DELSID" ErrorCodeFailedToSanitizeEmail = "FSANEM" ErrorCodeFailedToDeleteContact = "DELCNT" + ErrorCodeNoMailboxWithDraftRole = "NMBXDR" + ErrorCodeNoMailboxWithSentRole = "NMBXSE" ) var ( @@ -448,6 +450,18 @@ var ( Title: "Failed to delete contacts", Detail: "One or more contacts could not be deleted.", } + ErrorNoMailboxWithDraftRole = GroupwareError{ + Status: http.StatusExpectationFailed, + Code: ErrorCodeNoMailboxWithDraftRole, + Title: "Failed to find a Mailbox with the drafts role", + Detail: "We could not find a Mailbox that has the drafts role to store a draft email in.", + } + ErrorNoMailboxWithSentRole = GroupwareError{ + Status: http.StatusExpectationFailed, + Code: ErrorCodeNoMailboxWithSentRole, + Title: "Failed to find a Mailbox with the sent role", + Detail: "We could not find a Mailbox that has the sent role to store a sent email in.", + } ) type ErrorOpt interface {