diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 128f215..1549a18 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -13,16 +13,14 @@ jobs: steps: - name: Set up PHP - uses: shivammathur/setup-php@1.7.0 + uses: shivammathur/setup-php@v2 with: php-version: '7.2' - coverage: xdebug - ini-values: xdebug.overload_var_dump=1 + coverage: none extensions: mbstring, intl - tools: prestissimo - name: Checkout code - uses: actions/checkout@v2 + uses: actions/checkout@v7 - name: Download dependencies run: composer update --no-interaction --prefer-stable --prefer-lowest --prefer-dist @@ -34,21 +32,21 @@ jobs: name: Build runs-on: ubuntu-latest strategy: + fail-fast: false max-parallel: 10 matrix: - php: ['7.2', '7.3', '7.4', '8.0'] + php: ['7.2', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3', '8.4'] steps: - name: Set up PHP - uses: shivammathur/setup-php@1.7.0 + uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php }} - coverage: xdebug - ini-values: xdebug.overload_var_dump=1 + coverage: none extensions: mbstring, intl - name: Checkout code - uses: actions/checkout@v2 + uses: actions/checkout@v7 - name: Download dependencies run: composer update --no-interaction --prefer-dist diff --git a/.github/workflows/static.yml b/.github/workflows/static.yml index ffedcc6..119468e 100644 --- a/.github/workflows/static.yml +++ b/.github/workflows/static.yml @@ -6,77 +6,77 @@ on: jobs: phpstan: name: PHPStan - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - name: Checkout code - uses: actions/checkout@v2 + uses: actions/checkout@v7 - name: Setup PHP uses: shivammathur/setup-php@v2 with: - php-version: 8.0 + php-version: '8.4' coverage: none - tools: phpstan:0.12.88, cs2pr + tools: phpstan:2.3, cs2pr - name: Download dependencies - uses: ramsey/composer-install@v1 + uses: ramsey/composer-install@v4 - name: PHPStan run: phpstan analyze --no-progress --error-format=checkstyle | cs2pr php-cs-fixer: name: PHP-CS-Fixer - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - name: Checkout code - uses: actions/checkout@v2 + uses: actions/checkout@v7 - name: Setup PHP uses: shivammathur/setup-php@v2 with: - php-version: 8.0 + php-version: '8.4' coverage: none - tools: php-cs-fixer:2.19.0, cs2pr + tools: php-cs-fixer:3.95, cs2pr - name: PHP-CS-Fixer run: php-cs-fixer fix --dry-run --format=checkstyle | cs2pr psalm: name: Psalm - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - name: Checkout code - uses: actions/checkout@v2 + uses: actions/checkout@v7 - name: Setup PHP uses: shivammathur/setup-php@v2 with: - php-version: 8.0 + php-version: '8.4' coverage: none - tools: vimeo/psalm:4.7.2 + tools: psalm:6.20 - name: Download dependencies - uses: ramsey/composer-install@v1 + uses: ramsey/composer-install@v4 - name: Psalm run: psalm --no-progress --output-format=github composer-normalize: name: Composer Normalize - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - name: Setup PHP uses: shivammathur/setup-php@v2 with: - php-version: 7.4 + php-version: '8.4' coverage: none - tools: composer-normalize + tools: composer-normalize:2.56 - name: Checkout code - uses: actions/checkout@v2 + uses: actions/checkout@v7 - name: Normalize run: composer-normalize --dry-run diff --git a/.gitignore b/.gitignore index 3f66f83..c6a1c6d 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,4 @@ /vendor/ composer.lock -.php_cs.cache +.php-cs-fixer.cache .phpunit.result.cache \ No newline at end of file diff --git a/.php_cs b/.php-cs-fixer.dist.php similarity index 73% rename from .php_cs rename to .php-cs-fixer.dist.php index 1ca9819..4612049 100644 --- a/.php_cs +++ b/.php-cs-fixer.dist.php @@ -2,13 +2,13 @@ $finder = PhpCsFixer\Finder::create() ->in(__DIR__) - ->exclude(__DIR__.'/vendor') + ->exclude('vendor') ; -return PhpCsFixer\Config::create() +return (new PhpCsFixer\Config()) ->setRules([ '@Symfony' => true, 'array_syntax' => ['syntax' => 'short'], ]) ->setFinder($finder) -; \ No newline at end of file +; diff --git a/CHANGELOG.md b/CHANGELOG.md index 76df564..c98de06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,20 @@ # Change Log +## Unreleased + +### Added + +- Every request carries a `User-Agent: ipdata-php/` header. The version is the installed version of `ipdata/api-client` as recorded by Composer (`Composer\InstalledVersions`, Composer 2 and newer); without it the header is `ipdata-php`. +- `Ipdata::DEFAULT_BASE_URL` is public, so the trailing `$httpVersion` argument can be passed without repeating the endpoint: `new Ipdata('my_api_key', $httpClient, $psr17Factory, Ipdata::DEFAULT_BASE_URL, '2.0')`. + +### Fixed + +- The static-analysis workflow runs again, on a current runner image with current releases of PHPStan, Psalm, PHP-CS-Fixer and composer-normalize; the test workflow covers PHP 7.2 to 8.4 with current `actions/checkout` and `shivammathur/setup-php` releases and no longer requests the Composer 1 plugin `prestissimo`. +- The PHP-CS-Fixer configuration moved to `.php-cs-fixer.dist.php` (PHP-CS-Fixer 3 file name and API) and excludes `vendor/` correctly; the two remaining coding-style findings in `src/Ipdata.php` are fixed. +- Two PHPStan baseline entries that no longer matched a reported error are removed. +- `composer.json` is normalized. +- `bulkLookup()` throws an `\InvalidArgumentException` when the IP addresses cannot be encoded as JSON (for example invalid UTF-8) instead of failing with a `TypeError` while writing the request body. + ## 0.1.3 ### Added diff --git a/README.md b/README.md index d5d6ab6..432ad6a 100644 --- a/README.md +++ b/README.md @@ -202,7 +202,7 @@ The library is transport-agnostic, so "the platform" is the PSR-18 client you in use Ipdata\ApiClient\Ipdata; use Nyholm\Psr7\Factory\Psr17Factory; - $ipdata = new Ipdata('my_api_key', new Client(), new Psr17Factory(), 'https://api.ipdata.co', '2.0'); + $ipdata = new Ipdata('my_api_key', new Client(), new Psr17Factory(), Ipdata::DEFAULT_BASE_URL, '2.0'); ``` On PHP 8 you can skip the base URL with a named argument: `new Ipdata('my_api_key', new Client(), new Psr17Factory(), httpVersion: '2.0')`. @@ -225,6 +225,8 @@ A few things to keep in mind: * `Ipdata` has no `close()` method because it does not own the connections: they belong to the PSR-18 client you injected and are closed when that client is released, at the latest when the process ends. Timeouts, proxies and other transport settings are configured on that client as well. +* Every request carries a `User-Agent: ipdata-php/` header, where the version is the installed version of + this package as recorded by Composer. ## Available Fields diff --git a/composer.json b/composer.json index ea5db9a..35412c0 100644 --- a/composer.json +++ b/composer.json @@ -1,8 +1,8 @@ { "name": "ipdata/api-client", - "type": "library", "description": "API client for ipdata.co", "license": "MIT", + "type": "library", "authors": [ { "name": "Tobias Nyholm", @@ -17,7 +17,7 @@ "require-dev": { "nyholm/psr7": "^1.2", "php-http/discovery": "^1.7", - "php-http/mock-client": "^1.3", + "php-http/mock-client": "^1.5", "phpunit/phpunit": "^8.5" }, "autoload": { diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 0b9b046..a2276bb 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1,32 +1,26 @@ parameters: ignoreErrors: - - message: "#^Method Ipdata\\\\ApiClient\\\\Ipdata\\:\\:lookup\\(\\) return type has no value type specified in iterable type array\\.$#" + message: '#^Method Ipdata\\ApiClient\\Ipdata\:\:buildLookup\(\) return type has no value type specified in iterable type array\.$#' + identifier: missingType.iterableValue count: 1 path: src/Ipdata.php - - message: "#^Method Ipdata\\\\ApiClient\\\\Ipdata\\:\\:bulkLookup\\(\\) has parameter \\$ips with no value type specified in iterable type array\\.$#" + message: '#^Method Ipdata\\ApiClient\\Ipdata\:\:bulkLookup\(\) return type has no value type specified in iterable type array\.$#' + identifier: missingType.iterableValue count: 1 path: src/Ipdata.php - - message: "#^Method Ipdata\\\\ApiClient\\\\Ipdata\\:\\:bulkLookup\\(\\) return type has no value type specified in iterable type array\\.$#" + message: '#^Method Ipdata\\ApiClient\\Ipdata\:\:lookup\(\) return type has no value type specified in iterable type array\.$#' + identifier: missingType.iterableValue count: 1 path: src/Ipdata.php - - message: "#^Method Ipdata\\\\ApiClient\\\\Ipdata\\:\\:buildLookup\\(\\) has parameter \\$ips with no value type specified in iterable type array\\.$#" - count: 1 - path: src/Ipdata.php - - - - message: "#^Method Ipdata\\\\ApiClient\\\\Ipdata\\:\\:buildLookup\\(\\) return type has no value type specified in iterable type array\\.$#" - count: 1 - path: src/Ipdata.php - - - - message: "#^Method Ipdata\\\\ApiClient\\\\Ipdata\\:\\:parseResponse\\(\\) return type has no value type specified in iterable type array\\.$#" + message: '#^Method Ipdata\\ApiClient\\Ipdata\:\:parseResponse\(\) return type has no value type specified in iterable type array\.$#' + identifier: missingType.iterableValue count: 1 path: src/Ipdata.php diff --git a/psalm.xml b/psalm.xml index 4e9226b..1bba641 100644 --- a/psalm.xml +++ b/psalm.xml @@ -2,6 +2,7 @@ */ class Ipdata { - private const DEFAULT_BASE_URL = 'https://api.ipdata.co'; + public const DEFAULT_BASE_URL = 'https://api.ipdata.co'; public const EU_BASE_URL = 'https://eu-api.ipdata.co'; + /** + * Name of this package on Packagist, used to read the installed version for the User-Agent header. + */ + private const PACKAGE_NAME = 'ipdata/api-client'; + /** * @var string|null */ @@ -48,6 +56,11 @@ class Ipdata */ private $httpVersion; + /** + * @var string + */ + private $userAgent; + /** * Get an instance of the API client. Give it an API key, a PSR-18 client and a PSR-17 request factory. * @@ -60,7 +73,7 @@ class Ipdata * * @param ClientInterface|null $httpClient if null, we will try to use php-http/discovery to find an installed client * @param RequestFactoryInterface|null $requestFactory if null, we will try to use php-http/discovery to find an installed factory - * @param string $baseUrl API endpoint, pass self::EU_BASE_URL to use the EU servers + * @param string $baseUrl API endpoint: self::DEFAULT_BASE_URL, or self::EU_BASE_URL to use the EU servers * @param string|null $httpVersion HTTP protocol version applied to every request, null keeps the request factory's default */ public function __construct(string $apiKey, ?ClientInterface $httpClient = null, ?RequestFactoryInterface $requestFactory = null, string $baseUrl = self::DEFAULT_BASE_URL, ?string $httpVersion = null) @@ -94,6 +107,7 @@ public function __construct(string $apiKey, ?ClientInterface $httpClient = null, $this->requestFactory = $requestFactory; $this->baseUrl = $baseUrl; $this->httpVersion = $httpVersion; + $this->userAgent = self::buildUserAgent(); } /** @@ -111,7 +125,7 @@ public function lookup(string $ip = '', array $fields = []): array $query['fields'] = implode(',', $fields); } - $url = $ip !== '' ? sprintf('%s/%s', $this->baseUrl, $ip) : $this->baseUrl; + $url = '' !== $ip ? sprintf('%s/%s', $this->baseUrl, $ip) : $this->baseUrl; $request = $this->createRequest('GET', sprintf('%s?%s', $url, http_build_query($query))); $response = $this->httpClient->sendRequest($request); @@ -124,6 +138,7 @@ public function lookup(string $ip = '', array $fields = []): array * @param array $ips * @param array $fields * + * @throws \InvalidArgumentException when $ips cannot be encoded as JSON * @throws \Psr\Http\Client\ClientExceptionInterface */ public function bulkLookup(array $ips, array $fields = []): array @@ -136,8 +151,13 @@ public function bulkLookup(array $ips, array $fields = []): array $query['fields'] = implode(',', $fields); } + $body = json_encode($ips); + if (false === $body) { + throw new \InvalidArgumentException(sprintf('Cannot encode the IP addresses as JSON: %s', json_last_error_msg())); + } + $request = $this->createRequest('POST', sprintf('%s/bulk?%s', $this->baseUrl, http_build_query($query))); - $request->getBody()->write(json_encode($ips)); + $request->getBody()->write($body); $request = $request->withHeader('Content-Type', 'application/json'); $response = $this->httpClient->sendRequest($request); @@ -145,7 +165,7 @@ public function bulkLookup(array $ips, array $fields = []): array } /** - * @deprecated Use bulkLookup() instead. + * @deprecated use bulkLookup() instead * * @param array $ips * @param array $fields @@ -158,11 +178,12 @@ public function buildLookup(array $ips, array $fields = []): array } /** - * Create the request through the PSR-17 factory and apply the configured HTTP protocol version, if any. + * Create the request through the PSR-17 factory, add the User-Agent header and apply the configured HTTP + * protocol version, if any. */ private function createRequest(string $method, string $url): RequestInterface { - $request = $this->requestFactory->createRequest($method, $url); + $request = $this->requestFactory->createRequest($method, $url)->withHeader('User-Agent', $this->userAgent); if (null !== $this->httpVersion) { $request = $request->withProtocolVersion($this->httpVersion); } @@ -170,6 +191,21 @@ private function createRequest(string $method, string $url): RequestInterface return $request; } + /** + * Value of the User-Agent header: "ipdata-php/", where the version is the installed version of this + * package as recorded by Composer 2 in vendor/composer/installed.php. When Composer's runtime API is not + * available the header is sent as "ipdata-php". + */ + private static function buildUserAgent(): string + { + $version = null; + if (class_exists(InstalledVersions::class) && InstalledVersions::isInstalled(self::PACKAGE_NAME)) { + $version = InstalledVersions::getPrettyVersion(self::PACKAGE_NAME); + } + + return null === $version ? 'ipdata-php' : 'ipdata-php/'.$version; + } + private function parseResponse(ResponseInterface $response): array { $body = $response->getBody()->__toString(); diff --git a/tests/IpdataTest.php b/tests/IpdataTest.php index b62bddd..16196cb 100644 --- a/tests/IpdataTest.php +++ b/tests/IpdataTest.php @@ -4,6 +4,7 @@ namespace Ipdata\ApiClient\Tests; +use Composer\InstalledVersions; use Http\Mock\Client as MockClient; use Ipdata\ApiClient\Ipdata; use Nyholm\Psr7\Factory\Psr17Factory; @@ -378,6 +379,60 @@ public function testNewFieldNamesCanBeRequested() $this->assertEquals('api-key=secret_key&fields=is_satellite,is_anycast', urldecode($requests[1]->getUri()->getQuery())); } + public function testBulkLookupRejectsIpsThatCannotBeEncodedAsJson() + { + $httpClient = new MockClient(); + $ipdata = $this->createIpdata($httpClient); + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Cannot encode the IP addresses as JSON'); + try { + // invalid UTF-8 cannot be encoded by json_encode() + $ipdata->bulkLookup(["\xB1\x31"]); + } finally { + // nothing was sent + $this->assertCount(0, $httpClient->getRequests()); + } + } + + public function testDefaultBaseUrlConstantIsPublic() + { + $this->assertSame('https://api.ipdata.co', Ipdata::DEFAULT_BASE_URL); + $this->assertTrue((new \ReflectionClassConstant(Ipdata::class, 'DEFAULT_BASE_URL'))->isPublic()); + + $httpClient = new MockClient(); + $httpClient->addResponse($this->createResponse()); + // the constant lets callers pass the trailing $httpVersion without repeating the default endpoint + $ipdata = new Ipdata('secret_key', $httpClient, new Psr17Factory(), Ipdata::DEFAULT_BASE_URL, '2.0'); + $ipdata->lookup('8.8.8.8'); + + $request = $httpClient->getLastRequest(); + $this->assertSame('api.ipdata.co', $request->getUri()->getHost()); + $this->assertSame('/8.8.8.8', $request->getUri()->getPath()); + $this->assertSame('2.0', $request->getProtocolVersion()); + } + + public function testUserAgentHeaderIsSentWithEveryRequest() + { + $httpClient = new MockClient(); + $httpClient->addResponse($this->createResponse()); + $httpClient->addResponse($this->createResponse()); + $ipdata = $this->createIpdata($httpClient); + + $ipdata->lookup('8.8.8.8'); + $ipdata->bulkLookup(['8.8.8.8']); + + $requests = $httpClient->getRequests(); + $this->assertCount(2, $requests); + // the version is the one Composer recorded for this package in vendor/composer/installed.php + $expected = 'ipdata-php/'.InstalledVersions::getPrettyVersion('ipdata/api-client'); + $this->assertRegExp('#^ipdata-php/\S+$#', $expected); + $this->assertSame($expected, $requests[0]->getHeaderLine('User-Agent')); + $this->assertSame($expected, $requests[1]->getHeaderLine('User-Agent')); + // the bulk request keeps its Content-Type next to the User-Agent header + $this->assertSame('application/json', $requests[1]->getHeaderLine('Content-Type')); + } + private function createIpdata(ClientInterface $httpClient): Ipdata { return new Ipdata('secret_key', $httpClient, new Psr17Factory());