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 diff --git a/src/Rox/Core/Core.php b/src/Rox/Core/Core.php index 10de668..b31fbd2 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 = !$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; } /**