Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: force user agent #53

Merged
merged 4 commits into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,4 @@ jobs:
- name: Execute phpstan
run: vendor/bin/phpstan
- name: Execute tests
run: vendor/bin/phpunit --verbose
run: vendor/bin/phpunit
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
10 changes: 6 additions & 4 deletions phpunit.xml
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd" backupGlobals="false" colors="true" bootstrap="vendor/autoload.php">
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/10.3/phpunit.xsd" backupGlobals="false" colors="true" bootstrap="vendor/autoload.php">
<coverage>
<include>
<directory>src</directory>
</include>
<report>
<clover outputFile="./build/logs/clover.xml"/>
<html outputDirectory="./build/coverrage"/>
Expand All @@ -22,4 +19,9 @@
<junit outputFile="./build/logs/junit.xml"/>
<testdoxHtml outputFile="./build/logstestdox.html"/>
</logging>
<source>
<include>
<directory>src</directory>
</include>
</source>
</phpunit>
65 changes: 47 additions & 18 deletions src/Api.php
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ class Api
* @param CacheInterface|null $cacheInterface
*/
public function __construct(
public readonly string $userAgent,
Copy link
Collaborator Author

@Benoit382 Benoit382 May 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could add the user agent at the end BUT nobody will use or define it.

private readonly string $currentAPI = 'food',
string $geography = 'world',
?LoggerInterface $logger = null,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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'][] = [
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -538,11 +540,10 @@ 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,
$parameters
is_array($parameters) ? '' : $parameters
], function ($value) {
return !empty($value);
}));
Expand All @@ -552,4 +553,32 @@ 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
{
//Force the use of user agent on each http client no matter they come
return [
'User-Agent' => 'SDK PHP - ' . $this->userAgent,
];
}


}
11 changes: 3 additions & 8 deletions tests/ApiFoodCacheTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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('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;
}
Expand Down
13 changes: 7 additions & 6 deletions tests/ApiFoodTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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('Integration test', 'food', 'fr-en', $this->log);
$testFolder = 'tests/tmp';
if (file_exists($testFolder)) {
rmdir($testFolder);
}
mkdir($testFolder, 0755);

}

public function testApiNotFound(): void
Expand Down
8 changes: 4 additions & 4 deletions tests/ApiPetTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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('Integration test', 'pet', 'fr', $this->createMock(NullLogger::class));

foreach (glob('tests/images/*') ?: [] as $file) {
unlink($file);
Expand Down
2 changes: 1 addition & 1 deletion tests/ApiTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public function testUploadImageMustThrowAnExceptionForInvalidApi(): void
{
$this->expectException(BadRequestException::class);
$this->expectExceptionMessage('not Available yet');
$api = new Api('product');
$api = new Api('Integration test', 'product');
$api->uploadImage('unknown', 'foo', 'bar');
}
}
Loading