[full-ci] Test indirect resource existence leaks

[butonic]

We now expect a not found error instead of permission denied error for some trash interactions.

Needed for cs3org/reva#3300

Driven by owncloud/ocis#3561

[ownclouders]

💥 Acceptance tests pipeline cliManageApps-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/36948/113

[butonic]

@phil-davis so, I need to duplicate these, right?

runsh: Total unexpected failed scenarios throughout the test run:
apiTrashbin/trashbinFilesFolders.feature:218
apiTrashbinRestore/trashbinRestore.feature:204
apiTrashbinRestore/trashbinRestore.feature:205

[phil-davis]

so, I need to duplicate these, right?

If there are different results expected on oC10 and oCIS then you need to have Scenario Outline with multiple Examples tables with the expected results for each implementation. (and put the appropriate skipOnOcV10 or skipOnOcis tags so that the examples only run on the correct implementations.

That causes line numbers to change - a pain for expected failures! But I haven't been able to find a way to avoid that.

Another approach is to make the test expectations more general, for example:
Then the HTTP status code should be "401" or "404"

Then the HTTP status code should be between "400" and "499"

If the requirement of any implementation is just that, for example, a 4xx status is returned, then you can just test for that. But that means that clients have to know that requirement and be capable of accepting any of the allowed statuses, because the detail of a server implementation could change and the tests continue to pass.

[phil-davis]

Example tables can be in this format:

    @skipOnOcis
    Examples:
      | ocs_api_version | ocs_status_code | path                  |
      | 1               | 100             | /Shares/textfile0.txt |
      | 2               | 200             | /Shares/textfile0.txt |
    @skipOnOcV10
    Examples:
      | ocs_api_version | ocs_status_code | path           |
      | 1               | 100             | /textfile0.txt |
      | 2               | 200             | /textfile0.txt |

That allows different particular examples for each implementation.

[phil-davis]

You might want to add [full-ci] to the PR title, so that all the pipelines run to completion and you can then look through and see all the things that fail.

[butonic]

is ci flaky? I get tons of su-exec: not found or similar ...

[phil-davis]

is ci flaky? I get tons of su-exec: not found or similar ...

Fixed by #40409 - please rebase.

[butonic]

@phil-davis hmmmm is there code in the testsute that can deal with the /dav/spaces/trash-bin/{spaceid} endpoint that was introduced with cs3org/reva#2628 by @C0rby

I only found:

	/**
	 * List a collection in the trashbin
	 *
	 * @param string|null $user user
	 * @param string|null $collectionPath the string of ids of the folder and sub-folders
	 * @param string $depth
	 * @param int $level
	 *
	 * @return array response
	 * @throws Exception
	 */
	public function listTrashbinFolderCollection(?string $user, ?string $collectionPath = "", string $depth = "1", int $level = 1):array {
		// $collectionPath should be some list of file-ids like 2147497661/2147497662
		// or the empty string, which will list the whole trashbin from the top.
		$collectionPath = \trim($collectionPath, "/");
		$password = $this->featureContext->getPasswordForUser($user);
		$davPathVersion = $this->featureContext->getDavPathVersion();
		$response = WebDavHelper::listFolder(
			$this->featureContext->getBaseUrl(),
			$user,
			$password,
			$collectionPath,
			$depth,
			$this->featureContext->getStepLineRef(),
			[
				'oc:trashbin-original-filename',
				'oc:trashbin-original-location',
				'oc:trashbin-delete-timestamp',
				'd:resourcetype',
				'd:getlastmodified'
			],
			'trash-bin',
			$davPathVersion
		);
		$responseXml = HttpRequestHelper::getResponseXml(
			$response,
			__METHOD__ . " $collectionPath"
		);

		$files = $this->getTrashbinContentFromResponseXml($responseXml);
		// filter out the collection itself, we only want to return the members
		$files = \array_filter(
			$files,
			static function ($element) use ($user, $collectionPath) {
				$path = $collectionPath;
				if ($path !== "") {
					$path = $path . "/";
				}
				return ($element['href'] !== "/remote.php/dav/trash-bin/$user/$path");
			}
		);

		foreach ($files as $file) {
			// check for unexpected/invalid href values and fail early in order to
			// avoid "common" situations that could cause infinite recursion.
			$trashbinRef = $file["href"];
			$trimmedTrashbinRef = \trim($trashbinRef, "/");
			$expectedStart = "remote.php/dav/trash-bin/$user";
			$expectedStartLength = \strlen($expectedStart);
			if ((\substr($trimmedTrashbinRef, 0, $expectedStartLength) !== $expectedStart)
				|| (\strlen($trimmedTrashbinRef) === $expectedStartLength)
			) {
				// 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.
				throw new Exception(
					__METHOD__ . " Error: unexpected href in trashbin propfind at level $level: '$trashbinRef'"
				);
			}
			if ($file["collection"]) {
				$trimmedHref = \trim($trashbinRef, "/");
				$explodedHref = \explode("/", $trimmedHref);
				$trashbinId = $collectionPath . "/" . end($explodedHref);
				$nextFiles = $this->listTrashbinFolderCollection(
					$user,
					$trashbinId,
					$depth,
					$level + 1
				);
				// filter the collection element. We only want the members.
				$nextFiles = \array_filter(
					$nextFiles,
					static function ($element) use ($user, $trashbinRef) {
						return ($element['href'] !== $trashbinRef);
					}
				);
				\array_push($files, ...$nextFiles);
			}
		}
		return $files;
	}

and in the webdavhelpers getDavPath

			if ($type === "trash-bin") {
				return "/remote.php/dav/spaces/trash-bin/" . $spaceId . '/';
			}

but it does not seem to deal with the hrefs in the response

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[butonic]

@phil-davis @individual-it I also ran into owncloud/ocis#4820. The way the test is written right now can never work. Should we just leave out the spaces line?