From 497d5017b15b421fc110176769b6c9c478fabba1 Mon Sep 17 00:00:00 2001 From: Daniel Beardsley Date: Tue, 23 Dec 2025 12:53:02 -0700 Subject: [PATCH 1/6] Scope: minimize the number of gets If getting the scope prefix value is a miss, then we all keys under the scope are by definition a miss, so we don't need to bother looking. This defers the generating and storing of the scope prefix value till we actually need it (on a set). In practice, this will reduce the number of gets if using getMultiple() or if you don't always *set()* on a miss. --- library/iFixit/Matryoshka/Scope.php | 31 +++++++++++++++++----- tests/ScopeTest.php | 40 +++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 6 deletions(-) diff --git a/library/iFixit/Matryoshka/Scope.php b/library/iFixit/Matryoshka/Scope.php index 045d347..bf20a48 100644 --- a/library/iFixit/Matryoshka/Scope.php +++ b/library/iFixit/Matryoshka/Scope.php @@ -21,13 +21,16 @@ public function getPrefix() { return $this->scopePrefix ?: $this->getScopePrefix(); } - public function getScopePrefix(bool $reset = false) { + public function getScopePrefix(bool $reset = false, bool $generateOnMiss = true) { if ($this->scopePrefix === null || $reset) { - $scopeValue = $this->backend->getAndSet($this->getScopeKey(), - function() { - return substr(md5(microtime() . $this->scopeName), 0, 16); - }, 0, $reset); - + $scopeValue = $reset ? self::MISS : $this->backend->get($this->getScopeKey()); + if ($scopeValue === self::MISS && !$generateOnMiss) { + return self::MISS; + } + if ($reset || ($scopeValue === self::MISS && $generateOnMiss)) { + $scopeValue = substr(md5(microtime() . $this->scopeName), 0, 16); + $this->backend->set($this->getScopeKey(), $this->scopePrefix); + } $this->scopePrefix = "{$scopeValue}-"; } @@ -52,4 +55,20 @@ public function deleteScope(): bool { private function getScopeKey() { return "scope-{$this->scopeName}"; } + + public function get($key) { + // If the scope prefix doesn't exist, all keys in this scope are a miss. + if (!$this->getScopePrefix(generateOnMiss: false)) { + return self::MISS; + } + return parent::get($key); + } + + public function getMultiple(array $keys) { + // If the scope prefix doesn't exist, all keys in this scope are a miss. + if (!$this->getScopePrefix(generateOnMiss: false)) { + return [array_fill_keys(array_keys($keys), self::MISS), $keys]; + } + return parent::getMultiple($keys); + } } diff --git a/tests/ScopeTest.php b/tests/ScopeTest.php index 9313956..8f89374 100644 --- a/tests/ScopeTest.php +++ b/tests/ScopeTest.php @@ -43,6 +43,46 @@ public function testScope() { $this->assertSame($value1, $scopedCache->get($key1)); } + public function testScopeShortCircuitGet() { + $scope = 'scope'; + $mockBackend = $this->getMockBuilder(Matryoshka\Ephemeral::class) + ->setMethods(['get']) + ->getMock(); + + // Assert get() is called only once because a missing scope value means + // that all underlying values are also missing. + $mockBackend->expects($this->once()) + ->method('get') + ->with($this->stringContains($scope)) + ->willReturn(Matryoshka\Backend::MISS); + $scopedCache = new Matryoshka\Scope($mockBackend, $scope); + + $key = (string)rand(1, 100000); + $this->assertNull($scopedCache->get($key)); + } + + public function testScopeShortCircuitGetMultiple() { + $scope = 'scope'; + $mockBackend = $this->getMockBuilder(Matryoshka\Ephemeral::class) + ->setMethods(['get']) + ->getMock(); + + // Assert get() is called only once because a missing scope value means + // that all underlying values are also missing. + $mockBackend->expects($this->once()) + ->method('get') + ->with($this->stringContains($scope)) + ->willReturn(Matryoshka\Backend::MISS); + $scopedCache = new Matryoshka\Scope($mockBackend, $scope); + + $key = (string)rand(1, 100000); + $keys = [$key => 'no matter']; + $this->assertSame( + [[$key => Matryoshka\Backend::MISS], $keys], + $scopedCache->getMultiple($keys) + ); + } + public function testAbsoluteKey() { $memoryCache = new Matryoshka\Ephemeral(); $scope = 'scope'; From 06219205e11fe001475993b3d622885c23cf5b96 Mon Sep 17 00:00:00 2001 From: Daniel Beardsley Date: Tue, 23 Dec 2025 13:03:16 -0700 Subject: [PATCH 2/6] Actions: update to latest version of checkout --- .github/workflows/php.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index 49281a4..e169661 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -26,7 +26,7 @@ jobs: compiler: jit steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Install libmemcached run: sudo apt-get install -y libmemcached-dev @@ -48,7 +48,7 @@ jobs: - name: Cache Composer packages id: composer-cache - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: vendor key: ${{ runner.os }}-php-${{ hashFiles('**/composer.lock') }} From b553fc4d79f04e1f1e1aa62a0a1bb571ff67ee1f Mon Sep 17 00:00:00 2001 From: Daniel Beardsley Date: Tue, 23 Dec 2025 17:54:33 -0700 Subject: [PATCH 3/6] ScopeTest: assert getMultiple() isn't called The Short circuit should prevent getMultiple from being called and we weren't explicitly check for that. --- tests/ScopeTest.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/ScopeTest.php b/tests/ScopeTest.php index 8f89374..a435ec7 100644 --- a/tests/ScopeTest.php +++ b/tests/ScopeTest.php @@ -64,7 +64,7 @@ public function testScopeShortCircuitGet() { public function testScopeShortCircuitGetMultiple() { $scope = 'scope'; $mockBackend = $this->getMockBuilder(Matryoshka\Ephemeral::class) - ->setMethods(['get']) + ->setMethods(['get', 'getMultiple']) ->getMock(); // Assert get() is called only once because a missing scope value means @@ -73,6 +73,11 @@ public function testScopeShortCircuitGetMultiple() { ->method('get') ->with($this->stringContains($scope)) ->willReturn(Matryoshka\Backend::MISS); + // Assert getMultiple() is never called because a missi + // we don't need to check individual keys. + $mockBackend->expects($this->never()) + ->method('getMultiple'); + $scopedCache = new Matryoshka\Scope($mockBackend, $scope); $key = (string)rand(1, 100000); From 4662947e7ff3599177c8bb101122d7fb7c269650 Mon Sep 17 00:00:00 2001 From: Daniel Beardsley Date: Tue, 23 Dec 2025 17:59:54 -0700 Subject: [PATCH 4/6] Scope: Fix a bug! We weren't using the $scopeValue we just created when doing the set(). Also, refactor it for clarity. Add a test to ensure we're setting the value. --- library/iFixit/Matryoshka/Scope.php | 17 ++++++++++------- tests/ScopeTest.php | 6 ++++-- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/library/iFixit/Matryoshka/Scope.php b/library/iFixit/Matryoshka/Scope.php index bf20a48..282c05f 100644 --- a/library/iFixit/Matryoshka/Scope.php +++ b/library/iFixit/Matryoshka/Scope.php @@ -24,14 +24,17 @@ 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 && !$generateOnMiss) { - return self::MISS; + if ($scopeValue === self::MISS) { + if ($generateOnMiss) { + $scopeValue = substr(md5(microtime() . $this->scopeName), 0, 16); + $this->scopePrefix = "{$scopeValue}-"; + $this->backend->set($this->getScopeKey(), $scopeValue); + } else { + return self::MISS; + } + } else { + $this->scopePrefix = "{$scopeValue}-"; } - if ($reset || ($scopeValue === self::MISS && $generateOnMiss)) { - $scopeValue = substr(md5(microtime() . $this->scopeName), 0, 16); - $this->backend->set($this->getScopeKey(), $this->scopePrefix); - } - $this->scopePrefix = "{$scopeValue}-"; } return $this->scopePrefix; diff --git a/tests/ScopeTest.php b/tests/ScopeTest.php index a435ec7..49bb109 100644 --- a/tests/ScopeTest.php +++ b/tests/ScopeTest.php @@ -12,8 +12,7 @@ protected function getBackend() { public function testScope() { $memoryCache = new Matryoshka\Ephemeral(); $scope = 'scope'; - $scopedCache = new Matryoshka\Scope($memoryCache, - $scope); + $scopedCache = new Matryoshka\Scope($memoryCache, $scope); list($key1, $value1) = $this->getRandomKeyValue(); list($key2, $value2) = $this->getRandomKeyValue(); @@ -41,6 +40,9 @@ public function testScope() { $this->assertTrue($scopedCache->set($key1, $value1)); $this->assertSame($value1, $scopedCache->get($key1)); + + $scopedCache = new Matryoshka\Scope($memoryCache, $scope); + $this->assertSame($value1, $scopedCache->get($key1)); } public function testScopeShortCircuitGet() { From 52c060283890cbf1ad1934168d0890b1c363c573 Mon Sep 17 00:00:00 2001 From: Daniel Beardsley Date: Tue, 27 Jan 2026 13:28:16 -0700 Subject: [PATCH 5/6] unit tests: use native exception assertions so phpunit stops complaining --- tests/KeyFixTest.php | 20 +++++++------------- tests/KeyShortenTest.php | 8 ++------ tests/MultiScopeTest.php | 8 ++------ 3 files changed, 11 insertions(+), 25 deletions(-) diff --git a/tests/KeyFixTest.php b/tests/KeyFixTest.php index d47c78a..dcc5998 100644 --- a/tests/KeyFixTest.php +++ b/tests/KeyFixTest.php @@ -43,20 +43,14 @@ public function testKeyShorten() { $this->assertSame($cachedValues, $memoryCache->getCache()); } - public function testValidation() { - try { - new Matryoshka\KeyFix(new TestEphemeral(), 5, Memcached::INVALID_CHARS_REGEX); - $this->fail("Doesn't complain about bad regex"); - } catch (Throwable $e) { - // Do nothing. - } + public function testValidationInvalidCharsRegex() { + $this->expectException(InvalidArgumentException::class); + new Matryoshka\KeyFix(new TestEphemeral(), 5, Memcached::INVALID_CHARS_REGEX); + } - try { - new Matryoshka\KeyFix(new TestEphemeral(), 40, ''); - $this->fail("Doesn't complain about bad regex"); - } catch (Throwable $e) { - // Do nothing. - } + public function testValidationBlankRegex() { + $this->expectWarning(); + new Matryoshka\KeyFix(new TestEphemeral(), 40, ''); } public function testNoBadChars() { diff --git a/tests/KeyShortenTest.php b/tests/KeyShortenTest.php index c621904..06db098 100644 --- a/tests/KeyShortenTest.php +++ b/tests/KeyShortenTest.php @@ -46,12 +46,8 @@ public function testKeyShorten() { } public function testKeyShortenLength() { - try { - new Matryoshka\KeyShorten(new TestEphemeral(), 5); - $this->fail("Doesn't throw InvalidArgumentException"); - } catch (InvalidArgumentException $e) { - // Do nothing. - } + $this->expectException(InvalidArgumentException::class); + new Matryoshka\KeyShorten(new TestEphemeral(), 5); } public function testAbsoluteKey() { diff --git a/tests/MultiScopeTest.php b/tests/MultiScopeTest.php index bdcb30d..244f47f 100644 --- a/tests/MultiScopeTest.php +++ b/tests/MultiScopeTest.php @@ -16,12 +16,8 @@ protected function getBackend() { } public function testMultiScopeBadArgument() { - try { - new Matryoshka\MultiScope(new Matryoshka\Ephemeral(), ['string']); - $this->fail("Doesn't throw InvalidArgumentException"); - } catch (InvalidArgumentException $e) { - // Do nothing. - } + $this->expectException(InvalidArgumentException::class); + new Matryoshka\MultiScope(new Matryoshka\Ephemeral(), ['string']); } public function testMultiScope() { From b8b24d1486041fecc6836f289879b10d54f59e1f Mon Sep 17 00:00:00 2001 From: Daniel Beardsley Date: Tue, 27 Jan 2026 13:28:27 -0700 Subject: [PATCH 6/6] Scope: DRY up the assignment --- library/iFixit/Matryoshka/Scope.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/library/iFixit/Matryoshka/Scope.php b/library/iFixit/Matryoshka/Scope.php index 282c05f..bbea6b2 100644 --- a/library/iFixit/Matryoshka/Scope.php +++ b/library/iFixit/Matryoshka/Scope.php @@ -27,14 +27,12 @@ public function getScopePrefix(bool $reset = false, bool $generateOnMiss = true) if ($scopeValue === self::MISS) { if ($generateOnMiss) { $scopeValue = substr(md5(microtime() . $this->scopeName), 0, 16); - $this->scopePrefix = "{$scopeValue}-"; $this->backend->set($this->getScopeKey(), $scopeValue); } else { return self::MISS; } - } else { - $this->scopePrefix = "{$scopeValue}-"; } + $this->scopePrefix = "{$scopeValue}-"; } return $this->scopePrefix;