From 9f0cf5ef4d1c17fbecd2727fd8935560e94a035d Mon Sep 17 00:00:00 2001 From: Pascal Bleser Date: Thu, 19 Mar 2026 12:09:56 +0100 Subject: [PATCH] groupware: add contact sorting query parameter and fix default sorting (must use updated instead of name) --- .../groupware/pkg/groupware/api_contacts.go | 40 +++++++++--- services/groupware/pkg/groupware/error.go | 16 ++++- .../groupware/{test.go => framework_test.go} | 0 services/groupware/pkg/groupware/request.go | 54 ++++++++++++++++ .../groupware/pkg/groupware/request_test.go | 64 +++++++++++++++++++ services/groupware/pkg/groupware/route.go | 1 + 6 files changed, 164 insertions(+), 11 deletions(-) rename services/groupware/pkg/groupware/{test.go => framework_test.go} (100%) create mode 100644 services/groupware/pkg/groupware/request_test.go diff --git a/services/groupware/pkg/groupware/api_contacts.go b/services/groupware/pkg/groupware/api_contacts.go index 1e92447851..566b3fa0f9 100644 --- a/services/groupware/pkg/groupware/api_contacts.go +++ b/services/groupware/pkg/groupware/api_contacts.go @@ -8,6 +8,18 @@ import ( "github.com/opencloud-eu/opencloud/pkg/log" ) +var ( + /* + DefaultContactSort = []jmap.ContactCardComparator{ + {Property: string(jscontact.ContactCardPropertyName) + "/surname", IsAscending: true}, + {Property: string(jscontact.ContactCardPropertyName) + "/given", IsAscending: true}, + } + */ + DefaultContactSort = []jmap.ContactCardComparator{ + {Property: jscontact.ContactCardPropertyUpdated, IsAscending: true}, + } +) + // Get all addressbooks of an account. func (g *Groupware) GetAddressbooks(w http.ResponseWriter, r *http.Request) { g.respond(w, r, func(req Request) Response { @@ -63,18 +75,19 @@ func (g *Groupware) GetContactsInAddressbook(w http.ResponseWriter, r *http.Requ if !ok { return resp } + accountIds := single(accountId) l := req.logger.With() addressBookId, err := req.PathParam(UriParamAddressBookId) if err != nil { - return errorResponse(single(accountId), err) + return errorResponseWithSessionState(accountIds, err, req.session.State) } l = l.Str(UriParamAddressBookId, log.SafeString(addressBookId)) offset, ok, err := req.parseUIntParam(QueryParamOffset, 0) if err != nil { - return errorResponse(single(accountId), err) + return errorResponseWithSessionState(accountIds, err, req.session.State) } if ok { l = l.Uint(QueryParamOffset, offset) @@ -82,7 +95,7 @@ func (g *Groupware) GetContactsInAddressbook(w http.ResponseWriter, r *http.Requ limit, ok, err := req.parseUIntParam(QueryParamLimit, g.defaults.contactLimit) if err != nil { - return errorResponse(single(accountId), err) + return errorResponseWithSessionState(accountIds, err, req.session.State) } if ok { l = l.Uint(QueryParamLimit, limit) @@ -91,20 +104,23 @@ func (g *Groupware) GetContactsInAddressbook(w http.ResponseWriter, r *http.Requ filter := jmap.ContactCardFilterCondition{ InAddressBook: addressBookId, } - sortBy := []jmap.ContactCardComparator{{ - Property: jscontact.ContactCardPropertyName, IsAscending: true, - }} + var sortBy []jmap.ContactCardComparator + if sort, ok, resp := mapSort(accountIds, &req, DefaultContactSort, jscontact.ContactCardProperties, mapContactCardSort); !ok { + return resp + } else { + sortBy = sort + } logger := log.From(l) - contactsByAccountId, sessionState, state, lang, jerr := g.jmap.QueryContactCards(single(accountId), req.session, req.ctx, logger, req.language(), filter, sortBy, offset, limit) + contactsByAccountId, sessionState, state, lang, jerr := g.jmap.QueryContactCards(accountIds, req.session, req.ctx, logger, req.language(), filter, sortBy, offset, limit) if jerr != nil { - return req.errorResponseFromJmap(single(accountId), jerr) + return req.errorResponseFromJmap(accountIds, jerr) } if contacts, ok := contactsByAccountId[accountId]; ok { - return etagResponse(single(accountId), contacts, sessionState, ContactResponseObjectType, state, lang) + return etagResponse(accountIds, contacts, sessionState, ContactResponseObjectType, state, lang) } else { - return etagNotFoundResponse(single(accountId), sessionState, ContactResponseObjectType, state, lang) + return etagNotFoundResponse(accountIds, sessionState, ContactResponseObjectType, state, lang) } }) } @@ -207,3 +223,7 @@ func (g *Groupware) DeleteContact(w http.ResponseWriter, r *http.Request) { return noContentResponseWithEtag(single(accountId), sessionState, ContactResponseObjectType, state) }) } + +func mapContactCardSort(s SortCrit) jmap.ContactCardComparator { + return jmap.ContactCardComparator{Property: s.Attribute, IsAscending: s.Ascending} +} diff --git a/services/groupware/pkg/groupware/error.go b/services/groupware/pkg/groupware/error.go index f5b3666ec9..b1a035c522 100644 --- a/services/groupware/pkg/groupware/error.go +++ b/services/groupware/pkg/groupware/error.go @@ -201,6 +201,8 @@ const ( ErrorCodeFailedToDeleteContact = "DELCNT" ErrorCodeNoMailboxWithDraftRole = "NMBXDR" ErrorCodeNoMailboxWithSentRole = "NMBXSE" + ErrorCodeInvalidSortSpecification = "INVSSP" + ErrorCodeInvalidSortProperty = "INVSPR" ) var ( @@ -456,6 +458,18 @@ var ( 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.", } + ErrorInvalidSortSpecification = GroupwareError{ + Status: http.StatusBadRequest, + Code: ErrorCodeInvalidSortSpecification, + Title: "Invalid sort specification", + Detail: "The sort specification in the query parameter does not comply with the expected format.", + } + ErrorInvalidSortProperty = GroupwareError{ + Status: http.StatusBadRequest, + Code: ErrorCodeInvalidSortProperty, + Title: "Invalid sort property", + Detail: "The sort property in the query parameter does not exist or is not acceptable.", + } ) type ErrorOpt interface { @@ -638,5 +652,5 @@ func errorResponses(errors ...Error) ErrorResponse { } func (r *Request) errorResponseFromJmap(accountIds []string, err jmap.Error) Response { - return errorResponse(accountIds, r.apiErrorFromJmap(r.observeJmapError(err))) + return errorResponseWithSessionState(accountIds, r.apiErrorFromJmap(r.observeJmapError(err)), r.session.State) } diff --git a/services/groupware/pkg/groupware/test.go b/services/groupware/pkg/groupware/framework_test.go similarity index 100% rename from services/groupware/pkg/groupware/test.go rename to services/groupware/pkg/groupware/framework_test.go diff --git a/services/groupware/pkg/groupware/request.go b/services/groupware/pkg/groupware/request.go index b14df9e38a..26ce6f0f19 100644 --- a/services/groupware/pkg/groupware/request.go +++ b/services/groupware/pkg/groupware/request.go @@ -28,6 +28,8 @@ import ( const ( // TODO remove this once Stalwart has actual support for Tasks and we don't need to mock it any more IgnoreSessionCapabilityChecksForTasks = true + + MaxSortParams = 16 ) // using a wrapper class for requests, to group multiple parameters, really to avoid crowding the @@ -519,3 +521,55 @@ func (r *Request) needContactWithAccount() (bool, string, Response) { } return true, accountId, Response{} } + +type SortCrit struct { + Attribute string + Ascending bool +} + +func (r *Request) parseSort(s string, props []string) ([]SortCrit, *Error) { + parts := strings.SplitN(s, ",", MaxSortParams) + result := []SortCrit{} + for _, part := range parts { + name := strings.TrimSpace(part) + if name == "" { + continue + } + + asc := true + i := strings.LastIndex(name, ":") + if i == 0 { + // invalid spec, e.g. ':asc' + return nil, r.apiError(&ErrorInvalidSortProperty) + } else if i > 0 { + order := name[i+1:] + name = name[0:i] + switch order { + case "", "asc": + asc = true + case "desc": + asc = false + default: + return nil, r.apiError(&ErrorInvalidSortSpecification) + } + } + if len(props) > 0 && !slices.Contains(props, name) { + return nil, r.apiError(&ErrorInvalidSortProperty) + } else { + result = append(result, SortCrit{Attribute: name, Ascending: asc}) + } + } + return result, nil +} + +func mapSort[T any](accountIds []string, req *Request, defaultSort []T, props []string, mapper func(SortCrit) T) ([]T, bool, Response) { + if sortSpec, ok := req.getStringParam(QueryParamSort, ""); ok && strings.TrimSpace(sortSpec) != "" { + if sort, err := req.parseSort(sortSpec, props); err != nil { + return nil, false, errorResponseWithSessionState(accountIds, err, req.session.State) + } else { + return structs.Map(sort, mapper), true, Response{} + } + } else { + return defaultSort, true, Response{} + } +} diff --git a/services/groupware/pkg/groupware/request_test.go b/services/groupware/pkg/groupware/request_test.go new file mode 100644 index 0000000000..975cd03f2f --- /dev/null +++ b/services/groupware/pkg/groupware/request_test.go @@ -0,0 +1,64 @@ +package groupware + +import ( + "context" + "net/http" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestParseSort(t *testing.T) { + req := Request{ + r: &http.Request{}, + ctx: context.Background(), + } + require := require.New(t) + { + res, err := req.parseSort("name", []string{"name", "time"}) + require.Nil(err) + require.Len(res, 1) + require.Equal("name", res[0].Attribute) + require.True(res[0].Ascending) + } + { + res, err := req.parseSort("name:asc", []string{"name"}) + require.Nil(err) + require.Len(res, 1) + require.Equal("name", res[0].Attribute) + require.True(res[0].Ascending) + } + { + res, err := req.parseSort("name:desc", []string{"name"}) + require.Nil(err) + require.Len(res, 1) + require.Equal("name", res[0].Attribute) + require.False(res[0].Ascending) + } + { + res, err := req.parseSort("name:", []string{"name"}) + require.Nil(err) + require.Len(res, 1) + require.Equal("name", res[0].Attribute) + require.True(res[0].Ascending) + } + { + _, err := req.parseSort("name:xyz", []string{"name"}) + require.NotNil(err) + require.Equal(ErrorCodeInvalidSortSpecification, err.Code) + } + { + _, err := req.parseSort("age", []string{"name"}) + require.NotNil(err) + require.Equal(ErrorCodeInvalidSortProperty, err.Code) + } + { + res, err := req.parseSort("name:asc,updated:desc", []string{"name", "updated"}) + require.Nil(err) + require.Len(res, 2) + require.Equal("name", res[0].Attribute) + require.True(res[0].Ascending) + require.Equal("updated", res[1].Attribute) + require.False(res[1].Ascending) + } +} diff --git a/services/groupware/pkg/groupware/route.go b/services/groupware/pkg/groupware/route.go index 0184c5741a..48ebe5b55b 100644 --- a/services/groupware/pkg/groupware/route.go +++ b/services/groupware/pkg/groupware/route.go @@ -58,6 +58,7 @@ const ( QueryParamSeen = "seen" QueryParamUndesirable = "undesirable" QueryParamMarkAsSeen = "markAsSeen" + QueryParamSort = "sort" HeaderParamSince = "if-none-match" )