diff --git a/lib/private/DB/QueryBuilder/FunctionBuilder/FunctionBuilder.php b/lib/private/DB/QueryBuilder/FunctionBuilder/FunctionBuilder.php index 1d74530635..bd36797309 100644 --- a/lib/private/DB/QueryBuilder/FunctionBuilder/FunctionBuilder.php +++ b/lib/private/DB/QueryBuilder/FunctionBuilder/FunctionBuilder.php @@ -63,4 +63,12 @@ class FunctionBuilder implements IFunctionBuilder { public function lower($field) { return new QueryFunction('LOWER(' . $this->helper->quoteColumnName($field) . ')'); } + + public function add($x, $y) { + return new QueryFunction($this->helper->quoteColumnName($x) . ' + ' . $this->helper->quoteColumnName($y)); + } + + public function subtract($x, $y) { + return new QueryFunction($this->helper->quoteColumnName($x) . ' - ' . $this->helper->quoteColumnName($y)); + } } diff --git a/lib/private/Lock/DBLockingProvider.php b/lib/private/Lock/DBLockingProvider.php index 016be64c09..6538dcbc2c 100644 --- a/lib/private/Lock/DBLockingProvider.php +++ b/lib/private/Lock/DBLockingProvider.php @@ -55,6 +55,11 @@ class DBLockingProvider extends AbstractLockingProvider { private $sharedLocks = []; + /** + * @var bool + */ + private $cacheSharedLocks; + /** * Check if we have an open shared lock for a path * @@ -73,8 +78,10 @@ class DBLockingProvider extends AbstractLockingProvider { */ protected function markAcquire(string $path, int $type) { parent::markAcquire($path, $type); - if ($type === self::LOCK_SHARED) { - $this->sharedLocks[$path] = true; + if ($this->cacheSharedLocks) { + if ($type === self::LOCK_SHARED) { + $this->sharedLocks[$path] = true; + } } } @@ -86,10 +93,12 @@ class DBLockingProvider extends AbstractLockingProvider { */ protected function markChange(string $path, int $targetType) { parent::markChange($path, $targetType); - if ($targetType === self::LOCK_SHARED) { - $this->sharedLocks[$path] = true; - } else if ($targetType === self::LOCK_EXCLUSIVE) { - $this->sharedLocks[$path] = false; + if ($this->cacheSharedLocks) { + if ($targetType === self::LOCK_SHARED) { + $this->sharedLocks[$path] = true; + } else if ($targetType === self::LOCK_EXCLUSIVE) { + $this->sharedLocks[$path] = false; + } } } @@ -98,12 +107,20 @@ class DBLockingProvider extends AbstractLockingProvider { * @param \OCP\ILogger $logger * @param \OCP\AppFramework\Utility\ITimeFactory $timeFactory * @param int $ttl + * @param bool $cacheSharedLocks */ - public function __construct(IDBConnection $connection, ILogger $logger, ITimeFactory $timeFactory, int $ttl = 3600) { + public function __construct( + IDBConnection $connection, + ILogger $logger, + ITimeFactory $timeFactory, + int $ttl = 3600, + $cacheSharedLocks = true + ) { $this->connection = $connection; $this->logger = $logger; $this->timeFactory = $timeFactory; $this->ttl = $ttl; + $this->cacheSharedLocks = $cacheSharedLocks; } /** @@ -203,6 +220,13 @@ class DBLockingProvider extends AbstractLockingProvider { 'UPDATE `*PREFIX*file_locks` SET `lock` = 0 WHERE `key` = ? AND `lock` = -1', [$path] ); + } else if (!$this->cacheSharedLocks) { + $query = $this->connection->getQueryBuilder(); + $query->update('file_locks') + ->set('lock', $query->func()->subtract('lock', $query->createNamedParameter(1))) + ->where($query->expr()->eq('key', $query->createNamedParameter($path))) + ->andWhere($query->expr()->gt('lock', $query->createNamedParameter(0))); + $query->execute(); } } @@ -256,11 +280,15 @@ class DBLockingProvider extends AbstractLockingProvider { /** * release all lock acquired by this instance which were marked using the mark* methods + * * @suppress SqlInjectionChecker */ public function releaseAll() { parent::releaseAll(); + if (!$this->cacheSharedLocks) { + return; + } // since we keep shared locks we need to manually clean those $lockedPaths = array_keys($this->sharedLocks); $lockedPaths = array_filter($lockedPaths, function ($path) { diff --git a/lib/private/Server.php b/lib/private/Server.php index 109fb002ce..fd32b09033 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -847,7 +847,13 @@ class Server extends ServerContainer implements IServerContainer { if (!($memcache instanceof \OC\Memcache\NullCache)) { return new MemcacheLockingProvider($memcache, $ttl); } - return new DBLockingProvider($c->getDatabaseConnection(), $c->getLogger(), new TimeFactory(), $ttl); + return new DBLockingProvider( + $c->getDatabaseConnection(), + $c->getLogger(), + new TimeFactory(), + $ttl, + !\OC::$CLI + ); } return new NoopLockingProvider(); }); diff --git a/lib/public/DB/QueryBuilder/IFunctionBuilder.php b/lib/public/DB/QueryBuilder/IFunctionBuilder.php index d867d9e5ed..e0e331c080 100644 --- a/lib/public/DB/QueryBuilder/IFunctionBuilder.php +++ b/lib/public/DB/QueryBuilder/IFunctionBuilder.php @@ -80,4 +80,20 @@ interface IFunctionBuilder { * @since 14.0.0 */ public function lower($field); + + /** + * @param mixed $x The first input field or number + * @param mixed $y The second input field or number + * @return IQueryFunction + * @since 14.0.0 + */ + public function add($x, $y); + + /** + * @param mixed $x The first input field or number + * @param mixed $y The second input field or number + * @return IQueryFunction + * @since 14.0.0 + */ + public function subtract($x, $y); } diff --git a/tests/lib/DB/QueryBuilder/FunctionBuilderTest.php b/tests/lib/DB/QueryBuilder/FunctionBuilderTest.php index 869faccc5c..1b998287c9 100644 --- a/tests/lib/DB/QueryBuilder/FunctionBuilderTest.php +++ b/tests/lib/DB/QueryBuilder/FunctionBuilderTest.php @@ -21,6 +21,7 @@ namespace Test\DB\QueryBuilder; use OC\DB\QueryBuilder\Literal; +use OCP\DB\QueryBuilder\IQueryBuilder; use Test\TestCase; /** @@ -89,4 +90,24 @@ class FunctionBuilderTest extends TestCase { $this->assertEquals('foobar', $query->execute()->fetchColumn()); } + + public function testAdd() { + $query = $this->connection->getQueryBuilder(); + + $query->select($query->func()->add($query->createNamedParameter(2, IQueryBuilder::PARAM_INT), new Literal(1))); + $query->from('appconfig') + ->setMaxResults(1); + + $this->assertEquals(3, $query->execute()->fetchColumn()); + } + + public function testSubtract() { + $query = $this->connection->getQueryBuilder(); + + $query->select($query->func()->subtract($query->createNamedParameter(2, IQueryBuilder::PARAM_INT), new Literal(1))); + $query->from('appconfig') + ->setMaxResults(1); + + $this->assertEquals(1, $query->execute()->fetchColumn()); + } } diff --git a/tests/lib/Lock/DBLockingProviderTest.php b/tests/lib/Lock/DBLockingProviderTest.php index fb27f9957a..e8419815e3 100644 --- a/tests/lib/Lock/DBLockingProviderTest.php +++ b/tests/lib/Lock/DBLockingProviderTest.php @@ -40,14 +40,14 @@ class DBLockingProviderTest extends LockingProvider { /** * @var \OCP\IDBConnection */ - private $connection; + protected $connection; /** * @var \OCP\AppFramework\Utility\ITimeFactory */ - private $timeFactory; + protected $timeFactory; - private $currentTime; + protected $currentTime; public function setUp() { $this->currentTime = time(); @@ -96,4 +96,31 @@ class DBLockingProviderTest extends LockingProvider { $query->execute(); return $query->fetchColumn(); } + + protected function getLockValue($key) { + $query = $this->connection->getQueryBuilder(); + $query->select('lock') + ->from('file_locks') + ->where($query->expr()->eq('key', $query->createNamedParameter($key))); + return $query->execute()->fetchColumn(); + } + + public function testDoubleShared() { + $this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED); + $this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED); + + $this->assertEquals(1, $this->getLockValue('foo')); + + $this->instance->releaseLock('foo', ILockingProvider::LOCK_SHARED); + + $this->assertEquals(1, $this->getLockValue('foo')); + + $this->instance->releaseLock('foo', ILockingProvider::LOCK_SHARED); + + $this->assertEquals(1, $this->getLockValue('foo')); + + $this->instance->releaseAll(); + + $this->assertEquals(0, $this->getLockValue('foo')); + } } diff --git a/tests/lib/Lock/NonCachingDBLockingProviderTest.php b/tests/lib/Lock/NonCachingDBLockingProviderTest.php new file mode 100644 index 0000000000..be5cd1600c --- /dev/null +++ b/tests/lib/Lock/NonCachingDBLockingProviderTest.php @@ -0,0 +1,54 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * 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 + * along with this program. If not, see . + * + */ + +namespace Test\Lock; + +use OCP\Lock\ILockingProvider; + +/** + * @group DB + * + * @package Test\Lock + */ +class NonCachingDBLockingProviderTest extends DBLockingProviderTest { + /** + * @return \OCP\Lock\ILockingProvider + */ + protected function getInstance() { + $this->connection = \OC::$server->getDatabaseConnection(); + return new \OC\Lock\DBLockingProvider($this->connection, \OC::$server->getLogger(), $this->timeFactory, 3600, false); + } + + public function testDoubleShared() { + $this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED); + $this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED); + + $this->assertEquals(2, $this->getLockValue('foo')); + + $this->instance->releaseLock('foo', ILockingProvider::LOCK_SHARED); + + $this->assertEquals(1, $this->getLockValue('foo')); + + $this->instance->releaseLock('foo', ILockingProvider::LOCK_SHARED); + + $this->assertEquals(0, $this->getLockValue('foo')); + } +} \ No newline at end of file