Do not cache file ids in FileSystemTags inside group folders

[st3iny]

The numerical storage id returned by a cache might not be unique because it is passed down the CacheWrapper chain. Multiple caches may return the same numerical id. Additionally, when combined with the CacheJail wrapper multiple files may be mapped to the same numerical id + path combination even if they represent separate entries in the file cache.

In this case the first inserted file id for the combination of numerical id and path "wins" and blocks any subsequent insertions. This will lead to invalid tags being returned.

How to reproduce:

1. Install groupfolders and files_accesscontrol.
2. Create a group folder "Groupfolder".
3. Create another group folder nested inside the first one "Groupfolder/folder-1".
4. Create a system tag "AccessDenied".
5. Add a workflow to block access to all files with "AccessDenied" tag.
6. Tag the inner group folder at "Groupfolder/folder-1" with the new tag.
7. Try to access the inner folder at "Groupfolder/folder-1".

Expected: The access should be blocked.
Actual: You can navigate to "Groupfolder/folder-1" and list its contents.

[ChristophWurst]

👍 🐘

[blizzz]

@nickvergessen do you remember which motivation the younger @nickvergessen had when doing so in 2016? Performance (significant?) or side effects (loops?), or other?

[nickvergessen]

nickvergessen do you remember which motivation the younger nickvergessen had when doing so in 2016? Performance (significant?) or side effects (loops?), or other?

Yes basically. The paths are called X times for every request and repeated with the same path over and over again, because we do things like:

$file = $folder->get('path'); // Runs it once (2+ queries per nesting level)
if ($file->isWriteable()) { // Runs it again (2+ queries per nesting level)
$file->putContent('c'); // Runs it again (2+ queries per nesting level)

etc.

Building the system tag list every time again and again is very bad performancewise. But we also only do it on the owner level, so it worked with shares although there were JailCache applied.

But then again, also for a groupfolder the parents should be the same for everyone? Or can they be mounted to any nesting level? In that case I wouldn't even know which users tags should be considered. So maybe just ignore the cached fileIds when it's a group folder for now, so it continues to work for the rest, while we try to find a performant solution for groupfolders with the expected result.

[st3iny]

So maybe just ignore the cached fileIds when it's a group folder for now, so it continues to work for the rest, while we try to find a performant solution for groupfolders with the expected result.

I refactored it to behave this way. File ids will be cached for any storage other than group folders.

[st3iny]

/backport to stable22

[st3iny]

/backport to stable21

[st3iny]

/backport to stable20