Skip to content

Commit 042071e

Browse files
typottertypotter
authored
fix: set cache timestamp only when changes are made (#47)
* fixes ff-3947: throwOnFailedInit param to suppress Exceptions on initialization * only set timestamp and eTag if data is modified * test that init exception can be suppressed
1 parent b2b9965 commit 042071e

File tree

6 files changed

+125
-25
lines changed

6 files changed

+125
-25
lines changed

‎.github/pull_request_template.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
[//]: # (Describe your changes in detail)
1010

1111
## How has this been documented?
12-
[//]: # (Please describe how you documented the developer impact of your changes; link to PRs or issues or explan why no documentation changes are required)
12+
[//]: # (Please describe how you documented the developer impact of your changes; link to PRs or issues or explain why no documentation changes are required)
1313

1414
## How has this been tested?
1515
[//]: # (Please describe in detail how you tested your changes)

‎src/Cache/DefaultCacheFactory.php‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,6 @@
99

1010
class DefaultCacheFactory
1111
{
12-
/**
13-
* @throws Exception
14-
*/
1512
public static function create(): CacheInterface
1613
{
1714
$psr6Cache = new FilesystemAdapter(

‎src/Config/ConfigurationLoader.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,11 +109,11 @@ function ($json) {
109109
if ($indexer->hasBandits()) {
110110
$this->fetchBanditsAsRequired($indexer);
111111
}
112-
}
113112

114-
// Store metadata for next time.
115-
$this->configurationStore->setMetadata(self::KEY_FLAG_TIMESTAMP, $this->millitime());
116-
$this->configurationStore->setMetadata(self::KEY_FLAG_ETAG, $response->ETag);
113+
// Store metadata for next time.
114+
$this->configurationStore->setMetadata(self::KEY_FLAG_TIMESTAMP, $this->millitime());
115+
$this->configurationStore->setMetadata(self::KEY_FLAG_ETAG, $response->ETag);
116+
}
117117
}
118118

119119
private function getCacheAgeInMillis(): int

‎src/EppoClient.php‎

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ public static function init(
8484
RequestFactoryInterface $requestFactory = null,
8585
?bool $isGracefulMode = true,
8686
?PollingOptions $pollingOptions = null,
87+
?bool $throwOnFailedInit = false,
8788
): EppoClient {
8889
// Get SDK metadata to pass as params in the http client.
8990
$sdkData = new SDKData();
@@ -93,11 +94,7 @@ public static function init(
9394
];
9495

9596
if (!$cache) {
96-
try {
97-
$cache = (new DefaultCacheFactory())->create();
98-
} catch (Exception $e) {
99-
throw EppoClientInitializationException::from($e);
100-
}
97+
$cache = (new DefaultCacheFactory())->create();
10198
}
10299

103100
$configStore = new ConfigurationStore($cache);
@@ -143,27 +140,33 @@ function () use ($configLoader) {
143140
}
144141
);
145142

146-
self::$instance = self::createAndInitClient($configLoader, $poller, $assignmentLogger, $isGracefulMode);
143+
self::$instance = self::createAndInitClient($configLoader, $poller, $assignmentLogger, $isGracefulMode, throwOnFailedInit: $throwOnFailedInit);
147144

148145
return self::$instance;
149146
}
150147

151148
/**
152-
* @throws EppoClientInitializationException
149+
* @throws EppoClientInitializationException|InvalidConfigurationException
153150
*/
154151
private static function createAndInitClient(
155152
ConfigurationLoader $configLoader,
156153
PollerInterface $poller,
157154
?LoggerInterface $assignmentLogger,
158155
?bool $isGracefulMode,
159-
?IBanditEvaluator $banditEvaluator = null
156+
?IBanditEvaluator $banditEvaluator = null,
157+
?bool $throwOnFailedInit = false,
160158
): EppoClient {
161159
try {
162160
$configLoader->reloadConfigurationIfExpired();
163161
} catch (HttpRequestException | InvalidApiKeyException $e) {
164-
throw new EppoClientInitializationException(
165-
'Unable to initialize Eppo Client: ' . $e->getMessage()
166-
);
162+
$message = 'Unable to initialize Eppo Client: ' . $e->getMessage();
163+
if ($throwOnFailedInit) {
164+
throw new EppoClientInitializationException(
165+
$message
166+
);
167+
} else {
168+
syslog(LOG_INFO, "[Eppo SDK] " . $message);
169+
}
167170
}
168171
return new self($configLoader, $poller, $assignmentLogger, $isGracefulMode, $banditEvaluator);
169172
}
@@ -641,8 +644,16 @@ public static function createTestClient(
641644
PollerInterface $poller,
642645
?LoggerInterface $logger = null,
643646
?bool $isGracefulMode = false,
644-
?IBanditEvaluator $banditEvaluator = null
647+
?IBanditEvaluator $banditEvaluator = null,
648+
?bool $throwOnFailedInit = true,
645649
): EppoClient {
646-
return self::createAndInitClient($configurationLoader, $poller, $logger, $isGracefulMode, $banditEvaluator);
650+
return self::createAndInitClient(
651+
$configurationLoader,
652+
$poller,
653+
$logger,
654+
$isGracefulMode,
655+
$banditEvaluator,
656+
throwOnFailedInit: $throwOnFailedInit
657+
);
647658
}
648659
}

‎tests/Config/ConfigurationLoaderTest.php‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,58 @@ public function testLoadsConfiguration(): void
8989
$this->assertEquals('cold_start_bandit', $bandit->banditKey);
9090
}
9191

92+
93+
public function testSetsConfigurationTimestamp(): void
94+
{
95+
// Load mock response data
96+
$flagsRaw = file_get_contents(self::MOCK_RESPONSE_FILENAME);
97+
$flagsResourceResponse = new APIResource(
98+
$flagsRaw,
99+
true,
100+
"ETAG"
101+
);
102+
$banditsRaw = '{"bandits": {}}';
103+
104+
$apiWrapper = $this->getMockBuilder(APIRequestWrapper::class)->setConstructorArgs(
105+
['', [], new Psr18Client(), new Psr17Factory()]
106+
)->getMock();
107+
108+
$apiWrapper->expects($this->exactly(2))
109+
->method('getUFC')
110+
->willReturnCallback(
111+
function (?string $eTag) use ($flagsResourceResponse, $flagsRaw) {
112+
// Return not modified if the etag sent is not null.
113+
return $eTag == null ? $flagsResourceResponse : new APIResource(
114+
$flagsRaw,
115+
false,
116+
"ETAG"
117+
);
118+
}
119+
);
120+
121+
$apiWrapper->expects($this->once())
122+
->method('getBandits')
123+
->willReturn(new APIResource($banditsRaw, true, null));
124+
125+
$configStore = new ConfigurationStore(DefaultCacheFactory::create());
126+
127+
$loader = new ConfigurationLoader($apiWrapper, $configStore);
128+
$loader->fetchAndStoreConfigurations(null);
129+
130+
$timestamp1 = $configStore->getMetadata("flagTimestamp");
131+
$storedEtag = $configStore->getMetadata("flagETag");
132+
$this->assertEquals("ETAG", $storedEtag);
133+
134+
usleep(50 * 1000); // Sleep long enough for cache to expire.
135+
136+
$loader->fetchAndStoreConfigurations("ETAG");
137+
138+
$this->assertEquals("ETAG", $configStore->getMetadata("flagETag"));
139+
140+
// The timestamp should not have changed; the config did not change, so the timestamp should not be updated.
141+
$this->assertEquals($timestamp1, $configStore->getMetadata("flagTimestamp"));
142+
}
143+
92144
public function testLoadsOnGet(): void
93145
{
94146
// Arrange: Load some flag data to be returned by the APIRequestWrapper

‎tests/EppoClientTest.php‎

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,38 @@ public function testGracefulModeThrowsOnInit()
160160
);
161161
}
162162

163+
public function testSuppressInitExceptionThrow()
164+
{
165+
$pollerMock = $this->getPollerMock();
166+
167+
$apiRequestWrapper = $this->getMockBuilder(APIRequestWrapper::class)->setConstructorArgs(
168+
['', [], new Psr18Client(), new Psr17Factory()]
169+
)->getMock();
170+
171+
$apiRequestWrapper->expects($this->any())
172+
->method('getUFC')
173+
->willThrowException(new HttpRequestException());
174+
175+
$configStore = $this->getMockBuilder(IConfigurationStore::class)->getMock();
176+
$configStore->expects($this->any())->method('getMetadata')->willReturn(null);
177+
$mockLogger = $this->getMockBuilder(LoggerInterface::class)->getMock();
178+
179+
$client = EppoClient::createTestClient(
180+
new ConfigurationLoader($apiRequestWrapper, $configStore),
181+
$pollerMock,
182+
$mockLogger,
183+
true,
184+
throwOnFailedInit: false
185+
);
186+
187+
$this->assertEquals(
188+
'default',
189+
$client->getStringAssignment(self::EXPERIMENT_NAME, 'subject-10', [], 'default')
190+
);
191+
192+
// No exceptions thrown, default assignments.
193+
}
194+
163195
public function testReturnsDefaultWhenExperimentConfigIsAbsent()
164196
{
165197
$configLoaderMock = $this->getFlagConfigurationLoaderMock([]);
@@ -179,7 +211,12 @@ public function testReturnsDefaultWhenExperimentConfigIsAbsent()
179211
public function testRepoTestCases(): void
180212
{
181213
try {
182-
$client = EppoClient::init('dummy', self::$mockServer->serverAddress, isGracefulMode: false);
214+
$client = EppoClient::init(
215+
'dummy',
216+
self::$mockServer->serverAddress,
217+
isGracefulMode: false,
218+
throwOnFailedInit: true
219+
);
183220
} catch (Exception $exception) {
184221
self::fail('Failed to initialize EppoClient: ' . $exception->getMessage());
185222
}
@@ -315,7 +352,9 @@ public function testInitWithPollingOptions(): void
315352
);
316353

317354
$response = new Response(stream: Utils::streamFor(file_get_contents(__DIR__ . '/data/ufc/flags-v1.json')));
318-
$secondResponse = new Response(stream: Utils::streamFor(file_get_contents(__DIR__ . '/data/ufc/bandit-flags-v1.json')));
355+
$secondResponse = new Response(stream: Utils::streamFor(
356+
file_get_contents(__DIR__ . '/data/ufc/bandit-flags-v1.json')
357+
));
319358

320359
$httpClient = $this->createMock(ClientInterface::class);
321360
$httpClient->expects($this->atLeast(2))
@@ -327,15 +366,16 @@ public function testInitWithPollingOptions(): void
327366
"fake address",
328367
httpClient: $httpClient,
329368
isGracefulMode: false,
330-
pollingOptions: $pollingOptions
369+
pollingOptions: $pollingOptions,
370+
throwOnFailedInit: true
331371
);
332372

333373
$this->assertEquals(
334374
3.1415926,
335375
$client->getNumericAssignment(self::EXPERIMENT_NAME, 'subject-10', [], 0)
336376
);
337377
// Wait a little bit for the cache to age out and the mock server to spin up.
338-
usleep(75*1000);
378+
usleep(75 * 1000);
339379

340380
$this->assertEquals(
341381
0,

0 commit comments

Comments
 (0)