[RFC] Url generator

[aschempp]

Per our discussion, the UrlGenerator is supposed to replace Controller::generateFrontendUrl.

It allows for simple but flexible use of Contao urls (or pages actually).

1. $urlGenerator->generate('foo');

   Result: /foo.html

2. $urlGenerator->generate('index');

   Result: /

3. $urlGenerator->generate('foo/{items}', ['items' => 'bar']);

   Result: /foo/items/bar.html if items is not in $GLOBALS['TL_AUTO_ITEM']
   Result: /foo/bar.html if items is in $GLOBALS['TL_AUTO_ITEM']

4. $urlGenerator->generate('foo/{items}', ['items' => 'bar', 'auto_item' => 'items']);

   Result: /foo/bar.html (regardless of $GLOBALS['TL_AUTO_ITEM'])

5. $urlGenerator->generate('foo/{items}/{article}', ['items' => 'bar', 'article' => 'test']);

   Result: /foo/bar/article/test.html (assuming items is in $GLOBALS['TL_AUTO_ITEM'])

6. $urlGenerator->generate('foo/{items}.html', ['items' => 'bar', 'article' => 'test']);

   Result: /foo/bar.html?article=test (assuming items is in $GLOBALS['TL_AUTO_ITEM'])

7. Example with PageModel

   $page = \Contao\PageModel::findWithDetails(/* ... */);
   
   $urlGenerator->generate(
       $page->alias ?: $page->id, 
       [
           '_locale' => $page->rootLanguage,
           '_domain' => $page->domain,
           '_ssl' => (bool) $page->rootUseSSL,
       ]
   );

ToDo

• Handle host and schema
• Add docblocks with extensive info
• More unit tests ?

[qzminski]

Looks great but can you explain the behavior in 5th example? Why is {article} converted to article/test and simply not test?

[leofeyer]

This seems weird. Is it

if (array_key_exists('auto_item', $parameters)) {
    $autoItem = [$parameters['auto_item']];
} elseif (isset($GLOBALS['TL_AUTO_ITEM']) && is_array($GLOBALS['TL_AUTO_ITEM'])) {
    $autoItem = $GLOBALS['TL_AUTO_ITEM'];
} else {
    $autoItem = [];
}

?

[aschempp]

would result in the same, yeah

[aschempp]

Improved in 1ad5a83

[leofeyer]

As for your first TODO, we must not only handle host and scheme but all four reference types:

interface UrlGeneratorInterface extends RequestContextAwareInterface
{
    /**
     * Generates an absolute URL, e.g. "http://example.com/dir/file".
     */
    const ABSOLUTE_URL = 0;

    /**
     * Generates an absolute path, e.g. "/dir/file".
     */
    const ABSOLUTE_PATH = 1;

    /**
     * Generates a relative path based on the current request path, e.g. "../parent-file".
     *
     * @see UrlGenerator::getRelativePath()
     */
    const RELATIVE_PATH = 2;

    /**
     * Generates a network path, e.g. "//example.com/dir/file".
     * Such reference reuses the current scheme but specifies the host.
     */
    const NETWORK_PATH = 3;

[Toflar]

Aaaah, this is heaven! Thank you for working on it!

[aschempp]

@qzminski regarding point 5, this is the default behavior of Symfony UrlGenerator. If you pass additional parameters that are not used in the Url, they will be appended as query parameters.

[aschempp]

So that explained point 6 😂
Point 5: you can only have one auto_item, that's why the second one is ignored.

[aschempp]

PR updated and added a 7th example

[leofeyer]

Shouldn't we use the RELATIVE_PATH constant here and provided the base path in the context? Then we would not have to use substr().

[aschempp]

We don't want RELATIVE_PATH, it would return something like ../../foobar.html (depending on your current page).

[leofeyer]

depending on your current page

Exactly. And if we make sure that the base path is in the context, it will be relative to it. Just like we want it :)

https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Routing/Generator/UrlGenerator.php#L286

[Toflar]

As it is a specific Contao service anyway, can we add a helper method that accepts a PageModel?

[aschempp]

I thought about that too, but its rather useless if the pagemodel already has a method for that 😂

[Toflar]

Yeah, you're right. So we need an 8th example 😆

[Toflar]

Question: Can't we (or shouldn't we) deprecate Controller::generateFrontendUrl() then?

[DanielSchwiperich]

Not sure I get this right. You say

UrlGenerator is supposed to replace Controller::generateFrontendUrl.

But the implementation shown in this PR is not replacing it, it's be used within.

So is the goal to replace it in Contao 5 and now only change the inner workings of Controller::generateFrontendUrl ?

An option to add custom logic to it like now with generateFrontendUrl - Hook will remain, right?

[aschempp]

Imho it's supposed to replace it. You're right it does not (yet) fire events, but all the other stuff in Controller::generateFrontendUrl is actually not necessary to build a URL if you're working with Symfony. See example 7 above.

[leofeyer]

There is another problem: the router appends ?alias=index on the home page.

[leofeyer]

Why don't we just clone the context object?

[aschempp]

Because I want to change the object that is references in multiple places

[aschempp]

Not anymore 😎

[leofeyer]

As discussed in Mumble on May 12th, the PR can be merged. Additionally, we want to deprecate the Controller::getFrontendUrl() method and copy the path fixes, the sprintf fixes and the hook to the PageModel class. In the PageModel class, the hook shall trigger a deprecation warning.

[leofeyer]

Merged in 901ce97. I'll add a PR for the other changes.