diff --git a/src/Rox/Core/Core.php b/src/Rox/Core/Core.php index 9c6e77d..10de668 100644 --- a/src/Rox/Core/Core.php +++ b/src/Rox/Core/Core.php @@ -350,10 +350,13 @@ public function fetch($isSourcePushing = false) $this->_targetGroupRepository->setTargetGroups($configuration->getTargetGroups()); $this->_flagSetter->setExperiments(); - $hasChanges = ($this->_lastConfigurations == null || $this->_lastConfigurations->equals($result)); + $hasChanges = ($this->_lastConfigurations == null || !$this->_lastConfigurations->equals($result)); $this->_lastConfigurations = $result; + $fetcherStatus = $result->isFromCache() + ? FetcherStatus::AppliedFromLocalStorage + : FetcherStatus::AppliedFromNetwork; $this->_configurationFetchedInvoker->invoke( - FetcherStatus::AppliedFromNetwork, + $fetcherStatus, $configuration->getSignatureDate(), $hasChanges ); diff --git a/src/Rox/Core/Network/AbstractHttpResponse.php b/src/Rox/Core/Network/AbstractHttpResponse.php index 4a767f0..8413659 100644 --- a/src/Rox/Core/Network/AbstractHttpResponse.php +++ b/src/Rox/Core/Network/AbstractHttpResponse.php @@ -12,4 +12,12 @@ final function isSuccessfulStatusCode() return $this->getStatusCode() >= 200 && $this->getStatusCode() <= 299; } + + /** + * @return bool + */ + function isFromCache() + { + return false; + } } diff --git a/src/Rox/Core/Network/CdnCacheStrategy.php b/src/Rox/Core/Network/CdnCacheStrategy.php index 055825b..b8a0841 100644 --- a/src/Rox/Core/Network/CdnCacheStrategy.php +++ b/src/Rox/Core/Network/CdnCacheStrategy.php @@ -2,6 +2,7 @@ namespace Rox\Core\Network; +use Kevinrob\GuzzleCache\KeyValueHttpHeader; use Kevinrob\GuzzleCache\Strategy\GreedyCacheStrategy; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; @@ -11,6 +12,14 @@ class CdnCacheStrategy extends GreedyCacheStrategy { + protected function getCacheKey(RequestInterface $request, KeyValueHttpHeader $varyHeaders = null) + { + // Key on path only so that per-instance query params (e.g. distinct_id) + // don't cause cache misses across containers/pods. + $request = $request->withUri($request->getUri()->withQuery('')); + return parent::getCacheKey($request, $varyHeaders); + } + protected function getCacheObject(RequestInterface $request, ResponseInterface $response) { if ($response->getStatusCode() == 200) { diff --git a/src/Rox/Core/Network/ConfigurationFetchResult.php b/src/Rox/Core/Network/ConfigurationFetchResult.php index 5ec1309..6e0a43f 100644 --- a/src/Rox/Core/Network/ConfigurationFetchResult.php +++ b/src/Rox/Core/Network/ConfigurationFetchResult.php @@ -15,15 +15,22 @@ class ConfigurationFetchResult */ private $_parsedData; + /** + * @var bool $_isFromCache + */ + private $_isFromCache; + /** * ConfigurationFetchResult constructor. * @param array $parsedData * @param int $source + * @param bool $isFromCache */ - public function __construct($parsedData, $source) + public function __construct($parsedData, $source, $isFromCache = false) { $this->_source = $source; $this->_parsedData = $parsedData; + $this->_isFromCache = $isFromCache; } /** @@ -42,6 +49,14 @@ public function getParsedData() return $this->_parsedData; } + /** + * @return bool + */ + public function isFromCache() + { + return $this->_isFromCache; + } + /** * @param ConfigurationFetchResult $other * @return bool diff --git a/src/Rox/Core/Network/ConfigurationFetcher.php b/src/Rox/Core/Network/ConfigurationFetcher.php index d6a8f62..3a2a805 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); + $configurationFetchResult = $this->createConfigurationResult($responseAsString, $source, $fetchResult->isFromCache()); 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 30ff83e..927aec9 100644 --- a/src/Rox/Core/Network/ConfigurationFetcherBase.php +++ b/src/Rox/Core/Network/ConfigurationFetcherBase.php @@ -79,10 +79,11 @@ public function __construct( /** * @param string $data * @param int $source + * @param bool $isFromCache * @return ConfigurationFetchResult|null * @see ConfigurationSource */ - protected function createConfigurationResult($data, $source) + protected function createConfigurationResult($data, $source, $isFromCache = false) { if (!$data) { $this->_configurationFetchedInvoker->invokeWithError(FetcherError::EmptyJson); @@ -99,7 +100,7 @@ protected function createConfigurationResult($data, $source) return null; } - return new ConfigurationFetchResult($decoded, $source); + return new ConfigurationFetchResult($decoded, $source, $isFromCache); } /** diff --git a/src/Rox/Core/Network/HttpResponseInterface.php b/src/Rox/Core/Network/HttpResponseInterface.php index 272ccd7..5d27c5e 100644 --- a/src/Rox/Core/Network/HttpResponseInterface.php +++ b/src/Rox/Core/Network/HttpResponseInterface.php @@ -18,6 +18,11 @@ function getStatusCode(); */ function isSuccessfulStatusCode(); + /** + * @return bool + */ + function isFromCache(); + /** * @return HttpResponseContentInterface */ diff --git a/src/Rox/Core/Network/Psr7ResponseWrapper.php b/src/Rox/Core/Network/Psr7ResponseWrapper.php index 4e22f24..44427ce 100644 --- a/src/Rox/Core/Network/Psr7ResponseWrapper.php +++ b/src/Rox/Core/Network/Psr7ResponseWrapper.php @@ -25,6 +25,15 @@ function getStatusCode() return $this->_response->getStatusCode(); } + /** + * @return bool + */ + function isFromCache() + { + $header = $this->_response->getHeader('X-Kevinrob-Cache'); + return !empty($header) && $header[0] === 'HIT'; + } + /** * @return HttpResponseContentInterface */ diff --git a/tests/Rox/Core/Network/ConfigurationFetcherTests.php b/tests/Rox/Core/Network/ConfigurationFetcherTests.php index ecb16e9..86039c5 100644 --- a/tests/Rox/Core/Network/ConfigurationFetcherTests.php +++ b/tests/Rox/Core/Network/ConfigurationFetcherTests.php @@ -314,6 +314,44 @@ public function testWillReturnAPIDataWhenCDNFails404APIOK() $this->assertEquals(0, $numberOfTimerCalled[0]); } + public function testWillReturnIsFromCacheTrueWhenCDNResponseIsFromCache() + { + $confFetchInvoker = new ConfigurationFetchedInvoker(Mockery::mock(UserspaceUnhandledErrorInvokerInterface::class)); + + $response = new TestHttpResponse(200, "{\"a\": \"cached\"}", true); + + $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->assertEquals("cached", $result->getParsedData()["a"]); + $this->assertTrue($result->isFromCache()); + } + + public function testWillReturnIsFromCacheFalseWhenCDNResponseIsFromNetwork() + { + $confFetchInvoker = new ConfigurationFetchedInvoker(Mockery::mock(UserspaceUnhandledErrorInvokerInterface::class)); + + $response = new TestHttpResponse(200, "{\"a\": \"fresh\"}"); + + $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->assertEquals("fresh", $result->getParsedData()["a"]); + $this->assertFalse($result->isFromCache()); + } + 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 1736ccf..fee7e15 100644 --- a/tests/Rox/Core/Network/TestHttpResponse.php +++ b/tests/Rox/Core/Network/TestHttpResponse.php @@ -14,15 +14,22 @@ class TestHttpResponse extends AbstractHttpResponse */ private $_content; + /** + * @var bool $_isFromCache + */ + private $_isFromCache; + /** * TestHttpResponse constructor. * @param int $status * @param string $content + * @param bool $isFromCache */ - public function __construct($status, $content = "") + public function __construct($status, $content = "", $isFromCache = false) { $this->_status = $status; $this->_content = $content; + $this->_isFromCache = $isFromCache; } /** @@ -33,6 +40,14 @@ function getStatusCode() return $this->_status; } + /** + * @return bool + */ + function isFromCache() + { + return $this->_isFromCache; + } + /** * @return HttpResponseContentInterface */ diff --git a/tests/Rox/E2E/RoxE2ETests.php b/tests/Rox/E2E/RoxE2ETests.php index c919c82..8ea40c3 100644 --- a/tests/Rox/E2E/RoxE2ETests.php +++ b/tests/Rox/E2E/RoxE2ETests.php @@ -32,7 +32,8 @@ public static function setUpBeforeClass(): void $options = new RoxOptions((new RoxOptionsBuilder()) ->setConfigurationFetchedHandler(function (ConfigurationFetchedArgs $args) { - if ($args->getFetcherStatus() == FetcherStatus::AppliedFromNetwork) { + if ($args->getFetcherStatus() == FetcherStatus::AppliedFromNetwork || + $args->getFetcherStatus() == FetcherStatus::AppliedFromLocalStorage) { TestVars::$configurationFetchedCount++; } }) diff --git a/tests/Rox/Server/Client/ServerPropertiesTests.php b/tests/Rox/Server/Client/ServerPropertiesTests.php new file mode 100644 index 0000000..d46d64c --- /dev/null +++ b/tests/Rox/Server/Client/ServerPropertiesTests.php @@ -0,0 +1,39 @@ +assertEquals('my-instance-001', $props->getDistinctId()); + putenv(ServerProperties::INSTANCE_ID_ENV_VAR_NAME); + } + + public function testDistinctIdFallsBackToScriptPropertiesHashWhenEnvVarNotSet() + { + putenv(ServerProperties::INSTANCE_ID_ENV_VAR_NAME); + $props = new ServerProperties( + new SdkSettings('test-api-key', ''), + new RoxOptions((new RoxOptionsBuilder())) + ); + $expected = md5(join('.', [ + getmyuid(), + getmygid(), + get_current_user(), + getmyinode(), + getlastmod() + ])); + $this->assertEquals($expected, $props->getDistinctId()); + } +}