fix: set cache timestamp only when changes are made

.github/pull_request_template.md

@@ -9,7 +9,7 @@
 [//]: # (Describe your changes in detail)
 
 ## How has this been documented?
-[//]: # (Please describe how you documented the developer impact of your changes; link to PRs or issues or explan why no documentation changes are required)
+[//]: # (Please describe how you documented the developer impact of your changes; link to PRs or issues or explain why no documentation changes are required)
 
 ## How has this been tested?
 [//]: # (Please describe in detail how you tested your changes)

src/Cache/DefaultCacheFactory.php

@@ -9,9 +9,6 @@
 
 class DefaultCacheFactory
 {
-    /**
-     * @throws Exception
-     */
     public static function create(): CacheInterface
     {
         $psr6Cache = new FilesystemAdapter(

src/Config/ConfigurationLoader.php

@@ -109,11 +109,11 @@ function ($json) {
             if ($indexer->hasBandits()) {
                 $this->fetchBanditsAsRequired($indexer);
             }
-        }
 
-        // Store metadata for next time.
-        $this->configurationStore->setMetadata(self::KEY_FLAG_TIMESTAMP, $this->millitime());
-        $this->configurationStore->setMetadata(self::KEY_FLAG_ETAG, $response->ETag);
+            // Store metadata for next time.
+            $this->configurationStore->setMetadata(self::KEY_FLAG_TIMESTAMP, $this->millitime());
+            $this->configurationStore->setMetadata(self::KEY_FLAG_ETAG, $response->ETag);
+        }
     }
 
     private function getCacheAgeInMillis(): int

src/EppoClient.php

@@ -84,6 +84,7 @@ public static function init(
         RequestFactoryInterface $requestFactory = null,
         ?bool $isGracefulMode = true,
         ?PollingOptions $pollingOptions = null,
+        ?bool $throwOnFailedInit = false,
     ): EppoClient {
         // Get SDK metadata to pass as params in the http client.
         $sdkData = new SDKData();
@@ -93,11 +94,7 @@ public static function init(
         ];
 
         if (!$cache) {
-            try {
-                $cache = (new DefaultCacheFactory())->create();
-            } catch (Exception $e) {
-                throw EppoClientInitializationException::from($e);
-            }
+            $cache = (new DefaultCacheFactory())->create();
         }
 
         $configStore = new ConfigurationStore($cache);
@@ -143,27 +140,33 @@ function () use ($configLoader) {
             }
         );
 
-        self::$instance = self::createAndInitClient($configLoader, $poller, $assignmentLogger, $isGracefulMode);
+        self::$instance = self::createAndInitClient($configLoader, $poller, $assignmentLogger, $isGracefulMode, throwOnFailedInit: $throwOnFailedInit);
 
         return self::$instance;
     }
 
     /**
-     * @throws EppoClientInitializationException
+     * @throws EppoClientInitializationException|InvalidConfigurationException
      */
     private static function createAndInitClient(
         ConfigurationLoader $configLoader,
         PollerInterface $poller,
         ?LoggerInterface $assignmentLogger,
         ?bool $isGracefulMode,
-        ?IBanditEvaluator $banditEvaluator = null
+        ?IBanditEvaluator $banditEvaluator = null,
+        ?bool $throwOnFailedInit = false,
     ): EppoClient {
         try {
             $configLoader->reloadConfigurationIfExpired();
         } catch (HttpRequestException | InvalidApiKeyException $e) {
-            throw new EppoClientInitializationException(
-                'Unable to initialize Eppo Client: ' . $e->getMessage()
-            );
+            $message = 'Unable to initialize Eppo Client: ' . $e->getMessage();
+            if ($throwOnFailedInit) {
+                throw new EppoClientInitializationException(
+                    $message
+                );
+            } else {
+                syslog(LOG_INFO, "[Eppo SDK] " . $message);
+            }
         }
         return new self($configLoader, $poller, $assignmentLogger, $isGracefulMode, $banditEvaluator);
     }
@@ -641,8 +644,16 @@ public static function createTestClient(
         PollerInterface $poller,
         ?LoggerInterface $logger = null,
         ?bool $isGracefulMode = false,
-        ?IBanditEvaluator $banditEvaluator = null
+        ?IBanditEvaluator $banditEvaluator = null,
+        ?bool $throwOnFailedInit = true,
     ): EppoClient {
-        return self::createAndInitClient($configurationLoader, $poller, $logger, $isGracefulMode, $banditEvaluator);
+        return self::createAndInitClient(
+            $configurationLoader,
+            $poller,
+            $logger,
+            $isGracefulMode,
+            $banditEvaluator,
+            throwOnFailedInit: $throwOnFailedInit
+        );
     }
 }

tests/Config/ConfigurationLoaderTest.php

@@ -89,6 +89,58 @@ public function testLoadsConfiguration(): void
         $this->assertEquals('cold_start_bandit', $bandit->banditKey);
     }
 
+
+    public function testSetsConfigurationTimestamp(): void
+    {
+        // Load mock response data
+        $flagsRaw = file_get_contents(self::MOCK_RESPONSE_FILENAME);
+        $flagsResourceResponse = new APIResource(
+            $flagsRaw,
+            true,
+            "ETAG"
+        );
+        $banditsRaw = '{"bandits": {}}';
+
+        $apiWrapper = $this->getMockBuilder(APIRequestWrapper::class)->setConstructorArgs(
+            ['', [], new Psr18Client(), new Psr17Factory()]
+        )->getMock();
+
+        $apiWrapper->expects($this->exactly(2))
+            ->method('getUFC')
+            ->willReturnCallback(
+                function (?string $eTag) use ($flagsResourceResponse, $flagsRaw) {
+                    // Return not modified if the etag sent is not null.
+                    return $eTag == null ? $flagsResourceResponse : new APIResource(
+                        $flagsRaw,
+                        false,
+                        "ETAG"
+                    );
+                }
+            );
+
+        $apiWrapper->expects($this->once())
+            ->method('getBandits')
+            ->willReturn(new APIResource($banditsRaw, true, null));
+
+        $configStore = new ConfigurationStore(DefaultCacheFactory::create());
+
+        $loader = new ConfigurationLoader($apiWrapper, $configStore);
+        $loader->fetchAndStoreConfigurations(null);
+
+        $timestamp1 = $configStore->getMetadata("flagTimestamp");
+        $storedEtag = $configStore->getMetadata("flagETag");
+        $this->assertEquals("ETAG", $storedEtag);
+
+        usleep(50 * 1000); // Sleep long enough for cache to expire.
+
+        $loader->fetchAndStoreConfigurations("ETAG");
+
+        $this->assertEquals("ETAG", $configStore->getMetadata("flagETag"));
+
+        // The timestamp should not have changed; the config did not change, so the timestamp should not be updated.
+        $this->assertEquals($timestamp1, $configStore->getMetadata("flagTimestamp"));
+    }
+
     public function testLoadsOnGet(): void
     {
         // Arrange: Load some flag data to be returned by the APIRequestWrapper

tests/EppoClientTest.php

@@ -160,6 +160,38 @@ public function testGracefulModeThrowsOnInit()
         );
     }
 
+    public function testSuppressInitExceptionThrow()
+    {
+        $pollerMock = $this->getPollerMock();
+
+        $apiRequestWrapper = $this->getMockBuilder(APIRequestWrapper::class)->setConstructorArgs(
+            ['', [], new Psr18Client(), new Psr17Factory()]
+        )->getMock();
+
+        $apiRequestWrapper->expects($this->any())
+            ->method('getUFC')
+            ->willThrowException(new HttpRequestException());
+
+        $configStore = $this->getMockBuilder(IConfigurationStore::class)->getMock();
+        $configStore->expects($this->any())->method('getMetadata')->willReturn(null);
+        $mockLogger = $this->getMockBuilder(LoggerInterface::class)->getMock();
+
+        $client = EppoClient::createTestClient(
+            new ConfigurationLoader($apiRequestWrapper, $configStore),
+            $pollerMock,
+            $mockLogger,
+            true,
+            throwOnFailedInit: false
+        );
+
+        $this->assertEquals(
+            'default',
+            $client->getStringAssignment(self::EXPERIMENT_NAME, 'subject-10', [], 'default')
+        );
+
+        // No exceptions thrown, default assignments.
+    }
+
     public function testReturnsDefaultWhenExperimentConfigIsAbsent()
     {
         $configLoaderMock = $this->getFlagConfigurationLoaderMock([]);
@@ -179,7 +211,12 @@ public function testReturnsDefaultWhenExperimentConfigIsAbsent()
     public function testRepoTestCases(): void
     {
         try {
-            $client = EppoClient::init('dummy', self::$mockServer->serverAddress, isGracefulMode: false);
+            $client = EppoClient::init(
+                'dummy',
+                self::$mockServer->serverAddress,
+                isGracefulMode: false,
+                throwOnFailedInit: true
+            );
         } catch (Exception $exception) {
             self::fail('Failed to initialize EppoClient: ' . $exception->getMessage());
         }
@@ -315,7 +352,9 @@ public function testInitWithPollingOptions(): void
         );
 
         $response = new Response(stream: Utils::streamFor(file_get_contents(__DIR__ . '/data/ufc/flags-v1.json')));
-        $secondResponse = new Response(stream: Utils::streamFor(file_get_contents(__DIR__ . '/data/ufc/bandit-flags-v1.json')));
+        $secondResponse = new Response(stream: Utils::streamFor(
+            file_get_contents(__DIR__ . '/data/ufc/bandit-flags-v1.json')
+        ));
 
         $httpClient = $this->createMock(ClientInterface::class);
         $httpClient->expects($this->atLeast(2))
@@ -327,15 +366,16 @@ public function testInitWithPollingOptions(): void
             "fake address",
             httpClient: $httpClient,
             isGracefulMode: false,
-            pollingOptions: $pollingOptions
+            pollingOptions: $pollingOptions,
+            throwOnFailedInit: true
         );
 
         $this->assertEquals(
             3.1415926,
             $client->getNumericAssignment(self::EXPERIMENT_NAME, 'subject-10', [], 0)
         );
         // Wait a little bit for the cache to age out and the mock server to spin up.
-        usleep(75*1000);
+        usleep(75 * 1000);
 
         $this->assertEquals(
             0,