Merge pull request #144 from Sammyjo20/feature/v2-better-merging-pipeline

Feature | V2 Better Merging Pipeline

Author: Sammyjo20

src/Contracts/Pipeline.php

@@ -0,0 +1,42 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Saloon\Contracts;
+
+interface Pipeline
+{
+    /**
+     * Add a pipe to the pipeline
+     *
+     * @param callable $callable
+     * @param bool $prepend
+     * @param string|null $name
+     * @return \Saloon\Helpers\Pipeline
+     * @throws \Saloon\Exceptions\DuplicatePipeNameException
+     */
+    public function pipe(callable $callable, bool $prepend = false, ?string $name = null): static;
+
+    /**
+     * Process the pipeline.
+     *
+     * @param mixed $payload
+     * @return mixed
+     */
+    public function process(mixed $payload): mixed;
+
+    /**
+     * Set the pipes on the pipeline.
+     *
+     * @param array<\Saloon\Data\Pipe> $pipes
+     * @return $this
+     */
+    public function setPipes(array $pipes): static;
+
+    /**
+     * Get all the pipes in the pipeline
+     *
+     * @return array<\Saloon\Data\Pipe>
+     */
+    public function getPipes(): array;
+}

src/Contracts/Response.php

@@ -256,13 +256,6 @@ public function getSimulatedResponsePayload(): ?SimulatedResponsePayload;
      */
     public function getSenderException(): ?Throwable;
 
-    /**
-     * Get the raw response
-     *
-     * @return mixed
-     */
-    public function getRawResponse(): mixed;
-
     /**
      * Get the body of the response.
      *

src/Data/Pipe.php

@@ -0,0 +1,21 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Saloon\Data;
+
+class Pipe
+{
+    /**
+     * Constructor
+     *
+     * @param callable $callable
+     * @param string|null $name
+     */
+    public function __construct(
+        readonly public mixed $callable,
+        readonly public ?string $name = null,
+    ) {
+        //
+    }
+}

src/Helpers/MiddlewarePipeline.php

@@ -7,31 +7,32 @@
 use Saloon\Contracts\Response;
 use Saloon\Contracts\PendingRequest;
 use Saloon\Contracts\SimulatedResponsePayload;
