minor #43268 [FrameworkBundle][Routing] Minor improvement - No `array…

…_merge` in loop (simonberger)

This PR was merged into the 5.4 branch.

Discussion
----------

[FrameworkBundle][Routing] Minor improvement - No `array_merge` in loop

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| License | MIT

A tiny (here) performance improvement in moving an array_merge out of a foreach.
Added tests  for those lines which were missing.

I started to move more array_merge(_*) calls out of loops where it makes sense. In some cases the impact could be more relevant than in this small loop. I see this as a test.

Should i add further changes as a combined, single or component based pull request and is 5.4 the correct target?

Commits
-------

87e13a6 [FrameworkBundle] Minor improvement - No `array_merge` in loop

src/Symfony/Bundle/FrameworkBundle/Routing/Router.php

@@ -130,15 +130,15 @@ private function resolveParameters(RouteCollection $collection)
 
             $schemes = [];
             foreach ($route->getSchemes() as $scheme) {
-                $schemes = array_merge($schemes, explode('|', $this->resolve($scheme)));
+                $schemes[] = explode('|', $this->resolve($scheme));
             }
-            $route->setSchemes($schemes);
+            $route->setSchemes(array_merge([], ...$schemes));
 
             $methods = [];
             foreach ($route->getMethods() as $method) {
-                $methods = array_merge($methods, explode('|', $this->resolve($method)));
+                $methods[] = explode('|', $this->resolve($method));
             }
-            $route->setMethods($methods);
+            $route->setMethods(array_merge([], ...$methods));
             $route->setCondition($this->resolve($route->getCondition()));
         }
     }

src/Symfony/Bundle/FrameworkBundle/Tests/Routing/RouterTest.php

@@ -553,6 +553,44 @@ public function testCacheValidityWithContainerParameters($parameter)
         }
     }
 
+    public function testResolvingSchemes()
+    {
+        $routes = new RouteCollection();
+
+        $route = new Route('/test', [], [], [], '', ['%parameter.http%', '%parameter.https%']);
+        $routes->add('foo', $route);
+
+        $sc = $this->getPsr11ServiceContainer($routes);
+        $parameters = $this->getParameterBag([
+            'parameter.http' => 'http',
+            'parameter.https' => 'https',
+        ]);
+
+        $router = new Router($sc, 'foo', [], null, $parameters);
+        $route = $router->getRouteCollection()->get('foo');
+
+        $this->assertEquals(['http', 'https'], $route->getSchemes());
+    }
+
+    public function testResolvingMethods()
+    {
+        $routes = new RouteCollection();
+
+        $route = new Route('/test', [], [], [], '', [], ['%parameter.get%', '%parameter.post%']);
+        $routes->add('foo', $route);
+
+        $sc = $this->getPsr11ServiceContainer($routes);
+        $parameters = $this->getParameterBag([
+            'PARAMETER.GET' => 'GET',
+            'PARAMETER.POST' => 'POST',
+        ]);
+
+        $router = new Router($sc, 'foo', [], null, $parameters);
+        $route = $router->getRouteCollection()->get('foo');
+
+        $this->assertEquals(['GET', 'POST'], $route->getMethods());
+    }
+
     public function getContainerParameterForRoute()
     {
         yield 'String' => ['"foo"'];