From fe726943de0d7d67014ec29ee681e9de050fd88b Mon Sep 17 00:00:00 2001 From: Pascal Baljet Date: Thu, 14 Mar 2024 09:52:42 +0100 Subject: [PATCH 01/10] Added CI workflow + Streamlined Service Provider --- .github/workflows/ci.yml | 43 +++++++++++++++++++++++ src/Providers/EventServiceProvider.php | 47 -------------------------- src/Providers/ServiceProvider.php | 22 ++++++++++-- 3 files changed, 63 insertions(+), 49 deletions(-) create mode 100644 .github/workflows/ci.yml delete mode 100644 src/Providers/EventServiceProvider.php diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..f447303 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,43 @@ +name: CI + +on: [push] + +jobs: + test: + runs-on: ubuntu-latest + strategy: + fail-fast: true + matrix: + php: [8.3, 8.2, 8.1] + laravel: [11.*, 10.*] + stability: [prefer-lowest, prefer-stable] + include: + - laravel: 11.* + testbench: 9.* + - laravel: 10.* + testbench: 8.* + exclude: + - laravel: 11.* + php: 8.1 + + name: P${{ matrix.php }} - L${{ matrix.laravel }} - ${{ matrix.stability }} - ${{ matrix.os }} + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Setup PHP + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php }} + extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, sqlite, pdo_sqlite, bcmath, soap, intl, gd, exif, iconv, imagick, fileinfo + coverage: xdebug + + - name: Install dependencies + run: | + composer require "laravel/framework:${{ matrix.laravel }}" "orchestra/testbench:${{ matrix.testbench }}" --no-interaction --no-update + composer update --${{ matrix.stability }} --prefer-dist --no-interaction + + - name: Execute tests + run: | + vendor/bin/pest diff --git a/src/Providers/EventServiceProvider.php b/src/Providers/EventServiceProvider.php deleted file mode 100644 index f19e7e7..0000000 --- a/src/Providers/EventServiceProvider.php +++ /dev/null @@ -1,47 +0,0 @@ - [ - LogRequestSending::class, - ], - ResponseReceived::class => [ - LogResponseReceived::class, - ], - - // Saloon - SendingSaloonRequest::class => [ - LogRequestSending::class, - ], - SentSaloonRequest::class => [ - LogResponseReceived::class, - ], - ]; - - /** - * Get the events and handlers. - * - * @return array - */ - public function listens() - { - return config('http-client-global-logger.enabled') ? $this->listen : []; - } -} diff --git a/src/Providers/ServiceProvider.php b/src/Providers/ServiceProvider.php index 0134d2d..bfdf3a5 100644 --- a/src/Providers/ServiceProvider.php +++ b/src/Providers/ServiceProvider.php @@ -2,10 +2,17 @@ namespace Onlime\LaravelHttpClientGlobalLogger\Providers; +use Illuminate\Http\Client\Events\RequestSending; +use Illuminate\Http\Client\Events\ResponseReceived; use Illuminate\Http\Client\PendingRequest; +use Illuminate\Support\Facades\Event; use Illuminate\Support\ServiceProvider as BaseServiceProvider; use Monolog\Handler\StreamHandler; +use Onlime\LaravelHttpClientGlobalLogger\Listeners\LogRequestSending; +use Onlime\LaravelHttpClientGlobalLogger\Listeners\LogResponseReceived; use Onlime\LaravelHttpClientGlobalLogger\Mixins\PendingRequestMixin; +use Saloon\Laravel\Events\SendingSaloonRequest; +use Saloon\Laravel\Events\SentSaloonRequest; class ServiceProvider extends BaseServiceProvider { @@ -18,15 +25,26 @@ public function register() if (config('http-client-global-logger.enabled') && ! config('http-client-global-logger.mixin')) { - $this->app->register(EventServiceProvider::class); + $this->registerEventListeners(); } } + private function registerEventListeners(): void + { + // Laravel HTTP Client + Event::listen(RequestSending::class, LogRequestSending::class); + Event::listen(ResponseReceived::class, LogResponseReceived::class); + + // Saloon + Event::listen(SendingSaloonRequest::class, LogRequestSending::class); + Event::listen(SentSaloonRequest::class, LogResponseReceived::class); + } + public function boot() { $this->publishes([ $this->configFileLocation() => config_path('http-client-global-logger.php'), - ], 'config'); + ], 'http-client-global-logger'); $channel = config('http-client-global-logger.channel'); if (! array_key_exists($channel, config('logging.channels'))) { From 6a28eb92b0e5bae27e4923e2e87e19a446d6bdaf Mon Sep 17 00:00:00 2001 From: Pascal Baljet Date: Thu, 14 Mar 2024 09:53:48 +0100 Subject: [PATCH 02/10] Update composer.json --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index ccd68d7..e2a72e9 100644 --- a/composer.json +++ b/composer.json @@ -39,7 +39,7 @@ "pestphp/pest": "^2.33", "pestphp/pest-plugin-arch": "^2.0", "pestphp/pest-plugin-laravel": "^2.0", - "saloonphp/laravel-http-sender": "^2.0", + "saloonphp/laravel-http-sender": "^2.0|^3.0", "saloonphp/laravel-plugin": "^3.0" }, "suggest": { From 7291845b9c96ffd40693c348b7efd05c706e7b5a Mon Sep 17 00:00:00 2001 From: Pascal Baljet Date: Thu, 14 Mar 2024 09:56:10 +0100 Subject: [PATCH 03/10] Update composer.json --- composer.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/composer.json b/composer.json index e2a72e9..cc2575f 100644 --- a/composer.json +++ b/composer.json @@ -27,8 +27,7 @@ ], "require": { "php": "^8.1", - "guzzlehttp/guzzle": "^7.6", - "illuminate/http": "^10.0|^11.0", + "illuminate/http": "^10.32|^11.0", "illuminate/support": "^10.0|^11.0", "monolog/monolog": "^3.0" }, From eb404d415159ca8f2366b45c555b11b0796ab104 Mon Sep 17 00:00:00 2001 From: Pascal Baljet Date: Thu, 14 Mar 2024 10:20:08 +0100 Subject: [PATCH 04/10] Added test for `ServiceProvider` --- tests/ServiceProviderTest.php | 77 +++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 tests/ServiceProviderTest.php diff --git a/tests/ServiceProviderTest.php b/tests/ServiceProviderTest.php new file mode 100644 index 0000000..3fbe3fc --- /dev/null +++ b/tests/ServiceProviderTest.php @@ -0,0 +1,77 @@ + true, + 'http-client-global-logger.mixin' => false, + ]); + + (new ServiceProvider(app()))->register(); + + $listeners = Event::getRawListeners()[$eventName] ?? []; + + expect($listeners)->not->toBeEmpty() + ->and($listeners)->toContain($listenerName); +})->with([ + [RequestSending::class, LogRequestSending::class], + [ResponseReceived::class, LogResponseReceived::class], + [SendingSaloonRequest::class, LogRequestSending::class], + [SentSaloonRequest::class, LogResponseReceived::class], +]); + +it('merges the default config', function () { + $config = config('http-client-global-logger'); + + expect($config)->toBeArray(); + + foreach ([ + 'enabled', + 'mixin', + 'channel', + 'logfile', + 'format', + 'obfuscate', + 'trim_response_body', + ] as $key) { + expect($config)->toHaveKey($key); + } +}); + +it('can publish the config file', function () { + @unlink(config_path('http-client-global-logger.php')); + + $this->artisan('vendor:publish', ['--tag' => 'http-client-global-logger']); + + $this->assertFileExists(config_path('http-client-global-logger.php')); +}); + +it('configures the log channel', function () { + $defaultChannel = config('http-client-global-logger.channel'); + + $config = config('logging.channels.'.$defaultChannel); + + expect($config)->toBeArray(); +}); + +it('can register the mixin on the PendingRequest class', function (bool $mixin) { + config([ + 'http-client-global-logger.mixin' => $mixin, + ]); + + (new ServiceProvider(app()))->boot(); + + expect(PendingRequest::hasMacro('log'))->toBe($mixin); +})->with([ + true, + false, +]); From 70012d53f0180d2599f5a1b08949f2ecbcc513e9 Mon Sep 17 00:00:00 2001 From: Pascal Baljet Date: Thu, 14 Mar 2024 10:31:58 +0100 Subject: [PATCH 05/10] Added test for `PendingRequestMixin` --- src/Mixins/PendingRequestMixin.php | 5 +++-- tests/PendingRequestMixinTest.php | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 tests/PendingRequestMixinTest.php diff --git a/src/Mixins/PendingRequestMixin.php b/src/Mixins/PendingRequestMixin.php index 2ecbffb..b8b5b77 100644 --- a/src/Mixins/PendingRequestMixin.php +++ b/src/Mixins/PendingRequestMixin.php @@ -33,10 +33,11 @@ public function log() if (! is_null($name)) { $logger = $logger->withName($name); } - foreach ($messageFormats as $format) { + foreach ($messageFormats as $key => $format) { // We'll use unshift instead of push, to add the middleware to the bottom of the stack, not the top $stack->unshift( - Middleware::log($logger, new MessageFormatter($format)) + Middleware::log($logger, new MessageFormatter($format)), + 'http-client-global-logger-'.$key ); } diff --git a/tests/PendingRequestMixinTest.php b/tests/PendingRequestMixinTest.php new file mode 100644 index 0000000..d7ed38c --- /dev/null +++ b/tests/PendingRequestMixinTest.php @@ -0,0 +1,21 @@ + 'true'])->log(); + + /** @var HandlerStack $handler */ + $handler = $pendingRequest->getOptions()['handler']; + expect($handler)->toBeInstanceOf(HandlerStack::class); + + // It adds a logger for each format (see config('http-client-global-logger.format')) + expect($handler->hasHandler('http-client-global-logger-0'))->toBeTrue() + ->and($handler->hasHandler('http-client-global-logger-1'))->toBeTrue(); +}); From 13223e9bf18b5d3220ac9a3e33453650447427d3 Mon Sep 17 00:00:00 2001 From: Pascal Baljet Date: Thu, 14 Mar 2024 10:33:22 +0100 Subject: [PATCH 06/10] Update ServiceProviderTest.php --- tests/ServiceProviderTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/ServiceProviderTest.php b/tests/ServiceProviderTest.php index 3fbe3fc..9dd4491 100644 --- a/tests/ServiceProviderTest.php +++ b/tests/ServiceProviderTest.php @@ -64,6 +64,8 @@ }); it('can register the mixin on the PendingRequest class', function (bool $mixin) { + PendingRequest::flushMacros(); + config([ 'http-client-global-logger.mixin' => $mixin, ]); From aff4fcb05cd1af2a2f15796e6049ddd18cb438e6 Mon Sep 17 00:00:00 2001 From: Pascal Baljet Date: Thu, 14 Mar 2024 12:32:09 +0100 Subject: [PATCH 07/10] Added test for `EventHelper` to resolve PSR requests/responses --- tests/EventHelperTest.php | 113 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 tests/EventHelperTest.php diff --git a/tests/EventHelperTest.php b/tests/EventHelperTest.php new file mode 100644 index 0000000..622c90e --- /dev/null +++ b/tests/EventHelperTest.php @@ -0,0 +1,113 @@ + 'Bar']); +} + +function laravelHttpRequest(): ClientRequest +{ + return new ClientRequest(psr7Request()); +} + +function laravelHttpResponse(): ClientResponse +{ + return new ClientResponse(psr7Response()); +} + +it('resolves the psr request from Laravel\'s RequestSending event', function () { + $psrRequest = EventHelper::getPsrRequest(new RequestSending(laravelHttpRequest())); + + expect($psrRequest)->toBeInstanceOf(Psr7Request::class) + ->and($psrRequest->getMethod())->toBe('GET') + ->and($psrRequest->getUri()->__toString())->toBe('http://localhost/test'); +}); + +it('resolves the psr request from Laravel\'s ResponseReceived event', function () { + $psrRequest = EventHelper::getPsrRequest(new ResponseReceived(laravelHttpRequest(), laravelHttpResponse())); + + expect($psrRequest)->toBeInstanceOf(Psr7Request::class) + ->and($psrRequest->getMethod())->toBe('GET') + ->and($psrRequest->getUri()->__toString())->toBe('http://localhost/test'); +}); + +it('resolves the psr response from Laravel\'s ResponseReceived event', function () { + $psrResponse = EventHelper::getPsrResponse(new ResponseReceived(laravelHttpRequest(), laravelHttpResponse())); + + expect($psrResponse)->toBeInstanceOf(Psr7Response::class) + ->and($psrResponse->getStatusCode())->toBe(200) + ->and($psrResponse->getHeaderLine('X-Foo'))->toBe('Bar'); +}); + +function saloonPendingRequest(): PendingRequest +{ + return new PendingRequest( + new class extends Connector + { + public function resolveBaseUrl(): string + { + return 'http://localhost'; + } + }, + new class extends Request + { + protected Method $method = Method::GET; + + public function resolveEndpoint(): string + { + return '/test'; + } + } + ); +} + +function saloonResponse(PendingRequest $pendingRequest): Response +{ + return new Response(psr7Response(), $pendingRequest, $pendingRequest->createPsrRequest()); +} + +it('resolves the psr request from Saloon\'s SendingSaloonRequest event', function () { + $psrRequest = EventHelper::getPsrRequest(new SendingSaloonRequest(saloonPendingRequest())); + + expect($psrRequest)->toBeInstanceOf(Psr7Request::class) + ->and($psrRequest->getMethod())->toBe('GET') + ->and($psrRequest->getUri()->__toString())->toBe('http://localhost/test'); +}); + +it('resolves the psr request from Saloon\'s SentSaloonRequest event', function () { + $pendingRequest = saloonPendingRequest(); + $psrRequest = EventHelper::getPsrRequest(new SentSaloonRequest($pendingRequest, saloonResponse($pendingRequest))); + + expect($psrRequest)->toBeInstanceOf(Psr7Request::class) + ->and($psrRequest->getMethod())->toBe('GET') + ->and($psrRequest->getUri()->__toString())->toBe('http://localhost/test'); +}); + +it('resolves the psr response from Saloon\'s SentSaloonRequest event', function () { + $pendingRequest = saloonPendingRequest(); + $psrResponse = EventHelper::getPsrResponse(new SentSaloonRequest($pendingRequest, saloonResponse($pendingRequest))); + + expect($psrResponse)->toBeInstanceOf(Psr7Response::class) + ->and($psrResponse->getStatusCode())->toBe(200) + ->and($psrResponse->getHeaderLine('X-Foo'))->toBe('Bar'); +}); From d5f78995af9fd20d6651ef91203e6eb64c7d81be Mon Sep 17 00:00:00 2001 From: Pascal Baljet Date: Thu, 14 Mar 2024 14:08:10 +0100 Subject: [PATCH 08/10] Fix `PendingRequestMixinTest` --- tests/PendingRequestMixinTest.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/PendingRequestMixinTest.php b/tests/PendingRequestMixinTest.php index d7ed38c..1512562 100644 --- a/tests/PendingRequestMixinTest.php +++ b/tests/PendingRequestMixinTest.php @@ -15,7 +15,11 @@ $handler = $pendingRequest->getOptions()['handler']; expect($handler)->toBeInstanceOf(HandlerStack::class); + // String representation of the stack + $stack = $handler->__toString(); + // It adds a logger for each format (see config('http-client-global-logger.format')) - expect($handler->hasHandler('http-client-global-logger-0'))->toBeTrue() - ->and($handler->hasHandler('http-client-global-logger-1'))->toBeTrue(); + expect($stack)->toContain("Name: 'http-client-global-logger-0'") + ->and($stack)->toContain("Name: 'http-client-global-logger-1'") + ->and($stack)->not->toContain("Name: 'http-client-global-logger-2'"); }); From 51d1c9cd1f0b7529c2ce1dc48bea6bfb01db647a Mon Sep 17 00:00:00 2001 From: Pascal Baljet Date: Thu, 14 Mar 2024 15:27:43 +0100 Subject: [PATCH 09/10] Refactored `PendingRequestMixinTest` --- composer.json | 5 +++-- tests/PendingRequestMixinTest.php | 14 ++++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/composer.json b/composer.json index cc2575f..c798ca8 100644 --- a/composer.json +++ b/composer.json @@ -39,7 +39,8 @@ "pestphp/pest-plugin-arch": "^2.0", "pestphp/pest-plugin-laravel": "^2.0", "saloonphp/laravel-http-sender": "^2.0|^3.0", - "saloonphp/laravel-plugin": "^3.0" + "saloonphp/laravel-plugin": "^3.0", + "spatie/invade": "^2.0" }, "suggest": { "saloonphp/laravel-plugin": "To support logging of Saloon events (^3.0)" @@ -74,4 +75,4 @@ }, "minimum-stability": "stable", "prefer-stable": true -} \ No newline at end of file +} diff --git a/tests/PendingRequestMixinTest.php b/tests/PendingRequestMixinTest.php index 1512562..ef32d05 100644 --- a/tests/PendingRequestMixinTest.php +++ b/tests/PendingRequestMixinTest.php @@ -2,6 +2,7 @@ use GuzzleHttp\HandlerStack; use Illuminate\Http\Client\PendingRequest; +use Illuminate\Support\Arr; use Illuminate\Support\Facades\Http; use Onlime\LaravelHttpClientGlobalLogger\Mixins\PendingRequestMixin; @@ -15,11 +16,12 @@ $handler = $pendingRequest->getOptions()['handler']; expect($handler)->toBeInstanceOf(HandlerStack::class); - // String representation of the stack - $stack = $handler->__toString(); + // We need to invade the HandlerStack to access the stack property + $stack = invade($handler)->stack; + // The first key is the middlware, and the second key is the name of the middleware + $middlewareNames = Arr::pluck($stack, 1); - // It adds a logger for each format (see config('http-client-global-logger.format')) - expect($stack)->toContain("Name: 'http-client-global-logger-0'") - ->and($stack)->toContain("Name: 'http-client-global-logger-1'") - ->and($stack)->not->toContain("Name: 'http-client-global-logger-2'"); + expect($middlewareNames)->toContain('http-client-global-logger-0') + ->and($middlewareNames)->toContain('http-client-global-logger-1') + ->and($middlewareNames)->not->toContain('http-client-global-logger-2'); }); From 7fbada674aa4dcec21239aae1d0adf42fef1077a Mon Sep 17 00:00:00 2001 From: Philip Iezzi Date: Thu, 14 Mar 2024 16:40:01 +0100 Subject: [PATCH 10/10] Simplify PendingRequestMixinTest by invading `HandlerStack::findByName()` --- tests/PendingRequestMixinTest.php | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/tests/PendingRequestMixinTest.php b/tests/PendingRequestMixinTest.php index ef32d05..4a9a404 100644 --- a/tests/PendingRequestMixinTest.php +++ b/tests/PendingRequestMixinTest.php @@ -9,19 +9,15 @@ it('rebuilds the guzzle handler stack to include the logger middleware at the bottom of the stack', function () { PendingRequest::mixin(new PendingRequestMixin); - /** @var PendingRequest $pendingRequest */ $pendingRequest = Http::withHeaders(['X-Test' => 'true'])->log(); /** @var HandlerStack $handler */ $handler = $pendingRequest->getOptions()['handler']; - expect($handler)->toBeInstanceOf(HandlerStack::class); - // We need to invade the HandlerStack to access the stack property - $stack = invade($handler)->stack; - // The first key is the middlware, and the second key is the name of the middleware - $middlewareNames = Arr::pluck($stack, 1); - - expect($middlewareNames)->toContain('http-client-global-logger-0') - ->and($middlewareNames)->toContain('http-client-global-logger-1') - ->and($middlewareNames)->not->toContain('http-client-global-logger-2'); + // We need to invade the HandlerStack to access the stack property or findByName method + expect($handler)->toBeInstanceOf(HandlerStack::class) + ->and(invade($handler)->findByName('http-client-global-logger-0'))->toBeInt() + ->and(invade($handler)->findByName('http-client-global-logger-1'))->toBeInt() + ->and(fn () => invade($handler)->findByName('http-client-global-logger-2')) + ->toThrow(\InvalidArgumentException::class); });