diff --git a/changelog/unreleased/spaces-filters.md b/changelog/unreleased/spaces-filters.md new file mode 100644 index 0000000000..7b3f675e3f --- /dev/null +++ b/changelog/unreleased/spaces-filters.md @@ -0,0 +1,6 @@ +Enhancement: Add filter by driveType and id to /me/drives + +We added two possible filter terms (driveType, id) to the /me/drives endpoint on the graph api. These can be used with the odata query parameter "$filter". +We only support the "eq" operator for now. + +https://github.com/owncloud/ocis/pull/2946 diff --git a/graph/pkg/service/v0/drives.go b/graph/pkg/service/v0/drives.go index 12059dd0b7..bfed8d4dcc 100644 --- a/graph/pkg/service/v0/drives.go +++ b/graph/pkg/service/v0/drives.go @@ -12,6 +12,7 @@ import ( "strings" "time" + "github.com/CiscoM31/godata" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" cs3rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" @@ -31,6 +32,14 @@ import ( // GetDrives implements the Service interface. func (g Graph) GetDrives(w http.ResponseWriter, r *http.Request) { + sanitizedPath := strings.TrimPrefix(r.URL.Path, "/graph/v1.0/") + // Parse the request with odata parser + odataReq, err := godata.ParseRequest(r.Context(), sanitizedPath, r.URL.Query()) + if err != nil { + g.logger.Err(err).Interface("query", r.URL.Query()).Msg("query error") + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error()) + return + } g.logger.Info().Msg("Calling GetDrives") ctx := r.Context() @@ -57,6 +66,12 @@ func (g Graph) GetDrives(w http.ResponseWriter, r *http.Request) { errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) return } + filters, err := generateCs3Filters(odataReq) + if err != nil { + g.logger.Err(err).Interface("query", r.URL.Query()).Msg("query error") + errorcode.NotSupported.Render(w, r, http.StatusNotImplemented, err.Error()) + return + } res, err := client.ListStorageSpaces(ctx, &storageprovider.ListStorageSpacesRequest{ Opaque: &types.Opaque{Map: map[string]*types.OpaqueEntry{ "permissions": { @@ -64,7 +79,7 @@ func (g Graph) GetDrives(w http.ResponseWriter, r *http.Request) { Value: value, }, }}, - // TODO add filters? + Filters: filters, }) switch { case err != nil: @@ -553,3 +568,35 @@ func canSetSpaceQuota(ctx context.Context, user *userv1beta1.User) (bool, error) } return true, nil } + +func generateCs3Filters(request *godata.GoDataRequest) ([]*storageprovider.ListStorageSpacesRequest_Filter, error) { + var filters []*storageprovider.ListStorageSpacesRequest_Filter + if request.Query.Filter != nil { + if request.Query.Filter.Tree.Token.Value == "eq" { + switch request.Query.Filter.Tree.Children[0].Token.Value { + case "driveType": + filter1 := &storageprovider.ListStorageSpacesRequest_Filter{ + Type: storageprovider.ListStorageSpacesRequest_Filter_TYPE_SPACE_TYPE, + Term: &storageprovider.ListStorageSpacesRequest_Filter_SpaceType{ + SpaceType: strings.Trim(request.Query.Filter.Tree.Children[1].Token.Value, "'"), + }, + } + filters = append(filters, filter1) + case "id": + filter2 := &storageprovider.ListStorageSpacesRequest_Filter{ + Type: storageprovider.ListStorageSpacesRequest_Filter_TYPE_ID, + Term: &storageprovider.ListStorageSpacesRequest_Filter_Id{ + Id: &storageprovider.StorageSpaceId{ + OpaqueId: strings.Trim(request.Query.Filter.Tree.Children[1].Token.Value, "'"), + }, + }, + } + filters = append(filters, filter2) + } + } else { + err := fmt.Errorf("unsupported filter operand: %s", request.Query.Filter.Tree.Token.Value) + return nil, err + } + } + return filters, nil +} diff --git a/tests/acceptance/features/apiSpaces/listSpaces.feature b/tests/acceptance/features/apiSpaces/listSpaces.feature index 8705184e30..175c562cc7 100644 --- a/tests/acceptance/features/apiSpaces/listSpaces.feature +++ b/tests/acceptance/features/apiSpaces/listSpaces.feature @@ -19,6 +19,31 @@ Feature: List and create spaces | name | Alice Hansen | | quota@@@state | normal | | root@@@webDavUrl | %base_url%/dav/spaces/%space_id% | + And the json responded should contain a space "Shares Jail" with these key and value pairs: + | key | value | + | driveType | virtual | + | id | %space_id% | + | name | Shares Jail | + | root@@@webDavUrl | %base_url%/dav/spaces/%space_id% | + + Scenario: An ordinary user can request information about their Space via the Graph API using a filter + When user "Alice" lists all available spaces via the GraphApi with query "$filter=driveType eq 'personal'" + Then the HTTP status code should be "200" + And the json responded should contain a space "Alice Hansen" with these key and value pairs: + | key | value | + | driveType | personal | + | id | %space_id% | + | name | Alice Hansen | + | quota@@@state | normal | + | root@@@webDavUrl | %base_url%/dav/spaces/%space_id% | + And the json responded should not contain a space with name "Shares Jail" + And the json responded should only contain spaces of type "personal" + + Scenario: An ordinary user will not see any space when using a filter for project + When user "Alice" lists all available spaces via the GraphApi with query "$filter=driveType eq 'project'" + Then the HTTP status code should be "200" + And the json responded should not contain a space with name "Alice Hansen" + And the json responded should not contain spaces of type "personal" Scenario: An ordinary user can access their Space via the webDav API When user "Alice" lists all available spaces via the GraphApi diff --git a/tests/acceptance/features/bootstrap/SpacesContext.php b/tests/acceptance/features/bootstrap/SpacesContext.php index 7de32c7e3b..c5d5a220ae 100644 --- a/tests/acceptance/features/bootstrap/SpacesContext.php +++ b/tests/acceptance/features/bootstrap/SpacesContext.php @@ -350,6 +350,26 @@ class SpacesContext implements Context { $this->rememberTheAvailableSpaces(); } + /** + * @When /^user "([^"]*)" lists all available spaces via the GraphApi with query "([^"]*)"$/ + * + * @param string $user + * @param string $query + * + * @return void + * + * @throws GuzzleException + */ + public function theUserListsAllHisAvailableSpacesUsingTheGraphApiWithFilter(string $user, string $query): void { + $this->featureContext->setResponse( + $this->listSpacesRequest( + $user, + $this->featureContext->getPasswordForUser($user), + "?". $query + ) + ); + } + /** * @When /^user "([^"]*)" creates a space "([^"]*)" of type "([^"]*)" with the default quota using the GraphApi$/ * @@ -659,7 +679,7 @@ class SpacesContext implements Context { } /** - * @Then /^the json responded should not contain a space "([^"]*)"$/ + * @Then /^the json responded should not contain a space with name "([^"]*)"$/ * * @param string $spaceName * @@ -672,6 +692,45 @@ class SpacesContext implements Context { Assert::assertEmpty($this->getSpaceByNameFromResponse($spaceName), "space $spaceName should not be available for a user"); } + /** + * @Then /^the json responded should (not|only|)\s?contain spaces of type "([^"]*)"$/ + * + * @param string $shoulOrNot (not|only|) + * @param string $type + * + * @return void + * @throws Exception + */ + public function jsonRespondedShouldNotContainSpaceType( + string $onlyOrNot, + string $type + ): void { + Assert::assertNotEmpty( + $spaces = json_decode( + (string)$this->featureContext + ->getResponse()->getBody(), + true, + 512, + JSON_THROW_ON_ERROR + ) + ); + $matches = []; + foreach($spaces["value"] as $space) { + if($onlyOrNot === "not") { + Assert::assertNotEquals($space["driveType"], $type); + } + if($onlyOrNot === "only") { + Assert::assertEquals($space["driveType"], $type); + } + if($onlyOrNot === "" && $space["driveType"] === $type) { + $matches[] = $space; + } + } + if($onlyOrNot === "") { + Assert::assertNotEmpty($matches); + } + } + /** * @param string $shouldOrNot (not|) * @param TableNode $expectedFiles