upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-02-03 20:41:22 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions 37

Merge 7eb90928f8 into d09b8c99de

Browse source
This commit is contained in:
Kate 2026-02-03 21:59:07 +01:00 committed by GitHub
commit 6957eec368
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 96 additions and 182 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
.github/workflows/phpunit-mysql-sharding.yml vendored
View file

@ -56,7 +56,7 @@ jobs:
strategy:
fail-fast: false
matrix:
php-versions: ['8.2']
php-versions: ['8.5']
mysql-versions: ['8.4']
name: Sharding - MySQL ${{ matrix.mysql-versions }} (PHP ${{ matrix.php-versions }}) - database tests

32
apps/dav/tests/unit/Command/RemoveInvalidSharesTest.php
View file

@ -11,6 +11,7 @@ namespace OCA\DAV\Tests\unit\Command;
use OCA\DAV\Command\RemoveInvalidShares;
use OCA\DAV\Connector\Sabre\Principal;
use OCA\DAV\DAV\RemoteUserPrincipalBackend;
use OCP\DB\Exception;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IDBConnection;
use OCP\Server;
@ -39,18 +40,25 @@ class RemoveInvalidSharesTest extends TestCase {
$this->principalBackend = $this->createMock(Principal::class);
$this->remoteUserPrincipalBackend = $this->createMock(RemoteUserPrincipalBackend::class);
$this->db->insertIfNotExist('*PREFIX*dav_shares', [
'principaluri' => 'principal:unknown',
'type' => 'calendar',
'access' => 2,
'resourceid' => 666,
]);
$this->db->insertIfNotExist('*PREFIX*dav_shares', [
'principaluri' => 'principals/remote-users/foobar',
'type' => 'calendar',
'access' => 2,
'resourceid' => 666,
]);
$qb = $this->db->getQueryBuilder();
$qb->insert('dav_shares');
foreach (['principal:unknown', 'principals/remote-users/foobar'] as $principaluri) {
$qb->values([
'principaluri' => $qb->createNamedParameter($principaluri),
'type' => $qb->createNamedParameter('calendar'),
'access' => $qb->createNamedParameter(2, IQueryBuilder::PARAM_INT),
'resourceid' => $qb->createNamedParameter(666, IQueryBuilder::PARAM_INT),
]);
try {
$qb->executeStatement();
} catch (Exception $e) {
if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
throw $e;
}
}
}
$this->command = new RemoveInvalidShares(
$this->db,

34
apps/user_ldap/lib/Mapping/AbstractMapping.php
View file

@ -416,23 +416,29 @@ abstract class AbstractMapping {
return false;
}
$row = [
'ldap_dn_hash' => $this->getDNHash($fdn),
'ldap_dn' => $fdn,
'owncloud_name' => $name,
'directory_uuid' => $uuid
];
$qb = $this->dbc->getQueryBuilder();
$qb
->insert($this->getTableName())
->values([
'ldap_dn_hash' => $qb->createNamedParameter($this->getDNHash($fdn)),
'ldap_dn' => $qb->createNamedParameter($fdn),
'owncloud_name' => $qb->createNamedParameter($name),
'directory_uuid' => $qb->createNamedParameter($uuid),
]);
try {
$result = $this->dbc->insertIfNotExist($this->getTableName(), $row);
if ((bool)$result === true) {
$this->cache[$fdn] = $name;
$this->localNameToDnCache?->set($name, $fdn, self::LOCAL_CACHE_TTL);
$qb->executeStatement();
$this->cache[$fdn] = $name;
$this->localNameToDnCache?->set($name, $fdn, self::LOCAL_CACHE_TTL);
return true;
} catch (\OCP\DB\Exception $e) {
if ($e->getReason() === \OCP\DB\Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
return false;
}
// insertIfNotExist returns values as int
return (bool)$result;
} catch (\Exception $e) {
return false;
throw $e;
}
}

1
build/psalm-baseline.xml
View file

@ -2693,7 +2693,6 @@
<file src="apps/user_ldap/lib/Mapping/AbstractMapping.php">
<DeprecatedMethod>
<code><![CDATA[getDatabasePlatform]]></code>
<code><![CDATA[insertIfNotExist]]></code>
</DeprecatedMethod>
<RedundantCondition>
<code><![CDATA[isset($qb)]]></code>

49
lib/private/DB/Adapter.php
View file

@ -8,7 +8,6 @@
namespace OC\DB;
use Doctrine\DBAL\Exception;
use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
use OC\DB\Exceptions\DbalException;
/**
@ -64,54 +63,6 @@ class Adapter {
$this->conn->commit();
}
/**
* Insert a row if the matching row does not exists. To accomplish proper race condition avoidance
* it is needed that there is also a unique constraint on the values. Then this method will
* catch the exception and return 0.
*
* @param string $table The table name (will replace *PREFIX* with the actual prefix)
* @param array $input data that should be inserted into the table (column name => value)
* @param array|null $compare List of values that should be checked for "if not exists"
* If this is null or an empty array, all keys of $input will be compared
* Please note: text fields (clob) must not be used in the compare array
* @return int number of inserted rows
* @throws Exception
* @deprecated 15.0.0 - use unique index and "try { $db->insert() } catch (UniqueConstraintViolationException $e) {}" instead, because it is more reliable and does not have the risk for deadlocks - see https://github.com/nextcloud/server/pull/12371
*/
public function insertIfNotExist($table, $input, ?array $compare = null) {
$compare = $compare ?: array_keys($input);
// Prepare column names and generate placeholders
$columns = '`' . implode('`,`', array_keys($input)) . '`';
$placeholders = implode(', ', array_fill(0, count($input), '?'));
$query = 'INSERT INTO `' . $table . '` (' . $columns . ') '
. 'SELECT ' . $placeholders . ' '
. 'FROM `' . $table . '` WHERE ';
$inserts = array_values($input);
foreach ($compare as $key) {
$query .= '`' . $key . '`';
if (is_null($input[$key])) {
$query .= ' IS NULL AND ';
} else {
$inserts[] = $input[$key];
$query .= ' = ? AND ';
}
}
$query = substr($query, 0, -5);
$query .= ' HAVING COUNT(*) = 0';
try {
return $this->conn->executeUpdate($query, $inserts);
} catch (UniqueConstraintViolationException $e) {
// This exception indicates a concurrent insert happened between
// the insert and the sub-select in the insert, which is safe to ignore.
// More details: https://github.com/nextcloud/server/pull/12315
return 0;
}
}
/**
* @throws \OCP\DB\Exception
*/

49
lib/private/DB/AdapterSqlite.php
View file

@ -7,8 +7,6 @@
*/
namespace OC\DB;
use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
class AdapterSqlite extends Adapter {
/**
* @param string $tableName
@ -30,53 +28,6 @@ class AdapterSqlite extends Adapter {
return $statement;
}
/**
* Insert a row if the matching row does not exists. To accomplish proper race condition avoidance
* it is needed that there is also a unique constraint on the values. Then this method will
* catch the exception and return 0.
*
* @param string $table The table name (will replace *PREFIX* with the actual prefix)
* @param array $input data that should be inserted into the table (column name => value)
* @param array|null $compare List of values that should be checked for "if not exists"
* If this is null or an empty array, all keys of $input will be compared
* Please note: text fields (clob) must not be used in the compare array
* @return int number of inserted rows
* @throws \Doctrine\DBAL\Exception
* @deprecated 15.0.0 - use unique index and "try { $db->insert() } catch (UniqueConstraintViolationException $e) {}" instead, because it is more reliable and does not have the risk for deadlocks - see https://github.com/nextcloud/server/pull/12371
*/
public function insertIfNotExist($table, $input, ?array $compare = null) {
if (empty($compare)) {
$compare = array_keys($input);
}
$fieldList = '`' . implode('`,`', array_keys($input)) . '`';
$query = "INSERT INTO `$table` ($fieldList) SELECT "
. str_repeat('?,', count($input) - 1) . '? '
. " WHERE NOT EXISTS (SELECT 1 FROM `$table` WHERE ";
$inserts = array_values($input);
foreach ($compare as $key) {
$query .= '`' . $key . '`';
if (is_null($input[$key])) {
$query .= ' IS NULL AND ';
} else {
$inserts[] = $input[$key];
$query .= ' = ? AND ';
}
}
$query = substr($query, 0, -5);
$query .= ')';
try {
return $this->conn->executeUpdate($query, $inserts);
} catch (UniqueConstraintViolationException $e) {
// if this is thrown then a concurrent insert happened between the insert and the sub-select in the insert, that should have avoided it
// it's fine to ignore this then
//
// more discussions about this can be found at https://github.com/nextcloud/server/pull/12315
return 0;
}
}
public function insertIgnoreConflict(string $table, array $values): int {
$builder = $this->conn->getQueryBuilder();
$builder->insert($table);

23
lib/private/DB/Connection.php
View file

@ -531,29 +531,6 @@ class Connection extends PrimaryReadReplicaConnection {
return parent::lastInsertId($seqName);
}
/**
* Insert a row if the matching row does not exists. To accomplish proper race condition avoidance
* it is needed that there is also a unique constraint on the values. Then this method will
* catch the exception and return 0.
*
* @param string $table The table name (will replace *PREFIX* with the actual prefix)
* @param array $input data that should be inserted into the table (column name => value)
* @param array|null $compare List of values that should be checked for "if not exists"
* If this is null or an empty array, all keys of $input will be compared
* Please note: text fields (clob) must not be used in the compare array
* @return int number of inserted rows
* @throws \Doctrine\DBAL\Exception
* @deprecated 15.0.0 - use unique index and "try { $db->insert() } catch (UniqueConstraintViolationException $e) {}" instead, because it is more reliable and does not have the risk for deadlocks - see https://github.com/nextcloud/server/pull/12371
*/
public function insertIfNotExist($table, $input, ?array $compare = null) {
try {
return $this->adapter->insertIfNotExist($table, $input, $compare);
} catch (\Exception $e) {
$this->logDatabaseException($e);
throw $e;
}
}
public function insertIgnoreConflict(string $table, array $values) : int {
try {
return $this->adapter->insertIgnoreConflict($table, $values);

8
lib/private/DB/ConnectionAdapter.php
View file

@ -78,14 +78,6 @@ class ConnectionAdapter implements IDBConnection {
}
}
public function insertIfNotExist(string $table, array $input, ?array $compare = null) {
try {
return $this->inner->insertIfNotExist($table, $input, $compare);
} catch (Exception $e) {
throw DbalException::wrap($e);
}
}
public function insertIgnoreConflict(string $table, array $values): int {
try {
return $this->inner->insertIgnoreConflict($table, $values);

19
lib/private/DB/MigrationService.php
View file

@ -17,6 +17,7 @@ use OC\App\InfoParser;
use OC\Migration\SimpleOutput;
use OCP\App\IAppManager;
use OCP\AppFramework\App;
use OCP\DB\Exception;
use OCP\DB\ISchemaWrapper;
use OCP\DB\Types;
use OCP\IConfig;
@ -285,10 +286,20 @@ class MigrationService {
* @param string $version
*/
private function markAsExecuted($version): void {
$this->connection->insertIfNotExist('*PREFIX*migrations', [
'app' => $this->appName,
'version' => $version
]);
$qb = $this->connection->getQueryBuilder();
$qb
->insert('migrations')
->values([
'app' => $qb->createNamedParameter($this->appName),
'version' => $qb->createNamedParameter($version),
]);
try {
$qb->executeStatement();
} catch (Exception $e) {
if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
throw $e;
}
}
}
/**

20
lib/private/Files/Cache/Storage.php
View file

@ -7,6 +7,7 @@
*/
namespace OC\Files\Cache;
use OCP\DB\Exception;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\Files\Storage\IStorage;
use OCP\IDBConnection;
@ -56,9 +57,22 @@ class Storage {
$this->numericId = (int)$row['numeric_id'];
} else {
$available = $isAvailable ? 1 : 0;
if ($connection->insertIfNotExist('*PREFIX*storages', ['id' => $this->storageId, 'available' => $available])) {
$this->numericId = $connection->lastInsertId('*PREFIX*storages');
} else {
$qb = $connection->getQueryBuilder();
$qb
->insert('storages')
->values([
'id' => $qb->createNamedParameter($this->storageId),
'available' => $qb->createNamedParameter($available, IQueryBuilder::PARAM_INT),
]);
try {
$qb->executeStatement();
$this->numericId = $qb->getLastInsertId();
} catch (Exception $e) {
if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
throw $e;
}
if ($row = self::getStorageById($this->storageId)) {
$this->numericId = (int)$row['numeric_id'];
} else {

18
lib/public/IDBConnection.php
View file

@ -133,24 +133,6 @@ interface IDBConnection {
*/
public function lastInsertId(string $table): int;
/**
* Insert a row if the matching row does not exists. To accomplish proper race condition avoidance
* it is needed that there is also a unique constraint on the values. Then this method will
* catch the exception and return 0.
*
* @param string $table The table name (will replace *PREFIX* with the actual prefix)
* @param array $input data that should be inserted into the table (column name => value)
* @param array|null $compare List of values that should be checked for "if not exists"
* If this is null or an empty array, all keys of $input will be compared
* Please note: text fields (clob) must not be used in the compare array
* @return int number of inserted rows
* @throws Exception used to be the removed dbal exception, since 21.0.0 it's \OCP\DB\Exception
* @since 6.0.0 - parameter $compare was added in 8.1.0, return type changed from boolean in 8.1.0
* @deprecated 15.0.0 - use unique index and "try { $db->insert() } catch (\OCP\DB\Exception $e) { if ($e->getReason() === \OCP\DB\Exception::REASON_CONSTRAINT_VIOLATION) {} }" instead, because it is more reliable and does not have the risk for deadlocks - see https://github.com/nextcloud/server/pull/12371
*/
public function insertIfNotExist(string $table, array $input, ?array $compare = null);
/**
*
* Insert a row if the row does not exist. Eventual conflicts during insert will be ignored.

23
tests/lib/DB/MigrationServiceTest.php
View file

@ -20,6 +20,7 @@ use OC\DB\Connection;
use OC\DB\MigrationService;
use OC\DB\SchemaWrapper;
use OCP\App\AppPathNotFoundException;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IDBConnection;
use OCP\Migration\IMigrationStep;
use PHPUnit\Framework\Attributes\DataProvider;
@ -102,6 +103,17 @@ class MigrationServiceTest extends \Test\TestCase {
$this->db->expects($this->once())
->method('migrateToSchema');
$qb = $this->createMock(IQueryBuilder::class);
$qb
->expects($this->once())
->method('insert')
->willReturn($qb);
$this->db
->expects($this->once())
->method('getQueryBuilder')
->willReturn($qb);
$wrappedSchema = $this->createMock(Schema::class);
$wrappedSchema->expects($this->atLeast(2))
->method('getTables')
@ -146,6 +158,17 @@ class MigrationServiceTest extends \Test\TestCase {
$this->db->expects($this->never())
->method('migrateToSchema');
$qb = $this->createMock(IQueryBuilder::class);
$qb
->expects($this->once())
->method('insert')
->willReturn($qb);
$this->db
->expects($this->once())
->method('getQueryBuilder')
->willReturn($qb);
$step = $this->createMock(IMigrationStep::class);
$step->expects($this->once())
->method('preSchemaChange');