From c3862a5248f834806e34fd7035cce1aa05169554 Mon Sep 17 00:00:00 2001 From: Ankur Date: Fri, 19 Jun 2026 15:25:26 +0530 Subject: [PATCH 1/4] fix(CBP-51066): fix AppliedFromLocalStorage never firing and unstable distinctId causing excessive CDN requests --- src/Rox/Core/Core.php | 5 ++- src/Rox/Core/Network/AbstractHttpResponse.php | 8 ++++ .../Core/Network/ConfigurationFetchResult.php | 17 +++++++- src/Rox/Core/Network/ConfigurationFetcher.php | 2 +- .../Core/Network/ConfigurationFetcherBase.php | 5 ++- .../Core/Network/HttpResponseInterface.php | 5 +++ src/Rox/Core/Network/Psr7ResponseWrapper.php | 9 +++++ src/Rox/Server/Client/ServerProperties.php | 8 +--- .../Network/ConfigurationFetcherTests.php | 40 +++++++++++++++++++ tests/Rox/Core/Network/TestHttpResponse.php | 17 +++++++- tests/Rox/E2E/RoxE2ETests.php | 3 +- 11 files changed, 105 insertions(+), 14 deletions(-) diff --git a/src/Rox/Core/Core.php b/src/Rox/Core/Core.php index 9c6e77d..10cec95 100644 --- a/src/Rox/Core/Core.php +++ b/src/Rox/Core/Core.php @@ -352,8 +352,11 @@ public function fetch($isSourcePushing = false) $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/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/src/Rox/Server/Client/ServerProperties.php b/src/Rox/Server/Client/ServerProperties.php index addda7c..522c568 100644 --- a/src/Rox/Server/Client/ServerProperties.php +++ b/src/Rox/Server/Client/ServerProperties.php @@ -24,13 +24,7 @@ public function __construct( { parent::__construct($sdkSettings, $roxOptions); $this->_distinctId = getenv(self::INSTANCE_ID_ENV_VAR_NAME) - ?: md5(join('.', [ - getmyuid(), - getmygid(), - get_current_user(), - getmyinode(), - getlastmod() - ])); + ?: md5($sdkSettings->getApiKey()); } function getLibVersion() diff --git a/tests/Rox/Core/Network/ConfigurationFetcherTests.php b/tests/Rox/Core/Network/ConfigurationFetcherTests.php index ecb16e9..22ea929 100644 --- a/tests/Rox/Core/Network/ConfigurationFetcherTests.php +++ b/tests/Rox/Core/Network/ConfigurationFetcherTests.php @@ -314,6 +314,46 @@ public function testWillReturnAPIDataWhenCDNFails404APIOK() $this->assertEquals(0, $numberOfTimerCalled[0]); } + public function testWillReturnIsFromCacheTrueWhenCDNResponseIsFromCache() + { + $confFetchInvoker = new ConfigurationFetchedInvoker(Mockery::mock(UserspaceUnhandledErrorInvokerInterface::class)); + + // Simulate Guzzle cache middleware returning a HIT response (isFromCache = true) + $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)); + + // Simulate a real network response (isFromCache = false, the default) + $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++; } }) From 60618a4e6b9d8b4492e99b8a999cd3fe5ca86ac9 Mon Sep 17 00:00:00 2001 From: Ankur Date: Mon, 22 Jun 2026 19:40:22 +0530 Subject: [PATCH 2/4] remove distinctId from CDN query params and fix hasChanges inversion --- src/Rox/Core/Core.php | 2 +- src/Rox/Core/Network/ConfigurationFetcher.php | 2 - src/Rox/Server/Client/ServerProperties.php | 8 +++- .../Network/ConfigurationFetcherTests.php | 6 +-- .../Server/Client/ServerPropertiesTests.php | 39 +++++++++++++++++++ 5 files changed, 49 insertions(+), 8 deletions(-) create mode 100644 tests/Rox/Server/Client/ServerPropertiesTests.php diff --git a/src/Rox/Core/Core.php b/src/Rox/Core/Core.php index 10cec95..10de668 100644 --- a/src/Rox/Core/Core.php +++ b/src/Rox/Core/Core.php @@ -350,7 +350,7 @@ 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 diff --git a/src/Rox/Core/Network/ConfigurationFetcher.php b/src/Rox/Core/Network/ConfigurationFetcher.php index 3a2a805..ef1b0bf 100644 --- a/src/Rox/Core/Network/ConfigurationFetcher.php +++ b/src/Rox/Core/Network/ConfigurationFetcher.php @@ -23,8 +23,6 @@ private function _getCDNUrl(array $properties) private function _fetchFromCDN(array $properties) { return $this->_request->sendGet(new RequestData($this->_getCDNUrl($properties), [ - PropertyType::getDistinctId()->getName() => - (string)$properties[PropertyType::getDistinctId()->getName()], 'realPlatform' => (string)@$properties['platform'], 'sdkVersion' => diff --git a/src/Rox/Server/Client/ServerProperties.php b/src/Rox/Server/Client/ServerProperties.php index 522c568..addda7c 100644 --- a/src/Rox/Server/Client/ServerProperties.php +++ b/src/Rox/Server/Client/ServerProperties.php @@ -24,7 +24,13 @@ public function __construct( { parent::__construct($sdkSettings, $roxOptions); $this->_distinctId = getenv(self::INSTANCE_ID_ENV_VAR_NAME) - ?: md5($sdkSettings->getApiKey()); + ?: md5(join('.', [ + getmyuid(), + getmygid(), + get_current_user(), + getmyinode(), + getlastmod() + ])); } function getLibVersion() diff --git a/tests/Rox/Core/Network/ConfigurationFetcherTests.php b/tests/Rox/Core/Network/ConfigurationFetcherTests.php index 22ea929..61d0522 100644 --- a/tests/Rox/Core/Network/ConfigurationFetcherTests.php +++ b/tests/Rox/Core/Network/ConfigurationFetcherTests.php @@ -92,8 +92,8 @@ public function testWillReturnCDNDataWhenSuccessful() $this->assertEquals($reqData[0]->getUrl(), "https://conf.rollout.io/123/buid"); $qp = $reqData[0]->getQueryParams(); - $this->assertEquals(count($qp), 5); - $this->assertEquals($qp[PropertyType::getDistinctId()->getName()], "id"); + $this->assertEquals(count($qp), 4); + $this->assertArrayNotHasKey(PropertyType::getDistinctId()->getName(), $qp); $this->assertEquals($qp['realPlatform'], "PHP-test"); $this->assertEquals($qp['sdkVersion'], "1.2.3"); $this->assertEquals($qp['platformVersion'], php_sapi_name()); @@ -318,7 +318,6 @@ public function testWillReturnIsFromCacheTrueWhenCDNResponseIsFromCache() { $confFetchInvoker = new ConfigurationFetchedInvoker(Mockery::mock(UserspaceUnhandledErrorInvokerInterface::class)); - // Simulate Guzzle cache middleware returning a HIT response (isFromCache = true) $response = new TestHttpResponse(200, "{\"a\": \"cached\"}", true); $request = Mockery::mock(HttpClientInterface::class) @@ -338,7 +337,6 @@ public function testWillReturnIsFromCacheFalseWhenCDNResponseIsFromNetwork() { $confFetchInvoker = new ConfigurationFetchedInvoker(Mockery::mock(UserspaceUnhandledErrorInvokerInterface::class)); - // Simulate a real network response (isFromCache = false, the default) $response = new TestHttpResponse(200, "{\"a\": \"fresh\"}"); $request = Mockery::mock(HttpClientInterface::class) 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()); + } +} From 82e5cc830a572f522ccaf6fa718c95c284257b3c Mon Sep 17 00:00:00 2001 From: Ankur Date: Tue, 23 Jun 2026 16:23:19 +0530 Subject: [PATCH 3/4] exclude distinctId from CDN cache key instead of removing it from the request --- src/Rox/Core/Network/CdnCacheStrategy.php | 13 +++++++++++++ src/Rox/Core/Network/ConfigurationFetcher.php | 2 ++ .../Rox/Core/Network/ConfigurationFetcherTests.php | 4 ++-- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/Rox/Core/Network/CdnCacheStrategy.php b/src/Rox/Core/Network/CdnCacheStrategy.php index 055825b..5bd3a6e 100644 --- a/src/Rox/Core/Network/CdnCacheStrategy.php +++ b/src/Rox/Core/Network/CdnCacheStrategy.php @@ -2,15 +2,28 @@ namespace Rox\Core\Network; +use GuzzleHttp\Psr7\Uri; +use Kevinrob\GuzzleCache\KeyValueHttpHeader; use Kevinrob\GuzzleCache\Strategy\GreedyCacheStrategy; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; +use Rox\Core\Consts\PropertyType; // NOTE: Using GreedyCacheStrategy as a base class here because // our CDN prevents caching by using Cache Control headers. class CdnCacheStrategy extends GreedyCacheStrategy { + protected function getCacheKey(RequestInterface $request, KeyValueHttpHeader $varyHeaders = null) + { + // Strip distinct_id from the URI before computing the cache key so that + // per-instance identity doesn't cause cache misses across containers/pods. + // The param is still sent in the actual HTTP request. + $uri = Uri::withoutQueryValue($request->getUri(), PropertyType::getDistinctId()->getName()); + $request = $request->withUri($uri); + return parent::getCacheKey($request, $varyHeaders); + } + protected function getCacheObject(RequestInterface $request, ResponseInterface $response) { if ($response->getStatusCode() == 200) { diff --git a/src/Rox/Core/Network/ConfigurationFetcher.php b/src/Rox/Core/Network/ConfigurationFetcher.php index ef1b0bf..3a2a805 100644 --- a/src/Rox/Core/Network/ConfigurationFetcher.php +++ b/src/Rox/Core/Network/ConfigurationFetcher.php @@ -23,6 +23,8 @@ private function _getCDNUrl(array $properties) private function _fetchFromCDN(array $properties) { return $this->_request->sendGet(new RequestData($this->_getCDNUrl($properties), [ + PropertyType::getDistinctId()->getName() => + (string)$properties[PropertyType::getDistinctId()->getName()], 'realPlatform' => (string)@$properties['platform'], 'sdkVersion' => diff --git a/tests/Rox/Core/Network/ConfigurationFetcherTests.php b/tests/Rox/Core/Network/ConfigurationFetcherTests.php index 61d0522..86039c5 100644 --- a/tests/Rox/Core/Network/ConfigurationFetcherTests.php +++ b/tests/Rox/Core/Network/ConfigurationFetcherTests.php @@ -92,8 +92,8 @@ public function testWillReturnCDNDataWhenSuccessful() $this->assertEquals($reqData[0]->getUrl(), "https://conf.rollout.io/123/buid"); $qp = $reqData[0]->getQueryParams(); - $this->assertEquals(count($qp), 4); - $this->assertArrayNotHasKey(PropertyType::getDistinctId()->getName(), $qp); + $this->assertEquals(count($qp), 5); + $this->assertEquals($qp[PropertyType::getDistinctId()->getName()], "id"); $this->assertEquals($qp['realPlatform'], "PHP-test"); $this->assertEquals($qp['sdkVersion'], "1.2.3"); $this->assertEquals($qp['platformVersion'], php_sapi_name()); From 997a3d4772f01aafad4b815c9c2ed0612ea241de Mon Sep 17 00:00:00 2001 From: Ankur Date: Wed, 24 Jun 2026 11:00:00 +0530 Subject: [PATCH 4/4] refactor:key CDN cache on path only, stripping all query params --- src/Rox/Core/Network/CdnCacheStrategy.php | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/Rox/Core/Network/CdnCacheStrategy.php b/src/Rox/Core/Network/CdnCacheStrategy.php index 5bd3a6e..b8a0841 100644 --- a/src/Rox/Core/Network/CdnCacheStrategy.php +++ b/src/Rox/Core/Network/CdnCacheStrategy.php @@ -2,12 +2,10 @@ namespace Rox\Core\Network; -use GuzzleHttp\Psr7\Uri; use Kevinrob\GuzzleCache\KeyValueHttpHeader; use Kevinrob\GuzzleCache\Strategy\GreedyCacheStrategy; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; -use Rox\Core\Consts\PropertyType; // NOTE: Using GreedyCacheStrategy as a base class here because // our CDN prevents caching by using Cache Control headers. @@ -16,11 +14,9 @@ class CdnCacheStrategy extends GreedyCacheStrategy { protected function getCacheKey(RequestInterface $request, KeyValueHttpHeader $varyHeaders = null) { - // Strip distinct_id from the URI before computing the cache key so that - // per-instance identity doesn't cause cache misses across containers/pods. - // The param is still sent in the actual HTTP request. - $uri = Uri::withoutQueryValue($request->getUri(), PropertyType::getDistinctId()->getName()); - $request = $request->withUri($uri); + // 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); }