Fixed bug with incorrect multiple words processing (#1325)

vladvildanov · tillkruss · web-flow · commit 90da582efc88 · 2023-06-21T12:00:58.000+03:00
* Fixed bug with incorrect multiple words processing

* Convert subcommand string to lower case

* Update SubcommandStrategyResolver.php

* Added test coverage

* Codestyle fixes

---------

Co-authored-by: Till Krüss &lt;tillkruss@users.noreply.github.com&gt;
diff --git a/src/Command/Redis/FUNCTIONS.php b/src/Command/Redis/FUNCTIONS.php
@@ -41,7 +41,7 @@ public function getId()
 
     public function setArguments(array $arguments)
     {
-        $strategy = $this->strategyResolver->resolve('functions', $arguments[0]);
+        $strategy = $this->strategyResolver->resolve('functions', strtolower($arguments[0]));
         $arguments = $strategy->processArguments($arguments);
 
         parent::setArguments($arguments);
diff --git a/src/Command/Strategy/SubcommandStrategyResolver.php b/src/Command/Strategy/SubcommandStrategyResolver.php
@@ -18,13 +18,28 @@ class SubcommandStrategyResolver implements StrategyResolverInterface
 {
     private const CONTAINER_COMMANDS_NAMESPACE = 'Predis\Command\Strategy\ContainerCommands';
 
+    /**
+     * @var ?string
+     */
+    private $separator;
+
+    public function __construct(string $separator = null)
+    {
+        $this->separator = $separator;
+    }
+
     /**
      * {@inheritDoc}
      */
     public function resolve(string $commandId, string $subcommandId): SubcommandStrategyInterface
     {
-        $subcommandStrategyClass = ucfirst(strtolower($subcommandId)) . 'Strategy';
-        $commandDirectoryName = ucfirst(strtolower($commandId));
+        $subcommandStrategyClass = ucwords($subcommandId) . 'Strategy';
+        $commandDirectoryName = ucwords($commandId);
+
+        if (!is_null($this->separator)) {
+            $subcommandStrategyClass = str_replace($this->separator, '', $subcommandStrategyClass);
+            $commandDirectoryName = str_replace($this->separator, '', $commandDirectoryName);
+        }
 
         if (class_exists(
             $containerCommandClass = self::CONTAINER_COMMANDS_NAMESPACE . '\\' . $commandDirectoryName . '\\' . $subcommandStrategyClass
diff --git a/tests/Predis/Command/Strategy/SubcommandStrategyResolverTest.php b/tests/Predis/Command/Strategy/SubcommandStrategyResolverTest.php
@@ -19,33 +19,39 @@
 class SubcommandStrategyResolverTest extends TestCase
 {
     /**
-     * @var StrategyResolverInterface
+     * @group disconnected
+     * @return void
      */
-    private $resolver;
-
-    protected function setUp(): void
+    public function testResolveCorrectStrategy(): void
     {
-        $this->resolver = new SubcommandStrategyResolver();
+        $resolver = new SubcommandStrategyResolver();
+        $expectedStrategy = new LoadStrategy();
+
+        $this->assertEquals($expectedStrategy, $resolver->resolve('functions', 'load'));
     }
 
     /**
+     * @group disconnected
      * @return void
      */
-    public function testResolveCorrectStrategy(): void
+    public function testResolveCorrectlyResolvesStrategyWithGivenWordSeparator(): void
     {
+        $resolver = new SubcommandStrategyResolver('_');
         $expectedStrategy = new LoadStrategy();
 
-        $this->assertEquals($expectedStrategy, $this->resolver->resolve('functions', 'load'));
+        $this->assertEquals($expectedStrategy, $resolver->resolve('functions_', 'load_'));
     }
 
     /**
      * @return void
      */
     public function testResolveThrowsExceptionOnNonExistingStrategy(): void
     {
+        $resolver = new SubcommandStrategyResolver();
+
         $this->expectException(InvalidArgumentException::class);
         $this->expectExceptionMessage('Non-existing container command given');
 
-        $this->resolver->resolve('foo', 'bar');
+        $resolver->resolve('foo', 'bar');
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -41,7 +41,7 @@ public function getId()`
`41`	`41`
`42`	`42`	`public function setArguments(array $arguments)`
`43`	`43`	`{`
`44`		`- $strategy = $this->strategyResolver->resolve('functions', $arguments[0]);`
	`44`	`+ $strategy = $this->strategyResolver->resolve('functions', strtolower($arguments[0]));`
`45`	`45`	`$arguments = $strategy->processArguments($arguments);`
`46`	`46`
`47`	`47`	`parent::setArguments($arguments);`
Original file line number	Diff line number	Diff line change
`@@ -19,33 +19,39 @@`
`19`	`19`	`class SubcommandStrategyResolverTest extends TestCase`
`20`	`20`	`{`
`21`	`21`	`/**`
`22`		`- * @var StrategyResolverInterface`
	`22`	`+ * @group disconnected`
	`23`	`+ * @return void`
`23`	`24`	`*/`
`24`		`- private $resolver;`
`25`		`-`
`26`		`- protected function setUp(): void`
	`25`	`+ public function testResolveCorrectStrategy(): void`
`27`	`26`	`{`
`28`		`- $this->resolver = new SubcommandStrategyResolver();`
	`27`	`+ $resolver = new SubcommandStrategyResolver();`
	`28`	`+ $expectedStrategy = new LoadStrategy();`
	`29`	`+`
	`30`	`+ $this->assertEquals($expectedStrategy, $resolver->resolve('functions', 'load'));`
`29`	`31`	`}`
`30`	`32`
`31`	`33`	`/**`
	`34`	`+ * @group disconnected`
`32`	`35`	`* @return void`
`33`	`36`	`*/`
`34`		`- public function testResolveCorrectStrategy(): void`
	`37`	`+ public function testResolveCorrectlyResolvesStrategyWithGivenWordSeparator(): void`
`35`	`38`	`{`
	`39`	`+ $resolver = new SubcommandStrategyResolver('_');`
`36`	`40`	`$expectedStrategy = new LoadStrategy();`
`37`	`41`
`38`		`- $this->assertEquals($expectedStrategy, $this->resolver->resolve('functions', 'load'));`
	`42`	`+ $this->assertEquals($expectedStrategy, $resolver->resolve('functions_', 'load_'));`
`39`	`43`	`}`
`40`	`44`
`41`	`45`	`/**`
`42`	`46`	`* @return void`
`43`	`47`	`*/`
`44`	`48`	`public function testResolveThrowsExceptionOnNonExistingStrategy(): void`
`45`	`49`	`{`
	`50`	`+ $resolver = new SubcommandStrategyResolver();`
	`51`	`+`
`46`	`52`	`$this->expectException(InvalidArgumentException::class);`
`47`	`53`	`$this->expectExceptionMessage('Non-existing container command given');`
`48`	`54`
`49`		`- $this->resolver->resolve('foo', 'bar');`
	`55`	`+ $resolver->resolve('foo', 'bar');`
`50`	`56`	`}`
`51`	`57`	`}`