Total rewrite of middleware sorting

Middleware sorting was broken if middleware had parameters. Totally rewrote the algorithm and added more tests.
laravel · Oct 14, 2016 · 6b69fb8 · 6b69fb8
1 parent 98533d2
commit 6b69fb8
Show file tree

Hide file tree

Showing 3 changed files with 131 additions and 26 deletions.
diff --git a/src/Illuminate/Routing/Router.php b/src/Illuminate/Routing/Router.php
@@ -741,32 +741,7 @@ protected function parseMiddlewareGroup($name)
      */
     protected function sortMiddleware(Collection $middlewares)
     {
-        $priority = $this->middlewarePriority;
-
-        $sorted = [];
-
-        foreach ($middlewares as $middleware) {
-            if (in_array($middleware, $sorted)) {
-                continue;
-            }
-
-            if (($index = array_search($middleware, $priority)) !== false) {
-                $sorted = array_merge(
-                    $sorted,
-                    array_filter(
-                        array_slice($priority, 0, $index),
-                        function ($middleware) use ($middlewares, $sorted) {
-                            return $middlewares->contains($middleware) &&
-                                 ! in_array($middleware, $sorted);
-                        }
-                    )
-                );
-            }
-
-            $sorted[] = $middleware;
-        }
-
-        return $sorted;
+        return (new SortedMiddleware($this->middlewarePriority, $middlewares))->all();
     }
 
     /**

diff --git a/src/Illuminate/Routing/SortedMiddleware.php b/src/Illuminate/Routing/SortedMiddleware.php
@@ -0,0 +1,76 @@
+<?php
+
+namespace Illuminate\Routing;
+
+use Illuminate\Support\Collection;
+
+class SortedMiddleware extends Collection
+{
+    /**
+     * Create a new Sorted Middleware container.
+     *
+     * @param  array  $priorityMap
+     * @param  array  $middlewares
+     * @return void
+     */
+    public function __construct(array $priorityMap, $middlewares)
+    {
+        if ($middlewares instanceof Collection) {
+            $middlewares = $middlewares->all();
+        }
+
+        $this->items = $this->sortMiddleware($priorityMap, $middlewares);
+    }
+
+    /**
+     * Sort the middlewares by the given priority map.
+     *
+     * @param  array  $priorityMap
+     * @param  array  $middlewares
+     * @return array
+     */
+    protected function sortMiddleware($priorityMap, $middlewares)
+    {
+        $lastIndex = 0;
+
+        foreach ($middlewares as $index => $middleware) {
+            if (! is_string($middleware)) {
+                continue;
+            }
+
+            $stripped = head(explode(':', $middleware));
+
+            if (in_array($stripped, $priorityMap)) {
+                $priorityIndex = array_search($stripped, $priorityMap);
+
+                if (isset($lastPriorityIndex) && $priorityIndex < $lastPriorityIndex) {
+                    return $this->sortMiddleware(
+                        $priorityMap, array_values($this->spliceMiddleware($middlewares, $index, $lastIndex))
+                    );
+                } else {
+                    $lastIndex = $index;
+                    $lastPriorityIndex = $priorityIndex;
+                }
+            }
+        }
+
+        return $middlewares;
+    }
+
+    /**
+     * Splice a middleware into a new position and remove the old entry.
+     *
+     * @param  array  $middlewares
+     * @param  int  $from
+     * @param  int  $to
+     * @return array
+     */
+    protected function spliceMiddleware($middlewares, $from, $to)
+    {
+        array_splice($middlewares, $to, 0, $middlewares[$from]);
+
+        unset($middlewares[$from + 1]);
+
+        return $middlewares;
+    }
+}
diff --git a/tests/Routing/RoutingSortedMiddlewareTest.php b/tests/Routing/RoutingSortedMiddlewareTest.php
@@ -0,0 +1,54 @@
+<?php
+
+use Illuminate\Routing\SortedMiddleware;
+
+class RoutingSortedMiddlewareTest extends PHPUnit_Framework_TestCase
+{
+    public function testMiddlewareCanBeSortedByPriority()
+    {
+        $priority = [
+            'First',
+            'Second',
+            'Third',
+        ];
+
+        $middleware = [
+            'Something',
+            'Something',
+            'Something',
+            'Something',
+            'Second',
+            'Something',
+            'Something',
+            'First:api',
+            'Third:foo',
+            'First:foo,bar',
+            'Third',
+            'Second',
+        ];
+
+        $expected = [
+            'Something',
+            'Something',
+            'Something',
+            'Something',
+            'First:api',
+            'First:foo,bar',
+            'Second',
+            'Something',
+            'Something',
+            'Second',
+            'Third:foo',
+            'Third',
+        ];
+
+        $this->assertEquals($expected, (new SortedMiddleware($priority, $middleware))->all());
+
+        $this->assertEquals([], (new SortedMiddleware(['First'], []))->all());
+        $this->assertEquals(['First'], (new SortedMiddleware(['First'], ['First']))->all());
+        $this->assertEquals(['First', 'Second'], (new SortedMiddleware(['First', 'Second'], ['Second', 'First']))->all());
+
+        $closure = function () {};
+        $this->assertEquals(['Second', $closure], (new SortedMiddleware(['First', 'Second'], ['Second', $closure]))->all());
+    }
+}