diff --git a/.github/workflows/code-checks.yml b/.github/workflows/code-checks.yml index 7e31214..d57ddf2 100644 --- a/.github/workflows/code-checks.yml +++ b/.github/workflows/code-checks.yml @@ -11,11 +11,6 @@ jobs: strategy: matrix: php: - - '7.1' - - '7.2' - - '7.3' - - '7.4' - - '8.0' - '8.1' - '8.2' - '8.3' diff --git a/.gitignore b/.gitignore index 8557bf4..369d4a1 100644 --- a/.gitignore +++ b/.gitignore @@ -5,5 +5,5 @@ /composer.lock ### Tests -/.phpunit.result.cache +/.phpunit.cache/ /phpunit.xml diff --git a/.runConfigurations/All tests.run.xml b/.runConfigurations/All tests.run.xml index 65f948c..a6489a4 100644 --- a/.runConfigurations/All tests.run.xml +++ b/.runConfigurations/All tests.run.xml @@ -5,7 +5,7 @@ - + \ No newline at end of file diff --git a/.runConfigurations/All unit tests.run.xml b/.runConfigurations/All unit tests.run.xml index 2b8b16b..fd854d2 100644 --- a/.runConfigurations/All unit tests.run.xml +++ b/.runConfigurations/All unit tests.run.xml @@ -1,6 +1,6 @@ - + \ No newline at end of file diff --git a/.runConfigurations/Functional tests.run.xml b/.runConfigurations/Functional tests.run.xml index 44647a7..078ef31 100644 --- a/.runConfigurations/Functional tests.run.xml +++ b/.runConfigurations/Functional tests.run.xml @@ -5,7 +5,7 @@ - + \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e4f3e5..5411480 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +### Added +- Add PHP 8 type declarations. +### Removed +- **BC break**: Removed support for PHP versions <= v8.0 as they are no longer + [actively supported](https://php.net/supported-versions.php) by the PHP project. ## [5.1.1] - 2021-08-10 ### Fixed diff --git a/composer.json b/composer.json index 185cbd9..2c00314 100644 --- a/composer.json +++ b/composer.json @@ -1,7 +1,7 @@ { "name": "maxemail/api-php", "description": "Maxemail API Client", - "license" : "LGPL-3.0", + "license": "LGPL-3.0", "authors": [ { "name": "Maxemail Team & Contributors", @@ -16,17 +16,17 @@ }, "require": { - "php" : "^7.1|^8", + "php": "^8.1", "ext-fileinfo": "*", - "ext-json" : "*", + "ext-json": "*", "ext-zip": "*", - "psr/log" : "^1 || ^2 || ^3", + "psr/log": "^2 || ^3", "guzzlehttp/guzzle": "^6 || ^7" }, "suggest": { - "psr/log-implementation" : "A logger can be used to provide debug information" + "psr/log-implementation": "A logger can be used to provide debug information" }, "autoload-dev": { @@ -36,8 +36,12 @@ }, "require-dev": { - "phpunit/phpunit" : "^7|^8|^9", - "php-mock/php-mock-phpunit": "^2.0", - "symplify/easy-coding-standard": "^9" + "phpunit/phpunit" : "^10", + "php-mock/php-mock-phpunit": "^2", + "symplify/easy-coding-standard": "^12.1" + }, + "scripts": { + "check-cs": "vendor/bin/ecs check --ansi", + "fix-cs": "vendor/bin/ecs check --fix --ansi" } } diff --git a/ecs.php b/ecs.php index 81dc286..86501dc 100644 --- a/ecs.php +++ b/ecs.php @@ -2,25 +2,65 @@ declare(strict_types=1); -use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; -use Symplify\EasyCodingStandard\ValueObject\Option; +use Symplify\EasyCodingStandard\Config\ECSConfig; use Symplify\EasyCodingStandard\ValueObject\Set\SetList; -return static function (ContainerConfigurator $containerConfigurator): void { - $parameters = $containerConfigurator->parameters(); - $parameters->set(Option::PATHS, [ +return ECSConfig::configure() + ->withPaths([ __DIR__ . '/src', __DIR__ . '/tests', - ]); + ]) + ->withRootFiles() - $services = $containerConfigurator->services(); + ->withSets([ + SetList::COMMON, + SetList::PSR_12, + SetList::STRICT, + ]) - $containerConfigurator->import(SetList::COMMON); - // Remove sniff, from common/control-structures - $services->remove(\PhpCsFixer\Fixer\ClassNotation\OrderedClassElementsFixer::class); - // Remove sniff, from common/spaces - $services->remove(\PhpCsFixer\Fixer\Operator\NotOperatorWithSuccessorSpaceFixer::class); - $services->remove(\PhpCsFixer\Fixer\CastNotation\CastSpacesFixer::class); + ->withSkip([ + // Remove sniff, from common/control-structures + \PhpCsFixer\Fixer\ClassNotation\OrderedClassElementsFixer::class, - $containerConfigurator->import(SetList::PSR_12); -}; + // Remove sniff, from common/spaces + \PhpCsFixer\Fixer\Operator\NotOperatorWithSuccessorSpaceFixer::class, + ]) + + // Rule from common/spaces. No space after cast. + ->withConfiguredRule( + \PhpCsFixer\Fixer\CastNotation\CastSpacesFixer::class, + [ + 'space' => 'none', + ], + ) + + /* + * Rule missing from PSR12. PER Coding Style 3: + * "... each of the blocks [of import statements] MUST be separated by a single blank line ..." + */ + ->withRules([ + \PhpCsFixer\Fixer\Whitespace\BlankLineBetweenImportGroupsFixer::class, + ]) + + // Rule from PSR12. PER Coding Style 7.1: "The `fn` keyword MUST NOT be succeeded by a space." + ->withConfiguredRule( + \PhpCsFixer\Fixer\FunctionNotation\FunctionDeclarationFixer::class, + [ + 'closure_fn_spacing' => 'none', + ], + ) + + /* + * Rule from PER Coding Style 2.6: + * "If the list is split across multiple lines, then the last item MUST have a trailing comma." + */ + ->withConfiguredRule( + \PhpCsFixer\Fixer\ControlStructure\TrailingCommaInMultilineFixer::class, + [ + 'elements' => [ + \PhpCsFixer\Fixer\ControlStructure\TrailingCommaInMultilineFixer::ELEMENTS_ARGUMENTS, + \PhpCsFixer\Fixer\ControlStructure\TrailingCommaInMultilineFixer::ELEMENTS_ARRAYS, + \PhpCsFixer\Fixer\ControlStructure\TrailingCommaInMultilineFixer::ELEMENTS_PARAMETERS, + ], + ], + ); diff --git a/phpunit.xml.dist b/phpunit.xml.dist index e6230c1..07e8dd3 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,10 +1,10 @@ @@ -17,20 +17,13 @@ - + tests - - - - src/ - - - - + src - + diff --git a/src/Client.php b/src/Client.php index b041f32..14faf8f 100644 --- a/src/Client.php +++ b/src/Client.php @@ -6,6 +6,7 @@ use GuzzleHttp\Client as GuzzleClient; use GuzzleHttp\HandlerStack; +use Psr\Log\LoggerAwareInterface; use Psr\Log\LoggerInterface; /** @@ -56,59 +57,38 @@ * @property mixed transactional https://docs.maxemail.xtremepush.com/mxm-dev/api/services/transactional * @property mixed data_export_quick_transactional https://docs.maxemail.xtremepush.com/mxm-dev/api/services/data-export-quick-transactional */ -class Client implements \Psr\Log\LoggerAwareInterface +class Client implements LoggerAwareInterface { public const VERSION = '5.1'; - /** - * @var string - */ - private $uri = 'https://mxm.xtremepush.com/'; + private string $uri = 'https://mxm.xtremepush.com/'; - /** - * @var string - */ - private $username; + private readonly string $username; - /** - * @var string - */ - private $password; + private readonly string $password; /** * @var Service[] */ - private $services = []; + private array $services = []; - /** - * @var Helper - */ - private $helper; + private Helper $helper; - /** - * @var LoggerInterface - */ - private $logger; + private LoggerInterface $logger; - /** - * @var GuzzleClient - */ - private $httpClient; + private GuzzleClient $httpClient; - /** - * @var bool - */ - private $debugLoggingEnabled = false; + private bool $debugLoggingEnabled = false; /** - * @param array $config { - * @var string $username Required - * @var string $password Required - * @var string $uri Optional. Default https://mxm.xtremepush.com/ - * @var string $user @deprecated See username - * @var string $pass @deprecated See password - * @var bool $debugLogging Optional. Enable logging of request/response. Default false - * } + * @param array{ + * username: string, // Required + * password: string, // Required + * uri: string, // Optional. Default https://mxm.xtremepush.com/ + * user: string, // @deprecated See username + * pass: string, // @deprecated See password + * debugLogging: bool, // Optional. Enable logging of request/response. Default false + * } $config */ public function __construct(array $config) { @@ -159,7 +139,7 @@ private function getInstance(string $serviceName): Service private function getClient(): GuzzleClient { - if ($this->httpClient === null) { + if (!isset($this->httpClient)) { $stack = HandlerStack::create(); Middleware::addMaxemailErrorParser($stack); Middleware::addWarningLogging($stack, $this->getLogger()); @@ -187,10 +167,10 @@ private function getClient(): GuzzleClient /** * Get API connection config * - * @return array { - * @var string $uri - * @var string $username - * @var string $password + * @return array{ + * uri: string, + * username: string, + * password: string, * } */ public function getConfig(): array diff --git a/src/Helper.php b/src/Helper.php index 5c940ba..73fb208 100644 --- a/src/Helper.php +++ b/src/Helper.php @@ -16,25 +16,12 @@ */ class Helper { - /** - * @var Client - */ - private $api; - - /** - * @var GuzzleClient - */ - private $httpClient; + private string $logLevel = LogLevel::DEBUG; - /** - * @var string - */ - private $logLevel = LogLevel::DEBUG; - - public function __construct(Client $api, GuzzleClient $httpClient) - { - $this->api = $api; - $this->httpClient = $httpClient; + public function __construct( + private readonly Client $api, + private readonly GuzzleClient $httpClient, + ) { } public function setLogLevel(string $level): self @@ -108,15 +95,17 @@ public function uploadFile(string $path): string /** * Download file by type * - * @param string|int $primaryId - * @param array $options { - * @var bool $extract Whether to extract a compressed download, default true - * @var string $dir Directory to use for downloaded file(s), default sys_temp_dir - * } + * @param array{ + * extract: bool, // Whether to extract a compressed download, default true + * dir: string, // Directory to use for downloaded file(s), default sys_temp_dir + * } $options * @return string filename */ - public function downloadFile(string $type, $primaryId, array $options = []): string - { + public function downloadFile( + string $type, + string|int $primaryId, + array $options = [], + ): string { $typePrimary = [ 'file' => 'key', 'listexport' => 'id', @@ -130,7 +119,7 @@ public function downloadFile(string $type, $primaryId, array $options = []): str // Create target file $filename = tempnam( ($options['dir'] ?? sys_get_temp_dir()), - "mxm-{$type}-{$primaryId}-" + "mxm-{$type}-{$primaryId}-", ); $local = @fopen($filename, 'w'); if ($local === false) { @@ -185,7 +174,7 @@ public function downloadFile(string $type, $primaryId, array $options = []): str // Add extension to filename, optionally extract zip switch (true) { - case (strpos($mime, 'zip')): + case str_contains($mime, 'zip'): if (!isset($options['extract']) || $options['extract'] === true) { // Maxemail only compresses CSV files, and only contains one file in a zip $zip = new \ZipArchive(); @@ -204,13 +193,13 @@ public function downloadFile(string $type, $primaryId, array $options = []): str break; - case (strpos($mime, 'pdf')): + case str_contains($mime, 'pdf'): rename($filename, $filename . '.pdf'); $filename = $filename . '.pdf'; break; - case (strpos($mime, 'csv')): - case ($mime === 'text/plain'): + case str_contains($mime, 'csv'): + case $mime === 'text/plain': rename($filename, $filename . '.csv'); $filename = $filename . '.csv'; break; diff --git a/src/Middleware.php b/src/Middleware.php index 5f25c8b..5380959 100644 --- a/src/Middleware.php +++ b/src/Middleware.php @@ -38,10 +38,10 @@ public static function addLogging(HandlerStack $stack, LoggerInterface $logger): $middleware = GuzzleMiddleware::log( $logger, new MessageFormatter($messageFormat), - LogLevel::DEBUG + LogLevel::DEBUG, ); $stack->push($middleware, 'log' . $idx); - }; + } } /** @@ -50,7 +50,7 @@ public static function addLogging(HandlerStack $stack, LoggerInterface $logger): */ public static function addWarningLogging(HandlerStack $stack, LoggerInterface $logger): void { - $middleware = GuzzleMiddleware::mapResponse(function (ResponseInterface $response) use ($logger) { + $middleware = GuzzleMiddleware::mapResponse(function (ResponseInterface $response) use ($logger): ResponseInterface { if ($response->hasHeader('Warning')) { foreach ($response->getHeader('Warning') as $message) { // Code, agent, message, [date] @@ -76,7 +76,7 @@ public static function addWarningLogging(HandlerStack $stack, LoggerInterface $l */ public static function addMaxemailErrorParser(HandlerStack $stack): void { - $middleware = GuzzleMiddleware::mapResponse(function (ResponseInterface $response) { + $middleware = GuzzleMiddleware::mapResponse(function (ResponseInterface $response): ResponseInterface { $code = $response->getStatusCode(); if ($code < 400 || $code >= 500) { // Allow success response to continue, and 500-level errors to be handled by Guzzle @@ -89,7 +89,7 @@ public static function addMaxemailErrorParser(HandlerStack $stack): void if ($error instanceof \stdClass && isset($error->msg)) { throw new Exception\ClientException($error->msg, $response->getStatusCode()); } - } catch (\UnexpectedValueException $e) { + } catch (\UnexpectedValueException) { // Failed to decode as Maxemail error, leave Guzzle to handle it } return $response; diff --git a/src/Service.php b/src/Service.php index e56beeb..c3fc856 100644 --- a/src/Service.php +++ b/src/Service.php @@ -17,20 +17,10 @@ class Service { use JsonTrait; - /** - * @var string - */ - private $service; - - /** - * @var GuzzleClient - */ - private $httpClient; - - public function __construct(string $service, GuzzleClient $httpClient) - { - $this->service = $service; - $this->httpClient = $httpClient; + public function __construct( + private readonly string $service, + private readonly GuzzleClient $httpClient, + ) { } /** diff --git a/tests/ApiTest.php b/tests/ApiTest.php index 7aed6ae..d9103f0 100644 --- a/tests/ApiTest.php +++ b/tests/ApiTest.php @@ -16,20 +16,20 @@ */ class ApiTest extends TestCase { - private $testConfig = [ + private array $testConfig = [ 'uri' => 'https://maxemail.example.com/', 'username' => 'api@user.com', 'password' => 'apipass', ]; - public function testConfigValid() + public function testConfigValid(): void { $api = new Client($this->testConfig); - $this->assertSame($this->testConfig, $api->getConfig()); + static::assertSame($this->testConfig, $api->getConfig()); } - public function testConfigSupportDeprecatedUserPass() + public function testConfigSupportDeprecatedUserPass(): void { $config = [ 'user' => 'api@user.com', @@ -38,11 +38,11 @@ public function testConfigSupportDeprecatedUserPass() $api = new Client($config); - $this->assertSame($config['user'], $api->getConfig()['username']); - $this->assertSame($config['pass'], $api->getConfig()['password']); + static::assertSame($config['user'], $api->getConfig()['username']); + static::assertSame($config['pass'], $api->getConfig()['password']); } - public function testConfigDefaultHost() + public function testConfigDefaultHost(): void { $config = [ 'username' => 'api@user.com', @@ -51,10 +51,10 @@ public function testConfigDefaultHost() $api = new Client($config); - $this->assertSame('https://mxm.xtremepush.com/', $api->getConfig()['uri']); + static::assertSame('https://mxm.xtremepush.com/', $api->getConfig()['uri']); } - public function testConfigStripsUriPath() + public function testConfigStripsUriPath(): void { $config = [ 'uri' => 'http://maxemail.example.com/some/extra/path', @@ -64,10 +64,10 @@ public function testConfigStripsUriPath() $api = new Client($config); - $this->assertSame('http://maxemail.example.com/', $api->getConfig()['uri']); + static::assertSame('http://maxemail.example.com/', $api->getConfig()['uri']); } - public function testConfigInvalidUri() + public function testConfigInvalidUri(): void { $this->expectException(Exception\InvalidArgumentException::class); $this->expectExceptionMessage('URI malformed'); @@ -81,7 +81,7 @@ public function testConfigInvalidUri() new Client($config); } - public function testConfigMissingUriProtocol() + public function testConfigMissingUriProtocol(): void { $this->expectException(Exception\InvalidArgumentException::class); $this->expectExceptionMessage('URI must contain protocol scheme and host'); @@ -95,7 +95,7 @@ public function testConfigMissingUriProtocol() new Client($config); } - public function testSetGetLogger() + public function testSetGetLogger(): void { /** @var \Psr\Log\LoggerInterface|MockObject $logger */ $logger = $this->createMock(\Psr\Log\LoggerInterface::class); @@ -104,22 +104,22 @@ public function testSetGetLogger() $api->setLogger($logger); - $this->assertSame($logger, $api->getLogger()); + static::assertSame($logger, $api->getLogger()); } - public function testGetHelper() + public function testGetHelper(): void { $api = new Client($this->testConfig); $helper = $api->getHelper(); - $this->assertInstanceOf(Helper::class, $helper); + static::assertInstanceOf(Helper::class, $helper); } /** * Test getInstance() returns same Service instance for same name, different for different name */ - public function testGetInstance() + public function testGetInstance(): void { $api = new Client($this->testConfig); @@ -127,7 +127,7 @@ public function testGetInstance() $sameService = $api->service; $differentService = $api->different; - $this->assertSame($originalService, $sameService); - $this->assertNotSame($originalService, $differentService); + static::assertSame($originalService, $sameService); + static::assertNotSame($originalService, $differentService); } } diff --git a/tests/FunctionalTest.php b/tests/FunctionalTest.php index 255b5a1..84b174b 100644 --- a/tests/FunctionalTest.php +++ b/tests/FunctionalTest.php @@ -4,6 +4,7 @@ namespace Maxemail\Api; +use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\TestCase; /** @@ -12,20 +13,16 @@ * @package Maxemail\Api * @copyright 2007-2019 Emailcenter UK Ltd. (https://maxemail.xtremepush.com) * @license LGPL-3.0 - * - * @group functional */ +#[Group('functional')] class FunctionalTest extends TestCase { - /** - * @var Client - */ - private $client; + private Client $client; protected function setUp(): void { if (!getenv('FUNC_ENABLED')) { - $this->markTestSkipped('Functional tests are disabled'); + static::markTestSkipped('Functional tests are disabled'); } $config = [ @@ -39,29 +36,29 @@ protected function setUp(): void /** * The most basic of tests */ - public function testUserAuth() + public function testUserAuth(): void { $user = $this->client->user->isLoggedIn(); - $this->assertTrue($user); + static::assertTrue($user); } /** * Test non-scalar result using Email tree which will always exist */ - public function testFetchTree() + public function testFetchTree(): void { $tree = $this->client->tree->fetchRoot('email', []); $root = $tree[0]; - $this->assertSame('email', $root->text); - $this->assertTrue($root->rootNode); + static::assertSame('email', $root->text); + static::assertTrue($root->rootNode); } /** * This test isn't about the message per-se, * but it checks that we're getting the properly decoded Maxemail error */ - public function testFetchTreeError() + public function testFetchTreeError(): void { $this->expectException(Exception\ClientException::class); $this->expectExceptionMessage('Invalid Node Class'); @@ -69,26 +66,37 @@ public function testFetchTreeError() $this->client->tree->fetchRoot('notATree', []); } - public function testDeprecatedMethod() + public function testDeprecatedMethod(): void { - // @todo phpunit > v7, change to `expectDeprecation()` etc. - $this->expectException(\PHPUnit\Framework\Error\Deprecated::class); + // Capture deprecation error + $originalErrorLevel = error_reporting(); + error_reporting(E_ALL); + set_error_handler(static function (int $errno, string $errstr): never { + throw new \Exception($errstr, $errno); + }, E_USER_DEPRECATED); + + $this->expectException(\Exception::class); $this->expectExceptionMessage('searchLists Deprecated'); - $this->client->recipient->searchLists('test@example.com'); + try { + $this->client->recipient->searchLists('test@example.com'); + } finally { + error_reporting($originalErrorLevel); + restore_error_handler(); + } } /** * The file uploaded should be identical to the file then downloaded */ - public function testHelperUploadDownload() + public function testHelperUploadDownload(): void { $sampleFile = __DIR__ . '/__files/sample-file.csv'; $key = $this->client->getHelper()->uploadFile($sampleFile); $downloadFile = $this->client->getHelper()->downloadFile('file', $key); - $this->assertFileEquals($sampleFile, $downloadFile); + static::assertFileEquals($sampleFile, $downloadFile); unlink($downloadFile); } } diff --git a/tests/HelperTest.php b/tests/HelperTest.php index 7a5fef4..4b0f5d4 100644 --- a/tests/HelperTest.php +++ b/tests/HelperTest.php @@ -12,6 +12,7 @@ use GuzzleHttp\Psr7\Request; use GuzzleHttp\Psr7\Response; use phpmock\phpunit\PHPMock; +use PHPUnit\Framework\Attributes\RunTestsInSeparateProcesses; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -22,28 +23,20 @@ * @copyright 2007-2019 Emailcenter UK Ltd. (https://maxemail.xtremepush.com) * @license LGPL-3.0 * - * @runTestsInSeparateProcesses Used so global functions can be mocked after already accessed in other tests + * RunTestsInSeparateProcesses: Allow global functions to be mocked after already accessed in other tests */ +#[RunTestsInSeparateProcesses] class HelperTest extends TestCase { use PHPMock; - /** - * @var Client|MockObject - */ - private $apiClientMock; + private Client&MockObject $apiClientMock; - /** - * @var MockHandler - */ - private $mockHandler; + private MockHandler $mockHandler; - private $clientHistory = []; + private array $clientHistory = []; - /** - * @var Helper - */ - private $helper; + private Helper $helper; protected function setUp(): void { @@ -53,7 +46,7 @@ protected function setUp(): void $stack = HandlerStack::create($this->mockHandler); $stack->push( - GuzzleMiddleware::history($this->clientHistory) + GuzzleMiddleware::history($this->clientHistory), ); $httpClient = new GuzzleClient([ @@ -64,7 +57,7 @@ protected function setUp(): void $this->helper = new Helper($this->apiClientMock, $httpClient); } - public function testUpload() + public function testUpload(): void { $sampleFile = __DIR__ . '/__files/sample-file.csv'; $key = 'abc123def456'; @@ -73,12 +66,12 @@ public function testUpload() $fileUploadService = $this->createMock(Service::class); // The API client will be used to initialise the upload - $this->apiClientMock->expects($this->once()) + $this->apiClientMock->expects(static::once()) ->method('__get') ->with('file_upload') ->willReturn($fileUploadService); - $fileUploadService->expects($this->once()) + $fileUploadService->expects(static::once()) ->method('__call') ->with('initialise', []) ->willReturn((object)[ @@ -90,32 +83,31 @@ public function testUpload() // as the file handle will be closed by then $requestBody = ''; $this->mockHandler->append( - function (Request $request) use (&$requestBody) { + function (Request $request) use (&$requestBody): Response { $requestBody = (string)$request->getBody(); return new Response(200, [], ''); - } + }, ); $actual = $this->helper->uploadFile($sampleFile); - $this->assertCount(1, $this->clientHistory); + static::assertCount(1, $this->clientHistory); /** @var Request $request */ $request = $this->clientHistory[0]['request']; // Check request is multipart and body contains expected parameters - $this->assertTrue($request->hasHeader('Content-Type')); - $this->assertStringStartsWith('multipart/form-data', $request->getHeader('Content-Type')[0]); - // @todo phpunit > v8, change to `assertMatchesRegularExpression()` - $this->assertRegExp("/name=\"method\".*\r\n\r\nhandle\r\n/sU", $requestBody); - $this->assertRegExp("/name=\"key\".*\r\n\r\n{$key}\r\n/sU", $requestBody); + static::assertTrue($request->hasHeader('Content-Type')); + static::assertStringStartsWith('multipart/form-data', $request->getHeader('Content-Type')[0]); + static::assertMatchesRegularExpression("/name=\"method\".*\r\n\r\nhandle\r\n/sU", $requestBody); + static::assertMatchesRegularExpression("/name=\"key\".*\r\n\r\n{$key}\r\n/sU", $requestBody); $fileContents = file_get_contents($sampleFile); - $this->assertRegExp("/name=\"file\".*\r\n\r\n{$fileContents}\r\n/sU", $requestBody); + static::assertMatchesRegularExpression("/name=\"file\".*\r\n\r\n{$fileContents}\r\n/sU", $requestBody); - $this->assertSame($key, $actual); + static::assertSame($key, $actual); } - public function testUploadUnreadable() + public function testUploadUnreadable(): void { $this->expectException(Exception\InvalidArgumentException::class); $this->expectExceptionMessage('File path is not readable'); @@ -125,163 +117,171 @@ public function testUploadUnreadable() $this->helper->uploadFile($sampleFile); } - public function testUploadUnableToOpen() + public function testUploadUnableToOpen(): void { + $reasonMessage = sha1(uniqid('error')); + $this->expectException(Exception\RuntimeException::class); - $this->expectExceptionMessage('Unable to open local file'); + $this->expectExceptionMessage('Unable to open local file: ' . $reasonMessage); $sampleFile = __DIR__ . '/__files/sample-file.csv'; $fopenMock = $this->getFunctionMock(__NAMESPACE__, 'fopen'); - $fopenMock->expects($this->once()) + $fopenMock->expects(static::once()) ->with($sampleFile, 'r') ->willReturn(false); + $errorMock = $this->getFunctionMock(__NAMESPACE__, 'error_get_last'); + $errorMock->expects(static::once()) + ->willReturn([ + 'message' => $reasonMessage, + ]); + $this->helper->uploadFile($sampleFile); } - public function testDownloadCsv() + public function testDownloadCsv(): void { $sampleFile = __DIR__ . '/__files/sample-file.csv'; $key = 'abc123def456'; $this->mockHandler->append( - new Response(200, [], fopen($sampleFile, 'r')) + new Response(200, [], fopen($sampleFile, 'r')), ); $downloadedFile = $this->helper->downloadFile('file', $key); - $this->assertCount(1, $this->clientHistory); + static::assertCount(1, $this->clientHistory); /** @var Request $request */ $request = $this->clientHistory[0]['request']; - $this->assertSame('GET', $request->getMethod()); - $this->assertSame('/download/file/key/' . $key, $request->getUri()->getPath()); - $this->assertSame(['*'], $request->getHeader('Accept')); + static::assertSame('GET', $request->getMethod()); + static::assertSame('/download/file/key/' . $key, $request->getUri()->getPath()); + static::assertSame(['*'], $request->getHeader('Accept')); - $this->assertFileEquals($sampleFile, $downloadedFile); + static::assertFileEquals($sampleFile, $downloadedFile); } - public function testDownloadZip() + public function testDownloadZip(): void { $responseFile = __DIR__ . '/__files/sample-file.csv.zip'; $expectedFile = __DIR__ . '/__files/sample-file.csv'; $key = 'abc123def456'; $this->mockHandler->append( - new Response(200, [], fopen($responseFile, 'r')) + new Response(200, [], fopen($responseFile, 'r')), ); $downloadedFile = $this->helper->downloadFile('file', $key); - $this->assertCount(1, $this->clientHistory); + static::assertCount(1, $this->clientHistory); /** @var Request $request */ $request = $this->clientHistory[0]['request']; - $this->assertSame('GET', $request->getMethod()); - $this->assertSame('/download/file/key/' . $key, $request->getUri()->getPath()); - $this->assertSame(['*'], $request->getHeader('Accept')); + static::assertSame('GET', $request->getMethod()); + static::assertSame('/download/file/key/' . $key, $request->getUri()->getPath()); + static::assertSame(['*'], $request->getHeader('Accept')); - $this->assertFileEquals($expectedFile, $downloadedFile); + static::assertFileEquals($expectedFile, $downloadedFile); } - public function testDownloadZipNoExtract() + public function testDownloadZipNoExtract(): void { $sampleFile = __DIR__ . '/__files/sample-file.csv.zip'; $key = 'abc123def456'; $this->mockHandler->append( - new Response(200, [], fopen($sampleFile, 'r')) + new Response(200, [], fopen($sampleFile, 'r')), ); $downloadedFile = $this->helper->downloadFile('file', $key, [ 'extract' => false, ]); - $this->assertCount(1, $this->clientHistory); + static::assertCount(1, $this->clientHistory); /** @var Request $request */ $request = $this->clientHistory[0]['request']; - $this->assertSame('GET', $request->getMethod()); - $this->assertSame('/download/file/key/' . $key, $request->getUri()->getPath()); - $this->assertSame(['*'], $request->getHeader('Accept')); + static::assertSame('GET', $request->getMethod()); + static::assertSame('/download/file/key/' . $key, $request->getUri()->getPath()); + static::assertSame(['*'], $request->getHeader('Accept')); - $this->assertFileEquals($sampleFile, $downloadedFile); + static::assertFileEquals($sampleFile, $downloadedFile); } - public function testDownloadPdf() + public function testDownloadPdf(): void { $sampleFile = __DIR__ . '/__files/sample-file.pdf'; $key = 'abc123def456'; $this->mockHandler->append( - new Response(200, [], fopen($sampleFile, 'r')) + new Response(200, [], fopen($sampleFile, 'r')), ); $downloadedFilename = $this->helper->downloadFile('file', $key); - $this->assertCount(1, $this->clientHistory); + static::assertCount(1, $this->clientHistory); /** @var Request $request */ $request = $this->clientHistory[0]['request']; - $this->assertSame('GET', $request->getMethod()); - $this->assertSame('/download/file/key/' . $key, $request->getUri()->getPath()); - $this->assertSame(['*'], $request->getHeader('Accept')); + static::assertSame('GET', $request->getMethod()); + static::assertSame('/download/file/key/' . $key, $request->getUri()->getPath()); + static::assertSame(['*'], $request->getHeader('Accept')); - $this->assertFileEquals($sampleFile, $downloadedFilename); + static::assertFileEquals($sampleFile, $downloadedFilename); } - public function testDownloadListExport() + public function testDownloadListExport(): void { $sampleFile = __DIR__ . '/__files/sample-file.csv'; $key = 123; $this->mockHandler->append( - new Response(200, [], fopen($sampleFile, 'r')) + new Response(200, [], fopen($sampleFile, 'r')), ); $downloadedFile = $this->helper->downloadFile('listexport', $key); - $this->assertCount(1, $this->clientHistory); + static::assertCount(1, $this->clientHistory); /** @var Request $request */ $request = $this->clientHistory[0]['request']; - $this->assertSame('GET', $request->getMethod()); - $this->assertSame('/download/listexport/id/' . $key, $request->getUri()->getPath()); - $this->assertSame(['*'], $request->getHeader('Accept')); + static::assertSame('GET', $request->getMethod()); + static::assertSame('/download/listexport/id/' . $key, $request->getUri()->getPath()); + static::assertSame(['*'], $request->getHeader('Accept')); - $this->assertFileEquals($sampleFile, $downloadedFile); + static::assertFileEquals($sampleFile, $downloadedFile); } - public function testDownloadDataExport() + public function testDownloadDataExport(): void { $sampleFile = __DIR__ . '/__files/sample-file.csv'; $key = 123; $this->mockHandler->append( - new Response(200, [], fopen($sampleFile, 'r')) + new Response(200, [], fopen($sampleFile, 'r')), ); $downloadedFile = $this->helper->downloadFile('dataexport', $key); - $this->assertCount(1, $this->clientHistory); + static::assertCount(1, $this->clientHistory); /** @var Request $request */ $request = $this->clientHistory[0]['request']; - $this->assertSame('GET', $request->getMethod()); - $this->assertSame('/download/dataexport/id/' . $key, $request->getUri()->getPath()); - $this->assertSame(['*'], $request->getHeader('Accept')); + static::assertSame('GET', $request->getMethod()); + static::assertSame('/download/dataexport/id/' . $key, $request->getUri()->getPath()); + static::assertSame(['*'], $request->getHeader('Accept')); - $this->assertFileEquals($sampleFile, $downloadedFile); + static::assertFileEquals($sampleFile, $downloadedFile); } - public function testDownloadUnknownType() + public function testDownloadUnknownType(): void { $this->expectException(Exception\InvalidArgumentException::class); $this->expectExceptionMessage('Invalid download type specified'); @@ -289,59 +289,79 @@ public function testDownloadUnknownType() $this->helper->downloadFile('unknown', 123); } - /** - * Use exception to stop method early - */ - public function testDownloadTmpFileLocation() + public function testDownloadTmpFileLocation(): void { - $this->expectException(\Exception::class); - $fopenMock = $this->getFunctionMock(__NAMESPACE__, 'fopen'); - $fopenMock->expects($this->once()) - ->with($this->stringStartsWith(realpath(sys_get_temp_dir())), 'w') + $fopenMock->expects(static::once()) + ->with(static::stringStartsWith(realpath(sys_get_temp_dir())), 'w') + // Return false to exit early ->willReturn(false); - $this->helper->downloadFile('file', 123); + $errorMock = $this->getFunctionMock(__NAMESPACE__, 'error_get_last'); + $errorMock->expects(static::once()) + ->willReturn([ + 'message' => 'exit early', + ]); + + try { + $this->helper->downloadFile('file', 123); + } catch (Exception\RuntimeException) { + // Ignore exception from exiting early + } } - /** - * Use exception to stop method early - */ - public function testDownloadTmpFileLocationCustom() + public function testDownloadTmpFileLocationCustom(): void { - $this->expectException(\Exception::class); - $directory = __DIR__ . '/__files'; $fopenMock = $this->getFunctionMock(__NAMESPACE__, 'fopen'); - $fopenMock->expects($this->once()) - ->with($this->stringStartsWith($directory), 'w') + $fopenMock->expects(static::once()) + ->with(static::stringStartsWith($directory), 'w') + // Return false to exit early ->willReturn(false); - $this->helper->downloadFile('file', 123, [ - 'dir' => $directory, - ]); + $errorMock = $this->getFunctionMock(__NAMESPACE__, 'error_get_last'); + $errorMock->expects(static::once()) + ->willReturn([ + 'message' => 'exit early', + ]); + + try { + $this->helper->downloadFile('file', 123, [ + 'dir' => $directory, + ]); + } catch (Exception\RuntimeException) { + // Ignore exception from exiting early + } } - public function testDownloadTmpFileUnableToOpen() + public function testDownloadTmpFileUnableToOpen(): void { + $reasonMessage = sha1(uniqid('error')); + $this->expectException(Exception\RuntimeException::class); - $this->expectExceptionMessage('Unable to open local file'); + $this->expectExceptionMessage('Unable to open local file: ' . $reasonMessage); $fopenMock = $this->getFunctionMock(__NAMESPACE__, 'fopen'); - $fopenMock->expects($this->once()) + $fopenMock->expects(static::once()) ->willReturn(false); + $errorMock = $this->getFunctionMock(__NAMESPACE__, 'error_get_last'); + $errorMock->expects(static::once()) + ->willReturn([ + 'message' => $reasonMessage, + ]); + $this->helper->downloadFile('file', 123); } - public function testDownloadTmpFileDeletedRequestError() + public function testDownloadTmpFileDeletedRequestError(): void { $this->expectException(ClientException::class); $this->expectExceptionMessage('400 Bad Request'); $this->mockHandler->append( - new Response(400, [], 'Error') + new Response(400, [], 'Error'), ); $directory = __DIR__ . '/__files'; @@ -350,7 +370,7 @@ public function testDownloadTmpFileDeletedRequestError() $tmpResource = null; $fopenMock = $this->getFunctionMock(__NAMESPACE__, 'fopen'); - $fopenMock->expects($this->once()) + $fopenMock->expects(static::once()) ->willReturnCallback(function ($filename, $mode) use (&$tmpFile, &$tmpResource) { $tmpFile = $filename; $tmpResource = \fopen($filename, $mode); @@ -358,16 +378,16 @@ public function testDownloadTmpFileDeletedRequestError() }); $fcloseMock = $this->getFunctionMock(__NAMESPACE__, 'fclose'); - $fcloseMock->expects($this->once()) - ->willReturnCallback(function ($resource) use (&$tmpResource) { - $this->assertSame($tmpResource, $resource); + $fcloseMock->expects(static::once()) + ->willReturnCallback(function ($resource) use (&$tmpResource): bool { + static::assertSame($tmpResource, $resource); return \fclose($tmpResource); }); $unlinkMock = $this->getFunctionMock(__NAMESPACE__, 'unlink'); - $unlinkMock->expects($this->once()) - ->willReturnCallback(function ($filename) use (&$tmpFile) { - $this->assertSame($tmpFile, $filename); + $unlinkMock->expects(static::once()) + ->willReturnCallback(function ($filename) use (&$tmpFile): bool { + static::assertSame($tmpFile, $filename); return \unlink($tmpFile); }); @@ -376,14 +396,16 @@ public function testDownloadTmpFileDeletedRequestError() ]); } - public function testDownloadTmpFileDeletedWriteError() + public function testDownloadTmpFileDeletedWriteError(): void { + $reasonMessage = sha1(uniqid('error')); + $this->expectException(Exception\RuntimeException::class); - $this->expectExceptionMessage('Unable to write to local file'); + $this->expectExceptionMessage('Unable to write to local file: ' . $reasonMessage); $sampleFile = __DIR__ . '/__files/sample-file.csv'; $this->mockHandler->append( - new Response(200, [], fopen($sampleFile, 'r')) + new Response(200, [], fopen($sampleFile, 'r')), ); $directory = __DIR__ . '/__files'; @@ -392,7 +414,7 @@ public function testDownloadTmpFileDeletedWriteError() $tmpResource = null; $fopenMock = $this->getFunctionMock(__NAMESPACE__, 'fopen'); - $fopenMock->expects($this->once()) + $fopenMock->expects(static::once()) ->willReturnCallback(function ($filename, $mode) use (&$tmpFile, &$tmpResource) { $tmpFile = $filename; $tmpResource = \fopen($filename, $mode); @@ -400,20 +422,26 @@ public function testDownloadTmpFileDeletedWriteError() }); $fwriteMock = $this->getFunctionMock(__NAMESPACE__, 'fwrite'); - $fwriteMock->expects($this->once()) + $fwriteMock->expects(static::once()) ->willReturn(false); + $errorMock = $this->getFunctionMock(__NAMESPACE__, 'error_get_last'); + $errorMock->expects(static::once()) + ->willReturn([ + 'message' => $reasonMessage, + ]); + $fcloseMock = $this->getFunctionMock(__NAMESPACE__, 'fclose'); - $fcloseMock->expects($this->once()) - ->willReturnCallback(function ($resource) use (&$tmpResource) { - $this->assertSame($tmpResource, $resource); + $fcloseMock->expects(static::once()) + ->willReturnCallback(function ($resource) use (&$tmpResource): bool { + static::assertSame($tmpResource, $resource); return \fclose($tmpResource); }); $unlinkMock = $this->getFunctionMock(__NAMESPACE__, 'unlink'); - $unlinkMock->expects($this->once()) - ->willReturnCallback(function ($filename) use (&$tmpFile) { - $this->assertSame($tmpFile, $filename); + $unlinkMock->expects(static::once()) + ->willReturnCallback(function ($filename) use (&$tmpFile): bool { + static::assertSame($tmpFile, $filename); return \unlink($tmpFile); }); diff --git a/tests/MiddlewareTest.php b/tests/MiddlewareTest.php index d64c72b..ecb4152 100644 --- a/tests/MiddlewareTest.php +++ b/tests/MiddlewareTest.php @@ -24,20 +24,11 @@ */ class MiddlewareTest extends TestCase { - /** - * @var GuzzleClient - */ - private $httpClient; + private GuzzleClient $httpClient; - /** - * @var MockHandler - */ - private $mockHandler; + private MockHandler $mockHandler; - /** - * @var HandlerStack - */ - private $handlerStack; + private HandlerStack $handlerStack; protected function setUp(): void { @@ -50,25 +41,24 @@ protected function setUp(): void ]); } - public function testWarningLoggerCreatesLog() + public function testWarningLoggerCreatesLog(): void { $warningMsg = 'dummyMethod Deprecated: some example description'; $warning = "299 MxmApi/v100 \"{$warningMsg}\""; - // @todo phpunit > v7, change to `expectDeprecation()` etc. - // Requires convertDeprecationsToExceptions='true' in PHPUnit config - if (version_compare(\PHPUnit\Runner\Version::id(), '8.0.0') < 0) { - // PHPUnit v7 - $this->expectException(\PHPUnit\Framework\Error\Deprecated::class); - } else { - // PHPUnit v8+ - $this->expectDeprecation(); - } + // Capture deprecation error + $originalErrorLevel = error_reporting(); + error_reporting(E_ALL); + set_error_handler(static function (int $errno, string $errstr): never { + throw new \Exception($errstr, $errno); + }, E_USER_DEPRECATED); + + $this->expectException(\Exception::class); $this->expectExceptionMessage($warningMsg); /** @var LoggerInterface|MockObject $logger */ $logger = $this->createMock(LoggerInterface::class); - $logger->expects($this->once()) + $logger->expects(static::once()) ->method('warning') ->with($warningMsg); @@ -79,23 +69,28 @@ public function testWarningLoggerCreatesLog() [ 'Warning' => $warning, ], - json_encode('OK') - ) + json_encode('OK'), + ), ); $service = new Service('dummy_service', $this->httpClient); - $service->dummyMethod(); + try { + $service->dummyMethod(); + } finally { + error_reporting($originalErrorLevel); + restore_error_handler(); + } } - public function testWarningLoggerSkipsNoAgent() + public function testWarningLoggerSkipsNoAgent(): void { $warningMsg = 'some warning with no agent'; $warning = "299 - \"{$warningMsg}\""; /** @var LoggerInterface|MockObject $logger */ $logger = $this->createMock(LoggerInterface::class); - $logger->expects($this->never()) - ->method($this->anything()); + $logger->expects(static::never()) + ->method(static::anything()); Middleware::addWarningLogging($this->handlerStack, $logger); $this->mockHandler->append( @@ -104,23 +99,23 @@ public function testWarningLoggerSkipsNoAgent() [ 'Warning' => $warning, ], - json_encode('OK') - ) + json_encode('OK'), + ), ); $service = new Service('dummy_service', $this->httpClient); $service->dummyMethod(); } - public function testWarningLoggerSkipsWrongAgent() + public function testWarningLoggerSkipsWrongAgent(): void { $warningMsg = 'some other system'; $warning = "299 other/1.2.3 \"{$warningMsg}\""; /** @var LoggerInterface|MockObject $logger */ $logger = $this->createMock(LoggerInterface::class); - $logger->expects($this->never()) - ->method($this->anything()); + $logger->expects(static::never()) + ->method(static::anything()); Middleware::addWarningLogging($this->handlerStack, $logger); $this->mockHandler->append( @@ -129,23 +124,23 @@ public function testWarningLoggerSkipsWrongAgent() [ 'Warning' => $warning, ], - json_encode('OK') - ) + json_encode('OK'), + ), ); $service = new Service('dummy_service', $this->httpClient); $service->dummyMethod(); } - public function testWarningLoggerSkipsWrongCode() + public function testWarningLoggerSkipsWrongCode(): void { $warningMsg = 'something which looks like Maxemail'; $warning = "199 MxmApi/v100 \"{$warningMsg}\""; /** @var LoggerInterface|MockObject $logger */ $logger = $this->createMock(LoggerInterface::class); - $logger->expects($this->never()) - ->method($this->anything()); + $logger->expects(static::never()) + ->method(static::anything()); Middleware::addWarningLogging($this->handlerStack, $logger); $this->mockHandler->append( @@ -154,70 +149,70 @@ public function testWarningLoggerSkipsWrongCode() [ 'Warning' => $warning, ], - json_encode('OK') - ) + json_encode('OK'), + ), ); $service = new Service('dummy_service', $this->httpClient); $service->dummyMethod(); } - public function testWarningLoggerSkipsNoWarning() + public function testWarningLoggerSkipsNoWarning(): void { /** @var LoggerInterface|MockObject $logger */ $logger = $this->createMock(LoggerInterface::class); - $logger->expects($this->never()) - ->method($this->anything()); + $logger->expects(static::never()) + ->method(static::anything()); Middleware::addWarningLogging($this->handlerStack, $logger); $this->mockHandler->append( - new Response(200, [], json_encode('OK')) + new Response(200, [], json_encode('OK')), ); $service = new Service('dummy_service', $this->httpClient); $service->dummyMethod(); } - public function testErrorHandlerSkips200() + public function testErrorHandlerSkips200(): void { Middleware::addMaxemailErrorParser($this->handlerStack); $this->mockHandler->append( - new Response(200, [], json_encode('OK')) + new Response(200, [], json_encode('OK')), ); $service = new Service('dummy_service', $this->httpClient); $result = $service->dummyMethod(); - $this->assertSame('OK', $result); + static::assertSame('OK', $result); } - public function testErrorHandlerSkips500() + public function testErrorHandlerSkips500(): void { $this->expectException(ServerException::class); Middleware::addMaxemailErrorParser($this->handlerStack); $this->mockHandler->append( - new Response(500, [], 'Internal server error') + new Response(500, [], 'Internal server error'), ); $service = new Service('dummy_service', $this->httpClient); $service->dummyMethod(); } - public function testErrorHandlerHandles400Standard() + public function testErrorHandlerHandles400Standard(): void { $this->expectException(ClientException::class); Middleware::addMaxemailErrorParser($this->handlerStack); $this->mockHandler->append( - new Response(400, [], 'Error') + new Response(400, [], 'Error'), ); $service = new Service('dummy_service', $this->httpClient); $service->dummyMethod(); } - public function testErrorHandlerHandles400Maxemail() + public function testErrorHandlerHandles400Maxemail(): void { $errorMsg = 'Maxemail API error message'; @@ -230,7 +225,7 @@ public function testErrorHandlerHandles400Maxemail() 'msg' => $errorMsg, ]; $this->mockHandler->append( - new Response(400, [], json_encode($mxmError)) + new Response(400, [], json_encode($mxmError)), ); $service = new Service('dummy_service', $this->httpClient); diff --git a/tests/ServiceTest.php b/tests/ServiceTest.php index 321f529..fc6de35 100644 --- a/tests/ServiceTest.php +++ b/tests/ServiceTest.php @@ -21,17 +21,11 @@ */ class ServiceTest extends TestCase { - /** - * @var GuzzleClient - */ - private $httpClient; + private GuzzleClient $httpClient; - /** - * @var MockHandler - */ - private $mockHandler; + private MockHandler $mockHandler; - private $clientHistory = []; + private array $clientHistory = []; protected function setUp(): void { @@ -40,7 +34,7 @@ protected function setUp(): void $stack = HandlerStack::create($this->mockHandler); Middleware::addMaxemailErrorParser($stack); $stack->push( - GuzzleMiddleware::history($this->clientHistory) + GuzzleMiddleware::history($this->clientHistory), ); $this->httpClient = new GuzzleClient([ @@ -49,10 +43,10 @@ protected function setUp(): void ]); } - public function testMagicCallSendsRequest() + public function testMagicCallSendsRequest(): void { $this->mockHandler->append( - new Response(200, [], json_encode('OK')) + new Response(200, [], json_encode('OK')), ); $service = new Service('dummy_service', $this->httpClient); @@ -68,7 +62,7 @@ public function testMagicCallSendsRequest() 'foo' => [ 'bar' => 'bob', ], - ] + ], ); $expectedParams = [ @@ -83,13 +77,13 @@ public function testMagicCallSendsRequest() ]), ]; - $this->assertCount(1, $this->clientHistory); + static::assertCount(1, $this->clientHistory); /** @var Request $request */ $request = $this->clientHistory[0]['request']; - $this->assertSame('POST', $request->getMethod()); - $this->assertSame('/api/json/dummy_service', $request->getUri()->getPath()); - $this->assertSame(http_build_query($expectedParams), (string)$request->getBody()); + static::assertSame('POST', $request->getMethod()); + static::assertSame('/api/json/dummy_service', $request->getUri()->getPath()); + static::assertSame(http_build_query($expectedParams), (string)$request->getBody()); } }