From e6c7d641aa744fcffd6eeb020e8a983062ba3b44 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Tue, 13 Dec 2022 09:53:14 +0100 Subject: [PATCH] Put Mimeloader insertion and read in the same transaction Signed-off-by: Thomas Citharel --- lib/private/Files/Cache/SearchBuilder.php | 6 +- lib/private/Files/Type/Loader.php | 82 +++++++++++------------ lib/public/Files/IMimeTypeLoader.php | 8 +-- tests/lib/Files/Type/LoaderTest.php | 11 ++- 4 files changed, 50 insertions(+), 57 deletions(-) diff --git a/lib/private/Files/Cache/SearchBuilder.php b/lib/private/Files/Cache/SearchBuilder.php index 1a8c3637063de..fe00c6b5f9e2d 100644 --- a/lib/private/Files/Cache/SearchBuilder.php +++ b/lib/private/Files/Cache/SearchBuilder.php @@ -145,18 +145,18 @@ private function getOperatorFieldAndValue(ISearchComparison $operator) { if ($field === 'mimetype') { $value = (string)$value; if ($operator->getType() === ISearchComparison::COMPARE_EQUAL) { - $value = (int)$this->mimetypeLoader->getId($value); + $value = $this->mimetypeLoader->getId($value); } elseif ($operator->getType() === ISearchComparison::COMPARE_LIKE) { // transform "mimetype='foo/%'" to "mimepart='foo'" if (preg_match('|(.+)/%|', $value, $matches)) { $field = 'mimepart'; - $value = (int)$this->mimetypeLoader->getId($matches[1]); + $value = $this->mimetypeLoader->getId($matches[1]); $type = ISearchComparison::COMPARE_EQUAL; } elseif (strpos($value, '%') !== false) { throw new \InvalidArgumentException('Unsupported query value for mimetype: ' . $value . ', only values in the format "mime/type" or "mime/%" are supported'); } else { $field = 'mimetype'; - $value = (int)$this->mimetypeLoader->getId($value); + $value = $this->mimetypeLoader->getId($value); $type = ISearchComparison::COMPARE_EQUAL; } } diff --git a/lib/private/Files/Type/Loader.php b/lib/private/Files/Type/Loader.php index 8193a3f7b0350..3a196c329c419 100644 --- a/lib/private/Files/Type/Loader.php +++ b/lib/private/Files/Type/Loader.php @@ -24,6 +24,7 @@ */ namespace OC\Files\Type; +use OCP\AppFramework\Db\TTransactional; use OCP\Files\IMimeTypeLoader; use OCP\IDBConnection; @@ -33,15 +34,15 @@ * @package OC\Files\Type */ class Loader implements IMimeTypeLoader { + use TTransactional; - /** @var IDBConnection */ - private $dbConnection; + private IDBConnection $dbConnection; - /** @var array [id => mimetype] */ - protected $mimetypes; + /** @psalm-var array */ + protected array $mimetypes; - /** @var array [mimetype => id] */ - protected $mimetypeIds; + /** @psalm-var array */ + protected array $mimetypeIds; /** * @param IDBConnection $dbConnection @@ -54,11 +55,8 @@ public function __construct(IDBConnection $dbConnection) { /** * Get a mimetype from its ID - * - * @param int $id - * @return string|null */ - public function getMimetypeById($id) { + public function getMimetypeById(int $id): ?string { if (!$this->mimetypes) { $this->loadMimetypes(); } @@ -70,11 +68,8 @@ public function getMimetypeById($id) { /** * Get a mimetype ID, adding the mimetype to the DB if it does not exist - * - * @param string $mimetype - * @return int */ - public function getId($mimetype) { + public function getId(string $mimetype): int { if (!$this->mimetypeIds) { $this->loadMimetypes(); } @@ -86,11 +81,8 @@ public function getId($mimetype) { /** * Test if a mimetype exists in the database - * - * @param string $mimetype - * @return bool */ - public function exists($mimetype) { + public function exists(string $mimetype): bool { if (!$this->mimetypeIds) { $this->loadMimetypes(); } @@ -100,46 +92,50 @@ public function exists($mimetype) { /** * Clear all loaded mimetypes, allow for re-loading */ - public function reset() { + public function reset(): void { $this->mimetypes = []; $this->mimetypeIds = []; } /** * Store a mimetype in the DB - * - * @param string $mimetype - * @param int inserted ID */ - protected function store($mimetype) { - $this->dbConnection->insertIfNotExist('*PREFIX*mimetypes', [ - 'mimetype' => $mimetype - ]); - - $fetch = $this->dbConnection->getQueryBuilder(); - $fetch->select('id') - ->from('mimetypes') - ->where( - $fetch->expr()->eq('mimetype', $fetch->createNamedParameter($mimetype) - )); - - $result = $fetch->execute(); - $row = $result->fetch(); - $result->closeCursor(); + protected function store(string $mimetype): int { + $row = $this->atomic(function () use ($mimetype) { + $insert = $this->dbConnection->getQueryBuilder(); + $insert->insert('mimetypes') + ->values([ + 'mimetype' => $insert->createNamedParameter($mimetype) + ]) + ->executeStatement(); + + $fetch = $this->dbConnection->getQueryBuilder(); + $fetch->select('id') + ->from('mimetypes') + ->where( + $fetch->expr()->eq('mimetype', $fetch->createNamedParameter($mimetype) + )); + + $result = $fetch->executeQuery(); + $row = $result->fetch(); + $result->closeCursor(); + return $row; + }, $this->dbConnection); if (!$row) { throw new \Exception("Failed to get mimetype id for $mimetype after trying to store it"); } + $mimetypeId = (int) $row['id']; - $this->mimetypes[$row['id']] = $mimetype; - $this->mimetypeIds[$mimetype] = $row['id']; - return $row['id']; + $this->mimetypes[$mimetypeId] = $mimetype; + $this->mimetypeIds[$mimetype] = $mimetypeId; + return $mimetypeId; } /** * Load all mimetypes from DB */ - private function loadMimetypes() { + private function loadMimetypes(): void { $qb = $this->dbConnection->getQueryBuilder(); $qb->select('id', 'mimetype') ->from('mimetypes'); @@ -157,11 +153,9 @@ private function loadMimetypes() { /** * Update filecache mimetype based on file extension * - * @param string $ext file extension - * @param int $mimeTypeId * @return int number of changed rows */ - public function updateFilecache($ext, $mimeTypeId) { + public function updateFilecache(string $ext, int $mimeTypeId): int { $folderMimeTypeId = $this->getId('httpd/unix-directory'); $update = $this->dbConnection->getQueryBuilder(); $update->update('filecache') diff --git a/lib/public/Files/IMimeTypeLoader.php b/lib/public/Files/IMimeTypeLoader.php index faed18ed7d905..f8e36f74f7068 100644 --- a/lib/public/Files/IMimeTypeLoader.php +++ b/lib/public/Files/IMimeTypeLoader.php @@ -36,7 +36,7 @@ interface IMimeTypeLoader { * @return string|null * @since 8.2.0 */ - public function getMimetypeById($id); + public function getMimetypeById(int $id): ?string; /** * Get a mimetype ID, adding the mimetype to the DB if it does not exist @@ -45,7 +45,7 @@ public function getMimetypeById($id); * @return int * @since 8.2.0 */ - public function getId($mimetype); + public function getId(string $mimetype): int; /** * Test if a mimetype exists in the database @@ -54,12 +54,12 @@ public function getId($mimetype); * @return bool * @since 8.2.0 */ - public function exists($mimetype); + public function exists(string $mimetype): bool; /** * Clear all loaded mimetypes, allow for re-loading * * @since 8.2.0 */ - public function reset(); + public function reset(): void; } diff --git a/tests/lib/Files/Type/LoaderTest.php b/tests/lib/Files/Type/LoaderTest.php index fd3ec552dd249..ea016f9ededab 100644 --- a/tests/lib/Files/Type/LoaderTest.php +++ b/tests/lib/Files/Type/LoaderTest.php @@ -23,15 +23,14 @@ use OC\Files\Type\Loader; use OCP\IDBConnection; +use Test\TestCase; -class LoaderTest extends \Test\TestCase { - /** @var IDBConnection */ - protected $db; - /** @var Loader */ - protected $loader; +class LoaderTest extends TestCase { + protected IDBConnection $db; + protected Loader $loader; protected function setUp(): void { - $this->db = \OC::$server->getDatabaseConnection(); + $this->db = \OC::$server->get(IDBConnection::class); $this->loader = new Loader($this->db); }