diff options
| author | Joas Schilling <coding@schilljs.com> | 2024-09-13 09:16:35 +0200 |
|---|---|---|
| committer | backportbot[bot] <backportbot[bot]@users.noreply.github.com> | 2024-09-13 11:41:01 +0000 |
| commit | bc750a1d09daa8e0db7a1b61794c2ff7df19a279 (patch) | |
| tree | 8c1e61377c1b61f89a289dc30887ec7100de401d | |
| parent | 4e50fbb482e850f29bf6d7da8e326504b3b20611 (diff) | |
| download | nextcloud-server-backport/47933/stable29.tar.gz nextcloud-server-backport/47933/stable29.zip | |
fix(config): Throw PreconditionException always when it didn't matchbackport/47933/stable29
Previously even when the precondition did not match, the call "passed"
when the after value was the expected one. This however can lead to
race conditions, duplicate code excutions and other things.
Signed-off-by: Joas Schilling <coding@schilljs.com>
| -rw-r--r-- | lib/private/AllConfig.php | 6 | ||||
| -rw-r--r-- | tests/lib/AllConfigTest.php | 36 |
2 files changed, 39 insertions, 3 deletions
diff --git a/lib/private/AllConfig.php b/lib/private/AllConfig.php index de1c19c953a..f0169fee18d 100644 --- a/lib/private/AllConfig.php +++ b/lib/private/AllConfig.php @@ -269,10 +269,10 @@ class AllConfig implements IConfig { $prevValue = $this->getUserValue($userId, $appName, $key, null); if ($prevValue !== null) { - if ($prevValue === (string)$value) { - return; - } elseif ($preCondition !== null && $prevValue !== (string)$preCondition) { + if ($preCondition !== null && $prevValue !== (string)$preCondition) { throw new PreConditionNotMetException(); + } elseif ($prevValue === (string)$value) { + return; } else { $qb = $this->connection->getQueryBuilder(); $qb->update('preferences') diff --git a/tests/lib/AllConfigTest.php b/tests/lib/AllConfigTest.php index 540eff42e51..79599866fe2 100644 --- a/tests/lib/AllConfigTest.php +++ b/tests/lib/AllConfigTest.php @@ -183,6 +183,42 @@ class AllConfigTest extends \Test\TestCase { $config->deleteUserValue('userPreCond1', 'appPreCond', 'keyPreCond'); } + public function testSetUserValueWithPreConditionFailureWhenResultStillMatches(): void { + $this->expectException(\OCP\PreConditionNotMetException::class); + + $config = $this->getConfig(); + + $selectAllSQL = 'SELECT `userid`, `appid`, `configkey`, `configvalue` FROM `*PREFIX*preferences` WHERE `userid` = ?'; + + $config->setUserValue('userPreCond1', 'appPreCond', 'keyPreCond', 'valuePreCond'); + + $result = $this->connection->executeQuery($selectAllSQL, ['userPreCond1'])->fetchAll(); + + $this->assertCount(1, $result); + $this->assertEquals([ + 'userid' => 'userPreCond1', + 'appid' => 'appPreCond', + 'configkey' => 'keyPreCond', + 'configvalue' => 'valuePreCond' + ], $result[0]); + + // test if the method throws with invalid precondition when the value is the same + $config->setUserValue('userPreCond1', 'appPreCond', 'keyPreCond', 'valuePreCond', 'valuePreCond3'); + + $result = $this->connection->executeQuery($selectAllSQL, ['userPreCond1'])->fetchAll(); + + $this->assertCount(1, $result); + $this->assertEquals([ + 'userid' => 'userPreCond1', + 'appid' => 'appPreCond', + 'configkey' => 'keyPreCond', + 'configvalue' => 'valuePreCond' + ], $result[0]); + + // cleanup + $config->deleteUserValue('userPreCond1', 'appPreCond', 'keyPreCond'); + } + public function testSetUserValueUnchanged() { // TODO - FIXME until the dependency injection is handled properly (in AllConfig) $this->markTestSkipped('Skipped because this is just testable if database connection can be injected'); |