Put Mimeloader insertion and read in the same transaction

Signed-off-by: Thomas Citharel <tcit@tcit.fr>
nextcloud · Dec 14, 2022 · e6c7d64 · e6c7d64
1 parent 04f6862
commit e6c7d64
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 57 deletions.
diff --git 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
@@ -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<int, string> */
+	protected array $mimetypes;
 
-	/** @var array [mimetype => id] */
-	protected $mimetypeIds;
+	/** @psalm-var array<string, int> */
+	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
@@ -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
@@ -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);
 	}