From e7634b77f89c0517037246eec2af183ce19d99f8 Mon Sep 17 00:00:00 2001 From: Bart McLeod Date: Mon, 17 Oct 2016 17:34:48 +0200 Subject: [PATCH 1/9] fixes #94 Throws a RouteNotFoundException is a match is found where the matched locale from the prefix is different from that in the postfix --- .gitignore | 1 + .../NameInflector/PostfixInflector.php | 2 +- src/Routing/Router.php | 28 +++++++++------- .../AnnotatedRouteControllerLoaderTest.php | 26 +++++++-------- tests/Routing/Loader/XmlFileLoaderTest.php | 6 ++-- tests/Routing/Loader/YamlFileLoaderTest.php | 6 ++-- .../RouteGenerator/I18nRouteGeneratorTest.php | 8 ++--- .../NameInflector/PostfixInflectorTest.php | 4 +-- tests/Routing/RouterTest.php | 33 +++++++++++++++---- 9 files changed, 69 insertions(+), 45 deletions(-) diff --git a/.gitignore b/.gitignore index 5de5676cf..8bcc5d150 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ phpunit.xml composer.lock vendor/ +.idea diff --git a/src/Routing/RouteGenerator/NameInflector/PostfixInflector.php b/src/Routing/RouteGenerator/NameInflector/PostfixInflector.php index 7a59097e2..226da23fa 100644 --- a/src/Routing/RouteGenerator/NameInflector/PostfixInflector.php +++ b/src/Routing/RouteGenerator/NameInflector/PostfixInflector.php @@ -11,6 +11,6 @@ class PostfixInflector implements RouteNameInflectorInterface */ public function inflect($name, $locale) { - return $name.'.'.$locale; + return $name.'.be-simple-i18n.'.$locale; } } diff --git a/src/Routing/Router.php b/src/Routing/Router.php index 7029cd452..d59b65979 100644 --- a/src/Routing/Router.php +++ b/src/Routing/Router.php @@ -111,18 +111,22 @@ public function match($pathinfo) $match = $this->router->match($pathinfo); // 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]; - - // now also check if we want to translate parameters: - if (null !== $this->translator && isset($match['_translate'])) { - foreach ((array) $match['_translate'] as $attribute) { - $match[$attribute] = $this->translator->translate( - $match['_route'], - $match['_locale'], - $attribute, - $match[$attribute] - ); + if ( ! empty($match['_locale'])) { + if (preg_match('#^(.+)\.be-simple-i18n\.' . preg_quote($match['_locale'], '#') . '+$#', $match['_route'], $route)) { + $match['_route'] = $route[1]; + + // now also check if we want to translate parameters: + if (null !== $this->translator && isset($match['_translate'])) { + foreach ((array) $match['_translate'] as $attribute) { + $match[$attribute] = $this->translator->translate( + $match['_route'], $match['_locale'], $attribute, $match[$attribute] + ); + } + } + } else { + // check if this is a route, configured as i18n route, if so, throw the exception + if (preg_match("/^.*\.be-simple-i18n\.[a-z]{2}$/", $match['_route'])) { + throw new RouteNotFoundException('A route was matched, but the locale provided does not match the locale of the matched route.'); } } } diff --git a/tests/Routing/Loader/AnnotatedRouteControllerLoaderTest.php b/tests/Routing/Loader/AnnotatedRouteControllerLoaderTest.php index 967da950b..d1cf20c18 100644 --- a/tests/Routing/Loader/AnnotatedRouteControllerLoaderTest.php +++ b/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()); } diff --git a/tests/Routing/Loader/XmlFileLoaderTest.php b/tests/Routing/Loader/XmlFileLoaderTest.php index c3aa6994a..f56871a0c 100644 --- a/tests/Routing/Loader/XmlFileLoaderTest.php +++ b/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' )), diff --git a/tests/Routing/Loader/YamlFileLoaderTest.php b/tests/Routing/Loader/YamlFileLoaderTest.php index 93f7e756f..fa9e21cf4 100644 --- a/tests/Routing/Loader/YamlFileLoaderTest.php +++ b/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' )), diff --git a/tests/Routing/RouteGenerator/I18nRouteGeneratorTest.php b/tests/Routing/RouteGenerator/I18nRouteGeneratorTest.php index de8605735..94d59d898 100644 --- a/tests/Routing/RouteGenerator/I18nRouteGeneratorTest.php +++ b/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() diff --git a/tests/Routing/RouteGenerator/NameInflector/PostfixInflectorTest.php b/tests/Routing/RouteGenerator/NameInflector/PostfixInflectorTest.php index d0c2f7b42..868bf4434 100644 --- a/tests/Routing/RouteGenerator/NameInflector/PostfixInflectorTest.php +++ b/tests/Routing/RouteGenerator/NameInflector/PostfixInflectorTest.php @@ -11,11 +11,11 @@ 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') ); } diff --git a/tests/Routing/RouterTest.php b/tests/Routing/RouterTest.php index 06a54c9e1..b7f149920 100644 --- a/tests/Routing/RouterTest.php +++ b/tests/Routing/RouterTest.php @@ -15,13 +15,13 @@ public function testMatchLocaleRoute() ->expects($this->at(0)) ->method('match') ->with($this->equalTo('/foo')) - ->will($this->returnValue(array('_route' => 'test.en', '_locale' => 'en'))) + ->will($this->returnValue(array('_route' => 'test.be-simple-i18n.en', '_locale' => 'en'))) ; $parentRouter ->expects($this->at(1)) ->method('match') ->with($this->equalTo('/bar')) - ->will($this->returnValue(array('_route' => 'test.de', '_locale' => 'de'))) + ->will($this->returnValue(array('_route' => 'test.be-simple-i18n.de', '_locale' => 'de'))) ; $router = new Router($parentRouter); @@ -41,7 +41,7 @@ public function testMatchTranslateStringField() $parentRouter->expects($this->any()) ->method('match') ->with($this->equalTo('/foo/beberlei')) - ->will($this->returnValue(array('_route' => 'test.en', '_locale' => 'en', '_translate' => 'name', 'name' => 'beberlei'))) + ->will($this->returnValue(array('_route' => 'test.be-simple-i18n.en', '_locale' => 'en', '_translate' => 'name', 'name' => 'beberlei'))) ; $translator = $this->getMock('BeSimple\I18nRoutingBundle\Routing\Translator\AttributeTranslatorInterface'); $translator @@ -63,7 +63,7 @@ public function testGenerateI18n() $parentRouter = $this->mockParentRouter(); $parentRouter->expects($this->once()) ->method('generate') - ->with($this->equalTo('test_route.en'), $this->equalTo(array('foo' => 'bar')), $this->equalTo(false)) + ->with($this->equalTo('test_route.be-simple-i18n.en'), $this->equalTo(array('foo' => 'bar')), $this->equalTo(false)) ; $router = new Router($parentRouter); @@ -129,7 +129,7 @@ public function testGenerateI18nTranslated() $parentRouter = $this->mockParentRouter(); $parentRouter->expects($this->once()) ->method('generate') - ->with($this->equalTo('test_route.en'), $this->equalTo(array('foo' => 'baz')), $this->equalTo(false)) + ->with($this->equalTo('test_route.be-simple-i18n.en'), $this->equalTo(array('foo' => 'baz')), $this->equalTo(false)) ; $translator = $this->getMock('BeSimple\I18nRoutingBundle\Routing\Translator\AttributeTranslatorInterface'); $translator @@ -143,13 +143,32 @@ public function testGenerateI18nTranslated() $router->generate('test_route', array('foo' => 'bar', 'translate' => 'foo', 'locale' => 'en'), false); } + /** + * @expectedException \Symfony\Component\Routing\Exception\RouteNotFoundException + */ + public function testTheConfiguredLocaleIsTheSameAsThePrefixLocaleWhenMatched() + { + $parentRouter = $this->getMock('Symfony\Component\Routing\RouterInterface'); + + $parentRouter->expects($this->once()) + ->method('match') + ->with($this->equalTo('/en/testen')) + ->will($this->returnValue(array('_route' => 'test.be-simple-i18n.nl', '_locale' => 'en'))); + + $router = new Router($parentRouter); + + $this->assertInstanceOf(Router::class, $router); + + $match = $router->match('/en/testen'); + } + public function testGenerateI18nTranslatedContextLocale() { $parentRouter = $this->mockParentRouter(); $parentRouter->expects($this->once()) ->method('generate') - ->with($this->equalTo('test_route.fr'), $this->equalTo(array('foo' => 'baz')), $this->equalTo(false)) + ->with($this->equalTo('test_route.be-simple-i18n.fr'), $this->equalTo(array('foo' => 'baz')), $this->equalTo(false)) ; $translator = $this->getMock('BeSimple\I18nRoutingBundle\Routing\Translator\AttributeTranslatorInterface'); $translator @@ -229,7 +248,7 @@ public function testGenerateWithDefaultLocaleButWithoutRoute() ->method('generate') ->withConsecutive( array('test_route', array('foo' => 'bar'), false), - array('test_route.en', array('foo' => 'bar'), false) + array('test_route.be-simple-i18n.en', array('foo' => 'bar'), false) ) ->willThrowException(new RouteNotFoundException()); ; From 501cd6b3fd9dd241b71503e348ab807394315d12 Mon Sep 17 00:00:00 2001 From: Bart McLeod Date: Wed, 19 Oct 2016 12:12:45 +0200 Subject: [PATCH 2/9] fixed the issue when the paths are the same and an arbitrary locale is matched first and wins --- src/Routing/Router.php | 57 +++++++++++++++++++++++++++++++----------- 1 file changed, 43 insertions(+), 14 deletions(-) diff --git a/src/Routing/Router.php b/src/Routing/Router.php index d59b65979..20e78b4b0 100644 --- a/src/Routing/Router.php +++ b/src/Routing/Router.php @@ -109,26 +109,55 @@ public function generate($name, $parameters = array(), $referenceType = self::AB public function match($pathinfo) { $match = $this->router->match($pathinfo); + $route = $match['_route']; - // if a _locale parameter isset remove the .locale suffix that is appended to each route in I18nRoute - if ( ! empty($match['_locale'])) { - if (preg_match('#^(.+)\.be-simple-i18n\.' . preg_quote($match['_locale'], '#') . '+$#', $match['_route'], $route)) { - $match['_route'] = $route[1]; + // @todo: let the inflector take responsibility for this + // if the matched route is a BeSimple route remove the .be-simple-i18n.locale suffix that is appended to each route in I18nRoute + if (false !== ($truncateHere = strpos($route, '.be-simple-i18n.'))) { + // throw an exception if the locale does not match the postfixed locale + $locale = $match['_locale']; - // now also check if we want to translate parameters: - if (null !== $this->translator && isset($match['_translate'])) { - foreach ((array) $match['_translate'] as $attribute) { - $match[$attribute] = $this->translator->translate( - $match['_route'], $match['_locale'], $attribute, $match[$attribute] - ); - } + $matchedRoute = substr($route, 0, $truncateHere); + + // locale does not match postfixed locale + if ($locale !== ($otherLocale = substr($route, - strlen($locale)))) { + // check if no another registered route does match, and then throw an exception + $otherRoute = $this->routeNameInflector->inflect($matchedRoute, $locale); + + if (is_null($this->getRouteCollection())) { + throw new RouteNotFoundException('A route was matched, but the locale provided does not match the locale of the matched route.'); + } + + $allRoutes = $this->getRouteCollection()->getIterator(); + + // there is no valid route for the other locale + if (!key_exists($otherRoute, $allRoutes)) { + throw new RouteNotFoundException('A route was matched, but the locale provided does not match the locale of the matched route.'); } - } else { - // check if this is a route, configured as i18n route, if so, throw the exception - if (preg_match("/^.*\.be-simple-i18n\.[a-z]{2}$/", $match['_route'])) { + + // there is a valid route for the other locale, but does the pathinfo match? + $originalPathInfo = $allRoutes[$route]->getPath(); + $otherPathInfo = $allRoutes[$otherRoute]->getPath(); + + if ($originalPathInfo !== $otherPathInfo) { + // mismatch throw new RouteNotFoundException('A route was matched, but the locale provided does not match the locale of the matched route.'); } } + + $match['_route'] = $matchedRoute; + + // now also check if we want to translate parameters: + if (null !== $this->translator && isset($match['_translate'])) { + foreach ((array) $match['_translate'] as $attribute) { + $match[$attribute] = $this->translator->translate( + $match['_route'], + $match['_locale'], + $attribute, + $match[$attribute] + ); + } + } } return $match; From afb601fee24c45bf5ad02c0af5e30363a2c02ed8 Mon Sep 17 00:00:00 2001 From: Bart McLeod Date: Wed, 19 Oct 2016 15:17:56 +0200 Subject: [PATCH 3/9] extended the inflector and the interface to take care of strict route matching (the locale postfix should agree with the matched locale prefix) --- .../NameInflector/DefaultPostfixInflector.php | 4 +- .../NameInflector/PostfixInflector.php | 72 ++++++++++++++++++- .../RouteNameInflectorInterface.php | 31 +++++++- src/Routing/Router.php | 37 ++-------- .../DefaultPostfixInflectorTest.php | 3 +- .../NameInflector/PostfixInflectorTest.php | 47 ++++++++++++ tests/Routing/RouterTest.php | 20 +++--- 7 files changed, 167 insertions(+), 47 deletions(-) diff --git a/src/Routing/RouteGenerator/NameInflector/DefaultPostfixInflector.php b/src/Routing/RouteGenerator/NameInflector/DefaultPostfixInflector.php index e81cf48e4..892464a82 100644 --- a/src/Routing/RouteGenerator/NameInflector/DefaultPostfixInflector.php +++ b/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); } } diff --git a/src/Routing/RouteGenerator/NameInflector/PostfixInflector.php b/src/Routing/RouteGenerator/NameInflector/PostfixInflector.php index 226da23fa..4dde70876 100644 --- a/src/Routing/RouteGenerator/NameInflector/PostfixInflector.php +++ b/src/Routing/RouteGenerator/NameInflector/PostfixInflector.php @@ -1,16 +1,86 @@ 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; } } diff --git a/src/Routing/RouteGenerator/NameInflector/RouteNameInflectorInterface.php b/src/Routing/RouteGenerator/NameInflector/RouteNameInflectorInterface.php index f20218ef0..37e2e9662 100644 --- a/src/Routing/RouteGenerator/NameInflector/RouteNameInflectorInterface.php +++ b/src/Routing/RouteGenerator/NameInflector/RouteNameInflectorInterface.php @@ -1,5 +1,6 @@ router->match($pathinfo); $route = $match['_route']; + $locale = $match['_locale']; - // @todo: let the inflector take responsibility for this - // if the matched route is a BeSimple route remove the .be-simple-i18n.locale suffix that is appended to each route in I18nRoute - if (false !== ($truncateHere = strpos($route, '.be-simple-i18n.'))) { - // throw an exception if the locale does not match the postfixed locale - $locale = $match['_locale']; + if ($this->routeNameInflector->isBeSimpleRoute($route, $locale)) { - $matchedRoute = substr($route, 0, $truncateHere); - - // locale does not match postfixed locale - if ($locale !== ($otherLocale = substr($route, - strlen($locale)))) { - // check if no another registered route does match, and then throw an exception - $otherRoute = $this->routeNameInflector->inflect($matchedRoute, $locale); - - if (is_null($this->getRouteCollection())) { - throw new RouteNotFoundException('A route was matched, but the locale provided does not match the locale of the matched route.'); - } - - $allRoutes = $this->getRouteCollection()->getIterator(); - - // there is no valid route for the other locale - if (!key_exists($otherRoute, $allRoutes)) { - throw new RouteNotFoundException('A route was matched, but the locale provided does not match the locale of the matched route.'); - } - - // there is a valid route for the other locale, but does the pathinfo match? - $originalPathInfo = $allRoutes[$route]->getPath(); - $otherPathInfo = $allRoutes[$otherRoute]->getPath(); - - if ($originalPathInfo !== $otherPathInfo) { - // mismatch - throw new RouteNotFoundException('A route was matched, but the locale provided does not match the locale of the matched route.'); - } + if ( ! $this->routeNameInflector->isValidMatch($route, $locale, $this->getRouteCollection())) { + throw new RouteNotFoundException('A BeSimple route was matched, but it is not valid.'); } - $match['_route'] = $matchedRoute; + $match['_route'] = $this->routeNameInflector->unInflect($route, $locale); // now also check if we want to translate parameters: if (null !== $this->translator && isset($match['_translate'])) { diff --git a/tests/Routing/RouteGenerator/NameInflector/DefaultPostfixInflectorTest.php b/tests/Routing/RouteGenerator/NameInflector/DefaultPostfixInflectorTest.php index f7a9b0412..92f6509b2 100644 --- a/tests/Routing/RouteGenerator/NameInflector/DefaultPostfixInflectorTest.php +++ b/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') ); } diff --git a/tests/Routing/RouteGenerator/NameInflector/PostfixInflectorTest.php b/tests/Routing/RouteGenerator/NameInflector/PostfixInflectorTest.php index 868bf4434..43c3f3b97 100644 --- a/tests/Routing/RouteGenerator/NameInflector/PostfixInflectorTest.php +++ b/tests/Routing/RouteGenerator/NameInflector/PostfixInflectorTest.php @@ -19,4 +19,51 @@ public function testInflect() $inflector->inflect('route.name', 'nl') ); } + + public function testUnInflect() + { + $inflector = new PostfixInflector(); + $this->assertSame('test', $inflector->unInflect('test' . $inflector::INFIX . 'nl', $locale = 'nl')); + } + + public function testisBeSimpleRoute() + { + $inflector = new PostfixInflector(); + $this->assertFalse($inflector->isBeSimpleRoute('test', 'nl'), "'test' is not at BeSimple route"); + $this->assertTrue($inflector->isBeSimpleRoute('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')); + } } diff --git a/tests/Routing/RouterTest.php b/tests/Routing/RouterTest.php index b7f149920..750f6a033 100644 --- a/tests/Routing/RouterTest.php +++ b/tests/Routing/RouterTest.php @@ -2,9 +2,9 @@ namespace BeSimple\I18nRoutingBundle\Tests\Routing; +use BeSimple\I18nRoutingBundle\Routing\RouteGenerator\NameInflector\PostfixInflector; use BeSimple\I18nRoutingBundle\Routing\Router; use Symfony\Component\Routing\Exception\RouteNotFoundException; -use Symfony\Component\Routing\RequestContext; class RouterTest extends \PHPUnit_Framework_TestCase { @@ -15,13 +15,13 @@ public function testMatchLocaleRoute() ->expects($this->at(0)) ->method('match') ->with($this->equalTo('/foo')) - ->will($this->returnValue(array('_route' => 'test.be-simple-i18n.en', '_locale' => 'en'))) + ->will($this->returnValue(array('_route' => 'test' . PostfixInflector::INFIX . 'en', '_locale' => 'en'))) ; $parentRouter - ->expects($this->at(1)) + ->expects($this->at(2)) ->method('match') ->with($this->equalTo('/bar')) - ->will($this->returnValue(array('_route' => 'test.be-simple-i18n.de', '_locale' => 'de'))) + ->will($this->returnValue(array('_route' => 'test' . PostfixInflector::INFIX . 'de', '_locale' => 'de'))) ; $router = new Router($parentRouter); @@ -41,7 +41,7 @@ public function testMatchTranslateStringField() $parentRouter->expects($this->any()) ->method('match') ->with($this->equalTo('/foo/beberlei')) - ->will($this->returnValue(array('_route' => 'test.be-simple-i18n.en', '_locale' => 'en', '_translate' => 'name', 'name' => 'beberlei'))) + ->will($this->returnValue(array('_route' => 'test' . PostfixInflector::INFIX . 'en', '_locale' => 'en', '_translate' => 'name', 'name' => 'beberlei'))) ; $translator = $this->getMock('BeSimple\I18nRoutingBundle\Routing\Translator\AttributeTranslatorInterface'); $translator @@ -63,7 +63,7 @@ public function testGenerateI18n() $parentRouter = $this->mockParentRouter(); $parentRouter->expects($this->once()) ->method('generate') - ->with($this->equalTo('test_route.be-simple-i18n.en'), $this->equalTo(array('foo' => 'bar')), $this->equalTo(false)) + ->with($this->equalTo('test_route' . PostfixInflector::INFIX . 'en'), $this->equalTo(array('foo' => 'bar')), $this->equalTo(false)) ; $router = new Router($parentRouter); @@ -129,7 +129,7 @@ public function testGenerateI18nTranslated() $parentRouter = $this->mockParentRouter(); $parentRouter->expects($this->once()) ->method('generate') - ->with($this->equalTo('test_route.be-simple-i18n.en'), $this->equalTo(array('foo' => 'baz')), $this->equalTo(false)) + ->with($this->equalTo('test_route' . PostfixInflector::INFIX . 'en'), $this->equalTo(array('foo' => 'baz')), $this->equalTo(false)) ; $translator = $this->getMock('BeSimple\I18nRoutingBundle\Routing\Translator\AttributeTranslatorInterface'); $translator @@ -153,7 +153,7 @@ public function testTheConfiguredLocaleIsTheSameAsThePrefixLocaleWhenMatched() $parentRouter->expects($this->once()) ->method('match') ->with($this->equalTo('/en/testen')) - ->will($this->returnValue(array('_route' => 'test.be-simple-i18n.nl', '_locale' => 'en'))); + ->will($this->returnValue(array('_route' => 'test' . PostfixInflector::INFIX . 'nl', '_locale' => 'en'))); $router = new Router($parentRouter); @@ -168,7 +168,7 @@ public function testGenerateI18nTranslatedContextLocale() $parentRouter->expects($this->once()) ->method('generate') - ->with($this->equalTo('test_route.be-simple-i18n.fr'), $this->equalTo(array('foo' => 'baz')), $this->equalTo(false)) + ->with($this->equalTo('test_route' . PostfixInflector::INFIX . 'fr'), $this->equalTo(array('foo' => 'baz')), $this->equalTo(false)) ; $translator = $this->getMock('BeSimple\I18nRoutingBundle\Routing\Translator\AttributeTranslatorInterface'); $translator @@ -248,7 +248,7 @@ public function testGenerateWithDefaultLocaleButWithoutRoute() ->method('generate') ->withConsecutive( array('test_route', array('foo' => 'bar'), false), - array('test_route.be-simple-i18n.en', array('foo' => 'bar'), false) + array('test_route' . PostfixInflector::INFIX . 'en', array('foo' => 'bar'), false) ) ->willThrowException(new RouteNotFoundException()); ; From ed1ec3c3eaba08861c85e977c6c63656ca8afff0 Mon Sep 17 00:00:00 2001 From: Bart McLeod Date: Wed, 19 Oct 2016 16:06:19 +0200 Subject: [PATCH 4/9] fixed a wrong function call --- src/Routing/RouteGenerator/NameInflector/PostfixInflector.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Routing/RouteGenerator/NameInflector/PostfixInflector.php b/src/Routing/RouteGenerator/NameInflector/PostfixInflector.php index 4dde70876..944c7f9b4 100644 --- a/src/Routing/RouteGenerator/NameInflector/PostfixInflector.php +++ b/src/Routing/RouteGenerator/NameInflector/PostfixInflector.php @@ -52,8 +52,7 @@ public function isBeSimpleRoute($name, $locale = '') */ public function isValidMatch($name, $locale, RouteCollection $routeCollection = null) { - $truncateHere = strpos(self::INFIX, $name); - $matchedRoute = substr($name, 0, $truncateHere); + $matchedRoute = $this->unInflect($name, $locale); // locale does not match postfixed locale if ($locale !== ($otherLocale = substr($name, - strlen($locale)))) { From 60f2ed6f502b8869cb6a1dc5186f1a358850778e Mon Sep 17 00:00:00 2001 From: Bart McLeod Date: Thu, 20 Oct 2016 07:09:42 +0200 Subject: [PATCH 5/9] improved naming and return type documentation --- .../RouteGenerator/NameInflector/PostfixInflector.php | 10 +++++----- .../NameInflector/RouteNameInflectorInterface.php | 2 +- src/Routing/Router.php | 2 +- .../NameInflector/PostfixInflectorTest.php | 6 +++--- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Routing/RouteGenerator/NameInflector/PostfixInflector.php b/src/Routing/RouteGenerator/NameInflector/PostfixInflector.php index 944c7f9b4..4455faa6c 100644 --- a/src/Routing/RouteGenerator/NameInflector/PostfixInflector.php +++ b/src/Routing/RouteGenerator/NameInflector/PostfixInflector.php @@ -22,7 +22,7 @@ public function inflect($name, $locale) * * @param $name * @param $locale - * @return mixed + * @return string */ public function unInflect($name, $locale) { @@ -31,13 +31,13 @@ public function unInflect($name, $locale) } /** - * Is used in the matching process to determine if isValidMatch() should be checked on a matched route. + * Checks if $name has been inflected by this inflector. * * @param $name * @param $locale - * @return mixed + * @return bool */ - public function isBeSimpleRoute($name, $locale = '') + public function isInflected($name, $locale = '') { return false !== strpos($name, self::INFIX); } @@ -48,7 +48,7 @@ public function isBeSimpleRoute($name, $locale = '') * @param $name * @param $locale * @param RouteCollection $routeCollection - * @return mixed + * @return bool */ public function isValidMatch($name, $locale, RouteCollection $routeCollection = null) { diff --git a/src/Routing/RouteGenerator/NameInflector/RouteNameInflectorInterface.php b/src/Routing/RouteGenerator/NameInflector/RouteNameInflectorInterface.php index 37e2e9662..45240e594 100644 --- a/src/Routing/RouteGenerator/NameInflector/RouteNameInflectorInterface.php +++ b/src/Routing/RouteGenerator/NameInflector/RouteNameInflectorInterface.php @@ -33,7 +33,7 @@ public function unInflect($name, $locale); * @param $locale * @return mixed */ - public function isBeSimpleRoute($name, $locale = ''); + public function isInflected($name, $locale = ''); /** * Checks if the constraints defined in the route definition are actually met. diff --git a/src/Routing/Router.php b/src/Routing/Router.php index a41dc394e..226719be5 100644 --- a/src/Routing/Router.php +++ b/src/Routing/Router.php @@ -112,7 +112,7 @@ public function match($pathinfo) $route = $match['_route']; $locale = $match['_locale']; - if ($this->routeNameInflector->isBeSimpleRoute($route, $locale)) { + 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.'); diff --git a/tests/Routing/RouteGenerator/NameInflector/PostfixInflectorTest.php b/tests/Routing/RouteGenerator/NameInflector/PostfixInflectorTest.php index 43c3f3b97..0b437d312 100644 --- a/tests/Routing/RouteGenerator/NameInflector/PostfixInflectorTest.php +++ b/tests/Routing/RouteGenerator/NameInflector/PostfixInflectorTest.php @@ -26,11 +26,11 @@ public function testUnInflect() $this->assertSame('test', $inflector->unInflect('test' . $inflector::INFIX . 'nl', $locale = 'nl')); } - public function testisBeSimpleRoute() + public function testisInflected() { $inflector = new PostfixInflector(); - $this->assertFalse($inflector->isBeSimpleRoute('test', 'nl'), "'test' is not at BeSimple route"); - $this->assertTrue($inflector->isBeSimpleRoute('test' . PostfixInflector::INFIX . 'nl', 'nl'), 'Should have been a BeSimple route.'); + $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() From 459f7648780d298558c742bfa57ea7459f1b66f8 Mon Sep 17 00:00:00 2001 From: Bart McLeod Date: Fri, 21 Oct 2016 11:57:31 +0200 Subject: [PATCH 6/9] fixed: we should not count on the _locale to be present in the match --- src/Routing/Router.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Routing/Router.php b/src/Routing/Router.php index 226719be5..333d05335 100644 --- a/src/Routing/Router.php +++ b/src/Routing/Router.php @@ -110,7 +110,7 @@ public function match($pathinfo) { $match = $this->router->match($pathinfo); $route = $match['_route']; - $locale = $match['_locale']; + $locale = $match['_locale'] ?? ''; if ($this->routeNameInflector->isInflected($route, $locale)) { From 88543234ea95fab9805f047988bee9642853b569 Mon Sep 17 00:00:00 2001 From: Bart McLeod Date: Tue, 25 Oct 2016 21:22:08 +0200 Subject: [PATCH 7/9] guard against checking on an empty locale: in some cases, redirects may cause the locale parameter will be lost, in which case, strict checking is impossible --- src/Routing/Router.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Routing/Router.php b/src/Routing/Router.php index 333d05335..570b06099 100644 --- a/src/Routing/Router.php +++ b/src/Routing/Router.php @@ -114,7 +114,8 @@ public function match($pathinfo) if ($this->routeNameInflector->isInflected($route, $locale)) { - if ( ! $this->routeNameInflector->isValidMatch($route, $locale, $this->getRouteCollection())) { + // we can't check without a locale + if (! empty($locale) && ! $this->routeNameInflector->isValidMatch($route, $locale, $this->getRouteCollection())) { throw new RouteNotFoundException('A BeSimple route was matched, but it is not valid.'); } From a5ae1b40b77155cad15cea29c8e2afa04283dd68 Mon Sep 17 00:00:00 2001 From: Bart McLeod Date: Sat, 29 Oct 2016 22:41:19 +0200 Subject: [PATCH 8/9] No match if the is no _locale, this is better because it is backwards compatible with what we had before strict matching --- src/Routing/Router.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Routing/Router.php b/src/Routing/Router.php index 570b06099..c82e7fe49 100644 --- a/src/Routing/Router.php +++ b/src/Routing/Router.php @@ -112,10 +112,15 @@ public function match($pathinfo) $route = $match['_route']; $locale = $match['_locale'] ?? ''; + if (empty($locale)) { + // no point in trying to match when we have no locale + return $match; + } + if ($this->routeNameInflector->isInflected($route, $locale)) { // we can't check without a locale - if (! empty($locale) && ! $this->routeNameInflector->isValidMatch($route, $locale, $this->getRouteCollection())) { + if ( ! $this->routeNameInflector->isValidMatch($route, $locale, $this->getRouteCollection())) { throw new RouteNotFoundException('A BeSimple route was matched, but it is not valid.'); } From 32b343ac0217baf2ee4c2d6c7dd25c2e63e8f01b Mon Sep 17 00:00:00 2001 From: Bart McLeod Date: Sat, 29 Oct 2016 23:17:58 +0200 Subject: [PATCH 9/9] remove obsolete comment --- src/Routing/Router.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Routing/Router.php b/src/Routing/Router.php index c82e7fe49..ab11c3409 100644 --- a/src/Routing/Router.php +++ b/src/Routing/Router.php @@ -119,7 +119,6 @@ public function match($pathinfo) if ($this->routeNameInflector->isInflected($route, $locale)) { - // we can't check without a locale if ( ! $this->routeNameInflector->isValidMatch($route, $locale, $this->getRouteCollection())) { throw new RouteNotFoundException('A BeSimple route was matched, but it is not valid.'); }