diff --git a/changelog/unreleased/add-ordering-list-drives.md b/changelog/unreleased/add-ordering-list-drives.md new file mode 100644 index 00000000000..7ec005ee26e --- /dev/null +++ b/changelog/unreleased/add-ordering-list-drives.md @@ -0,0 +1,9 @@ +Enhancement: Add sorting to list Spaces + +We added the OData query param "orderBy" for listing spaces. We can now order by Space Name and LastModifiedDateTime. + +Example 1: https://localhost:9200/graph/v1.0/me/drives/?$orderby=lastModifiedDateTime desc +Example 2: https://localhost:9200/graph/v1.0/me/drives/?$orderby=name asc + +https://github.com/owncloud/ocis/issues/3200 +https://github.com/owncloud/ocis/pull/3201 diff --git a/graph/pkg/service/v0/drives.go b/graph/pkg/service/v0/drives.go index 24104af77e9..b5b185be256 100644 --- a/graph/pkg/service/v0/drives.go +++ b/graph/pkg/service/v0/drives.go @@ -7,6 +7,7 @@ import ( "math" "net/http" "net/url" + "sort" "strconv" "strings" @@ -72,15 +73,22 @@ func (g Graph) GetDrives(w http.ResponseWriter, r *http.Request) { errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) return } - files, err := g.formatDrives(ctx, wdu, res.StorageSpaces) + spaces, err := g.formatDrives(ctx, wdu, res.StorageSpaces) if err != nil { g.logger.Error().Err(err).Msg("error encoding response as json") errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) return } + spaces, err = sortSpaces(odataReq, spaces) + if err != nil { + g.logger.Error().Err(err).Msg("error sorting the spaces list") + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error()) + return + } + render.Status(r, http.StatusOK) - render.JSON(w, r, &listResponse{Value: files}) + render.JSON(w, r, &listResponse{Value: spaces}) } // GetSingleDrive does a lookup of a single space by spaceId @@ -710,3 +718,24 @@ func (g Graph) DeleteDrive(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) } + +func sortSpaces(req *godata.GoDataRequest, spaces []*libregraph.Drive) ([]*libregraph.Drive, error) { + var sorter sort.Interface + if req.Query.OrderBy == nil || len(req.Query.OrderBy.OrderByItems) != 1 { + return spaces, nil + } + switch req.Query.OrderBy.OrderByItems[0].Field.Value { + case "name": + sorter = spacesByName{spaces} + case "lastModifiedDateTime": + sorter = spacesByLastModifiedDateTime{spaces} + default: + return nil, fmt.Errorf("we do not support <%s> as a order parameter", req.Query.OrderBy.OrderByItems[0].Field.Value) + } + + if req.Query.OrderBy.OrderByItems[0].Order == "asc" { + sorter = sort.Reverse(sorter) + } + sort.Sort(sorter) + return spaces, nil +} diff --git a/graph/pkg/service/v0/drives_test.go b/graph/pkg/service/v0/drives_test.go new file mode 100644 index 00000000000..036ca90f5dc --- /dev/null +++ b/graph/pkg/service/v0/drives_test.go @@ -0,0 +1,123 @@ +package svc + +import ( + "testing" + "time" + + "github.com/CiscoM31/godata" + libregraph "github.com/owncloud/libre-graph-api-go" + "github.com/stretchr/testify/assert" +) + +type sortTest struct { + Drives []*libregraph.Drive + Query godata.GoDataRequest + DrivesSorted []*libregraph.Drive +} + +var time1 = time.Date(2022, 02, 02, 15, 00, 00, 00, time.UTC) +var time2 = time.Date(2022, 02, 03, 15, 00, 00, 00, time.UTC) +var time3, time5, time6 *time.Time +var time4 = time.Date(2022, 02, 05, 15, 00, 00, 00, time.UTC) +var drives = []*libregraph.Drive{ + drive("3", "project", "Admin", time3), + drive("1", "project", "Einstein", &time1), + drive("2", "project", "Marie", &time2), + drive("4", "project", "Richard", &time4), +} +var drivesLong = append(drives, []*libregraph.Drive{ + drive("5", "project", "Bob", time5), + drive("6", "project", "Alice", time6), +}...) + +var sortTests = []sortTest{ + { + Drives: drives, + Query: godata.GoDataRequest{ + Query: &godata.GoDataQuery{ + OrderBy: &godata.GoDataOrderByQuery{ + OrderByItems: []*godata.OrderByItem{ + {Field: &godata.Token{Value: "name"}, Order: "asc"}, + }, + }, + }, + }, + DrivesSorted: []*libregraph.Drive{ + drive("3", "project", "Admin", time3), + drive("1", "project", "Einstein", &time1), + drive("2", "project", "Marie", &time2), + drive("4", "project", "Richard", &time4), + }, + }, + { + Drives: drives, + Query: godata.GoDataRequest{ + Query: &godata.GoDataQuery{ + OrderBy: &godata.GoDataOrderByQuery{ + OrderByItems: []*godata.OrderByItem{ + {Field: &godata.Token{Value: "name"}, Order: "desc"}, + }, + }, + }, + }, + DrivesSorted: []*libregraph.Drive{ + drive("4", "project", "Richard", &time4), + drive("2", "project", "Marie", &time2), + drive("1", "project", "Einstein", &time1), + drive("3", "project", "Admin", time3), + }, + }, + { + Drives: drivesLong, + Query: godata.GoDataRequest{ + Query: &godata.GoDataQuery{ + OrderBy: &godata.GoDataOrderByQuery{ + OrderByItems: []*godata.OrderByItem{ + {Field: &godata.Token{Value: "lastModifiedDateTime"}, Order: "asc"}, + }, + }, + }, + }, + DrivesSorted: []*libregraph.Drive{ + drive("3", "project", "Admin", time3), + drive("6", "project", "Alice", time6), + drive("5", "project", "Bob", time5), + drive("1", "project", "Einstein", &time1), + drive("2", "project", "Marie", &time2), + drive("4", "project", "Richard", &time4), + }, + }, + { + Drives: drivesLong, + Query: godata.GoDataRequest{ + Query: &godata.GoDataQuery{ + OrderBy: &godata.GoDataOrderByQuery{ + OrderByItems: []*godata.OrderByItem{ + {Field: &godata.Token{Value: "lastModifiedDateTime"}, Order: "desc"}, + }, + }, + }, + }, + DrivesSorted: []*libregraph.Drive{ + drive("4", "project", "Richard", &time4), + drive("2", "project", "Marie", &time2), + drive("1", "project", "Einstein", &time1), + drive("5", "project", "Bob", time5), + drive("6", "project", "Alice", time6), + drive("3", "project", "Admin", time3), + }, + }, +} + +func drive(ID string, dType string, name string, lastModified *time.Time) *libregraph.Drive { + return &libregraph.Drive{Id: libregraph.PtrString(ID), DriveType: libregraph.PtrString(dType), Name: libregraph.PtrString(name), LastModifiedDateTime: lastModified} +} + +// TestSort tests the available orderby queries +func TestSort(t *testing.T) { + for _, test := range sortTests { + sorted, err := sortSpaces(&test.Query, test.Drives) + assert.NoError(t, err) + assert.Equal(t, test.DrivesSorted, sorted) + } +} diff --git a/graph/pkg/service/v0/graph_test.go b/graph/pkg/service/v0/graph_test.go index 976f4509aa6..3d446bd74a0 100644 --- a/graph/pkg/service/v0/graph_test.go +++ b/graph/pkg/service/v0/graph_test.go @@ -2,6 +2,7 @@ package svc_test import ( "context" + "encoding/json" "fmt" "io" "net/http" @@ -12,9 +13,11 @@ import ( "github.com/cs3org/reva/pkg/rgrpc/status" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + libregraph "github.com/owncloud/libre-graph-api-go" "github.com/owncloud/ocis/graph/mocks" "github.com/owncloud/ocis/graph/pkg/config" service "github.com/owncloud/ocis/graph/pkg/service/v0" + "github.com/owncloud/ocis/graph/pkg/service/v0/errorcode" "github.com/stretchr/testify/mock" ) @@ -101,5 +104,114 @@ var _ = Describe("Graph", func() { } `)) }) + It("can list a spaces with sort", func() { + gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(&provider.ListStorageSpacesResponse{ + Status: status.NewOK(ctx), + StorageSpaces: []*provider.StorageSpace{ + { + Id: &provider.StorageSpaceId{OpaqueId: "bspaceid"}, + SpaceType: "bspacetype", + Root: &provider.ResourceId{ + StorageId: "bspaceid", + OpaqueId: "bopaqueid", + }, + Name: "bspacename", + }, + { + Id: &provider.StorageSpaceId{OpaqueId: "aspaceid"}, + SpaceType: "aspacetype", + Root: &provider.ResourceId{ + StorageId: "aspaceid", + OpaqueId: "anopaqueid", + }, + Name: "aspacename", + }, + }, + }, nil) + gatewayClient.On("InitiateFileDownload", mock.Anything, mock.Anything).Return(&gateway.InitiateFileDownloadResponse{ + Status: status.NewNotFound(ctx, "not found"), + }, nil) + gatewayClient.On("GetQuota", mock.Anything, mock.Anything).Return(&provider.GetQuotaResponse{ + Status: status.NewUnimplemented(ctx, fmt.Errorf("not supported"), "not supported"), + }, nil) + + r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/drives?$orderby=name%20asc", nil) + rr := httptest.NewRecorder() + svc.GetDrives(rr, r) + + Expect(rr.Code).To(Equal(http.StatusOK)) + + body, _ := io.ReadAll(rr.Body) + Expect(body).To(MatchJSON(` + { + "value":[ + { + "driveType":"aspacetype", + "id":"aspaceid", + "name":"aspacename", + "root":{ + "id":"aspaceid!anopaqueid", + "webDavUrl":"https://localhost:9200/dav/spaces/aspaceid" + } + }, + { + "driveType":"bspacetype", + "id":"bspaceid", + "name":"bspacename", + "root":{ + "id":"bspaceid!bopaqueid", + "webDavUrl":"https://localhost:9200/dav/spaces/bspaceid" + } + } + ] + } + `)) + }) + It("can not list spaces with wrong sort parameter", func() { + gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(&provider.ListStorageSpacesResponse{ + Status: status.NewOK(ctx), + StorageSpaces: []*provider.StorageSpace{}}, nil) + gatewayClient.On("InitiateFileDownload", mock.Anything, mock.Anything).Return(&gateway.InitiateFileDownloadResponse{ + Status: status.NewNotFound(ctx, "not found"), + }, nil) + gatewayClient.On("GetQuota", mock.Anything, mock.Anything).Return(&provider.GetQuotaResponse{ + Status: status.NewUnimplemented(ctx, fmt.Errorf("not supported"), "not supported"), + }, nil) + + r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/drives?$orderby=owner%20asc", nil) + rr := httptest.NewRecorder() + svc.GetDrives(rr, r) + Expect(rr.Code).To(Equal(http.StatusBadRequest)) + + body, _ := io.ReadAll(rr.Body) + var libreError libregraph.OdataError + err := json.Unmarshal(body, &libreError) + Expect(err).To(Not(HaveOccurred())) + Expect(libreError.Error.Message).To(Equal("we do not support as a order parameter")) + Expect(libreError.Error.Code).To(Equal(errorcode.InvalidRequest.String())) + }) + It("can list a spaces with invalid query parameter", func() { + gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(&provider.ListStorageSpacesResponse{ + Status: status.NewOK(ctx), + StorageSpaces: []*provider.StorageSpace{}}, nil) + gatewayClient.On("InitiateFileDownload", mock.Anything, mock.Anything).Return(&gateway.InitiateFileDownloadResponse{ + Status: status.NewNotFound(ctx, "not found"), + }, nil) + gatewayClient.On("GetQuota", mock.Anything, mock.Anything).Return(&provider.GetQuotaResponse{ + Status: status.NewUnimplemented(ctx, fmt.Errorf("not supported"), "not supported"), + }, nil) + + r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/drives?§orderby=owner%20asc", nil) + rr := httptest.NewRecorder() + svc.GetDrives(rr, r) + Expect(rr.Code).To(Equal(http.StatusBadRequest)) + + body, _ := io.ReadAll(rr.Body) + var libreError libregraph.OdataError + err := json.Unmarshal(body, &libreError) + Expect(err).To(Not(HaveOccurred())) + Expect(libreError.Error.Message).To(Equal("Query parameter '§orderby' is not supported. Cause: Query parameter '§orderby' is not supported")) + Expect(libreError.Error.Code).To(Equal(errorcode.InvalidRequest.String())) + }) }) }) diff --git a/graph/pkg/service/v0/ordering.go b/graph/pkg/service/v0/ordering.go new file mode 100644 index 00000000000..7749cafb097 --- /dev/null +++ b/graph/pkg/service/v0/ordering.go @@ -0,0 +1,45 @@ +package svc + +import ( + libregraph "github.com/owncloud/libre-graph-api-go" +) + +type spacesSlice []*libregraph.Drive + +// Len is the number of elements in the collection. +func (d spacesSlice) Len() int { return len(d) } + +// Swap swaps the elements with indexes i and j. +func (d spacesSlice) Swap(i, j int) { d[i], d[j] = d[j], d[i] } + +type spacesByName struct { + spacesSlice +} +type spacesByLastModifiedDateTime struct { + spacesSlice +} + +// Less reports whether the element with index i +// must sort before the element with index j. +func (s spacesByName) Less(i, j int) bool { + return *s.spacesSlice[i].Name > *s.spacesSlice[j].Name +} + +// Less reports whether the element with index i +// must sort before the element with index j. +func (s spacesByLastModifiedDateTime) Less(i, j int) bool { + // compare the items when both dates are set + if s.spacesSlice[i].LastModifiedDateTime != nil && s.spacesSlice[j].LastModifiedDateTime != nil { + return s.spacesSlice[i].LastModifiedDateTime.After(*s.spacesSlice[j].LastModifiedDateTime) + } + // move left item down if it has no value + if s.spacesSlice[i].LastModifiedDateTime == nil && s.spacesSlice[j].LastModifiedDateTime != nil { + return false + } + // move right item down if it has no value + if s.spacesSlice[i].LastModifiedDateTime != nil && s.spacesSlice[j].LastModifiedDateTime == nil { + return true + } + // fallback to name if no dateTime is set on both items + return *s.spacesSlice[i].Name > *s.spacesSlice[j].Name +} diff --git a/sonar-project.properties b/sonar-project.properties index 776386c5bc7..ef6f15411af 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -38,3 +38,4 @@ sonar.go.golangci-lint.reportPaths=cache/checkstyle/accounts_checkstyle.xml,cach # Exclude files sonar.exclusions=**/third_party,docs/**,changelog/**,*/pkg/assets/embed.go,idp/assets/identifier/**,**/package.json,**/rollup.config.js,CHANGELOG.md,**/pkg/proto/**/*.pb.*,deployments/**,tests/**,vendor-bin/**,README.md,**/mocks/ sonar.coverage.exclusions=**/*_test.go +sonar.cpd.exclusions=**/*_test.go