Merge pull request #2946 from owncloud/spaces-filters

Spaces filters

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

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
+}

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

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