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

Cli - Deault domain/host - for generating routes #47

Open
nakashu opened this issue Jan 28, 2015 · 9 comments
Open

Cli - Deault domain/host - for generating routes #47

nakashu opened this issue Jan 28, 2015 · 9 comments

Comments

@nakashu
Copy link

nakashu commented Jan 28, 2015

Current Nette\Http\RequestFactory doesnt support adding default domain for the request

Problems starts when we need to generate links, after executing the script in CLI
eg.:

    MyCliPresenter->generateLinksAction() {
        echo $this->link('Homepage:default');
    }
    // outputs:  http:/// 

Whole problem could be solved by adding support for default values for domain and adding a bit of code for using them inside RequestFactory::createHttpRequest() method

        ...
        // after line 94 add
        // use default host, if we didnt get any from $SERVER.
        if (!$url->host)
        {
            $url->host = $this->domain;
        }

this->domain should be populated by setter function, with DI in setup

Another sollution is to use change RequestFactory to be easily extendebale, currently if we want to override createHttpRequest(), with own implementation, it fail when when the script tries to write to $this->binary as there is private access on the variable.
For easier extendebility createHttpRequest should be split to smaller pieces. eg. process Query, Hostnamem, Scheme, Binary, etc.

Bellow are 2 current solutions for the problem,

  1. create own HttpRequest and add it as service instead of one generated by factory
  2. change service for httpRequestFactory and HttpRequest for own classes.

bootstrap.php:

    ...
    /**
     * @param \Nette\Configurator $configurator
     * @param \Nette\DI\Compiler $compiler
     */
    $configurator->onCompile[] = function($configurator, $compiler)
    {
        $compiler->addExtension('cli_http', new CliModule\Http\CliHttpRequestExtension());
    };
    ...

CliHttpRequestExtension.php

    namespace CliModule\Http;

    use Nette\DI\CompilerExtension;
    use Nette\DI\Container;
    use Nette\DI\ContainerBuilder;
    use Nette\DI\ServiceDefinition;
    use Nette\Http\Request;
    use Nette\Http\UrlScript;
    use Nette\Utils\Strings;

    class CliHttpRequestExtension extends CompilerExtension
    {
        public function loadConfiguration()
        {
            $container = $this->getContainerBuilder();
            $config    = $this->getConfig();


            /** @var ContainerBuilder $container */
            if ($container->parameters['consoleMode']) {
                // ** variant 1 - create own Request object
                $url = new UrlScript();
                $url->setHost($container->parameters['domain']);
                $url->setScheme('http');

                $arguments = [
                    'url'           => $url,
                    'query'         => NULL,
                    'post'          => NULL,
                    'files'         => NULL,
                    'cookies'       => NULL,
                    'headers'       => NULL,
                    'method'        => NULL,
                    'remoteAddress' => NULL,
                    'remoteHost'    => NULL
                ];

                $container->removeDefinition('httpRequest');
                $container->addDefinition('httpRequest')->setClass('\Nette\Http\Request', $arguments);

                // ** varianta 2- use own RequestFactory **
    //            $container->removeDefinition('httpRequestFactory');
    //            $container->addDefinition('httpRequestFactory')
    //                ->setClass('CliModule\Http\CliRequestFactory')
    //                ->addSetup('setDomain', array($container->parameters['domain']));
    //
    //            $container->getDefinition('httpRequest')
    //                ->setFactory('@CliModule\\CliRequestFactory::createHttpRequest');
    //
            }
        }

    }

CliRequestFactory.php - direct copy of Request factory except added part after line 94

     ...
     public function createHttpRequest()
    {
        // DETECTS URI, base path and script path of the request.
        $url = new UrlScript;
        $url->scheme = !empty($_SERVER['HTTPS']) && strcasecmp($_SERVER['HTTPS'], 'off') ? 'https' : 'http';
        $url->user = isset($_SERVER['PHP_AUTH_USER']) ? $_SERVER['PHP_AUTH_USER'] : '';
        $url->password = isset($_SERVER['PHP_AUTH_PW']) ? $_SERVER['PHP_AUTH_PW'] : '';

        // host & port
        if ((isset($_SERVER[$tmp = 'HTTP_HOST']) || isset($_SERVER[$tmp = 'SERVER_NAME']))
            && preg_match('#^([a-z0-9_.-]+|\[[a-f0-9:]+\])(:\d+)?\z#i', $_SERVER[$tmp], $pair)
        ) {
            $url->host = strtolower($pair[1]);
            if (isset($pair[2])) {
                $url->port = (int) substr($pair[2], 1);
            } elseif (isset($_SERVER['SERVER_PORT'])) {
                $url->port = (int) $_SERVER['SERVER_PORT'];
            }
        }

        // Default vyplnenie Host-u
        if (!$url->host)
        {
            $url->host = $this->domain;
        }
        ...
@xificurk
Copy link

I agree that this problem should be addressed, but I think this is a wrong place.
If I'm not mistaken, the only thing needed is to make sure that router always gets reasonable $refUrl, so there should be some kind of default fallback value in CLI mode.

@nakashu
Copy link
Author

nakashu commented Jan 28, 2015

Ahh .. somehow missed that it can be fixed with that ... damn :)
Anyway forgot to mention, that problem only exists if you try to generate absolute Url.

@JanTvrdik
Copy link
Contributor

Why don't you use nextras/link-factory?

@xificurk
Copy link

@JanTvrdik Because it has all the same problems in CLI as any other Nette router.

@f3l1x
Copy link
Member

f3l1x commented Jun 16, 2017

I think we can close it, because it's easy to setup http.url address in CLI directly or via extension.

$builder->getDefinition('http.request')
              ->setClass(Request::class, [new Statement(UrlScript::class, ['https://my.url'])]);

Right @dg @JanTvrdik @xificurk ?

@Majkl578
Copy link
Contributor

@f3l1x No, HTTP request should not be a service... See the discussion on the public Slack.

@f3l1x
Copy link
Member

f3l1x commented Jun 16, 2017

@Majkl578 Well, it's pretty cool, but in nette 2.4 and lower there is httpRequest as service, so I've posted a hint how to do it.

This issue is not about to make httpService non-service.

@fprochazka
Copy link
Contributor

I disagree with closing this. There should be a configurable default for cli. http://symfony.com/doc/current/console/request_context.html

@f3l1x
Copy link
Member

f3l1x commented Jun 18, 2017

I see your point @fprochazka. Should I prepare a PR?

http:
    url: nette.org 
http:
    cli: nette.org 

Or something like that?

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

6 participants