diff options
author | Carl Schwan <carl@carlschwan.eu> | 2022-04-19 13:05:38 +0200 |
---|---|---|
committer | Côme Chilliet (Rebase PR Action) <come-nc@users.noreply.github.com> | 2022-09-29 15:15:28 +0000 |
commit | 92a5a8f075fe5e6b72037cc4a668090d3a8cacf5 (patch) | |
tree | 198134dd6411f82a077a70b37bec735024437e1d | |
parent | b4708fb9f003a306edcfe0346e25f9583c4c162a (diff) | |
download | nextcloud-server-92a5a8f075fe5e6b72037cc4a668090d3a8cacf5.tar.gz nextcloud-server-92a5a8f075fe5e6b72037cc4a668090d3a8cacf5.zip |
Cleanup tags and Share component
- Port to LoggerInterface
- Use IDBConnection and IQueryBuilder instead of raw SQL and OC_DB
- Use IEventListener instead of hooks
- Remove the now unused OC_DB and OC_DB_StatementWrapper legacy utils
Signed-off-by: Carl Schwan <carl@carlschwan.eu>
-rw-r--r-- | core/Application.php | 10 | ||||
-rw-r--r-- | lib/composer/composer/autoload_classmap.php | 2 | ||||
-rw-r--r-- | lib/composer/composer/autoload_static.php | 2 | ||||
-rw-r--r-- | lib/private/Accounts/Hooks.php | 3 | ||||
-rw-r--r-- | lib/private/Files/View.php | 6 | ||||
-rw-r--r-- | lib/private/Metadata/FileEventListener.php | 5 | ||||
-rw-r--r-- | lib/private/Share/Share.php | 405 | ||||
-rw-r--r-- | lib/private/Share20/UserRemovedListener.php | 3 | ||||
-rw-r--r-- | lib/private/TagManager.php | 81 | ||||
-rw-r--r-- | lib/private/Tags.php | 389 | ||||
-rw-r--r-- | lib/private/legacy/OC_DB.php | 184 | ||||
-rw-r--r-- | lib/private/legacy/OC_DB_StatementWrapper.php | 135 | ||||
-rw-r--r-- | lib/private/legacy/OC_Util.php | 9 | ||||
-rw-r--r-- | lib/public/ITags.php | 34 | ||||
-rw-r--r-- | tests/lib/Share/ShareTest.php | 166 | ||||
-rw-r--r-- | tests/lib/TagsTest.php | 8 |
16 files changed, 586 insertions, 856 deletions
diff --git a/core/Application.php b/core/Application.php index 158375984d1..749f2d176d4 100644 --- a/core/Application.php +++ b/core/Application.php @@ -49,6 +49,7 @@ use OC\DB\MissingIndexInformation; use OC\DB\MissingPrimaryKeyInformation; use OC\DB\SchemaWrapper; use OC\Metadata\FileEventListener; +use OC\TagManager; use OCP\AppFramework\App; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Events\Node\NodeDeletedEvent; @@ -78,7 +79,7 @@ class Application extends App { $server = $container->getServer(); /** @var IEventDispatcher $eventDispatcher */ - $eventDispatcher = $server->query(IEventDispatcher::class); + $eventDispatcher = $server->get(IEventDispatcher::class); $notificationManager = $server->getNotificationManager(); $notificationManager->registerNotifierService(CoreNotifier::class); @@ -325,10 +326,15 @@ class Application extends App { /** @var IConfig $config */ $config = $container->get(IConfig::class); if ($config->getSystemValueBool('enable_file_metadata', true)) { - $eventDispatcher = \OC::$server->get(IEventDispatcher::class); + /** @psalm-suppress InvalidArgument */ $eventDispatcher->addServiceListener(NodeDeletedEvent::class, FileEventListener::class); + /** @psalm-suppress InvalidArgument */ $eventDispatcher->addServiceListener(NodeRemovedFromCache::class, FileEventListener::class); + /** @psalm-suppress InvalidArgument */ $eventDispatcher->addServiceListener(NodeWrittenEvent::class, FileEventListener::class); } + + // Tags + $eventDispatcher->addServiceListener(UserDeletedEvent::class, TagManager::class); } } diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index dad4a864f8d..ededda8d55c 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1595,8 +1595,6 @@ return array( 'OC\\User\\User' => $baseDir . '/lib/private/User/User.php', 'OC_API' => $baseDir . '/lib/private/legacy/OC_API.php', 'OC_App' => $baseDir . '/lib/private/legacy/OC_App.php', - 'OC_DB' => $baseDir . '/lib/private/legacy/OC_DB.php', - 'OC_DB_StatementWrapper' => $baseDir . '/lib/private/legacy/OC_DB_StatementWrapper.php', 'OC_Defaults' => $baseDir . '/lib/private/legacy/OC_Defaults.php', 'OC_EventSource' => $baseDir . '/lib/private/legacy/OC_EventSource.php', 'OC_FileChunking' => $baseDir . '/lib/private/legacy/OC_FileChunking.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index f2436f4302f..d2cdea38a81 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1628,8 +1628,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\User\\User' => __DIR__ . '/../../..' . '/lib/private/User/User.php', 'OC_API' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_API.php', 'OC_App' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_App.php', - 'OC_DB' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_DB.php', - 'OC_DB_StatementWrapper' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_DB_StatementWrapper.php', 'OC_Defaults' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_Defaults.php', 'OC_EventSource' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_EventSource.php', 'OC_FileChunking' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_FileChunking.php', diff --git a/lib/private/Accounts/Hooks.php b/lib/private/Accounts/Hooks.php index 93918284180..b440bc394d6 100644 --- a/lib/private/Accounts/Hooks.php +++ b/lib/private/Accounts/Hooks.php @@ -32,6 +32,9 @@ use OCP\IUser; use OCP\User\Events\UserChangedEvent; use Psr\Log\LoggerInterface; +/** + * @template-implements IEventListener<UserChangedEvent> + */ class Hooks implements IEventListener { /** @var IAccountManager */ diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php index e5394e72ffe..294912e698b 100644 --- a/lib/private/Files/View.php +++ b/lib/private/Files/View.php @@ -50,6 +50,8 @@ use Icewind\Streams\CallbackWrapper; use OC\Files\Mount\MoveableMount; use OC\Files\Storage\Storage; use OC\User\LazyUser; +use OC\Share\Share; +use OC\User\User; use OCA\Files_Sharing\SharedMount; use OCP\Constants; use OCP\Files\Cache\ICacheEntry; @@ -1800,10 +1802,10 @@ class View { } // check if any of the parents were shared by the current owner (include collections) - $shares = \OCP\Share::getItemShared( + $shares = Share::getItemShared( 'folder', $fileId, - \OCP\Share::FORMAT_NONE, + \OC\Share\Constants::FORMAT_NONE, null, true ); diff --git a/lib/private/Metadata/FileEventListener.php b/lib/private/Metadata/FileEventListener.php index 6d41ccdef30..162e85ff3aa 100644 --- a/lib/private/Metadata/FileEventListener.php +++ b/lib/private/Metadata/FileEventListener.php @@ -33,6 +33,11 @@ use OCP\Files\NotFoundException; use OCP\Files\FileInfo; use Psr\Log\LoggerInterface; +/** + * @template-implements IEventListener<NodeRemovedFromCache> + * @template-implements IEventListener<NodeDeletedEvent> + * @template-implements IEventListener<NodeWrittenEvent> + */ class FileEventListener implements IEventListener { private IMetadataManager $manager; private LoggerInterface $logger; diff --git a/lib/private/Share/Share.php b/lib/private/Share/Share.php index f47c042df29..9e7b913d60a 100644 --- a/lib/private/Share/Share.php +++ b/lib/private/Share/Share.php @@ -32,9 +32,13 @@ * along with this program. If not, see <http://www.gnu.org/licenses/> * */ + namespace OC\Share; +use OCA\Files_Sharing\ShareBackend\File; +use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IDBConnection; use OCP\Share\IShare; use Psr\Log\LoggerInterface; @@ -59,11 +63,13 @@ class Share extends Constants { * * Apps are required to handle permissions on their own, this class only * stores and manages the permissions of shares + * * @see lib/public/constants.php */ /** * Register a sharing backend class that implements OCP\Share_Backend for an item type + * * @param string $itemType Item type * @param string $class Backend class * @param string $collectionOf (optional) Depends on item type @@ -83,13 +89,14 @@ class Share extends Constants { \OC::$server->get(LoggerInterface::class)->warning( 'Sharing backend '.$class.' not registered, '.self::$backendTypes[$itemType]['class'] .' is already registered for '.$itemType, - ['app' => 'OCP\Share']); + ['app' => 'files_sharing']); } return false; } /** * Get the items of item type shared with the current user + * * @param string $itemType * @param int $format (optional) Format type must be defined by the backend * @param mixed $parameters (optional) @@ -106,6 +113,7 @@ class Share extends Constants { /** * Get the items of item type shared with a user + * * @param string $itemType * @param string $user id for which user we want the shares * @param int $format (optional) Format type must be defined by the backend @@ -123,53 +131,46 @@ class Share extends Constants { /** * Get the item of item type shared with a given user by source + * * @param string $itemType * @param string $itemSource - * @param string $user User to whom the item was shared - * @param string $owner Owner of the share - * @param int $shareType only look for a specific share type + * @param ?string $user User to whom the item was shared + * @param ?string $owner Owner of the share + * @param ?int $shareType only look for a specific share type * @return array Return list of items with file_target, permissions and expiration + * @throws Exception */ - public static function getItemSharedWithUser($itemType, $itemSource, $user, $owner = null, $shareType = null) { + public static function getItemSharedWithUser(string $itemType, string $itemSource, ?string $user = null, ?string $owner = null, ?int $shareType = null) { $shares = []; - $fileDependent = false; - - $where = 'WHERE'; - $fileDependentWhere = ''; - if ($itemType === 'file' || $itemType === 'folder') { - $fileDependent = true; + $fileDependent = $itemType === 'file' || $itemType === 'folder'; + $qb = self::getSelectStatement(self::FORMAT_NONE, $fileDependent); + $qb->from('share', 's'); + if ($fileDependent) { + $qb->innerJoin('s', 'filecache', 'f', $qb->expr()->eq('file_source', 'f.fileid')); + $qb->innerJoin('s', 'storages', 'st', $qb->expr()->eq('numeric_id', 'f.storage')); $column = 'file_source'; - $fileDependentWhere = 'INNER JOIN `*PREFIX*filecache` ON `file_source` = `*PREFIX*filecache`.`fileid` '; - $fileDependentWhere .= 'INNER JOIN `*PREFIX*storages` ON `numeric_id` = `*PREFIX*filecache`.`storage` '; } else { $column = 'item_source'; } - $select = self::createSelectStatement(self::FORMAT_NONE, $fileDependent); + $qb->where($qb->expr()->eq($column, $qb->createNamedParameter($itemSource))) + ->andWhere($qb->expr()->eq('item_type', $qb->createNamedParameter($itemType))); - $where .= ' `' . $column . '` = ? AND `item_type` = ? '; - $arguments = [$itemSource, $itemType]; // for link shares $user === null if ($user !== null) { - $where .= ' AND `share_with` = ? '; - $arguments[] = $user; + $qb->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter($user))); } if ($shareType !== null) { - $where .= ' AND `share_type` = ? '; - $arguments[] = $shareType; + $qb->andWhere($qb->expr()->eq('share_type', $qb->createNamedParameter($shareType, IQueryBuilder::PARAM_INT))); } if ($owner !== null) { - $where .= ' AND `uid_owner` = ? '; - $arguments[] = $owner; + $qb->andWhere($qb->expr()->eq('uid_owner', $qb->createNamedParameter($owner))); } - $query = \OC_DB::prepare('SELECT ' . $select . ' FROM `*PREFIX*share` '. $fileDependentWhere . $where); - - $result = \OC_DB::executeAudited($query, $arguments); - - while ($row = $result->fetchRow()) { + $result = $qb->executeQuery(); + while ($row = $result->fetch()) { if ($fileDependent && !self::isFileReachable($row['path'], $row['storage_id'])) { continue; } @@ -183,7 +184,7 @@ class Share extends Constants { $path = substr($path, strlen($owner) + 1); //normalize path to 'files/foo.txt` $row['path'] = $path; } else { - \OC::$server->getLogger()->warning( + \OC::$server->get(LoggerInterface::class)->warning( 'Could not resolve mount point for ' . $row['storage_id'], ['app' => 'OCP\Share'] ); @@ -193,7 +194,7 @@ class Share extends Constants { } $result->closeCursor(); - //if didn't found a result than let's look for a group share. + // if we didn't found a result then let's look for a group share. if (empty($shares) && $user !== null) { $userObject = \OC::$server->getUserManager()->get($user); $groups = []; @@ -202,28 +203,27 @@ class Share extends Constants { } if (!empty($groups)) { - $where = $fileDependentWhere . ' WHERE `' . $column . '` = ? AND `item_type` = ? AND `share_with` in (?)'; - $arguments = [$itemSource, $itemType, $groups]; - $types = [null, null, IQueryBuilder::PARAM_STR_ARRAY]; + $qb = self::getSelectStatement(self::FORMAT_NONE, $fileDependent); + $qb->from('share', 's'); - if ($owner !== null) { - $where .= ' AND `uid_owner` = ?'; - $arguments[] = $owner; - $types[] = null; + if ($fileDependent) { + $qb->innerJoin('s', 'filecache', 'f', $qb->expr()->eq('file_source', 'f.fileid')) + ->innerJoin('s', 'storages', 'st', $qb->expr()->eq('numeric_id', 'f.storage')); } - // TODO: inject connection, hopefully one day in the future when this - // class isn't static anymore... - $conn = \OC::$server->getDatabaseConnection(); - $result = $conn->executeQuery( - 'SELECT ' . $select . ' FROM `*PREFIX*share` ' . $where, - $arguments, - $types - ); + $qb->where($qb->expr()->eq($column, $qb->createNamedParameter($itemSource))) + ->andWhere($qb->expr()->eq('item_type', $qb->createNamedParameter($itemType, IQueryBuilder::PARAM_STR))) + ->andWhere($qb->expr()->in('share_with', $qb->createNamedParameter($groups, IQueryBuilder::PARAM_STR_ARRAY))); + + if ($owner !== null) { + $qb->andWhere($qb->expr()->eq('uid_owner', $qb->createNamedParameter($owner))); + } + $result = $qb->executeQuery(); while ($row = $result->fetch()) { $shares[] = $row; } + $result->closeCursor(); } } @@ -232,6 +232,7 @@ class Share extends Constants { /** * Get the shared item of item type owned by the current user + * * @param string $itemType * @param string $itemSource * @param int $format (optional) Format type must be defined by the backend @@ -250,9 +251,10 @@ class Share extends Constants { /** * Get the backend class for the specified item type + * * @param string $itemType - * @throws \Exception * @return \OCP\Share_Backend + * @throws \Exception */ public static function getBackend($itemType) { $l = \OC::$server->getL10N('lib'); @@ -285,6 +287,7 @@ class Share extends Constants { /** * Check if resharing is allowed + * * @return boolean true if allowed or false * * Resharing is allowed by default if not configured @@ -302,10 +305,11 @@ class Share extends Constants { /** * Get a list of collection item types for the specified item type + * * @param string $itemType - * @return array + * @return array|false */ - private static function getCollectionItemTypes($itemType) { + private static function getCollectionItemTypes(string $itemType) { $collectionTypes = [$itemType]; foreach (self::$backendTypes as $type => $backend) { if (in_array($backend['collectionOf'], $collectionTypes)) { @@ -326,6 +330,7 @@ class Share extends Constants { /** * Get shared items from the database + * * @param string $itemType * @param string $item Item source or target (optional) * @param int $shareType SHARE_TYPE_USER, SHARE_TYPE_GROUP, SHARE_TYPE_LINK, $shareTypeUserAndGroups, or $shareTypeGroupUserUnique @@ -344,31 +349,36 @@ class Share extends Constants { * Refactoring notes: * * defacto $limit, $itemsShareWithBySource, $checkExpireDate, $parameters and $format is always the default and therefore is removed in the subsequent call */ - public static function getItems($itemType, $item = null, $shareType = null, $shareWith = null, + public static function getItems($itemType, ?string $item = null, ?int $shareType = null, $shareWith = null, $uidOwner = null, $format = self::FORMAT_NONE, $parameters = null, $limit = -1, $includeCollections = false, $itemShareWithBySource = false, $checkExpireDate = true) { if (\OC::$server->getConfig()->getAppValue('core', 'shareapi_enabled', 'yes') != 'yes') { return []; } + $fileDependent = $itemType == 'file' || $itemType == 'folder'; + $qb = self::getSelectStatement(self::FORMAT_NONE, $fileDependent, $uidOwner); + $qb->from('share', 's'); + $backend = self::getBackend($itemType); $collectionTypes = false; // Get filesystem root to add it to the file target and remove from the // file source, match file_source with the file cache - if ($itemType == 'file' || $itemType == 'folder') { + if ($fileDependent) { if (!is_null($uidOwner)) { $root = \OC\Files\Filesystem::getRoot(); } else { $root = ''; } - $where = 'INNER JOIN `*PREFIX*filecache` ON `file_source` = `*PREFIX*filecache`.`fileid` '; - if (!isset($item)) { - $where .= ' AND `file_target` IS NOT NULL '; + if (isset($item)) { + $qb->innerJoin('s', 'filecache', 'f', $qb->expr()->eq('file_source', 'f.fileid')); + } else { + $qb->innerJoin('s', 'filecache', 'f', $qb->expr()->andX( + $qb->expr()->eq('file_source', 'f.fileid'), + $qb->expr()->isNotNull('file_target') + )); } - $where .= 'INNER JOIN `*PREFIX*storages` ON `numeric_id` = `*PREFIX*filecache`.`storage` '; - $fileDependent = true; - $queryArgs = []; + $qb->innerJoin('s', 'storages', 'st', $qb->expr()->eq('numeric_id', 'f.storage')); } else { - $fileDependent = false; $root = ''; $collectionTypes = self::getCollectionItemTypes($itemType); if ($includeCollections && !isset($item) && $collectionTypes) { @@ -378,25 +388,21 @@ class Share extends Constants { } else { $itemTypes = $collectionTypes; } - $placeholders = implode(',', array_fill(0, count($itemTypes), '?')); - $where = ' WHERE `item_type` IN ('.$placeholders.'))'; - $queryArgs = $itemTypes; + $qb->where($qb->expr()->in('item_type', $qb->createNamedParameter($itemTypes, IQueryBuilder::PARAM_STR_ARRAY))); } else { - $where = ' WHERE `item_type` = ?'; - $queryArgs = [$itemType]; + $qb->where($qb->expr()->eq('item_type', $qb->createNamedParameter($itemType))); } } if (\OC::$server->getConfig()->getAppValue('core', 'shareapi_allow_links', 'yes') !== 'yes') { - $where .= ' AND `share_type` != ?'; - $queryArgs[] = IShare::TYPE_LINK; + $qb->andWhere($qb->expr()->neq('share_type', $qb->createNamedParameter(IShare::TYPE_LINK, IQueryBuilder::PARAM_INT))); } if (isset($shareType)) { // Include all user and group items - if ($shareType == self::$shareTypeUserAndGroups && isset($shareWith)) { - $where .= ' AND ((`share_type` in (?, ?) AND `share_with` = ?) '; - $queryArgs[] = IShare::TYPE_USER; - $queryArgs[] = self::$shareTypeGroupUserUnique; - $queryArgs[] = $shareWith; + if ($shareType === self::$shareTypeUserAndGroups && isset($shareWith)) { + $qb->andWhere($qb->expr()->andX( + $qb->expr()->in('share_type', $qb->createNamedParameter([IShare::TYPE_USER, self::$shareTypeGroupUserUnique], IQueryBuilder::PARAM_INT_ARRAY)), + $qb->expr()->eq('share_with', $qb->createNamedParameter($shareWith)) + )); $user = \OC::$server->getUserManager()->get($shareWith); $groups = []; @@ -404,31 +410,26 @@ class Share extends Constants { $groups = \OC::$server->getGroupManager()->getUserGroupIds($user); } if (!empty($groups)) { - $placeholders = implode(',', array_fill(0, count($groups), '?')); - $where .= ' OR (`share_type` = ? AND `share_with` IN ('.$placeholders.')) '; - $queryArgs[] = IShare::TYPE_GROUP; - $queryArgs = array_merge($queryArgs, $groups); + $qb->orWhere($qb->expr()->andX( + $qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_GROUP, IQueryBuilder::PARAM_INT)), + $qb->expr()->in('share_with', $qb->createNamedParameter($groups, IQueryBuilder::PARAM_STR_ARRAY)) + )); } - $where .= ')'; + // Don't include own group shares - $where .= ' AND `uid_owner` != ?'; - $queryArgs[] = $shareWith; + $qb->andWhere($qb->expr()->neq('uid_owner', $qb->createNamedParameter($shareWith))); } else { - $where .= ' AND `share_type` = ?'; - $queryArgs[] = $shareType; + $qb->andWhere($qb->expr()->eq('share_type', $qb->createNamedParameter($shareType, IQueryBuilder::PARAM_INT))); if (isset($shareWith)) { - $where .= ' AND `share_with` = ?'; - $queryArgs[] = $shareWith; + $qb->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter($shareWith, IQueryBuilder::PARAM_STR))); } } } if (isset($uidOwner)) { - $where .= ' AND `uid_owner` = ?'; - $queryArgs[] = $uidOwner; + $qb->andWhere($qb->expr()->eq('uid_owner', $qb->createNamedParameter($uidOwner))); if (!isset($shareType)) { // Prevent unique user targets for group shares from being selected - $where .= ' AND `share_type` != ?'; - $queryArgs[] = self::$shareTypeGroupUserUnique; + $qb->andWhere($qb->expr()->neq('share_type', $qb->createNamedParameter(self::$shareTypeGroupUserUnique, IQueryBuilder::PARAM_INT))); } if ($fileDependent) { $column = 'file_source'; @@ -444,54 +445,53 @@ class Share extends Constants { } if (isset($item)) { $collectionTypes = self::getCollectionItemTypes($itemType); - if ($includeCollections && $collectionTypes && !in_array('folder', $collectionTypes)) { - $where .= ' AND ('; - } else { - $where .= ' AND'; - } // If looking for own shared items, check item_source else check item_target if (isset($uidOwner)) { // If item type is a file, file source needs to be checked in case the item was converted if ($fileDependent) { - $where .= ' `file_source` = ?'; + $expr = $qb->expr()->eq('file_source', $qb->createNamedParameter($item)); $column = 'file_source'; } else { - $where .= ' `item_source` = ?'; + $expr = $qb->expr()->eq('item_source', $qb->createNamedParameter($item)); $column = 'item_source'; } } else { if ($fileDependent) { - $where .= ' `file_target` = ?'; $item = \OC\Files\Filesystem::normalizePath($item); + $expr = $qb->expr()->eq('file_target', $qb->createNamedParameter($item)); } else { - $where .= ' `item_target` = ?'; + $expr = $qb->expr()->eq('item_target', $qb->createNamedParameter($item)); } } - $queryArgs[] = $item; if ($includeCollections && $collectionTypes && !in_array('folder', $collectionTypes)) { - $placeholders = implode(',', array_fill(0, count($collectionTypes), '?')); - $where .= ' OR `item_type` IN ('.$placeholders.'))'; - $queryArgs = array_merge($queryArgs, $collectionTypes); - } + $qb->andWhere($qb->expr()->orX( + $expr, + $qb->expr()->in('item_type', $qb->createNamedParameter($collectionTypes, IQueryBuilder::PARAM_STR_ARRAY)) + )); + } else { + $qb->andWhere($expr); + } + } + $qb->orderBy('s.id', 'ASC'); + try { + $result = $qb->executeQuery(); + } catch (\Exception $e) { + \OCP\Server::get(LoggerInterface::class)->error( + 'Error while selecting shares: ' . $qb->getSQL(), + [ + 'app' => 'files_sharing', + 'exception' => $e + ]); + throw new \RuntimeException('Wrong SQL query', 500, $e); } - $where .= ' ORDER BY `*PREFIX*share`.`id` ASC'; - - $queryLimit = null; - $select = self::createSelectStatement(self::FORMAT_NONE, $fileDependent, $uidOwner); $root = strlen($root); - $query = \OC_DB::prepare('SELECT '.$select.' FROM `*PREFIX*share` '.$where, $queryLimit); - $result = $query->execute($queryArgs); - if ($result === false) { - \OC::$server->get(LoggerInterface::class)->error( - \OC_DB::getErrorMessage() . ', select=' . $select . ' where=', - ['app' => 'OCP\Share']); - } $items = []; $targets = []; $switchedItems = []; $mounts = []; - while ($row = $result->fetchRow()) { + while ($row = $result->fetch()) { + //var_dump($row); self::transformDBResults($row); // Filter out duplicate group shares for users with unique targets if ($fileDependent && !self::isFileReachable($row['path'], $row['storage_id'])) { @@ -548,16 +548,12 @@ class Share extends Constants { ->from('share') ->where($query->expr()->eq('id', $query->createNamedParameter($row['parent']))); - $parentResult = $query->execute(); - $parentRow = $parentResult->fetch(); - $parentResult->closeCursor(); + $parentRow = false; + try { + $parentResult = $query->executeQuery(); + $parentRow = $parentResult->fetchOne(); + $parentResult->closeCursor(); - if ($parentRow === false) { - \OC::$server->get(LoggerInterface::class)->error( - 'Can\'t select parent: ' . - \OC_DB::getErrorMessage() . ', select=' . $select . ' where=' . $where, - ['app' => 'OCP\Share']); - } else { $tmpPath = $parentRow['file_target']; // find the right position where the row path continues from the target path $pos = strrpos($row['path'], $parentRow['file_target']); @@ -567,6 +563,12 @@ class Share extends Constants { $tmpPath = $tmpPath . '/' . $pathPart; } $row['path'] = $tmpPath; + } catch (Exception $e) { + \OCP\Server::get(LoggerInterface::class) + ->error('Can\'t select parent :' . $e->getMessage() . ' query=' . $query->getSQL(), [ + 'exception' => $e, + 'app' => 'core' + ]); } } else { if (!isset($mounts[$row['storage']])) { @@ -576,7 +578,7 @@ class Share extends Constants { } } if (!empty($mounts[$row['storage']])) { - $path = $mounts[$row['storage']]->getMountPoint().$row['path']; + $path = $mounts[$row['storage']]->getMountPoint() . $row['path']; $relPath = substr($path, $root); // path relative to data/user $row['path'] = rtrim($relPath, '/'); } @@ -618,6 +620,7 @@ class Share extends Constants { $items[$row['id']] = $row; } } + $result->closeCursor(); // group items if we are looking for items shared with the current user if (isset($shareWith) && $shareWith === \OC_User::getUser()) { @@ -641,7 +644,7 @@ class Share extends Constants { $collection['path'] = basename($row['path']); } $row['collection'] = $collection; - // Fetch all of the children sources + // Fetch all the children sources $children = $collectionBackend->getChildren($row[$column]); foreach ($children as $child) { $childItem = $row; @@ -739,7 +742,7 @@ class Share extends Constants { if (!isset($result[$key]['grouped'])) { $result[$key]['grouped'][] = $result[$key]; } - $result[$key]['permissions'] = (int) $item['permissions'] | (int) $r['permissions']; + $result[$key]['permissions'] = (int)$item['permissions'] | (int)$r['permissions']; $result[$key]['grouped'][] = $item; $grouped = true; break; @@ -755,83 +758,142 @@ class Share extends Constants { } /** - * construct select statement - * @param int $format - * @param boolean $fileDependent ist it a file/folder share or a general share - * @param string $uidOwner - * @return string select statement + * Construct select statement + * + * @param bool $fileDependent ist it a file/folder share or a general share */ - private static function createSelectStatement($format, $fileDependent, $uidOwner = null) { - $select = '*'; + private static function getSelectStatement(int $format, bool $fileDependent, ?string $uidOwner = null): IQueryBuilder { + /** @var IDBConnection $connection */ + $connection = \OC::$server->get(IDBConnection::class); + $qb = $connection->getQueryBuilder(); if ($format == self::FORMAT_STATUSES) { if ($fileDependent) { - $select = '`*PREFIX*share`.`id`, `*PREFIX*share`.`parent`, `share_type`, `path`, `storage`, ' - . '`share_with`, `uid_owner` , `file_source`, `stime`, `*PREFIX*share`.`permissions`, ' - . '`*PREFIX*storages`.`id` AS `storage_id`, `*PREFIX*filecache`.`parent` as `file_parent`, ' - . '`uid_initiator`'; - } else { - $select = '`id`, `parent`, `share_type`, `share_with`, `uid_owner`, `item_source`, `stime`, `*PREFIX*share`.`permissions`'; - } - } else { - if (isset($uidOwner)) { - if ($fileDependent) { - $select = '`*PREFIX*share`.`id`, `item_type`, `item_source`, `*PREFIX*share`.`parent`,' - . ' `share_type`, `share_with`, `file_source`, `file_target`, `path`, `*PREFIX*share`.`permissions`, `stime`,' - . ' `expiration`, `token`, `storage`, `mail_send`, `uid_owner`, ' - . '`*PREFIX*storages`.`id` AS `storage_id`, `*PREFIX*filecache`.`parent` as `file_parent`'; - } else { - $select = '`id`, `item_type`, `item_source`, `parent`, `share_type`, `share_with`, `*PREFIX*share`.`permissions`,' - . ' `stime`, `file_source`, `expiration`, `token`, `mail_send`, `uid_owner`'; - } - } else { - if ($fileDependent) { - if ($format == \OCA\Files_Sharing\ShareBackend\File::FORMAT_GET_FOLDER_CONTENTS || $format == \OCA\Files_Sharing\ShareBackend\File::FORMAT_FILE_APP_ROOT) { - $select = '`*PREFIX*share`.`id`, `item_type`, `item_source`, `*PREFIX*share`.`parent`, `uid_owner`, ' - . '`share_type`, `share_with`, `file_source`, `path`, `file_target`, `stime`, ' - . '`*PREFIX*share`.`permissions`, `expiration`, `storage`, `*PREFIX*filecache`.`parent` as `file_parent`, ' - . '`name`, `mtime`, `mimetype`, `mimepart`, `size`, `encrypted`, `etag`, `mail_send`'; - } else { - $select = '`*PREFIX*share`.`id`, `item_type`, `item_source`, `item_target`,' - . '`*PREFIX*share`.`parent`, `share_type`, `share_with`, `uid_owner`,' - . '`file_source`, `path`, `file_target`, `*PREFIX*share`.`permissions`,' - . '`stime`, `expiration`, `token`, `storage`, `mail_send`,' - . '`*PREFIX*storages`.`id` AS `storage_id`, `*PREFIX*filecache`.`parent` as `file_parent`'; - } - } - } + return $qb->select( + 's.id', + 's.parent', + 'share_type', + 'path', + 'storage', + 'share_with', + 'uid_owner', + 'file_source', + 'stime', + 's.permissions', + 'uid_initiator' + )->selectAlias('st.id', 'storage_id') + ->selectAlias('f.parent', 'file_parent'); + } + return $qb->select('id', 'parent', 'share_type', 'share_with', 'uid_owner', 'item_source', 'stime', 's.permissions'); } - return $select; + + if (isset($uidOwner)) { + if ($fileDependent) { + return $qb->select( + 's.id', + 'item_type', + 'item_source', + 's.parent', + 'share_type', + 'share_with', + 'file_source', + 'file_target', + 'path', + 's.permissions', + 'stime', + 'expiration', + 'token', + 'storage', + 'mail_send', + 'uid_owner', + 'uid_initiator' + )->selectAlias('st.id', 'storage_id') + ->selectAlias('f.parent', 'file_parent'); + } + return $qb->select('id', 'item_type', 'item_source', 'parent', 'share_type', + 'share_with', 'uid_owner', 'file_source', 'stime', 's.permissions', + 'expiration', 'token', 'mail_send'); + } + + if ($fileDependent) { + if ($format == File::FORMAT_GET_FOLDER_CONTENTS || $format == File::FORMAT_FILE_APP_ROOT) { + return $qb->select( + 's.id', + 'item_type', + 'item_source', + 's.parent', + 'uid_owner', + 'share_type', + 'share_with', + 'file_source', + 'path', + 'file_target', + 's.permissions', + 'stime', + 'expiration', + 'storage', + 'name', + 'mtime', + 'mimepart', + 'size', + 'encrypted', + 'etag', + 'mail_send' + )->selectAlias('f.parent', 'file_parent'); + } + return $qb->select( + 's.id', + 'item_type', + 'item_source', + 'item_target', + 's.parent', + 'share_type', + 'share_with', + 'uid_owner', + 'file_source', + 'path', + 'file_target', + 's.permissions', + 'stime', + 'expiration', + 'token', + 'storage', + 'mail_send', + )->selectAlias('f.parent', 'file_parent') + ->selectAlias('st.id', 'storage_id'); + } + return $qb->select('*'); } /** * transform db results + * * @param array $row result */ private static function transformDBResults(&$row) { if (isset($row['id'])) { - $row['id'] = (int) $row['id']; + $row['id'] = (int)$row['id']; } if (isset($row['share_type'])) { - $row['share_type'] = (int) $row['share_type']; + $row['share_type'] = (int)$row['share_type']; } if (isset($row['parent'])) { - $row['parent'] = (int) $row['parent']; + $row['parent'] = (int)$row['parent']; } if (isset($row['file_parent'])) { - $row['file_parent'] = (int) $row['file_parent']; + $row['file_parent'] = (int)$row['file_parent']; } if (isset($row['file_source'])) { - $row['file_source'] = (int) $row['file_source']; + $row['file_source'] = (int)$row['file_source']; } if (isset($row['permissions'])) { - $row['permissions'] = (int) $row['permissions']; + $row['permissions'] = (int)$row['permissions']; } if (isset($row['storage'])) { - $row['storage'] = (int) $row['storage']; + $row['storage'] = (int)$row['storage']; } if (isset($row['stime'])) { - $row['stime'] = (int) $row['stime']; + $row['stime'] = (int)$row['stime']; } if (isset($row['expiration']) && $row['share_type'] !== IShare::TYPE_LINK) { // discard expiration date for non-link shares, which might have been @@ -842,6 +904,7 @@ class Share extends Constants { /** * format result + * * @param array $items result * @param string $column is it a file share or a general share ('file_target' or 'item_target') * @param \OCP\Share_Backend $backend sharing backend diff --git a/lib/private/Share20/UserRemovedListener.php b/lib/private/Share20/UserRemovedListener.php index 6bf184a9309..6db0f2e0910 100644 --- a/lib/private/Share20/UserRemovedListener.php +++ b/lib/private/Share20/UserRemovedListener.php @@ -30,6 +30,9 @@ use OCP\EventDispatcher\IEventListener; use OCP\Group\Events\UserRemovedEvent; use OCP\Share\IManager; +/** + * @template-implements IEventListener<UserRemovedEvent> + */ class UserRemovedListener implements IEventListener { /** @var IManager */ diff --git a/lib/private/TagManager.php b/lib/private/TagManager.php index 8c9dca98c92..82c4dd2188d 100644 --- a/lib/private/TagManager.php +++ b/lib/private/TagManager.php @@ -28,26 +28,30 @@ namespace OC; use OC\Tagging\TagMapper; use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\EventDispatcher\Event; +use OCP\EventDispatcher\IEventListener; use OCP\IDBConnection; use OCP\ITagManager; use OCP\ITags; use OCP\IUserSession; +use OCP\User\Events\UserDeletedEvent; +use OCP\Db\Exception as DBException; +use Psr\Log\LoggerInterface; -class TagManager implements ITagManager { - - /** @var TagMapper */ - private $mapper; - - /** @var IUserSession */ - private $userSession; - - /** @var IDBConnection */ - private $connection; +/** + * @template-implements IEventListener<UserDeletedEvent> + */ +class TagManager implements ITagManager, IEventListener { + private TagMapper $mapper; + private IUserSession $userSession; + private IDBConnection $connection; + private LoggerInterface $logger; - public function __construct(TagMapper $mapper, IUserSession $userSession, IDBConnection $connection) { + public function __construct(TagMapper $mapper, IUserSession $userSession, IDBConnection $connection, LoggerInterface $logger) { $this->mapper = $mapper; $this->userSession = $userSession; $this->connection = $connection; + $this->logger = $logger; } /** @@ -72,7 +76,7 @@ class TagManager implements ITagManager { } $userId = $this->userSession->getUser()->getUId(); } - return new Tags($this->mapper, $userId, $type, $defaultTags); + return new Tags($this->mapper, $userId, $type, $this->logger, $this->connection, $defaultTags); } /** @@ -97,4 +101,57 @@ class TagManager implements ITagManager { return $users; } + + public function handle(Event $event): void { + if (!($event instanceof UserDeletedEvent)) { + return; + } + + // Find all objectid/tagId pairs. + $user = $event->getUser(); + $qb = $this->connection->getQueryBuilder(); + $qb->select('id') + ->from('vcategory') + ->where($qb->expr()->eq('uid', $qb->createNamedParameter($user->getUID()))); + try { + $result = $qb->executeQuery(); + } catch (DBException $e) { + $this->logger->error($e->getMessage(), [ + 'app' => 'core', + 'exception' => $e, + ]); + return; + } + + $tagsIds = array_map(fn (array $row) => (int)$row['id'], $result->fetchAll()); + $result->closeCursor(); + + if (count($tagsIds) === 0) { + return; + } + + // Clean vcategory_to_object table + $qb = $this->connection->getQueryBuilder(); + $qb = $qb->delete('vcategory_to_object') + ->where($qb->expr()->in('categoryid', $qb->createParameter('chunk'))); + + // Clean vcategory + $qb1 = $this->connection->getQueryBuilder(); + $qb1 = $qb1->delete('vcategory') + ->where($qb1->expr()->in('uid', $qb1->createParameter('chunk'))); + + foreach (array_chunk($tagsIds, 1000) as $tagChunk) { + $qb->setParameter('chunk', $tagChunk, IQueryBuilder::PARAM_INT_ARRAY); + $qb1->setParameter('chunk', $tagChunk, IQueryBuilder::PARAM_INT_ARRAY); + try { + $qb->executeStatement(); + $qb1->executeStatement(); + } catch (DBException $e) { + $this->logger->error($e->getMessage(), [ + 'app' => 'core', + 'exception' => $e, + ]); + } + } + } } diff --git a/lib/private/Tags.php b/lib/private/Tags.php index c11d83e41cf..8da1e7edc3b 100644 --- a/lib/private/Tags.php +++ b/lib/private/Tags.php @@ -32,72 +32,49 @@ namespace OC; use OC\Tagging\Tag; use OC\Tagging\TagMapper; +use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IDBConnection; use OCP\ILogger; use OCP\ITags; +use OCP\Share_Backend; +use Psr\Log\LoggerInterface; class Tags implements ITags { - - /** - * Tags - * - * @var array - */ - private $tags = []; - /** * Used for storing objectid/categoryname pairs while rescanning. - * - * @var array */ - private static $relations = []; - - /** - * Type - * - * @var string - */ - private $type; - - /** - * User - * - * @var string - */ - private $user; + private static array $relations = []; + private string $type; + private string $user; + private IDBConnection $db; + private LoggerInterface $logger; + private array $tags = []; /** * Are we including tags for shared items? - * - * @var bool */ - private $includeShared = false; + private bool $includeShared = false; /** * The current user, plus any owners of the items shared with the current * user, if $this->includeShared === true. - * - * @var array */ - private $owners = []; + private array $owners = []; /** - * The Mapper we're using to communicate our Tag objects to the database. - * - * @var TagMapper + * The Mapper we are using to communicate our Tag objects to the database. */ - private $mapper; + private TagMapper $mapper; /** * The sharing backend for objects of $this->type. Required if * $this->includeShared === true to determine ownership of items. - * - * @var \OCP\Share_Backend */ - private $backend; + private ?Share_Backend $backend = null; - public const TAG_TABLE = '*PREFIX*vcategory'; - public const RELATION_TABLE = '*PREFIX*vcategory_to_object'; + public const TAG_TABLE = 'vcategory'; + public const RELATION_TABLE = 'vcategory_to_object'; /** * Constructor. @@ -109,12 +86,14 @@ class Tags implements ITags { * * since 20.0.0 $includeShared isn't used anymore */ - public function __construct(TagMapper $mapper, $user, $type, $defaultTags = []) { + public function __construct(TagMapper $mapper, string $user, string $type, LoggerInterface $logger, IDBConnection $connection, array $defaultTags = []) { $this->mapper = $mapper; $this->user = $user; $this->type = $type; $this->owners = [$this->user]; $this->tags = $this->mapper->loadTags($this->owners, $this->type); + $this->db = $connection; + $this->logger = $logger; if (count($defaultTags) > 0 && count($this->tags) === 0) { $this->addMultiple($defaultTags, true); @@ -126,7 +105,7 @@ class Tags implements ITags { * * @return boolean */ - public function isEmpty() { + public function isEmpty(): bool { return count($this->tags) === 0; } @@ -137,7 +116,7 @@ class Tags implements ITags { * @param string $id The ID of the tag that is going to be mapped * @return array|false */ - public function getTag($id) { + public function getTag(string $id) { $key = $this->getTagById($id); if ($key !== false) { return $this->tagMap($this->tags[$key]); @@ -154,9 +133,9 @@ class Tags implements ITags { * ['id' => 1, 'name' = 'Shared tag', 'owner' = 'Other user', 'type' => 'tagtype'], * ] * - * @return array + * @return array<array-key, array{id: int, name: string}> */ - public function getTags() { + public function getTags(): array { if (!count($this->tags)) { return []; } @@ -181,7 +160,7 @@ class Tags implements ITags { * @param string $user The user whose tags are to be checked. * @return array An array of Tag objects. */ - public function getTagsForUser($user) { + public function getTagsForUser(string $user): array { return array_filter($this->tags, function ($tag) use ($user) { return $tag->getOwner() === $user; @@ -193,23 +172,26 @@ class Tags implements ITags { * Get the list of tags for the given ids. * * @param array $objIds array of object ids - * @return array|boolean of tags id as key to array of tag names + * @return array|false of tags id as key to array of tag names * or false if an error occurred */ public function getTagsForObjects(array $objIds) { $entries = []; try { - $conn = \OC::$server->getDatabaseConnection(); $chunks = array_chunk($objIds, 900, false); + $qb = $this->db->getQueryBuilder(); + $qb->select('category', 'categoryid', 'objid') + ->from(self::RELATION_TABLE, 'r') + ->join('r', self::TAG_TABLE, 't', $qb->expr()->eq('r.categoryid', 't.id')) + ->where($qb->expr()->eq('uid', $qb->createParameter('uid'))) + ->andWhere($qb->expr()->eq('r.type', $qb->createParameter('type'))) + ->andWhere($qb->expr()->in('objid', $qb->createParameter('chunk'))); foreach ($chunks as $chunk) { - $result = $conn->executeQuery( - 'SELECT `category`, `categoryid`, `objid` ' . - 'FROM `' . self::RELATION_TABLE . '` r, `' . self::TAG_TABLE . '` ' . - 'WHERE `categoryid` = `id` AND `uid` = ? AND r.`type` = ? AND `objid` IN (?)', - [$this->user, $this->type, $chunk], - [null, null, IQueryBuilder::PARAM_INT_ARRAY] - ); + $qb->setParameter('uid', $this->user, IQueryBuilder::PARAM_STR); + $qb->setParameter('type', $this->type, IQueryBuilder::PARAM_STR); + $qb->setParameter('chunk', $chunk, IQueryBuilder::PARAM_INT_ARRAY); + $result = $qb->executeQuery(); while ($row = $result->fetch()) { $objId = (int)$row['objid']; if (!isset($entries[$objId])) { @@ -217,11 +199,11 @@ class Tags implements ITags { } $entries[$objId][] = $row['category']; } + $result->closeCursor(); } } catch (\Exception $e) { - \OC::$server->getLogger()->logException($e, [ - 'message' => __METHOD__, - 'level' => ILogger::ERROR, + $this->logger->error($e->getMessage(), [ + 'exception' => $e, 'app' => 'core', ]); return false; @@ -236,18 +218,17 @@ class Tags implements ITags { * Throws an exception if the tag could not be found. * * @param string $tag Tag id or name. - * @return array|false An array of object ids or false on error. + * @return int[]|false An array of object ids or false on error. * @throws \Exception */ public function getIdsForTag($tag) { - $result = null; $tagId = false; if (is_numeric($tag)) { $tagId = $tag; } elseif (is_string($tag)) { $tag = trim($tag); if ($tag === '') { - \OCP\Util::writeLog('core', __METHOD__.', Cannot use empty tag names', ILogger::DEBUG); + $this->logger->debug(__METHOD__ . ' Cannot use empty tag names', ['app' => 'core']); return false; } $tagId = $this->getTagId($tag); @@ -261,32 +242,24 @@ class Tags implements ITags { } $ids = []; - $sql = 'SELECT `objid` FROM `' . self::RELATION_TABLE - . '` WHERE `categoryid` = ?'; - try { - $stmt = \OC_DB::prepare($sql); - $result = $stmt->execute([$tagId]); - if ($result === null) { - $stmt->closeCursor(); - \OCP\Util::writeLog('core', __METHOD__. 'DB error: ' . \OC::$server->getDatabaseConnection()->getError(), ILogger::ERROR); - return false; - } - } catch (\Exception $e) { - \OC::$server->getLogger()->logException($e, [ - 'message' => __METHOD__, - 'level' => ILogger::ERROR, + $qb = $this->db->getQueryBuilder(); + $qb->select('objid') + ->from(self::RELATION_TABLE) + ->where($qb->expr()->eq('categoryid', $qb->createNamedParameter($tagId, IQueryBuilder::PARAM_STR))); + $result = $qb->executeQuery(); + } catch (Exception $e) { + $this->logger->error($e->getMessage(), [ 'app' => 'core', + 'exception' => $e, ]); return false; } - if (!is_null($result)) { - while ($row = $result->fetchRow()) { - $ids[] = (int)$row['objid']; - } - $result->closeCursor(); + while ($row = $result->fetch()) { + $ids[] = (int)$row['objid']; } + $result->closeCursor(); return $ids; } @@ -297,9 +270,8 @@ class Tags implements ITags { * * @param string $name The tag name to check for. * @param string $user The user whose tags are to be checked. - * @return bool */ - public function userHasTag($name, $user) { + public function userHasTag(string $name, string $user): bool { $key = $this->array_searchi($name, $this->getTagsForUser($user)); return ($key !== false) ? $this->tags[$key]->getId() : false; } @@ -308,9 +280,8 @@ class Tags implements ITags { * Checks whether a tag is saved for or shared with the current user. * * @param string $name The tag name to check for. - * @return bool */ - public function hasTag($name) { + public function hasTag(string $name): bool { return $this->getTagId($name) !== false; } @@ -320,15 +291,16 @@ class Tags implements ITags { * @param string $name A string with a name of the tag * @return false|int the id of the added tag or false on error. */ - public function add($name) { + public function add(string $name) { $name = trim($name); if ($name === '') { - \OCP\Util::writeLog('core', __METHOD__.', Cannot add an empty tag', ILogger::DEBUG); + $this->logger->debug(__METHOD__ . ' Cannot add an empty tag', ['app' => 'core']); return false; } if ($this->userHasTag($name, $this->user)) { - \OCP\Util::writeLog('core', __METHOD__.', name: ' . $name. ' exists already', ILogger::DEBUG); + // TODO use unique db properties instead of an additional check + $this->logger->debug(__METHOD__ . ' Tag with name already exists', ['app' => 'core']); return false; } try { @@ -336,14 +308,13 @@ class Tags implements ITags { $tag = $this->mapper->insert($tag); $this->tags[] = $tag; } catch (\Exception $e) { - \OC::$server->getLogger()->logException($e, [ - 'message' => __METHOD__, - 'level' => ILogger::ERROR, + $this->logger->error($e->getMessage(), [ + 'exception' => $e, 'app' => 'core', ]); return false; } - \OCP\Util::writeLog('core', __METHOD__.', id: ' . $tag->getId(), ILogger::DEBUG); + $this->logger->debug(__METHOD__ . ' Added an tag with ' . $tag->getId(), ['app' => 'core']); return $tag->getId(); } @@ -354,12 +325,12 @@ class Tags implements ITags { * @param string $to The new name of the tag. * @return bool */ - public function rename($from, $to) { + public function rename($from, string $to): bool { $from = trim($from); $to = trim($to); if ($to === '' || $from === '') { - \OCP\Util::writeLog('core', __METHOD__.', Cannot use empty tag names', ILogger::DEBUG); + $this->logger->debug(__METHOD__ . 'Cannot use an empty tag names', ['app' => 'core']); return false; } @@ -369,13 +340,13 @@ class Tags implements ITags { $key = $this->getTagByName($from); } if ($key === false) { - \OCP\Util::writeLog('core', __METHOD__.', tag: ' . $from. ' does not exist', ILogger::DEBUG); + $this->logger->debug(__METHOD__ . 'Tag ' . $from . 'does not exist', ['app' => 'core']); return false; } $tag = $this->tags[$key]; if ($this->userHasTag($to, $tag->getOwner())) { - \OCP\Util::writeLog('core', __METHOD__.', A tag named ' . $to. ' already exists for user ' . $tag->getOwner() . '.', ILogger::DEBUG); + $this->logger->debug(__METHOD__ . 'A tag named' . $to . 'already exists for user' . $tag->getOwner(), ['app' => 'core']); return false; } @@ -383,9 +354,8 @@ class Tags implements ITags { $tag->setName($to); $this->tags[$key] = $this->mapper->update($tag); } catch (\Exception $e) { - \OC::$server->getLogger()->logException($e, [ - 'message' => __METHOD__, - 'level' => ILogger::ERROR, + $this->logger->error($e->getMessage(), [ + 'exception' => $e, 'app' => 'core', ]); return false; @@ -396,13 +366,13 @@ class Tags implements ITags { /** * Add a list of new tags. * - * @param string[] $names A string with a name or an array of strings containing + * @param string|string[] $names A string with a name or an array of strings containing * the name(s) of the tag(s) to add. * @param bool $sync When true, save the tags * @param int|null $id int Optional object id to add to this|these tag(s) * @return bool Returns false on error. */ - public function addMultiple($names, $sync = false, $id = null) { + public function addMultiple($names, bool $sync = false, ?int $id = null): bool { if (!is_array($names)) { $names = [$names]; } @@ -430,122 +400,50 @@ class Tags implements ITags { /** * Save the list of tags and their object relations */ - protected function save() { - if (is_array($this->tags)) { - foreach ($this->tags as $tag) { - try { - if (!$this->mapper->tagExists($tag)) { - $this->mapper->insert($tag); - } - } catch (\Exception $e) { - \OC::$server->getLogger()->logException($e, [ - 'message' => __METHOD__, - 'level' => ILogger::ERROR, - 'app' => 'core', - ]); - } - } - - // reload tags to get the proper ids. - $this->tags = $this->mapper->loadTags($this->owners, $this->type); - \OCP\Util::writeLog('core', __METHOD__.', tags: ' . print_r($this->tags, true), - ILogger::DEBUG); - // Loop through temporarily cached objectid/tagname pairs - // and save relations. - $tags = $this->tags; - // For some reason this is needed or array_search(i) will return 0..? - ksort($tags); - $dbConnection = \OC::$server->getDatabaseConnection(); - foreach (self::$relations as $relation) { - $tagId = $this->getTagId($relation['tag']); - \OCP\Util::writeLog('core', __METHOD__ . 'catid, ' . $relation['tag'] . ' ' . $tagId, ILogger::DEBUG); - if ($tagId) { - try { - $dbConnection->insertIfNotExist(self::RELATION_TABLE, - [ - 'objid' => $relation['objid'], - 'categoryid' => $tagId, - 'type' => $this->type, - ]); - } catch (\Exception $e) { - \OC::$server->getLogger()->logException($e, [ - 'message' => __METHOD__, - 'level' => ILogger::ERROR, - 'app' => 'core', - ]); - } - } - } - self::$relations = []; // reset - } else { - \OCP\Util::writeLog('core', __METHOD__.', $this->tags is not an array! ' - . print_r($this->tags, true), ILogger::ERROR); - } - } - - /** - * Delete tags and tag/object relations for a user. - * - * For hooking up on post_deleteUser - * - * @param array $arguments - */ - public static function post_deleteUser($arguments) { - // Find all objectid/tagId pairs. - $result = null; - try { - $stmt = \OC_DB::prepare('SELECT `id` FROM `' . self::TAG_TABLE . '` ' - . 'WHERE `uid` = ?'); - $result = $stmt->execute([$arguments['uid']]); - if ($result === null) { - \OCP\Util::writeLog('core', __METHOD__. 'DB error: ' . \OC::$server->getDatabaseConnection()->getError(), ILogger::ERROR); - } - } catch (\Exception $e) { - \OC::$server->getLogger()->logException($e, [ - 'message' => __METHOD__, - 'level' => ILogger::ERROR, - 'app' => 'core', - ]); - } - - if (!is_null($result)) { + protected function save(): void { + foreach ($this->tags as $tag) { try { - $stmt = \OC_DB::prepare('DELETE FROM `' . self::RELATION_TABLE . '` ' - . 'WHERE `categoryid` = ?'); - while ($row = $result->fetchRow()) { - try { - $stmt->execute([$row['id']]); - } catch (\Exception $e) { - \OC::$server->getLogger()->logException($e, [ - 'message' => __METHOD__, - 'level' => ILogger::ERROR, - 'app' => 'core', - ]); - } + if (!$this->mapper->tagExists($tag)) { + $this->mapper->insert($tag); } - $result->closeCursor(); } catch (\Exception $e) { - \OC::$server->getLogger()->logException($e, [ - 'message' => __METHOD__, - 'level' => ILogger::ERROR, + $this->logger->error($e->getMessage(), [ + 'exception' => $e, 'app' => 'core', ]); } } - try { - $stmt = \OC_DB::prepare('DELETE FROM `' . self::TAG_TABLE . '` ' - . 'WHERE `uid` = ?'); - $result = $stmt->execute([$arguments['uid']]); - if ($result === null) { - \OCP\Util::writeLog('core', __METHOD__. ', DB error: ' . \OC::$server->getDatabaseConnection()->getError(), ILogger::ERROR); + + // reload tags to get the proper ids. + $this->tags = $this->mapper->loadTags($this->owners, $this->type); + $this->logger->debug(__METHOD__ . 'tags' . print_r($this->tags, true), ['app' => 'core']); + // Loop through temporarily cached objectid/tagname pairs + // and save relations. + $tags = $this->tags; + // For some reason this is needed or array_search(i) will return 0..? + ksort($tags); + foreach (self::$relations as $relation) { + $tagId = $this->getTagId($relation['tag']); + $this->logger->debug(__METHOD__ . 'catid ' . $relation['tag'] . ' ' . $tagId, ['app' => 'core']); + if ($tagId) { + $qb = $this->db->getQueryBuilder(); + $qb->insert(self::RELATION_TABLE) + ->values([ + 'objid' => $qb->createNamedParameter($relation['objid'], IQueryBuilder::PARAM_INT), + 'categoryid' => $qb->createNamedParameter($tagId, IQueryBuilder::PARAM_INT), + 'type' => $qb->createNamedParameter($this->type), + ]); + try { + $qb->executeStatement(); + } catch (Exception $e) { + $this->logger->error($e->getMessage(), [ + 'exception' => $e, + 'app' => 'core', + ]); + } } - } catch (\Exception $e) { - \OC::$server->getLogger()->logException($e, [ - 'message' => __METHOD__, - 'level' => ILogger::ERROR, - 'app' => 'core', - ]); } + self::$relations = []; // reset } /** @@ -554,28 +452,21 @@ class Tags implements ITags { * @param array $ids The ids of the objects * @return boolean Returns false on error. */ - public function purgeObjects(array $ids) { + public function purgeObjects(array $ids): bool { if (count($ids) === 0) { // job done ;) return true; } $updates = $ids; + $qb = $this->db->getQueryBuilder(); + $qb->delete(self::RELATION_TABLE) + ->where($qb->expr()->in('objid', $qb->createNamedParameter($ids))); try { - $query = 'DELETE FROM `' . self::RELATION_TABLE . '` '; - $query .= 'WHERE `objid` IN (' . str_repeat('?,', count($ids) - 1) . '?) '; - $query .= 'AND `type`= ?'; - $updates[] = $this->type; - $stmt = \OC_DB::prepare($query); - $result = $stmt->execute($updates); - if ($result === null) { - \OCP\Util::writeLog('core', __METHOD__. 'DB error: ' . \OC::$server->getDatabaseConnection()->getError(), ILogger::ERROR); - return false; - } - } catch (\Exception $e) { - \OC::$server->getLogger()->logException($e, [ - 'message' => __METHOD__, - 'level' => ILogger::ERROR, + $qb->executeStatement(); + } catch (Exception $e) { + $this->logger->error($e->getMessage(), [ 'app' => 'core', + 'exception' => $e, ]); return false; } @@ -648,18 +539,19 @@ class Tags implements ITags { } else { $tagId = $tag; } + $qb = $this->db->getQueryBuilder(); + $qb->insert(self::RELATION_TABLE) + ->values([ + 'objid' => $qb->createNamedParameter($objid, IQueryBuilder::PARAM_INT), + 'categoryid' => $qb->createNamedParameter($tagId, IQueryBuilder::PARAM_INT), + 'type' => $qb->createNamedParameter($this->type, IQueryBuilder::PARAM_STR), + ]); try { - \OC::$server->getDatabaseConnection()->insertIfNotExist(self::RELATION_TABLE, - [ - 'objid' => $objid, - 'categoryid' => $tagId, - 'type' => $this->type, - ]); + $qb->executeStatement(); } catch (\Exception $e) { - \OC::$server->getLogger()->logException($e, [ - 'message' => __METHOD__, - 'level' => ILogger::ERROR, + \OC::$server->getLogger()->error($e->getMessage(), [ 'app' => 'core', + 'exception' => $e, ]); return false; } @@ -686,15 +578,17 @@ class Tags implements ITags { } try { - $sql = 'DELETE FROM `' . self::RELATION_TABLE . '` ' - . 'WHERE `objid` = ? AND `categoryid` = ? AND `type` = ?'; - $stmt = \OC_DB::prepare($sql); - $stmt->execute([$objid, $tagId, $this->type]); + $qb = $this->db->getQueryBuilder(); + $qb->delete(self::RELATION_TABLE) + ->where($qb->expr()->andX( + $qb->expr()->eq('objid', $qb->createNamedParameter($objid)), + $qb->expr()->eq('categoryid', $qb->createNamedParameter($tagId)), + $qb->expr()->eq('type', $qb->createNamedParameter($this->type)), + ))->executeStatement(); } catch (\Exception $e) { - \OC::$server->getLogger()->logException($e, [ - 'message' => __METHOD__, - 'level' => ILogger::ERROR, + $this->logger->error($e->getMessage(), [ 'app' => 'core', + 'exception' => $e, ]); return false; } @@ -736,21 +630,14 @@ class Tags implements ITags { } if (!is_null($id) && $id !== false) { try { - $sql = 'DELETE FROM `' . self::RELATION_TABLE . '` ' - . 'WHERE `categoryid` = ?'; - $stmt = \OC_DB::prepare($sql); - $result = $stmt->execute([$id]); - if ($result === null) { - \OCP\Util::writeLog('core', - __METHOD__. 'DB error: ' . \OC::$server->getDatabaseConnection()->getError(), - ILogger::ERROR); - return false; - } + $qb = $this->db->getQueryBuilder(); + $qb->delete(self::RELATION_TABLE) + ->where($qb->expr()->eq('categoryid', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT))) + ->executeStatement(); } catch (\Exception $e) { - \OC::$server->getLogger()->logException($e, [ - 'message' => __METHOD__, - 'level' => ILogger::ERROR, + $this->logger->error($e->getMessage(), [ 'app' => 'core', + 'exception' => $e, ]); return false; } diff --git a/lib/private/legacy/OC_DB.php b/lib/private/legacy/OC_DB.php deleted file mode 100644 index 39faf8e80aa..00000000000 --- a/lib/private/legacy/OC_DB.php +++ /dev/null @@ -1,184 +0,0 @@ -<?php -/** - * @copyright Copyright (c) 2016, ownCloud, Inc. - * - * @author Andreas Fischer <bantu@owncloud.com> - * @author Bart Visscher <bartv@thisnet.nl> - * @author Christoph Wurst <christoph@winzerhof-wurst.at> - * @author Joas Schilling <coding@schilljs.com> - * @author Jörn Friedrich Dreyer <jfd@butonic.de> - * @author Lukas Reschke <lukas@statuscode.ch> - * @author Morris Jobke <hey@morrisjobke.de> - * @author Robin Appelman <robin@icewind.nl> - * @author Thomas Müller <thomas.mueller@tmit.eu> - * @author Vincent Petry <vincent@nextcloud.com> - * - * @license AGPL-3.0 - * - * This code is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License, version 3, - * as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License, version 3, - * along with this program. If not, see <http://www.gnu.org/licenses/> - * - */ -class OC_DB { - - /** - * Prepare a SQL query - * @param string $query Query string - * @param int|null $limit - * @param int|null $offset - * @param bool|null $isManipulation - * @throws \OC\DatabaseException - * @return OC_DB_StatementWrapper prepared SQL query - * @deprecated 21.0.0 Please use \OCP\IDBConnection::getQueryBuilder() instead - * - * SQL query via Doctrine prepare(), needs to be execute()'d! - */ - public static function prepare($query, $limit = null, $offset = null, $isManipulation = null) { - $connection = \OC::$server->getDatabaseConnection(); - - if ($isManipulation === null) { - //try to guess, so we return the number of rows on manipulations - $isManipulation = self::isManipulation($query); - } - - // return the result - try { - $result = $connection->prepare($query, $limit, $offset); - } catch (\Doctrine\DBAL\Exception $e) { - throw new \OC\DatabaseException($e->getMessage()); - } - // differentiate between query and manipulation - return new OC_DB_StatementWrapper($result, $isManipulation); - } - - /** - * tries to guess the type of statement based on the first 10 characters - * the current check allows some whitespace but does not work with IF EXISTS or other more complex statements - * - * @param string $sql - * @return bool - */ - public static function isManipulation($sql) { - $sql = trim($sql); - $selectOccurrence = stripos($sql, 'SELECT'); - if ($selectOccurrence === 0) { - return false; - } - $insertOccurrence = stripos($sql, 'INSERT'); - if ($insertOccurrence === 0) { - return true; - } - $updateOccurrence = stripos($sql, 'UPDATE'); - if ($updateOccurrence === 0) { - return true; - } - $deleteOccurrence = stripos($sql, 'DELETE'); - if ($deleteOccurrence === 0) { - return true; - } - - // This is triggered with "SHOW VERSION" and some more, so until we made a list, we keep this out. - // \OC::$server->getLogger()->logException(new \Exception('Can not detect if query is manipulating: ' . $sql)); - - return false; - } - - /** - * execute a prepared statement, on error write log and throw exception - * @param mixed $stmt OC_DB_StatementWrapper, - * an array with 'sql' and optionally 'limit' and 'offset' keys - * .. or a simple sql query string - * @param array $parameters - * @return OC_DB_StatementWrapper - * @throws \OC\DatabaseException - * @deprecated 21.0.0 Please use \OCP\IDBConnection::getQueryBuilder() instead - */ - public static function executeAudited($stmt, array $parameters = []) { - if (is_string($stmt)) { - // convert to an array with 'sql' - if (stripos($stmt, 'LIMIT') !== false) { //OFFSET requires LIMIT, so we only need to check for LIMIT - // TODO try to convert LIMIT OFFSET notation to parameters - $message = 'LIMIT and OFFSET are forbidden for portability reasons,' - . ' pass an array with \'limit\' and \'offset\' instead'; - throw new \OC\DatabaseException($message); - } - $stmt = ['sql' => $stmt, 'limit' => null, 'offset' => null]; - } - if (is_array($stmt)) { - // convert to prepared statement - if (! array_key_exists('sql', $stmt)) { - $message = 'statement array must at least contain key \'sql\''; - throw new \OC\DatabaseException($message); - } - if (! array_key_exists('limit', $stmt)) { - $stmt['limit'] = null; - } - if (! array_key_exists('limit', $stmt)) { - $stmt['offset'] = null; - } - $stmt = self::prepare($stmt['sql'], $stmt['limit'], $stmt['offset']); - } - self::raiseExceptionOnError($stmt, 'Could not prepare statement'); - if ($stmt instanceof OC_DB_StatementWrapper) { - $result = $stmt->execute($parameters); - self::raiseExceptionOnError($result, 'Could not execute statement'); - } else { - if (is_object($stmt)) { - $message = 'Expected a prepared statement or array got ' . get_class($stmt); - } else { - $message = 'Expected a prepared statement or array got ' . gettype($stmt); - } - throw new \OC\DatabaseException($message); - } - return $result; - } - - /** - * check if a result is an error and throws an exception, works with \Doctrine\DBAL\Exception - * @param mixed $result - * @param string $message - * @return void - * @throws \OC\DatabaseException - */ - public static function raiseExceptionOnError($result, $message = null) { - if ($result === false) { - if ($message === null) { - $message = self::getErrorMessage(); - } else { - $message .= ', Root cause:' . self::getErrorMessage(); - } - throw new \OC\DatabaseException($message); - } - } - - /** - * returns the error code and message as a string for logging - * works with DoctrineException - * @return string - */ - public static function getErrorMessage() { - $connection = \OC::$server->getDatabaseConnection(); - return $connection->getError(); - } - - /** - * Checks if a table exists in the database - the database prefix will be prepended - * - * @param string $table - * @return bool - * @throws \OC\DatabaseException - */ - public static function tableExists($table) { - $connection = \OC::$server->getDatabaseConnection(); - return $connection->tableExists($table); - } -} diff --git a/lib/private/legacy/OC_DB_StatementWrapper.php b/lib/private/legacy/OC_DB_StatementWrapper.php deleted file mode 100644 index d6214a49a33..00000000000 --- a/lib/private/legacy/OC_DB_StatementWrapper.php +++ /dev/null @@ -1,135 +0,0 @@ -<?php -/** - * @copyright Copyright (c) 2016, ownCloud, Inc. - * - * @author Bart Visscher <bartv@thisnet.nl> - * @author Christoph Wurst <christoph@winzerhof-wurst.at> - * @author Joas Schilling <coding@schilljs.com> - * @author Jörn Friedrich Dreyer <jfd@butonic.de> - * @author Lukas Reschke <lukas@statuscode.ch> - * @author Morris Jobke <hey@morrisjobke.de> - * @author Robin Appelman <robin@icewind.nl> - * @author Thomas Müller <thomas.mueller@tmit.eu> - * - * @license AGPL-3.0 - * - * This code is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License, version 3, - * as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License, version 3, - * along with this program. If not, see <http://www.gnu.org/licenses/> - * - */ -use OCP\DB\IPreparedStatement; - -/** - * small wrapper around \Doctrine\DBAL\Driver\Statement to make it behave, more like an MDB2 Statement - * - * @method boolean bindValue(mixed $param, mixed $value, integer $type = null); - * @method string errorCode(); - * @method array errorInfo(); - * @method integer rowCount(); - * @method array fetchAll(integer $fetchMode = null); - */ -class OC_DB_StatementWrapper { - /** @var IPreparedStatement */ - private $statement = null; - - /** @var bool */ - private $isManipulation = false; - - /** @var array */ - private $lastArguments = []; - - /** - * @param IPreparedStatement $statement - * @param boolean $isManipulation - */ - public function __construct(IPreparedStatement $statement, $isManipulation) { - $this->statement = $statement; - $this->isManipulation = $isManipulation; - } - - /** - * pass all other function directly to the \Doctrine\DBAL\Driver\Statement - */ - public function __call($name, $arguments) { - return call_user_func_array([$this->statement,$name], $arguments); - } - - /** - * make execute return the result instead of a bool - * - * @param mixed[] $input - * @return \OC_DB_StatementWrapper|int|bool - * @deprecated - */ - public function execute($input = []) { - $this->lastArguments = $input; - try { - if (count($input) > 0) { - $result = $this->statement->execute($input); - } else { - $result = $this->statement->execute(); - } - } catch (\Doctrine\DBAL\Exception $e) { - return false; - } - - if ($this->isManipulation) { - return $this->statement->rowCount(); - } - - return $this; - } - - /** - * provide an alias for fetch - * - * @return mixed - * @deprecated - */ - public function fetchRow() { - return $this->statement->fetch(); - } - - /** - * Provide a simple fetchOne. - * - * fetch single column from the next row - * @return string - * @deprecated - */ - public function fetchOne() { - return $this->statement->fetchOne(); - } - - /** - * Closes the cursor, enabling the statement to be executed again. - * - * @deprecated Use Result::free() instead. - */ - public function closeCursor(): void { - $this->statement->closeCursor(); - } - - /** - * Binds a PHP variable to a corresponding named or question mark placeholder in the - * SQL statement that was use to prepare the statement. - * - * @param mixed $column Either the placeholder name or the 1-indexed placeholder index - * @param mixed $variable The variable to bind - * @param integer|null $type one of the PDO::PARAM_* constants - * @param integer|null $length max length when using an OUT bind - * @return boolean - */ - public function bindParam($column, &$variable, $type = null, $length = null) { - return $this->statement->bindParam($column, $variable, $type, $length); - } -} diff --git a/lib/private/legacy/OC_Util.php b/lib/private/legacy/OC_Util.php index bf9ed6e29ff..4778fe33097 100644 --- a/lib/private/legacy/OC_Util.php +++ b/lib/private/legacy/OC_Util.php @@ -69,6 +69,7 @@ use OC\AppFramework\Http\Request; use OC\Files\SetupManager; use OCP\Files\Template\ITemplateManager; use OCP\IConfig; +use OCP\IDBConnection; use OCP\IGroupManager; use OCP\IURLGenerator; use OCP\IUser; @@ -749,9 +750,13 @@ class OC_Util { $dbType = \OC::$server->getSystemConfig()->getValue('dbtype', 'sqlite'); if ($dbType === 'pgsql') { // check PostgreSQL version + // TODO latest postgresql 8 released was 8 years ago, maybe remove the + // check completely? try { - $result = \OC_DB::executeAudited('SHOW SERVER_VERSION'); - $data = $result->fetchRow(); + /** @var IDBConnection $connection */ + $connection = \OC::$server->get(IDBConnection::class); + $result = $connection->executeQuery('SHOW SERVER_VERSION'); + $data = $result->fetch(); $result->closeCursor(); if (isset($data['server_version'])) { $version = $data['server_version']; diff --git a/lib/public/ITags.php b/lib/public/ITags.php index 03bcb845f4e..06497781bee 100644 --- a/lib/public/ITags.php +++ b/lib/public/ITags.php @@ -32,9 +32,6 @@ namespace OCP; use OC\Tags; -// FIXME: Where should I put this? Or should it be implemented as a Listener? -\OC_Hook::connect('OC_User', 'post_deleteUser', Tags::class, 'post_deleteUser'); - /** * Class for easily tagging objects by their id * @@ -55,11 +52,9 @@ interface ITags { /** * Check if any tags are saved for this type and user. - * - * @return boolean * @since 6.0.0 */ - public function isEmpty(); + public function isEmpty(): bool; /** * Returns an array mapping a given tag's properties to its values: @@ -69,34 +64,40 @@ interface ITags { * @return array|false * @since 8.0.0 */ - public function getTag($id); + public function getTag(string $id); /** * Get the tags for a specific user. * * This returns an array with id/name maps: + * + * ```php * [ * ['id' => 0, 'name' = 'First tag'], * ['id' => 1, 'name' = 'Second tag'], * ] + * ``` * - * @return array + * @return array<array-key, array{id: int, name: string}> * @since 6.0.0 */ - public function getTags(); + public function getTags(): array; /** * Get a list of tags for the given item ids. * * This returns an array with object id / tag names: + * + * ```php * [ * 1 => array('First tag', 'Second tag'), * 2 => array('Second tag'), * 3 => array('Second tag', 'Third tag'), * ] + * ``` * * @param array $objIds item ids - * @return array|boolean with object id as key and an array + * @return array|false with object id as key and an array * of tag names as value or false if an error occurred * @since 8.0.0 */ @@ -117,10 +118,9 @@ interface ITags { * Checks whether a tag is already saved. * * @param string $name The name to check for. - * @return bool * @since 6.0.0 */ - public function hasTag($name); + public function hasTag(string $name): bool; /** * Checks whether a tag is saved for the given user, @@ -131,7 +131,7 @@ interface ITags { * @return bool * @since 8.0.0 */ - public function userHasTag($name, $user); + public function userHasTag(string $name, string $user): bool; /** * Add a new tag. @@ -140,7 +140,7 @@ interface ITags { * @return int|false the id of the added tag or false if it already exists. * @since 6.0.0 */ - public function add($name); + public function add(string $name); /** * Rename tag. @@ -150,19 +150,19 @@ interface ITags { * @return bool * @since 6.0.0 */ - public function rename($from, $to); + public function rename($from, string $to): bool; /** * Add a list of new tags. * - * @param string[] $names A string with a name or an array of strings containing + * @param string|string[] $names A string with a name or an array of strings containing * the name(s) of the to add. * @param bool $sync When true, save the tags * @param int|null $id int Optional object id to add to this|these tag(s) * @return bool Returns false on error. * @since 6.0.0 */ - public function addMultiple($names, $sync = false, $id = null); + public function addMultiple($names, bool $sync = false, ?int $id = null): bool; /** * Delete tag/object relations from the db diff --git a/tests/lib/Share/ShareTest.php b/tests/lib/Share/ShareTest.php index c690ada99ca..35dc00739f6 100644 --- a/tests/lib/Share/ShareTest.php +++ b/tests/lib/Share/ShareTest.php @@ -21,6 +21,9 @@ namespace Test\Share; +use OC\Share\Share; +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IDBConnection; use OCP\IGroup; use OCP\IGroupManager; use OCP\IUser; @@ -35,36 +38,25 @@ use OCP\Share\IShare; class ShareTest extends \Test\TestCase { protected $itemType; - /** @var IUser */ - protected $user1; - /** @var IUser */ - protected $user2; - /** @var IUser */ - protected $user3; - /** @var IUser */ - protected $user4; - /** @var IUser */ - protected $user5; - /** @var IUser */ - protected $user6; - /** @var IUser */ - protected $groupAndUser_user; - - /** @var IGroup */ - protected $group1; - /** @var IGroup */ - protected $group2; - /** @var IGroup */ - protected $groupAndUser_group; - - protected $resharing; - protected $dateInFuture; - protected $dateInPast; - - /** @var IGroupManager */ - protected $groupManager; - /** @var IUserManager */ - protected $userManager; + protected IUser $user1; + protected IUser $user2; + protected IUser $user3; + protected IUser $user4; + protected IUser $user5; + protected IUser $user6; + protected IUser $groupAndUser_user; + + protected IGroup $group1; + protected IGroup $group2; + protected IGroup $groupAndUser_group; + + protected string $resharing; + protected string $dateInFuture; + protected string $dateInPast; + + protected IGroupManager $groupManager; + protected IUserManager $userManager; + private IDBConnection $connection; protected function setUp(): void { parent::setUp(); @@ -90,6 +82,7 @@ class ShareTest extends \Test\TestCase { $this->group1 = $this->groupManager->createGroup($this->getUniqueID('group1_')); $this->group2 = $this->groupManager->createGroup($this->getUniqueID('group2_')); $this->groupAndUser_group = $this->groupManager->createGroup($groupAndUserId); + $this->connection = \OC::$server->get(IDBConnection::class); $this->group1->addUser($this->user1); $this->group1->addUser($this->user2); @@ -99,7 +92,7 @@ class ShareTest extends \Test\TestCase { $this->groupAndUser_group->addUser($this->user2); $this->groupAndUser_group->addUser($this->user3); - \OC\Share\Share::registerBackend('test', 'Test\Share\Backend'); + Share::registerBackend('test', 'Test\Share\Backend'); \OC_Hook::clear('OCP\\Share'); \OC::registerShareHooks(\OC::$server->getSystemConfig()); $this->resharing = \OC::$server->getConfig()->getAppValue('core', 'shareapi_allow_resharing', 'yes'); @@ -113,8 +106,9 @@ class ShareTest extends \Test\TestCase { } protected function tearDown(): void { - $query = \OC_DB::prepare('DELETE FROM `*PREFIX*share` WHERE `item_type` = ?'); - $query->execute(['test']); + $query = $this->connection->getQueryBuilder(); + $query->delete('share')->andWhere($query->expr()->eq('item_type', $query->createNamedParameter('test'))); + $query->executeStatement(); \OC::$server->getConfig()->setAppValue('core', 'shareapi_allow_resharing', $this->resharing); $this->user1->delete(); @@ -136,39 +130,51 @@ class ShareTest extends \Test\TestCase { public function testGetItemSharedWithUser() { \OC_User::setUserId($this->user1->getUID()); - //add dummy values to the share table - $query = \OC_DB::prepare('INSERT INTO `*PREFIX*share` (' - .' `item_type`, `item_source`, `item_target`, `share_type`,' - .' `share_with`, `uid_owner`) VALUES (?,?,?,?,?,?)'); - $args = ['test', 99, 'target1', IShare::TYPE_USER, $this->user2->getUID(), $this->user1->getUID()]; - $query->execute($args); - $args = ['test', 99, 'target2', IShare::TYPE_USER, $this->user4->getUID(), $this->user1->getUID()]; - $query->execute($args); - $args = ['test', 99, 'target3', IShare::TYPE_USER, $this->user3->getUID(), $this->user2->getUID()]; - $query->execute($args); - $args = ['test', 99, 'target4', IShare::TYPE_USER, $this->user3->getUID(), $this->user4->getUID()]; - $query->execute($args); - $args = ['test', 99, 'target4', IShare::TYPE_USER, $this->user6->getUID(), $this->user4->getUID()]; - $query->execute($args); - - - $result1 = \OCP\Share::getItemSharedWithUser('test', 99, $this->user2->getUID(), $this->user1->getUID()); + // add dummy values to the share table + $query = $this->connection->getQueryBuilder(); + $query->insert('share') + ->values([ + 'item_type' => $query->createParameter('itemType'), + 'item_source' => $query->createParameter('itemSource'), + 'item_target' => $query->createParameter('itemTarget'), + 'share_type' => $query->createParameter('shareType'), + 'share_with' => $query->createParameter('shareWith'), + 'uid_owner' => $query->createParameter('uidOwner') + ]); + $args = [ + ['test', 99, 'target1', IShare::TYPE_USER, $this->user2->getUID(), $this->user1->getUID()], + ['test', 99, 'target2', IShare::TYPE_USER, $this->user4->getUID(), $this->user1->getUID()], + ['test', 99, 'target3', IShare::TYPE_USER, $this->user3->getUID(), $this->user2->getUID()], + ['test', 99, 'target4', IShare::TYPE_USER, $this->user3->getUID(), $this->user4->getUID()], + ['test', 99, 'target4', IShare::TYPE_USER, $this->user6->getUID(), $this->user4->getUID()], + ]; + foreach ($args as $row) { + $query->setParameter('itemType', $row[0]); + $query->setParameter('itemSource', $row[1], IQueryBuilder::PARAM_INT); + $query->setParameter('itemTarget', $row[2]); + $query->setParameter('shareType', $row[3], IQueryBuilder::PARAM_INT); + $query->setParameter('shareWith', $row[4]); + $query->setParameter('uidOwner', $row[5]); + $query->executeStatement(); + } + + $result1 = Share::getItemSharedWithUser('test', 99, $this->user2->getUID(), $this->user1->getUID()); $this->assertSame(1, count($result1)); $this->verifyResult($result1, ['target1']); - $result2 = \OCP\Share::getItemSharedWithUser('test', 99, null, $this->user1->getUID()); + $result2 = Share::getItemSharedWithUser('test', 99, null, $this->user1->getUID()); $this->assertSame(2, count($result2)); $this->verifyResult($result2, ['target1', 'target2']); - $result3 = \OCP\Share::getItemSharedWithUser('test', 99, $this->user3->getUID()); + $result3 = Share::getItemSharedWithUser('test', 99, $this->user3->getUID()); $this->assertSame(2, count($result3)); $this->verifyResult($result3, ['target3', 'target4']); - $result4 = \OCP\Share::getItemSharedWithUser('test', 99, null, null); + $result4 = Share::getItemSharedWithUser('test', 99, null, null); $this->assertSame(5, count($result4)); // 5 because target4 appears twice $this->verifyResult($result4, ['target1', 'target2', 'target3', 'target4']); - $result6 = \OCP\Share::getItemSharedWithUser('test', 99, $this->user6->getUID(), null); + $result6 = Share::getItemSharedWithUser('test', 99, $this->user6->getUID(), null); $this->assertSame(1, count($result6)); $this->verifyResult($result6, ['target4']); } @@ -176,38 +182,52 @@ class ShareTest extends \Test\TestCase { public function testGetItemSharedWithUserFromGroupShare() { \OC_User::setUserId($this->user1->getUID()); - //add dummy values to the share table - $query = \OC_DB::prepare('INSERT INTO `*PREFIX*share` (' - .' `item_type`, `item_source`, `item_target`, `share_type`,' - .' `share_with`, `uid_owner`) VALUES (?,?,?,?,?,?)'); - $args = ['test', 99, 'target1', IShare::TYPE_GROUP, $this->group1->getGID(), $this->user1->getUID()]; - $query->execute($args); - $args = ['test', 99, 'target2', IShare::TYPE_GROUP, $this->group2->getGID(), $this->user1->getUID()]; - $query->execute($args); - $args = ['test', 99, 'target3', IShare::TYPE_GROUP, $this->group1->getGID(), $this->user2->getUID()]; - $query->execute($args); - $args = ['test', 99, 'target4', IShare::TYPE_GROUP, $this->group1->getGID(), $this->user4->getUID()]; - $query->execute($args); + // add dummy values to the share table + $query = $this->connection->getQueryBuilder(); + $query->insert('share') + ->values([ + 'item_type' => $query->createParameter('itemType'), + 'item_source' => $query->createParameter('itemSource'), + 'item_target' => $query->createParameter('itemTarget'), + 'share_type' => $query->createParameter('shareType'), + 'share_with' => $query->createParameter('shareWith'), + 'uid_owner' => $query->createParameter('uidOwner') + ]); + $args = [ + ['test', 99, 'target1', IShare::TYPE_GROUP, $this->group1->getGID(), $this->user1->getUID()], + ['test', 99, 'target2', IShare::TYPE_GROUP, $this->group2->getGID(), $this->user1->getUID()], + ['test', 99, 'target3', IShare::TYPE_GROUP, $this->group1->getGID(), $this->user2->getUID()], + ['test', 99, 'target4', IShare::TYPE_GROUP, $this->group1->getGID(), $this->user4->getUID()], + ]; + foreach ($args as $row) { + $query->setParameter('itemType', $row[0]); + $query->setParameter('itemSource', $row[1], IQueryBuilder::PARAM_INT); + $query->setParameter('itemTarget', $row[2]); + $query->setParameter('shareType', $row[3], IQueryBuilder::PARAM_INT); + $query->setParameter('shareWith', $row[4]); + $query->setParameter('uidOwner', $row[5]); + $query->executeStatement(); + } // user2 is in group1 and group2 - $result1 = \OCP\Share::getItemSharedWithUser('test', 99, $this->user2->getUID(), $this->user1->getUID()); + $result1 = Share::getItemSharedWithUser('test', 99, $this->user2->getUID(), $this->user1->getUID()); $this->assertSame(2, count($result1)); $this->verifyResult($result1, ['target1', 'target2']); - $result2 = \OCP\Share::getItemSharedWithUser('test', 99, null, $this->user1->getUID()); + $result2 = Share::getItemSharedWithUser('test', 99, null, $this->user1->getUID()); $this->assertSame(2, count($result2)); $this->verifyResult($result2, ['target1', 'target2']); // user3 is in group1 and group2 - $result3 = \OCP\Share::getItemSharedWithUser('test', 99, $this->user3->getUID()); + $result3 = Share::getItemSharedWithUser('test', 99, $this->user3->getUID()); $this->assertSame(3, count($result3)); $this->verifyResult($result3, ['target1', 'target3', 'target4']); - $result4 = \OCP\Share::getItemSharedWithUser('test', 99, null, null); + $result4 = Share::getItemSharedWithUser('test', 99, null, null); $this->assertSame(4, count($result4)); $this->verifyResult($result4, ['target1', 'target2', 'target3', 'target4']); - $result6 = \OCP\Share::getItemSharedWithUser('test', 99, $this->user6->getUID(), null); + $result6 = Share::getItemSharedWithUser('test', 99, $this->user6->getUID(), null); $this->assertSame(0, count($result6)); } @@ -227,7 +247,7 @@ class ShareTest extends \Test\TestCase { * @param string $expectedResult */ public function testRemoveProtocolFromUrl($url, $expectedResult) { - $share = new \OC\Share\Share(); + $share = new Share(); $result = self::invokePrivate($share, 'removeProtocolFromUrl', [$url]); $this->assertSame($expectedResult, $result); } @@ -316,7 +336,7 @@ class ShareTest extends \Test\TestCase { } } -class DummyShareClass extends \OC\Share\Share { +class DummyShareClass extends Share { public static function groupItemsTest($items) { return parent::groupItems($items, 'test'); } diff --git a/tests/lib/TagsTest.php b/tests/lib/TagsTest.php index f6acc662424..2a68dfe3124 100644 --- a/tests/lib/TagsTest.php +++ b/tests/lib/TagsTest.php @@ -22,8 +22,10 @@ namespace Test; +use OCP\IDBConnection; use OCP\IUser; use OCP\IUserSession; +use Psr\Log\LoggerInterface; /** * Class TagsTest @@ -60,8 +62,8 @@ class TagsTest extends \Test\TestCase { ->willReturn($this->user); $this->objectType = $this->getUniqueID('type_'); - $this->tagMapper = new \OC\Tagging\TagMapper(\OC::$server->getDatabaseConnection()); - $this->tagMgr = new \OC\TagManager($this->tagMapper, $this->userSession, \OC::$server->getDatabaseConnection()); + $this->tagMapper = new \OC\Tagging\TagMapper(\OC::$server->get(IDBConnection::class)); + $this->tagMgr = new \OC\TagManager($this->tagMapper, $this->userSession, \OC::$server->get(IDBConnection::class), \OC::$server->get(LoggerInterface::class)); } protected function tearDown(): void { @@ -78,7 +80,7 @@ class TagsTest extends \Test\TestCase { ->expects($this->any()) ->method('getUser') ->willReturn(null); - $this->tagMgr = new \OC\TagManager($this->tagMapper, $this->userSession, \OC::$server->getDatabaseConnection()); + $this->tagMgr = new \OC\TagManager($this->tagMapper, $this->userSession, \OC::$server->getDatabaseConnection(), \OC::$server->get(LoggerInterface::class)); $this->assertNull($this->tagMgr->load($this->objectType)); } |