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

1.3.0: Bugs in href Phalcon\Mvc\Url or Phalcon\Mvc\Router #1938

Closed
xboston opened this issue Jan 29, 2014 · 10 comments
Closed

1.3.0: Bugs in href Phalcon\Mvc\Url or Phalcon\Mvc\Router #1938

xboston opened this issue Jan 29, 2014 · 10 comments

Comments

@xboston
Copy link
Contributor

xboston commented Jan 29, 2014

Example:

$di = new Phalcon\DI\FactoryDefault();

$di['url'] = function () {

    $url = new Phalcon\Mvc\Url();
    $url->setBasePath('/example/');

    return $url;
};

$router = new Phalcon\Mvc\Router();
$router->add('test')->setName('test');
$router->add('test/{id}')->setName('test-0');
$router->add('/test/{id}')->setName('test-00');


$group = new Phalcon\Mvc\Router\Group();
$group->setPrefix('/test');
$group->add('/test')->setName('test-1');
$group->add('/test/{id}')->setName('test-2');
$group->add('/test/test/teeeeest')->setName('test-3');


class TestRoute extends Phalcon\Mvc\Router\Group
{
    public function initialize()
    {
        $this->setPrefix('/test-class');
        $this->add('/test')->setName('test-4');
        $this->add('/test/{id:\d}')->setName('test-5');
        $this->add('/test/test/teeeeest')->setName('test-6');

    }
}

$router->mount($group);
$router->mount(new TestRoute);

$di['router'] = $router;

echo '1: ' . $di['url']->get(['for' => 'test']) . '<br />';
echo '2: ' . $di['url']->get(['for' => 'test-0', 'id' => 'none']) . '<br />';
echo '2.2: ' . $di['url']->get(['for' => 'test-00', 'id' => 'none']) . '<br />';
echo '3: ' . $di['url']->get(['for' => 'test-1']) . '<br />';
echo '4: ' . $di['url']->get(['for' => 'test-2', 'id' => 'none']) . '<br />';
echo '5: ' . $di['url']->get(['for' => 'test-3']) . '<br />';
echo '6: ' . $di['url']->get(['for' => 'test-4']) . '<br />';
echo '7: ' . $di['url']->get(['for' => 'test-5', 'id' => 'none']) . '<br />';
echo '8: ' . $di['url']->get(['for' => 'test-6']) . '<br />';

echo '9: ' . $di['url']->get(['for' => 'test']) . '<br />';
echo '10: ' . $di['url']->get(['for' => 'test-0']) . '<br />';
echo '11: ' . $di['url']->get(['for' => 'test-1']) . '<br />';
echo '12: ' . $di['url']->get(['for' => 'test-2']) . '<br />';
echo '13: ' . $di['url']->get(['for' => 'test-3']) . '<br />';
echo '14: ' . $di['url']->get(['for' => 'test-4']) . '<br />';
echo '15: ' . $di['url']->get(['for' => 'test-5']) . '<br />';
echo '16: ' . $di['url']->get(['for' => 'test-6']) . '<br />';

Results:

1.3.0
    1: /test
    2: /est/none
    2.2: /test/none
    3: //test/test
    4: /test/test/none
    5: //test/test/test/teeeeest
    6: //test-class/test
    7: /test-class/test/none
    8: //test-class/test/test/teeeeest
    9: /test
    10: /
    11: //test/test
    12: /
    13: //test/test/test/teeeeest
    14: //test-class/test
    15: /
    16: //test-class/test/test/teeeeest

1.2.6
    1: /test
    2: /est/none
    2.2: /test/none
    3: //test/test
    4: /test/test/none
    5: //test/test/test/teeeeest
    6: //test-class/test
    7: /test-class/test/none
    8: //test-class/test/test/teeeeest
    9: /test
    10: /est/
    11: //test/test
    12: /test/test/
    13: //test/test/test/teeeeest
    14: //test-class/test
    15: /test-class/test/
    16: //test-class/test/test/teeeeest
@ghost
Copy link

ghost commented Feb 2, 2014

Можешь вытащить бранч router из моего репозитория и проверить? Я откатил там проблемный коммит.

@xboston
Copy link
Contributor Author

xboston commented Feb 2, 2014

@sjinks ок, проверю

@xboston
Copy link
Contributor Author

xboston commented Feb 2, 2014

@sjinks намного лучше. Осталась лишь проблема с обрезанием ссылок.
$router->add('test') - нормаль, $router->add('test/{id}') - уберёт первую t, останется est. Но если работать с группой и указать преффикс - всё норм. Вот полный код проверки:

<?php

$di = new Phalcon\DI\FactoryDefault();

$di['url'] = function () {

    $url = new Phalcon\Mvc\Url();
    $url->setBasePath('/example/');

    return $url;
};

