From 86b931f1767cab150ee16ef17b874d0ad9118bf9 Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Fri, 26 Apr 2024 15:27:00 +0545 Subject: [PATCH] tests: code review of acceptance tests --- .../delayPostprocessing.feature | 2 +- .../apiNotification/spaceNotification.feature | 4 +- .../features/bootstrap/ArchiverContext.php | 2 +- .../features/bootstrap/AuthContext.php | 9 ++-- .../features/bootstrap/ChecksumContext.php | 2 +- .../features/bootstrap/FeatureContext.php | 53 ++++++++----------- .../bootstrap/FilesVersionsContext.php | 2 +- .../features/bootstrap/GraphContext.php | 16 +++--- .../bootstrap/NotificationContext.php | 31 +++++------ .../features/bootstrap/OcisConfigContext.php | 5 +- .../features/bootstrap/Provisioning.php | 51 ++---------------- .../bootstrap/PublicWebDavContext.php | 1 - .../features/bootstrap/SearchContext.php | 4 +- .../features/bootstrap/SettingsContext.php | 6 ++- .../acceptance/features/bootstrap/Sharing.php | 36 ++++++------- .../features/bootstrap/SharingNgContext.php | 14 ++--- .../features/bootstrap/SpacesContext.php | 31 ++++++----- .../features/bootstrap/SpacesTUSContext.php | 1 - .../features/bootstrap/TUSContext.php | 37 +++++++------ .../features/bootstrap/TagContext.php | 2 +- .../features/bootstrap/TrashbinContext.php | 13 +++-- .../acceptance/features/bootstrap/WebDav.php | 45 ++++++++-------- .../bootstrap/WebDavLockingContext.php | 4 +- .../bootstrap/WebDavPropertiesContext.php | 28 +++++----- 24 files changed, 161 insertions(+), 238 deletions(-) diff --git a/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature b/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature index a6884ada0..537a34700 100644 --- a/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature +++ b/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature @@ -6,7 +6,7 @@ Feature: delay post-processing of uploaded files Background: Given user "Alice" has been created with default attributes and without skeleton files - And async upload has been enabled with post processing delayed to "30" seconds + And async upload has been enabled with post-processing delayed to "30" seconds @issue-5326 Scenario Outline: user sends GET request to the file while it's still being processed diff --git a/tests/acceptance/features/apiNotification/spaceNotification.feature b/tests/acceptance/features/apiNotification/spaceNotification.feature index db68bd123..b9de72f40 100644 --- a/tests/acceptance/features/apiNotification/spaceNotification.feature +++ b/tests/acceptance/features/apiNotification/spaceNotification.feature @@ -285,7 +285,7 @@ Feature: Notification And user "Alice" has disabled a space "notification checking" When user "Brian" lists all notifications Then the HTTP status code should be "200" - And user "Brian" should have "2" notifications + And there should be "2" notifications And the JSON response should contain a notification message with the subject "Space disabled" and the message-details should match """ { @@ -462,4 +462,4 @@ Feature: Notification And user "Alice" has disabled a space "notification checking" When user "Brian" lists all notifications Then the HTTP status code should be "200" - And user "Brian" should have "2" notifications + And there should be "2" notifications diff --git a/tests/acceptance/features/bootstrap/ArchiverContext.php b/tests/acceptance/features/bootstrap/ArchiverContext.php index cb1a0897b..256ef56e4 100644 --- a/tests/acceptance/features/bootstrap/ArchiverContext.php +++ b/tests/acceptance/features/bootstrap/ArchiverContext.php @@ -27,7 +27,7 @@ use GuzzleHttp\Exception\GuzzleException; use TestHelpers\HttpRequestHelper; use TestHelpers\SetupHelper; use PHPUnit\Framework\Assert; -use \Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\ResponseInterface; require_once 'bootstrap.php'; diff --git a/tests/acceptance/features/bootstrap/AuthContext.php b/tests/acceptance/features/bootstrap/AuthContext.php index 6a85dbcb0..03fc32e72 100644 --- a/tests/acceptance/features/bootstrap/AuthContext.php +++ b/tests/acceptance/features/bootstrap/AuthContext.php @@ -24,16 +24,13 @@ use TestHelpers\HttpRequestHelper; use Behat\Gherkin\Node\TableNode; use Behat\Behat\Context\Context; use TestHelpers\SetupHelper; -use \Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\ResponseInterface; use TestHelpers\WebDavHelper; /** * Authentication functions */ class AuthContext implements Context { - private string $clientToken; - private string $appToken; - private array $appTokens; private FeatureContext $featureContext; /** @@ -548,12 +545,12 @@ class AuthContext implements Context { * * @param string $asUser * @param string $method - * @param string $body + * @param string|null $body * @param string $ofUser * @param TableNode $table * * @return void - * @throws Exception + * @throws JsonException */ public function userRequestsTheseEndpointsIncludingBodyUsingPasswordOfUser(string $asUser, string $method, ?string $body, string $ofUser, TableNode $table):void { $asUser = $this->featureContext->getActualUsername($asUser); diff --git a/tests/acceptance/features/bootstrap/ChecksumContext.php b/tests/acceptance/features/bootstrap/ChecksumContext.php index fd7f08768..8363813a7 100644 --- a/tests/acceptance/features/bootstrap/ChecksumContext.php +++ b/tests/acceptance/features/bootstrap/ChecksumContext.php @@ -398,7 +398,7 @@ class ChecksumContext implements Context { Assert::assertFalse( $isHeader, "Expected no checksum header but got " - . $this->featureContext->getResponse()->getHeader('OC-Checksum') + . print_r($this->featureContext->getResponse()->getHeader('OC-Checksum'), true) ); } diff --git a/tests/acceptance/features/bootstrap/FeatureContext.php b/tests/acceptance/features/bootstrap/FeatureContext.php index 1937f2076..22f157d92 100644 --- a/tests/acceptance/features/bootstrap/FeatureContext.php +++ b/tests/acceptance/features/bootstrap/FeatureContext.php @@ -30,7 +30,6 @@ use Behat\Behat\Hook\Scope\AfterScenarioScope; use Behat\Gherkin\Node\PyStringNode; use Behat\Gherkin\Node\TableNode; use Behat\Testwork\Hook\Scope\BeforeSuiteScope; -use Behat\Testwork\Hook\Scope\AfterSuiteScope; use GuzzleHttp\Cookie\CookieJar; use Psr\Http\Message\ResponseInterface; use PHPUnit\Framework\Assert; @@ -60,17 +59,17 @@ class FeatureContext extends BehatVariablesContext { * json schema validator keywords * See: https://json-schema.org/draft-06/draft-wright-json-schema-validation-01#rfc.section.6 */ - private array $jsonSchemaValidators = []; + private array $jsonSchemaValidators; /** * Unix timestamp seconds */ private int $scenarioStartTime; - private string $adminUsername = ''; - private string $adminPassword = ''; + private string $adminUsername; + private string $adminPassword; private string $adminDisplayName = ''; private string $adminEmailAddress = ''; - private string $originalAdminPassword = ''; + private string $originalAdminPassword; /** * An array of values of replacement values of user attributes. @@ -80,55 +79,55 @@ class FeatureContext extends BehatVariablesContext { * Key is the username, value is an array of user attributes */ private ?array $userReplacements = null; - private string $regularUserPassword = ''; - private string $alt1UserPassword = ''; - private string $alt2UserPassword = ''; - private string $alt3UserPassword = ''; - private string $alt4UserPassword = ''; + private string $regularUserPassword; + private string $alt1UserPassword; + private string $alt2UserPassword; + private string $alt3UserPassword; + private string $alt4UserPassword; /** * The password to use in tests that create a sub-admin user */ - private string $subAdminPassword = ''; + private string $subAdminPassword; /** * The password to use in tests that create another admin user */ - private string $alternateAdminPassword = ''; + private string $alternateAdminPassword; /** * The password to use in tests that create public link shares */ - private string $publicLinkSharePassword = ''; - private string $ocPath = ''; + private string $publicLinkSharePassword; + private string $ocPath; /** * Location of the root folder of ownCloud on the local server under test */ private ?string $localServerRoot = null; private string $currentUser = ''; - private string $currentServer = ''; + private string $currentServer; /** * The base URL of the current server under test, * without any terminating slash * e.g. http://localhost:8080 */ - private string $baseUrl = ''; + private string $baseUrl; /** * The base URL of the local server under test, * without any terminating slash * e.g. http://localhost:8080 */ - private string $localBaseUrl = ''; + private string $localBaseUrl; /** * The base URL of the remote (federated) server under test, * without any terminating slash * e.g. http://localhost:8180 */ - private string $remoteBaseUrl = ''; + private string $remoteBaseUrl; /** * The suite name, feature name and scenario line number. @@ -149,7 +148,7 @@ class FeatureContext extends BehatVariablesContext { /** * @var boolean true if TEST_SERVER_FED_URL is defined */ - private bool $federatedServerExists = false; + private bool $federatedServerExists; private int $ocsApiVersion = 1; private ?ResponseInterface $response = null; private string $responseUser = ''; @@ -1407,9 +1406,11 @@ class FeatureContext extends BehatVariablesContext { * @param string $user * @param string $verb * @param string $url - * @param string $headersTable + * @param TableNode $headersTable * * @return void + * @throws GuzzleException + * @throws JsonException */ public function userSendsHTTPMethodToUrlWithHeaders(string $user, string $verb, string $url, TableNode $headersTable): void { $this->verifyTableNodeColumns( @@ -3599,18 +3600,6 @@ class FeatureContext extends BehatVariablesContext { WebDavHelper::$SPACE_ID_FROM_OCIS = ''; } - /** - * @BeforeSuite - * - * @param BeforeSuiteScope $scope - * - * @return void - * @throws Exception - */ - public static function useBigFileIDs(BeforeSuiteScope $scope): void { - return; - } - /** * Verify that the tableNode contains expected headers * diff --git a/tests/acceptance/features/bootstrap/FilesVersionsContext.php b/tests/acceptance/features/bootstrap/FilesVersionsContext.php index 97cb4feea..2e8081178 100644 --- a/tests/acceptance/features/bootstrap/FilesVersionsContext.php +++ b/tests/acceptance/features/bootstrap/FilesVersionsContext.php @@ -462,7 +462,7 @@ class FilesVersionsContext implements Context { } /** - * returns the result parsed into an SimpleXMLElement + * returns the result parsed into a SimpleXMLElement * with a registered namespace with 'd' as prefix and 'DAV:' as namespace * * @param string $user diff --git a/tests/acceptance/features/bootstrap/GraphContext.php b/tests/acceptance/features/bootstrap/GraphContext.php index 9faf086cb..4a634967f 100644 --- a/tests/acceptance/features/bootstrap/GraphContext.php +++ b/tests/acceptance/features/bootstrap/GraphContext.php @@ -140,7 +140,7 @@ class GraphContext implements Context { public function theUserChangesTheUserNameOfUserToUsingTheGraphApi(string $byUser, string $user, string $userName): void { $response = $this->editUserUsingTheGraphApi($byUser, $user, $userName); $this->featureContext->setResponse($response); - // need add user to list to delete him after test + // need to add user to list to delete him after test if (!empty($userName)) { $this->featureContext->addUserToCreatedUsersList($userName, $this->featureContext->getUserPassword($user)); } @@ -324,7 +324,7 @@ class GraphContext implements Context { } /** - * remove user from group + * remove user from a group * * @param string $group * @param string $user @@ -675,7 +675,7 @@ class GraphContext implements Context { } /** - * returns a list of members in group + * returns a list of members in a group * * @param string $group * @@ -761,7 +761,6 @@ class GraphContext implements Context { * * @param string $user * @param string $group - * @param bool $checkResult * * @return void * @throws Exception @@ -769,8 +768,7 @@ class GraphContext implements Context { */ public function adminHasAddedUserToGroupUsingTheGraphApi( string $user, - string $group, - bool $checkResult = true + string $group ): void { $response = $this->addUserToGroup($group, $user); $this->featureContext->theHTTPStatusCodeShouldBe(204, '', $response); @@ -1887,7 +1885,7 @@ class GraphContext implements Context { * * @return void */ - public function userSearchesForGroupUsingGraphApi($user, $searchTerm):void { + public function userSearchesForGroupUsingGraphApi(string $user, string $searchTerm):void { $credentials = $this->getAdminOrUserCredentials($user); $this->featureContext->setResponse( GraphHelper::searchGroup( @@ -2242,7 +2240,7 @@ class GraphContext implements Context { * @param PyStringNode|null $schemaString * * @return void - * @throws GuzzleException + * @throws Exception * */ public function downloadedJsonContentShouldContainEventTypeInItemAndShouldMatch(string $eventType, ?string $spaceType=null, PyStringNode $schemaString=null): void { @@ -2324,7 +2322,7 @@ class GraphContext implements Context { * @return ResponseInterface * @throws GuzzleException */ - public function getAssignedRole(string $user) { + public function getAssignedRole(string $user): ResponseInterface { $userId = $this->featureContext->getAttributeOfCreatedUser($user, 'id') ?? $this->featureContext->getUserIdByUserName($user); return ( GraphHelper::getAssignedRole( diff --git a/tests/acceptance/features/bootstrap/NotificationContext.php b/tests/acceptance/features/bootstrap/NotificationContext.php index e5c694189..8e2449e66 100644 --- a/tests/acceptance/features/bootstrap/NotificationContext.php +++ b/tests/acceptance/features/bootstrap/NotificationContext.php @@ -247,15 +247,14 @@ class NotificationContext implements Context { } /** - * @Then /^user "([^"]*)" should have "([^"]*)" notifications$/ + * @Then /^there should be "([^"]*)" notifications$/ * - * @param string $user * @param int $numberOfNotification * * @return void * @throws Exception */ - public function userShouldHaveNotifications(string $user, int $numberOfNotification): void { + public function userShouldHaveNotifications(int $numberOfNotification): void { if (!isset($this->featureContext->getJsonDecodedResponseBodyContent()->ocs->data)) { throw new Exception("Notification is empty"); } @@ -264,7 +263,7 @@ class NotificationContext implements Context { Assert::assertEquals( $numberOfNotification, $actualNumber, - "Expected number of notification was '$numberOfNotification', but got '$actualNumber'" + "Expected number of notifications was '$numberOfNotification', but got '$actualNumber'" ); } @@ -351,7 +350,7 @@ class NotificationContext implements Context { } } else { $responseBodyArray[] = $this->featureContext->getJsonDecodedResponseBodyContent($response); - Assert::fail("Response should contain notification but found: $responseBodyArray"); + Assert::fail("Response should contain notification but found: " . print_r($responseBodyArray, true)); } return $responseBodyArray; } @@ -368,8 +367,8 @@ class NotificationContext implements Context { */ public function userShouldGetANotificationWithMessage(string $user, string $subject, TableNode $table):void { $count = 0; - // sometimes the test might try to get notification before the notification is created by the server - // in order to prevent test from failing because of that try to list the notifications again + // Sometimes the test might try to get the notifications before the server has created the notification. + // To prevent the test from failing because of that, try to list the notifications again do { if ($count > 0) { \sleep(1); @@ -418,9 +417,9 @@ class NotificationContext implements Context { $this->userDeletesNotificationOfResourceAndSubject($user, $resource, $subject); $this->featureContext->theHTTPStatusCodeShouldBe(200); } elseif (\count($notification) === 0) { - throw new \Exception("Response doesn't contain any notification with resource '$resource' and subject '$subject'.\n$notification"); + throw new \Exception("Response doesn't contain any notification with resource '$resource' and subject '$subject'.\n" . print_r($notification, true)); } else { - throw new \Exception("Response contains more than one notification with resource '$resource' and subject '$subject'.\n$notification"); + throw new \Exception("Response contains more than one notification with resource '$resource' and subject '$subject'.\n" . print_r($notification, true)); } } @@ -472,9 +471,7 @@ class NotificationContext implements Context { [$this->spacesContext, "getSpaceIdByName"], "parameter" => [$sender, $spaceName] ], - ], - null, - null + ] ); $this->assertEmailContains($user, $expectedEmailBodyContent); } @@ -527,11 +524,11 @@ class NotificationContext implements Context { public function clearInbucketMessages():void { try { if (!empty($this->featureContext->emailRecipients)) { - foreach ($this->featureContext->emailRecipients as $emailRecipent) { + foreach ($this->featureContext->emailRecipients as $emailRecipient) { EmailHelper::deleteAllEmailsForAMailbox( EmailHelper::getLocalEmailUrl(), $this->featureContext->getStepLineRef(), - $emailRecipent + $emailRecipient ); } } @@ -568,8 +565,7 @@ class NotificationContext implements Context { 'POST', $this->globalNotificationEndpointPath, $this->featureContext->getStepLineRef(), - json_encode($payload), - 2 + json_encode($payload) ); } @@ -635,8 +631,7 @@ class NotificationContext implements Context { 'DELETE', $this->globalNotificationEndpointPath, $this->featureContext->getStepLineRef(), - json_encode($payload), - 2 + json_encode($payload) ); $this->featureContext->setResponse($response); } diff --git a/tests/acceptance/features/bootstrap/OcisConfigContext.php b/tests/acceptance/features/bootstrap/OcisConfigContext.php index 3250bad1a..43b2f94b4 100644 --- a/tests/acceptance/features/bootstrap/OcisConfigContext.php +++ b/tests/acceptance/features/bootstrap/OcisConfigContext.php @@ -25,21 +25,20 @@ use Behat\Gherkin\Node\TableNode; use GuzzleHttp\Exception\GuzzleException; use TestHelpers\OcisConfigHelper; use PHPUnit\Framework\Assert; -use TestHelpers\UploadHelper; /** * steps needed to re-configure oCIS server */ class OcisConfigContext implements Context { /** - * @Given async upload has been enabled with post processing delayed to :delayTime seconds + * @Given async upload has been enabled with post-processing delayed to :delayTime seconds * * @param string $delayTime * * @return void * @throws GuzzleException */ - public function asyncUploadHasbeenEnabledWithDelayedPostProcessing(string $delayTime): void { + public function asyncUploadHasBeenEnabledWithDelayedPostProcessing(string $delayTime): void { $envs = [ "OCIS_ASYNC_UPLOADS" => true, "OCIS_EVENTS_ENABLE_TLS" => false, diff --git a/tests/acceptance/features/bootstrap/Provisioning.php b/tests/acceptance/features/bootstrap/Provisioning.php index 97a7122b0..ae6146c83 100644 --- a/tests/acceptance/features/bootstrap/Provisioning.php +++ b/tests/acceptance/features/bootstrap/Provisioning.php @@ -24,7 +24,6 @@ use GuzzleHttp\Exception\ClientException; use GuzzleHttp\Exception\GuzzleException; use Psr\Http\Message\ResponseInterface; use PHPUnit\Framework\Assert; -use TestHelpers\OcsApiHelper; use TestHelpers\UserHelper; use TestHelpers\HttpRequestHelper; use TestHelpers\OcisHelper; @@ -519,7 +518,7 @@ trait Provisioning { } /** - * prepares suitable nested array with user-attributes for multiple users to be created + * prepares a suitable nested array with user-attributes for multiple users to be created * * @param boolean $setDefaultAttributes * @param array $table @@ -558,7 +557,7 @@ trait Provisioning { } else { $userAttribute['password'] = $this->getPasswordForUser($row['username']); } - // Add request body to the bodies array. we will use that later to loop through created users. + // Add request body to the bodies array. We will use that later to loop through created users. $usersAttributes[] = $userAttribute; } return $usersAttributes; @@ -799,7 +798,6 @@ trait Provisioning { $attributesToCreateUser['userid'] = $userAttributes['userid']; $attributesToCreateUser['password'] = $userAttributes['password']; $attributesToCreateUser['displayname'] = $userAttributes['displayName']; - $attributesToCreateUser['username'] = $userAttributes['userid']; if ($userAttributes['email'] === null) { Assert::assertArrayHasKey( 'userid', @@ -3669,7 +3667,7 @@ trait Provisioning { public function theDisplayNameOfUserShouldBe(string $user, string $displayname):void { $actualUser = $this->getActualUsername($user); $response = $this->retrieveUserInformationAsAdminUsingProvisioningApi($actualUser); - $actualDisplayName = $this->getDisplayNameFromResponse($response, $displayname); + $actualDisplayName = $this->getDisplayNameFromResponse($response); Assert::assertEquals( $displayname, $actualDisplayName @@ -4361,47 +4359,4 @@ trait Provisioning { } return false; } - - /** - * sets the skeletondirectory according to the type - * - * @param string $skeletonType can be "tiny", "small", "large" OR empty. - * If an empty string is given, the current - * setting will not be changed - * - * @return string skeleton folder before the change - * @throws Exception - */ - private function setSkeletonDirByType(string $skeletonType): string { - $originalSkeletonPath = \getenv("SKELETON_DIR"); - if ($originalSkeletonPath === false) { - $originalSkeletonPath = ''; - } - if ($skeletonType !== '') { - $skeletonDirName = $skeletonType . "Skeleton"; - $newSkeletonPath = \dirname($originalSkeletonPath) . '/' . $skeletonDirName; - \putenv( - "SKELETON_DIR=" . $newSkeletonPath - ); - } - return $originalSkeletonPath; - } - - /** - * sets the skeletondirectory - * - * @param string $skeletonDir Full path of the skeleton directory - * If an empty string is given, the current - * setting will not be changed - * - * @return string skeleton folder before the change - * @throws Exception - */ - private function setSkeletonDir(string $skeletonDir): string { - $originalSkeletonPath = \getenv("SKELETON_DIR"); - if ($skeletonDir !== '') { - \putenv("SKELETON_DIR=" . $skeletonDir); - } - return $originalSkeletonPath; - } } diff --git a/tests/acceptance/features/bootstrap/PublicWebDavContext.php b/tests/acceptance/features/bootstrap/PublicWebDavContext.php index 6fc026b7c..b6a3e0406 100644 --- a/tests/acceptance/features/bootstrap/PublicWebDavContext.php +++ b/tests/acceptance/features/bootstrap/PublicWebDavContext.php @@ -1262,7 +1262,6 @@ class PublicWebDavContext implements Context { string $password, string $expectedHttpCode ):void { - $filename = ""; if ($publicWebDAVAPIVersion === "old") { return; } diff --git a/tests/acceptance/features/bootstrap/SearchContext.php b/tests/acceptance/features/bootstrap/SearchContext.php index 8009b2608..343e4773a 100644 --- a/tests/acceptance/features/bootstrap/SearchContext.php +++ b/tests/acceptance/features/bootstrap/SearchContext.php @@ -195,12 +195,12 @@ class SearchContext implements Context { null, true ); - foreach ($elementRows as $index => $expectedFile) { + foreach ($elementRows as $expectedFile) { $filename = $expectedFile[0]; $content = $foundEntries[$filename]; // Extract the content between the tags preg_match('/(.*?)<\/mark>/s', $content, $matches); - $actualContent = isset($matches[1]) ? $matches[1] : ''; + $actualContent = $matches[1] ?? ''; // Remove any leading/trailing whitespace for comparison $actualContent = trim($actualContent); diff --git a/tests/acceptance/features/bootstrap/SettingsContext.php b/tests/acceptance/features/bootstrap/SettingsContext.php index c099a131b..a66288a6e 100644 --- a/tests/acceptance/features/bootstrap/SettingsContext.php +++ b/tests/acceptance/features/bootstrap/SettingsContext.php @@ -155,8 +155,7 @@ class SettingsContext implements Context { */ public function getRoleIdByRoleName(string $user, string $role): string { // Sometimes the response body is not complete and results invalid json. - // so we try again until we get a valid json. - $tryAgain = false; + // So we try again until we get a valid json. $retried = 0; do { $response = $this->getRoles($user); @@ -376,6 +375,7 @@ class SettingsContext implements Context { $headersTable, ['header', 'value'] ); + $headers = []; foreach ($headersTable as $row) { $headers[$row['header']] = $row ['value']; } @@ -409,6 +409,8 @@ class SettingsContext implements Context { return $value["value"]["listValue"]["values"][0]["stringValue"]; } } + // if a language setting was still not found, return English + return "en"; } /** diff --git a/tests/acceptance/features/bootstrap/Sharing.php b/tests/acceptance/features/bootstrap/Sharing.php index 5f3e43a9f..086e8dcef 100755 --- a/tests/acceptance/features/bootstrap/Sharing.php +++ b/tests/acceptance/features/bootstrap/Sharing.php @@ -229,7 +229,7 @@ trait Sharing { } /** - * Split given permissions string each separated with "," into array of strings + * Split given permissions string each separated with "," into an array of strings * * @param string $str * @@ -662,16 +662,15 @@ trait Sharing { } /** - * @Then /^user "([^"]*)" should not be able to create a public link share of (file|folder) "([^"]*)" using the sharing API$/ + * @Then /^user "([^"]*)" should not be able to create a public link share of (?:file|folder) "([^"]*)" using the sharing API$/ * * @param string $sharer - * @param string $entry * @param string $filepath * * @return void * @throws Exception */ - public function shouldNotBeAbleToCreatePublicLinkShare(string $sharer, string $entry, string $filepath):void { + public function shouldNotBeAbleToCreatePublicLinkShare(string $sharer, string $filepath):void { $this->createAPublicShare($sharer, $filepath); Assert::assertEquals( 404, @@ -964,7 +963,7 @@ trait Sharing { if (($contentExpected === "ANY_VALUE") || (($contentExpected === "A_TOKEN") && (\strlen($value) === 15)) || (($contentExpected === "A_NUMBER") && \is_numeric($value)) - || (($contentExpected === "A_STRING") && \is_string($value) && $value !== "") + || (($contentExpected === "A_STRING") && $value !== "") || (($contentExpected === "AN_URL") && $this->isAPublicLinkUrl($value)) || (($field === 'remote') && (\rtrim($value, "/") === $contentExpected)) || ($contentExpected === $value) @@ -1585,11 +1584,10 @@ trait Sharing { } /** - * @Then /^user "([^"]*)" should not be able to share (file|folder|entry) "([^"]*)" with (user|group) "([^"]*)"(?: with permissions (\d+))? using the sharing API$/ - * @Then /^user "([^"]*)" should not be able to share (file|folder|entry) "([^"]*)" with (user|group) "([^"]*)" with permissions "([^"]*)" using the sharing API$/ + * @Then /^user "([^"]*)" should not be able to share (?:file|folder|entry) "([^"]*)" with (user|group) "([^"]*)"(?: with permissions (\d+))? using the sharing API$/ + * @Then /^user "([^"]*)" should not be able to share (?:file|folder|entry) "([^"]*)" with (user|group) "([^"]*)" with permissions "([^"]*)" using the sharing API$/ * * @param string $sharer - * @param string $entry * @param string $filepath * @param string $userOrGroupShareType * @param string $sharee @@ -1600,7 +1598,6 @@ trait Sharing { */ public function userTriesToShareFileUsingTheSharingApi( string $sharer, - string $entry, string $filepath, string $userOrGroupShareType, string $sharee, @@ -1624,11 +1621,10 @@ trait Sharing { } /** - * @Then /^user "([^"]*)" should be able to share (file|folder|entry) "([^"]*)" with (user|group) "([^"]*)"(?: with permissions (\d+))? using the sharing API$/ - * @Then /^user "([^"]*)" should be able to share (file|folder|entry) "([^"]*)" with (user|group) "([^"]*)" with permissions "([^"]*)" using the sharing API$/ + * @Then /^user "([^"]*)" should be able to share (?:file|folder|entry) "([^"]*)" with (user|group) "([^"]*)"(?: with permissions (\d+))? using the sharing API$/ + * @Then /^user "([^"]*)" should be able to share (?:file|folder|entry) "([^"]*)" with (user|group) "([^"]*)" with permissions "([^"]*)" using the sharing API$/ * * @param string $sharer - * @param string $entry * @param string $filepath * @param string $userOrGroupShareType * @param string $sharee @@ -1639,7 +1635,6 @@ trait Sharing { */ public function userShouldBeAbleToShareUsingTheSharingApi( string $sharer, - string $entry, string $filepath, string $userOrGroupShareType, string $sharee, @@ -2236,10 +2231,10 @@ trait Sharing { $this->getShareData($user, $share_id); foreach ($bodyRows as $field => $value) { - if ($type === "user" && \in_array($field, ["share_with"])) { + if ($type === "user" && $field == "share_with") { $value = $this->getActualUsername($value); } - if (\in_array($field, ["uid_owner"])) { + if ($field == "uid_owner") { $value = $this->getActualUsername($value); } $value = $this->replaceValuesFromTable($field, $value); @@ -2454,8 +2449,8 @@ trait Sharing { $value = (explode("$", $value))[1]; } if ($field === "space_id") { - $exploadedSpaceId = explode("$", $value); - $value = $exploadedSpaceId[0] . "$" . $exploadedSpaceId[1] . "!" . $exploadedSpaceId[1]; + $explodedSpaceId = explode("$", $value); + $value = $explodedSpaceId[0] . "$" . $explodedSpaceId[1] . "!" . $explodedSpaceId[1]; } } $value = $this->getActualUsername($value); @@ -2609,7 +2604,7 @@ trait Sharing { $errorMessage, (string) $receivedErrorMessage[0], "Expected error message was '$errorMessage' but got '" - . (string) $receivedErrorMessage[0] + . $receivedErrorMessage[0] . "'" ); } @@ -2715,16 +2710,15 @@ trait Sharing { } /** - * @Then /^as user "([^"]*)" the (file|folder) "([^"]*)" should not have any shares$/ + * @Then /^as user "([^"]*)" the (?:file|folder) "([^"]*)" should not have any shares$/ * * @param string $user - * @param string $entry * @param string $path * * @return void * @throws Exception */ - public function checkPublicSharesAreEmpty(string $user, string $entry, string $path):void { + public function checkPublicSharesAreEmpty(string $user, string $path):void { $user = $this->getActualUsername($user); $response = $this->getShares($user, $path); //It shouldn't have public shares diff --git a/tests/acceptance/features/bootstrap/SharingNgContext.php b/tests/acceptance/features/bootstrap/SharingNgContext.php index 48b03fe08..c5670080f 100644 --- a/tests/acceptance/features/bootstrap/SharingNgContext.php +++ b/tests/acceptance/features/bootstrap/SharingNgContext.php @@ -94,12 +94,12 @@ class SharingNgContext implements Context { /** * @param string $user - * @param string $fileOrFolder (file|folder) + * @param string $fileOrFolder (file|folder) * @param string $space - * @param string $resource + * @param string|null $resource * * @return ResponseInterface - * @throws Exception + * @throws GuzzleException */ public function getPermissionsList(string $user, string $fileOrFolder, string $space, ?string $resource = ''):ResponseInterface { $spaceId = ($this->spacesContext->getSpaceByName($user, $space))["id"]; @@ -131,7 +131,7 @@ class SharingNgContext implements Context { * @return void * @throws Exception */ - public function userGetsPermissionsListForResourceOfTheSpaceUsingTheGraphiAPI(string $user, string $fileOrFolder, string $resource, string $space):void { + public function userGetsPermissionsListForResourceOfTheSpaceUsingTheGraphAPI(string $user, string $fileOrFolder, string $resource, string $space):void { $this->featureContext->setResponse( $this->getPermissionsList($user, $fileOrFolder, $space, $resource) ); @@ -146,7 +146,7 @@ class SharingNgContext implements Context { * @return void * @throws Exception */ - public function userListsThePermissionsOfSpaceUsingTheGraphApi($user, $space):void { + public function userListsThePermissionsOfSpaceUsingTheGraphApi(string $user, string $space):void { $this->featureContext->setResponse( $this->getPermissionsList($user, 'folder', $space) ); @@ -212,6 +212,8 @@ class SharingNgContext implements Context { } } + $shareeIds = []; + if (\array_key_exists('shareeId', $rows)) { $shareeIds[] = $rows['shareeId']; $shareTypes[] = $rows['shareType']; @@ -832,7 +834,7 @@ class SharingNgContext implements Context { //this details is needed for result logging purpose to determine whether the resource shared is a resource or a project space $resourceDetail = ($resource) ? "resource '" . $resource : "space '" . $space; foreach ($allowedPermissionRoles as $role) { - //we should be able to send share invitation for each of the role allowed for the files/folders which are listed in permissions (allowed) + //we should be able to send share invitation for each of the role allowed for the files/folders which are listed in permissions (allowed) $roleAllowed = GraphHelper::getPermissionNameByPermissionRoleId($role->id); $responseSendInvitation = $this->sendShareInvitation($user, new TableNode(array_merge($table->getTable(), [['permissionsRole', $roleAllowed]]))); $jsonResponseSendInvitation = $this->featureContext->getJsonDecodedResponseBodyContent($responseSendInvitation); diff --git a/tests/acceptance/features/bootstrap/SpacesContext.php b/tests/acceptance/features/bootstrap/SpacesContext.php index 8c67ac107..9fda89eff 100644 --- a/tests/acceptance/features/bootstrap/SpacesContext.php +++ b/tests/acceptance/features/bootstrap/SpacesContext.php @@ -215,9 +215,8 @@ class SpacesContext implements Context { $listSpacesFn = 'listAllAvailableSpacesOfUser'; } - // Sometimes listing available spaces might not return newly created/shared spaces - // so we try again until we find the space or we reach the max number of retries (i.e. 10) - $tryAgain = false; + // Sometimes listing available spaces might not return newly created/shared spaces. + // So we try again until we find the space or we reach the max number of retries (i.e. 10) $retried = 0; do { // empty the available spaces array @@ -611,6 +610,7 @@ class SpacesContext implements Context { $headersTable, ['header', 'value'] ); + $headers = []; foreach ($headersTable as $row) { $headers[$row['header']] = $row ['value']; } @@ -1492,7 +1492,7 @@ class SpacesContext implements Context { string $newDescription ): void { $bodyData = ["description" => $newDescription]; - $response = $this->updateSpace($user, $spaceName, $bodyData, ''); + $response = $this->updateSpace($user, $spaceName, $bodyData); $this->featureContext->theHTTPStatusCodeShouldBe( 200, "Expected response status code should be 200", @@ -1541,7 +1541,7 @@ class SpacesContext implements Context { int $newQuota ): void { $bodyData = ["quota" => ["total" => $newQuota]]; - $response = $this->updateSpace($user, $spaceName, $bodyData, ''); + $response = $this->updateSpace($user, $spaceName, $bodyData); $this->featureContext->theHTTPStatusCodeShouldBe( 200, "Expected response status code should be 200", @@ -1991,7 +1991,7 @@ class SpacesContext implements Context { $space = $this->getSpaceByName($user, $spaceName); $fileDestination = $this->escapePath(\ltrim($fileDestination, "/")); if ($endPath && str_contains($endPath, 'remote.php')) { - // this is check for when we want to test with the endpoint having `remote.php` in space webdav + // this is a check for when we want to test with the endpoint having `remote.php` in space webdav // by default spaces webdav is '/dav/spaces' return $this->featureContext->getBaseUrl() . '/remote.php/dav/spaces/' . $space['id'] . '/' . $fileDestination; } @@ -2088,9 +2088,8 @@ class SpacesContext implements Context { if ($this->featureContext->getDavPathVersion() === WebDavHelper::DAV_VERSION_SPACES) { $suffix = $this->getSpaceIdByName($user, $sourceSpace) . "/"; } - $fullUrl = $baseUrl . \rtrim($destinationPath, "/") . "/{$suffix}{$source}"; + $fullUrl = $baseUrl . \rtrim($destinationPath, "/") . "/$suffix$source"; - $destinationId = ""; if ($destinationType === "space") { $destinationId = $this->getSpaceIdByName($user, $destinationName); } else { @@ -2853,7 +2852,7 @@ class SpacesContext implements Context { } /** - * User get all objects in the trash of project space + * User gets all objects in the trash of project space * * Method "getTrashbinContentFromResponseXml" borrowed from core repository * and return array like: @@ -3167,7 +3166,7 @@ class SpacesContext implements Context { * @return string * @throws GuzzleException */ - public function userGetsEtagOfElementInASpace(string $user, string $space, string $path) { + public function userGetsEtagOfElementInASpace(string $user, string $space, string $path): string { $this->setSpaceIDByName($user, $space); $xmlObject = $this->webDavPropertiesContext->storeEtagOfElement($user, $path); return $this->featureContext->getEtagFromResponseXmlObject($xmlObject); @@ -3421,7 +3420,6 @@ class SpacesContext implements Context { string $shouldOrNot, string $fileName = '' ): void { - $body = ''; if (!empty($fileName)) { $body = $this->getFileId($user, $spaceName, $fileName); } else { @@ -3529,10 +3527,9 @@ class SpacesContext implements Context { } /** - * @Given /^user "([^"]*)" has stored id of (file|folder) "([^"]*)" of the space "([^"]*)"$/ + * @Given /^user "([^"]*)" has stored id of (?:file|folder) "([^"]*)" of the space "([^"]*)"$/ * * @param string $user - * @param string $fileOrFolder * @param string $path * @param string $spaceName * @@ -3540,9 +3537,9 @@ class SpacesContext implements Context { * * @throws GuzzleException */ - public function userHasStoredIdOfPathOfTheSpace(string $user, string $fileOrFolder, string $path, string $spaceName): void { + public function userHasStoredIdOfPathOfTheSpace(string $user, string $path, string $spaceName): void { $this->setSpaceIDByName($user, $spaceName); - $this->featureContext->userStoresFileIdForPath($user, $fileOrFolder, $path); + $this->featureContext->userStoresFileIdForPath($user, $path); } /** @@ -3650,11 +3647,12 @@ class SpacesContext implements Context { * * @throws GuzzleException */ - public function userSendsPropfindRequestToSpaceWithHeaders(string $user, string $spaceName, $headersTable): void { + public function userSendsPropfindRequestToSpaceWithHeaders(string $user, string $spaceName, TableNode $headersTable): void { $this->featureContext->verifyTableNodeColumns( $headersTable, ['header', 'value'] ); + $headers = []; foreach ($headersTable as $row) { $headers[$row['header']] = $row ['value']; } @@ -3850,6 +3848,7 @@ class SpacesContext implements Context { $user, $this->featureContext->getPasswordForUser($user) ); + $driveItemId = ''; $jsonResponseBody = $this->featureContext->getJsonDecodedResponseBodyContent($jsonResponse); foreach ($jsonResponseBody->value as $value) { if ($value->name === "$resource") { diff --git a/tests/acceptance/features/bootstrap/SpacesTUSContext.php b/tests/acceptance/features/bootstrap/SpacesTUSContext.php index 15f97d913..ef3835bc0 100644 --- a/tests/acceptance/features/bootstrap/SpacesTUSContext.php +++ b/tests/acceptance/features/bootstrap/SpacesTUSContext.php @@ -224,7 +224,6 @@ class SpacesTUSContext implements Context { $mtime = date('Y-m' . '-01', strtotime('-1 year')); break; default: - $mtime; } $this->spacesContext->setSpaceIDByName($user, $spaceName); $mtime = new DateTime($mtime); diff --git a/tests/acceptance/features/bootstrap/TUSContext.php b/tests/acceptance/features/bootstrap/TUSContext.php index 6d980e1fc..cb4daabd5 100644 --- a/tests/acceptance/features/bootstrap/TUSContext.php +++ b/tests/acceptance/features/bootstrap/TUSContext.php @@ -150,7 +150,7 @@ class TUSContext implements Context { 'Upload-Offset' => $offset ]; $headers = empty($extraHeaders) ? $headers : array_merge($headers, $extraHeaders); - + return HttpRequestHelper::sendRequest( $this->resourceLocation, $this->featureContext->getStepLineRef(), @@ -304,18 +304,18 @@ class TUSContext implements Context { string $content, string $destination ): void { - $tmpfname = $this->writeDataToTempFile($content); + $temporaryFileName = $this->writeDataToTempFile($content); try { $this->uploadFileUsingTus( $user, - \basename($tmpfname), + \basename($temporaryFileName), $destination ); $this->featureContext->setLastUploadDeleteTime(\time()); } catch (Exception $e) { Assert::assertStringContainsString('TusPhp\Exception\FileException: Unable to create resource', (string)$e); } - \unlink($tmpfname); + \unlink($temporaryFileName); } /** @@ -341,16 +341,16 @@ class TUSContext implements Context { ?int $noOfChunks, string $destination ): void { - $tmpfname = $this->writeDataToTempFile($content); + $temporaryFileName = $this->writeDataToTempFile($content); $this->uploadFileUsingTus( $user, - \basename($tmpfname), + \basename($temporaryFileName), $destination, [], $noOfChunks ); $this->featureContext->setLastUploadDeleteTime(\time()); - \unlink($tmpfname); + \unlink($temporaryFileName); } /** @@ -390,20 +390,20 @@ class TUSContext implements Context { * @throws Exception */ public function writeDataToTempFile(string $content): string { - $tmpfname = \tempnam( + $temporaryFileName = \tempnam( $this->featureContext->acceptanceTestsDirLocation(), "tus-upload-test-" ); - if ($tmpfname === false) { + if ($temporaryFileName === false) { throw new \Exception("could not create a temporary filename"); } - $tempfile = \fopen($tmpfname, "w"); - if ($tempfile === false) { - throw new \Exception("could not open " . $tmpfname . " for write"); + $temporaryFile = \fopen($temporaryFileName, "w"); + if ($temporaryFile === false) { + throw new \Exception("could not open " . $temporaryFileName . " for write"); } - \fwrite($tempfile, $content); - \fclose($tempfile); - return $tmpfname; + \fwrite($temporaryFile, $content); + \fclose($temporaryFile); + return $temporaryFileName; } /** @@ -449,24 +449,23 @@ class TUSContext implements Context { * * @return void * @throws Exception - * @throws GuzzleException */ public function userUploadsWithCreatesWithUpload( string $user, string $source, string $content ): void { - $tmpfname = $this->writeDataToTempFile($content); + $temporaryFileName = $this->writeDataToTempFile($content); $this->uploadFileUsingTus( $user, - \basename($tmpfname), + \basename($temporaryFileName), $source, [], 1, -1 ); $this->featureContext->setLastUploadDeleteTime(\time()); - \unlink($tmpfname); + \unlink($temporaryFileName); } /** diff --git a/tests/acceptance/features/bootstrap/TagContext.php b/tests/acceptance/features/bootstrap/TagContext.php index c9deb7f29..62a27662b 100644 --- a/tests/acceptance/features/bootstrap/TagContext.php +++ b/tests/acceptance/features/bootstrap/TagContext.php @@ -24,7 +24,7 @@ use Behat\Behat\Hook\Scope\BeforeScenarioScope; use Behat\Gherkin\Node\TableNode; use PHPUnit\Framework\Assert; use TestHelpers\GraphHelper; -use \Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\ResponseInterface; require_once 'bootstrap.php'; diff --git a/tests/acceptance/features/bootstrap/TrashbinContext.php b/tests/acceptance/features/bootstrap/TrashbinContext.php index a04d64d15..f8eeb2645 100644 --- a/tests/acceptance/features/bootstrap/TrashbinContext.php +++ b/tests/acceptance/features/bootstrap/TrashbinContext.php @@ -124,7 +124,7 @@ class TrashbinContext implements Context { 'href' => (string) $href, 'name' => isset($name[0]) ? (string) $name[0] : null, 'mtime' => isset($mtime[0]) ? (string) $mtime[0] : null, - 'collection' => isset($collection[0]) ? $collection[0] : false, + 'collection' => $collection[0] ?? false, 'original-location' => isset($originalLocation[0]) ? (string) $originalLocation[0] : null ]; }, @@ -283,7 +283,7 @@ class TrashbinContext implements Context { ) { // A top href (maybe without even the username) has been returned // in the response. That should never happen, or have been filtered out - // by code above. + // by the code above. throw new Exception( __METHOD__ . " Error: unexpected href in trashbin propfind at level $level: '$trashbinRef'" ); @@ -298,7 +298,7 @@ class TrashbinContext implements Context { $depth, $level + 1 ); - // filter the collection element. We only want the members. + // Filter the collection element. We only want the members. $nextFiles = \array_filter( $nextFiles, static function ($element) use ($user, $trashbinRef) { @@ -598,9 +598,7 @@ class TrashbinContext implements Context { * @param string $user * @param string $originalPath * - * @return int the number of items that were matched and requested for delete - * @throws JsonException - * @throws Exception + * @return void */ public function userTriesToDeleteFileWithOriginalPathFromTrashbinUsingTrashbinAPI(string $user, string $originalPath):void { $responseArray = $this->tryToDeleteFileFromTrashbin($user, $originalPath); @@ -623,6 +621,7 @@ class TrashbinContext implements Context { $listing = $this->listTrashbinFolder($user); $originalPath = \trim($originalPath, '/'); + $responseArray = []; foreach ($listing as $entry) { // The entry for the trashbin root can have original-location null. // That is reasonable, because the trashbin root is not something that can be restored. @@ -730,7 +729,7 @@ class TrashbinContext implements Context { $listing = []; } - // query was on the main element ? + // query was on the main element? if (\count($sections) === 1) { // already found, return return; diff --git a/tests/acceptance/features/bootstrap/WebDav.php b/tests/acceptance/features/bootstrap/WebDav.php index 3bb34b0e0..bfa3bafbe 100644 --- a/tests/acceptance/features/bootstrap/WebDav.php +++ b/tests/acceptance/features/bootstrap/WebDav.php @@ -68,7 +68,7 @@ trait WebDav { * add resource created by admin in an array * This array is used while cleaning up the resource created by admin during test run * As of now it tracks only for (files|folder) creation - * This can be expanded and modified to track other actions like (upload, deleted ..) + * This can be expanded and modified to track other actions like (upload, deleted.) */ private array $adminResources = []; @@ -169,7 +169,7 @@ trait WebDav { * @return boolean */ public function isEtagValid(string $eTag): bool { - if (\preg_match("/^\"[a-f0-9:\.]{1,32}\"$/", $eTag) + if (\preg_match("/^\"[a-f0-9:.]{1,32}\"$/", $eTag) ) { return true; } else { @@ -647,8 +647,8 @@ trait WebDav { ):void { $user = $this->getActualUsername($user); foreach ($table->getHash() as $row) { - // Allow the "filename" column to be optionally be called "foldername" - // to help readability of scenarios that test moving folders + // Allow the "filename" column to optionally be called "foldername" + // to help the readability of scenarios that test moving folders $targetName = $row['foldername'] ?? $row['filename']; $this->userMovesFileUsingTheAPI( $user, @@ -2231,7 +2231,7 @@ trait WebDav { * @return void */ public function userOnHasUploadedAFileTo(string $user, string $server, string $source, string $destination):void { - $this->userOnUploadsAFileTo($user, $server, $source, $destination, true); + $this->userOnUploadsAFileTo($user, $server, $source, $destination); $this->theHTTPStatusCodeShouldBe( ["201", "204"], "HTTP status code was not 201 or 204 while trying to upload file '$source' to '$destination' for user '$user' on server '$server'" @@ -2919,8 +2919,7 @@ trait WebDav { $user, $destination, 'a', - $bytes, - $isGivenStep + $bytes ); } @@ -2936,7 +2935,7 @@ trait WebDav { * @throws Exception */ public function userHasUploadedFileToEndingWithOfSizeBytes(string $user, string $destination, string $text, string $bytes):void { - $this->userUploadsAFileToEndingWithOfSizeBytes($user, $destination, $text, $bytes, true); + $this->userUploadsAFileToEndingWithOfSizeBytes($user, $destination, $text, $bytes); $expectedElements = new TableNode([["$destination"]]); $this->checkElementList($user, $expectedElements); } @@ -3528,7 +3527,7 @@ trait WebDav { * @return void */ public function fileHasBeenDeleted(string $file, string $user):void { - $this->userHasDeletedResource($user, "deleted", "file", $file); + $this->userHasDeletedResource($user, "deleted"); } /** @@ -3636,15 +3635,14 @@ trait WebDav { } /** - * @Given /^the user has deleted (file|folder) "([^"]*)"$/ + * @Given /^the user has deleted (?:file|folder) "([^"]*)"$/ * - * @param string $fileOrFolder * @param string $file * * @return void */ - public function theUserHasDeletedFile(string $fileOrFolder, string $file):void { - $this->userHasDeletedResource($this->getCurrentUser(), $fileOrFolder, $file); + public function theUserHasDeletedFile(string $file):void { + $this->userHasDeletedResource($this->getCurrentUser(), $file); } /** @@ -3970,7 +3968,7 @@ trait WebDav { * @param string $user * @param int $num * @param int $total - * @param string $data + * @param string|null $data * @param string $destination * * @return void @@ -4810,7 +4808,7 @@ trait WebDav { foreach ($elementList as $element) { $element = \substr((string)$element, \strlen($davPrefix)); if ($checkEachDelete) { - $this->userHasDeletedResource($user, "deleted", "file", $element); + $this->userHasDeletedResource($user, $element); } else { $this->userDeletesFile($user, $element); } @@ -4916,7 +4914,7 @@ trait WebDav { Assert::assertNotEquals( $newResponseBodyContents, // different users can download files before and after an update is made to a file - // previous response content is fetched from user response body content array for that user + // previous response content is fetched from the user response body content array entry for that user $this->userResponseBodyContents[$user], __METHOD__ . " previous and current previews content is same but expected to be different", ); @@ -5175,7 +5173,7 @@ trait WebDav { Assert::assertNotEquals( $newResponseBodyContents, // different users can download files before and after an update is made to a file - // previous response content is fetched from user response body content array for that user + // previous response content is fetched from the user response body content array entry for that user $this->userResponseBodyContents[$user], __METHOD__ . " previous and current previews content is same but expected to be different", ); @@ -5206,16 +5204,15 @@ trait WebDav { } /** - * @Given /^user "([^"]*)" has stored id of (file|folder) "([^"]*)"$/ - * @When /^user "([^"]*)" stores id of (file|folder) "([^"]*)"$/ + * @Given /^user "([^"]*)" has stored id of (?:file|folder) "([^"]*)"$/ + * @When /^user "([^"]*)" stores id of (?:file|folder) "([^"]*)"$/ * * @param string $user - * @param string $fileOrFolder * @param string $path * * @return void */ - public function userStoresFileIdForPath(string $user, string $fileOrFolder, string $path):void { + public function userStoresFileIdForPath(string $user, string $path):void { $this->storedFileID = $this->getFileIdForPath($user, $path); } @@ -5774,7 +5771,7 @@ trait WebDav { $shareRootXml = $item->xpath("d:propstat//oc:shareroot"); $href = \str_replace($spacesBaseUrl, "", $href); $resourcePath = $href; - // do not try to parse the resouce path + // do not try to parse the resource path // if the item to search is space itself if (!GraphHelper::isSpaceId($entryNameToSearch ?? '')) { $resourcePath = \substr($href, \strpos($href, '/') + 1); @@ -5785,8 +5782,8 @@ trait WebDav { } $resourcePath = \rawurldecode($resourcePath); if ($entryNameToSearch === $resourcePath) { - // If searching for single entry, - // we return an SimpleXmlElement of found item + // If searching for a single entry, + // we return a SimpleXmlElement of found item return $item; } if ($searchForHighlightString) { diff --git a/tests/acceptance/features/bootstrap/WebDavLockingContext.php b/tests/acceptance/features/bootstrap/WebDavLockingContext.php index a907e5fa4..376c27cd3 100644 --- a/tests/acceptance/features/bootstrap/WebDavLockingContext.php +++ b/tests/acceptance/features/bootstrap/WebDavLockingContext.php @@ -51,11 +51,13 @@ class WebDavLockingContext implements Context { * @param string $user * @param string $file * @param TableNode $properties table with no heading with | property | value | - * @param string $fullUrl + * @param string|null $fullUrl * @param boolean $public if the file is in a public share or not * @param boolean $expectToSucceed * * @return void + * @throws GuzzleException + * @throws JsonException */ private function lockFile( string $user, diff --git a/tests/acceptance/features/bootstrap/WebDavPropertiesContext.php b/tests/acceptance/features/bootstrap/WebDavPropertiesContext.php index c4b111109..8e550ee9d 100644 --- a/tests/acceptance/features/bootstrap/WebDavPropertiesContext.php +++ b/tests/acceptance/features/bootstrap/WebDavPropertiesContext.php @@ -188,7 +188,7 @@ class WebDavPropertiesContext implements Context { * * @param string $user * @param string $propertyName - * @param string $namespace namespace in form of "x1='http://whatever.org/ns'" + * @param string $namespace namespace in the form of "x1='http://whatever.org/ns'" * @param string $path * * @return void @@ -258,7 +258,7 @@ class WebDavPropertiesContext implements Context { /** * @param string $user user id who sets the property * @param string $propertyName name of property in Clark notation - * @param string $namespace namespace in form of "x1='http://whatever.org/ns'" + * @param string $namespace namespace in the form of "x1='http://whatever.org/ns'" * @param string $path path on which to set properties to * @param string $propertyValue property value * @@ -291,7 +291,7 @@ class WebDavPropertiesContext implements Context { * * @param string $user user id who sets the property * @param string $propertyName name of property in Clark notation - * @param string $namespace namespace in form of "x1='http://whatever.org/ns'" + * @param string $namespace namespace in the form of "x1='http://whatever.org/ns'" * @param string $path path on which to set properties to * @param string $propertyValue property value * @@ -320,7 +320,7 @@ class WebDavPropertiesContext implements Context { * * @param string $user user id who sets the property * @param string $propertyName name of property in Clark notation - * @param string $namespace namespace in form of "x1='http://whatever.org/ns'" + * @param string $namespace namespace in the form of "x1='http://whatever.org/ns'" * @param string $path path on which to set properties to * @param string $propertyValue property value * @@ -690,11 +690,10 @@ class WebDavPropertiesContext implements Context { } /** - * @param $xmlResponse + * @param SimpleXMLElement $xmlResponse * @param string $xpath * * @return string - * @throws Exception */ public function getXmlItemByXpath( SimpleXMLElement $xmlResponse, @@ -913,10 +912,9 @@ class WebDavPropertiesContext implements Context { * @param string $path * @param string $property * @param string $expectedValue - * @param string $altExpectedValue + * @param string|null $altExpectedValue * * @return void - * @throws Exception */ public function asUserFolderShouldContainAPropertyWithValueOrWithValue( string $user, @@ -1134,9 +1132,9 @@ class WebDavPropertiesContext implements Context { * only supports new DAV version * * @param string $username - * @param TableNode $expectedPropTable with following columns: + * @param TableNode $expectedPropTable with the following columns: * resource: full path of resource(file/folder/entry) from root of your oc storage - * property: expected name of property to be asserted, eg: status, href, customPropName + * property: expected name of property to be asserted, e.g.: status, href, customPropName * value: expected value of expected property * * @return void @@ -1147,13 +1145,13 @@ class WebDavPropertiesContext implements Context { $this->featureContext->verifyTableNodeColumns($expectedPropTable, ['resource', 'propertyName', 'propertyValue']); $responseXmlObject = $this->featureContext->getResponseXml(); - $hrefSplittedUptoUsername = \explode("/", (string)$responseXmlObject->xpath("//d:href")[0]); - $xmlHrefSplittedArray = \array_slice( - $hrefSplittedUptoUsername, + $hrefSplitUptoUsername = \explode("/", (string)$responseXmlObject->xpath("//d:href")[0]); + $xmlHrefSplitArray = \array_slice( + $hrefSplitUptoUsername, 0, - \array_search($username, $hrefSplittedUptoUsername) + 1 + \array_search($username, $hrefSplitUptoUsername) + 1 ); - $xmlHref = \implode("/", $xmlHrefSplittedArray); + $xmlHref = \implode("/", $xmlHrefSplitArray); foreach ($expectedPropTable->getColumnsHash() as $col) { if ($col["propertyName"] === "status") { $xmlPart = $responseXmlObject->xpath(