diff --git a/lib/private/AllConfig.php b/lib/private/AllConfig.php index 58eee772fbf91..d48de298fad5e 100644 --- a/lib/private/AllConfig.php +++ b/lib/private/AllConfig.php @@ -242,10 +242,10 @@ public function setUserValue($userId, $appName, $key, $value, $preCondition = nu $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 c703c47c94eb4..48df3416df196 100644 --- a/tests/lib/AllConfigTest.php +++ b/tests/lib/AllConfigTest.php @@ -182,6 +182,42 @@ public function testSetUserValueWithPreConditionFailure() { $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');