diff --git a/library/iFixit/Matryoshka/Backend.php b/library/iFixit/Matryoshka/Backend.php index 166940d..f9e74b5 100644 --- a/library/iFixit/Matryoshka/Backend.php +++ b/library/iFixit/Matryoshka/Backend.php @@ -137,7 +137,11 @@ public function getAndSet($key, callable $callback, int $expiration = 0, $value = $callback(); if ($value !== self::MISS) { - $this->set($key, $value, $expiration); + if ($reset) { + $this->set($key, $value, $expiration); + } else if (!$this->add($key, $value, $expiration)) { + $value = $this->get($key) ?? $value; + } } } diff --git a/library/iFixit/Matryoshka/Scope.php b/library/iFixit/Matryoshka/Scope.php index bbea6b2..9abeab5 100644 --- a/library/iFixit/Matryoshka/Scope.php +++ b/library/iFixit/Matryoshka/Scope.php @@ -23,15 +23,22 @@ public function getPrefix() { public function getScopePrefix(bool $reset = false, bool $generateOnMiss = true) { if ($this->scopePrefix === null || $reset) { - $scopeValue = $reset ? self::MISS : $this->backend->get($this->getScopeKey()); - if ($scopeValue === self::MISS) { - if ($generateOnMiss) { - $scopeValue = substr(md5(microtime() . $this->scopeName), 0, 16); - $this->backend->set($this->getScopeKey(), $scopeValue); - } else { + if (!$reset && !$generateOnMiss) { + $scopeValue = $this->backend->get($this->getScopeKey()); + if ($scopeValue === self::MISS) { return self::MISS; } + } else { + $scopeValue = $this->backend->getAndSet( + $this->getScopeKey(), + function() { + return substr(md5(microtime() . $this->scopeName), 0, 16); + }, + 0, + $reset + ); } + $this->scopePrefix = "{$scopeValue}-"; } diff --git a/tests/AbstractBackendTest.php b/tests/AbstractBackendTest.php index d12c56b..e5fd867 100644 --- a/tests/AbstractBackendTest.php +++ b/tests/AbstractBackendTest.php @@ -284,6 +284,42 @@ function() { return null; }, 0, $reset = true); $this->assertSame($value, $backend->get($key)); } + public function testGetAndSetReturnsComputedValue() { + $backend = new class extends Matryoshka\Ephemeral { + public function add($key, $value, $expiration = 0) { + return false; + } + }; + + [$key] = $this->getRandomKeyValue(); + $computedValue = 'computed'; + + $result = $backend->getAndSet($key, function() use ($computedValue) { + return $computedValue; + }); + + $this->assertSame($computedValue, $result); + } + + public function testGetAndSetConcurrentInitUsesFirstWriter() { + $racing = new RacingBackend($this->getBackend()); + [$key] = $this->getRandomKeyValue(); + + $firstWriterValue = 'first-writer'; + $secondWriterValue = 'second-writer'; + + $racing->afterNextGet(function($key) use ($racing, $firstWriterValue) { + $racing->set($key, $firstWriterValue); + }); + + $result = $racing->getAndSet($key, function() use ($secondWriterValue) { + return $secondWriterValue; + }); + + $this->assertSame($firstWriterValue, $result); + $this->assertSame($firstWriterValue, $racing->get($key)); + } + public function testgetAndSetMultiple() { $backend = $this->getBackend(); list($key1, $value1, $id1) = $this->getRandomKeyValueId(); @@ -490,6 +526,27 @@ protected function isCharExemptFromKeyEquivalence($char) { } } +/** + * Simulates a concurrent writer on a shared backend by injecting + * behavior between get() returning and the caller acting on the result. + */ +class RacingBackend extends Matryoshka\BackendWrap { + private $afterNextGet = null; + + public function afterNextGet(callable $fn) { + $this->afterNextGet = $fn; + } + + public function get($key) { + $result = $this->backend->get($key); + if ($fn = $this->afterNextGet) { + $this->afterNextGet = null; + $fn($key); + } + return $result; + } +} + // Exposes the array of cached values. class TestEphemeral extends Matryoshka\Ephemeral { public function getCache() { diff --git a/tests/ScopeTest.php b/tests/ScopeTest.php index 49bb109..234988f 100644 --- a/tests/ScopeTest.php +++ b/tests/ScopeTest.php @@ -98,4 +98,76 @@ public function testAbsoluteKey() { $this->assertEquals($scopedCache->getScopePrefix() . $key, $scopedCache->getAbsoluteKey($key)); } + + public function testConcurrentPrefixInitUsesFirstWriter() { + $racing = new RacingBackend(new Matryoshka\Ephemeral()); + $scope = new Matryoshka\Scope($racing, 'test-scope'); + + $competitorPrefix = 'competitor-won'; + + $racing->afterNextGet(function($key) use ($racing, $competitorPrefix) { + $racing->set($key, $competitorPrefix); + }); + + $prefix = $scope->getScopePrefix(); + + $this->assertSame("{$competitorPrefix}-", $prefix); + $this->assertSame($competitorPrefix, $racing->get('scope-test-scope')); + } + + public function testScopePrefixInitializationNoRace() { + $inner = new Matryoshka\Ephemeral(); + $scope = new Matryoshka\Scope($inner, 'test-scope'); + + $prefix = $scope->getScopePrefix(); + + $this->assertNotEmpty($prefix); + $this->assertStringEndsWith('-', $prefix); + $this->assertSame($prefix, $scope->getScopePrefix()); + } + + public function testDeleteScopeOverwritesIntentionally() { + $inner = new Matryoshka\Ephemeral(); + $scope = new Matryoshka\Scope($inner, 'test-scope'); + + $originalPrefix = $scope->getScopePrefix(); + $scope->deleteScope(); + $newPrefix = $scope->getScopePrefix(); + + $this->assertNotSame($originalPrefix, $newPrefix); + } + + public function testConcurrentPrefixInitPreservesFirstWriterData() { + $racing = new RacingBackend(new Matryoshka\Ephemeral()); + $scope = new Matryoshka\Scope($racing, 'test-scope'); + + $competitorPrefix = 'competitor-won'; + + $racing->afterNextGet(function($key) use ($racing, $competitorPrefix) { + $racing->set($key, $competitorPrefix); + $racing->set("{$competitorPrefix}-user-data", 'competitor-data'); + }); + + $scope->getScopePrefix(); + + $this->assertSame('competitor-data', $scope->get('user-data')); + } + + /** + * If add() fails and the re-fetch also misses (e.g. the backend lost + * the key between add and get), the generated prefix is still used. + */ + public function testScopePrefixNotEmpty() { + $backend = new class extends Matryoshka\Ephemeral { + public function add($key, $value, $expiration = 0) { + return false; + } + }; + $scope = new Matryoshka\Scope($backend, 'test-scope'); + + $prefix = $scope->getScopePrefix(); + + $this->assertNotSame('-', $prefix); + $this->assertStringEndsWith('-', $prefix); + } }