fixes #94 Throws a RouteNotFoundException is a match is found where t…

.gitignore

@@ -1,3 +1,4 @@
 phpunit.xml
 composer.lock
 vendor/
+.idea

src/Routing/RouteGenerator/NameInflector/DefaultPostfixInflector.php

@@ -4,7 +4,7 @@
 /**
  * A route name inflector that appends the locale to the routes name except when the locale is the default locale.
  */
-class DefaultPostfixInflector implements RouteNameInflectorInterface
+class DefaultPostfixInflector extends PostfixInflector
 {
     /**
      * @var string
@@ -25,6 +25,6 @@ public function inflect($name, $locale)
             return $name;
         }
 
-        return $name.'.'.$locale;
+        return parent::inflect($name, $locale);
     }
 }

src/Routing/RouteGenerator/NameInflector/PostfixInflector.php

@@ -1,16 +1,85 @@
 <?php
 namespace BeSimple\I18nRoutingBundle\Routing\RouteGenerator\NameInflector;
+use Symfony\Component\Routing\RouteCollection;
 
 /**
  * A route name inflector that appends the locale to the routes name.
  */
 class PostfixInflector implements RouteNameInflectorInterface
 {
+    const INFIX = '.be-simple-i18n.';
+
     /**
      * @inheritdoc
      */
     public function inflect($name, $locale)
     {
-        return $name.'.'.$locale;
+        return $name . self::INFIX . $locale;
+    }
+
+    /**
+     * Reverse the inflection on an inflected route name.
+     *
+     * @param $name
+     * @param $locale
+     * @return string
+     */
+    public function unInflect($name, $locale)
+    {
+        $truncateHere = strpos($name, self::INFIX);
+        return substr($name, 0, $truncateHere);
+    }
+
+    /**
+     * Checks if $name has been inflected by this inflector.
+     *
+     * @param $name
+     * @param $locale
+     * @return bool
+     */
+    public function isInflected($name, $locale = '')
+    {
+        return false !== strpos($name, self::INFIX);
+    }
+
+    /**
+     * Checks if the constraints defined in the route definition are actually met.
+     *
+     * @param                 $name
+     * @param                 $locale
+     * @param RouteCollection $routeCollection
+     * @return bool
+     */
+    public function isValidMatch($name, $locale, RouteCollection $routeCollection = null)
+    {
+        $matchedRoute = $this->unInflect($name, $locale);
+
+         // locale does not match postfixed locale
+         if ($locale !== ($otherLocale = substr($name, - strlen($locale)))) {
+             if (is_null($routeCollection)) {
+                 return false;
+             }
+
+             // check if no another registered route does match, and then throw an exception
+             $otherRoute = $this->inflect($matchedRoute, $locale);
+
+             $allRoutes = $routeCollection->getIterator();
+
+             // there is no valid route for the other locale
+             if ( ! key_exists($otherRoute, $allRoutes)) {
+                 return false;
+             }
+
+             // there is a valid route for the other locale, but does the pathinfo match?
+             $originalPathInfo = $allRoutes[$name]->getPath();
+             $otherPathInfo = $allRoutes[$otherRoute]->getPath();
+
+             if ($originalPathInfo !== $otherPathInfo) {
+                 // mismatch
+                 return false;
+             }
+         }
+
+         return true;
     }
 }

src/Routing/RouteGenerator/NameInflector/RouteNameInflectorInterface.php

@@ -1,18 +1,47 @@
 <?php
 namespace BeSimple\I18nRoutingBundle\Routing\RouteGenerator\NameInflector;
+use Symfony\Component\Routing\RouteCollection;
 
 /**
  * Deduce the route name to use for a localized route.
  */
 interface RouteNameInflectorInterface
 {
     /**
-     * Return the route name and return it.
+     * Inflect the route name.
      *
      * @param string $name The route base name
      * @param string $locale The local
      *
      * @return string
      */
     public function inflect($name, $locale);
+
+    /**
+     * Reverse the inflection on an inflected route name.
+     *
+     * @param $name
+     * @param $locale
+     * @return mixed
+     */
+    public function unInflect($name, $locale);
+
+    /**
+     * Is used in the matching process to determine if isValidMatch() should be checked on a matched route.
+     *
+     * @param $name
+     * @param $locale
+     * @return mixed
+     */
+    public function isInflected($name, $locale = '');
+
+    /**
+     * Checks if the constraints defined in the route definition are actually met.
+     *
+     * @param                 $name
+     * @param                 $locale
+     * @param RouteCollection $routeCollection
+     * @return mixed
+     */
+    public function isValidMatch($name, $locale, RouteCollection $routeCollection);
 }

src/Routing/Router.php

@@ -109,10 +109,21 @@ public function generate($name, $parameters = array(), $referenceType = self::AB
     public function match($pathinfo)
     {
         $match = $this->router->match($pathinfo);
+        $route = $match['_route'];
+        $locale = $match['_locale'] ?? '';
 
-        // if a _locale parameter isset remove the .locale suffix that is appended to each route in I18nRoute
-        if (!empty($match['_locale']) && preg_match('#^(.+)\.'.preg_quote($match['_locale'], '#').'+$#', $match['_route'], $route)) {
-            $match['_route'] = $route[1];
+        if (empty($locale)) {
+            // no point in trying to match when we have no locale
+            return $match;
+        }
+
+        if ($this->routeNameInflector->isInflected($route, $locale)) {
+
+            if ( ! $this->routeNameInflector->isValidMatch($route, $locale, $this->getRouteCollection())) {
+                throw new RouteNotFoundException('A BeSimple route was matched, but it is not valid.');
+            }
+
+            $match['_route'] = $this->routeNameInflector->unInflect($route, $locale);
 
             // now also check if we want to translate parameters:
             if (null !== $this->translator && isset($match['_translate'])) {

tests/Routing/Loader/AnnotatedRouteControllerLoaderTest.php

@@ -43,10 +43,10 @@ public function testRoutesWithLocales()
         $this->assertContainsOnlyInstancesOf('Symfony\Component\Routing\Route', $rc);
         $this->assertCount(4, $rc);
 
-        $this->assertEquals('/', $rc->get('besimple_i18nrouting_tests_fixtures_noprefix_index.en')->getPath());
-        $this->assertEquals('/nl/', $rc->get('besimple_i18nrouting_tests_fixtures_noprefix_index.nl')->getPath());
-        $this->assertEquals('/new', $rc->get('new_action.en')->getPath());
-        $this->assertEquals('/nieuw', $rc->get('new_action.nl')->getPath());
+        $this->assertEquals('/', $rc->get('besimple_i18nrouting_tests_fixtures_noprefix_index.be-simple-i18n.en')->getPath());
+        $this->assertEquals('/nl/', $rc->get('besimple_i18nrouting_tests_fixtures_noprefix_index.be-simple-i18n.nl')->getPath());
+        $this->assertEquals('/new', $rc->get('new_action.be-simple-i18n.en')->getPath());
+        $this->assertEquals('/nieuw', $rc->get('new_action.be-simple-i18n.nl')->getPath());
     }
 
     public function testRoutesWithPrefixedLocales()
@@ -60,15 +60,15 @@ public function testRoutesWithPrefixedLocales()
         $this->assertContainsOnlyInstancesOf('Symfony\Component\Routing\Route', $rc);
         $this->assertCount(7, $rc);
 
-        $this->assertEquals('/en/', $rc->get('idx.en')->getPath());
-        $this->assertEquals('/nl/', $rc->get('idx.nl')->getPath());
-        $this->assertEquals('/fr/', $rc->get('idx.fr')->getPath());
+        $this->assertEquals('/en/', $rc->get('idx.be-simple-i18n.en')->getPath());
+        $this->assertEquals('/nl/', $rc->get('idx.be-simple-i18n.nl')->getPath());
+        $this->assertEquals('/fr/', $rc->get('idx.be-simple-i18n.fr')->getPath());
 
-        $this->assertEquals('/en/edit', $rc->get('edit.en')->getPath());
+        $this->assertEquals('/en/edit', $rc->get('edit.be-simple-i18n.en')->getPath());
 
-        $this->assertEquals('/en/new', $rc->get('new.en')->getPath());
-        $this->assertEquals('/nl/nieuw', $rc->get('new.nl')->getPath());
-        $this->assertEquals('/fr/nouveau', $rc->get('new.fr')->getPath());
+        $this->assertEquals('/en/new', $rc->get('new.be-simple-i18n.en')->getPath());
+        $this->assertEquals('/nl/nieuw', $rc->get('new.be-simple-i18n.nl')->getPath());
+        $this->assertEquals('/fr/nouveau', $rc->get('new.be-simple-i18n.fr')->getPath());
     }
 
     public function testRoutesWithStringPrefix()
@@ -82,8 +82,8 @@ public function testRoutesWithStringPrefix()
         $this->assertContainsOnlyInstancesOf('Symfony\Component\Routing\Route', $rc);
         $this->assertCount(3, $rc);
 
-        $this->assertEquals('/color/', $rc->get('idx.en')->getPath());
-        $this->assertEquals('/color/test', $rc->get('idx.test')->getPath());
+        $this->assertEquals('/color/', $rc->get('idx.be-simple-i18n.en')->getPath());
+        $this->assertEquals('/color/test', $rc->get('idx.be-simple-i18n.test')->getPath());
         $this->assertEquals('/color/plain', $rc->get('new')->getPath());
     }
 

tests/Routing/Loader/XmlFileLoaderTest.php

@@ -19,15 +19,15 @@ public function testBasicI18nRoute()
 
         $this->assertEquals(
             array(
-                'homepage_locale.en' => new Route('/en/', array(
+                'homepage_locale.be-simple-i18n.en' => new Route('/en/', array(
                     '_locale' => 'en',
                     '_controller' => 'TestBundle:Frontend:homepageLocale'
                 )),
-                'homepage_locale.de' => new Route('/de/', array(
+                'homepage_locale.be-simple-i18n.de' => new Route('/de/', array(
                     '_locale' => 'de',
                     '_controller' => 'TestBundle:Frontend:homepageLocale'
                 )),
-                'homepage_locale.fr' => new Route('/fr/', array(
+                'homepage_locale.be-simple-i18n.fr' => new Route('/fr/', array(
                     '_locale' => 'fr',
                     '_controller' => 'TestBundle:Frontend:homepageLocale'
                 )),

tests/Routing/Loader/YamlFileLoaderTest.php

@@ -65,15 +65,15 @@ public function testBasicI18nRoute()
 
         $this->assertEquals(
             array(
-                'homepage_locale.en' => new Route('/en/', array(
+                'homepage_locale.be-simple-i18n.en' => new Route('/en/', array(
                     '_locale' => 'en',
                     '_controller' => 'TestBundle:Frontend:homepageLocale'
                 )),
-                'homepage_locale.de' => new Route('/de/', array(
+                'homepage_locale.be-simple-i18n.de' => new Route('/de/', array(
                     '_locale' => 'de',
                     '_controller' => 'TestBundle:Frontend:homepageLocale'
                 )),
-                'homepage_locale.fr' => new Route('/fr/', array(
+                'homepage_locale.be-simple-i18n.fr' => new Route('/fr/', array(
                     '_locale' => 'fr',
                     '_controller' => 'TestBundle:Frontend:homepageLocale'
                 )),

tests/Routing/RouteGenerator/I18nRouteGeneratorTest.php

@@ -14,8 +14,8 @@ public function testGenerateRoutes()
 
         $this->assertEquals(
             array(
-                'test.en' => new Route('/en/', array('_locale' => 'en')),
-                'test.nl' => new Route('/nl/', array('_locale' => 'nl')),
+                'test.be-simple-i18n.en' => new Route('/en/', array('_locale' => 'en')),
+                'test.be-simple-i18n.nl' => new Route('/nl/', array('_locale' => 'nl')),
             ),
             $collection->all()
         );
@@ -79,8 +79,8 @@ public function testGenerateCollectionLocalizeRoutes()
 
         $this->assertEquals(
             array(
-                'test.en' => new Route('/en/test', array('_locale' => 'en')),
-                'test.nl' => new Route('/nl/test', array('_locale' => 'nl')),
+                'test.be-simple-i18n.en' => new Route('/en/test', array('_locale' => 'en')),
+                'test.be-simple-i18n.nl' => new Route('/nl/test', array('_locale' => 'nl')),
                 'test_localized' => new Route('/en/testing', array('_locale' => 'en')),
             ),
             $localizedCollection->all()

tests/Routing/RouteGenerator/NameInflector/DefaultPostfixInflectorTest.php

@@ -2,6 +2,7 @@
 namespace BeSimple\I18nRoutingBundle\Tests\Routing\RouteGenerator\NameInflector;
 
 use BeSimple\I18nRoutingBundle\Routing\RouteGenerator\NameInflector\DefaultPostfixInflector;
+use BeSimple\I18nRoutingBundle\Routing\RouteGenerator\NameInflector\PostfixInflector;
 
 class DefaultPostfixInflectorTest extends \PHPUnit_Framework_TestCase
 {
@@ -14,7 +15,7 @@ public function testInflect()
             $inflector->inflect('route.name', 'en')
         );
         $this->assertSame(
-            'route.name.nl',
+            'route.name' . PostfixInflector::INFIX .'nl',
             $inflector->inflect('route.name', 'nl')
         );
     }

tests/Routing/RouteGenerator/NameInflector/PostfixInflectorTest.php

@@ -11,12 +11,59 @@ public function testInflect()
         $inflector = new PostfixInflector();
 
         $this->assertSame(
-            'route.name.en',
+            'route.name.be-simple-i18n.en',
             $inflector->inflect('route.name', 'en')
         );
         $this->assertSame(
-            'route.name.nl',
+            'route.name.be-simple-i18n.nl',
             $inflector->inflect('route.name', 'nl')
         );
     }
+
+    public function testUnInflect()
+    {
+        $inflector = new PostfixInflector();
+        $this->assertSame('test', $inflector->unInflect('test' . $inflector::INFIX . 'nl', $locale = 'nl'));
+    }
+
+    public function testisInflected()
+    {
+        $inflector = new PostfixInflector();
+        $this->assertFalse($inflector->isInflected('test', 'nl'), "'test' is not at BeSimple route");
+        $this->assertTrue($inflector->isInflected('test' . PostfixInflector::INFIX . 'nl', 'nl'), 'Should have been a BeSimple route.');
+    }
+
+    public function testIsValidMatch()
+    {
+        $inflector = new PostfixInflector();
+        $this->assertTrue($inflector->isValidMatch('test' . PostfixInflector::INFIX . 'nl', 'nl'));
+        $this->assertFalse($inflector->isValidMatch('test' . PostfixInflector::INFIX . 'nl', 'en'));
+
+        // see if it works with more than one locale in a collection, all with the same path spec
+        $routeMock = $this->getMockBuilder('Symfony\Component\Routing\Route')
+            ->disableOriginalConstructor()->getMock();
+        $routeMock->expects($this->any())->method('getPath')->willReturn('/{_locale}/sites');
+
+        $routes = [
+            $inflector->inflect('sites', 'nl') => $routeMock,
+            $inflector->inflect('sites', 'de') => $routeMock,
+            $inflector->inflect('sites', 'se') => $routeMock,
+            $inflector->inflect('sites', 'fr') => $routeMock,
+            $inflector->inflect('sites', 'es') => $routeMock,
+        ];
+
+        $routeCollection = $this->getMock('Symfony\Component\Routing\RouteCollection');
+        $routeCollection->expects($this->any())->method('getIterator')->willReturn(new \ArrayIterator($routes));
+
+        $this->assertTrue($inflector->isValidMatch('sites' . PostfixInflector::INFIX . 'nl', 'nl'));
+        $this->assertTrue($inflector->isValidMatch('sites' . PostfixInflector::INFIX . 'de', 'de'));
+        $this->assertTrue($inflector->isValidMatch('sites' . PostfixInflector::INFIX . 'se', 'se'));
+        $this->assertTrue($inflector->isValidMatch('sites' . PostfixInflector::INFIX . 'fr', 'fr'));
+        $this->assertTrue($inflector->isValidMatch('sites' . PostfixInflector::INFIX . 'es', 'es'));
+
+        $this->assertFalse($inflector->isValidMatch('sites' . PostfixInflector::INFIX . 'es', 'nl'));
+        $this->assertFalse($inflector->isValidMatch('sites' . PostfixInflector::INFIX . 'de', 'nl'));
+        $this->assertFalse($inflector->isValidMatch('sites' . PostfixInflector::INFIX . 'fr', 'nl'));
+        $this->assertFalse($inflector->isValidMatch('sites' . PostfixInflector::INFIX . 'se', 'nl'));
+    }
 }