From f0c2b97b769a8f7922053a21300f6ab3f242ca15 Mon Sep 17 00:00:00 2001 From: Roman Dmytrenko Date: Sat, 5 Oct 2024 16:37:28 +0300 Subject: [PATCH] fix(php): add error handler for json serde errors. add throws in the annotations Signed-off-by: Roman Dmytrenko --- flipt-php/composer.json | 3 +- flipt-php/src/Flipt/Client/FliptClient.php | 116 ++++++++++++++++-- .../Models/DefaultVariantEvaluationResult.php | 6 + .../Flipt/Models/VariantEvaluationResult.php | 3 + flipt-php/tests/FliptClientTest.php | 63 +++++++++- 5 files changed, 177 insertions(+), 14 deletions(-) diff --git a/flipt-php/composer.json b/flipt-php/composer.json index 61f5f6a..07d5eba 100644 --- a/flipt-php/composer.json +++ b/flipt-php/composer.json @@ -11,7 +11,8 @@ "homepage": "https://flipt.io", "require": { "php": ">=8.0", - "guzzlehttp/guzzle": "^7" + "guzzlehttp/guzzle": "^7", + "psr/log": "^1.0|^2.0|^3.0" }, "repositories": [ { diff --git a/flipt-php/src/Flipt/Client/FliptClient.php b/flipt-php/src/Flipt/Client/FliptClient.php index a42e7e5..4f6b151 100644 --- a/flipt-php/src/Flipt/Client/FliptClient.php +++ b/flipt-php/src/Flipt/Client/FliptClient.php @@ -7,6 +7,8 @@ use Flipt\Models\VariantEvaluationResult; use Flipt\Models\DefaultBooleanEvaluationResult; use Flipt\Models\DefaultVariantEvaluationResult; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; final class FliptClient @@ -16,8 +18,11 @@ final class FliptClient protected string $namespace; protected string $entityId; protected array $context; + protected LoggerInterface $logger; - + /** + * @param array $context + */ public function __construct(string|Client $host, string $namespace = "default", array $context = [], string $entityId = '', AuthenticationStrategy $authentication = null) { $this->authentication = $authentication; @@ -25,33 +30,96 @@ public function __construct(string|Client $host, string $namespace = "default", $this->context = $context; $this->entityId = $entityId; $this->client = (is_string($host)) ? new Client(['base_uri' => $host]) : $host; + $this->logger = new NullLogger(); } + /** + * Set logger to use + */ + public function setLogger(LoggerInterface $logger) { + $this->logger = $logger; + } /** * Returns the boolean evaluation result + * + * @param array $context + * + * @throws \JsonException if request or response includes invalid json data + * @throws \Psr\Http\Client\ClientExceptionInterface if network or request error occurs */ - public function boolean(string $name, $context = [], $entityId = NULL, $reference = ""): BooleanEvaluationResult + public function boolean(string $name, ?array $context = [], ?string $entityId = null, ?string $reference = ""): BooleanEvaluationResult { $response = $this->apiRequest('/evaluate/v1/boolean', $this->mergeRequestParams($name, $context, $entityId, $reference)); return new DefaultBooleanEvaluationResult($response['flagKey'], $response['enabled'], $response['reason'], $response['requestDurationMillis'], $response['requestId'], $response['timestamp']); } + /** + * Returns the bool result or default + * + * @param string $name - the flag key + * @param bool $fallback - default value in case of error + * @param array $context + * + * @return bool + */ + public function booleanValue(string $name, bool $fallback, ?array $context = [], ?string $entityId = null, ?string $reference = ""): bool + { + try { + return $this->boolean($name, $context, $entityId, $reference)->getEnabled(); + } catch (\JsonException | \Psr\Http\Client\ClientExceptionInterface $e) { + $this->logger->error($e->getMessage()); + } + return $fallback; + } + /** * Returns the variant evaluation result + * + * @param array $context + * + * @throws \JsonException if request or response includes invalid json data + * @throws \Psr\Http\Client\ClientExceptionInterface if network or request error occurs */ - public function variant(string $name, $context = [], $entityId = NULL, $reference = ""): VariantEvaluationResult + public function variant(string $name, ?array $context = [], ?string $entityId = null, ?string $reference = ""): VariantEvaluationResult { $response = $this->apiRequest('/evaluate/v1/variant', $this->mergeRequestParams($name, $context, $entityId, $reference)); return new DefaultVariantEvaluationResult($response['flagKey'], $response['match'], $response['reason'], $response['requestDurationMillis'], $response['requestId'], $response['timestamp'], $response['segmentKeys'], $response['variantKey'], $response['variantAttachment']); } + /** + * Returns the variant evaluation variantKey or default + * + * @param string $name - the flag key + * @param string $fallback - default value in case of error + * @param array $context + * + * @return string + */ + public function variantValue(string $name, string $fallback, ?array $context = [], ?string $entityId = null, ?string $reference = ""): string + { + try { + return $this->variant($name, $context, $entityId, $reference)->getVariantKey(); + } catch (\JsonException | \Psr\Http\Client\ClientExceptionInterface $e) { + $this->logger->error($e->getMessage()); + } + return $fallback; + } + /** * Batch return evaluation requests + * + * @param array $names + * @param array $context + * + * @return array + * + * @throws \JsonException if request or response includes invalid json data + * @throws \Psr\Http\Client\ClientExceptionInterface if network or request error occurs */ - public function batch(array $names, $context = [], $entityId = NULL, $reference = ""): array + public function batch(array $names, $context = [], ?string $entityId = null, ?string $reference = ""): array { $response = $this->apiRequest('/evaluate/v1/batch', [ @@ -81,8 +149,12 @@ public function batch(array $names, $context = [], $entityId = NULL, $reference }, $response['responses']); } - - protected function mergeRequestParams(string $name, $context = [], $entityId = NULL, $reference = "") + /** + * @param array $context + * + * @return array + */ + protected function mergeRequestParams(string $name, $context = [], ?string $entityId = null, ?string $reference = "") { return [ 'context' => array_merge($this->context, $context), @@ -97,6 +169,13 @@ protected function mergeRequestParams(string $name, $context = [], $entityId = N /** * Helper function to perform a guzzle request with the correct headers and body + * + * @param array $body + * + * @return array + * + * @throws \JsonException if request or response includes invalid json data + * @throws \Psr\Http\Client\ClientExceptionInterface if network or request error occurs */ protected function apiRequest(string $path, array $body = [], string $method = 'POST') { @@ -112,15 +191,17 @@ protected function apiRequest(string $path, array $body = [], string $method = ' // execute request $response = $this->client->request($method, $path, [ 'headers' => $headers, - 'body' => json_encode($body, JSON_FORCE_OBJECT), + 'body' => json_encode($body, JSON_FORCE_OBJECT | JSON_THROW_ON_ERROR), ]); - return json_decode($response->getBody(), true); + return json_decode($response->getBody(), true, 512, JSON_THROW_ON_ERROR); } /** * Create a new client with a different namespace + * + * @return FliptClient */ public function withNamespace(string $namespace) { @@ -129,6 +210,10 @@ public function withNamespace(string $namespace) /** * Create a new client with a different context + * + * @param array $context + * + * @return FliptClient */ public function withContext(array $context) { @@ -137,6 +222,8 @@ public function withContext(array $context) /** * Create a new client with a different authentication strategy + * + * @return FliptClient */ public function withAuthentication(AuthenticationStrategy $authentication) { @@ -146,11 +233,17 @@ public function withAuthentication(AuthenticationStrategy $authentication) interface AuthenticationStrategy { + /** + * @param array $headers + * + * @return array + */ public function authenticate(array $headers); } /** * Authenticate with a client token + * * @see https://www.flipt.io/docs/authentication/methods#static-token */ class ClientTokenAuthentication implements AuthenticationStrategy @@ -162,6 +255,9 @@ public function __construct(string $token) $this->token = $token; } + /** + * @inheritDoc + */ public function authenticate(array $headers) { $headers['Authorization'] = 'Bearer ' . $this->token; @@ -171,6 +267,7 @@ public function authenticate(array $headers) /** * Authenticate with a JWT token + * * @see https://www.flipt.io/docs/authentication/methods#json-web-tokens */ class JWTAuthentication implements AuthenticationStrategy @@ -182,6 +279,9 @@ public function __construct(string $token) $this->token = $token; } + /** + * @inheritDoc + */ public function authenticate(array $headers) { $headers['Authorization'] = 'JWT ' . $this->token; diff --git a/flipt-php/src/Flipt/Models/DefaultVariantEvaluationResult.php b/flipt-php/src/Flipt/Models/DefaultVariantEvaluationResult.php index 926c38f..6fdf019 100644 --- a/flipt-php/src/Flipt/Models/DefaultVariantEvaluationResult.php +++ b/flipt-php/src/Flipt/Models/DefaultVariantEvaluationResult.php @@ -18,6 +18,9 @@ final class DefaultVariantEvaluationResult implements VariantEvaluationResult public ?string $variantKey; public ?string $variantAttachment; + /** + * @param array $segmentKeys + */ public function __construct( string $flagKey, bool $match, @@ -70,6 +73,9 @@ public function getTimestamp(): string return $this->timestamp; } + /** + * @return array + */ public function getSegmentKeys(): ?array { return $this->segmentKeys; diff --git a/flipt-php/src/Flipt/Models/VariantEvaluationResult.php b/flipt-php/src/Flipt/Models/VariantEvaluationResult.php index eaf4080..c19a914 100644 --- a/flipt-php/src/Flipt/Models/VariantEvaluationResult.php +++ b/flipt-php/src/Flipt/Models/VariantEvaluationResult.php @@ -12,6 +12,9 @@ public function getReason(): string; public function getRequestDurationMillis(): float; public function getRequestId(): string; public function getTimestamp(): string; + /** + * @return array + */ public function getSegmentKeys(): ?array; public function getVariantKey(): ?string; public function getVariantAttachment(): ?string; diff --git a/flipt-php/tests/FliptClientTest.php b/flipt-php/tests/FliptClientTest.php index c22b9d7..c5a660d 100644 --- a/flipt-php/tests/FliptClientTest.php +++ b/flipt-php/tests/FliptClientTest.php @@ -3,16 +3,20 @@ use PHPUnit\Framework\TestCase; use Flipt\Client\FliptClient; use Flipt\Models\ResponseReasons; +use GuzzleHttp\Handler\MockHandler; +use GuzzleHttp\HandlerStack; +use GuzzleHttp\Psr7\Response; +use GuzzleHttp\Psr7\Request; +use GuzzleHttp\Exception\RequestException; +use GuzzleHttp\Client; +use Psr\Http\Client\ClientExceptionInterface; final class FliptClientTest extends TestCase { - - protected array $history; protected FliptClient $apiClient; public function setUp(): void { - $fliptUrl = getenv('FLIPT_URL'); if (!$fliptUrl) { @@ -24,7 +28,6 @@ public function setUp(): void $this->fail('FLIPT_AUTH_TOKEN environment variable not set'); } - $this->apiClient = new FliptClient($fliptUrl, authentication: new Flipt\Client\ClientTokenAuthentication($authToken)); } @@ -36,6 +39,9 @@ public function testBoolean(): void $this->assertTrue($result->getEnabled()); $this->assertEquals($result->getReason(), ResponseReasons::MATCH_EVALUATION_REASON); $this->assertEquals('flag_boolean', $result->getFlagKey()); + + $value = $this->apiClient->booleanValue('flag_boolean', false, ['fizz' => 'buzz'], 'entity'); + $this->assertTrue($value); } @@ -43,11 +49,58 @@ public function testVariant(): void { $result = $this->apiClient->variant('flag1', ['fizz' => 'buzz'], 'entity'); - $this->assertTrue($result->getMatch()); $this->assertEquals($result->getReason(), ResponseReasons::MATCH_EVALUATION_REASON); $this->assertEquals('flag1', $result->getFlagKey()); $this->assertEquals($result->getSegmentKeys(), ['segment1']); $this->assertEquals($result->getVariantKey(), 'variant1'); + + $value = $this->apiClient->variantValue('flag1', 'fallback', ['fizz' => 'buzz'], 'entity'); + $this->assertEquals($value, 'variant1'); + } + + public function testCommunicationError(): void + { + $mock = new MockHandler([ + new RequestException('Error Communicating with Server', new Request('POST', '/evaluate/v1/boolean')) + ]); + $handlerStack = HandlerStack::create($mock); + $client = new Client(['handler' => $handlerStack]); + $apiClient = new FliptClient($client); + + $this->expectException(ClientExceptionInterface::class); + $this->expectExceptionMessage('Error Communicating with Server'); + $apiClient->boolean('flag1', ['fizz' => 'buzz'], 'entity'); + } + + public function testInvalidJsonData(): void + { + $mock = new MockHandler([ + new Response(200, [], '{"Hello": "invalid json') + ]); + $handlerStack = HandlerStack::create($mock); + $client = new Client(['handler' => $handlerStack]); + $apiClient = new FliptClient($client); + + $this->expectException(JsonException::class); + $this->expectExceptionMessage('Control character error, possibly incorrectly encoded'); + $apiClient->boolean('flag1', ['fizz' => 'buzz'], 'entity'); + } + + public function testFallback(): void + { + $mock = new MockHandler([ + new Response(200, [], '{"Hello": "invalid json'), + new Response(200, [], '{"Hello": "invalid json') + ]); + $handlerStack = HandlerStack::create($mock); + $client = new Client(['handler' => $handlerStack]); + $apiClient = new FliptClient($client); + + $result = $apiClient->booleanValue('flag-b', true, ['fizz' => 'buzz'], 'entity', ''); + $this->assertTrue($result); + + $result = $apiClient->variantValue('flag-e', 'variant-a', ['fizz' => 'buzz'], 'entity', ''); + $this->assertEquals($result, 'variant-a'); } }