Merge pull request #102 from Xerkus/hotfix/recursive-performance-drop

Extract recursive method from ConfigPostProcessor::__invoke() to avoid performance hit
laminas · Jul 29, 2022 · e112dd2 · e112dd2
2 parents d74d2da + 07d4ce7
commit e112dd2
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 58 deletions.
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="4.9.3@4c262932602b9bbab5020863d1eb22d49de0dbf4">
+<files psalm-version="4.21.0@d8bec4c7aaee111a532daec32fb09de5687053d1">
   <file src="config/replacements.php">
     <DuplicateArrayKey occurrences="3">
       <code>'ZendAcl' =&gt; 'LaminasAcl'</code>
@@ -41,9 +41,7 @@
       <code>$newKey</code>
       <code>$target</code>
     </MixedArgument>
-    <MixedArgumentTypeCoercion occurrences="1">
-      <code>[$key]</code>
-    </MixedArgumentTypeCoercion>
+    <MixedArgumentTypeCoercion occurrences="1"/>
     <MixedArrayAssignment occurrences="4">
       <code>$config[$key]</code>
       <code>$config['aliases'][$alias]</code>
@@ -62,7 +60,7 @@
     <MixedArrayTypeCoercion occurrences="1">
       <code>$aliases[$name]</code>
     </MixedArrayTypeCoercion>
-    <MixedAssignment occurrences="27">
+    <MixedAssignment occurrences="26">
       <code>$a[$key]</code>
       <code>$a[$key]</code>
       <code>$a[]</code>
@@ -78,7 +76,6 @@
       <code>$key</code>
       <code>$name</code>
       <code>$newKey</code>
-      <code>$newValue</code>
       <code>$notIn[]</code>
       <code>$result</code>
       <code>$rewritten[$key]</code>

diff --git a/src/ConfigPostProcessor.php b/src/ConfigPostProcessor.php
@@ -29,17 +29,17 @@ class ConfigPostProcessor
         'zf-apigility'    => 'api-tools',
     ];
 
-    /** @var Replacements */
+    /**
+     * @psalm-suppress PropertyNotSetInConstructor Initialized during call to the only public method __invoke()
+     * @var Replacements
+     */
     private $replacements;
 
     /** @var callable[] */
     private $rulesets;
 
     public function __construct()
     {
-        // This value will be reset during __invoke(); setting here to prevent Psalm errors.
-        $this->replacements = new Replacements();
-
         /* Define the rulesets for replacements.
          *
          * Each ruleset has the following signature:
@@ -86,7 +86,7 @@ function ($value) {
             // Array values
             function ($value, array $keys) {
                 return $keys !== [] && is_array($value)
-                    ? [$this, '__invoke']
+                    ? [$this, 'processConfig']
                     : null;
             },
         ];
@@ -100,51 +100,7 @@ function ($value, array $keys) {
     public function __invoke(array $config, array $keys = [])
     {
         $this->replacements = $this->initializeReplacements($config);
-        $rewritten          = [];
-
-        foreach ($config as $key => $value) {
-            // Do not rewrite configuration for the bridge
-            if ($key === 'laminas-zendframework-bridge') {
-                $rewritten[$key] = $value;
-                continue;
-            }
-
-            // Determine new key from replacements
-            $newKey = is_string($key) ? $this->replace($key, $keys) : $key;
-
-            // Keep original values with original key, if the key has changed, but only at the top-level.
-            if (empty($keys) && $newKey !== $key) {
-                $rewritten[$key] = $value;
-            }
-
-            // Perform value replacements, if any
-            $newValue = $this->replace($value, $keys, $newKey);
-
-            // Key does not already exist and/or is not an array value
-            if (! array_key_exists($newKey, $rewritten) || ! is_array($rewritten[$newKey])) {
-                // Do not overwrite existing values with null values
-                $rewritten[$newKey] = array_key_exists($newKey, $rewritten) && null === $newValue
-                    ? $rewritten[$newKey]
-                    : $newValue;
-                continue;
-            }
-
-            // New value is null; nothing to do.
-            if (null === $newValue) {
-                continue;
-            }
-
-            // Key already exists as an array value, but $value is not an array
-            if (! is_array($newValue)) {
-                $rewritten[$newKey][] = $newValue;
-                continue;
-            }
-
-            // Key already exists as an array value, and $value is also an array
-            $rewritten[$newKey] = static::merge($rewritten[$newKey], $newValue);
-        }
-
-        return $rewritten;
+        return $this->processConfig($config, $keys);
     }
 
     /**
@@ -270,7 +226,7 @@ private function replaceDependencyConfiguration(array $config)
                 continue;
             }
 
-            $config[$key] = is_array($data) ? $this->__invoke($data, [$key]) :  $data;
+            $config[$key] = is_array($data) ? $this->processConfig($data, [$key]) :  $data;
         }
 
         return $config;
@@ -414,7 +370,7 @@ private function replaceDependencyServices(array $config)
             }
 
             $replacedService = $this->replacements->replace($service);
-            $serviceInstance = is_array($serviceInstance) ? $this->__invoke($serviceInstance) : $serviceInstance;
+            $serviceInstance = is_array($serviceInstance) ? $this->processConfig($serviceInstance) : $serviceInstance;
 
             $config['services'][$replacedService] = $serviceInstance;
 
@@ -461,4 +417,57 @@ private function initializeReplacements(array $config): Replacements
 
         return new Replacements($replacements);
     }
+
+    /**
+     * @param string[] $keys Hierarchy of keys, for determining location in
+     *     nested configuration.
+     */
+    private function processConfig(array $config, array $keys = []): array
+    {
+        $rewritten = [];
+
+        foreach ($config as $key => $value) {
+            // Do not rewrite configuration for the bridge
+            if ($key === 'laminas-zendframework-bridge') {
+                $rewritten[$key] = $value;
+                continue;
+            }
+
+            // Determine new key from replacements
+            $newKey = is_string($key) ? $this->replace($key, $keys) : $key;
+
+            // Keep original values with original key, if the key has changed, but only at the top-level.
+            if (empty($keys) && $newKey !== $key) {
+                $rewritten[$key] = $value;
+            }
+
+            // Perform value replacements, if any
+            $newValue = $this->replace($value, $keys, $newKey);
+
+            // Key does not already exist and/or is not an array value
+            if (!array_key_exists($newKey, $rewritten) || !is_array($rewritten[$newKey])) {
+                // Do not overwrite existing values with null values
+                $rewritten[$newKey] = array_key_exists($newKey, $rewritten) && null === $newValue
+                    ? $rewritten[$newKey]
+                    : $newValue;
+                continue;
+            }
+
+            // New value is null; nothing to do.
+            if (null === $newValue) {
+                continue;
+            }
+
+            // Key already exists as an array value, but $value is not an array
+            if (!is_array($newValue)) {
+                $rewritten[$newKey][] = $newValue;
+                continue;
+            }
+
+            // Key already exists as an array value, and $value is also an array
+            $rewritten[$newKey] = static::merge($rewritten[$newKey], $newValue);
+        }
+
+        return $rewritten;
+    }
 }