Restrict query when searching for versions of trashbin files

[juliusknorr]

The current way of obtaining the versions from files in the trashbin is quite slow since it does a like operation on the unindexed name column. Since we know the full path of the file in the trashbin we can limit the search to the parent id. This should drastically speed up query times on large file cache tables.

We can ignore other things than single file restores since all usages have a separate code path for that.

[ChristophWurst]

👍 🐘

[rullzer]

nice one. but I'd really love the input from our storage overloard @icewind1991

[juliusknorr]

Yes, I'd highly appreciate a closer look from @icewind1991 as well.

Some more comments on my validation paths since I also don't know that code very well.

I've tested this to still work when:

• Restore a file with versions
• Restore a file with versions from within a deleted folder
• Restore a deleted folder containing files with versions

View::searchCommon also includes mountpoints in the search, so this is no longer the case however I assumed there should never be any mountpoints inside of the /[uid]/files_trashbin/versions fake root.

server/lib/private/Files/View.php

Lines 1653 to 1671 in cfde744

	$mounts = Filesystem::getMountManager()->findIn($this->fakeRoot);
	foreach ($mountsas$mount) {
	$mountPoint = $mount->getMountPoint();
	$storage = $mount->getStorage();
	if ($storage) {
	$cache = $storage->getCache('');
	$relativeMountPoint = substr($mountPoint, $rootLength);
	$results = call_user_func_array([$cache, $method], $args);
	if ($results) {
	foreach ($resultsas$result) {
	$internalPath = $result['path'];
	$result['path'] = rtrim($relativeMountPoint . $result['path'], '/');
	$path = rtrim($mountPoint . $internalPath, '/');
	$owner = \OC::$server->getUserManager()->get($storage->getOwner($internalPath));
	$files[] = newFileInfo($path, $storage, $internalPath, $result, $mount, $owner);
	}
	}
	}

[rullzer]

mmmm so for some reason the static analysis complains. Lets summon the wizards: @MorrisJobke@kesselb

[kesselb]

Oh. I'm not sure what Psalm is complaining about 🙈

• array_map always returns an array hence the is_array check is not longer required (but that should be a different line).
• $matches = array_map there are two spaces before array_map (but this is also another line - and a job for php-cs-fixer anyway)
• $ma inside the foreach is resolved (by PHPStorm) to mixed|OC\Files\Cache\CacheEntry.

Mind to try the above version? I'm not sure if Psalm is reading the inline /** @var **/ annotation. The above code works at least with PHPStrom and $ma is resolved correctly.

[juliusknorr]

Let's see if psalm is happy with that

[MorrisJobke]

Let's see if psalm is happy with that

Looks good 👍 Even one error is "fixed". 😆

[MorrisJobke]

Code itself makes sense 👍

[faily-bot]

🤖 beep boop beep 🤖

Here are the logs for the failed build:

Status of 34140: failure

mariadb10.1-php7.3

• I'm a little sad 🤖 and was not able to find the logs for this failed job - please improve me at https://github.com/MorrisJobke/drone-logs to provide this to you

mysql8.0-php7.4

• I'm a little sad 🤖 and was not able to find the logs for this failed job - please improve me at https://github.com/MorrisJobke/drone-logs to provide this to you

[MorrisJobke]

@juliushaertl Backport or not?

[MorrisJobke]

ping @juliushaertl

[juliusknorr]

/backport to stable20

[juliusknorr]

/backport to stable19

[juliusknorr]

/backport to stable18