Merge pull request #877 from im-denisenko/strategy-autoload

StrategyFactory should check predefined strategies first.
ruflin · Jun 15, 2015 · 97b9c66 · 97b9c66
2 parents ebcf6fd + 274c20a
commit 97b9c66
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 14 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,6 +14,7 @@ All notable changes to this project will be documented in this file based on the
 
 ### Improvements
 - `CallbackStrategy` now will accept any `callable` as callback, not only instance of `Closure`. [#871](https://github.com/ruflin/Elastica/pull/871)
+- `StrategyFactory` now will try to find predefined strategy before looking to global namespace. [#877](https://github.com/ruflin/Elastica/pull/877)
 
 
 ## [2.1.0](https://github.com/ruflin/Elastica/releases/tag/2.1.0) - 2015-06-01

diff --git a/lib/Elastica/Connection/Strategy/StrategyFactory.php b/lib/Elastica/Connection/Strategy/StrategyFactory.php
@@ -19,24 +19,27 @@ class StrategyFactory
      */
     public static function create($strategyName)
     {
-        $strategy = null;
         if ($strategyName instanceof StrategyInterface) {
-            $strategy = $strategyName;
-        } elseif (CallbackStrategy::isValid($strategyName)) {
-            $strategy = new CallbackStrategy($strategyName);
-        } elseif (is_string($strategyName) && class_exists($strategyName)) {
-            $strategy = new $strategyName();
-        } elseif (is_string($strategyName)) {
-            $pathToStrategy = '\\Elastica\\Connection\\Strategy\\'.$strategyName;
-            if (class_exists($pathToStrategy)) {
-                $strategy = new $pathToStrategy();
-            }
+            return $strategyName;
+        }
+
+        if (CallbackStrategy::isValid($strategyName)) {
+            return new CallbackStrategy($strategyName);
         }
 
-        if (!$strategy instanceof StrategyInterface) {
-            throw new InvalidException('Can\'t load strategy class');
+        if (is_string($strategyName)) {
+            $requiredInterface = '\\Elastica\\Connection\\Strategy\\StrategyInterface';
+            $predefinedStrategy = '\\Elastica\\Connection\\Strategy\\'.$strategyName;
+
+            if (class_exists($predefinedStrategy) && class_implements($predefinedStrategy, $requiredInterface)) {
+                return new $predefinedStrategy();
+            }
+
+            if (class_exists($strategyName) && class_implements($strategyName, $requiredInterface)) {
+                return new $strategyName();
+            }
         }
 
-        return $strategy;
+        throw new InvalidException('Can\'t create strategy instance by given argument');
     }
 }
diff --git a/test/lib/Elastica/Test/Connection/Strategy/StrategyFactoryTest.php b/test/lib/Elastica/Test/Connection/Strategy/StrategyFactoryTest.php
@@ -68,4 +68,17 @@ public function testFailCreate()
 
         StrategyFactory::create($strategy);
     }
+
+    /**
+     * @group unit
+     */
+    public function testNoCollisionWithGlobalNamespace()
+    {
+        // create collision
+        if (!class_exists('Simple')) {
+            class_alias('Elastica\Util', 'Simple');
+        }
+        $strategy = StrategyFactory::create('Simple');
+        $this->assertInstanceOf('Elastica\Connection\Strategy\Simple', $strategy);
+    }
 }