Fixes

Author: Nyholm

src/Plugin/AddPathPlugin.php

@@ -35,20 +35,30 @@ public function __construct(UriInterface $uri)
     }
 
     /**
-     * Adds a prefix in the beginning of the URL's path. We will not add the prefix
-     * if it already exists. This will of course not cover all the edge cases. For
-     * example when you have a valid URL like `https://example.com/api/api/foo` and
-     * you are using the prefix "/api".
+     * Adds a prefix in the beginning of the URL's path.
      *
-     * Some of the other edge cases are:
-     *  - request objects can be reused
-     *  - plugin chains could be restarted
-     *  - multiple AddPathPlugins may exist in one chain
+     * The prefix is not added if that prefix is already on the URL's path. This will fail on the edge
+     * case of the prefix being repeated, for example if `https://example.com/api/api/foo` is a valid
+     * URL on the server and the configured prefix is `/api`.
      *
-     * If one would like to cover all these scenarios one would have to add a custom
-     * header to the request to indicate it has been rewritten by the plugin. This
-     * implementation does not aim for pleasing all edge cases rather to be great
-     * value for the 95% other use cases.
+     * We looked at other solutions, but they are all much more complicated, while still having edge
+     * cases:
+     * - Doing an spl_object_hash on `$first` will lead to collisions over time because over time the
+     *   hash can collide.
+     * - Have the PluginClient provide a magic header to identify the request chain and only apply
+     *   this plugin once.
+     *
+     * There are 2 reasons for the AddPathPlugin to be executed twice on the same request:
+     * - A plugin can restart the chain by calling `$first`, e.g. redirect
+     * - A plugin can call `$next` more than once, e.g. retry
+     *
+     * Depending on the scenario, the path should or should not be added. E.g. `$first` could
+     * be called after a redirect response from the server. The server likely already has the
+     * correct path.
+     *
+     * No solution fits all use cases. This implementation will work fine for the common use cases.
+     * If you have a specific situation where this is not the right thing, you can build a custom plugin
+     * that does exactly what you need.
      *
      * {@inheritdoc}
      */

tests/Plugin/AddPathPluginTest.php

@@ -73,19 +73,6 @@ public function testRewriteWithDifferentUrl()
         }, $this->first);
     }
 
-    public function testRewriteWithDifferentUrlWhenSecondUrlIncludesAddedPath()
-    {
-        $verify = function (RequestInterface $request) {
-            $this->assertEquals('https://example.com/api/foo', $request->getUri()->__toString());
-        };
-
-        $request = new Request('GET', 'https://example.com/foo');
-        $this->plugin->handleRequest($request, $verify, $this->first);
-
-        $request = new Request('GET', 'https://example.com/api/foo');
-        $this->plugin->handleRequest($request, $verify, $this->first);
-    }
-
     public function testRewriteWhenPathIsIncluded()
     {
         $verify = function (RequestInterface $request) {
@@ -95,42 +82,4 @@ public function testRewriteWhenPathIsIncluded()
         $request = new Request('GET', 'https://example.com/api/foo');
         $this->plugin->handleRequest($request, $verify, $this->first);
     }
-    public function testRewriteOnRestart()
-    {
-        $secondPlugin = new class implements Plugin {
-            private $firstRun = true;
-
-            public function handleRequest(RequestInterface $request, callable $next, callable $first)
-            {
-                if ($this->firstRun) {
-                    $this->firstRun = false;
-                    return $first($request)->wait();
-                }
-                return $next($request);
-            }
-        };
-
-        $verify = function (RequestInterface $request) {
-            $this->assertEquals('https://example.com/api/foo', $request->getUri()->__toString());
-        };
-
-        $client = new class($verify) implements HttpClient {
-            private $callable;
-            public function __construct(callable $callable)
-            {
-                $this->callable = $callable;
-            }
-
-            public function sendRequest(RequestInterface $request)
-            {
-                $verify = $this->callable;
-                $verify($request);
-                return new Response();
-            }
-        };
-
-        $pluginClient = new PluginClient($client, [$this->plugin, $secondPlugin]);
-        $request = new Request('GET', 'https://example.com/foo');
-        $response = $pluginClient->sendAsyncRequest($request);
-    }
 }