From db37cb9f09769674ee1728890231b7fb297e5944 Mon Sep 17 00:00:00 2001 From: calliostro Date: Fri, 18 Sep 2026 20:53:13 +0200 Subject: [PATCH] Release v4.1.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Support Guzzle 7 & 8 and drop legacy Guzzle 6.5 - Add built-in rate limit (429) and 503 retry resilience with exponential backoff and Retry-After support - Upgrade PHPStan to 2.x (Level 8) and refine internal type declarations - Upgrade GitHub Actions to Node 24 compatible action versions and Ubuntu 24.04 - Expand CI test matrix for PHP 8.1–8.6 and both Guzzle 7 & 8 - Rename PHP-CS-Fixer configuration to .php-cs-fixer.dist.php - Harmonize README format, acknowledgments, and composer metadata --- .github/workflows/ci.yml | 64 ++++-- .gitignore | 1 + .php-cs-fixer.php => .php-cs-fixer.dist.php | 13 +- CHANGELOG.md | 21 ++ README.md | 156 ++++++++----- composer.json | 8 +- phpstan.neon.dist | 9 + resources/service.php | 2 +- src/ConfigCache.php | 5 +- src/DiscogsClient.php | 2 + src/DiscogsClientFactory.php | 95 +++++++- tests/Integration/AuthenticationTest.php | 18 +- tests/Integration/ClientWorkflowTest.php | 1 - tests/Integration/IntegrationTestCase.php | 16 ++ tests/Unit/ConfigCacheTest.php | 3 - tests/Unit/DiscogsClientFactoryTest.php | 229 +++++++++++++++++++- tests/Unit/DiscogsClientTest.php | 45 ++-- tests/Unit/HeaderSecurityTest.php | 6 +- tests/Unit/ProductionRealisticTest.php | 4 - tests/Unit/SecurityTest.php | 4 - tests/Unit/UnitTestCase.php | 17 +- 21 files changed, 580 insertions(+), 139 deletions(-) rename .php-cs-fixer.php => .php-cs-fixer.dist.php (56%) create mode 100644 phpstan.neon.dist diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2c55cf0..19bb6c8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,7 +16,7 @@ on: jobs: test: - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 strategy: fail-fast: false @@ -33,6 +33,8 @@ jobs: allowed-to-fail: false - php: '8.5' allowed-to-fail: false + - php: '8.6' + allowed-to-fail: true # Development stability tests - php: '8.4' @@ -41,14 +43,42 @@ jobs: - php: '8.5' stability: 'dev' allowed-to-fail: true + - php: '8.6' + stability: 'dev' + allowed-to-fail: true + + # Guzzle 7.0 backwards compatibility tests + - php: '8.1' + guzzle: '7' + allowed-to-fail: false + - php: '8.4' + guzzle: '7' + allowed-to-fail: false + - php: '8.5' + guzzle: '7' + allowed-to-fail: false + - php: '8.6' + guzzle: '7' + allowed-to-fail: true + + # Guzzle 8.0 compatibility tests + - php: '8.4' + guzzle: '8' + allowed-to-fail: false + - php: '8.5' + guzzle: '8' + allowed-to-fail: false + - php: '8.6' + guzzle: '8' + allowed-to-fail: true - name: "PHP ${{ matrix.php }}${{ matrix.stability && format(' | {0}', matrix.stability) || '' }}" + name: "PHP ${{ matrix.php }}${{ matrix.guzzle && format(' | Guzzle {0}', matrix.guzzle) || '' }}${{ matrix.stability && format(' | {0}', matrix.stability) || '' }}" continue-on-error: ${{ matrix.allowed-to-fail }} steps: - name: Checkout code - uses: actions/checkout@v4 + uses: actions/checkout@v7 - name: Setup PHP uses: shivammathur/setup-php@v2 @@ -63,7 +93,7 @@ jobs: run: echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT - name: Cache composer dependencies - uses: actions/cache@v4 + uses: actions/cache@v6 with: path: ${{ steps.composer-cache.outputs.dir }} key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.json') }} @@ -76,7 +106,14 @@ jobs: composer config prefer-stable true - name: Install dependencies - run: composer install --prefer-dist --no-interaction --no-progress + run: | + if [ "${{ matrix.guzzle }}" = "7" ]; then + composer require "guzzlehttp/guzzle:^7.0" --with-all-dependencies --prefer-dist --no-interaction --no-progress + elif [ "${{ matrix.guzzle }}" = "8" ]; then + composer require "guzzlehttp/guzzle:^8.0" --with-all-dependencies --prefer-dist --no-interaction --no-progress + else + composer install --prefer-dist --no-interaction --no-progress + fi - name: Validate composer.json and composer.lock run: composer validate --strict @@ -101,12 +138,12 @@ jobs: composer test-integration -- --testdox code-quality: - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 name: Code Quality Checks steps: - name: Checkout code - uses: actions/checkout@v4 + uses: actions/checkout@v7 - name: Setup PHP uses: shivammathur/setup-php@v2 @@ -121,7 +158,7 @@ jobs: run: echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT - name: Cache composer dependencies - uses: actions/cache@v4 + uses: actions/cache@v6 with: path: ${{ steps.composer-cache.outputs.dir }} key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.json') }} @@ -134,12 +171,12 @@ jobs: run: composer validate --strict coverage: - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 name: Code Coverage steps: - name: Checkout code - uses: actions/checkout@v4 + uses: actions/checkout@v7 - name: Setup PHP uses: shivammathur/setup-php@v2 @@ -154,7 +191,7 @@ jobs: run: echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT - name: Cache composer dependencies - uses: actions/cache@v4 + uses: actions/cache@v6 with: path: ${{ steps.composer-cache.outputs.dir }} key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.json') }} @@ -167,9 +204,10 @@ jobs: run: composer test-coverage - name: Upload coverage to Codecov - uses: codecov/codecov-action@v4 + uses: codecov/codecov-action@v7 with: - file: ./coverage.xml + files: ./coverage.xml flags: unittests name: codecov-umbrella fail_ci_if_error: false + diff --git a/.gitignore b/.gitignore index c92bde5..d62183e 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,7 @@ composer.lock # PHP CS Fixer .php-cs-fixer.cache +.php-cs-fixer.php # Coverage reports coverage/ diff --git a/.php-cs-fixer.php b/.php-cs-fixer.dist.php similarity index 56% rename from .php-cs-fixer.php rename to .php-cs-fixer.dist.php index 0502928..beb43ec 100644 --- a/.php-cs-fixer.php +++ b/.php-cs-fixer.dist.php @@ -1,9 +1,14 @@ in(__DIR__ . '/src') ->in(__DIR__ . '/tests') - ->exclude('vendor'); + ->exclude('vendor') + ->exclude('coverage') + ->exclude('.phpunit.cache') + ->name('*.php'); $config = new PhpCsFixer\Config(); $config->setFinder($finder) @@ -11,9 +16,7 @@ '@PSR12' => true, '@PSR12:risky' => true, ]) - ->setRiskyAllowed(true); - -// @phpstan-ignore-next-line Method exists but not detected by static analysis -$config->setUnsupportedPhpVersionAllowed(true); + ->setRiskyAllowed(true) + ->setUnsupportedPhpVersionAllowed(true); return $config; diff --git a/CHANGELOG.md b/CHANGELOG.md index 5187de8..74a8549 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,27 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [4.1.0] - 2026-09-18 + +### Added + +- Support for `guzzlehttp/guzzle` 8.0 alongside 7.0 (`^7.0 || ^8.0`). +- Compatibility testing and CI matrix coverage for PHP 8.1–8.6 and both Guzzle 7 & 8. +- Built-in retry resilience for Discogs rate limits (`429` and `503`) with exponential backoff and `Retry-After` header support. + +### Changed + +- Upgraded PHPStan to 2.x (Level 8) and GitHub Actions to Node 24 compatible runners. +- Updated default User-Agent version to `4.1.0`. + +### Fixed + +- Fixed documentation example for `search()` named parameter (`q` instead of `query`) ([#4](https://github.com/calliostro/php-discogs-api/pull/4) by [@JamieBradders](https://github.com/JamieBradders)). + +### Removed + +- Dropped legacy `guzzlehttp/guzzle` 6.5 constraint. + ## [4.0.0](https://github.com/calliostro/php-discogs-api/releases/tag/v4.0.0) – 2025-12-01 ### πŸš€ Complete Library Redesign – v4.0 is a Fresh Start diff --git a/README.md b/README.md index 59da01e..affe3f1 100644 --- a/README.md +++ b/README.md @@ -1,16 +1,16 @@ -# ⚑ Discogs API Client for PHP 8.1+ – Lightweight with Maximum Developer Comfort +# Discogs API Client for PHP 8.1+ [![Package Version](https://img.shields.io/packagist/v/calliostro/php-discogs-api.svg)](https://packagist.org/packages/calliostro/php-discogs-api) [![Total Downloads](https://img.shields.io/packagist/dt/calliostro/php-discogs-api.svg)](https://packagist.org/packages/calliostro/php-discogs-api) [![License](https://poser.pugx.org/calliostro/php-discogs-api/license)](https://packagist.org/packages/calliostro/php-discogs-api) [![PHP Version](https://img.shields.io/badge/php-%5E8.1-blue.svg)](https://php.net) -[![Guzzle](https://img.shields.io/badge/guzzle-%5E6.5%7C%5E7.0-orange.svg)](https://docs.guzzlephp.org/) +[![Guzzle](https://img.shields.io/badge/guzzle-%5E7.0%20%7C%7C%20%5E8.0-orange.svg)](https://docs.guzzlephp.org/) [![CI](https://github.com/calliostro/php-discogs-api/actions/workflows/ci.yml/badge.svg)](https://github.com/calliostro/php-discogs-api/actions/workflows/ci.yml) [![Code Coverage](https://codecov.io/gh/calliostro/php-discogs-api/graph/badge.svg?token=0SV4IXE9V1)](https://codecov.io/gh/calliostro/php-discogs-api) [![PHPStan Level](https://img.shields.io/badge/PHPStan-level%208-brightgreen.svg)](https://phpstan.org/) [![Code Style](https://img.shields.io/badge/code%20style-PSR12-brightgreen.svg)](https://github.com/FriendsOfPHP/PHP-CS-Fixer) -> **πŸš€ MINIMAL YET POWERFUL!** Focused, lightweight Discogs API client β€” as compact as possible while maintaining modern PHP comfort and clean APIs. +A lightweight, modern PHP client for the [Discogs API](https://www.discogs.com/developers/), supporting database queries, marketplace, user collection, wantlist, and full OAuth flows for PHP 8.1+. ## πŸ“¦ Installation @@ -33,11 +33,15 @@ composer require calliostro/php-discogs-api **Symfony Users:** For easier integration, there's also a [Symfony Bundle](https://github.com/calliostro/discogs-bundle) available. +--- + ## πŸš€ Quick Start -**Public data (no registration needed):** +### Public Data (No Registration Needed) ```php +use Calliostro\Discogs\DiscogsClientFactory; + $discogs = DiscogsClientFactory::create(); $artist = $discogs->getArtist(5590213); // Billie Eilish @@ -45,9 +49,11 @@ $release = $discogs->getRelease(19929817); // Olivia Rodrigo - Sour $label = $discogs->getLabel(2311); // Interscope Records ``` -**Search with consumer credentials:** +### Search with Consumer Credentials ```php +use Calliostro\Discogs\DiscogsClientFactory; + $discogs = DiscogsClientFactory::createWithConsumerCredentials('key', 'secret'); // Positional parameters (traditional) @@ -55,7 +61,7 @@ $results = $discogs->search('Billie Eilish', 'artist'); $releases = $discogs->listArtistReleases(4470662, 'year', 'desc', 50); // Named parameters (PHP 8.0+, recommended for clarity) -$results = $discogs->search(query: 'Taylor Swift', type: 'release'); +$results = $discogs->search(q: 'Taylor Swift', type: 'release'); $releases = $discogs->listArtistReleases( artistId: 4470662, sort: 'year', @@ -64,10 +70,12 @@ $releases = $discogs->listArtistReleases( ); ``` -**Your collections (personal token):** +### Your Collections (Personal Token) ```php -$discogs = DiscogsClientFactory::createWithPersonalAccessToken('token'); +use Calliostro\Discogs\DiscogsClientFactory; + +$discogs = DiscogsClientFactory::createWithPersonalAccessToken('key', 'secret', 'token'); $collection = $discogs->listCollectionFolders('your-username'); $wantlist = $discogs->getUserWantlist('your-username'); @@ -80,80 +88,100 @@ $discogs->addToCollection( ); ``` -**Multi-user apps (OAuth):** +### Multi-User Apps (OAuth 1.0a) ```php +use Calliostro\Discogs\DiscogsClientFactory; + $discogs = DiscogsClientFactory::createWithOAuth('key', 'secret', 'oauth_token', 'oauth_secret'); $identity = $discogs->getIdentity(); ``` +--- + ## ✨ Key Features -- **Simple Setup** – Works immediately with public data, easy authentication for advanced features -- **Complete API Coverage** – All 60 Discogs API endpoints supported -- **Clean Parameter API** – Natural method calls: `getArtist(123)` with named parameter support -- **Lightweight Focus** – Minimal codebase with only essential dependencies -- **Modern PHP Comfort** – Full IDE support, type safety, PHPStan Level 8 without bloat -- **Secure Authentication** – Full OAuth and Personal Access Token support -- **Well Tested** – 100% test coverage, PSR-12 compliant -- **Future-Ready** – PHP 8.1–8.5 compatible (beta/dev testing) -- **Pure Guzzle** – Modern HTTP client, no custom transport layers +- **Simple Setup** – Works immediately with public data, easy authentication for advanced features. +- **Complete API Coverage** – All 60 Discogs API endpoints supported. +- **Built-in Resilience** – Automatic retries on `429` rate limits and `503 Service Unavailable` with exponential backoff and `Retry-After` header support. +- **Clean Parameter API** – Natural method calls: `getArtist(123)` with named parameter support. +- **Lightweight Focus** – Minimal codebase with only essential dependencies (Guzzle 7 or 8). +- **Modern PHP Comfort** – Full IDE support, type safety, PHPStan Level 8 without bloat. +- **Secure Authentication** – Full OAuth 1.0a and Personal Access Token support. +- **Battle-Tested** – 100% test coverage, PSR-12 compliant. +- **Future-Ready** – PHP 8.1–8.6 compatible (beta/dev testing). +- **Pure Guzzle** – Modern HTTP client, no custom transport layers. + +--- ## 🎡 All Discogs API Methods as Direct Calls -- **Database Methods** – search(), getArtist(), listArtistReleases(), getRelease(), updateUserReleaseRating(), deleteUserReleaseRating(), getUserReleaseRating(), getCommunityReleaseRating(), getReleaseStats(), getMaster(), listMasterVersions(), getLabel(), listLabelReleases() -- **Marketplace Methods** – getUserInventory(), getMarketplaceListing(), createMarketplaceListing(), updateMarketplaceListing(), deleteMarketplaceListing(), getMarketplaceFee(), getMarketplaceFeeByCurrency(), getMarketplacePriceSuggestions(), getMarketplaceStats(), getMarketplaceOrder(), getMarketplaceOrders(), updateMarketplaceOrder(), getMarketplaceOrderMessages(), addMarketplaceOrderMessage() -- **Inventory Export Methods** – createInventoryExport(), listInventoryExports(), getInventoryExport(), downloadInventoryExport() -- **Inventory Upload Methods** – addInventoryUpload(), changeInventoryUpload(), deleteInventoryUpload(), listInventoryUploads(), getInventoryUpload() -- **User Identity Methods** – getIdentity(), getUser(), updateUser(), listUserSubmissions(), listUserContributions() -- **User Collection Methods** – listCollectionFolders(), getCollectionFolder(), createCollectionFolder(), updateCollectionFolder(), deleteCollectionFolder(), listCollectionItems(), getCollectionItemsByRelease(), addToCollection(), updateCollectionItem(), removeFromCollection(), getCustomFields(), setCustomFields(), getCollectionValue() -- **User Wantlist Methods** – getUserWantlist(), addToWantlist(), updateWantlistItem(), removeFromWantlist() -- **User Lists Methods** – getUserLists(), getUserList() +- **Database Methods** – `search()`, `getArtist()`, `listArtistReleases()`, `getRelease()`, `updateUserReleaseRating()`, `deleteUserReleaseRating()`, `getUserReleaseRating()`, `getCommunityReleaseRating()`, `getReleaseStats()`, `getMaster()`, `listMasterVersions()`, `getLabel()`, `listLabelReleases()` +- **Marketplace Methods** – `getUserInventory()`, `getMarketplaceListing()`, `createMarketplaceListing()`, `updateMarketplaceListing()`, `deleteMarketplaceListing()`, `getMarketplaceFee()`, `getMarketplaceFeeByCurrency()`, `getMarketplacePriceSuggestions()`, `getMarketplaceStats()`, `getMarketplaceOrder()`, `getMarketplaceOrders()`, `updateMarketplaceOrder()`, `getMarketplaceOrderMessages()`, `addMarketplaceOrderMessage()` +- **Inventory Export Methods** – `createInventoryExport()`, `listInventoryExports()`, `getInventoryExport()`, `downloadInventoryExport()` +- **Inventory Upload Methods** – `addInventoryUpload()`, `changeInventoryUpload()`, `deleteInventoryUpload()`, `listInventoryUploads()`, `getInventoryUpload()` +- **User Identity Methods** – `getIdentity()`, `getUser()`, `updateUser()`, `listUserSubmissions()`, `listUserContributions()` +- **User Collection Methods** – `listCollectionFolders()`, `getCollectionFolder()`, `createCollectionFolder()`, `updateCollectionFolder()`, `deleteCollectionFolder()`, `listCollectionItems()`, `getCollectionItemsByRelease()`, `addToCollection()`, `updateCollectionItem()`, `removeFromCollection()`, `getCustomFields()`, `setCustomFields()`, `getCollectionValue()` +- **User Wantlist Methods** – `getUserWantlist()`, `addToWantlist()`, `updateWantlistItem()`, `removeFromWantlist()` +- **User Lists Methods** – `getUserLists()`, `getUserList()` + +*All Discogs API endpoints are supported with clean documentation β€” see [Discogs API Documentation](https://www.discogs.com/developers/) for complete method reference.* -*All Discogs API endpoints are supported with clean documentation β€” see [Discogs API Documentation](https://www.discogs.com/developers/) for complete method reference* +> [!NOTE] +> Some endpoints require special permissions (seller accounts, data ownership). -> πŸ’‘ **Note:** Some endpoints require special permissions (seller accounts, data ownership). +--- ## πŸ“‹ Requirements -- **php** ^8.1 -- **guzzlehttp/guzzle** ^6.5 || ^7.0 +- **PHP** `^8.1` +- **guzzlehttp/guzzle** `^7.0 || ^8.0` + +--- ## βš™οΈ Configuration -### Configuration +### Rate Limiting & Retries -**Simple (works out of the box):** +Discogs enforces rate limits (25 requests/min for unauthenticated requests, 60 requests/min for authenticated requests) and returns `429 Too Many Requests` (or `503 Service Unavailable`) when busy. By default (`auto_retry => true`, `max_retries => 3`), the client automatically retries `429` and `503` responses with intelligent exponential backoff and respects the `Retry-After` header. + +You can customize or disable retries: ```php use Calliostro\Discogs\DiscogsClientFactory; -$discogs = DiscogsClientFactory::create(); +// Custom retry count +$discogs = DiscogsClientFactory::create([ + 'auto_retry' => true, // Automatically wait and retry on 429/503 (default: true) + 'max_retries' => 5, // Maximum number of retry attempts (default: 3) +]); + +// Disable automatic retries (e.g. in tests or to handle exceptions immediately) +$discogs = DiscogsClientFactory::create([ + 'auto_retry' => false, +]); ``` -**Advanced (middleware, custom options, etc.):** +### Advanced (Custom Guzzle handler, timeouts, headers) ```php use Calliostro\Discogs\DiscogsClientFactory; -use GuzzleHttp\{HandlerStack, Middleware}; - -$handler = HandlerStack::create(); -$handler->push(Middleware::retry( - fn ($retries, $request, $response) => $retries < 3 && $response?->getStatusCode() === 429, - fn ($retries) => 1000 * 2 ** ($retries + 1) // Rate limit handling -)); $discogs = DiscogsClientFactory::create([ 'timeout' => 30, - 'handler' => $handler, 'headers' => [ 'User-Agent' => 'MyApp/1.0 (+https://myapp.com)', - ] + ], + 'auto_retry' => true, + 'max_retries' => 3, ]); ``` -> πŸ’‘ **Note:** By default, the client uses `DiscogsClient/4.0.0 +https://github.com/calliostro/php-discogs-api` as User-Agent. You can override this by setting custom headers as shown above. +> [!NOTE] +> By default, the client uses `DiscogsClient/4.1.0 +https://github.com/calliostro/php-discogs-api` as User-Agent. You can override this by setting custom headers as shown above. + +--- ## πŸ” Authentication @@ -170,7 +198,7 @@ Get credentials at [Discogs Developer Settings](https://www.discogs.com/settings ### Complete OAuth Flow Example -**Step 1: authorize.php** - Redirect user to Discogs +#### Step 1: authorize.php – Redirect user to Discogs ```php getIdentity(); echo "Hello " . $identity['username']; ``` +--- + +## πŸ§ͺ Development & Testing Guide + +See [DEVELOPMENT.md](DEVELOPMENT.md) for detailed setup instructions, test suite commands, static analysis, and contribution guidelines. + +--- + ## 🀝 Contributing -Contributions are welcome! See [DEVELOPMENT.md](DEVELOPMENT.md) for detailed setup instructions, testing guide, and development workflow. +Contributions are welcome! Please ensure all tests pass and coding standards are maintained: + +```bash +composer cs-fix +composer analyse +composer test +``` + +--- ## πŸ“„ License -MIT License – see [LICENSE](LICENSE) file. +MIT License – see the [LICENSE](LICENSE) file for details. -## πŸ™ Acknowledgments +--- + +## βš–οΈ Disclaimer -- [Discogs](https://www.discogs.com/) for the excellent API -- [Guzzle](https://docs.guzzlephp.org/) for an HTTP client -- Previous PHP Discogs implementations for inspiration +Discogs is a registered trademark of Zink Media, LLC. This project is an independent, unofficial open-source library and is not affiliated with, endorsed by, or sponsored by Discogs or Zink Media, LLC. --- -> ⭐ **Star this repo if you find it useful!** +## πŸ™ Acknowledgments + +- [Discogs](https://www.discogs.com/) for providing the comprehensive database and API. +- [Guzzle](https://docs.guzzlephp.org/) for the rock-solid HTTP transport. +- Previous PHP Discogs implementations for inspiration. +- Sister projects: [`calliostro/spotify-client`](https://github.com/calliostro/spotify-client), [`calliostro/musicbrainz-client`](https://github.com/calliostro/musicbrainz-client), and [`calliostro/lastfm-client`](https://github.com/calliostro/lastfm-client). + diff --git a/composer.json b/composer.json index c1b6432..17f73d5 100644 --- a/composer.json +++ b/composer.json @@ -1,6 +1,6 @@ { "name": "calliostro/php-discogs-api", - "description": "Lightweight Discogs API client for PHP 8.1+ with modern developer comfort β€” Clean parameter API and minimal dependencies", + "description": "Lightweight Discogs API client for PHP 8.1+ with built-in resilience and minimal dependencies.", "type": "library", "keywords": [ "php", @@ -32,11 +32,11 @@ ], "require": { "php": "^8.1", - "guzzlehttp/guzzle": "^6.5 || ^7.0" + "guzzlehttp/guzzle": "^7.0 || ^8.0" }, "require-dev": { "phpunit/phpunit": "^10.0", - "phpstan/phpstan": "^1.0", + "phpstan/phpstan": "^2.0", "friendsofphp/php-cs-fixer": "^3.0" }, "config": { @@ -65,7 +65,7 @@ "test-coverage-all": "vendor/bin/phpunit --testsuite=\"All Tests\" --coverage-html coverage --coverage-clover coverage.xml", "cs": "vendor/bin/php-cs-fixer fix --dry-run --diff --verbose", "cs-fix": "vendor/bin/php-cs-fixer fix --verbose", - "analyse": "phpstan analyse src/ tests/ --level=8" + "analyse": "vendor/bin/phpstan analyse" }, "minimum-stability": "stable", "prefer-stable": true diff --git a/phpstan.neon.dist b/phpstan.neon.dist new file mode 100644 index 0000000..eb68831 --- /dev/null +++ b/phpstan.neon.dist @@ -0,0 +1,9 @@ +parameters: + level: 8 + paths: + - src + - tests + tmpDir: .phpunit.cache/phpstan + treatPhpDocTypesAsCertain: false + ignoreErrors: + - identifier: method.alreadyNarrowedType diff --git a/resources/service.php b/resources/service.php index 530a2cd..fffc1bb 100644 --- a/resources/service.php +++ b/resources/service.php @@ -610,7 +610,7 @@ 'base_uri' => 'https://api.discogs.com/', 'timeout' => 30, 'headers' => [ - 'User-Agent' => 'DiscogsClient/4.0.0 +https://github.com/calliostro/php-discogs-api', + 'User-Agent' => 'DiscogsClient/4.1.0 +https://github.com/calliostro/php-discogs-api', 'Accept' => 'application/json', ], ], diff --git a/src/ConfigCache.php b/src/ConfigCache.php index fe17290..3071b5b 100644 --- a/src/ConfigCache.php +++ b/src/ConfigCache.php @@ -29,8 +29,11 @@ private function __construct() public static function get(): array { if (self::$config === null) { - self::$config = require __DIR__ . '/../resources/service.php'; + /** @var array $config */ + $config = require __DIR__ . '/../resources/service.php'; + self::$config = $config; } + return self::$config; } diff --git a/src/DiscogsClient.php b/src/DiscogsClient.php index bf23a73..c30c3c5 100644 --- a/src/DiscogsClient.php +++ b/src/DiscogsClient.php @@ -163,7 +163,9 @@ private function buildParamsFromArguments(string $method, array $arguments): arr return []; } + /** @var list $parameterNames */ $parameterNames = array_keys($this->config['operations'][$operationName]['parameters']); + /** @var array $params */ $params = []; // Check if we have named parameters (associative array with string keys) diff --git a/src/DiscogsClientFactory.php b/src/DiscogsClientFactory.php index 48a2426..2b49038 100644 --- a/src/DiscogsClientFactory.php +++ b/src/DiscogsClientFactory.php @@ -6,16 +6,22 @@ use Exception; use GuzzleHttp\Client as GuzzleClient; +use GuzzleHttp\Exception\BadResponseException; +use GuzzleHttp\Exception\ConnectException; +use GuzzleHttp\HandlerStack; +use GuzzleHttp\Middleware; +use Psr\Http\Message\RequestInterface; +use Psr\Http\Message\ResponseInterface; /** - * Simple factory for creating Discogs clients with proper authentication + * Simple factory for creating Discogs clients with proper authentication and rate-limit handling */ final class DiscogsClientFactory { /** * Create a basic unauthenticated Discogs client * - * @param array|GuzzleClient $optionsOrClient + * @param array|GuzzleClient $optionsOrClient Client options (timeout, auto_retry, max_retries, etc.) or pre-configured Guzzle client */ public static function create(array|GuzzleClient $optionsOrClient = []): DiscogsClient { @@ -24,12 +30,15 @@ public static function create(array|GuzzleClient $optionsOrClient = []): Discogs return new DiscogsClient($optionsOrClient); } + $options = $optionsOrClient; + self::configureHandler($options); + $config = ConfigCache::get(); // Merge user options with base configuration - $clientOptions = array_merge($optionsOrClient, [ + $clientOptions = array_merge([ 'base_uri' => $config['baseUrl'], - ]); + ], $options); return new DiscogsClient(new GuzzleClient($clientOptions)); } @@ -93,12 +102,14 @@ public static function createWithOAuth( */ private static function createClientWithAuth(string $authHeader, array $optionsOrClient): DiscogsClient { + self::configureHandler($optionsOrClient); + $config = ConfigCache::get(); // Merge user options but ALWAYS override the Authorization header for security - $clientOptions = array_merge($optionsOrClient, [ + $clientOptions = array_merge([ 'base_uri' => $config['baseUrl'], - ]); + ], $optionsOrClient); // Ensure our authentication headers take priority over user-provided ones $clientOptions['headers'] = array_merge( @@ -157,4 +168,76 @@ public static function createWithPersonalAccessToken( return self::createClientWithAuth($authHeader, $optionsOrClient); } + + /** + * Configures the Guzzle HandlerStack with retry middleware in client options. + * + * @param array $options + */ + private static function configureHandler(array &$options): void + { + if (isset($options['handler']) && $options['handler'] instanceof HandlerStack) { + return; + } + + $handler = $options['handler'] ?? null; + $stack = $handler !== null ? HandlerStack::create($handler) : HandlerStack::create(); + + $autoRetry = (bool) ($options['auto_retry'] ?? true); + $maxRetries = (int) ($options['max_retries'] ?? 3); + + if ($autoRetry && $maxRetries > 0) { + $stack->push(Middleware::retry( + static function ( + int $retries, + RequestInterface $request, + ?ResponseInterface $response = null, + mixed $reason = null, + ) use ($maxRetries): bool { + if ($retries >= $maxRetries) { + return false; + } + + if ($reason instanceof ConnectException) { + return true; + } + + if ($response === null && $reason instanceof BadResponseException) { + $response = $reason->getResponse(); + } + + return $response !== null && in_array($response->getStatusCode(), [429, 503], true); + }, + $options['retry_delay'] ?? static fn (int $retries, ?ResponseInterface $response = null): int => self::defaultRetryDelay($retries, $response), + ), 'discogs_retry'); + } + + $options['handler'] = $stack; + } + + /** + * Calculates the retry delay in milliseconds. + * Respects Retry-After header (seconds or HTTP-date) if provided, + * otherwise applies exponential backoff (1s, 2s, etc.). + */ + private static function defaultRetryDelay(int $retries, ?ResponseInterface $response = null): int + { + if ($response !== null && $response->hasHeader('Retry-After')) { + $retryAfter = $response->getHeaderLine('Retry-After'); + if (is_numeric($retryAfter) && (int) $retryAfter > 0) { + return (int) $retryAfter * 1000; + } + + $time = strtotime($retryAfter); + if ($time !== false) { + $diff = $time - time(); + if ($diff > 0) { + return $diff * 1000; + } + } + } + + // Exponential backoff: 1000ms for 1st retry, 2000ms for 2nd retry, etc. + return 1000 * (2 ** ($retries - 1)); + } } diff --git a/tests/Integration/AuthenticationTest.php b/tests/Integration/AuthenticationTest.php index a890e6c..acbad64 100644 --- a/tests/Integration/AuthenticationTest.php +++ b/tests/Integration/AuthenticationTest.php @@ -44,8 +44,9 @@ public function testPersonalAccessTokenSendsCorrectHeaders(): void // Make a request that requires authentication $result = $client->search('Taylor Swift', 'artist'); + $this->assertIsArray($container); $this->assertCount(1, $container); - $request = $container[0]['request']; + $request = $this->getHistoryRequest($container, 0); $this->assertTrue($request->hasHeader('Authorization')); $authHeader = $request->getHeaderLine('Authorization'); @@ -53,13 +54,11 @@ public function testPersonalAccessTokenSendsCorrectHeaders(): void $this->assertStringContainsString('test-personal-token', $authHeader); // Verify the response was properly decoded - $this->assertIsArray($result); $this->assertArrayHasKey('results', $result); } /** * @param array $data - * @throws Exception If test setup or execution fails */ private function jsonEncode(array $data): string { @@ -97,8 +96,9 @@ public function testOAuthSendsCorrectHeaders(): void // Make a request that requires OAuth $result = $client->getIdentity(); + $this->assertIsArray($container); $this->assertCount(1, $container); - $request = $container[0]['request']; + $request = $this->getHistoryRequest($container, 0); $this->assertTrue($request->hasHeader('Authorization')); $authHeader = $request->getHeaderLine('Authorization'); @@ -109,7 +109,6 @@ public function testOAuthSendsCorrectHeaders(): void $this->assertStringContainsString('oauth_signature="test-consumer-secret&test-token-secret"', $authHeader); // Verify the response was properly decoded - $this->assertIsArray($result); $this->assertArrayHasKey('username', $result); $this->assertEquals('testuser', $result['username']); } @@ -140,8 +139,9 @@ public function testPersonalAccessTokenWorksWithCollectionEndpoints(): void $result = $client->listCollectionFolders('testuser'); + $this->assertIsArray($container); $this->assertCount(1, $container); - $request = $container[0]['request']; + $request = $this->getHistoryRequest($container, 0); $authHeader = $request->getHeaderLine('Authorization'); $this->assertValidPersonalTokenHeader($authHeader); $this->assertStringContainsString('personal-token', $authHeader); @@ -178,8 +178,9 @@ public function testOAuthWorksWithMarketplaceEndpoints(): void $result = $client->getMarketplaceOrders('All'); + $this->assertIsArray($container); $this->assertCount(1, $container); - $request = $container[0]['request']; + $request = $this->getHistoryRequest($container, 0); $authHeader = $request->getHeaderLine('Authorization'); $this->assertValidOAuthHeader($authHeader); $this->assertStringContainsString('oauth_token="access-token"', $authHeader); @@ -208,8 +209,9 @@ public function testUnauthenticatedClientDoesNotSendAuthHeaders(): void $result = $client->getArtist('139250'); + $this->assertIsArray($container); $this->assertCount(1, $container); - $request = $container[0]['request']; + $request = $this->getHistoryRequest($container, 0); $this->assertFalse($request->hasHeader('Authorization')); $this->assertValidArtistResponse($result); diff --git a/tests/Integration/ClientWorkflowTest.php b/tests/Integration/ClientWorkflowTest.php index 8277513..7c96d23 100644 --- a/tests/Integration/ClientWorkflowTest.php +++ b/tests/Integration/ClientWorkflowTest.php @@ -56,7 +56,6 @@ public function testCompleteWorkflowWithFactoryAndApiCalls(): void * Helper method to safely encode JSON for Response body * * @param array $data - * @throws Exception If test setup or execution fails */ private function jsonEncode(array $data): string { diff --git a/tests/Integration/IntegrationTestCase.php b/tests/Integration/IntegrationTestCase.php index 47c44ae..c660887 100644 --- a/tests/Integration/IntegrationTestCase.php +++ b/tests/Integration/IntegrationTestCase.php @@ -102,6 +102,22 @@ protected function assertValidPersonalTokenHeader(string $authHeader): void $this->assertStringNotContainsString('secret=', $authHeader); } + /** + * Helper to safely extract recorded request from Guzzle history container + * + * @param array|\ArrayAccess $container + */ + protected function getHistoryRequest(array|\ArrayAccess $container, int $index = 0): \Psr\Http\Message\RequestInterface + { + $this->assertArrayHasKey($index, $container); + $entry = $container[$index]; + $this->assertIsArray($entry); + $this->assertArrayHasKey('request', $entry); + $this->assertInstanceOf(\Psr\Http\Message\RequestInterface::class, $entry['request']); + + return $entry['request']; + } + /** * Override PHPUnit's runTest to add automatic retry on rate limiting * This uses reflection to access the private runTest method diff --git a/tests/Unit/ConfigCacheTest.php b/tests/Unit/ConfigCacheTest.php index 6d408d2..207295c 100644 --- a/tests/Unit/ConfigCacheTest.php +++ b/tests/Unit/ConfigCacheTest.php @@ -56,9 +56,6 @@ public function testConstructorIsPrivateToPreventInstantiation(): void $this->assertNotNull($constructor); $this->assertTrue($constructor->isPrivate()); - - // Ensure the constructor method is defined (even if empty) - $this->assertTrue(method_exists(ConfigCache::class, '__construct')); } public function testCannotInstantiateConfigCache(): void diff --git a/tests/Unit/DiscogsClientFactoryTest.php b/tests/Unit/DiscogsClientFactoryTest.php index 45ff7f3..25b0ed2 100644 --- a/tests/Unit/DiscogsClientFactoryTest.php +++ b/tests/Unit/DiscogsClientFactoryTest.php @@ -9,12 +9,18 @@ use Calliostro\Discogs\DiscogsClientFactory; use Exception; use GuzzleHttp\Client; +use GuzzleHttp\Exception\ClientException; +use GuzzleHttp\Exception\ConnectException; +use GuzzleHttp\Exception\ServerException; use GuzzleHttp\Handler\MockHandler; use GuzzleHttp\HandlerStack; use GuzzleHttp\Middleware; +use GuzzleHttp\Psr7\Request; use GuzzleHttp\Psr7\Response; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\UsesClass; +use Psr\Http\Message\ResponseInterface; +use ReflectionMethod; #[CoversClass(DiscogsClientFactory::class)] #[UsesClass(DiscogsClient::class)] @@ -128,9 +134,11 @@ public function testCreateWithOAuthAddsAuthorizationHeader(): void $client->getArtist(1); // Should have one request with an auth header + $this->assertIsArray($container); $this->assertCount(1, $container); - $this->assertTrue($container[0]['request']->hasHeader('Authorization')); - $authHeader = $container[0]['request']->getHeaderLine('Authorization'); + $request = $this->getHistoryRequest($container, 0); + $this->assertTrue($request->hasHeader('Authorization')); + $authHeader = $request->getHeaderLine('Authorization'); $this->assertValidOAuthHeader($authHeader); $this->assertStringContainsString('oauth_consumer_key="consumer_key"', $authHeader); $this->assertStringContainsString('oauth_token="token"', $authHeader); @@ -159,9 +167,11 @@ public function testCreateWithPersonalAccessTokenAddsAuthorizationHeader(): void $client->getArtist(1); // Should have one request with an auth header + $this->assertIsArray($container); $this->assertCount(1, $container); - $this->assertTrue($container[0]['request']->hasHeader('Authorization')); - $authHeader = $container[0]['request']->getHeaderLine('Authorization'); + $request = $this->getHistoryRequest($container, 0); + $this->assertTrue($request->hasHeader('Authorization')); + $authHeader = $request->getHeaderLine('Authorization'); $this->assertValidPersonalTokenHeader($authHeader); $this->assertStringContainsString('token=personal_token', $authHeader); } @@ -193,9 +203,11 @@ public function testCreateWithConsumerCredentialsAddsAuthorizationHeader(): void $client->getArtist(1); // Should have one request with an auth header + $this->assertIsArray($container); $this->assertCount(1, $container); - $this->assertTrue($container[0]['request']->hasHeader('Authorization')); - $authHeader = $container[0]['request']->getHeaderLine('Authorization'); + $request = $this->getHistoryRequest($container, 0); + $this->assertTrue($request->hasHeader('Authorization')); + $authHeader = $request->getHeaderLine('Authorization'); $this->assertStringContainsString('Discogs', $authHeader); $this->assertStringContainsString('key=consumer_key', $authHeader); $this->assertStringContainsString('secret=consumer_secret', $authHeader); @@ -226,4 +238,209 @@ public function testConfigLoadingFromFresh(): void $this->assertIsArray($config); $this->assertArrayHasKey('baseUrl', $config); } + + public function testRetryMiddlewareRetriesOn503AndSucceeds(): void + { + $mock = new MockHandler([ + new Response(503, ['Content-Type' => 'application/json'], '{"message": "The Discogs server is currently busy."}'), + new Response(200, ['Content-Type' => 'application/json'], '{"id": 4470662, "name": "Billie Eilish"}'), + ]); + + $client = DiscogsClientFactory::create([ + 'handler' => $mock, + 'retry_delay' => static fn (): int => 0, + ]); + $result = $client->getArtist(4470662); + + $this->assertSame('Billie Eilish', $result['name']); + $this->assertSame(0, $mock->count()); + } + + public function testRetryMiddlewareRetriesOn429AndSucceeds(): void + { + $mock = new MockHandler([ + new Response(429, ['Content-Type' => 'application/json'], '{"message": "You are making requests too quickly."}'), + new Response(200, ['Content-Type' => 'application/json'], '{"id": 4470662, "name": "Billie Eilish"}'), + ]); + + $client = DiscogsClientFactory::create([ + 'handler' => $mock, + 'retry_delay' => static fn (): int => 0, + ]); + $result = $client->getArtist(4470662); + + $this->assertSame('Billie Eilish', $result['name']); + $this->assertSame(0, $mock->count()); + } + + public function testRetryMiddlewareRetriesOnConnectException(): void + { + $mock = new MockHandler([ + new ConnectException('Connection timed out', new Request('GET', 'https://api.discogs.com/artists/4470662')), + new Response(200, ['Content-Type' => 'application/json'], '{"id": 4470662, "name": "Billie Eilish"}'), + ]); + + $client = DiscogsClientFactory::create([ + 'handler' => $mock, + 'retry_delay' => static fn (): int => 0, + ]); + $result = $client->getArtist(4470662); + + $this->assertSame('Billie Eilish', $result['name']); + $this->assertSame(0, $mock->count()); + } + + public function testRetryMiddlewareRetriesOnServerException(): void + { + $request = new Request('GET', 'https://api.discogs.com/artists/4470662'); + $response503 = new Response(503, ['Content-Type' => 'application/json'], '{"message": "busy"}'); + $mock = new MockHandler([ + new ServerException('Server error', $request, $response503), + new Response(200, ['Content-Type' => 'application/json'], '{"id": 4470662, "name": "Billie Eilish"}'), + ]); + + $client = DiscogsClientFactory::create([ + 'handler' => $mock, + 'retry_delay' => static fn (): int => 0, + ]); + $result = $client->getArtist(4470662); + + $this->assertSame('Billie Eilish', $result['name']); + $this->assertSame(0, $mock->count()); + } + + public function testRetryMiddlewareFailsWhenRetriesExhausted(): void + { + $mock = new MockHandler([ + new Response(503, ['Content-Type' => 'application/json'], '{"message": "The Discogs server is busy."}'), + new Response(503, ['Content-Type' => 'application/json'], '{"message": "The Discogs server is busy."}'), + new Response(503, ['Content-Type' => 'application/json'], '{"message": "The Discogs server is busy."}'), + ]); + + $client = DiscogsClientFactory::create([ + 'handler' => $mock, + 'max_retries' => 2, + 'retry_delay' => static fn (): int => 0, + ]); + + $this->expectException(ServerException::class); + $client->getArtist(4470662); + } + + public function testRetryMiddlewareDoesNotRetryOn400(): void + { + $mock = new MockHandler([ + new Response(400, ['Content-Type' => 'application/json'], '{"message": "Bad Request"}'), + new Response(200, ['Content-Type' => 'application/json'], '{"id": 4470662, "name": "Billie Eilish"}'), + ]); + + $client = DiscogsClientFactory::create([ + 'handler' => $mock, + 'retry_delay' => static fn (): int => 0, + ]); + + $this->expectException(ClientException::class); + $client->getArtist(4470662); + } + + public function testAutoRetryDisabledThrowsImmediatelyOn503(): void + { + $mock = new MockHandler([ + new Response(503, ['Content-Type' => 'application/json'], '{"message": "The Discogs server is busy."}'), + new Response(200, ['Content-Type' => 'application/json'], '{"id": 4470662, "name": "Billie Eilish"}'), + ]); + + $client = DiscogsClientFactory::create([ + 'handler' => $mock, + 'auto_retry' => false, + ]); + + $this->expectException(ServerException::class); + $client->getArtist(4470662); + } + + public function testMaxRetriesZeroThrowsImmediatelyOn503(): void + { + $mock = new MockHandler([ + new Response(503, ['Content-Type' => 'application/json'], '{"message": "The Discogs server is busy."}'), + new Response(200, ['Content-Type' => 'application/json'], '{"id": 4470662, "name": "Billie Eilish"}'), + ]); + + $client = DiscogsClientFactory::create([ + 'handler' => $mock, + 'max_retries' => 0, + ]); + + $this->expectException(ServerException::class); + $client->getArtist(4470662); + } + + public function testMaxRetriesCustomCountSucceeds(): void + { + $mock = new MockHandler([ + new Response(503, ['Content-Type' => 'application/json'], '{"message": "busy"}'), + new Response(503, ['Content-Type' => 'application/json'], '{"message": "busy"}'), + new Response(503, ['Content-Type' => 'application/json'], '{"message": "busy"}'), + new Response(200, ['Content-Type' => 'application/json'], '{"id": 4470662, "name": "Billie Eilish"}'), + ]); + + $client = DiscogsClientFactory::create([ + 'handler' => $mock, + 'auto_retry' => true, + 'max_retries' => 3, + 'retry_delay' => static fn (): int => 0, + ]); + $result = $client->getArtist(4470662); + + $this->assertSame('Billie Eilish', $result['name']); + $this->assertSame(0, $mock->count()); + } + + public function testDefaultRetryDelayCalculatesBackoff(): void + { + $this->assertSame(1000, $this->invokeDefaultRetryDelay(1)); + $this->assertSame(2000, $this->invokeDefaultRetryDelay(2)); + $this->assertSame(4000, $this->invokeDefaultRetryDelay(3)); + } + + public function testDefaultRetryDelayWithNumericRetryAfter(): void + { + $response = new Response(503, ['Retry-After' => '5']); + + $this->assertSame(5000, $this->invokeDefaultRetryDelay(1, $response)); + } + + public function testDefaultRetryDelayWithZeroNumericRetryAfter(): void + { + $response = new Response(503, ['Retry-After' => '0']); + + $this->assertSame(1000, $this->invokeDefaultRetryDelay(1, $response)); + } + + public function testDefaultRetryDelayWithHttpDateRetryAfter(): void + { + $futureTime = time() + 10; + $httpDate = gmdate('D, d M Y H:i:s \G\M\T', $futureTime); + $response = new Response(503, ['Retry-After' => $httpDate]); + + $delay = $this->invokeDefaultRetryDelay(1, $response); + $this->assertGreaterThan(0, $delay); + $this->assertLessThanOrEqual(10000, $delay); + } + + public function testDefaultRetryDelayWithInvalidOrPastHttpDateRetryAfter(): void + { + $pastTime = time() - 10; + $httpDate = gmdate('D, d M Y H:i:s \G\M\T', $pastTime); + $response = new Response(503, ['Retry-After' => $httpDate]); + + $this->assertSame(1000, $this->invokeDefaultRetryDelay(1, $response)); + } + + private function invokeDefaultRetryDelay(int $retries, ?ResponseInterface $response = null): int + { + $reflection = new ReflectionMethod(DiscogsClientFactory::class, 'defaultRetryDelay'); + + return (int) $reflection->invoke(null, $retries, $response); + } } diff --git a/tests/Unit/DiscogsClientTest.php b/tests/Unit/DiscogsClientTest.php index 2a6d846..d8138c4 100644 --- a/tests/Unit/DiscogsClientTest.php +++ b/tests/Unit/DiscogsClientTest.php @@ -467,7 +467,7 @@ public function testQueryParameterSeparation(): void // Test case 1: URI parameter should NOT appear in the query string $client->listArtistReleases(4470662, null, null, 10); - $request = $container[0]['request']; + $request = $this->getHistoryRequest($container, 0); $this->assertEquals('/artists/4470662/releases', $request->getUri()->getPath()); $this->assertEquals('per_page=10', $request->getUri()->getQuery()); @@ -496,7 +496,7 @@ public function testQueryParameterEdgeCases(): void // Test case 1: No URI parameters, all should be query parameters $client->search('Ariana Grande', 'artist'); - $request = $container[0]['request']; + $request = $this->getHistoryRequest($container, 0); $this->assertEquals('/database/search', $request->getUri()->getPath()); $query = $request->getUri()->getQuery(); @@ -506,7 +506,7 @@ public function testQueryParameterEdgeCases(): void // Test case 2: Multiple URI parameters should not appear in the query $client->listCollectionFolders('testuser'); - $request = $container[1]['request']; + $request = $this->getHistoryRequest($container, 1); $this->assertEquals('/users/testuser/collection/folders', $request->getUri()->getPath()); $this->assertEquals('', $request->getUri()->getQuery()); // No query params expected } @@ -532,7 +532,7 @@ public function testPreventsDuplicateParametersInUrl(): void // Test case 1: getArtist should NOT have 'id' in the query when it's in URI $client->getArtist(4470662); - $request = $container[0]['request']; + $request = $this->getHistoryRequest($container, 0); $this->assertEquals('/artists/4470662', $request->getUri()->getPath()); $this->assertEquals('', $request->getUri()->getQuery()); // Should be empty! @@ -544,7 +544,7 @@ public function testPreventsDuplicateParametersInUrl(): void // Test case 2: listCollectionItems with mixed URI + query parameters $client->listCollectionItems('testuser', 0, 10); - $request = $container[1]['request']; + $request = $this->getHistoryRequest($container, 1); $this->assertEquals('/users/testuser/collection/folders/0/releases', $request->getUri()->getPath()); $this->assertEquals('per_page=10', $request->getUri()->getQuery()); @@ -590,7 +590,7 @@ public function testDefaultUserAgentIsSet(): void $client->getArtist(1); - $request = $container[0]['request']; + $request = $this->getHistoryRequest($container, 0); $userAgent = $request->getHeaderLine('User-Agent'); // Test that User-Agent follows an expected format (not a specific version) @@ -621,7 +621,7 @@ public function testUserAgentComesFromConfiguration(): void $client->getArtist(1); - $request = $container[0]['request']; + $request = $this->getHistoryRequest($container, 0); $actualUserAgent = $request->getHeaderLine('User-Agent'); $this->assertEquals( @@ -649,7 +649,7 @@ public function testCustomUserAgentCanBeOverridden(): void $client->getArtist(1); - $request = $container[0]['request']; + $request = $this->getHistoryRequest($container, 0); $userAgent = $request->getHeaderLine('User-Agent'); $this->assertEquals('MyCustomApp/1.0', $userAgent); } @@ -689,7 +689,7 @@ public function testEmptyParametersArray(): void // This should work without throwing exceptions $result = $client->search(); - $this->assertIsArray($result); + $this->assertEquals([], $result['results']); } public function testMarketplaceEndpoints(): void @@ -711,23 +711,24 @@ public function testMarketplaceEndpoints(): void // Test marketplace fee calculation $client->getMarketplaceFee(10.00); - $request1 = $container[0]['request']; + $request1 = $this->getHistoryRequest($container, 0); $this->assertEquals('https://api.discogs.com/marketplace/fee/10.00', (string)$request1->getUri()); // Test marketplace fee with currency $client->getMarketplaceFeeByCurrency(10.00, 'USD'); - $request2 = $container[1]['request']; + $request2 = $this->getHistoryRequest($container, 1); $this->assertEquals('https://api.discogs.com/marketplace/fee/10.00/USD', (string)$request2->getUri()); // Test marketplace price suggestions $client->getMarketplacePriceSuggestions(16151073); - $request3 = $container[2]['request']; + $request3 = $this->getHistoryRequest($container, 2); $this->assertEquals( 'https://api.discogs.com/marketplace/price_suggestions/16151073', (string)$request3->getUri() ); // Verify no double slashes or URL typos in the path part + $this->assertIsArray($container); foreach ($container as $transaction) { $url = (string)$transaction['request']->getUri(); $path = parse_url($url, PHP_URL_PATH); @@ -779,8 +780,6 @@ public function testConfigFileLoadingOnFirstInstantiation(): void // Verify the config was loaded $config = ConfigCache::get(); - $this->assertNotNull($config); - $this->assertIsArray($config); $this->assertArrayHasKey('baseUrl', $config); } @@ -952,7 +951,6 @@ public function testUnicodeDataHandling(): void $result = $this->client->getRelease(1); - $this->assertIsArray($result); $this->assertEquals('BjΓΆrk', $result['name']); $this->assertEquals('ε‚ζœ¬ιΎδΈ€', $result['artist']); $this->assertStringContainsString('🎡', $result['notes']); @@ -983,7 +981,6 @@ public function testLargeResponseHandling(): void $result = $this->client->getArtist(1); - $this->assertIsArray($result); $this->assertCount(1000, $result['releases']); $this->assertEquals(1000, $result['pagination']['items']); } @@ -1066,7 +1063,6 @@ public function testValidateRequiredParametersOptionalNullValue(): void perPage: 50 ); - $this->assertIsArray($result); $this->assertEquals('Billie Eilish', $result['name']); } @@ -1104,7 +1100,7 @@ public function testValidateRequiredParametersNonExistentOperation(): void $method->invokeArgs($this->client, ['nonExistentOperation', [], []]); // If we reach here without exception, the test passes - $this->assertTrue(true); + $this->expectNotToPerformAssertions(); } /** @@ -1122,7 +1118,6 @@ public function testValidateRequiredParametersMixedParameters(): void username: 'testuser' ); - $this->assertIsArray($result); $this->assertEquals('testuser', $result['username']); $this->assertEquals(123, $result['release_id']); } @@ -1212,7 +1207,7 @@ public function testValidateRequiredParametersInternalLogicValid(): void ]); // If we reach here, the validation worked correctly - $this->assertTrue(true); + $this->expectNotToPerformAssertions(); } /** @@ -1318,7 +1313,6 @@ public function testAddInventoryUploadWithStringParameter(): void $csvContent = "Release ID,Condition,Price\n1234,Mint (M),15.99\n5678,Very Good+ (VG+),8.50"; $result = $this->client->addInventoryUpload($csvContent); - $this->assertIsArray($result); $this->assertTrue($result['success']); $this->assertEquals('Upload successful', $result['message']); } @@ -1335,7 +1329,6 @@ public function testChangeInventoryUploadWithStringParameter(): void $csvContent = "Release ID,Condition,Price\n1234,Near Mint (NM),18.99"; $result = $this->client->changeInventoryUpload($csvContent); - $this->assertIsArray($result); $this->assertTrue($result['success']); $this->assertEquals(2, $result['updated']); } @@ -1353,7 +1346,6 @@ public function testDeleteInventoryUpload(): void $csvContent = "listing_id\n12345678\n98765432"; $result = $this->client->deleteInventoryUpload($csvContent); - $this->assertIsArray($result); $this->assertTrue($result['success']); $this->assertEquals('Upload deleted', $result['message']); } @@ -1388,11 +1380,12 @@ public function testUploadMethodsWithCorrectEndpoints(): void $client->deleteInventoryUpload($deleteCsv); // Verify all requests were made to correct endpoints + $this->assertIsArray($container); $this->assertCount(3, $container); - $request1 = $container[0]['request']; - $request2 = $container[1]['request']; - $request3 = $container[2]['request']; + $request1 = $this->getHistoryRequest($container, 0); + $request2 = $this->getHistoryRequest($container, 1); + $request3 = $this->getHistoryRequest($container, 2); $this->assertEquals('/inventory/upload/add', $request1->getUri()->getPath()); $this->assertEquals('/inventory/upload/change', $request2->getUri()->getPath()); diff --git a/tests/Unit/HeaderSecurityTest.php b/tests/Unit/HeaderSecurityTest.php index 71ed851..0ed9caa 100644 --- a/tests/Unit/HeaderSecurityTest.php +++ b/tests/Unit/HeaderSecurityTest.php @@ -38,7 +38,7 @@ public function testUserCannotOverrideAuthorizationWithPersonalAccessToken(): vo $client->search('test'); - $request = $history[0]['request']; + $request = $this->getHistoryRequest($history, 0); $authHeader = $request->getHeaderLine('Authorization'); $this->assertValidPersonalTokenHeader($authHeader); @@ -78,7 +78,7 @@ public function testUserCannotOverrideAuthorizationWithOAuth(): void $client->getIdentity(); - $request = $history[0]['request']; + $request = $this->getHistoryRequest($history, 0); $authHeader = $request->getHeaderLine('Authorization'); $this->assertValidOAuthHeader($authHeader); @@ -111,7 +111,7 @@ public function testUserCanSetCustomHeadersWithoutConflicts(): void $client->search('test'); - $request = $history[0]['request']; + $request = $this->getHistoryRequest($history, 0); $authHeader = $request->getHeaderLine('Authorization'); $this->assertValidPersonalTokenHeader($authHeader); diff --git a/tests/Unit/ProductionRealisticTest.php b/tests/Unit/ProductionRealisticTest.php index b94b59e..96423f0 100644 --- a/tests/Unit/ProductionRealisticTest.php +++ b/tests/Unit/ProductionRealisticTest.php @@ -154,8 +154,6 @@ public function testExtremelyLargeIds(): void ); $result = $this->client->getArtist(999999999999); - - $this->assertIsArray($result); $this->assertEquals(999999999999, $result['id']); } @@ -171,7 +169,6 @@ public function testSpecialCharactersInSearch(): void // Test with problematic characters that might break URL encoding $result = $this->client->search('Post Malone: Hollywood\'s Bleeding [Deluxe]'); - $this->assertIsArray($result); $this->assertArrayHasKey('results', $result); } @@ -211,7 +208,6 @@ public function testDeeplyNestedJsonResponse(): void $result = $this->client->getArtist(1); - $this->assertIsArray($result); $this->assertArrayHasKey('data', $result); } diff --git a/tests/Unit/SecurityTest.php b/tests/Unit/SecurityTest.php index f7c68aa..5afbf6e 100644 --- a/tests/Unit/SecurityTest.php +++ b/tests/Unit/SecurityTest.php @@ -185,7 +185,6 @@ public function testValidInputPassesThroughSafely(): void // Normal, safe input should work fine $result = $client->getArtist(139250); - $this->assertIsArray($result); $this->assertEquals(139250, $result['id']); $this->assertEquals('Test Artist', $result['name']); } @@ -204,10 +203,7 @@ public function testSecurityValidationDoesNotBreakNormalFlow(): void $searchResult = $client->search('test'); $artistResult = $client->getArtist(139250); - $this->assertIsArray($searchResult); $this->assertEquals([], $searchResult['results']); - - $this->assertIsArray($artistResult); $this->assertEquals(139250, $artistResult['id']); } } diff --git a/tests/Unit/UnitTestCase.php b/tests/Unit/UnitTestCase.php index 1eeefe7..05c1e96 100644 --- a/tests/Unit/UnitTestCase.php +++ b/tests/Unit/UnitTestCase.php @@ -40,7 +40,6 @@ protected function assertValidArtistResponse(array $artist): void */ protected function assertValidResponse(array $response): void { - $this->assertIsArray($response); $this->assertNotEmpty($response); } @@ -76,4 +75,20 @@ protected function assertValidPersonalTokenHeader(string $authHeader): void $this->assertStringNotContainsString('key=', $authHeader); $this->assertStringNotContainsString('secret=', $authHeader); } + + /** + * Helper to safely extract recorded request from Guzzle history container + * + * @param array|\ArrayAccess $container + */ + protected function getHistoryRequest(array|\ArrayAccess $container, int $index = 0): \Psr\Http\Message\RequestInterface + { + $this->assertArrayHasKey($index, $container); + $entry = $container[$index]; + $this->assertIsArray($entry); + $this->assertArrayHasKey('request', $entry); + $this->assertInstanceOf(\Psr\Http\Message\RequestInterface::class, $entry['request']); + + return $entry['request']; + } }