Skip to content

Commit

Permalink
refactor: don't join on filecache in defaultshareprovider
Browse files Browse the repository at this point in the history 
Signed-off-by: Robin Appelman <robin@icewind.nl>
  • Loading branch information
@icewind1991
icewind1991 committed May 22, 2024
1 parent 221883f commit 5d0ab68
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 141 deletions.
163 changes: 53 additions & 110 deletions lib/private/Share20/DefaultShareProvider.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -657,10 +628,7 @@ public function getSharesInFolder($userId, Folder $node, $reshares, $shallow = t
}

$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')),
Expand All @@ -687,27 +655,17 @@ public function getSharesInFolder($userId, Folder $node, $reshares, $shallow = t
);
}

// 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) {
Expand All @@ -718,7 +676,7 @@ public function getSharesInFolder($userId, Folder $node, $reshares, $shallow = t
$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();
}
Expand Down Expand Up @@ -860,25 +818,9 @@ public function getSharesByPath(Node $path) {
* 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/'));
}

/**
Expand All @@ -891,15 +833,8 @@ public function getSharedWith($userId, $shareType, $node, $limit, $offset) {
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');
Expand All @@ -925,14 +860,7 @@ public function getSharedWith($userId, $shareType, $node, $limit, $offset) {
$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) {
Expand All @@ -952,15 +880,8 @@ public function getSharedWith($userId, $shareType, $node, $limit, $offset) {
}

$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);

Expand Down Expand Up @@ -993,10 +914,8 @@ public function getSharedWith($userId, $shareType, $node, $limit, $offset) {
continue;
}

if ($this->isAccessibleResult($data)) {
$share = $this->createShare($data);
$shares2[$share->getId()] = $share;
}
$share = $this->createShare($data);
$shares2[$share->getId()] = $share;
}
$cursor->closeCursor();
}
Expand All @@ -1010,7 +929,31 @@ public function getSharedWith($userId, $shareType, $node, $limit, $offset) {
}


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;
}

/**
Expand Down
13 changes: 1 addition & 12 deletions lib/private/Share20/ProviderFactory.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
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;
Expand Down Expand Up @@ -99,17 +98,7 @@ public function registerProvider(string $shareProviderClass): void {
*/
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;
Expand Down
Loading

0 comments on commit 5d0ab68

Please sign in to comment.