$router = new Phalcon\Mvc\Router();
$router->add('test/')->setName('test-00');
$router->add('test/{id}')->setName('test-0');


class TestRoute extends Phalcon\Mvc\Router\Group
{
    public function initialize()
    {
        $this->setPrefix('/test-class/');
        $this->add('test/{id:\d}')->setName('test-5');

    }
}

class TestRoute2 extends Phalcon\Mvc\Router\Group
{
    public function initialize()
    {

        $this->add('test/{id:\d}')->setName('test-6');

    }
}


$router->mount(new TestRoute);
$router->mount(new TestRoute2);


$di['router'] = $router;

echo '0: ' . $di['url']->get(['for' => 'test-00']) . '<br />';
echo '1: ' . $di['url']->get(['for' => 'test-0', 'id' => 'none']) . '<br />';
echo '2: ' . $di['url']->get(['for' => 'test-0']) . '<br />';
echo '3: ' . $di['url']->get(['for' => 'test-5', 'id' => 1]) . '<br />';
echo '4: ' . $di['url']->get(['for' => 'test-5']) . '<br />';
echo '5: ' . $di['url']->get(['for' => 'test-6', 'id' => 1]) . '<br />';
echo '6: ' . $di['url']->get(['for' => 'test-6']) . '<br />';

Результат:

0: /test/
1: /est/none
2: /est/
3: /test-class/test/1
4: /test-class/test/
5: /est/1
6: /est/

PS. Может подскажешь как оформить это в нормальные тесты? Покрыл бы тогда весь роутинг проверками ;)

@ghost
Copy link

ghost commented Feb 2, 2014

Самый простой способ — с использованием PHPT (http://qa.php.net/phpt_details.php)

Тест будет выглядеть примерно так:

--TEST--
Router test
--SKIPIF--
<?php include('skipif.inc'); ?>
--FILE--
<?php
$di = new Phalcon\DI\FactoryDefault();

$di['url'] = function () {

    $url = new Phalcon\Mvc\Url();
    $url->setBasePath('/example/');

    return $url;
};

$router = new Phalcon\Mvc\Router();
$router->add('test/')->setName('test-00');
$router->add('test/{id}')->setName('test-0');

class TestRoute extends Phalcon\Mvc\Router\Group
{
    public function initialize()
    {
        $this->setPrefix('/test-class/');
        $this->add('test/{id:\d}')->setName('test-5');
    }
}

class TestRoute2 extends Phalcon\Mvc\Router\Group
{
    public function initialize()
    {
        $this->add('test/{id:\d}')->setName('test-6');
    }
}

$router->mount(new TestRoute);
$router->mount(new TestRoute2);

$di['router'] = $router;

echo $di['url']->get(array('for' => 'test-00')), PHP_EOL;
echo $di['url']->get(array('for' => 'test-0', 'id' => 'none')), PHP_EOL;
echo $di['url']->get(array('for' => 'test-0')), PHP_EOL;
echo $di['url']->get(array('for' => 'test-5', 'id' => 1)), PHP_EOL;
echo $di['url']->get(array('for' => 'test-5')), PHP_EOL;
echo $di['url']->get(array('for' => 'test-6', 'id' => 1)), PHP_EOL;
echo $di['url']->get(array('for' => 'test-6')), PHP_EOL;
?>
--EXPECT--
/test/
/test/none
/test/
/test-class/test/1
/test-class/test/
/test/1
/test/

@ghost
Copy link

ghost commented Feb 2, 2014

Грубо говоря, то, что находится под --EXPECT--, будет сравниваться с выводом скрипта из-под --FILE--.

Тест кладётся в папку ext/tests, расширение у него должно быть .phpt

Запуск тестов — из ext/:

NO_INTERACTION=1 make test

@ghost
Copy link

ghost commented Feb 2, 2014

Тут фигня всплыла: https://github.com/phalcon/cphalcon/blob/1.3.0/ext/kernel/framework/router.c#L175

Грубо говоря, Url::get() ожидает, что маршрут всегда начинается со слэша.

То есть:

-$this->add('test/{id:\d}')->setName('test-5');
+$this->add('/test/{id:\d}')->setName('test-5');

В случае с setPrefix у префикса есть ведущий слэш, поэтому всё пушисто.

@ghost
Copy link

ghost commented Feb 2, 2014

Обновил свой бранч:

  • маршрут может не начинаться со слэша;
  • если префикс заканчиается слэшем, а маршрут начинается со слэша, лишний слэш при конкатенации убирается.

@xboston
Copy link
Contributor Author

xboston commented Feb 2, 2014

Про слеш и тесты понял, замучу.
Теперь всё работает отлично, всё ссылки как и ожидалось ;)

@ghost
Copy link

ghost commented Feb 25, 2014

Can this one be closed?

@xboston
Copy link
Contributor Author

xboston commented Feb 25, 2014

Yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants