Regression in filesystem handling breaks Talk/ attachment folder

[nickvergessen]

Since the merge of #36589 yesterday all Talk/ folder related integration tests started failing.

I did a bisect:

5d4efb4d5fd8e4389856df5d94c3b92c7019e603 is the first bad commit
commit 5d4efb4d5fd8e4389856df5d94c3b92c7019e603
Author: Anna Larch <anna@nextcloud.com>
Date:   Tue Feb 7 15:55:55 2023 +0100

    Do not set up filesystem on every call
    
    Also remove old Oc_FileChunking logis that produced GC- collectable chunks
    
    Signed-off-by: Anna Larch <anna@nextcloud.com>

 apps/dav/lib/Connector/Sabre/Directory.php         |  75 +++----
 apps/dav/lib/Connector/Sabre/File.php              | 135 -------------
 apps/dav/lib/Connector/Sabre/FilesPlugin.php       |   9 -
 apps/dav/lib/Connector/Sabre/LockPlugin.php        |   4 +-
 apps/dav/lib/Connector/Sabre/ObjectTree.php        |  38 ----
 apps/dav/lib/Connector/Sabre/QuotaPlugin.php       |  17 --
 apps/dav/tests/unit/Connector/Sabre/FileTest.php   | 221 ---------------------
 .../tests/unit/Connector/Sabre/ObjectTreeTest.php  |  60 ++----
 .../tests/unit/Connector/Sabre/QuotaPluginTest.php |  91 +--------
 .../Sabre/RequestTest/RequestTestCase.php          |   2 -
 .../Connector/Sabre/RequestTest/UploadTest.php     | 110 ----------
 apps/files/appinfo/info.xml                        |   9 +-
 apps/files/composer/composer/ClassLoader.php       |  12 +-
 apps/files/composer/composer/autoload_classmap.php |   1 +
 apps/files/composer/composer/autoload_static.php   |   1 +
 apps/files/composer/composer/installed.php         |   4 +-
 .../lib/BackgroundJob/FileChunkCleanupJob.php      |  62 ++++++
 lib/base.php                                       |  17 --
 lib/composer/composer/autoload_classmap.php        |   1 -
 lib/composer/composer/autoload_static.php          |   1 -
 lib/private/Cache/File.php                         |  24 ++-
 lib/private/Server.php                             |   1 +
 lib/private/legacy/OC_FileChunking.php             | 184 -----------------
 tests/lib/FileChunkingTest.php                     |  72 -------
 24 files changed, 144 insertions(+), 1007 deletions(-)
 create mode 100644 apps/files/lib/BackgroundJob/FileChunkCleanupJob.php
 delete mode 100644 lib/private/legacy/OC_FileChunking.php
 delete mode 100644 tests/lib/FileChunkingTest.php

$ git bisect log
git bisect start
# status: waiting for both good and bad commits
# good: [39b13e096ccfd23d8b9b4527e098f53b7535f26b] Merge pull request #36675 from nextcloud/rakekniven-patch-3
git bisect good 39b13e096ccfd23d8b9b4527e098f53b7535f26b
# status: waiting for bad commit, 1 good commit known
# bad: [93e703bbfc7c8ef654b7b0185474397ec1bbaa6b] Fix(l10n): 🔠 Update translations from Transifex
git bisect bad 93e703bbfc7c8ef654b7b0185474397ec1bbaa6b
# good: [38ca4685023da0efd9b56faa8a8db22cbd6176bc] Merge pull request #36515 from nextcloud/fix/sharees-remove-deck
git bisect good 38ca4685023da0efd9b56faa8a8db22cbd6176bc
# bad: [bba3a1ccf0b41ee212cfb43ceee2b20e66904654] Merge pull request #36589 from nextcloud/enh/perf-remove-icache
git bisect bad bba3a1ccf0b41ee212cfb43ceee2b20e66904654
# good: [d6a3ebc79f405f5294803ce04386832e526e447b] Merge pull request #36751 from nextcloud/fix/mobile_breakpoint
git bisect good d6a3ebc79f405f5294803ce04386832e526e447b
# bad: [ec356504eaadba5411c30e6a06abd8eb177b6b7b] tests: Remove legacy chunking tests
git bisect bad ec356504eaadba5411c30e6a06abd8eb177b6b7b
# bad: [20058eb9dea035ea541ff351d9a1e8a0d777a7c7] tests: Fix test isolation on object storage
git bisect bad 20058eb9dea035ea541ff351d9a1e8a0d777a7c7
# bad: [5d4efb4d5fd8e4389856df5d94c3b92c7019e603] Do not set up filesystem on every call
git bisect bad 5d4efb4d5fd8e4389856df5d94c3b92c7019e603
# first bad commit: [5d4efb4d5fd8e4389856df5d94c3b92c7019e603] Do not set up filesystem on every call

