[tests-only] full-ci] Fix path calculation in listTrashbinFolderCollection

[phil-davis]

Description

1. fix test code problem in listTrashbinFolderCollection

	static function ($element) use ($user, $collectionPath) {
		$path = $collectionPath;
		if ($path !== "") {
			$path = $path . "/";
		}
		return ($element['href'] !== "/remote.php/dav/trash-bin/$user/$path");
	}

Before this change, when $collectionPath was empty, the code would end up with 2 / on the end for the comparison with $element['href']. It would try to match a string like /remote.php/dav/trash-bin/Alice// - and that would never match. That caused the top-level trash-bin root itself to end up in the list of items in the trash-bin for consideration by later steps.

Later code was then trying to traverse down into an invalid trashbin "folder" like /remote.php/dav/trash-bin/Alice/Alice/ which is an incorrect thing to even try to do (and looks like it might be part of the cause of trouble when running against oCIS)

2. Handle trashbin PROPFIND href problem that happens in oCIS

When doing a PROPFIND to the root of the empty trashbin in oC10, you get a response like:

<?xml version="1.0"?>
<d:multistatus xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns" xmlns:oc="http://owncloud.org/ns">
  <d:response>
    <d:href>/remote.php/dav/trash-bin/admin/</d:href>
    <d:propstat>
      <d:prop>
        <d:resourcetype><d:collection/></d:resourcetype>
      </d:prop>
      <d:status>HTTP/1.1 200 OK</d:status>
    </d:propstat>
    <d:propstat>
      <d:prop>
        <oc:trashbin-original-filename/>
        <oc:trashbin-original-location/>
        <oc:trashbin-delete-datetime/>
        <d:getcontentlength/>
      </d:prop>
      <d:status>HTTP/1.1 404 Not Found</d:status>
    </d:propstat>
  </d:response>
</d:multistatus>

But on oCIS the response is like:

<?xml version="1.0" encoding="utf-8"?>
<d:multistatus xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns" xmlns:oc="http://owncloud.org/ns">
  <d:response>
    <d:href>/remote.php/dav/trash-bin/</d:href>
    <d:propstat>
      <d:prop>
        <d:resourcetype><d:collection/></d:resourcetype>
      </d:prop>
      <d:status>HTTP/1.1 200 OK</d:status>
    </d:propstat>
    <d:propstat>
      <d:prop>
        <oc:trashbin-original-filename></oc:trashbin-original-filename>
        <oc:trashbin-original-location></oc:trashbin-original-location>
        <oc:trashbin-delete-datetime></oc:trashbin-delete-datetime>
        <d:getcontentlength></d:getcontentlength>
      </d:prop>
      <d:status>HTTP/1.1 404 Not Found</d:status>
    </d:propstat>
  </d:response>
</d:multistatus>

The "href" is missing the username at the end of the string /remote.php/dav/trash-bin/ - it should be something like /remote.php/dav/trash-bin/Alice/.

And that creates a problem for any client code that tries to use the "href" value.

How Has This Been Tested?

CI and local test runs.

oCIS CI is tested in owncloud/ocis#3638 - that gets the regular trashbin test scenarios passing like they previously did. There are other issues with using the spaces endpoint and trying to access the trashbin - those are a separate thing.

https://drone.owncloud.com/owncloud/ocis/11191/31/6

    Examples:
      | dav-path | filename1     | filename2     |
      | new      | textfile0.txt | textfile1.txt |
        │ {closure}Warning: unexpected href in trashbin propfind: /remote.php/dav/trash-bin/
        │ 
        │ {closure}Warning: unexpected href in trashbin propfind: /remote.php/dav/trash-bin/
        │ 

The test code works-around the problem, and actually the rest of the test passes. We can see in the log output that the problem is there, so that will help when fixing it. After it is fixed in reva and oCIS we then need to go back and remove the work-around from the test code. I would rather do this than have every trashbin test scenario added to expected-failures because of this newly-discovered problem.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Database schema changes (next release will require increase of minor version instead of patch)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests only (no source changes)

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised:
• Changelog item, see TEMPLATE

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[phil-davis]

I found old issue owncloud/ocis#1846 "Trashbin root element doesnt contain username on propfind" - that is exactly the problem that I have noticed here. The problem has been noticed again because of refactoring if the trashbin test code to handle the personal spaces endpoint and to do PROPFIND with depth 1 (rather than depth infinity).

It would be good to fix that issue.