+use Saloon\Contracts\Pipeline as PipelineContract;
 use Saloon\Contracts\MiddlewarePipeline as MiddlewarePipelineContract;
 
 class MiddlewarePipeline implements MiddlewarePipelineContract
 {
     /**
      * Request Pipeline
      *
-     * @var Pipeline
+     * @var \Saloon\Contracts\Pipeline
      */
-    protected Pipeline $requestPipeline;
+    protected PipelineContract $requestPipeline;
 
     /**
      * Response Pipeline
      *
-     * @var Pipeline
+     * @var \Saloon\Contracts\Pipeline
      */
-    protected Pipeline $responsePipeline;
+    protected PipelineContract $responsePipeline;
 
     /**
      * Constructor
      */
     public function __construct()
     {
-        $this->requestPipeline = new Pipeline();
-        $this->responsePipeline = new Pipeline();
+        $this->requestPipeline = new Pipeline;
+        $this->responsePipeline = new Pipeline;
     }
 
     /**

src/Helpers/MockConfig.php

@@ -4,7 +4,7 @@
 
 namespace Saloon\Helpers;
 
-class MockConfig
+final class MockConfig
 {
     /**
      * Default fixture path

src/Helpers/Pipeline.php

@@ -4,45 +4,40 @@
 
 namespace Saloon\Helpers;
 
+use Saloon\Data\Pipe;
 use Saloon\Exceptions\DuplicatePipeNameException;
+use Saloon\Contracts\Pipeline as PipelineContract;
 
-class Pipeline
+class Pipeline implements PipelineContract
 {
     /**
      * The pipes in the pipeline.
      *
-     * @var array
+     * @var array<\Saloon\Data\Pipe>
      */
     protected array $pipes = [];
 
     /**
-     * The named pipes that have been added.
+     * Add a pipe to the pipeline
      *
-     * @var array
-     */
-    protected array $namedPipes = [];
-
-    /**
-     * Add a pipe to the pipeline.
-     *
-     * @param callable $pipe
+     * @param callable $callable
      * @param bool $prepend
      * @param string|null $name
      * @return $this
      * @throws \Saloon\Exceptions\DuplicatePipeNameException
      */
-    public function pipe(callable $pipe, bool $prepend = false, ?string $name = null): static
+    public function pipe(callable $callable, bool $prepend = false, ?string $name = null): static
     {
-        if ($prepend === true) {
-            array_unshift($this->pipes, $pipe);
-        } else {
-            $this->pipes[] = $pipe;
-        }
+        $pipe = new Pipe($callable, $name);
 
-        if (! is_null($name)) {
-            in_array($name, $this->namedPipes, true) ? throw new DuplicatePipeNameException($name) : $this->namedPipes[] = $name;
+        if (is_string($name) && $this->pipeExists($name)) {
+            throw new DuplicatePipeNameException($name);
         }
 
+        $prepend === true
+            ? array_unshift($this->pipes, $pipe)
+            : $this->pipes[] = $pipe;
+
         return $this;
     }
 
@@ -55,7 +50,7 @@ public function pipe(callable $pipe, bool $prepend = false, ?string $name = null
     public function process(mixed $payload): mixed
     {
         foreach ($this->pipes as $pipe) {
-            $payload = $pipe($payload);
+            $payload = call_user_func($pipe->callable, $payload);
         }
 
         return $payload;
@@ -66,21 +61,46 @@ public function process(mixed $payload): mixed
      *
      * @param array $pipes
      * @return $this
+     * @throws \Saloon\Exceptions\DuplicatePipeNameException
      */
     public function setPipes(array $pipes): static
     {
-        $this->pipes = $pipes;
+        $this->pipes = [];
+
+        // Loop through each of the pipes and manually add each pipe
+        // so we can check if the name already exists.
+
+        foreach ($pipes as $pipe) {
+            $this->pipe($pipe->callable, false, $pipe->name);
+        }
 
         return $this;
     }
 
     /**
      * Get all the pipes in the pipeline
      *
-     * @return array
+     * @return array<\Saloon\Data\Pipe>
      */
     public function getPipes(): array
     {
         return $this->pipes;
     }
+
+    /**
+     * Check if a given pipe exists for a name
+     *
+     * @param string $name
+     * @return bool
+     */
+    protected function pipeExists(string $name): bool
+    {
+        foreach ($this->pipes as $pipe) {
+            if ($pipe->name === $name) {
+                return true;
+            }
+        }
+        
+        return false;
+    }
 }

src/Helpers/Storage.php

@@ -21,12 +21,14 @@ class Storage
      * Constructor
      *
      * @param string $baseDirectory
-     * @throws DirectoryNotFoundException
+     * @param bool $createMissingBaseDirectory
+     * @throws \Saloon\Exceptions\DirectoryNotFoundException
+     * @throws \Saloon\Exceptions\UnableToCreateDirectoryException
      */
-    public function __construct(string $baseDirectory)
+    public function __construct(string $baseDirectory, bool $createMissingBaseDirectory = false)
     {
         if (! is_dir($baseDirectory)) {
-            throw new DirectoryNotFoundException($baseDirectory);
+            $createMissingBaseDirectory ? $this->createDirectory($baseDirectory) : throw new DirectoryNotFoundException($baseDirectory);
         }
 
         $this->baseDirectory = $baseDirectory;
@@ -104,11 +106,7 @@ public function put(string $path, string $contents): static
         $directoryWithoutFilename = implode(DIRECTORY_SEPARATOR, explode(DIRECTORY_SEPARATOR, $fullPath, -1));
 
         if (empty($directoryWithoutFilename) === false && is_dir($directoryWithoutFilename) === false) {
-            $createdDirectory = mkdir($directoryWithoutFilename, 0777, true);
-
-            if ($createdDirectory === false && is_dir($directoryWithoutFilename) === false) {
-                throw new UnableToCreateDirectoryException($directoryWithoutFilename);
-            }
+            $this->createDirectory($directoryWithoutFilename);
         }
 
         $createdFile = file_put_contents($fullPath, $contents);
@@ -119,4 +117,22 @@ public function put(string $path, string $contents): static
 
         return $this;
     }
+
+    /**
+     * Create a directory
+     *
+     * @param string $directory
+     * @return bool
+     * @throws \Saloon\Exceptions\UnableToCreateDirectoryException
+     */
+    public function createDirectory(string $directory): bool
+    {
+        $createdDirectory = mkdir($directory, 0777, true);
+
+        if ($createdDirectory === false && is_dir($directory) === false) {
+            throw new UnableToCreateDirectoryException($directory);
+        }
+
+        return true;
+    }
 }

src/Http/Faking/Fixture.php

@@ -8,7 +8,6 @@
 use Saloon\Helpers\MockConfig;
 use Saloon\Data\RecordedResponse;
 use Saloon\Exceptions\FixtureMissingException;
-use Saloon\Exceptions\DirectoryNotFoundException;
 
 class Fixture
 {
@@ -37,13 +36,14 @@ class Fixture
      * Constructor
      *
      * @param string $name
-     * @param Storage|null $storage
-     * @throws DirectoryNotFoundException
+     * @param \Saloon\Helpers\Storage|null $storage
+     * @throws \Saloon\Exceptions\DirectoryNotFoundException
+     * @throws \Saloon\Exceptions\UnableToCreateDirectoryException
      */
     public function __construct(string $name, Storage $storage = null)
     {
         $this->name = $name;
-        $this->storage = $storage ?? new Storage(MockConfig::getFixturePath());
+        $this->storage = $storage ?? new Storage(MockConfig::getFixturePath(), true);
     }
 
     /**

src/Http/Response.php

@@ -87,16 +87,6 @@ public function getRequest(): Request
         return $this->pendingRequest->getRequest();
     }
 
-    /**
-     * Get the raw response
-     *
-     * @return mixed
-     */
-    public function getRawResponse(): mixed
-    {
-        return $this->psrResponse;
-    }
-
     /**
      * Get the body of the response as string.
      *

tests/Unit/MiddlewarePipelineTest.php

@@ -183,7 +183,7 @@
         })
         ->onResponse(function (Response $response) {
             return $response->throw();
-        });
+        }, false, 'response');
 
     expect($pipelineB->getRequestPipeline()->getPipes())->toBeEmpty();
     expect($pipelineB->getResponsePipeline()->getPipes())->toBeEmpty();
@@ -192,6 +192,21 @@
 
     expect($pipelineB->getRequestPipeline()->getPipes())->toHaveCount(2);
     expect($pipelineB->getResponsePipeline()->getPipes())->toHaveCount(1);
+    expect($pipelineA->getRequestPipeline()->getPipes())->toEqual($pipelineB->getRequestPipeline()->getPipes());
+    expect($pipelineA->getResponsePipeline()->getPipes())->toEqual($pipelineB->getResponsePipeline()->getPipes());
+});
+
+test('when merging a middleware pipeline together if two pipelines exist with the same pipe it throws an exception', function () {
+    $pipelineA = new MiddlewarePipeline;
+    $pipelineB = new MiddlewarePipeline;
+
+    $pipelineA->onRequest(fn () => null, false, 'howdy');
+    $pipelineB->onRequest(fn () => null, false, 'howdy');
+
+    $this->expectException(DuplicatePipeNameException::class);
+    $this->expectExceptionMessage('The "howdy" pipe already exists on the pipeline');
+
+    $pipelineA->merge($pipelineB);
 });
 
 test('a request pipeline is run in order of pipes', function () {