From 5de021cb9aed754191072ffd9b81cac39c5b8f86 Mon Sep 17 00:00:00 2001 From: Lucas Azevedo Date: Fri, 1 Sep 2023 14:05:15 -0300 Subject: [PATCH 1/2] fix(mimetype): Remove unnecessary transaction when storing a mime type Fixes #40064. This could be fixed by adding a rollback and starting a new transaction before the SELECT query, but in this case that would have the same effect as not using one. See https://dev.mysql.com/doc/refman/8.0/en/innodb-autocommit-commit-rollback.html and https://www.postgresql.org/docs/7.1/sql-begin.html#R1-SQL-BEGIN-1 Signed-off-by: Lucas Azevedo --- lib/private/Files/Type/Loader.php | 48 ++++++++++++++----------------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/lib/private/Files/Type/Loader.php b/lib/private/Files/Type/Loader.php index 20c298f21b3d5..7ca1b677fb591 100644 --- a/lib/private/Files/Type/Loader.php +++ b/lib/private/Files/Type/Loader.php @@ -116,35 +116,31 @@ public function reset() { * @return int inserted ID */ protected function store($mimetype) { - $mimetypeId = $this->atomic(function () use ($mimetype) { - try { - $insert = $this->dbConnection->getQueryBuilder(); - $insert->insert('mimetypes') - ->values([ - 'mimetype' => $insert->createNamedParameter($mimetype) - ]) - ->executeStatement(); - return $insert->getLastInsertId(); - } catch (DbalException $e) { - if ($e->getReason() !== DBException::REASON_UNIQUE_CONSTRAINT_VIOLATION) { - throw $e; - } - $qb = $this->dbConnection->getQueryBuilder(); - $qb->select('id') - ->from('mimetypes') - ->where($qb->expr()->eq('mimetype', $qb->createNamedParameter($mimetype))); - $result = $qb->executeQuery(); - $id = $result->fetchOne(); - $result->closeCursor(); - if ($id !== false) { - return (int) $id; - } + try { + $insert = $this->dbConnection->getQueryBuilder(); + $insert->insert('mimetypes') + ->values([ + 'mimetype' => $insert->createNamedParameter($mimetype) + ]) + ->executeStatement(); + $mimetypeId = $insert->getLastInsertId(); + } catch (DbalException $e) { + if ($e->getReason() !== DBException::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + throw $e; + } + + $qb = $this->dbConnection->getQueryBuilder(); + $qb->select('id') + ->from('mimetypes') + ->where($qb->expr()->eq('mimetype', $qb->createNamedParameter($mimetype))); + $result = $qb->executeQuery(); + $id = $result->fetchOne(); + $result->closeCursor(); + if ($id === false) { throw new \Exception("Database threw an unique constraint on inserting a new mimetype, but couldn't return the ID for this very mimetype"); } - }, $this->dbConnection); - if (!$mimetypeId) { - throw new \Exception("Failed to get mimetype id for $mimetype after trying to store it"); + $mimetypeId = (int) $id; } $this->mimetypes[$mimetypeId] = $mimetype; From c587f684bd18b2572249d46cd9ffe45e43f9c98f Mon Sep 17 00:00:00 2001 From: Lucas Azevedo Date: Mon, 4 Sep 2023 08:02:35 -0300 Subject: [PATCH 2/2] Wrap mimetype insert and getLastInsertId in a transaction Signed-off-by: Lucas Azevedo --- lib/private/Files/Type/Loader.php | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/private/Files/Type/Loader.php b/lib/private/Files/Type/Loader.php index 7ca1b677fb591..7032e6193850f 100644 --- a/lib/private/Files/Type/Loader.php +++ b/lib/private/Files/Type/Loader.php @@ -117,13 +117,15 @@ public function reset() { */ protected function store($mimetype) { try { - $insert = $this->dbConnection->getQueryBuilder(); - $insert->insert('mimetypes') - ->values([ - 'mimetype' => $insert->createNamedParameter($mimetype) - ]) - ->executeStatement(); - $mimetypeId = $insert->getLastInsertId(); + $mimetypeId = $this->atomic(function () use ($mimetype) { + $insert = $this->dbConnection->getQueryBuilder(); + $insert->insert('mimetypes') + ->values([ + 'mimetype' => $insert->createNamedParameter($mimetype) + ]) + ->executeStatement(); + return $insert->getLastInsertId(); + }, $this->dbConnection); } catch (DbalException $e) { if ($e->getReason() !== DBException::REASON_UNIQUE_CONSTRAINT_VIOLATION) { throw $e;