From 387d05254c09a7049277511cef00f4631f6640e2 Mon Sep 17 00:00:00 2001 From: Benoit C Date: Tue, 22 Aug 2023 15:56:40 +0200 Subject: [PATCH 1/4] Force user agent and update (squash) --- phpunit.xml | 10 +++-- src/Api.php | 62 ++++++++++++++++++++++-------- src/Document.php | 3 +- tests/ApiFoodCacheTest.php | 4 +- tests/ApiFoodTest.php | 3 +- tests/ApiPetTest.php | 2 +- tests/ApiTest.php | 2 +- {src => tests}/FilesystemTrait.php | 2 +- 8 files changed, 60 insertions(+), 28 deletions(-) rename {src => tests}/FilesystemTrait.php (88%) diff --git a/phpunit.xml b/phpunit.xml index d9c590836..bf1d1457e 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -1,9 +1,6 @@ - + - - src - @@ -22,4 +19,9 @@ + + + src + + diff --git a/src/Api.php b/src/Api.php index 43dfee137..877d9511a 100644 --- a/src/Api.php +++ b/src/Api.php @@ -120,6 +120,7 @@ class Api * @param CacheInterface|null $cacheInterface */ public function __construct( + public readonly string $userAgent, private readonly string $currentAPI = 'food', string $geography = 'world', ?LoggerInterface $logger = null, @@ -364,7 +365,14 @@ public function downloadData(string $filePath, string $fileType = 'mongodb') $url = $this->buildUrl('data', self::FILE_TYPE_MAP[$fileType]); try { - $response = $this->httpClient->request('get', $url, ['sink' => $filePath]); + $response = $this->httpClient->request( + 'get', + $url, + [ + 'sink' => $filePath, + 'headers' => $this->getDefaultHeaders() + ] + ); } catch (GuzzleException $guzzleException) { $this->logger->warning(sprintf('OpenFoodFact - fetch - failed - GET : %s', $url), ['exception' => $guzzleException]); $exception = new BadRequestException($guzzleException->getMessage(), $guzzleException->getCode(), $guzzleException); @@ -400,17 +408,13 @@ private function fetch(string $url, bool $isJsonFile = true): array return $cachedResult; } + $data = $this->getDefaultOptions(); - $data = [ - 'on_stats' => function (TransferStats $stats) use (&$realUrl) { - // this function help to find redirection - // On redirect we lost some parameters like page - $realUrl = (string)$stats->getEffectiveUri(); - } - ]; - if ($this->auth) { - $data['auth'] = array_values($this->auth); - } + $data['on_stats'] = function (TransferStats $stats) use (&$realUrl) { + // this function help to find redirection + // On redirect we lost some parameters like page + $realUrl = (string)$stats->getEffectiveUri(); + }; try { $response = $this->httpClient->request('get', $url, $data); @@ -447,10 +451,8 @@ private function fetch(string $url, bool $isJsonFile = true): array */ private function fetchPost(string $url, array $postData, bool $isMultipart = false): array { - $data = []; - if ($this->auth) { - $data['auth'] = array_values($this->auth); - } + $data = $this->getDefaultOptions(); + if ($isMultipart) { foreach ($postData as $key => $value) { $data['multipart'][] = [ @@ -491,7 +493,7 @@ private function fetchPost(string $url, array $postData, bool $isMultipart = fal * This private function generates an url according to the parameters * @param string|null $service * @param string|array|null $resourceType - * @param integer|string|array|null $parameters + * @param int|string|array|null $parameters * @return string the generated url */ private function buildUrl(string $service = null, $resourceType = null, $parameters = null): string @@ -542,7 +544,7 @@ private function buildUrl(string $service = null, $resourceType = null, $paramet $baseUrl = implode('/', array_filter([ $this->geoUrl, $resourceType, - $parameters + is_array($parameters) ? '' : $parameters ], function ($value) { return !empty($value); })); @@ -552,4 +554,30 @@ private function buildUrl(string $service = null, $resourceType = null, $paramet return $baseUrl; } + + /** + * @return array + */ + private function getDefaultOptions(): array + { + $data = [ + 'headers' => $this->getDefaultHeaders(), + ]; + if ($this->auth) { + $data['auth'] = array_values($this->auth); + } + return $data; + } + + /** + * @return array + */ + private function getDefaultHeaders(): array + { + return [ + 'User-Agent' => 'SDK PHP - ' . $this->userAgent, + ]; + } + + } diff --git a/src/Document.php b/src/Document.php index 0e7679548..69f613f24 100644 --- a/src/Document.php +++ b/src/Document.php @@ -20,8 +20,9 @@ class Document /** * Initialization the document and specify from which API it was extract * @param array $data the whole data + * @param string|null $api the api name */ - public function __construct(array $data) + public function __construct(array $data, string $api = null) { $this->recursiveSortArray($data); $this->data = $data; diff --git a/tests/ApiFoodCacheTest.php b/tests/ApiFoodCacheTest.php index a3f065460..bbbc9652c 100644 --- a/tests/ApiFoodCacheTest.php +++ b/tests/ApiFoodCacheTest.php @@ -32,12 +32,12 @@ protected function setUp(): void CURLOPT_FRESH_CONNECT => true, 'defaults' => [ 'headers' => [ - 'CURLOPT_USERAGENT' => 'OFF - PHP - SDK - Unit Test', + CURLOPT_USERAGENT => 'OFF - PHP - SDK - Unit Test', ], ], ]); - $api = new Api('food', 'fr-en', $this->log, $httpClient, $cache); + $api = new Api('SDK Unit test', 'food', 'fr-en', $this->log, $httpClient, $cache); $this->assertInstanceOf(Api::class, $api); $this->api = $api; } diff --git a/tests/ApiFoodTest.php b/tests/ApiFoodTest.php index a3db2134d..67f365273 100644 --- a/tests/ApiFoodTest.php +++ b/tests/ApiFoodTest.php @@ -28,12 +28,13 @@ protected function setUp(): void { $this->log = $this->createMock(NullLogger::class); - $this->api = new Api('food', 'fr-en', $this->log); + $this->api = new Api('SDK Unit test', 'food', 'fr-en', $this->log); $testFolder = 'tests/tmp'; if (file_exists($testFolder)) { rmdir($testFolder); } mkdir($testFolder, 0755); + } public function testApiNotFound(): void diff --git a/tests/ApiPetTest.php b/tests/ApiPetTest.php index ebc9ab519..04604c086 100644 --- a/tests/ApiPetTest.php +++ b/tests/ApiPetTest.php @@ -19,7 +19,7 @@ class ApiPetTest extends TestCase protected function setUp(): void { - $this->api = new Api('pet', 'fr', $this->createMock(NullLogger::class)); + $this->api = new Api('SDK Unit test', 'pet', 'fr', $this->createMock(NullLogger::class)); foreach (glob('tests/images/*') ?: [] as $file) { unlink($file); diff --git a/tests/ApiTest.php b/tests/ApiTest.php index c7fd7248d..29b7f11f1 100644 --- a/tests/ApiTest.php +++ b/tests/ApiTest.php @@ -12,7 +12,7 @@ public function testUploadImageMustThrowAnExceptionForInvalidApi(): void { $this->expectException(BadRequestException::class); $this->expectExceptionMessage('not Available yet'); - $api = new Api('product'); + $api = new Api('SDK Unit test', 'product'); $api->uploadImage('unknown', 'foo', 'bar'); } } diff --git a/src/FilesystemTrait.php b/tests/FilesystemTrait.php similarity index 88% rename from src/FilesystemTrait.php rename to tests/FilesystemTrait.php index 21b6ccd6e..f9f7aadf1 100644 --- a/src/FilesystemTrait.php +++ b/tests/FilesystemTrait.php @@ -4,7 +4,7 @@ trait FilesystemTrait { - public function recursiveDeleteDirectory(string $dir): void + public static function recursiveDeleteDirectory(string $dir): void { if (is_dir($dir)) { $objects = scandir($dir) ?: []; From 1cc05fe4f9f0e96945685adcc11c3ae5ff8edbb9 Mon Sep 17 00:00:00 2001 From: Benoit Colin Date: Wed, 1 May 2024 14:12:10 +0200 Subject: [PATCH 2/4] feat: rebase --- composer.json | 2 +- src/Api.php | 2 ++ src/Document.php | 3 +-- {tests => src}/FilesystemTrait.php | 2 +- tests/ApiFoodCacheTest.php | 11 +++-------- tests/ApiFoodTest.php | 12 ++++++------ tests/ApiPetTest.php | 8 ++++---- tests/ApiTest.php | 2 +- 8 files changed, 19 insertions(+), 23 deletions(-) rename {tests => src}/FilesystemTrait.php (88%) diff --git a/composer.json b/composer.json index 8f5a807f3..ddc61504f 100644 --- a/composer.json +++ b/composer.json @@ -42,7 +42,7 @@ }, "require-dev": { "symfony/cache": "^6.1.3", - "phpunit/phpunit": "^9.5.21", + "phpunit/phpunit": "^10.5", "phpstan/phpstan": "^1.8.2", "phpstan/phpstan-phpunit": "^1.1.1", "friendsofphp/php-cs-fixer": "^3.9.5" diff --git a/src/Api.php b/src/Api.php index 877d9511a..dca17b9b5 100644 --- a/src/Api.php +++ b/src/Api.php @@ -566,6 +566,7 @@ private function getDefaultOptions(): array if ($this->auth) { $data['auth'] = array_values($this->auth); } + return $data; } @@ -574,6 +575,7 @@ private function getDefaultOptions(): array */ private function getDefaultHeaders(): array { + //Force the use of user agent on each http client no matter they come return [ 'User-Agent' => 'SDK PHP - ' . $this->userAgent, ]; diff --git a/src/Document.php b/src/Document.php index 69f613f24..0e7679548 100644 --- a/src/Document.php +++ b/src/Document.php @@ -20,9 +20,8 @@ class Document /** * Initialization the document and specify from which API it was extract * @param array $data the whole data - * @param string|null $api the api name */ - public function __construct(array $data, string $api = null) + public function __construct(array $data) { $this->recursiveSortArray($data); $this->data = $data; diff --git a/tests/FilesystemTrait.php b/src/FilesystemTrait.php similarity index 88% rename from tests/FilesystemTrait.php rename to src/FilesystemTrait.php index f9f7aadf1..21b6ccd6e 100644 --- a/tests/FilesystemTrait.php +++ b/src/FilesystemTrait.php @@ -4,7 +4,7 @@ trait FilesystemTrait { - public static function recursiveDeleteDirectory(string $dir): void + public function recursiveDeleteDirectory(string $dir): void { if (is_dir($dir)) { $objects = scandir($dir) ?: []; diff --git a/tests/ApiFoodCacheTest.php b/tests/ApiFoodCacheTest.php index bbbc9652c..df7711866 100644 --- a/tests/ApiFoodCacheTest.php +++ b/tests/ApiFoodCacheTest.php @@ -3,8 +3,8 @@ namespace OpenFoodFactsTests; use GuzzleHttp; -use OpenFoodFacts\FilesystemTrait; use OpenFoodFacts\Api; +use OpenFoodFacts\FilesystemTrait; use Symfony\Component\Cache\Adapter\FilesystemAdapter; use Symfony\Component\Cache\Psr16Cache; @@ -29,15 +29,10 @@ protected function setUp(): void // "http_errors" => false, // MUST not use as it crashes error handling 'Connection' => 'close', CURLOPT_FORBID_REUSE => true, - CURLOPT_FRESH_CONNECT => true, - 'defaults' => [ - 'headers' => [ - CURLOPT_USERAGENT => 'OFF - PHP - SDK - Unit Test', - ], - ], + CURLOPT_FRESH_CONNECT => true ]); - $api = new Api('SDK Unit test', 'food', 'fr-en', $this->log, $httpClient, $cache); + $api = new Api('Integration test', 'food', 'fr-en', $this->log, $httpClient, $cache); $this->assertInstanceOf(Api::class, $api); $this->api = $api; } diff --git a/tests/ApiFoodTest.php b/tests/ApiFoodTest.php index 67f365273..0da4ed625 100644 --- a/tests/ApiFoodTest.php +++ b/tests/ApiFoodTest.php @@ -2,15 +2,15 @@ namespace OpenFoodFactsTests; -use OpenFoodFacts\FilesystemTrait; -use PHPUnit\Framework\MockObject\MockObject; -use PHPUnit\Framework\TestCase; use OpenFoodFacts\Api; use OpenFoodFacts\Collection; -use OpenFoodFacts\Document\FoodDocument; use OpenFoodFacts\Document; -use OpenFoodFacts\Exception\ProductNotFoundException; +use OpenFoodFacts\Document\FoodDocument; use OpenFoodFacts\Exception\BadRequestException; +use OpenFoodFacts\Exception\ProductNotFoundException; +use OpenFoodFacts\FilesystemTrait; +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; use Psr\Log\NullLogger; class ApiFoodTest extends TestCase @@ -28,7 +28,7 @@ protected function setUp(): void { $this->log = $this->createMock(NullLogger::class); - $this->api = new Api('SDK Unit test', 'food', 'fr-en', $this->log); + $this->api = new Api('Integration test', 'food', 'fr-en', $this->log); $testFolder = 'tests/tmp'; if (file_exists($testFolder)) { rmdir($testFolder); diff --git a/tests/ApiPetTest.php b/tests/ApiPetTest.php index 04604c086..b703f22d1 100644 --- a/tests/ApiPetTest.php +++ b/tests/ApiPetTest.php @@ -2,13 +2,13 @@ namespace OpenFoodFactsTests; -use OpenFoodFacts\FilesystemTrait; -use PHPUnit\Framework\TestCase; use OpenFoodFacts\Api; use OpenFoodFacts\Collection; -use OpenFoodFacts\Document\PetDocument; use OpenFoodFacts\Document; +use OpenFoodFacts\Document\PetDocument; use OpenFoodFacts\Exception\BadRequestException; +use OpenFoodFacts\FilesystemTrait; +use PHPUnit\Framework\TestCase; use Psr\Log\NullLogger; class ApiPetTest extends TestCase @@ -19,7 +19,7 @@ class ApiPetTest extends TestCase protected function setUp(): void { - $this->api = new Api('SDK Unit test', 'pet', 'fr', $this->createMock(NullLogger::class)); + $this->api = new Api('Integration test', 'pet', 'fr', $this->createMock(NullLogger::class)); foreach (glob('tests/images/*') ?: [] as $file) { unlink($file); diff --git a/tests/ApiTest.php b/tests/ApiTest.php index 29b7f11f1..a0b924078 100644 --- a/tests/ApiTest.php +++ b/tests/ApiTest.php @@ -12,7 +12,7 @@ public function testUploadImageMustThrowAnExceptionForInvalidApi(): void { $this->expectException(BadRequestException::class); $this->expectExceptionMessage('not Available yet'); - $api = new Api('SDK Unit test', 'product'); + $api = new Api('Integration test', 'product'); $api->uploadImage('unknown', 'foo', 'bar'); } } From 37daf85241a8d9cb06603a72c337ef9ef92b52c1 Mon Sep 17 00:00:00 2001 From: Benoit Colin Date: Wed, 1 May 2024 14:18:06 +0200 Subject: [PATCH 3/4] fix: phpstan --- src/Api.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Api.php b/src/Api.php index dca17b9b5..5dd223000 100644 --- a/src/Api.php +++ b/src/Api.php @@ -540,7 +540,6 @@ private function buildUrl(string $service = null, $resourceType = null, $paramet $resourceType = implode('/', ['state', 'ingredients-completed']); $parameters = 1; } - /** @phpstan-ignore-next-line */ $baseUrl = implode('/', array_filter([ $this->geoUrl, $resourceType, From 76d289209d72228be296618952a58156c2cad379 Mon Sep 17 00:00:00 2001 From: Benoit Colin Date: Wed, 1 May 2024 14:20:17 +0200 Subject: [PATCH 4/4] fix: update --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4419346f7..f98531337 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,4 +48,4 @@ jobs: - name: Execute phpstan run: vendor/bin/phpstan - name: Execute tests - run: vendor/bin/phpunit --verbose + run: vendor/bin/phpunit