From 2db958f6578686d1a23f06463e8628efe5433c34 Mon Sep 17 00:00:00 2001 From: Ankur Date: Wed, 24 Jun 2026 18:53:36 +0530 Subject: [PATCH 1/5] fix: persist config hash to disk so hasChanges works correctly across PHP-FPM processes --- src/Rox/Core/Core.php | 40 +++++-- tests/Rox/Core/ConfigChangeDetectionTests.php | 103 ++++++++++++++++++ 2 files changed, 136 insertions(+), 7 deletions(-) create mode 100644 tests/Rox/Core/ConfigChangeDetectionTests.php diff --git a/src/Rox/Core/Core.php b/src/Rox/Core/Core.php index 10de668..66bfbe2 100644 --- a/src/Rox/Core/Core.php +++ b/src/Rox/Core/Core.php @@ -140,11 +140,6 @@ final class Core */ private $_errorReporter; - /** - * @var ConfigurationFetchResult $_lastConfigurations - */ - private $_lastConfigurations = null; - /** * @var InternalFlagsInterface $_internalFlags */ @@ -350,8 +345,7 @@ public function fetch($isSourcePushing = false) $this->_targetGroupRepository->setTargetGroups($configuration->getTargetGroups()); $this->_flagSetter->setExperiments(); - $hasChanges = ($this->_lastConfigurations == null || !$this->_lastConfigurations->equals($result)); - $this->_lastConfigurations = $result; + $hasChanges = $this->_detectAndPersistConfigChanges($result); $fetcherStatus = $result->isFromCache() ? FetcherStatus::AppliedFromLocalStorage : FetcherStatus::AppliedFromNetwork; @@ -405,6 +399,38 @@ public function dynamicApi(EntitiesProviderInterface $entitiesProvider) return new DynamicApi($this->_flagRepository, $entitiesProvider); } + /** + * Compares the fetched config against the last known hash stored on disk. + * Returns true if the config changed (or is seen for the first time). + * Persists the new hash so the next process can compare against it. + * + * @param ConfigurationFetchResult $result + * @return bool + */ + private function _detectAndPersistConfigChanges(ConfigurationFetchResult $result) + { + $currentHash = md5(json_encode($result->getParsedData())); + $hashFile = join(DIRECTORY_SEPARATOR, [ + sys_get_temp_dir(), + 'rollout', + 'cache', + 'config_hash_' . md5($this->_sdkSettings->getApiKey()) . '.txt' + ]); + + $lastHash = @file_get_contents($hashFile); + $hasChanges = ($lastHash === false || $lastHash !== $currentHash); + + if ($hasChanges) { + $dir = dirname($hashFile); + if (!is_dir($dir)) { + @mkdir($dir, 0755, true); + } + @file_put_contents($hashFile, $currentHash); + } + + return $hasChanges; + } + /** * @param RoxOptionsInterface|null $options * @param int|null $cacheTtl diff --git a/tests/Rox/Core/ConfigChangeDetectionTests.php b/tests/Rox/Core/ConfigChangeDetectionTests.php new file mode 100644 index 0000000..3f8a6ab --- /dev/null +++ b/tests/Rox/Core/ConfigChangeDetectionTests.php @@ -0,0 +1,103 @@ +_hashFile = join(DIRECTORY_SEPARATOR, [ + sys_get_temp_dir(), + 'rollout', + 'cache', + 'config_hash_' . md5('aaaaaaaaaaaaaaaaaaaaaaaa') . '.txt' + ]); + + @unlink($this->_hashFile); + } + + protected function tearDown(): void + { + @unlink($this->_hashFile); + parent::tearDown(); + } + + private function _callDetect(Core $core, ConfigurationFetchResult $result) + { + $method = new \ReflectionMethod(Core::class, '_detectAndPersistConfigChanges'); + $method->setAccessible(true); + return $method->invoke($core, $result); + } + + private function _makeCore() + { + $sdkSettings = \Mockery::mock(SdkSettingsInterface::class) + ->shouldReceive('getApiKey') + ->andReturn('aaaaaaaaaaaaaaaaaaaaaaaa') + ->getMock(); + + $core = new Core(); + + $prop = new \ReflectionProperty(Core::class, '_sdkSettings'); + $prop->setAccessible(true); + $prop->setValue($core, $sdkSettings); + + return $core; + } + + public function testHasChangesIsTrueOnFirstFetch() + { + $core = $this->_makeCore(); + $result = new ConfigurationFetchResult(['flag' => 'on'], ConfigurationSource::CDN); + + $this->assertTrue($this->_callDetect($core, $result)); + } + + public function testHasChangesIsFalseWhenConfigUnchangedAcrossProcesses() + { + $core = $this->_makeCore(); + $result = new ConfigurationFetchResult(['flag' => 'on'], ConfigurationSource::CDN); + + // First call simulates the previous process writing the hash + $this->_callDetect($core, $result); + + // Second call simulates a new process with the same config + $core2 = $this->_makeCore(); + $this->assertFalse($this->_callDetect($core2, $result)); + } + + public function testHasChangesIsTrueWhenConfigChangedAcrossProcesses() + { + $core = $this->_makeCore(); + $oldResult = new ConfigurationFetchResult(['flag' => 'on'], ConfigurationSource::CDN); + $this->_callDetect($core, $oldResult); + + $core2 = $this->_makeCore(); + $newResult = new ConfigurationFetchResult(['flag' => 'off'], ConfigurationSource::CDN); + $this->assertTrue($this->_callDetect($core2, $newResult)); + } + + public function testHashFileIsWrittenOnChange() + { + $core = $this->_makeCore(); + $result = new ConfigurationFetchResult(['flag' => 'on'], ConfigurationSource::CDN); + + $this->_callDetect($core, $result); + + $this->assertFileExists($this->_hashFile); + $this->assertEquals(md5(json_encode(['flag' => 'on'])), file_get_contents($this->_hashFile)); + } +} From bbf69a402c6c22b8966c3650603fa13cc7e76ba8 Mon Sep 17 00:00:00 2001 From: Ankur Date: Wed, 24 Jun 2026 19:27:23 +0530 Subject: [PATCH 2/5] demo app update with CBP --- demo.php | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/demo.php b/demo.php index aa0c328..22715f5 100644 --- a/demo.php +++ b/demo.php @@ -6,9 +6,10 @@ use Kevinrob\GuzzleCache\Storage\DoctrineCacheStorage; use Monolog\Handler\StreamHandler; use Monolog\Logger; -use Rox\Core\Consts\Environment; use Rox\Core\Logging\LoggerFactory; use Rox\Core\Logging\MonologLoggerFactory; +use Rox\Core\Configuration\ConfigurationFetchedArgs; +use Rox\Core\Configuration\FetcherStatus; use Rox\Server\Flags\RoxFlag; use Rox\Server\Rox; use Rox\Server\RoxOptions; @@ -33,16 +34,19 @@ public function __construct() $devModeKey = getenv('ROLLOUT_DEV_MODE_KEY') ?: DEFAULT_DEV_MODE_KEY; +$fetchInfo = ['status' => 'not fired', 'hasChanges' => null]; + $roxOptionsBuilder = (new RoxOptionsBuilder()) ->setDevModeKey($devModeKey) - ->setNetworkConfigurationsOptions(new NetworkConfigurationsOptions( - 'https://api.test.rollout.io/device/get_configuration', - 'https://rox-conf.test.rollout.io', - 'https://api.test.rollout.io/device/update_state_store', - 'https://rox-state.test.rollout.io', - 'https://analytic.test.rollout.io', - 'https://notify.bugsnag.com' - )); + ->setConfigurationFetchedHandler(function (ConfigurationFetchedArgs $args) use (&$fetchInfo) { + $statusMap = [ + FetcherStatus::AppliedFromNetwork => 'AppliedFromNetwork', + FetcherStatus::AppliedFromLocalStorage => 'AppliedFromLocalStorage', + FetcherStatus::ErrorFetchedFailed => 'ErrorFetchedFailed', + ]; + $fetchInfo['status'] = $statusMap[$args->getFetcherStatus()] ?? $args->getFetcherStatus(); + $fetchInfo['hasChanges'] = $args->isHasChanges() ? 'true' : 'false'; + }); echo '
';
 
@@ -86,7 +90,7 @@ public function __construct()
         ->setCacheStorage(new DoctrineCacheStorage(
             new ChainCache([
                 new ArrayCache(),
-                new FilesystemCache('/tmp/rollout/cache'),
+                new FilesystemCache(join(DIRECTORY_SEPARATOR, [sys_get_temp_dir(), 'rollout', 'cache'])),
             ])
         ))
         ->setLogCacheHitsAndMisses(true)
@@ -109,4 +113,10 @@ public function __construct()
     echo "demo.demoFlag: FEATURE IS OFF\n";
 }
 
+echo "\n--- Configuration Fetch Info ---\n";
+echo "Status:     {$fetchInfo['status']}\n";
+echo "hasChanges: {$fetchInfo['hasChanges']}\n";
+echo "--------------------------------\n";
+echo "(Refresh the page - second load should show AppliedFromLocalStorage and hasChanges: false)\n";
+
 echo '
'; \ No newline at end of file From 8e6383607014849e02932e7395025cb2e6485d76 Mon Sep 17 00:00:00 2001 From: Ankur Date: Thu, 25 Jun 2026 11:16:34 +0530 Subject: [PATCH 3/5] use FilesystemCache for config hash persistence instead of raw file I/O --- src/Rox/Core/Core.php | 17 +++++------- tests/Rox/Core/ConfigChangeDetectionTests.php | 27 +++++++++---------- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/src/Rox/Core/Core.php b/src/Rox/Core/Core.php index 66bfbe2..117df25 100644 --- a/src/Rox/Core/Core.php +++ b/src/Rox/Core/Core.php @@ -410,22 +410,19 @@ public function dynamicApi(EntitiesProviderInterface $entitiesProvider) private function _detectAndPersistConfigChanges(ConfigurationFetchResult $result) { $currentHash = md5(json_encode($result->getParsedData())); - $hashFile = join(DIRECTORY_SEPARATOR, [ + $cacheKey = 'config_hash_' . $this->_sdkSettings->getApiKey(); + + $cache = new FilesystemCache(join(DIRECTORY_SEPARATOR, [ sys_get_temp_dir(), 'rollout', - 'cache', - 'config_hash_' . md5($this->_sdkSettings->getApiKey()) . '.txt' - ]); + 'cache' + ])); - $lastHash = @file_get_contents($hashFile); + $lastHash = $cache->fetch($cacheKey); $hasChanges = ($lastHash === false || $lastHash !== $currentHash); if ($hasChanges) { - $dir = dirname($hashFile); - if (!is_dir($dir)) { - @mkdir($dir, 0755, true); - } - @file_put_contents($hashFile, $currentHash); + $cache->save($cacheKey, $currentHash); } return $hasChanges; diff --git a/tests/Rox/Core/ConfigChangeDetectionTests.php b/tests/Rox/Core/ConfigChangeDetectionTests.php index 3f8a6ab..8a4c34f 100644 --- a/tests/Rox/Core/ConfigChangeDetectionTests.php +++ b/tests/Rox/Core/ConfigChangeDetectionTests.php @@ -2,36 +2,36 @@ namespace Rox\Core; +use Doctrine\Common\Cache\FilesystemCache; use Rox\Core\Client\SdkSettingsInterface; use Rox\Core\Network\ConfigurationFetchResult; use Rox\Core\Network\ConfigurationSource; use Rox\RoxTestCase; +const TEST_API_KEY = 'aaaaaaaaaaaaaaaaaaaaaaaa'; +const TEST_CACHE_KEY = 'config_hash_' . TEST_API_KEY; + class ConfigChangeDetectionTests extends RoxTestCase { /** - * @var string + * @var FilesystemCache */ - private $_hashFile; + private $_cache; protected function setUp(): void { parent::setUp(); - - // Use a predictable hash file path matching what Core produces for the test api key - $this->_hashFile = join(DIRECTORY_SEPARATOR, [ + $this->_cache = new FilesystemCache(join(DIRECTORY_SEPARATOR, [ sys_get_temp_dir(), 'rollout', - 'cache', - 'config_hash_' . md5('aaaaaaaaaaaaaaaaaaaaaaaa') . '.txt' - ]); - - @unlink($this->_hashFile); + 'cache' + ])); + $this->_cache->delete(TEST_CACHE_KEY); } protected function tearDown(): void { - @unlink($this->_hashFile); + $this->_cache->delete(TEST_CACHE_KEY); parent::tearDown(); } @@ -90,14 +90,13 @@ public function testHasChangesIsTrueWhenConfigChangedAcrossProcesses() $this->assertTrue($this->_callDetect($core2, $newResult)); } - public function testHashFileIsWrittenOnChange() + public function testHashIsPersistedOnChange() { $core = $this->_makeCore(); $result = new ConfigurationFetchResult(['flag' => 'on'], ConfigurationSource::CDN); $this->_callDetect($core, $result); - $this->assertFileExists($this->_hashFile); - $this->assertEquals(md5(json_encode(['flag' => 'on'])), file_get_contents($this->_hashFile)); + $this->assertEquals(md5(json_encode(['flag' => 'on'])), $this->_cache->fetch(TEST_CACHE_KEY)); } } From 5a1333e02c550c70cc5a0192bf631835c215e39a Mon Sep 17 00:00:00 2001 From: Ankur Date: Thu, 25 Jun 2026 19:47:47 +0530 Subject: [PATCH 4/5] now hasChanges from CDN cache hit/miss instead of a separate hash file --- src/Rox/Core/Core.php | 31 +----- tests/Rox/Core/ConfigChangeDetectionTests.php | 102 ------------------ 2 files changed, 1 insertion(+), 132 deletions(-) delete mode 100644 tests/Rox/Core/ConfigChangeDetectionTests.php diff --git a/src/Rox/Core/Core.php b/src/Rox/Core/Core.php index 117df25..e69f0ca 100644 --- a/src/Rox/Core/Core.php +++ b/src/Rox/Core/Core.php @@ -345,7 +345,7 @@ public function fetch($isSourcePushing = false) $this->_targetGroupRepository->setTargetGroups($configuration->getTargetGroups()); $this->_flagSetter->setExperiments(); - $hasChanges = $this->_detectAndPersistConfigChanges($result); + $hasChanges = !$result->isFromCache(); $fetcherStatus = $result->isFromCache() ? FetcherStatus::AppliedFromLocalStorage : FetcherStatus::AppliedFromNetwork; @@ -399,35 +399,6 @@ public function dynamicApi(EntitiesProviderInterface $entitiesProvider) return new DynamicApi($this->_flagRepository, $entitiesProvider); } - /** - * Compares the fetched config against the last known hash stored on disk. - * Returns true if the config changed (or is seen for the first time). - * Persists the new hash so the next process can compare against it. - * - * @param ConfigurationFetchResult $result - * @return bool - */ - private function _detectAndPersistConfigChanges(ConfigurationFetchResult $result) - { - $currentHash = md5(json_encode($result->getParsedData())); - $cacheKey = 'config_hash_' . $this->_sdkSettings->getApiKey(); - - $cache = new FilesystemCache(join(DIRECTORY_SEPARATOR, [ - sys_get_temp_dir(), - 'rollout', - 'cache' - ])); - - $lastHash = $cache->fetch($cacheKey); - $hasChanges = ($lastHash === false || $lastHash !== $currentHash); - - if ($hasChanges) { - $cache->save($cacheKey, $currentHash); - } - - return $hasChanges; - } - /** * @param RoxOptionsInterface|null $options * @param int|null $cacheTtl diff --git a/tests/Rox/Core/ConfigChangeDetectionTests.php b/tests/Rox/Core/ConfigChangeDetectionTests.php deleted file mode 100644 index 8a4c34f..0000000 --- a/tests/Rox/Core/ConfigChangeDetectionTests.php +++ /dev/null @@ -1,102 +0,0 @@ -_cache = new FilesystemCache(join(DIRECTORY_SEPARATOR, [ - sys_get_temp_dir(), - 'rollout', - 'cache' - ])); - $this->_cache->delete(TEST_CACHE_KEY); - } - - protected function tearDown(): void - { - $this->_cache->delete(TEST_CACHE_KEY); - parent::tearDown(); - } - - private function _callDetect(Core $core, ConfigurationFetchResult $result) - { - $method = new \ReflectionMethod(Core::class, '_detectAndPersistConfigChanges'); - $method->setAccessible(true); - return $method->invoke($core, $result); - } - - private function _makeCore() - { - $sdkSettings = \Mockery::mock(SdkSettingsInterface::class) - ->shouldReceive('getApiKey') - ->andReturn('aaaaaaaaaaaaaaaaaaaaaaaa') - ->getMock(); - - $core = new Core(); - - $prop = new \ReflectionProperty(Core::class, '_sdkSettings'); - $prop->setAccessible(true); - $prop->setValue($core, $sdkSettings); - - return $core; - } - - public function testHasChangesIsTrueOnFirstFetch() - { - $core = $this->_makeCore(); - $result = new ConfigurationFetchResult(['flag' => 'on'], ConfigurationSource::CDN); - - $this->assertTrue($this->_callDetect($core, $result)); - } - - public function testHasChangesIsFalseWhenConfigUnchangedAcrossProcesses() - { - $core = $this->_makeCore(); - $result = new ConfigurationFetchResult(['flag' => 'on'], ConfigurationSource::CDN); - - // First call simulates the previous process writing the hash - $this->_callDetect($core, $result); - - // Second call simulates a new process with the same config - $core2 = $this->_makeCore(); - $this->assertFalse($this->_callDetect($core2, $result)); - } - - public function testHasChangesIsTrueWhenConfigChangedAcrossProcesses() - { - $core = $this->_makeCore(); - $oldResult = new ConfigurationFetchResult(['flag' => 'on'], ConfigurationSource::CDN); - $this->_callDetect($core, $oldResult); - - $core2 = $this->_makeCore(); - $newResult = new ConfigurationFetchResult(['flag' => 'off'], ConfigurationSource::CDN); - $this->assertTrue($this->_callDetect($core2, $newResult)); - } - - public function testHashIsPersistedOnChange() - { - $core = $this->_makeCore(); - $result = new ConfigurationFetchResult(['flag' => 'on'], ConfigurationSource::CDN); - - $this->_callDetect($core, $result); - - $this->assertEquals(md5(json_encode(['flag' => 'on'])), $this->_cache->fetch(TEST_CACHE_KEY)); - } -} From 3eeb2a2db4791589e0fdf2d7710bd809974b3616 Mon Sep 17 00:00:00 2001 From: Ankur Date: Fri, 26 Jun 2026 11:50:50 +0530 Subject: [PATCH 5/5] treat REVALIDATED (304) as unchanged config; centralise cache states in CacheStatus --- src/Rox/Core/Core.php | 2 +- src/Rox/Core/Network/AbstractHttpResponse.php | 20 ++++++++++++++-- src/Rox/Core/Network/CacheStatus.php | 20 ++++++++++++++++ .../Core/Network/ConfigurationFetchResult.php | 20 ++++++++-------- src/Rox/Core/Network/ConfigurationFetcher.php | 2 +- .../Core/Network/ConfigurationFetcherBase.php | 8 ++++--- src/Rox/Core/Network/GuzzleHttpClient.php | 9 +++++-- .../Core/Network/HttpResponseInterface.php | 5 ++-- src/Rox/Core/Network/Psr7ResponseWrapper.php | 10 +++++--- .../Network/ConfigurationFetcherTests.php | 24 ++++++++++++++++++- tests/Rox/Core/Network/TestHttpResponse.php | 17 ++++++------- 11 files changed, 104 insertions(+), 33 deletions(-) create mode 100644 src/Rox/Core/Network/CacheStatus.php diff --git a/src/Rox/Core/Core.php b/src/Rox/Core/Core.php index e69f0ca..b31fbd2 100644 --- a/src/Rox/Core/Core.php +++ b/src/Rox/Core/Core.php @@ -345,7 +345,7 @@ public function fetch($isSourcePushing = false) $this->_targetGroupRepository->setTargetGroups($configuration->getTargetGroups()); $this->_flagSetter->setExperiments(); - $hasChanges = !$result->isFromCache(); + $hasChanges = !$result->isContentUnchanged(); $fetcherStatus = $result->isFromCache() ? FetcherStatus::AppliedFromLocalStorage : FetcherStatus::AppliedFromNetwork; diff --git a/src/Rox/Core/Network/AbstractHttpResponse.php b/src/Rox/Core/Network/AbstractHttpResponse.php index 8413659..59e6907 100644 --- a/src/Rox/Core/Network/AbstractHttpResponse.php +++ b/src/Rox/Core/Network/AbstractHttpResponse.php @@ -13,11 +13,27 @@ final function isSuccessfulStatusCode() $this->getStatusCode() <= 299; } + /** + * @return string + */ + function getCacheStatus() + { + return CacheStatus::MISS; + } + + /** + * @return bool + */ + final function isFromCache() + { + return CacheStatus::isFromCache($this->getCacheStatus()); + } + /** * @return bool */ - function isFromCache() + final function isContentUnchanged() { - return false; + return CacheStatus::isContentUnchanged($this->getCacheStatus()); } } diff --git a/src/Rox/Core/Network/CacheStatus.php b/src/Rox/Core/Network/CacheStatus.php new file mode 100644 index 0000000..836fdcb --- /dev/null +++ b/src/Rox/Core/Network/CacheStatus.php @@ -0,0 +1,20 @@ +_source = $source; $this->_parsedData = $parsedData; - $this->_isFromCache = $isFromCache; + $this->_cacheStatus = $cacheStatus; } /** @@ -54,16 +56,14 @@ public function getParsedData() */ public function isFromCache() { - return $this->_isFromCache; + return CacheStatus::isFromCache($this->_cacheStatus); } /** - * @param ConfigurationFetchResult $other * @return bool */ - public function equals(ConfigurationFetchResult $other) + public function isContentUnchanged() { - return $this->_parsedData != null && $other->_parsedData != null && - json_encode($this->_parsedData) === json_encode($other->_parsedData); + return CacheStatus::isContentUnchanged($this->_cacheStatus); } } diff --git a/src/Rox/Core/Network/ConfigurationFetcher.php b/src/Rox/Core/Network/ConfigurationFetcher.php index 3a2a805..52348f4 100644 --- a/src/Rox/Core/Network/ConfigurationFetcher.php +++ b/src/Rox/Core/Network/ConfigurationFetcher.php @@ -51,7 +51,7 @@ function fetch() if ($fetchResult->isSuccessfulStatusCode()) { $responseAsString = $fetchResult->getContent()->readAsString(); - $configurationFetchResult = $this->createConfigurationResult($responseAsString, $source, $fetchResult->isFromCache()); + $configurationFetchResult = $this->createConfigurationResult($responseAsString, $source, $fetchResult->getCacheStatus()); if ($configurationFetchResult == null || $configurationFetchResult->getParsedData() == null) { return null; diff --git a/src/Rox/Core/Network/ConfigurationFetcherBase.php b/src/Rox/Core/Network/ConfigurationFetcherBase.php index 927aec9..c6fe5ca 100644 --- a/src/Rox/Core/Network/ConfigurationFetcherBase.php +++ b/src/Rox/Core/Network/ConfigurationFetcherBase.php @@ -4,6 +4,7 @@ use Exception; use Psr\Log\LoggerInterface; +use Rox\Core\Network\CacheStatus; use Rox\Core\Client\BUIDInterface; use Rox\Core\Client\DevicePropertiesInterface; use Rox\Core\Configuration\ConfigurationFetchedInvokerInterface; @@ -79,11 +80,12 @@ public function __construct( /** * @param string $data * @param int $source - * @param bool $isFromCache + * @param string $cacheStatus * @return ConfigurationFetchResult|null * @see ConfigurationSource + * @see CacheStatus */ - protected function createConfigurationResult($data, $source, $isFromCache = false) + protected function createConfigurationResult($data, $source, $cacheStatus = CacheStatus::MISS) { if (!$data) { $this->_configurationFetchedInvoker->invokeWithError(FetcherError::EmptyJson); @@ -100,7 +102,7 @@ protected function createConfigurationResult($data, $source, $isFromCache = fals return null; } - return new ConfigurationFetchResult($decoded, $source, $isFromCache); + return new ConfigurationFetchResult($decoded, $source, $cacheStatus); } /** diff --git a/src/Rox/Core/Network/GuzzleHttpClient.php b/src/Rox/Core/Network/GuzzleHttpClient.php index c511149..a4f75bb 100644 --- a/src/Rox/Core/Network/GuzzleHttpClient.php +++ b/src/Rox/Core/Network/GuzzleHttpClient.php @@ -10,6 +10,7 @@ use GuzzleHttp\RequestOptions; use Psr\Log\LoggerInterface; use Rox\Core\Logging\LoggerFactory; +use Rox\Core\Network\CacheStatus; class GuzzleHttpClient implements HttpClientInterface { @@ -90,13 +91,17 @@ function sendGet(RequestData $requestData) $cacheState = $response->getHeader("X-Kevinrob-Cache"); if (is_array($cacheState) && !empty($cacheState)) { switch ($cacheState[0]) { - case 'HIT': + case CacheStatus::HIT: $this->_log->debug("{$request->getUri()}: HIT"); break; - case 'MISS': + case CacheStatus::MISS: $this->_log->debug("{$request->getUri()}: MISS"); break; + + case CacheStatus::REVALIDATED: + $this->_log->debug("{$request->getUri()}: REVALIDATED"); + break; } } } diff --git a/src/Rox/Core/Network/HttpResponseInterface.php b/src/Rox/Core/Network/HttpResponseInterface.php index 5d27c5e..c3cdd04 100644 --- a/src/Rox/Core/Network/HttpResponseInterface.php +++ b/src/Rox/Core/Network/HttpResponseInterface.php @@ -19,9 +19,10 @@ function getStatusCode(); function isSuccessfulStatusCode(); /** - * @return bool + * Returns the Guzzle cache state: 'HIT', 'MISS', or 'REVALIDATED'. + * @return string */ - function isFromCache(); + function getCacheStatus(); /** * @return HttpResponseContentInterface diff --git a/src/Rox/Core/Network/Psr7ResponseWrapper.php b/src/Rox/Core/Network/Psr7ResponseWrapper.php index 44427ce..00a53c4 100644 --- a/src/Rox/Core/Network/Psr7ResponseWrapper.php +++ b/src/Rox/Core/Network/Psr7ResponseWrapper.php @@ -3,6 +3,7 @@ namespace Rox\Core\Network; use Psr\Http\Message\ResponseInterface; +use Rox\Core\Network\CacheStatus; class Psr7ResponseWrapper extends AbstractHttpResponse { @@ -26,12 +27,15 @@ function getStatusCode() } /** - * @return bool + * @return string */ - function isFromCache() + function getCacheStatus() { $header = $this->_response->getHeader('X-Kevinrob-Cache'); - return !empty($header) && $header[0] === 'HIT'; + $value = !empty($header) ? $header[0] : CacheStatus::MISS; + return in_array($value, [CacheStatus::HIT, CacheStatus::REVALIDATED], true) + ? $value + : CacheStatus::MISS; } /** diff --git a/tests/Rox/Core/Network/ConfigurationFetcherTests.php b/tests/Rox/Core/Network/ConfigurationFetcherTests.php index 86039c5..d6d075f 100644 --- a/tests/Rox/Core/Network/ConfigurationFetcherTests.php +++ b/tests/Rox/Core/Network/ConfigurationFetcherTests.php @@ -318,7 +318,7 @@ public function testWillReturnIsFromCacheTrueWhenCDNResponseIsFromCache() { $confFetchInvoker = new ConfigurationFetchedInvoker(Mockery::mock(UserspaceUnhandledErrorInvokerInterface::class)); - $response = new TestHttpResponse(200, "{\"a\": \"cached\"}", true); + $response = new TestHttpResponse(200, "{\"a\": \"cached\"}", CacheStatus::HIT); $request = Mockery::mock(HttpClientInterface::class) ->shouldReceive('sendGet') @@ -352,6 +352,28 @@ public function testWillReturnIsFromCacheFalseWhenCDNResponseIsFromNetwork() $this->assertFalse($result->isFromCache()); } + public function testRevalidatedResponseIsNotFromCacheButContentIsUnchanged() + { + // REVALIDATED = server returned 304 Not Modified; content is identical to cached copy. + // isFromCache() must be false (we hit the network), but isContentUnchanged() must be + // true (no changes to apply) so hasChanges is correctly reported as false. + $confFetchInvoker = new ConfigurationFetchedInvoker(Mockery::mock(UserspaceUnhandledErrorInvokerInterface::class)); + + $response = new TestHttpResponse(200, "{\"a\": \"same\"}", CacheStatus::REVALIDATED); + + $request = Mockery::mock(HttpClientInterface::class) + ->shouldReceive('sendGet') + ->andReturn($response) + ->getMock(); + + $confFetcher = new ConfigurationFetcher($request, $this->_bu, $this->_dp, $confFetchInvoker, $this->_errorReporter, $this->_environment); + $result = $confFetcher->fetch(); + + $this->assertNotNull($result); + $this->assertFalse($result->isFromCache()); + $this->assertTrue($result->isContentUnchanged()); + } + public function testWillReturnNullDataWhenBothNotFound() { $confFetchInvoker = new ConfigurationFetchedInvoker(Mockery::mock(UserspaceUnhandledErrorInvokerInterface::class)); diff --git a/tests/Rox/Core/Network/TestHttpResponse.php b/tests/Rox/Core/Network/TestHttpResponse.php index fee7e15..18eb010 100644 --- a/tests/Rox/Core/Network/TestHttpResponse.php +++ b/tests/Rox/Core/Network/TestHttpResponse.php @@ -15,21 +15,22 @@ class TestHttpResponse extends AbstractHttpResponse private $_content; /** - * @var bool $_isFromCache + * @var string $_cacheStatus */ - private $_isFromCache; + private $_cacheStatus; /** * TestHttpResponse constructor. * @param int $status * @param string $content - * @param bool $isFromCache + * @param string $cacheStatus + * @see CacheStatus */ - public function __construct($status, $content = "", $isFromCache = false) + public function __construct($status, $content = "", $cacheStatus = CacheStatus::MISS) { $this->_status = $status; $this->_content = $content; - $this->_isFromCache = $isFromCache; + $this->_cacheStatus = $cacheStatus; } /** @@ -41,11 +42,11 @@ function getStatusCode() } /** - * @return bool + * @return string */ - function isFromCache() + function getCacheStatus() { - return $this->_isFromCache; + return $this->_cacheStatus; } /**