From 80fe797ac4f37d1441c4a4007b41f967a7709c25 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Tue, 14 Jan 2025 10:05:04 -0500 Subject: [PATCH 1/4] chore: Inline shared test package --- .github/actions/ci/action.yml | 8 - .github/workflows/ci.yml | 16 - composer.json | 6 - tests/DatabaseFeatureRequesterTestBase.php | 301 ++++++++++++++++++ tests/PHPRedisFeatureRequesterTest.php | 1 - ...PHPRedisFeatureRequesterWithClientTest.php | 3 - 6 files changed, 301 insertions(+), 34 deletions(-) create mode 100644 tests/DatabaseFeatureRequesterTestBase.php diff --git a/.github/actions/ci/action.yml b/.github/actions/ci/action.yml index 63544f4..af46518 100644 --- a/.github/actions/ci/action.yml +++ b/.github/actions/ci/action.yml @@ -10,10 +10,6 @@ inputs: type: boolean required: false default: false - shared-test-version: - description: 'Which version of the shared test package should we required' - required: false - default: 4.x-dev token: description: 'Token used to prevent composer rate limiting' required: true @@ -33,10 +29,6 @@ runs: shell: bash run: composer install --no-progress - - name: Require appropriate shared tests package - shell: bash - run: composer require --dev 'launchdarkly/server-sdk-shared-tests:${{ inputs.shared-test-version }}' - - name: Downgrade to lowest versions if: ${{ inputs.use-lowest-dependencies }} shell: bash diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0d84ab9..69f2d20 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,29 +23,14 @@ jobs: fail-fast: false matrix: include: - # 8.1 configurations - php-version: 8.1 use-lowest-dependencies: true - shared-test-version: 5.x-dev - - php-version: 8.1 - use-lowest-dependencies: true - shared-test-version: dev-main - # 8.2 configurations - - php-version: 8.2 - use-lowest-dependencies: false - shared-test-version: 5.x-dev - php-version: 8.2 use-lowest-dependencies: false - shared-test-version: dev-main - # 8.3 configurations - - php-version: 8.3 - use-lowest-dependencies: false - shared-test-version: 5.x-dev - php-version: 8.3 use-lowest-dependencies: false - shared-test-version: dev-main steps: - uses: actions/checkout@v4 @@ -56,5 +41,4 @@ jobs: with: php-version: ${{ matrix.php-version }} use-lowest-dependencies: ${{ matrix.use-lowest-dependencies }} - shared-test-version: ${{ matrix.shared-test-version }} token: ${{ secrets.GITHUB_TOKEN }} diff --git a/composer.json b/composer.json index b1f551d..18cb87a 100644 --- a/composer.json +++ b/composer.json @@ -1,10 +1,4 @@ { - "repositories": [ - { - "type": "vcs", - "url": "https://github.com/launchdarkly/php-server-sdk-shared-tests" - } - ], "name": "launchdarkly/server-sdk-redis-phpredis", "description": "LaunchDarkly PHP SDK Redis integration using the phpredis extension", "keywords": [ diff --git a/tests/DatabaseFeatureRequesterTestBase.php b/tests/DatabaseFeatureRequesterTestBase.php new file mode 100644 index 0000000..9f14a53 --- /dev/null +++ b/tests/DatabaseFeatureRequesterTestBase.php @@ -0,0 +1,301 @@ +clearExistingData($prefix); + $fr = $this->makeRequester($prefix); + + $flagKey = 'foo'; + $flagVersion = 10; + $flagJson = self::makeFlagJson($flagKey, $flagVersion); + $this->putSerializedItem($prefix, 'features', $flagKey, $flagVersion, $flagJson); + + $fr = $this->makeRequester($prefix); + $flag = $fr->getFeature($flagKey); + + $this->assertInstanceOf(FeatureFlag::class, $flag); + $this->assertEquals($flagVersion, $flag->getVersion()); + } + + /** + * @dataProvider prefixParameters + */ + public function testGetMissingFeature(?string $prefix): void + { + $this->clearExistingData($prefix); + $fr = $this->makeRequester($prefix); + + $flag = $fr->getFeature('unavailable'); + $this->assertNull($flag); + } + + /** + * @dataProvider prefixParameters + */ + public function testGetDeletedFeature(?string $prefix): void + { + $this->clearExistingData($prefix); + $fr = $this->makeRequester($prefix); + + $flagKey = 'foo'; + $flagVersion = 10; + $flagJson = self::makeFlagJson($flagKey, $flagVersion, true); + $this->putSerializedItem($prefix, 'features', $flagKey, $flagVersion, $flagJson); + + $flag = $fr->getFeature($flagKey); + + $this->assertNull($flag); + } + + /** + * @dataProvider prefixParameters + */ + public function testGetAllFeatures(?string $prefix): void + { + $this->clearExistingData($prefix); + $fr = $this->makeRequester($prefix); + + $flagKey1 = 'foo'; + $flagKey2 = 'bar'; + $flagKey3 = 'deleted'; + $flagVersion = 10; + $flagJson1 = self::makeFlagJson($flagKey1, $flagVersion); + $flagJson2 = self::makeFlagJson($flagKey2, $flagVersion); + $flagJson3 = self::makeFlagJson($flagKey3, $flagVersion, true); + + $this->putSerializedItem($prefix, 'features', $flagKey1, $flagVersion, $flagJson1); + $this->putSerializedItem($prefix, 'features', $flagKey2, $flagVersion, $flagJson2); + $this->putSerializedItem($prefix, 'features', $flagKey3, $flagVersion, $flagJson3); + + $flags = $fr->getAllFeatures(); + + $this->assertEquals(2, count($flags)); + $flag1 = $flags[$flagKey1]; + $this->assertEquals($flagKey1, $flag1->getKey()); + $this->assertEquals($flagVersion, $flag1->getVersion()); + $flag2 = $flags[$flagKey2]; + $this->assertEquals($flagKey2, $flag2->getKey()); + $this->assertEquals($flagVersion, $flag2->getVersion()); + } + + /** + * @dataProvider prefixParameters + */ + public function testAllFeaturesWithEmptyStore(?string $prefix): void + { + $this->clearExistingData($prefix); + $fr = $this->makeRequester($prefix); + + $flags = $fr->getAllFeatures(); + $this->assertEquals(array(), $flags); + } + + /** + * @dataProvider prefixParameters + */ + public function testGetSegment(?string $prefix): void + { + $this->clearExistingData($prefix); + $fr = $this->makeRequester($prefix); + + $segKey = 'foo'; + $segVersion = 10; + $segJson = self::makeSegmentJson($segKey, $segVersion); + $this->putSerializedItem($prefix, 'segments', $segKey, $segVersion, $segJson); + + $segment = $fr->getSegment($segKey); + + $this->assertInstanceOf(Segment::class, $segment); + $this->assertEquals($segVersion, $segment->getVersion()); + } + + /** + * @dataProvider prefixParameters + */ + public function testGetMissingSegment(?string $prefix): void + { + $this->clearExistingData($prefix); + $fr = $this->makeRequester($prefix); + + $segment = $fr->getSegment('unavailable'); + $this->assertNull($segment); + } + + /** + * @dataProvider prefixParameters + */ + public function testGetDeletedSegment(?string $prefix): void + { + $this->clearExistingData($prefix); + $fr = $this->makeRequester($prefix); + + $segKey = 'foo'; + $segVersion = 10; + $segJson = self::makeSegmentJson($segKey, $segVersion, true); + $this->putSerializedItem($prefix, 'segments', $segKey, $segVersion, $segJson); + + $segment = $fr->getSegment($segKey); + + $this->assertNull($segment); + } + + public function testPrefixIndependence(): void + { + $prefix1 = 'prefix1'; + $prefix2 = 'prefix2'; + + $this->clearExistingData(null); + $this->clearExistingData($prefix1); + $this->clearExistingData($prefix2); + + $flagKey = 'my-flag'; + $segmentKey = 'my-segment'; + $version0 = 10; + $version1 = 11; + $version2 = 12; + $this->setupForPrefix(null, $flagKey, $segmentKey, $version0); + $this->setupForPrefix($prefix1, $flagKey, $segmentKey, $version1); + $this->setupForPrefix($prefix2, $flagKey, $segmentKey, $version2); + + $this->verifyForPrefix($this->makeRequester(null), $flagKey, $segmentKey, $version0); + $this->verifyForPrefix($this->makeRequester(''), $flagKey, $segmentKey, $version0); + $this->verifyForPrefix($this->makeRequester($prefix1), $flagKey, $segmentKey, $version1); + $this->verifyForPrefix($this->makeRequester($prefix2), $flagKey, $segmentKey, $version2); + } + + private function setupForPrefix(?string $prefix, string $flagKey, string $segmentKey, int $flagVersion): void + { + $segmentVersion = $flagVersion * 2; + $this->putSerializedItem($prefix, 'features', $flagKey, $flagVersion, + self::makeFlagJson($flagKey, $flagVersion)); + $this->putSerializedItem($prefix, 'segments', $segmentKey, $segmentVersion, + self::makeSegmentJson($flagKey, $segmentVersion)); + } + + private function verifyForPrefix($fr, string $flagKey, string $segmentKey, int $flagVersion): void + { + $segmentVersion = $flagVersion * 2; + + $flag = $fr->getFeature($flagKey); + $this->assertNotNull($flag); + $this->assertEquals($flagVersion, $flag->getVersion()); + + $flags = $fr->getAllFeatures(); + $this->assertEquals(1, count($flags)); + $this->assertEquals($flagVersion, $flags[$flagKey]->getVersion()); + + $segment = $fr->getSegment($segmentKey); + $this->assertNotNull($segment); + $this->assertEquals($segmentVersion, $segment->getVersion()); + } + + /** + * @return array + */ + public function prefixParameters(): array + { + return [ + [ self::TEST_PREFIX ], + [ '' ], + [ null ] + ]; + } + + private static function makeFlagJson(string $key, int $version, bool $deleted = false): string|bool + { + return json_encode(array( + 'key' => $key, + 'version' => $version, + 'on' => true, + 'prerequisites' => [], + 'salt' => '', + 'targets' => [], + 'rules' => [], + 'fallthrough' => [ + 'variation' => 0, + ], + 'offVariation' => null, + 'variations' => [ + true, + false, + ], + 'deleted' => $deleted + )); + } + + private static function makeSegmentJson(string $key, int $version, bool $deleted = false): string|bool + { + return json_encode(array( + 'key' => $key, + 'version' => $version, + 'included' => array(), + 'excluded' => array(), + 'rules' => [], + 'salt' => '', + 'deleted' => $deleted + )); + } +} diff --git a/tests/PHPRedisFeatureRequesterTest.php b/tests/PHPRedisFeatureRequesterTest.php index 65d730a..c841915 100644 --- a/tests/PHPRedisFeatureRequesterTest.php +++ b/tests/PHPRedisFeatureRequesterTest.php @@ -3,7 +3,6 @@ namespace LaunchDarkly\Impl\Integrations\Tests; use LaunchDarkly\Integrations\PHPRedis; -use LaunchDarkly\SharedTest\DatabaseFeatureRequesterTestBase; class PHPRedisFeatureRequesterTest extends DatabaseFeatureRequesterTestBase { diff --git a/tests/PHPRedisFeatureRequesterWithClientTest.php b/tests/PHPRedisFeatureRequesterWithClientTest.php index a03186e..1234a84 100644 --- a/tests/PHPRedisFeatureRequesterWithClientTest.php +++ b/tests/PHPRedisFeatureRequesterWithClientTest.php @@ -2,10 +2,7 @@ namespace LaunchDarkly\Impl\Integrations\Tests; -use \Redis; -use LaunchDarkly\Impl\Integrations\PHPRedisFeatureRequester; use LaunchDarkly\Integrations\PHPRedis; -use LaunchDarkly\SharedTest\DatabaseFeatureRequesterTestBase; class PHPRedisFeatureRequesterWithClientTest extends DatabaseFeatureRequesterTestBase { From 86bbb9e8a59617df1e5eaf94b488e383bb153953 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Tue, 14 Jan 2025 14:55:17 -0500 Subject: [PATCH 2/4] feat: FeatureRequester requires configured Redis instance --- .../Integrations/PHPRedisFeatureRequester.php | 63 +--- src/LaunchDarkly/Integrations/PHPRedis.php | 28 +- tests/DatabaseFeatureRequesterTestBase.php | 301 ---------------- .../PHPRedisFeatureRequesterTest.php | 331 ++++++++++++++++++ tests/PHPRedisFeatureRequesterTest.php | 53 --- ...PHPRedisFeatureRequesterWithClientTest.php | 70 ---- 6 files changed, 357 insertions(+), 489 deletions(-) delete mode 100644 tests/DatabaseFeatureRequesterTestBase.php create mode 100644 tests/Impl/Integrations/PHPRedisFeatureRequesterTest.php delete mode 100644 tests/PHPRedisFeatureRequesterTest.php delete mode 100644 tests/PHPRedisFeatureRequesterWithClientTest.php diff --git a/src/LaunchDarkly/Impl/Integrations/PHPRedisFeatureRequester.php b/src/LaunchDarkly/Impl/Integrations/PHPRedisFeatureRequester.php index 8b957c1..9d26f79 100644 --- a/src/LaunchDarkly/Impl/Integrations/PHPRedisFeatureRequester.php +++ b/src/LaunchDarkly/Impl/Integrations/PHPRedisFeatureRequester.php @@ -10,65 +10,32 @@ */ class PHPRedisFeatureRequester extends FeatureRequesterBase { - private ?array $redisOptions = null; - private ?Redis $redisInstance = null; - private ?string $prefix; - - public function __construct(string $baseUri, string $sdkKey, array $options) - { + private readonly string $prefix; + + public function __construct( + private readonly Redis $client, + string $baseUri, + string $sdkKey, + array $options + ) { parent::__construct($baseUri, $sdkKey, $options); - /** @var ?string **/ - $this->prefix = $options['redis_prefix'] ?? null; - if ($this->prefix === null || $this->prefix === '') { - $this->prefix = Integrations\PHPRedis::DEFAULT_PREFIX; - } - - /** @var ?Redis */ - $client = $this->_options['phpredis_client'] ?? null; - if ($client instanceof Redis) { - $this->redisInstance = $client; - } else { - $this->redisOptions = [ - "timeout" => $options['redis_timeout'] ?? 5, - "host" => $options['redis_host'] ?? 'localhost', - "port" => $options['redis_port'] ?? 6379, - "password" => $options['redis_password'] ?? null - ]; + /** @var ?string */ + $prefix = $options['prefix'] ?? null; + if ($prefix === null || $prefix === '') { + $prefix = Integrations\PHPRedis::DEFAULT_PREFIX; } + $this->prefix = $prefix; } protected function readItemString(string $namespace, string $key): ?string { - $redis = $this->getConnection(); - return $redis->hget("$this->prefix:$namespace", $key); + return $this->client->hget("$this->prefix:$namespace", $key); } protected function readItemStringList(string $namespace): ?array { - $redis = $this->getConnection(); - $raw = $redis->hgetall("$this->prefix:$namespace"); + $raw = $this->client->hgetall("$this->prefix:$namespace"); return $raw ? array_values($raw) : null; } - - protected function getConnection(): Redis - { - if ($this->redisInstance instanceof Redis) { - return $this->redisInstance; - } - - $redis = new Redis(); - $redis->pconnect( - $this->redisOptions["host"], - $this->redisOptions["port"], - $this->redisOptions["timeout"], - 'launchdarkly/php-server-sdk-redis-phpredis' - ); - - if ($this->redisOptions['password']) { - $redis->auth($this->redisOptions['password']); - } - - return $this->redisInstance = $redis; - } } diff --git a/src/LaunchDarkly/Integrations/PHPRedis.php b/src/LaunchDarkly/Integrations/PHPRedis.php index cfc45ba..de206a5 100644 --- a/src/LaunchDarkly/Integrations/PHPRedis.php +++ b/src/LaunchDarkly/Integrations/PHPRedis.php @@ -4,6 +4,7 @@ use LaunchDarkly\Impl\Integrations; use LaunchDarkly\Subsystems; +use LaunchDarkly\Subsystems\FeatureRequester; use Psr\Log\LoggerInterface; use Redis; @@ -28,40 +29,33 @@ class PHPRedis * [SDK reference guide](https://docs.launchdarkly.com/sdk/features/storing-data). * * @param array $options Configuration settings (can also be passed in the main client configuration): - * - `redis_host`: hostname of the Redis server; defaults to `localhost` - * - `redis_port`: port of the Redis server; defaults to 6379 - * - `redis_password`: password to auth against the Redis server; optional - * - `redis_timeout`: connection timeout in seconds; defaults to 5 - * - `redis_prefix`: a string to be prepended to all database keys; corresponds to the prefix - * setting in ld-relay - * - `phpredis_client`: an already-configured Redis client instance if you wish to reuse one + * - `prefix`: a string to be prepended to all database keys; corresponds + * to the prefix setting in ld-relay * - `apc_expiration`: expiration time in seconds for local caching, if `APCu` is installed - * @return mixed an object to be stored in the `feature_requester` configuration property + * @return callable(string, string, array): FeatureRequester an object to be stored in the `feature_requester` configuration property */ - public static function featureRequester($options = []) + public static function featureRequester(Redis $client, $options = []): callable { if (!extension_loaded('redis')) { throw new \RuntimeException("phpredis extension is required to use Integrations\\PHPRedis"); } - return function (string $baseUri, string $sdkKey, array $baseOptions) use ($options) { - return new Integrations\PHPRedisFeatureRequester($baseUri, $sdkKey, array_merge($baseOptions, $options)); + return function (string $baseUri, string $sdkKey, array $baseOptions) use ($client, $options): FeatureRequester { + return new Integrations\PHPRedisFeatureRequester($client, $baseUri, $sdkKey, array_merge($baseOptions, $options)); }; } /** * @param array $options - * - `prefix`: namespace prefix to add to all hash keys - * @return callable(LoggerInterface, array): Subsystems\BigSegmentsStore + * - `prefix`: a string to be prepended to all database keys; corresponds + * to the prefix setting in ld-relay */ - public static function bigSegmentsStore(Redis $client, array $options = []): callable + public static function bigSegmentsStore(Redis $client, LoggerInterface $logger, array $options = []): Subsystems\BigSegmentsStore { if (!extension_loaded('redis')) { throw new \RuntimeException("phpredis extension is required to use Integrations\\PHPRedis"); } - return function (LoggerInterface $logger, array $baseOptions) use ($client, $options): Subsystems\BigSegmentsStore { - return new Integrations\PHPRedisBigSegmentsStore($client, $logger, array_merge($baseOptions, $options)); - }; + return new Integrations\PHPRedisBigSegmentsStore($client, $logger, $options); } } diff --git a/tests/DatabaseFeatureRequesterTestBase.php b/tests/DatabaseFeatureRequesterTestBase.php deleted file mode 100644 index 9f14a53..0000000 --- a/tests/DatabaseFeatureRequesterTestBase.php +++ /dev/null @@ -1,301 +0,0 @@ -clearExistingData($prefix); - $fr = $this->makeRequester($prefix); - - $flagKey = 'foo'; - $flagVersion = 10; - $flagJson = self::makeFlagJson($flagKey, $flagVersion); - $this->putSerializedItem($prefix, 'features', $flagKey, $flagVersion, $flagJson); - - $fr = $this->makeRequester($prefix); - $flag = $fr->getFeature($flagKey); - - $this->assertInstanceOf(FeatureFlag::class, $flag); - $this->assertEquals($flagVersion, $flag->getVersion()); - } - - /** - * @dataProvider prefixParameters - */ - public function testGetMissingFeature(?string $prefix): void - { - $this->clearExistingData($prefix); - $fr = $this->makeRequester($prefix); - - $flag = $fr->getFeature('unavailable'); - $this->assertNull($flag); - } - - /** - * @dataProvider prefixParameters - */ - public function testGetDeletedFeature(?string $prefix): void - { - $this->clearExistingData($prefix); - $fr = $this->makeRequester($prefix); - - $flagKey = 'foo'; - $flagVersion = 10; - $flagJson = self::makeFlagJson($flagKey, $flagVersion, true); - $this->putSerializedItem($prefix, 'features', $flagKey, $flagVersion, $flagJson); - - $flag = $fr->getFeature($flagKey); - - $this->assertNull($flag); - } - - /** - * @dataProvider prefixParameters - */ - public function testGetAllFeatures(?string $prefix): void - { - $this->clearExistingData($prefix); - $fr = $this->makeRequester($prefix); - - $flagKey1 = 'foo'; - $flagKey2 = 'bar'; - $flagKey3 = 'deleted'; - $flagVersion = 10; - $flagJson1 = self::makeFlagJson($flagKey1, $flagVersion); - $flagJson2 = self::makeFlagJson($flagKey2, $flagVersion); - $flagJson3 = self::makeFlagJson($flagKey3, $flagVersion, true); - - $this->putSerializedItem($prefix, 'features', $flagKey1, $flagVersion, $flagJson1); - $this->putSerializedItem($prefix, 'features', $flagKey2, $flagVersion, $flagJson2); - $this->putSerializedItem($prefix, 'features', $flagKey3, $flagVersion, $flagJson3); - - $flags = $fr->getAllFeatures(); - - $this->assertEquals(2, count($flags)); - $flag1 = $flags[$flagKey1]; - $this->assertEquals($flagKey1, $flag1->getKey()); - $this->assertEquals($flagVersion, $flag1->getVersion()); - $flag2 = $flags[$flagKey2]; - $this->assertEquals($flagKey2, $flag2->getKey()); - $this->assertEquals($flagVersion, $flag2->getVersion()); - } - - /** - * @dataProvider prefixParameters - */ - public function testAllFeaturesWithEmptyStore(?string $prefix): void - { - $this->clearExistingData($prefix); - $fr = $this->makeRequester($prefix); - - $flags = $fr->getAllFeatures(); - $this->assertEquals(array(), $flags); - } - - /** - * @dataProvider prefixParameters - */ - public function testGetSegment(?string $prefix): void - { - $this->clearExistingData($prefix); - $fr = $this->makeRequester($prefix); - - $segKey = 'foo'; - $segVersion = 10; - $segJson = self::makeSegmentJson($segKey, $segVersion); - $this->putSerializedItem($prefix, 'segments', $segKey, $segVersion, $segJson); - - $segment = $fr->getSegment($segKey); - - $this->assertInstanceOf(Segment::class, $segment); - $this->assertEquals($segVersion, $segment->getVersion()); - } - - /** - * @dataProvider prefixParameters - */ - public function testGetMissingSegment(?string $prefix): void - { - $this->clearExistingData($prefix); - $fr = $this->makeRequester($prefix); - - $segment = $fr->getSegment('unavailable'); - $this->assertNull($segment); - } - - /** - * @dataProvider prefixParameters - */ - public function testGetDeletedSegment(?string $prefix): void - { - $this->clearExistingData($prefix); - $fr = $this->makeRequester($prefix); - - $segKey = 'foo'; - $segVersion = 10; - $segJson = self::makeSegmentJson($segKey, $segVersion, true); - $this->putSerializedItem($prefix, 'segments', $segKey, $segVersion, $segJson); - - $segment = $fr->getSegment($segKey); - - $this->assertNull($segment); - } - - public function testPrefixIndependence(): void - { - $prefix1 = 'prefix1'; - $prefix2 = 'prefix2'; - - $this->clearExistingData(null); - $this->clearExistingData($prefix1); - $this->clearExistingData($prefix2); - - $flagKey = 'my-flag'; - $segmentKey = 'my-segment'; - $version0 = 10; - $version1 = 11; - $version2 = 12; - $this->setupForPrefix(null, $flagKey, $segmentKey, $version0); - $this->setupForPrefix($prefix1, $flagKey, $segmentKey, $version1); - $this->setupForPrefix($prefix2, $flagKey, $segmentKey, $version2); - - $this->verifyForPrefix($this->makeRequester(null), $flagKey, $segmentKey, $version0); - $this->verifyForPrefix($this->makeRequester(''), $flagKey, $segmentKey, $version0); - $this->verifyForPrefix($this->makeRequester($prefix1), $flagKey, $segmentKey, $version1); - $this->verifyForPrefix($this->makeRequester($prefix2), $flagKey, $segmentKey, $version2); - } - - private function setupForPrefix(?string $prefix, string $flagKey, string $segmentKey, int $flagVersion): void - { - $segmentVersion = $flagVersion * 2; - $this->putSerializedItem($prefix, 'features', $flagKey, $flagVersion, - self::makeFlagJson($flagKey, $flagVersion)); - $this->putSerializedItem($prefix, 'segments', $segmentKey, $segmentVersion, - self::makeSegmentJson($flagKey, $segmentVersion)); - } - - private function verifyForPrefix($fr, string $flagKey, string $segmentKey, int $flagVersion): void - { - $segmentVersion = $flagVersion * 2; - - $flag = $fr->getFeature($flagKey); - $this->assertNotNull($flag); - $this->assertEquals($flagVersion, $flag->getVersion()); - - $flags = $fr->getAllFeatures(); - $this->assertEquals(1, count($flags)); - $this->assertEquals($flagVersion, $flags[$flagKey]->getVersion()); - - $segment = $fr->getSegment($segmentKey); - $this->assertNotNull($segment); - $this->assertEquals($segmentVersion, $segment->getVersion()); - } - - /** - * @return array - */ - public function prefixParameters(): array - { - return [ - [ self::TEST_PREFIX ], - [ '' ], - [ null ] - ]; - } - - private static function makeFlagJson(string $key, int $version, bool $deleted = false): string|bool - { - return json_encode(array( - 'key' => $key, - 'version' => $version, - 'on' => true, - 'prerequisites' => [], - 'salt' => '', - 'targets' => [], - 'rules' => [], - 'fallthrough' => [ - 'variation' => 0, - ], - 'offVariation' => null, - 'variations' => [ - true, - false, - ], - 'deleted' => $deleted - )); - } - - private static function makeSegmentJson(string $key, int $version, bool $deleted = false): string|bool - { - return json_encode(array( - 'key' => $key, - 'version' => $version, - 'included' => array(), - 'excluded' => array(), - 'rules' => [], - 'salt' => '', - 'deleted' => $deleted - )); - } -} diff --git a/tests/Impl/Integrations/PHPRedisFeatureRequesterTest.php b/tests/Impl/Integrations/PHPRedisFeatureRequesterTest.php new file mode 100644 index 0000000..d9e1235 --- /dev/null +++ b/tests/Impl/Integrations/PHPRedisFeatureRequesterTest.php @@ -0,0 +1,331 @@ +client($clientPrefix); + $this->clearExistingData($client); + $fr = $this->makeRequester($client, $prefix); + + $flagKey = 'foo'; + $flagVersion = 10; + $flagJson = self::makeFlagJson($flagKey, $flagVersion); + $this->putSerializedItem($client, $prefix, 'features', $flagKey, $flagVersion, $flagJson); + + $fr = $this->makeRequester($client, $prefix); + $flag = $fr->getFeature($flagKey); + + $this->assertInstanceOf(FeatureFlag::class, $flag); + $this->assertEquals($flagVersion, $flag->getVersion()); + } + + /** + * @dataProvider prefixParameters + */ + public function testGetMissingFeature(?string $clientPrefix, ?string $prefix): void + { + $client = $this->client($clientPrefix); + $this->clearExistingData($client); + $fr = $this->makeRequester($client, $prefix); + + $flag = $fr->getFeature('unavailable'); + $this->assertNull($flag); + } + + /** + * @dataProvider prefixParameters + */ + public function testGetDeletedFeature(?string $clientPrefix, ?string $prefix): void + { + $client = $this->client($clientPrefix); + $this->clearExistingData($client); + $fr = $this->makeRequester($client, $prefix); + + $flagKey = 'foo'; + $flagVersion = 10; + $flagJson = self::makeFlagJson($flagKey, $flagVersion, true); + $this->putSerializedItem($client, $prefix, 'features', $flagKey, $flagVersion, $flagJson); + + $flag = $fr->getFeature($flagKey); + + $this->assertNull($flag); + } + + /** + * @dataProvider prefixParameters + */ + public function testGetAllFeatures(?string $clientPrefix, ?string $prefix): void + { + $client = $this->client($clientPrefix); + $this->clearExistingData($client); + $fr = $this->makeRequester($client, $prefix); + + $flagKey1 = 'foo'; + $flagKey2 = 'bar'; + $flagKey3 = 'deleted'; + $flagVersion = 10; + $flagJson1 = self::makeFlagJson($flagKey1, $flagVersion); + $flagJson2 = self::makeFlagJson($flagKey2, $flagVersion); + $flagJson3 = self::makeFlagJson($flagKey3, $flagVersion, true); + + $this->putSerializedItem($client, $prefix, 'features', $flagKey1, $flagVersion, $flagJson1); + $this->putSerializedItem($client, $prefix, 'features', $flagKey2, $flagVersion, $flagJson2); + $this->putSerializedItem($client, $prefix, 'features', $flagKey3, $flagVersion, $flagJson3); + + $flags = $fr->getAllFeatures(); + + $this->assertEquals(2, count($flags)); + $flag1 = $flags[$flagKey1]; + $this->assertEquals($flagKey1, $flag1->getKey()); + $this->assertEquals($flagVersion, $flag1->getVersion()); + $flag2 = $flags[$flagKey2]; + $this->assertEquals($flagKey2, $flag2->getKey()); + $this->assertEquals($flagVersion, $flag2->getVersion()); + } + + /** + * @dataProvider prefixParameters + */ + public function testAllFeaturesWithEmptyStore(?string $clientPrefix, ?string $prefix): void + { + $client = $this->client($clientPrefix); + $this->clearExistingData($client); + $fr = $this->makeRequester($client, $prefix); + + $flags = $fr->getAllFeatures(); + $this->assertEquals([], $flags); + } + + /** + * @dataProvider prefixParameters + */ + public function testGetSegment(?string $clientPrefix, ?string $prefix): void + { + $client = $this->client($clientPrefix); + $this->clearExistingData($client); + $fr = $this->makeRequester($client, $prefix); + + $segKey = 'foo'; + $segVersion = 10; + $segJson = self::makeSegmentJson($segKey, $segVersion); + $this->putSerializedItem($client, $prefix, 'segments', $segKey, $segVersion, $segJson); + + $segment = $fr->getSegment($segKey); + + $this->assertInstanceOf(Segment::class, $segment); + $this->assertEquals($segVersion, $segment->getVersion()); + } + + /** + * @dataProvider prefixParameters + */ + public function testGetMissingSegment(?string $clientPrefix, ?string $prefix): void + { + $client = $this->client($clientPrefix); + $this->clearExistingData($client); + $fr = $this->makeRequester($client, $prefix); + + $segment = $fr->getSegment('unavailable'); + $this->assertNull($segment); + } + + /** + * @dataProvider prefixParameters + */ + public function testGetDeletedSegment(?string $clientPrefix, ?string $prefix): void + { + $client = $this->client($clientPrefix); + $this->clearExistingData($client); + $fr = $this->makeRequester($client, $prefix); + + $segKey = 'foo'; + $segVersion = 10; + $segJson = self::makeSegmentJson($segKey, $segVersion, true); + $this->putSerializedItem($client, $prefix, 'segments', $segKey, $segVersion, $segJson); + + $segment = $fr->getSegment($segKey); + + $this->assertNull($segment); + } + + public function testPrefixIndependence(): void + { + $prefix1 = 'prefix1'; + $prefix2 = 'prefix2'; + + $client = $this->client(null); + $this->clearExistingData($client); + + $flagKey = 'my-flag'; + $segmentKey = 'my-segment'; + $version0 = 10; + $version1 = 11; + $version2 = 12; + $this->setupForPrefix($client, null, $flagKey, $segmentKey, $version0); + $this->setupForPrefix($client, $prefix1, $flagKey, $segmentKey, $version1); + $this->setupForPrefix($client, $prefix2, $flagKey, $segmentKey, $version2); + + $this->verifyForPrefix($this->makeRequester($client, null), $flagKey, $segmentKey, $version0); + $this->verifyForPrefix($this->makeRequester($client, ''), $flagKey, $segmentKey, $version0); + $this->verifyForPrefix($this->makeRequester($client, $prefix1), $flagKey, $segmentKey, $version1); + $this->verifyForPrefix($this->makeRequester($client, $prefix2), $flagKey, $segmentKey, $version2); + } + + private function clearExistingData(Redis $client): void + { + $client->flushAll(); + } + + private function makeRequester(Redis $client, ?string $prefix): Subsystems\FeatureRequester + { + $factory = PHPRedis::featureRequester($client, ['prefix' => $prefix]); + return $factory('', '', []); + } + + private function putSerializedItem( + Redis $client, + ?string $prefix, + string $namespace, + string $key, + int $version, + string $json + ): void { + $p = self::realPrefix($prefix); + $client->hset("$p:$namespace", $key, $json); + } + + private static function realPrefix(?string $prefix): string + { + if ($prefix === null || $prefix === '') { + return 'launchdarkly'; + } + return $prefix; + } + + + private function setupForPrefix(Redis $client, ?string $prefix, string $flagKey, string $segmentKey, int $flagVersion): void + { + $segmentVersion = $flagVersion * 2; + $this->putSerializedItem( + $client, + $prefix, + 'features', + $flagKey, + $flagVersion, + self::makeFlagJson($flagKey, $flagVersion) + ); + $this->putSerializedItem( + $client, + $prefix, + 'segments', + $segmentKey, + $segmentVersion, + self::makeSegmentJson($flagKey, $segmentVersion) + ); + } + + private function verifyForPrefix(FeatureRequester $fr, string $flagKey, string $segmentKey, int $flagVersion): void + { + $segmentVersion = $flagVersion * 2; + + $flag = $fr->getFeature($flagKey); + $this->assertNotNull($flag); + $this->assertEquals($flagVersion, $flag->getVersion()); + + $flags = $fr->getAllFeatures(); + $this->assertEquals(1, count($flags)); + $this->assertEquals($flagVersion, $flags[$flagKey]->getVersion()); + + $segment = $fr->getSegment($segmentKey); + $this->assertNotNull($segment); + $this->assertEquals($segmentVersion, $segment->getVersion()); + } + + /** + * @return array> + */ + public function prefixParameters(): array + { + return [ + [self::TEST_PREFIX, self::TEST_PREFIX], + [self::TEST_PREFIX, ''], + [self::TEST_PREFIX, null], + + ['', self::TEST_PREFIX], + ['', ''], + ['', null], + + [null, self::TEST_PREFIX], + [null, ''], + [null, null], + ]; + } + + private static function makeFlagJson(string $key, int $version, bool $deleted = false): string|bool + { + return json_encode([ + 'key' => $key, + 'version' => $version, + 'on' => true, + 'prerequisites' => [], + 'salt' => '', + 'targets' => [], + 'rules' => [], + 'fallthrough' => [ + 'variation' => 0, + ], + 'offVariation' => null, + 'variations' => [ + true, + false, + ], + 'deleted' => $deleted + ]); + } + + private static function makeSegmentJson(string $key, int $version, bool $deleted = false): string|bool + { + return json_encode([ + 'key' => $key, + 'version' => $version, + 'included' => [], + 'excluded' => [], + 'rules' => [], + 'salt' => '', + 'deleted' => $deleted + ]); + } + + private function client(?string $prefix): Redis + { + $client = new Redis(); + $client->pconnect( + 'localhost', + 6379, + 0, + self::class, + ); + + if ($prefix !== null) { + $client->setOption(Redis::OPT_PREFIX, $prefix); + } + + return $client; + } +} diff --git a/tests/PHPRedisFeatureRequesterTest.php b/tests/PHPRedisFeatureRequesterTest.php deleted file mode 100644 index c841915..0000000 --- a/tests/PHPRedisFeatureRequesterTest.php +++ /dev/null @@ -1,53 +0,0 @@ -pconnect( - 'localhost', - 6379, - null, - 'RedisFeatureRequesterTest' - ); - } - - protected function clearExistingData($prefix): void - { - $p = self::realPrefix($prefix); - $keys = self::$redisClient->keys("$p:*"); - foreach ($keys as $key) { - self::$redisClient->del($key); - } - } - - protected function makeRequester($prefix) - { - $factory = PHPRedis::featureRequester([ - 'redis_prefix' => $prefix - ]); - 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; - } -} diff --git a/tests/PHPRedisFeatureRequesterWithClientTest.php b/tests/PHPRedisFeatureRequesterWithClientTest.php deleted file mode 100644 index 1234a84..0000000 --- a/tests/PHPRedisFeatureRequesterWithClientTest.php +++ /dev/null @@ -1,70 +0,0 @@ -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) - { - $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; - } -} From e469943d1a04a880fa94f7bf779ede63f0301def Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Tue, 14 Jan 2025 14:58:05 -0500 Subject: [PATCH 3/4] chore: Add missing documentation on big segments store method --- src/LaunchDarkly/Integrations/PHPRedis.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/LaunchDarkly/Integrations/PHPRedis.php b/src/LaunchDarkly/Integrations/PHPRedis.php index de206a5..8c69356 100644 --- a/src/LaunchDarkly/Integrations/PHPRedis.php +++ b/src/LaunchDarkly/Integrations/PHPRedis.php @@ -46,6 +46,15 @@ public static function featureRequester(Redis $client, $options = []): callable } /** + * Configures a big segments store instance backed by Redis. + * + * After calling this method, store its return value in the `store` property of your Big Segment configuration: + * + * $store = LaunchDarkly\Integrations\PHPRedis::bigSegmentsStore(["prefix" => "env1"]); + * $bigSegmentsConfig = new LaunchDarkly\BigSegmentConfig(store: $store); + * $config = ["big_segments" => $bigSegmentsConfig]; + * $client = new LDClient("sdk_key", $config); + * * @param array $options * - `prefix`: a string to be prepended to all database keys; corresponds * to the prefix setting in ld-relay From 4977a058ee226345fc05343f26b353985075d663 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Tue, 14 Jan 2025 15:14:01 -0500 Subject: [PATCH 4/4] chore: Cleanup and strict types --- .github/workflows/ci.yml | 4 ++-- Makefile | 2 +- README.md | 14 ++++++------- phpunit.xml | 12 ++++++++++- .../Integrations/PHPRedisFeatureRequester.php | 18 +++++++++++++++-- src/LaunchDarkly/Integrations/PHPRedis.php | 20 ++++++++++++------- .../PHPRedisBigSegmentsStoreTest.php | 2 ++ .../PHPRedisFeatureRequesterTest.php | 2 ++ 8 files changed, 53 insertions(+), 21 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 69f2d20..510281e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,11 +1,11 @@ name: Run CI on: push: - branches: [ main ] + branches: [ main, 'feat/**' ] paths-ignore: - '**.md' # Do not need to run CI for markdown changes. pull_request: - branches: [ main ] + branches: [ main, 'feat/**' ] paths-ignore: - '**.md' diff --git a/Makefile b/Makefile index 7a91435..4bca0d9 100644 --- a/Makefile +++ b/Makefile @@ -7,7 +7,7 @@ help: #! Show this help message .PHONY: test test: #! Run unit tests - php -d xdebug.mode=coverage vendor/bin/phpunit + php vendor/bin/phpunit .PHONY: coverage coverage: #! Run unit tests with test coverage diff --git a/README.md b/README.md index 0bda576..48848bd 100644 --- a/README.md +++ b/README.md @@ -6,9 +6,9 @@ This library provides a Redis-backed data source for the [LaunchDarkly PHP SDK](https://github.com/launchdarkly/php-server-sdk), replacing the default behavior of querying the LaunchDarkly service endpoints. The underlying Redis client implementation is the [`phpredis`](https://github.com/phpredis/phpredis) extension. If you want to use the Predis package instead, see https://github.com/launchdarkly/php-server-sdk-redis-predis. -The minimum version of the LaunchDarkly PHP SDK for use with this library is 4.0.0. In earlier versions of the SDK, the Redis integrations were bundled in the main SDK package. +The minimum version of the LaunchDarkly PHP SDK for use with this library is 6.4.0. -The minimum PHP version is 7.3. +The minimum PHP version is 8.1. For more information, see [our SDK documentation](https://docs.launchdarkly.com/sdk/features/storing-data). @@ -27,15 +27,13 @@ php composer.phar install launchdarkly/server-sdk-redis-phpredis --save 3. In your SDK configuration code, configure the Redis integration: ```php - $fr = LaunchDarkly\Integrations\PHPRedis::featureRequester([ - "prefix" => "my-key-prefix" - ]); - $config = [ "feature_requester" => $fr ]; + $fr = LaunchDarkly\Integrations\PHPRedis::featureRequester( + $redisClient, ["prefix" => "my-key-prefix"] + ); + $config = ["feature_requester" => $fr]; $client = new LDClient("sdk_key", $config); ``` -By default, the store will try to connect to a local Redis instance on port 6379. You may specify an alternate configuration as described in the API documentation for `PHPRedis::featureRequester`. Make sure the `prefix` option corresponds to the key prefix that is being used by the Relay Proxy. - ## About LaunchDarkly * LaunchDarkly is a continuous delivery platform that provides feature flags as a service and allows developers to iterate quickly and safely. We allow you to easily flag your features and manage them from the LaunchDarkly dashboard. With LaunchDarkly, you can: diff --git a/phpunit.xml b/phpunit.xml index 9ca7909..a349c20 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -1,10 +1,20 @@ - + + + + src + + + + + + + tests diff --git a/src/LaunchDarkly/Impl/Integrations/PHPRedisFeatureRequester.php b/src/LaunchDarkly/Impl/Integrations/PHPRedisFeatureRequester.php index 9d26f79..8b6d934 100644 --- a/src/LaunchDarkly/Impl/Integrations/PHPRedisFeatureRequester.php +++ b/src/LaunchDarkly/Impl/Integrations/PHPRedisFeatureRequester.php @@ -1,5 +1,7 @@ client->hget("$this->prefix:$namespace", $key); + /** @var string|false */ + $result = $this->client->hget("$this->prefix:$namespace", $key); + if ($result === false) { + return null; + } + + return $result; } protected function readItemStringList(string $namespace): ?array { + /** @var ?array|false */ $raw = $this->client->hgetall("$this->prefix:$namespace"); - return $raw ? array_values($raw) : null; + + if ($raw === null || $raw === false) { + return null; + } + + return array_values($raw); } } diff --git a/src/LaunchDarkly/Integrations/PHPRedis.php b/src/LaunchDarkly/Integrations/PHPRedis.php index 8c69356..97e7d34 100644 --- a/src/LaunchDarkly/Integrations/PHPRedis.php +++ b/src/LaunchDarkly/Integrations/PHPRedis.php @@ -1,5 +1,7 @@ "env1" ]); - * $config = [ "feature_requester" => $fr ]; - * $client = new LDClient("sdk_key", $config); + * ```php + * $fr = LaunchDarkly\Integrations\PHPRedis::featureRequester(["prefix" => "env1"]); + * $config = ["feature_requester" => $fr]; + * $client = new LDClient("sdk_key", $config); + * ``` * * For more about using LaunchDarkly with databases, see the * [SDK reference guide](https://docs.launchdarkly.com/sdk/features/storing-data). @@ -50,10 +54,12 @@ public static function featureRequester(Redis $client, $options = []): callable * * After calling this method, store its return value in the `store` property of your Big Segment configuration: * - * $store = LaunchDarkly\Integrations\PHPRedis::bigSegmentsStore(["prefix" => "env1"]); - * $bigSegmentsConfig = new LaunchDarkly\BigSegmentConfig(store: $store); - * $config = ["big_segments" => $bigSegmentsConfig]; - * $client = new LDClient("sdk_key", $config); + * ```php + * $store = LaunchDarkly\Integrations\PHPRedis::bigSegmentsStore(["prefix" => "env1"]); + * $bigSegmentsConfig = new LaunchDarkly\BigSegmentConfig(store: $store); + * $config = ["big_segments" => $bigSegmentsConfig]; + * $client = new LDClient("sdk_key", $config); + * ``` * * @param array $options * - `prefix`: a string to be prepended to all database keys; corresponds diff --git a/tests/Impl/Integrations/PHPRedisBigSegmentsStoreTest.php b/tests/Impl/Integrations/PHPRedisBigSegmentsStoreTest.php index 71c6c8a..fe142e0 100644 --- a/tests/Impl/Integrations/PHPRedisBigSegmentsStoreTest.php +++ b/tests/Impl/Integrations/PHPRedisBigSegmentsStoreTest.php @@ -1,5 +1,7 @@