Ensure namespace separator on appended prefixes

Appending a prefix to search new rules under required that the namespace (prefix) being added always ended with a trailing namespace character so rules could successfully be found under it. This ensures that the separator is always present. Changes a test for a rule which does not implement Respect's interface to an actual class so we don't need to declare one to use as a stub.
Respect · Apr 24, 2016 · 9460a4c · 9460a4c
1 parent bb0e40a
commit 9460a4c
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 16 deletions.
diff --git a/docs/README.md b/docs/README.md
@@ -256,7 +256,7 @@ By default `with()` appends the given prefix, but you can change this behavior
 in order to overwrite default rules:
 
 ```php
-v::with('My\\Validation\\Rules\\', true);
+v::with('My\\Validation\\Rules', true);
 v::alnum(); // Try to use "My\Validation\Rules\Alnum" if any
 ```
 

diff --git a/library/Factory.php b/library/Factory.php
@@ -23,14 +23,22 @@ public function getRulePrefixes()
         return $this->rulePrefixes;
     }
 
+    private function filterRulePrefix($rulePrefix)
+    {
+        $namespaceSeparator = '\\';
+        $rulePrefix = rtrim($rulePrefix, $namespaceSeparator);
+
+        return $rulePrefix.$namespaceSeparator;
+    }
+
     public function appendRulePrefix($rulePrefix)
     {
-        array_push($this->rulePrefixes, $rulePrefix);
+        array_push($this->rulePrefixes, $this->filterRulePrefix($rulePrefix));
     }
 
     public function prependRulePrefix($rulePrefix)
     {
-        array_unshift($this->rulePrefixes, $rulePrefix);
+        array_unshift($this->rulePrefixes, $this->filterRulePrefix($rulePrefix));
     }
 
     public function rule($ruleName, array $arguments = [])

diff --git a/tests/unit/FactoryTest.php b/tests/unit/FactoryTest.php
@@ -23,20 +23,62 @@ public function testShouldHaveRulePrefixesByDefault()
         $this->assertEquals(['Respect\\Validation\\Rules\\'], $factory->getRulePrefixes());
     }
 
-    public function testShouldBeAbleToAppendANewPrefix()
+    /**
+     * @dataProvider provideRulePrefixes
+     */
+    public function testShouldBeAbleToAppendANewPrefix($namespace, $expectedNamespace)
     {
         $factory = new Factory();
-        $factory->appendRulePrefix('My\\Validation\\Rules\\');
+        $factory->appendRulePrefix($namespace);
+
+        $currentRulePrefixes = $factory->getRulePrefixes();
 
-        $this->assertEquals(['Respect\\Validation\\Rules\\', 'My\\Validation\\Rules\\'], $factory->getRulePrefixes());
+        $this->assertSame(
+            $expectedNamespace,
+            array_pop($currentRulePrefixes),
+            'Appended namespace rule was not found as expected into the prefix list.' . PHP_EOL .
+            sprintf(
+                'Appended "%s", current list is ' . PHP_EOL . '%s',
+                $namespace,
+                implode(PHP_EOL, $factory->getRulePrefixes())
+            )
+        );
     }
 
-    public function testShouldBeAbleToPrependANewRulePrefix()
+    /**
+     * @dataProvider provideRulePrefixes
+     */
+    public function testShouldBeAbleToPrependANewRulePrefix($namespace, $expectedNamespace)
     {
         $factory = new Factory();
-        $factory->prependRulePrefix('My\\Validation\\Rules\\');
+        $factory->prependRulePrefix($namespace);
+
+        $currentRulePrefixes = $factory->getRulePrefixes();
 
-        $this->assertEquals(['My\\Validation\\Rules\\', 'Respect\\Validation\\Rules\\'], $factory->getRulePrefixes());
+        $this->assertContains(
+            $expectedNamespace,
+            array_shift($currentRulePrefixes),
+            'Prepended namespace rule was not found as expected into the prefix list.' . PHP_EOL .
+            sprintf(
+                'Prepended "%s", current list is ' . PHP_EOL . '%s',
+                $namespace,
+                implode(PHP_EOL, $factory->getRulePrefixes())
+            )
+        );
+    }
+
+    public function provideRulePrefixes()
+    {
+        return [
+            'Namespace with trailing separator' => [
+                'namespace' => 'My\\Validation\\Rules\\',
+                'expected' => 'My\\Validation\\Rules\\'
+            ],
+            'Namespace without trailing separator' => [
+                'namespace' => 'My\\Validation\\Rules',
+                'expected' => 'My\\Validation\\Rules\\'
+            ]
+        ];
     }
 
     public function testShouldCreateARuleByName()
@@ -66,16 +108,12 @@ public function testShouldThrowsAnExceptionWhenRuleNameIsNotValid()
 
     /**
      * @expectedException Respect\Validation\Exceptions\ComponentException
-     * @expectedExceptionMessage "Respect\Validation\TestNonRule" is not a valid respect rule
+     * @expectedExceptionMessage "Respect\Validation\Exceptions\AgeException" is not a valid respect rule
      */
     public function testShouldThrowsAnExceptionWhenRuleIsNotInstanceOfRuleInterface()
     {
         $factory = new Factory();
-        $factory->appendRulePrefix('Respect\\Validation\\Test');
-        $factory->rule('nonRule');
+        $factory->appendRulePrefix('Respect\\Validation\\Exceptions\\');
+        $factory->rule('AgeException');
     }
 }
-
-class TestNonRule
-{
-}