Add an in-memory cache layer on top of the custom cache (#48)

* Add an in-memory cache layer on top of the custom cache * Bump version / reformat * Push coverage report to sonarcloud * Add more tests * Add more tests * Update badges
configcat · Apr 15, 2024 · eca206b · eca206b
1 parent ee8c833
commit eca206b
Show file tree

Hide file tree

Showing 10 changed files with 161 additions and 47 deletions.
diff --git a/.github/workflows/php-ci.yml b/.github/workflows/php-ci.yml
@@ -44,7 +44,7 @@ jobs:
     - name: Execute tests
       run: vendor/bin/phpunit
 
-  coverage:
+  analysis:
     needs: test
     runs-on: ubuntu-latest
     steps:
@@ -76,13 +76,14 @@ jobs:
         composer install --prefer-dist --no-progress --no-suggest --no-plugins
 
     - name: Execute coverage
-      run: vendor/bin/phpunit --coverage-clover clover.xml
+      run: vendor/bin/phpunit --coverage-clover=coverage.xml --log-junit=tests.xml
+      env:
+        XDEBUG_MODE: coverage
 
-    - name: Upload coverage
-      uses: codecov/codecov-action@v3
-      with:
-        fail_ci_if_error: true
-        files: ./clover.xml
+    - name: SonarCloud Scan
+      uses: SonarSource/sonarcloud-github-action@master
+      env:
+        SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
 
   php-cs-fixer:
     runs-on: ubuntu-latest

diff --git a/README.md b/README.md
@@ -8,10 +8,10 @@ ConfigCat is a feature flag and configuration management service that lets you s
 ConfigCat is a <a href="https://configcat.com" target="_blank">hosted feature flag service</a>. Manage feature toggles across frontend, backend, mobile, desktop apps. <a href="https://configcat.com" target="_blank">Alternative to LaunchDarkly</a>. Management app + feature flag SDKs.
 
 [![Build Status](https://github.com/configcat/php-sdk/actions/workflows/php-ci.yml/badge.svg?branch=master)](https://github.com/configcat/php-sdk/actions/workflows/php-ci.yml)
-[![Coverage Status](https://img.shields.io/codecov/c/github/ConfigCat/php-sdk.svg)](https://codecov.io/gh/ConfigCat/php-sdk)
 [![Latest Stable Version](https://poser.pugx.org/configcat/configcat-client/version)](https://packagist.org/packages/configcat/configcat-client)
 [![Total Downloads](https://poser.pugx.org/configcat/configcat-client/downloads)](https://packagist.org/packages/configcat/configcat-client)
-[![Latest Unstable Version](https://poser.pugx.org/configcat/configcat-client/v/unstable)](https://packagist.org/packages/configcat/configcat-client)
+[![Sonar Quality Gate](https://img.shields.io/sonar/quality_gate/configcat_php-sdk?logo=sonarcloud&server=https%3A%2F%2Fsonarcloud.io)](https://sonarcloud.io/project/overview?id=configcat_php-sdk)
+[![Sonar Coverage](https://img.shields.io/sonar/coverage/configcat_php-sdk?logo=SonarCloud&server=https%3A%2F%2Fsonarcloud.io)](https://sonarcloud.io/project/overview?id=configcat_php-sdk)
 
 ## Requirements
 - PHP >= 8.1

diff --git a/sonar-project.properties b/sonar-project.properties
@@ -0,0 +1,7 @@
+sonar.projectKey=configcat_php-sdk
+sonar.organization=configcat
+sonar.sources=src
+sonar.tests=tests
+
+sonar.php.tests.reportPath=tests.xml
+sonar.php.coverage.reportPaths=coverage.xml
diff --git a/src/Cache/ConfigCache.php b/src/Cache/ConfigCache.php
@@ -14,6 +14,11 @@
  */
 abstract class ConfigCache implements LoggerAwareInterface
 {
+    /**
+     * @var array<string, ConfigEntry>
+     */
+    private static array $inMemoryCache = [];
+
     private LoggerInterface $logger;
 
     /**
@@ -32,6 +37,7 @@ public function store(string $key, ConfigEntry $value): void
         }
 
         try {
+            self::$inMemoryCache[$key] = $value;
             $this->set($key, $value->serialize());
         } catch (Throwable $exception) {
             $this->logger->error('Error occurred while writing the cache.', [
@@ -59,17 +65,20 @@ public function load(string $key): ConfigEntry
         try {
             $cached = $this->get($key);
             if (empty($cached)) {
-                return ConfigEntry::empty();
+                return self::readFromMemory($key);
             }
 
-            return ConfigEntry::fromCached($cached);
+            $fromCache = ConfigEntry::fromCached($cached);
+            self::$inMemoryCache[$key] = $fromCache;
+
+            return $fromCache;
         } catch (Throwable $exception) {
             $this->logger->error('Error occurred while reading the cache.', [
                 'event_id' => 2200, 'exception' => $exception,
             ]);
         }
 
-        return ConfigEntry::empty();
+        return self::readFromMemory($key);
     }
 
     /**
@@ -96,4 +105,9 @@ abstract protected function get(string $key): ?string;
      * @param string $value the value to cache
      */
     abstract protected function set(string $key, string $value): void;
+
+    private function readFromMemory(string $key): ConfigEntry
+    {
+        return self::$inMemoryCache[$key] ?? ConfigEntry::empty();
+    }
 }
diff --git a/src/ConfigCatClient.php b/src/ConfigCatClient.php
@@ -30,7 +30,7 @@
  */
 final class ConfigCatClient implements ClientInterface
 {
-    public const SDK_VERSION = '9.1.0';
+    public const SDK_VERSION = '9.1.1';
     private const CONFIG_JSON_CACHE_VERSION = 'v2';
 
     private InternalLogger $logger;

diff --git a/tests/CacheTest.php b/tests/CacheTest.php
@@ -8,9 +8,11 @@
 use ConfigCat\ConfigCatClient;
 use ConfigCat\Http\GuzzleFetchClient;
 use DateTime;
+use Exception;
 use GuzzleHttp\Handler\MockHandler;
 use GuzzleHttp\Psr7\Response;
 use PHPUnit\Framework\TestCase;
+use Psr\Log\NullLogger;
 
 class CacheTest extends TestCase
 {
@@ -40,13 +42,66 @@ public function testCachePayload()
         $this->assertEquals($time, $fromCache->getFetchTime());
     }
 
+    public function testCustomCacheKeepsCurrentInMemory()
+    {
+        $cache = new TestCache();
+        $cache->store('key', ConfigEntry::fromConfigJson(self::TEST_JSON, 'etag', 1234567));
+
+        $cached = $cache->load('key');
+
+        $this->assertEquals(self::TEST_JSON, $cached->getConfigJson());
+        $this->assertEquals('etag', $cached->getEtag());
+        $this->assertEquals(1234567, $cached->getFetchTime());
+    }
+
+    public function testThrowingCacheKeepsCurrentInMemory()
+    {
+        $cache = new TestCache(true);
+        $cache->setLogger(new NullLogger());
+        $cache->store('key', ConfigEntry::fromConfigJson(self::TEST_JSON, 'etag', 1234567));
+
+        $cached = $cache->load('key');
+
+        $this->assertEquals(self::TEST_JSON, $cached->getConfigJson());
+        $this->assertEquals('etag', $cached->getEtag());
+        $this->assertEquals(1234567, $cached->getFetchTime());
+    }
+
+    public function testCacheLoadValueFromCacheInMemory()
+    {
+        $secondValue = '{"f":{"testKey":{"t":1,"v":{"s":"testValue"}}}}';
+
+        $cache = new TestCache();
+        $cache->setLogger(new NullLogger());
+        $cache->store('key', ConfigEntry::fromConfigJson(self::TEST_JSON, 'etag', 1234567));
+
+        $cached = $cache->load('key');
+
+        $this->assertEquals(self::TEST_JSON, $cached->getConfigJson());
+        $this->assertEquals('etag', $cached->getEtag());
+        $this->assertEquals(1234567, $cached->getFetchTime());
+
+        $cache->setDefaultValue(ConfigEntry::fromConfigJson($secondValue, 'etag2', 12345678)->serialize());
+
+        $cached = $cache->load('key');
+
+        $this->assertEquals($secondValue, $cached->getConfigJson());
+        $this->assertEquals('etag2', $cached->getEtag());
+        $this->assertEquals(12345678, $cached->getFetchTime());
+
+        $cache->setThrowException(true);
+
+        $cached = $cache->load('key');
+
+        $this->assertEquals($secondValue, $cached->getConfigJson());
+        $this->assertEquals('etag2', $cached->getEtag());
+        $this->assertEquals(12345678, $cached->getFetchTime());
+    }
+
     /**
      * @dataProvider cacheKeyTestData
-     *
-     * @param mixed $sdkKey
-     * @param mixed $cacheKey
      */
-    public function testCacheKeyGeneration($sdkKey, $cacheKey)
+    public function testCacheKeyGeneration(mixed $sdkKey, mixed $cacheKey)
     {
         $cache = $this->getMockBuilder(ConfigCache::class)->getMock();
         $client = new ConfigCatClient($sdkKey, [
@@ -81,3 +136,44 @@ public function cacheKeyTestData(): array
         ];
     }
 }
+
+class TestCache extends ConfigCache
+{
+    private bool $throwException;
+
+    private ?string $defaultValue;
+
+    public function __construct($throwException = false, $defaultValue = null)
+    {
+        $this->throwException = $throwException;
+        $this->defaultValue = $defaultValue;
+    }
+
+    public function setDefaultValue(string $value): void
+    {
+        $this->defaultValue = $value;
+    }
+
+    public function setThrowException(bool $throw): void
+    {
+        $this->throwException = $throw;
+    }
+
+    /**
+     * @throws Exception
+     */
+    protected function get(string $key): ?string
+    {
+        return $this->throwException ? throw new Exception() : $this->defaultValue;
+    }
+
+    /**
+     * @throws Exception
+     */
+    protected function set(string $key, string $value): void
+    {
+        if ($this->throwException) {
+            throw new Exception();
+        }
+    }
+}
diff --git a/tests/ConfigV2EvaluationTests.php b/tests/ConfigV2EvaluationTests.php
@@ -19,7 +19,7 @@ class ConfigV2EvaluationTests extends TestCase
 {
     private const TEST_DATA_ROOT_PATH = 'tests/data';
 
-    public function provideTestDataForComparisonAttributeConversionToCanonicalStringRepresentation()
+    public function provideTestDataForComparisonAttributeConversionToCanonicalStringRepresentation(): array
     {
         return Utils::withDescription([
             ['numberToStringConversion', .12345, '1'],
@@ -70,7 +70,7 @@ public function testComparisonAttributeConversionToCanonicalStringRepresentation
         $this->assertSame($expectedReturnValue, $actualReturnValue);
     }
 
-    public function provideTestDataForComparisonAttributeTrimming()
+    public function provideTestDataForComparisonAttributeTrimming(): array
     {
         return Utils::withDescription([
             ['isoneof', 'no trim'],
@@ -181,7 +181,7 @@ public function provideTestDataForComparisonValueTrimming()
     }
 
     /**
-     * @dataProvider provideTestDataForComparisonValueTrimming_Test
+     * @dataProvider provideTestDataForComparisonValueTrimming
      */
     public function testComparisonValueTrimming(string $key, string $expectedReturnValue)
     {

diff --git a/tests/DataGovernanceTest.php b/tests/DataGovernanceTest.php
@@ -33,7 +33,7 @@ public function testShouldStayOnServer()
         $response = $fetcher->fetch('');
 
         // Assert
-        $this->assertEquals(1, count($requests));
+        $this->assertCount(1, $requests);
         $this->assertStringContainsString($requests[0]['request']->getUri()->getHost(), ConfigFetcher::GLOBAL_URL);
         $this->assertEquals(Config::deserialize($body), $response->getConfigEntry()->getConfig());
     }
@@ -54,7 +54,7 @@ public function testShouldStayOnSameUrlWithRedirect()
         $response = $fetcher->fetch('');
 
         // Assert
-        $this->assertEquals(1, count($requests));
+        $this->assertCount(1, $requests);
         $this->assertStringContainsString($requests[0]['request']->getUri()->getHost(), ConfigFetcher::GLOBAL_URL);
         $this->assertEquals(Config::deserialize($body), $response->getConfigEntry()->getConfig());
     }
@@ -75,7 +75,7 @@ public function testShouldStayOnSameUrlEvenWhenForced()
         $response = $fetcher->fetch('');
 
         // Assert
-        $this->assertEquals(1, count($requests));
+        $this->assertCount(1, $requests);
         $this->assertStringContainsString($requests[0]['request']->getUri()->getHost(), ConfigFetcher::GLOBAL_URL);
         $this->assertEquals(Config::deserialize($body), $response->getConfigEntry()->getConfig());
     }
@@ -98,7 +98,7 @@ public function testShouldRedirectToAnotherServer()
         $response = $fetcher->fetch('');
 
         // Assert
-        $this->assertEquals(2, count($requests));
+        $this->assertCount(2, $requests);
         $this->assertStringContainsString($requests[0]['request']->getUri()->getHost(), ConfigFetcher::GLOBAL_URL);
         $this->assertStringContainsString($requests[1]['request']->getUri()->getHost(), ConfigFetcher::EU_ONLY_URL);
         $this->assertEquals(Config::deserialize($secondBody), $response->getConfigEntry()->getConfig());
@@ -122,7 +122,7 @@ public function testShouldRedirectToAnotherServerWhenForced()
         $response = $fetcher->fetch('');
 
         // Assert
-        $this->assertEquals(2, count($requests));
+        $this->assertCount(2, $requests);
         $this->assertStringContainsString($requests[0]['request']->getUri()->getHost(), ConfigFetcher::GLOBAL_URL);
         $this->assertStringContainsString($requests[1]['request']->getUri()->getHost(), ConfigFetcher::EU_ONLY_URL);
         $this->assertEquals(Config::deserialize($secondBody), $response->getConfigEntry()->getConfig());
@@ -147,7 +147,7 @@ public function testShouldBreakRedirectLoop()
         $response = $fetcher->fetch('');
 
         // Assert
-        $this->assertEquals(3, count($requests));
+        $this->assertCount(3, $requests);
         $this->assertStringContainsString($requests[0]['request']->getUri()->getHost(), ConfigFetcher::GLOBAL_URL);
         $this->assertStringContainsString($requests[1]['request']->getUri()->getHost(), ConfigFetcher::EU_ONLY_URL);
         $this->assertStringContainsString($requests[2]['request']->getUri()->getHost(), ConfigFetcher::GLOBAL_URL);
@@ -173,7 +173,7 @@ public function testShouldBreakRedirectLoopWhenForced()
         $response = $fetcher->fetch('');
 
         // Assert
-        $this->assertEquals(3, count($requests));
+        $this->assertCount(3, $requests);
         $this->assertStringContainsString($requests[0]['request']->getUri()->getHost(), ConfigFetcher::GLOBAL_URL);
         $this->assertStringContainsString($requests[1]['request']->getUri()->getHost(), ConfigFetcher::EU_ONLY_URL);
         $this->assertStringContainsString($requests[2]['request']->getUri()->getHost(), ConfigFetcher::GLOBAL_URL);
@@ -197,15 +197,15 @@ public function testShouldRespectCustomUrlWhenNotForced()
         $response = $fetcher->fetch('');
 
         // Assert
-        $this->assertEquals(1, count($requests));
+        $this->assertCount(1, $requests);
         $this->assertStringContainsString($requests[0]['request']->getUri()->getHost(), self::CUSTOM_CDN_URL);
         $this->assertEquals(Config::deserialize($firstBody), $response->getConfigEntry()->getConfig());
 
         // Act
         $response = $fetcher->fetch('');
 
         // Assert
-        $this->assertEquals(2, count($requests));
+        $this->assertCount(2, $requests);
         $this->assertStringContainsString($requests[1]['request']->getUri()->getHost(), self::CUSTOM_CDN_URL);
         $this->assertEquals(Config::deserialize($firstBody), $response->getConfigEntry()->getConfig());
     }
@@ -228,13 +228,13 @@ public function testShouldNotRespectCustomUrlWhenForced()
         $response = $fetcher->fetch('');
 
         // Assert
-        $this->assertEquals(2, count($requests));
+        $this->assertCount(2, $requests);
         $this->assertStringContainsString($requests[0]['request']->getUri()->getHost(), self::CUSTOM_CDN_URL);
         $this->assertStringContainsString($requests[1]['request']->getUri()->getHost(), ConfigFetcher::GLOBAL_URL);
         $this->assertEquals(Config::deserialize($secondBody), $response->getConfigEntry()->getConfig());
     }
 
-    private function getHandlerStack(array $responses, array &$container = [])
+    private function getHandlerStack(array $responses, array &$container = []): HandlerStack
     {
         $history = Middleware::history($container);
         $stack = HandlerStack::create(new MockHandler($responses));
@@ -243,7 +243,7 @@ private function getHandlerStack(array $responses, array &$container = [])
         return $stack;
     }
 
-    private function getFetcher($handler, $customUrl = '')
+    private function getFetcher($handler, $customUrl = ''): ConfigFetcher
     {
         return new ConfigFetcher('fakeKey', Utils::getTestLogger(), [
             ClientOptions::CUSTOM_HANDLER => $handler,