[Bug]: Excessive HTTP requests when not feature flags have been defined

[eskil-booli]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

I'm using the Unleash PHP client to connect to GitLab. If no feature flags have been defined, then the Unleash PHP SDK makes a HTTP request to GitLab for every feature flag check, instead of using the cached response.

To reproduce

1. Connect the Unleash PHP SDK to a backend that returns {"version":1,"features":[]} on requests to GET /client/features.
2. Run `$unleash->isEnabled("test")" multiple times.

Actual behavior:

• Each invocation triggers a call to the backend.

Sample code (optional)

No response

Version

v1.6.080

Expected behavior

For the first request to have been cached and reused.

Logs (optional)

No response

Additional context (optional)

Possible fix:

From d2096fd10ef93e9358930a4c1ac3ef1df78299af Mon Sep 17 00:00:00 2001
From: Eskil Bylund <eskil.bylund@booli.se>
Date: Fri, 8 Jul 2022 11:35:09 +0200
Subject: [PATCH] Fix empty feature list not cached properly by the repository

This caused excessive HTTP requests to GitLab when no feature flags
had been configured.
---
 src/Repository/DefaultUnleashRepository.php   |  2 +-
 .../DefaultUnleashRepositoryTest.php          | 28 +++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/src/Repository/DefaultUnleashRepository.php b/src/Repository/DefaultUnleashRepository.php
index 002e8fc..9b4332c 100755
--- a/src/Repository/DefaultUnleashRepository.php
+++ b/src/Repository/DefaultUnleashRepository.php
@@ -55,7 +55,7 @@ final class DefaultUnleashRepository implements UnleashRepository
      */
     public function getFeatures(): iterable
     {
-        if (!$features = $this->getCachedFeatures()) {
+        if (($features = $this->getCachedFeatures()) === null) {
             if (!$this->configuration->isFetchingEnabled()) {
                 if (!$data = $this->getBootstrappedResponse()) {
                     throw new LogicException('Fetching of Unleash api is disabled but no bootstrap is provided');
diff --git a/tests/Repository/DefaultUnleashRepositoryTest.php b/tests/Repository/DefaultUnleashRepositoryTest.php
index 0ed19a6..857560f 100755
--- a/tests/Repository/DefaultUnleashRepositoryTest.php
+++ b/tests/Repository/DefaultUnleashRepositoryTest.php
@@ -112,6 +112,34 @@ final class DefaultUnleashRepositoryTest extends AbstractHttpClientTest
         self::assertEquals($feature->getName(), $repository->findFeature('test')->getName());
     }
 
+    public function testCacheZeroFeatures()
+    {
+        $response = [
+            'version' => 1,
+            'features' => [],
+        ];
+
+        $cache = $this->getRealCache();
+        $repository = new DefaultUnleashRepository(
+            new Client([
+                'handler' => HandlerStack::create($this->mockHandler),
+            ]),
+            new HttpFactory(),
+            (new UnleashConfiguration('', '', ''))
+                ->setCache($cache)
+                ->setTtl(5)
+        );
+
+        $this->pushResponse($response, 2);
+        $repository->getFeatures();
+        $repository->getFeatures();
+        self::assertEquals(1, $this->mockHandler->count());
+        $cache->clear();
+
+        $this->pushResponse($response);
+        self::assertNull($repository->findFeature('test'));
+    }
+
     public function testCustomHeaders()
     {
         $this->pushResponse($this->response);
-- 
2.30.1 (Apple Git-130)

[RikudouSage]

Thank you for the report and for the fix! I always forget that empty array evaluates to false.