Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
phpunit.xml
composer.lock
vendor/
.idea
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -25,6 +25,6 @@ public function inflect($name, $locale)
return $name;
}

return $name.'.'.$locale;
return parent::inflect($name, $locale);
}
}
71 changes: 70 additions & 1 deletion src/Routing/RouteGenerator/NameInflector/PostfixInflector.php
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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);
}
17 changes: 14 additions & 3 deletions src/Routing/Router.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'])) {
Expand Down
26 changes: 13 additions & 13 deletions tests/Routing/Loader/AnnotatedRouteControllerLoaderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand All @@ -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());
}

Expand Down
6 changes: 3 additions & 3 deletions tests/Routing/Loader/XmlFileLoaderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'
)),
Expand Down
6 changes: 3 additions & 3 deletions tests/Routing/Loader/YamlFileLoaderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'
)),
Expand Down
8 changes: 4 additions & 4 deletions tests/Routing/RouteGenerator/I18nRouteGeneratorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
);
Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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')
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
}
}
Loading