From 2faf997f478df9825c5d1e4afe4ee65cee79c714 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 6 Oct 2026 12:52:07 +0200 Subject: [PATCH] fix(share): Hide circle shares of files in the trashbin Signed-off-by: Carl Schwan --- lib/Db/ShareWrapperRequest.php | 31 ++++++++++ lib/Model/FileCacheWrapper.php | 13 ++++- lib/Service/ShareWrapperService.php | 33 ++++++++++- lib/ShareByCircleProvider.php | 12 ++++ tests/unit/lib/Model/FileCacheWrapperTest.php | 56 +++++++++++++++++++ 5 files changed, 142 insertions(+), 3 deletions(-) create mode 100644 tests/unit/lib/Model/FileCacheWrapperTest.php diff --git a/lib/Db/ShareWrapperRequest.php b/lib/Db/ShareWrapperRequest.php index 67bd58e08..7533d6f19 100644 --- a/lib/Db/ShareWrapperRequest.php +++ b/lib/Db/ShareWrapperRequest.php @@ -13,6 +13,7 @@ use OCA\Circles\Exceptions\RequestBuilderException; use OCA\Circles\Exceptions\ShareWrapperNotFoundException; use OCA\Circles\Model\FederatedUser; +use OCA\Circles\Model\FileCacheWrapper; use OCA\Circles\Model\Membership; use OCA\Circles\Model\Probes\CircleProbe; use OCA\Circles\Model\ShareWrapper; @@ -302,6 +303,36 @@ public function getSharesByFileIds(array $fileIds, bool $getData = false, bool $ return $this->getItemsFromRequest($qb); } + /** + * returns current file cache entries, not cached, indexed by file id + * + * @param int[] $fileIds + * + * @return array + */ + public function getFileCaches(array $fileIds): array { + $fileCaches = []; + foreach (array_chunk(array_values(array_unique($fileIds)), 1000) as $chunk) { + $qb = $this->getQueryBuilder(); + $qb->select('f.fileid', 'f.path', 's.id') + ->from(self::TABLE_FILE_CACHE, 'f') + ->leftJoin('f', self::TABLE_STORAGES, 's', $qb->expr()->eq('f.storage', 's.numeric_id')) + ->where($qb->expr()->in('f.fileid', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))); + + $result = $qb->executeQuery(); + while ($row = $result->fetch()) { + $fileCache = new FileCacheWrapper(); + $fileCache->setId((int)$row['fileid']) + ->setPath((string)$row['path']) + ->setStorage((string)$row['id']); + $fileCaches[$fileCache->getId()] = $fileCache; + } + $result->closeCursor(); + } + + return $fileCaches; + } + /** * @param FederatedUser $federatedUser * @param int $nodeId diff --git a/lib/Model/FileCacheWrapper.php b/lib/Model/FileCacheWrapper.php index fa18ed72a..0eeacbad7 100644 --- a/lib/Model/FileCacheWrapper.php +++ b/lib/Model/FileCacheWrapper.php @@ -231,7 +231,18 @@ public function isAccessible(): bool { return (in_array($storageType, ['local', 'webdav', 'ftp', 'sftp', 'swift', 'smb', 'amazon'])); } - return !(explode('/', $path, 2)[0] !== 'files' && $storageType === 'home'); + // on home storages and on group folders with a separate storage, only files/ is accessible + $storage = $this->getStorage(); + if (explode('/', $path, 2)[0] !== 'files' + && (str_starts_with($storage, 'home::') + || str_starts_with($storage, 'object::user') + || str_starts_with($storage, 'object::groupfolder:') + || preg_match('#^local::.*/__groupfolders/\d+/$#', $storage) === 1)) { + return false; + } + + // group folders stored in the root storage have their trashbin in __groupfolders/trash/ + return !str_starts_with($path, '__groupfolders/trash/'); } /** diff --git a/lib/Service/ShareWrapperService.php b/lib/Service/ShareWrapperService.php index 55cfc2bff..bcfe2a73d 100644 --- a/lib/Service/ShareWrapperService.php +++ b/lib/Service/ShareWrapperService.php @@ -14,6 +14,7 @@ use OCA\Circles\Exceptions\RequestBuilderException; use OCA\Circles\Exceptions\ShareWrapperNotFoundException; use OCA\Circles\Model\FederatedUser; +use OCA\Circles\Model\FileCacheWrapper; use OCA\Circles\Model\Probes\CircleProbe; use OCA\Circles\Model\ShareWrapper; use OCA\Circles\Tools\Exceptions\InvalidItemException; @@ -169,7 +170,7 @@ public function getSharedWith( throw new InvalidItemException(); } - return $this->deserializeList($cachedData, ShareWrapper::class); + return $this->refreshFileCaches($this->deserializeList($cachedData, ShareWrapper::class)); } catch (InvalidItemException) { // Cache miss, continue to fetch from database } @@ -197,7 +198,7 @@ public function getSharedWithByPath( throw new InvalidItemException(); } - return $this->deserializeList($cachedData, ShareWrapper::class); + return $this->refreshFileCaches($this->deserializeList($cachedData, ShareWrapper::class)); } catch (InvalidItemException) { // Cache miss, continue to fetch from database } @@ -208,6 +209,34 @@ public function getSharedWithByPath( return $shares; } + /** + * Cached file cache entries are outdated when a shared file is moved to the trashbin or restored + * + * @param ShareWrapper[] $shares + * + * @return ShareWrapper[] + */ + private function refreshFileCaches(array $shares): array { + $fileCaches = $this->shareWrapperRequest->getFileCaches( + array_map(fn (ShareWrapper $share): int => $share->getFileSource(), $shares) + ); + + $refreshed = []; + foreach ($shares as $share) { + if (!array_key_exists($share->getFileSource(), $fileCaches)) { + continue; + } + + $fileCache = $share->hasFileCache() ? $share->getFileCache() : new FileCacheWrapper(); + $fileCache->setPath($fileCaches[$share->getFileSource()]->getPath()) + ->setStorage($fileCaches[$share->getFileSource()]->getStorage()); + $share->setFileCache($fileCache); + $refreshed[] = $share; + } + + return $refreshed; + } + /** * @return ShareWrapper[] * @throws RequestBuilderException diff --git a/lib/ShareByCircleProvider.php b/lib/ShareByCircleProvider.php index c5b753d4c..e2d5db08d 100644 --- a/lib/ShareByCircleProvider.php +++ b/lib/ShareByCircleProvider.php @@ -547,6 +547,12 @@ public function getSharedWith($userId, $shareType, $node, $limit, $offset): arra $probe ); + // exclude shares leading to deleted files or to the trashbin + $wrappedShares = array_filter( + $wrappedShares, + fn (ShareWrapper $wrapper): bool => $wrapper->hasFileCache() && $wrapper->getFileCache()->isAccessible() + ); + return array_filter( array_map( fn (ShareWrapper $wrapper) => $wrapper->getShare( @@ -584,6 +590,12 @@ public function getSharedWithByPath( $probe, ); + // exclude shares leading to deleted files or to the trashbin + $wrappedShares = array_filter( + $wrappedShares, + fn (ShareWrapper $wrapper): bool => $wrapper->hasFileCache() && $wrapper->getFileCache()->isAccessible() + ); + /** @var array */ return array_filter( array_map( diff --git a/tests/unit/lib/Model/FileCacheWrapperTest.php b/tests/unit/lib/Model/FileCacheWrapperTest.php new file mode 100644 index 000000000..703784fc6 --- /dev/null +++ b/tests/unit/lib/Model/FileCacheWrapperTest.php @@ -0,0 +1,56 @@ + ['home::alice', 'files/doc.txt', true], + 'home trashbin' => ['home::alice', 'files_trashbin/files/doc.txt.d1700000000', false], + 'home versions' => ['home::alice', 'files_versions/doc.txt.v1700000000', false], + 'object store home file' => ['object::user:alice', 'files/doc.txt', true], + 'object store home trashbin' => ['object::user:alice', 'files_trashbin/files/doc.txt.d1700000000', false], + 'group folder in root storage' => ['local::/var/www/html/data/', '__groupfolders/1/doc.txt', true], + 'group folder trashbin in root storage' => ['local::/var/www/html/data/', '__groupfolders/trash/1/doc.txt.d1700000000', false], + 'group folder in root object storage' => ['object::store:amazon::bucket', '__groupfolders/1/doc.txt', true], + 'group folder trashbin in root object storage' => ['object::store:amazon::bucket', '__groupfolders/trash/1/doc.txt.d1700000000', false], + 'group folder in separate local storage' => ['local::/var/www/html/data/__groupfolders/1/', 'files/doc.txt', true], + 'group folder trashbin in separate local storage' => ['local::/var/www/html/data/__groupfolders/1/', 'trash/doc.txt.d1700000000', false], + 'group folder versions in separate local storage' => ['local::/var/www/html/data/__groupfolders/1/', 'versions/1/1700000000', false], + 'group folder in separate object storage' => ['object::groupfolder:1.object::store:amazon::bucket', 'files/doc.txt', true], + 'group folder trashbin in separate object storage' => ['object::groupfolder:1.object::store:amazon::bucket', 'trash/doc.txt.d1700000000', false], + 'external storage file' => ['local::/mnt/external/', 'trash/doc.txt', true], + 'external storage root' => ['local::/mnt/external/', '', true], + 'home root' => ['home::alice', '', false], + ]; + } + + #[DataProvider('dataIsAccessible')] + public function testIsAccessible(string $storage, string $path, bool $expected): void { + $fileCache = new FileCacheWrapper(); + $fileCache->setId(42) + ->setStorage($storage) + ->setPath($path); + + $this->assertSame($expected, $fileCache->isAccessible()); + } + + public function testIsAccessibleWithoutFileId(): void { + $fileCache = new FileCacheWrapper(); + $fileCache->setStorage('home::alice') + ->setPath('files/doc.txt'); + + $this->assertFalse($fileCache->isAccessible()); + } +}