I guess the problem is that one of the following events is not triggered anymore:
https://github.com/nextcloud/spreed/blob/545d3ab5cb4349dde7581bba67cd1e5facef9a1a/lib/Share/Listener.php#L39-L40

Given the state of feature freeze I would vote to go with the simple solution for now and simply revert the PR?
Once stable26 is branched off we can start a follow up to see what exactly is missing with the PR and see if we can fix that manually.

cc @miaulalala

[nickvergessen]

PS also with #36774 Talk tests still fail.

[nickvergessen]

Revert of #36589 in #36788

[juliusknorr]

This seems to be the code part where the event should get triggered but doesn't due to the missing full fs setup:

server/apps/files_sharing/lib/Controller/ShareAPIController.php

Lines 202 to 232 in a7abe2c

	$userFolder = $this->rootFolder->getUserFolder($this->currentUser);
	if ($recipientNode) {
	$node = $recipientNode;
	} else {
	$nodes = $userFolder->getById($share->getNodeId());
	if (empty($nodes)) {
	// fallback to guessing the path
	$node = $userFolder->get($share->getTarget());
	if ($node === null || $share->getTarget() === '') {
	throw new NotFoundException();
	}
	} else {
	$node = reset($nodes);
	}
	}
	$result['path'] = $userFolder->getRelativePath($node->getPath());
	if ($node instanceof Folder) {
	$result['item_type'] = 'folder';
	} else {
	$result['item_type'] = 'file';
	}
	$result['mimetype'] = $node->getMimetype();
	$result['has_preview'] = $this->previewManager->isAvailable($node);
	$result['storage_id'] = $node->getStorage()->getId();
	$result['storage'] = $node->getStorage()->getCache()->getNumericStorageId();
	$result['item_source'] = $node->getId();
	$result['file_source'] = $node->getId();
	$result['file_parent'] = $node->getParent()->getId();
	$result['file_target'] = $share->getTarget();

    And user "participant3" gets last share                                                   # SharingContext::userGetsLastShare()
10261	[Tue Feb 21 01:59:19 2023] 127.0.0.1:46394 Closing
10262	    And share is returned with                                                                # SharingContext::shareIsReturnedWith()
10263	      | uid_owner              | participant1              |
10264	      | displayname_owner      | participant1-displayname  |
10265	      | path                   | /Talk/welcome.txt         |
10266	      | item_type              | file                      |
10267	      | mimetype               | text/plain                |
10268	      | storage_id             | shared::/Talk/welcome.txt |
10269	      | file_target            | /Talk/welcome.txt         |
10270	      | share_with             | own public room           |
10271	      | share_with_displayname | Own public room           |
10272	      | token                  | A_TOKEN                   |
10273	      Field 'file_target' does not match
10274	      Failed asserting that two strings are equal.
10275	      --- Expected
10276	      +++ Actual
10277	      @@ @@
10278	      -'/Talk/welcome.txt'
10279	      +'/{TALK_PLACEHOLDER}/welcome.txt'

Maybe getById is not triggering the

[nickvergessen]

Shall we close, or you want to retry @juliushaertl ?

[juliusknorr]

We can close this one, I reopened #31357 and would look into it again at some point after 26 is out.