add unit test for passing a preconfigured client + improve prefix logic (#5)

Author: eli-darkly

src/LaunchDarkly/Impl/Integrations/PHPRedisFeatureRequester.php

@@ -22,7 +22,7 @@ public function __construct($baseUri, $sdkKey, $options)
         }
 
         $client = $this->_options['phpredis_client'] ?? null;
-        if ($client instanceof Redis) {
+        if ($client instanceof \Redis) {
             $this->_redisInstance = $client;
         } else {
             $this->_redisOptions = [
@@ -36,13 +36,13 @@ public function __construct($baseUri, $sdkKey, $options)
     protected function readItemString(string $namespace, string $key): ?string
     {
         $redis = $this->getConnection();
-        return $redis->hget($namespace, $key);
+        return $redis->hget("$this->_prefix:$namespace", $key);
     }
 
     protected function readItemStringList(string $namespace): ?array
     {
         $redis = $this->getConnection();
-        $raw = $redis->hgetall($namespace);
+        $raw = $redis->hgetall("$this->_prefix:$namespace");
         return $raw ? array_values($raw) : null;
     }
 
@@ -51,7 +51,7 @@ protected function readItemStringList(string $namespace): ?array
      */
     protected function getConnection()
     {
-        if ($this->_redisInstance instanceof Redis) {
+        if ($this->_redisInstance instanceof \Redis) {
             return $this->_redisInstance;
         }
 
@@ -62,7 +62,6 @@ protected function getConnection()
             $this->_redisOptions["timeout"],
             'launchdarkly/php-server-sdk-redis-phpredis'
         );
-        $redis->setOption(\Redis::OPT_PREFIX, "$this->_prefix:");	// use custom prefix on all keys
         return $this->_redisInstance = $redis;
     }
 }

tests/PHPRedisFeatureRequesterTest.php

@@ -35,8 +35,10 @@ protected function clearExistingData($prefix): void
 
     protected function makeRequester($prefix): FeatureRequester
     {
-        $factory = PHPRedis::featureRequester();
-        return $factory('', '', ['redis_prefix' => $prefix]);
+        $factory = PHPRedis::featureRequester([
+            'redis_prefix' => $prefix
+        ]);
+        return $factory('', '', []);
     }
 
     protected function putSerializedItem($prefix, $namespace, $key, $version, $json): void

tests/PHPRedisFeatureRequesterWithClientTest.php

@@ -0,0 +1,74 @@
+<?php
+
+namespace LaunchDarkly\Impl\Integrations\Tests;
+
+use LaunchDarkly\FeatureRequester;
+use LaunchDarkly\Impl\Integrations\PHPRedisFeatureRequester;
+use LaunchDarkly\Integrations\PHPRedis;
+use LaunchDarkly\SharedTest\DatabaseFeatureRequesterTestBase;
+use \Redis;
+
+class PHPRedisFeatureRequesterWithClientTest extends DatabaseFeatureRequesterTestBase
+{
+    const CLIENT_PREFIX = 'clientprefix';
+
+    /** @var ClientInterface */
+    private static $redisClient;
+    
+    public static function setUpBeforeClass(): void
+    {
+        self::$redisClient = new \Redis();
+        self::$redisClient->pconnect(
+            'localhost',
+            6379,
+            null,
+            'RedisFeatureRequesterWithClientTest'
+        );
+        self::$redisClient->setOption(\Redis::OPT_PREFIX, self::CLIENT_PREFIX);
+
+        // Setting a prefix parameter on the Redis client causes it to prepend
+        // that string to every key *in addition to* the other prefix that the SDK
+        // integration is applying. This is done transparently so we do not need to
+        // add CLIENT_PREFIX in putItem below. We're doing it so we can be sure
+        // that the PHPRedisFeatureRequester really is using the same client we
+        // passed to it; if it didn't, the tests would fail because putItem was
+        // creating keys with both prefixes but PHPRedisFeatureRequester was
+        // looking for keys with only one prefix.
+    }
+
+    protected function clearExistingData($prefix): void
+    {
+        $p = self::realPrefix($prefix);
+        $keys = self::$redisClient->keys("$p:*");
+        foreach ($keys as $key) {
+            if (substr($key, 0, strlen(self::CLIENT_PREFIX)) === self::CLIENT_PREFIX) {
+                // remove extra prefix from the queried keys because del() will re-add it
+                $key = substr($key, strlen(self::CLIENT_PREFIX));
+            }
+            self::$redisClient->del($key);
+        }
+    }
+    
+    protected function makeRequester($prefix): FeatureRequester
+    {
+        $factory = PHPRedis::featureRequester([
+            'redis_prefix' => $prefix,
+            'phpredis_client' => self::$redisClient
+        ]);
+        return $factory('', '', []);
+    }
+
+    protected function putSerializedItem($prefix, $namespace, $key, $version, $json): void
+    {
+        $p = self::realPrefix($prefix);
+        self::$redisClient->hset("$p:$namespace", $key, $json);
+    }
+
+    private static function realPrefix($prefix)
+    {
+        if ($prefix === null || $prefix === '') {
+            return 'launchdarkly';
+        }
+        return $prefix;
+    }
+}