diff --git a/.gitattributes b/.gitattributes index cd0ff7b..0c35ea4 100644 --- a/.gitattributes +++ b/.gitattributes @@ -3,8 +3,6 @@ .github/ export-ignore .gitignore export-ignore .php_cs export-ignore -.scrutinizer.yml export-ignore .styleci.yml export-ignore -phpspec.yml.ci export-ignore -phpspec.yml.dist export-ignore -spec/ export-ignore +phpunit.xml export-ignore +tests export-ignore diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index e4d0050..f7250cc 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -5,12 +5,15 @@ on: pull_request: jobs: - latest: - name: PHP ${{ matrix.php }} Latest + tests: + name: PHP ${{ matrix.php }} ${{ matrix.dependencies }} runs-on: ubuntu-latest strategy: matrix: - php: ['7.1', '7.2', '7.3', '7.4', '8.0'] + php: ['8.2', '8.3', '8.4'] + dependencies: + - "lowest" + - "highest" steps: - name: Checkout code @@ -20,72 +23,12 @@ jobs: uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php }} - tools: composer:v2 - coverage: none - - - name: Install PHP 7 dependencies - run: composer update --prefer-dist --no-interaction --no-progress - if: "matrix.php != '8.0'" - - - name: Install PHP 8 dependencies - run: | - composer require "phpdocumentor/reflection-docblock:^5.2@dev" --no-interaction --no-update - composer update --prefer-dist --prefer-stable --no-interaction --no-progress --ignore-platform-req=php - if: "matrix.php == '8.0'" - - - name: Execute tests - run: composer test - - lowest: - name: PHP ${{ matrix.php }} Lowest - runs-on: ubuntu-latest - strategy: - matrix: - php: ['7.1', '7.2', '7.3', '7.4'] - - steps: - - name: Checkout code - uses: actions/checkout@v2 - - - name: Setup PHP - uses: shivammathur/setup-php@v2 - with: - php-version: ${{ matrix.php }} - tools: composer:v2 coverage: none - name: Install dependencies - run: | - composer require "sebastian/comparator:^3.0.2" --no-interaction --no-update - composer update --prefer-dist --prefer-stable --prefer-lowest --no-interaction --no-progress - - - name: Execute tests - run: composer test - - coverage: - name: Code Coverage - runs-on: ubuntu-latest - - steps: - - name: Checkout code - uses: actions/checkout@v2 - - - name: Setup PHP - uses: shivammathur/setup-php@v2 + uses: "ramsey/composer-install@v3" with: - php-version: 7.4 - tools: composer:v2 - coverage: xdebug + dependency-versions: "${{ matrix.dependencies }}" - - name: Install dependencies - run: | - composer require "friends-of-phpspec/phpspec-code-coverage:^4.3.2" --no-interaction --no-update - composer update --prefer-dist --no-interaction --no-progress - - - name: Execute tests - run: composer test-ci - - - name: Upload coverage - run: | - wget https://scrutinizer-ci.com/ocular.phar - php ocular.phar code-coverage:upload --format=php-clover build/coverage.xml + - name: Run tests + run: composer test diff --git a/.gitignore b/.gitignore index 7608c4b..90f12a8 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,3 @@ -build/ vendor/ composer.lock -phpspec.yml -phpunit.xml +/.phpunit.cache/ diff --git a/.scrutinizer.yml b/.scrutinizer.yml deleted file mode 100644 index 46a7560..0000000 --- a/.scrutinizer.yml +++ /dev/null @@ -1,8 +0,0 @@ -filter: - paths: [src/*] -checks: - php: - code_rating: true - duplication: true -tools: - external_code_coverage: true diff --git a/composer.json b/composer.json index d916b70..b2d4cd4 100644 --- a/composer.json +++ b/composer.json @@ -11,27 +11,31 @@ } ], "require": { - "php": "^7.0 || ^8.0", - "psr/log": "^1.0 || ^2 || ^3", - "php-http/client-common": "^1.9 || ^2.0", - "php-http/message": "^1.0", - "symfony/polyfill-php73": "^1.17" - }, - "require-dev": { - "phpspec/phpspec": "^5.1 || ^6.0" + "php": "^8.2", + "psr/log": "^2 || ^3", + "php-http/client-common": "^2.0", + "php-http/message": "^1.13" }, "autoload": { "psr-4": { "Http\\Client\\Common\\Plugin\\": "src/" } }, + "autoload-dev": { + "psr-4": { + "Http\\Client\\Common\\Plugin\\": "tests/" + } + }, "scripts": { - "test": "vendor/bin/phpspec run", - "test-ci": "vendor/bin/phpspec run -c phpspec.yml.ci" + "test": "vendor/bin/phpunit" }, "extra": { "branch-alias": { "dev-master": "1.2-dev" } + }, + "require-dev": { + "phpunit/phpunit": "^11.4.3", + "nyholm/psr7": "^1.8" } } diff --git a/phpspec.yml.ci b/phpspec.yml.ci deleted file mode 100644 index 087f808..0000000 --- a/phpspec.yml.ci +++ /dev/null @@ -1,10 +0,0 @@ -suites: - logger_plugin_suite: - namespace: Http\Client\Common\Plugin - psr4_prefix: Http\Client\Common\Plugin -formatter.name: pretty -extensions: - FriendsOfPhpSpec\PhpSpec\CodeCoverage\CodeCoverageExtension: ~ -code_coverage: - format: clover - output: build/coverage.xml diff --git a/phpspec.yml.dist b/phpspec.yml.dist deleted file mode 100644 index 93dd666..0000000 --- a/phpspec.yml.dist +++ /dev/null @@ -1,5 +0,0 @@ -suites: - logger_plugin_suite: - namespace: Http\Client\Common\Plugin - psr4_prefix: Http\Client\Common\Plugin -formatter.name: pretty diff --git a/phpunit.xml b/phpunit.xml new file mode 100644 index 0000000..a825b0e --- /dev/null +++ b/phpunit.xml @@ -0,0 +1,26 @@ + + + + + tests + + + + + + src + + + diff --git a/spec/LoggerPluginSpec.php b/spec/LoggerPluginSpec.php deleted file mode 100644 index 91325a1..0000000 --- a/spec/LoggerPluginSpec.php +++ /dev/null @@ -1,149 +0,0 @@ -beConstructedWith($logger, $formatter); - } - - function it_is_initializable() - { - $this->shouldHaveType(LoggerPlugin::class); - } - - function it_is_a_plugin() - { - $this->shouldImplement(Plugin::class); - } - - function it_logs_request_and_response( - LoggerInterface $logger, - Formatter $formatter, - RequestInterface $request, - ResponseInterface $response - ) { - $formatter->formatRequest($request)->willReturn('GET / 1.1'); - if (method_exists(Formatter\SimpleFormatter::class, 'formatResponseForRequest')) { - $formatter->formatResponseForRequest($response, $request)->willReturn('200 OK 1.1'); - } else { - $formatter->formatResponse($response)->willReturn('200 OK 1.1'); - } - - $logger->info( - "Sending request:\nGET / 1.1", - Argument::that( - function(array $context) { - return is_string($context['uid']); - } - ) - )->shouldBeCalled(); - $logger->info( - "Received response:\n200 OK 1.1", - Argument::that( - function(array $context) { - return is_int($context['milliseconds']) - && is_string($context['uid']) - ; - } - ) - )->shouldBeCalled(); - - $next = function () use ($response) { - return new FulfilledPromise($response->getWrappedObject()); - }; - - $this->handleRequest($request, $next, function () {}); - } - - function it_logs_exception(LoggerInterface $logger, Formatter $formatter, RequestInterface $request) - { - $formatter->formatRequest($request)->willReturn('GET / 1.1'); - - $exception = new NetworkException('Cannot connect', $request->getWrappedObject()); - - $logger->info( - "Sending request:\nGET / 1.1", - Argument::that( - function(array $context) { - return is_string($context['uid']); - } - ) - )->shouldBeCalled(); - $logger->error( - "Error:\nCannot connect\nwhen sending request:\nGET / 1.1", - Argument::that( - function(array $context) use ($exception) { - return $context['exception'] === $exception - && is_int($context['milliseconds']) - && is_string($context['uid']) - ; - } - ) - )->shouldBeCalled(); - - $next = function () use ($exception) { - return new RejectedPromise($exception); - }; - - $this->handleRequest($request, $next, function () {}); - } - - function it_logs_response_within_exception( - LoggerInterface $logger, - Formatter $formatter, - RequestInterface $request, - ResponseInterface $response - ) { - $formatter->formatRequest($request)->willReturn('GET / 1.1'); - if (method_exists(Formatter\SimpleFormatter::class, 'formatResponseForRequest')) { - $formatter->formatResponseForRequest($response, $request)->willReturn('403 Forbidden 1.1'); - } else { - $formatter->formatResponse($response)->willReturn('403 Forbidden 1.1'); - } - - $exception = new HttpException('Forbidden', $request->getWrappedObject(), $response->getWrappedObject()); - - $logger->info( - "Sending request:\nGET / 1.1", - Argument::that( - function(array $context) { - return is_string($context['uid']); - } - ) - )->shouldBeCalled(); - $logger->error( - "Error:\nForbidden\nwith response:\n403 Forbidden 1.1", - Argument::that( - function(array $context) use ($exception) { - return $context['exception'] === $exception - && is_int($context['milliseconds']) - && is_string($context['uid']) - ; - } - ) - )->shouldBeCalled(); - - $next = function () use ($exception) { - return new RejectedPromise($exception); - }; - - $this->handleRequest($request, $next, function () {}); - } -} diff --git a/src/LoggerPlugin.php b/src/LoggerPlugin.php index e6126f7..87dc914 100644 --- a/src/LoggerPlugin.php +++ b/src/LoggerPlugin.php @@ -6,6 +6,7 @@ use Http\Client\Exception; use Http\Message\Formatter; use Http\Message\Formatter\SimpleFormatter; +use Http\Promise\Promise; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; use Psr\Log\LoggerInterface; @@ -15,21 +16,18 @@ * * @author Joel Wurtz */ -final class LoggerPlugin implements Plugin +final readonly class LoggerPlugin implements Plugin { - use VersionBridgePlugin; + private Formatter $formatter; - private $logger; - - private $formatter; - - public function __construct(LoggerInterface $logger, Formatter $formatter = null) - { - $this->logger = $logger; - $this->formatter = $formatter ?: new SimpleFormatter(); + public function __construct( + private LoggerInterface $logger, + ?Formatter $formatter = null + ) { + $this->formatter = $formatter ?? new SimpleFormatter(); } - protected function doHandleRequest(RequestInterface $request, callable $next, callable $first) + public function handleRequest(RequestInterface $request, callable $next, callable $first): Promise { $start = hrtime(true) / 1E6; $uid = uniqid('', true); @@ -37,9 +35,7 @@ protected function doHandleRequest(RequestInterface $request, callable $next, ca return $next($request)->then(function (ResponseInterface $response) use ($start, $uid, $request) { $milliseconds = (int) round(hrtime(true) / 1E6 - $start); - $formattedResponse = method_exists($this->formatter, 'formatResponseForRequest') - ? $this->formatter->formatResponseForRequest($response, $request) - : $this->formatter->formatResponse($response); + $formattedResponse = $this->formatter->formatResponseForRequest($response, $request); $this->logger->info( sprintf("Received response:\n%s", $formattedResponse), [ @@ -52,9 +48,7 @@ protected function doHandleRequest(RequestInterface $request, callable $next, ca }, function (Exception $exception) use ($request, $start, $uid) { $milliseconds = (int) round(hrtime(true) / 1E6 - $start); if ($exception instanceof Exception\HttpException) { - $formattedResponse = method_exists($this->formatter, 'formatResponseForRequest') - ? $this->formatter->formatResponseForRequest($exception->getResponse(), $exception->getRequest()) - : $this->formatter->formatResponse($exception->getResponse()); + $formattedResponse = $this->formatter->formatResponseForRequest($exception->getResponse(), $exception->getRequest()); $this->logger->error( sprintf("Error:\n%s\nwith response:\n%s", $exception->getMessage(), $formattedResponse), [ diff --git a/tests/LoggerPluginTest.php b/tests/LoggerPluginTest.php new file mode 100644 index 0000000..78d56a6 --- /dev/null +++ b/tests/LoggerPluginTest.php @@ -0,0 +1,83 @@ +logger = new TestLogger(); + $this->plugin = new LoggerPlugin($this->logger, new SimpleFormatter()); + } + + public function testLogsRequestAndResponse() + { + $response = new Response(); + + $actualResponse = $this->plugin->handleRequest( + new Request('GET', 'http://example.com/'), + fn (RequestInterface $req) => new FulfilledPromise($response), + function () {} + )->wait(); + + self::assertSame($response, $actualResponse); + + self::assertCount(2, $this->logger->logMessages); + self::assertSame("Sending request:\nGET http://example.com/ 1.1", $this->logger->logMessages[0]['info']); + self::assertSame("Received response:\n200 OK 1.1", $this->logger->logMessages[1]['info']); + } + + public function testLogsRequestException() + { + $this->expectException(NetworkException::class); + + try { + $this->plugin->handleRequest( + new Request('GET', 'http://example.com/'), + fn (RequestInterface $req) => new RejectedPromise(new NetworkException('Network error', $req)), + function () {} + )->wait(); + } catch (NetworkException $exception) { + self::assertCount(2, $this->logger->logMessages); + self::assertSame("Sending request:\nGET http://example.com/ 1.1", $this->logger->logMessages[0]['info']); + self::assertSame("Error:\nNetwork error\nwhen sending request:\nGET http://example.com/ 1.1", $this->logger->logMessages[1]['error']); + + throw $exception; + } + } + + public function testLogsHttpException() + { + $this->expectException(HttpException::class); + + try { + $this->plugin->handleRequest( + new Request('GET', 'http://example.com/'), + fn (RequestInterface $req) => new RejectedPromise(new HttpException('Not Found', $req, new Response())), + function () {} + )->wait(); + } catch (HttpException $exception) { + // Expected + $this->assertCount(2, $this->logger->logMessages); + $this->assertSame("Sending request:\nGET http://example.com/ 1.1", $this->logger->logMessages[0]['info']); + // Ensure there's an error log for the exception + $this->assertStringContainsString("Error:\nNot Found", $this->logger->logMessages[1]['error']); + throw $exception; + } + } +} diff --git a/tests/TestLogger.php b/tests/TestLogger.php new file mode 100644 index 0000000..f6c69b9 --- /dev/null +++ b/tests/TestLogger.php @@ -0,0 +1,55 @@ +logMessages[] = ['emergency' => $message, 'context' => $context]; + } + + public function alert($message, array $context = []): void + { + $this->logMessages[] = ['alert' => $message, 'context' => $context]; + } + + public function critical($message, array $context = []): void + { + $this->logMessages[] = ['critical' => $message, 'context' => $context]; + } + + public function error($message, array $context = []): void + { + $this->logMessages[] = ['error' => $message, 'context' => $context]; + } + + public function warning($message, array $context = []): void + { + $this->logMessages[] = ['warning' => $message, 'context' => $context]; + } + + public function notice($message, array $context = []): void + { + $this->logMessages[] = ['notice' => $message, 'context' => $context]; + } + + public function info($message, array $context = []): void + { + $this->logMessages[] = ['info' => $message, 'context' => $context]; + } + + public function debug($message, array $context = []): void + { + $this->logMessages[] = ['debug' => $message, 'context' => $context]; + } + + public function log($level, $message, array $context = []): void + { + $this->logMessages[] = [$level => $message, 'context' => $context]; + } +}