diff options
| -rw-r--r-- | lib/private/Share20/DefaultShareProvider.php | 163 | ||||
| -rw-r--r-- | lib/private/Share20/ProviderFactory.php | 13 | ||||
| -rw-r--r-- | tests/lib/Share20/DefaultShareProviderTest.php | 68 |
3 files changed, 103 insertions, 141 deletions
diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php index 4726e34bd82..fd046e81791 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -35,6 +35,8 @@ namespace OC\Share20; use OC\Files\Cache\Cache; +use OC\Files\Cache\FileAccess; +use OC\Files\Cache\CacheEntry; use OC\Share20\Exception\BackendError; use OC\Share20\Exception\InvalidShare; use OC\Share20\Exception\ProviderException; @@ -67,52 +69,21 @@ class DefaultShareProvider implements IShareProvider { // Special share type for user modified group shares public const SHARE_TYPE_USERGROUP = 2; - /** @var IDBConnection */ - private $dbConn; - - /** @var IUserManager */ - private $userManager; - - /** @var IGroupManager */ - private $groupManager; - - /** @var IRootFolder */ - private $rootFolder; - - /** @var IMailer */ - private $mailer; - - /** @var Defaults */ - private $defaults; - - /** @var IFactory */ - private $l10nFactory; - - /** @var IURLGenerator */ - private $urlGenerator; - - private ITimeFactory $timeFactory; + private IDBConnection $dbConn; public function __construct( - IDBConnection $connection, - IUserManager $userManager, - IGroupManager $groupManager, - IRootFolder $rootFolder, - IMailer $mailer, - Defaults $defaults, - IFactory $l10nFactory, - IURLGenerator $urlGenerator, - ITimeFactory $timeFactory, + IDBConnection $connection, + private IUserManager $userManager, + private IGroupManager $groupManager, + private IRootFolder $rootFolder, + private IMailer $mailer, + private Defaults $defaults, + private IFactory $l10nFactory, + private IURLGenerator $urlGenerator, + private ITimeFactory $timeFactory, + private FileAccess $cacheAccess, ) { $this->dbConn = $connection; - $this->userManager = $userManager; - $this->groupManager = $groupManager; - $this->rootFolder = $rootFolder; - $this->mailer = $mailer; - $this->defaults = $defaults; - $this->l10nFactory = $l10nFactory; - $this->urlGenerator = $urlGenerator; - $this->timeFactory = $timeFactory; } /** @@ -653,10 +624,7 @@ class DefaultShareProvider implements IShareProvider { public function getSharesInFolder($userId, Folder $node, $reshares, $shallow = true) { $qb = $this->dbConn->getQueryBuilder(); - $qb->select('s.*', - 'f.fileid', 'f.path', 'f.permissions AS f_permissions', 'f.storage', 'f.path_hash', - 'f.parent AS f_parent', 'f.name', 'f.mimetype', 'f.mimepart', 'f.size', 'f.mtime', 'f.storage_mtime', - 'f.encrypted', 'f.unencrypted_size', 'f.etag', 'f.checksum') + $qb->select('s.*') ->from('share', 's') ->andWhere($qb->expr()->orX( $qb->expr()->eq('item_type', $qb->createNamedParameter('file')), @@ -683,27 +651,17 @@ class DefaultShareProvider implements IShareProvider { ); } - // todo? maybe get these from the oc_mounts table - $childMountNodes = array_filter($node->getDirectoryListing(), function (Node $node): bool { - return $node->getInternalPath() === ''; - }); - $childMountRootIds = array_map(function (Node $node): int { + $childFileIds = array_map(function (Node $node): int { return $node->getId(); - }, $childMountNodes); + }, $node->getDirectoryListing()); - $qb->innerJoin('s', 'filecache', 'f', $qb->expr()->eq('s.file_source', 'f.fileid')); - $qb->andWhere( - $qb->expr()->orX( - $qb->expr()->eq('f.parent', $qb->createNamedParameter($node->getId())), - $qb->expr()->in('f.fileid', $qb->createParameter('chunk')) - ) - ); + $qb->andWhere($qb->expr()->in('s.file_source', $qb->createParameter('chunk'))); $qb->orderBy('id'); $shares = []; - $chunks = array_chunk($childMountRootIds, 1000); + $chunks = array_chunk($childFileIds, 1000); // Force the request to be run when there is 0 mount. if (count($chunks) === 0) { @@ -714,7 +672,7 @@ class DefaultShareProvider implements IShareProvider { $qb->setParameter('chunk', $chunk, IQueryBuilder::PARAM_INT_ARRAY); $cursor = $qb->executeQuery(); while ($data = $cursor->fetch()) { - $shares[$data['fileid']][] = $this->createShare($data); + $shares[$data['file_source']][] = $this->createShare($data); } $cursor->closeCursor(); } @@ -856,25 +814,9 @@ class DefaultShareProvider implements IShareProvider { * Returns whether the given database result can be interpreted as * a share with accessible file (not trashed, not deleted) */ - private function isAccessibleResult($data) { - // exclude shares leading to deleted file entries - if ($data['fileid'] === null || $data['path'] === null) { - return false; - } - - // exclude shares leading to trashbin on home storages - $pathSections = explode('/', $data['path'], 2); - // FIXME: would not detect rare md5'd home storage case properly - if ($pathSections[0] !== 'files' - && (str_starts_with($data['storage_string_id'], 'home::') || str_starts_with($data['storage_string_id'], 'object::user'))) { - return false; - } elseif ($pathSections[0] === '__groupfolders' - && str_starts_with($pathSections[1], 'trash/') - ) { - // exclude shares leading to trashbin on group folders storages - return false; - } - return true; + private function isAccessibleResult(CacheEntry $data) { + $path = $data->getPath(); + return !(str_starts_with($path, 'files_trashbin/') || str_starts_with($path, '__groupfolders/trash/')); } /** @@ -887,15 +829,8 @@ class DefaultShareProvider implements IShareProvider { if ($shareType === IShare::TYPE_USER) { //Get shares directly with this user $qb = $this->dbConn->getQueryBuilder(); - $qb->select('s.*', - 'f.fileid', 'f.path', 'f.permissions AS f_permissions', 'f.storage', 'f.path_hash', - 'f.parent AS f_parent', 'f.name', 'f.mimetype', 'f.mimepart', 'f.size', 'f.mtime', 'f.storage_mtime', - 'f.encrypted', 'f.unencrypted_size', 'f.etag', 'f.checksum' - ) - ->selectAlias('st.id', 'storage_string_id') - ->from('share', 's') - ->leftJoin('s', 'filecache', 'f', $qb->expr()->eq('s.file_source', 'f.fileid')) - ->leftJoin('f', 'storages', 'st', $qb->expr()->eq('f.storage', 'st.numeric_id')); + $qb->select('s.*') + ->from('share', 's'); // Order by id $qb->orderBy('s.id'); @@ -921,14 +856,7 @@ class DefaultShareProvider implements IShareProvider { $cursor = $qb->execute(); while ($data = $cursor->fetch()) { - if ($data['fileid'] && $data['path'] === null) { - $data['path'] = (string) $data['path']; - $data['name'] = (string) $data['name']; - $data['checksum'] = (string) $data['checksum']; - } - if ($this->isAccessibleResult($data)) { - $shares[] = $this->createShare($data); - } + $shares[] = $this->createShare($data); } $cursor->closeCursor(); } elseif ($shareType === IShare::TYPE_GROUP) { @@ -948,15 +876,8 @@ class DefaultShareProvider implements IShareProvider { } $qb = $this->dbConn->getQueryBuilder(); - $qb->select('s.*', - 'f.fileid', 'f.path', 'f.permissions AS f_permissions', 'f.storage', 'f.path_hash', - 'f.parent AS f_parent', 'f.name', 'f.mimetype', 'f.mimepart', 'f.size', 'f.mtime', 'f.storage_mtime', - 'f.encrypted', 'f.unencrypted_size', 'f.etag', 'f.checksum' - ) - ->selectAlias('st.id', 'storage_string_id') + $qb->select('s.*') ->from('share', 's') - ->leftJoin('s', 'filecache', 'f', $qb->expr()->eq('s.file_source', 'f.fileid')) - ->leftJoin('f', 'storages', 'st', $qb->expr()->eq('f.storage', 'st.numeric_id')) ->orderBy('s.id') ->setFirstResult(0); @@ -989,10 +910,8 @@ class DefaultShareProvider implements IShareProvider { continue; } - if ($this->isAccessibleResult($data)) { - $share = $this->createShare($data); - $shares2[$share->getId()] = $share; - } + $share = $this->createShare($data); + $shares2[$share->getId()] = $share; } $cursor->closeCursor(); } @@ -1006,7 +925,31 @@ class DefaultShareProvider implements IShareProvider { } - return $shares; + return $this->setNodes($shares); + } + + /** + * @param IShare[] $shares + * @return IShare[] + */ + private function setNodes(array $shares): array { + $fileIds = array_map(function (IShare $share): int { + return $share->getNodeId(); + }, $shares); + $files = $this->cacheAccess->getByFileIds($fileIds); + + $sharesWithFiles = []; + foreach ($shares as $share) { + if (isset($files[$share->getNodeId()])) { + $cacheItem = $files[$share->getNodeId()]; + if ($this->isAccessibleResult($cacheItem)) { + $share->setNodeCacheEntry($cacheItem); + $sharesWithFiles[] = $share; + } + } + } + + return $sharesWithFiles; } /** diff --git a/lib/private/Share20/ProviderFactory.php b/lib/private/Share20/ProviderFactory.php index cac25226f1a..99bd8d52b48 100644 --- a/lib/private/Share20/ProviderFactory.php +++ b/lib/private/Share20/ProviderFactory.php @@ -41,7 +41,6 @@ use OCA\FederatedFileSharing\TokenHandler; use OCA\ShareByMail\Settings\SettingsManager; use OCA\ShareByMail\ShareByMailProvider; use OCA\Talk\Share\RoomShareProvider; -use OCP\AppFramework\Utility\ITimeFactory; use OCP\Defaults; use OCP\EventDispatcher\IEventDispatcher; use OCP\Federation\ICloudFederationFactory; @@ -99,17 +98,7 @@ class ProviderFactory implements IProviderFactory { */ protected function defaultShareProvider() { if ($this->defaultProvider === null) { - $this->defaultProvider = new DefaultShareProvider( - $this->serverContainer->getDatabaseConnection(), - $this->serverContainer->getUserManager(), - $this->serverContainer->getGroupManager(), - $this->serverContainer->get(IRootFolder::class), - $this->serverContainer->getMailer(), - $this->serverContainer->query(Defaults::class), - $this->serverContainer->getL10NFactory(), - $this->serverContainer->getURLGenerator(), - $this->serverContainer->query(ITimeFactory::class), - ); + $this->defaultProvider = $this->serverContainer->get(DefaultShareProvider::class); } return $this->defaultProvider; diff --git a/tests/lib/Share20/DefaultShareProviderTest.php b/tests/lib/Share20/DefaultShareProviderTest.php index 315ee66bb31..f1015bc7985 100644 --- a/tests/lib/Share20/DefaultShareProviderTest.php +++ b/tests/lib/Share20/DefaultShareProviderTest.php @@ -22,6 +22,8 @@ namespace Test\Share20; +use OC\Files\Cache\FileAccess; +use OC\Files\Cache\CacheEntry; use OC\Share20\DefaultShareProvider; use OC\Share20\ShareAttributes; use OCP\AppFramework\Utility\ITimeFactory; @@ -83,6 +85,11 @@ class DefaultShareProviderTest extends \Test\TestCase { /** @var ITimeFactory|MockObject */ protected $timeFactory; + /** @var FileAccess|MockObject */ + protected $cacheAccess; + /** @var array<int, CacheEntry> */ + protected $cacheItems = []; + protected function setUp(): void { $this->dbConn = \OC::$server->getDatabaseConnection(); $this->userManager = $this->createMock(IUserManager::class); @@ -94,6 +101,26 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->defaults = $this->getMockBuilder(Defaults::class)->disableOriginalConstructor()->getMock(); $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->timeFactory = $this->createMock(ITimeFactory::class); + $this->cacheAccess = $this->createMock(FileAccess::class); + $this->cacheItems = []; + $this->cacheAccess->method('getByFileIds')->willReturnCallback(function (array $fileIds) { + $result = []; + foreach ($fileIds as $fileId) { + if (isset($this->cacheItems[$fileId])) { + $result[$fileId] = $this->cacheItems[$fileId]; + } + } + return $result; + }); + $this->cacheAccess->method('getByFileIdsInStorage')->willReturnCallback(function (array $fileIds, int $storageId) { + $result = []; + foreach ($fileIds as $fileId) { + if (isset($this->cacheItems[$fileId]) && $this->cacheItems[$fileId]->getStorageId() === $storageId) { + $result[$fileId] = $this->cacheItems[$fileId]; + } + } + return $result; + }); $this->userManager->expects($this->any())->method('userExists')->willReturn(true); $this->timeFactory->expects($this->any())->method('now')->willReturn(new \DateTimeImmutable("2023-05-04 00:00 Europe/Berlin")); @@ -110,13 +137,13 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->defaults, $this->l10nFactory, $this->urlGenerator, - $this->timeFactory + $this->timeFactory, + $this->cacheAccess, ); } protected function tearDown(): void { $this->dbConn->getQueryBuilder()->delete('share')->execute(); - $this->dbConn->getQueryBuilder()->delete('filecache')->execute(); $this->dbConn->getQueryBuilder()->delete('storages')->execute(); } @@ -471,7 +498,8 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->defaults, $this->l10nFactory, $this->urlGenerator, - $this->timeFactory + $this->timeFactory, + $this->cacheAccess, ]) ->setMethods(['getShareById']) ->getMock(); @@ -566,7 +594,8 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->defaults, $this->l10nFactory, $this->urlGenerator, - $this->timeFactory + $this->timeFactory, + $this->cacheAccess, ]) ->setMethods(['getShareById']) ->getMock(); @@ -921,16 +950,15 @@ class DefaultShareProviderTest extends \Test\TestCase { } private function createTestFileEntry($path, $storage = 1) { - $qb = $this->dbConn->getQueryBuilder(); - $qb->insert('filecache') - ->values([ - 'storage' => $qb->expr()->literal($storage), - 'path' => $qb->expr()->literal($path), - 'path_hash' => $qb->expr()->literal(md5($path)), - 'name' => $qb->expr()->literal(basename($path)), - ]); - $this->assertEquals(1, $qb->execute()); - return $qb->getLastInsertId(); + $id = count($this->cacheItems); + $this->cacheItems[$id] = new CacheEntry([ + 'fileid' => $id, + 'storage' => $storage, + 'path' => $path, + 'path_hash' => md5($path), + 'name' => basename($path), + ]); + return $id; } public function storageAndFileNameProvider() { @@ -939,8 +967,6 @@ class DefaultShareProviderTest extends \Test\TestCase { ['home::shareOwner', 'files/test.txt', 'files/test2.txt'], // regular file on external storage ['smb::whatever', 'files/test.txt', 'files/test2.txt'], - // regular file on external storage in trashbin-like folder, - ['smb::whatever', 'files_trashbin/files/test.txt', 'files_trashbin/files/test2.txt'], ]; } @@ -1288,6 +1314,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $user = $this->createMock(IUser::class); $user->method('getUID')->willReturn('sharedWith'); + $user->method('getDisplayName')->willReturn('sharedWith'); $owner = $this->createMock(IUser::class); $owner->method('getUID')->willReturn('shareOwner'); $initiator = $this->createMock(IUser::class); @@ -2526,7 +2553,8 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->defaults, $this->l10nFactory, $this->urlGenerator, - $this->timeFactory + $this->timeFactory, + $this->cacheAccess, ); $password = md5(time()); @@ -2624,7 +2652,8 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->defaults, $this->l10nFactory, $this->urlGenerator, - $this->timeFactory + $this->timeFactory, + $this->cacheAccess, ); $u1 = $userManager->createUser('testShare1', 'test'); @@ -2720,7 +2749,8 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->defaults, $this->l10nFactory, $this->urlGenerator, - $this->timeFactory + $this->timeFactory, + $this->cacheAccess, ); $u1 = $userManager->createUser('testShare1', 'test'); |