From 0c6b9f780024450ccfd488eaa5af590e096cf652 Mon Sep 17 00:00:00 2001 From: "v.bartusevicius" Date: Wed, 8 Nov 2023 09:45:05 +0200 Subject: [PATCH 1/5] Do not store invalid response --- src/Repository/DefaultUnleashRepository.php | 1 + .../DefaultUnleashRepositoryTest.php | 44 +++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/src/Repository/DefaultUnleashRepository.php b/src/Repository/DefaultUnleashRepository.php index 46c7dca9..2f173159 100755 --- a/src/Repository/DefaultUnleashRepository.php +++ b/src/Repository/DefaultUnleashRepository.php @@ -109,6 +109,7 @@ public function getFeatures(): iterable $response = $this->httpClient->sendRequest($request); if ($response->getStatusCode() === 200) { $data = (string) $response->getBody(); + json_decode($data, true, 512, JSON_THROW_ON_ERROR); $this->setLastValidState($data); } else { throw new HttpResponseException("Invalid status code: '{$response->getStatusCode()}'"); diff --git a/tests/Repository/DefaultUnleashRepositoryTest.php b/tests/Repository/DefaultUnleashRepositoryTest.php index 12377b85..0a177ca7 100755 --- a/tests/Repository/DefaultUnleashRepositoryTest.php +++ b/tests/Repository/DefaultUnleashRepositoryTest.php @@ -7,6 +7,8 @@ use GuzzleHttp\Middleware; use GuzzleHttp\Psr7\HttpFactory; use GuzzleHttp\Psr7\Request; +use GuzzleHttp\Psr7\Response; +use JsonException; use LogicException; use Psr\Http\Message\ResponseInterface; use Symfony\Component\Cache\Adapter\ArrayAdapter; @@ -15,6 +17,7 @@ use Unleash\Client\Bootstrap\JsonSerializableBootstrapProvider; use Unleash\Client\Configuration\UnleashConfiguration; use Unleash\Client\DTO\Feature; +use Unleash\Client\Enum\CacheKey; use Unleash\Client\Event\FetchingDataFailedEvent; use Unleash\Client\Event\UnleashEvents; use Unleash\Client\Exception\HttpResponseException; @@ -424,4 +427,45 @@ public function testFallbackStaleCacheDifferentHandlers() $this->expectException(HttpResponseException::class); $repository->getFeatures(); } + + public function testGetFeaturesCorruptedBodyIsNotStored() + { + $this->mockHandler->append(new Response( + 200, + ['Content-Type' => 'application/json'], + '{"version":1,"features":[' + )); + + $cache = $this->getRealCache(); + $failCount = 0; + + $eventDispatcher = new EventDispatcher(); + $eventDispatcher->addListener( + UnleashEvents::FETCHING_DATA_FAILED, + function (FetchingDataFailedEvent $event) use (&$failCount): void { + self::assertInstanceOf(JsonException::class, $event->getException()); + ++$failCount; + } + ); + + $repository = new DefaultUnleashRepository( + new Client([ + 'handler' => $this->handlerStack, + ]), + new HttpFactory(), + (new UnleashConfiguration('', '', '')) + ->setCache($cache) + ->setFetchingEnabled(true) + ->setTtl(5) + ->setBootstrapProvider(new JsonSerializableBootstrapProvider(['features' => []])) + ->setEventDispatcher($eventDispatcher) + ); + + $features = $repository->getFeatures(); + $lastResponse = $cache->get(CacheKey::FEATURES_RESPONSE); + + self::assertEmpty($features); + self::assertNull($lastResponse); + self::assertEquals(1, $failCount); + } } From 11d98760b3313694f3731a75c91c6b744d21764c Mon Sep 17 00:00:00 2001 From: "v.bartusevicius" Date: Wed, 8 Nov 2023 14:59:10 +0200 Subject: [PATCH 2/5] Optimize JSON decoding, make PHP7.2 compatible --- src/Repository/DefaultUnleashRepository.php | 30 ++++++++++++------- .../DefaultUnleashRepositoryTest.php | 2 +- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/Repository/DefaultUnleashRepository.php b/src/Repository/DefaultUnleashRepository.php index 2f173159..81c6725a 100755 --- a/src/Repository/DefaultUnleashRepository.php +++ b/src/Repository/DefaultUnleashRepository.php @@ -89,8 +89,9 @@ public function getFeatures(): iterable { $features = $this->getCachedFeatures(); if ($features === null) { + $data = null; if (!$this->configuration->isFetchingEnabled()) { - if (!$data = $this->getBootstrappedResponse()) { + if (!$rawData = $this->getBootstrappedResponse()) { throw new LogicException('Fetching of Unleash api is disabled but no bootstrap is provided'); } } else { @@ -108,9 +109,15 @@ public function getFeatures(): iterable try { $response = $this->httpClient->sendRequest($request); if ($response->getStatusCode() === 200) { - $data = (string) $response->getBody(); - json_decode($data, true, 512, JSON_THROW_ON_ERROR); - $this->setLastValidState($data); + $rawData = (string) $response->getBody(); + $data = json_decode($rawData, true); + if (($lastError = json_last_error()) !== JSON_ERROR_NONE) { + throw new InvalidValueException( + sprintf("JsonException: '%s'", json_last_error_msg()), + $lastError + ); + } + $this->setLastValidState($rawData); } else { throw new HttpResponseException("Invalid status code: '{$response->getStatusCode()}'"); } @@ -119,10 +126,10 @@ public function getFeatures(): iterable new FetchingDataFailedEvent($exception), UnleashEvents::FETCHING_DATA_FAILED, ); - $data = $this->getLastValidState(); + $rawData = $this->getLastValidState(); } - $data ??= $this->getBootstrappedResponse(); - if ($data === null) { + $rawData ??= $this->getBootstrappedResponse(); + if ($rawData === null) { throw new HttpResponseException(sprintf( 'Got invalid response code when getting features and no default bootstrap provided: %s', isset($response) ? $response->getStatusCode() : 'unknown response status code' @@ -130,6 +137,10 @@ public function getFeatures(): iterable } } + if ($data === null) { + $data = json_decode($rawData, true); + } + $features = $this->parseFeatures($data); $this->setCache($features); } @@ -172,12 +183,9 @@ private function setCache(array $features): void * * @return array */ - private function parseFeatures(string $rawBody): array + private function parseFeatures(array $body): array { $features = []; - $body = json_decode($rawBody, true, 512, JSON_THROW_ON_ERROR); - assert(is_array($body)); - $globalSegments = $this->parseSegments($body['segments'] ?? []); if (!isset($body['features']) || !is_array($body['features'])) { diff --git a/tests/Repository/DefaultUnleashRepositoryTest.php b/tests/Repository/DefaultUnleashRepositoryTest.php index 0a177ca7..202e192d 100755 --- a/tests/Repository/DefaultUnleashRepositoryTest.php +++ b/tests/Repository/DefaultUnleashRepositoryTest.php @@ -443,7 +443,7 @@ public function testGetFeaturesCorruptedBodyIsNotStored() $eventDispatcher->addListener( UnleashEvents::FETCHING_DATA_FAILED, function (FetchingDataFailedEvent $event) use (&$failCount): void { - self::assertInstanceOf(JsonException::class, $event->getException()); + self::assertInstanceOf(InvalidValueException::class, $event->getException()); ++$failCount; } ); From 1d69c33aeab90769639cd550c8d307b8c7a10f3f Mon Sep 17 00:00:00 2001 From: "v.bartusevicius" Date: Wed, 8 Nov 2023 14:59:31 +0200 Subject: [PATCH 3/5] typo --- tests/Repository/DefaultUnleashRepositoryTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Repository/DefaultUnleashRepositoryTest.php b/tests/Repository/DefaultUnleashRepositoryTest.php index 202e192d..c9554bec 100755 --- a/tests/Repository/DefaultUnleashRepositoryTest.php +++ b/tests/Repository/DefaultUnleashRepositoryTest.php @@ -8,7 +8,6 @@ use GuzzleHttp\Psr7\HttpFactory; use GuzzleHttp\Psr7\Request; use GuzzleHttp\Psr7\Response; -use JsonException; use LogicException; use Psr\Http\Message\ResponseInterface; use Symfony\Component\Cache\Adapter\ArrayAdapter; From c0dd231af4758e3b993f713cb8826af7c3490b4e Mon Sep 17 00:00:00 2001 From: "v.bartusevicius" Date: Wed, 8 Nov 2023 16:17:26 +0200 Subject: [PATCH 4/5] typo --- src/Repository/DefaultUnleashRepository.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Repository/DefaultUnleashRepository.php b/src/Repository/DefaultUnleashRepository.php index 81c6725a..037a8b54 100755 --- a/src/Repository/DefaultUnleashRepository.php +++ b/src/Repository/DefaultUnleashRepository.php @@ -180,6 +180,7 @@ private function setCache(array $features): void /** * @throws JsonException + * @param array $body * * @return array */ From 6abab96a4c73354e18b1046047e78208ba4e66b1 Mon Sep 17 00:00:00 2001 From: "v.bartusevicius" Date: Fri, 10 Nov 2023 09:15:11 +0200 Subject: [PATCH 5/5] fix code style --- src/Repository/DefaultUnleashRepository.php | 24 ++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/src/Repository/DefaultUnleashRepository.php b/src/Repository/DefaultUnleashRepository.php index 037a8b54..873654cd 100755 --- a/src/Repository/DefaultUnleashRepository.php +++ b/src/Repository/DefaultUnleashRepository.php @@ -55,6 +55,24 @@ * type:string, * value: string, * } + * @phpstan-type StrategyArray array{ + * constraints?: array, + * variants?: array, + * segments?: array, + * name: string, + * parameters: array, + * } + * @phpstan-type SegmentArray array{ + * id: int, + * constraints: array, + * } + * @phpstan-type FeatureArray array{ + * strategies: array, + * variants: array, + * name: string, + * enabled: bool, + * impressionData?: bool, + * } */ final readonly class DefaultUnleashRepository implements UnleashRepository { @@ -141,6 +159,7 @@ public function getFeatures(): iterable $data = json_decode($rawData, true); } + assert(is_array($data)); $features = $this->parseFeatures($data); $this->setCache($features); } @@ -179,8 +198,7 @@ private function setCache(array $features): void } /** - * @throws JsonException - * @param array $body + * @param array{segments?: array, features?: array} $body * * @return array */ @@ -263,7 +281,7 @@ private function setLastValidState(string $data): void } /** - * @param array}> $segmentsRaw + * @param array $segmentsRaw * * @return array */