From 1874280a560805ccc5dfe498efba153e3ecee8a9 Mon Sep 17 00:00:00 2001 From: "sagargurung1001@gmail.com" Date: Mon, 19 Dec 2022 11:34:25 +0545 Subject: [PATCH] Addressed review --- tests/TestHelpers/GraphHelper.php | 25 +++---- ...ected-failures-localAPI-on-OCIS-storage.md | 2 +- .../features/apiGraph/getUser.feature | 68 +++++++------------ .../features/bootstrap/GraphContext.php | 50 +++++++------- 4 files changed, 60 insertions(+), 85 deletions(-) diff --git a/tests/TestHelpers/GraphHelper.php b/tests/TestHelpers/GraphHelper.php index 9b633505c..4d783da4c 100644 --- a/tests/TestHelpers/GraphHelper.php +++ b/tests/TestHelpers/GraphHelper.php @@ -40,7 +40,7 @@ class GraphHelper { * @return int (1 = true | 0 = false) */ public static function isUUIDv4(string $id): int { - $regex = "/^" . self::getUUIDv4Regex() . "/i"; + $regex = "/^" . self::getUUIDv4Regex() . "$/i"; return preg_match($regex, $id); } @@ -50,19 +50,10 @@ class GraphHelper { * @return int (1 = true | 0 = false) */ public static function isSpaceId(string $spaceId): int { - $regex = "/^" . self::getUUIDv4Regex() . '\\$' . self::getUUIDv4Regex() . "/i"; + $regex = "/^" . self::getUUIDv4Regex() . '\\$' . self::getUUIDv4Regex() . "$/i"; return preg_match($regex, $spaceId); } - /** - * @param string $id - * - * @return string - */ - public static function isUUIDv44(string $id): string { - return "hello"; - } - /** * @param string $baseUrl * @param string $path @@ -272,9 +263,9 @@ class GraphHelper { /** * @param string $baseUrl * @param string $xRequestId - * @param string $user + * @param string $byUser * @param string $userPassword - * @param string|null $ofUser + * @param string|null $user * * @return ResponseInterface * @throws GuzzleException @@ -282,15 +273,15 @@ class GraphHelper { public static function getUserWithDriveInformation( string $baseUrl, string $xRequestId, - string $user, + string $byUser, string $userPassword, - ?string $ofUser = null + ?string $user = null ): ResponseInterface { - $url = self::getFullUrl($baseUrl, 'users/' . $ofUser . '?%24select=&%24expand=drive'); + $url = self::getFullUrl($baseUrl, 'users/' . $user . '?%24select=&%24expand=drive'); return HttpRequestHelper::get( $url, $xRequestId, - $user, + $byUser, $userPassword, ); } diff --git a/tests/acceptance/expected-failures-localAPI-on-OCIS-storage.md b/tests/acceptance/expected-failures-localAPI-on-OCIS-storage.md index cdbbcc186..4073214d0 100644 --- a/tests/acceptance/expected-failures-localAPI-on-OCIS-storage.md +++ b/tests/acceptance/expected-failures-localAPI-on-OCIS-storage.md @@ -80,4 +80,4 @@ The expected failures in this file are from features in the owncloud/ocis repo. - [apiCors/cors.feature:68](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiCors/cors.feature#L68) #### [A User can get information of another user with Graph API](https://github.com/owncloud/ocis/issues/5125) -- [apiGraph/getUser.feature:21](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiGraph/getUser.feature#L21) +- [apiGraph/getUser.feature:23](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiGraph/getUser.feature#L23) diff --git a/tests/acceptance/features/apiGraph/getUser.feature b/tests/acceptance/features/apiGraph/getUser.feature index 96e1a41d2..4a849165a 100644 --- a/tests/acceptance/features/apiGraph/getUser.feature +++ b/tests/acceptance/features/apiGraph/getUser.feature @@ -5,57 +5,44 @@ Feature: get users So that I can see the information Background: - Given user "Alice" has been created with default attributes and without skeleton files + Given these users have been created with default attributes and without skeleton files: + | username | + | Alice | + | Brian | And the administrator has given "Alice" the role "Admin" using the settings api - Scenario: admin user tries get information of a user - Given user "Brian" has been created with default attributes and without skeleton files - When user "Alice" tries to get information of user "Brian" using Graph API + Scenario: admin user gets the information of a user + When user "Alice" gets information of user "Brian" using Graph API Then the HTTP status code should be "200" And the user retrieve API response should contain the following information: | displayName | id | mail | onPremisesSamAccountName | | Brian Murphy | %uuid_v4% | brian@example.org | Brian | - Scenario: non-admin user tries get information of a user - Given user "Brian" has been created with default attributes and without skeleton files + Scenario: non-admin user tries to get the information of a user When user "Brian" tries to get information of user "Alice" using Graph API - Then the HTTP status code should be "200" - And the last response should be an unauthorized response - - - Scenario: admin user tries get all user - Given these users have been created with default attributes and without skeleton files: - | username | - | Brian | - | Carol | - | David | - When user "Alice" tries to get all user using the Graph API - Then the HTTP status code should be "200" - And the API response should contain all user with following information: - | displayName | id | mail | onPremisesSamAccountName | - | Brian Murphy | %uuid_v4% | brian@example.org | Brian | - | David Lopez | %uuid_v4% | david@example.org | David | - | Carol King | %uuid_v4% | carol@example.org | Carol | - - - Scenario: non-admin user tries get all user - Given these users have been created with default attributes and without skeleton files: - | username | - | Brian | - | Carol | - | David | - When user "Brian" tries to get all user using the Graph API Then the HTTP status code should be "401" And the last response should be an unauthorized response - Scenario: admin user tries to get drive information of a user - Given these users have been created with default attributes and without skeleton files: - | username | - | Brian | - When the user "Alice" tries to get user "Brian" along with his drive information using Graph API + Scenario: admin user gets all users + When user "Alice" gets all users using the Graph API + Then the HTTP status code should be "200" + And the API response should contain all users with the following information: + | displayName | id | mail | onPremisesSamAccountName | + | Alice Hansen | %uuid_v4% | alice@example.org | Alice | + | Brian Murphy | %uuid_v4% | brian@example.org | Brian | + + + Scenario: non-admin user tries get all user + When user "Brian" tries to get all users using the Graph API + Then the HTTP status code should be "401" + And the last response should be an unauthorized response + + + Scenario: admin user gets the drive information of a user + When the user "Alice" gets user "Brian" along with his drive information using Graph API Then the HTTP status code should be "200" And the user retrieve API response should contain the following information: | displayName | id | mail | onPremisesSamAccountName | @@ -72,11 +59,8 @@ Feature: get users | webUrl | %base_url%/f/%space_id% | - Scenario: normal user tries to get hid/her own drive information - Given these users have been created with default attributes and without skeleton files: - | username | - | Brian | - When the user "Brian" tries to get his drive information using Graph API + Scenario: normal user gets his/her own drive information + When the user "Brian" gets his drive information using Graph API Then the HTTP status code should be "200" And the user retrieve API response should contain the following information: | displayName | id | mail | onPremisesSamAccountName | diff --git a/tests/acceptance/features/bootstrap/GraphContext.php b/tests/acceptance/features/bootstrap/GraphContext.php index f66678ee1..2d7e46608 100644 --- a/tests/acceptance/features/bootstrap/GraphContext.php +++ b/tests/acceptance/features/bootstrap/GraphContext.php @@ -1247,31 +1247,35 @@ class GraphContext implements Context { } /** - * @When user :user tries to get information of user :ofUser using Graph API + * @When user :byUser tries to get information of user :user using Graph API + * @When user :byUser gets information of user :user using Graph API * + * @param string $byUser * @param string $user - * @param string $ofUser * * @return void + * @throws GuzzleException */ - public function userTriesToGetInformationOfUser($user, $ofUser): void { - $credentials = $this->getAdminOrUserCredentials($user); + public function userTriesToGetInformationOfUser(string $byUser, string $user): void { + $credentials = $this->getAdminOrUserCredentials($byUser); $response = GraphHelper::getUser( $this->featureContext->getBaseUrl(), $this->featureContext->getStepLineRef(), $credentials['username'], $credentials['password'], - $ofUser + $user ); $this->featureContext->setResponse($response); } /** - * @When user :user tries to get all user using the Graph API + * @When user :user tries to get all users using the Graph API + * @When user :user gets all users using the Graph API * * @param string $user * * @return void + * @throws GuzzleException */ public function userGetsAllUserUsingTheGraphApi(string $user) { $credentials = $this->getAdminOrUserCredentials($user); @@ -1285,7 +1289,7 @@ class GraphContext implements Context { } /** - * @Then the API response should contain all user with following information: + * @Then the API response should contain all users with the following information: * * @param TableNode $table * @@ -1312,47 +1316,44 @@ class GraphContext implements Context { } /** - * @param string $user - * @param string|null $ofUser + * @param string $byUser + * @param string|null $user * * @return ResponseInterface * @throws JsonException * @throws GuzzleException */ public function retrieveUserInformationAlongWithDriveUsingGraphApi( - string $user, - ?string $ofUser = null + string $byUser, + ?string $user = null ):ResponseInterface { - if ($ofUser === null) { - $ofUser = $user; - } + $user = $user ?? $byUser; $credentials = $this->getAdminOrUserCredentials($user); return GraphHelper::getUserWithDriveInformation( $this->featureContext->getBaseUrl(), $this->featureContext->getStepLineRef(), $credentials["username"], $credentials["password"], - $ofUser + $user ); } /** + * @When /^the user "([^"]*)" gets user "([^"]*)" along with (his|her) drive information using Graph API$/ * - * @When /^the user "([^"]*)" tries to get user "([^"]*)" along with (his|her) drive information using Graph API$/ - * + * @param string $byUser * @param string $user - * @param string $ofUser * * @return void */ - public function userTriesToGetInformationOfUserAlongWithHisDriveData(string $user, $ofUser) { - $response = $this->retrieveUserInformationAlongWithDriveUsingGraphApi($user, $ofUser); + public function userTriesToGetInformationOfUserAlongWithHisDriveData(string $byUser, string $user) { + $response = $this->retrieveUserInformationAlongWithDriveUsingGraphApi($byUser, $user); $this->featureContext->setResponse($response); } /** * - * @When /^the user "([^"]*)" tries to get (his|her) drive information using Graph API$/ + * @When /^the user "([^"]*)" gets (his|her) drive information using Graph API$/ * * @param string $user * @@ -1385,13 +1386,12 @@ class GraphContext implements Context { // break the segment with @@@ to find the actual value from the drive infromation from response $separatedKey = explode("@@@", $keyName); // this stores the actual value of each key from drive information response used for assertion - $actualKeyValueFromDriveInformation = null; - $tempDriveInfo = $actualValueDriveInformation; + $actualKeyValueFromDriveInformation = $actualValueDriveInformation; foreach ($separatedKey as $key) { - $actualKeyValueFromDriveInformation = $tempDriveInfo[$key]; - $tempDriveInfo = $actualKeyValueFromDriveInformation; + $actualKeyValueFromDriveInformation = $actualKeyValueFromDriveInformation[$key]; } + switch ($expectedDriveInformation[$keyName]) { case '%user_id%': Assert::assertEquals(