Fix setting url to http.request

[jfilla]

Service http.request can be any class implementing interface IRequest. If you have custom IRequest implementation and console.url config is set, DI container cache looks like this:

public function createServiceHttp__request(): Flowgate\FunctionalTests\RequestMock
{
	return new Nette\Http\Request(new Nette\Http\UrlScript('http://localhost'));
}

, which causes type error. It should look like this

public function createServiceHttp__request(): Flowgate\FunctionalTests\RequestMock
{
	return new Flowgate\FunctionalTests\RequestMock;
}

[mabar]

I believe this works in your case in tests, but it breaks behavior of ConsoleExtension for anyone who overrides Request for standard usage. Imho what should be done here is to override RequestFactory by ConsoleExtension instead so you are still able to override Request

[jfilla]

I tried what i think is standard usage (is it?). Creating class

<?php


namespace App;


use Nette\Http\Request;
use Nette\Http\UrlScript;

class CustomRequest extends Request
{

    public function __construct()
    {
        parent::__construct(new UrlScript('http://whatever'));
    }

}

registering it as http.request

services:
    http.request: App\CustomRequest

in cli command i tried to call

$this->container->getByType(IRequest::class);

which fails with error

TypeError: Return value of Container_b2518d818a::createServiceHttp__request() must be an instance of App\CustomRequest, instance of Nette\Http\Request returned

because of this inside DI container

public function createServiceHttp__request(): App\CustomRequest
{
	return new Nette\Http\Request(new Nette\Http\UrlScript('http://localhost'));
}

I think with console.url enabled is imposible to override http.request anyway. I will think about it and figure out better solution.. and fix tests of course.

[f3l1x]

What about this.

$httpDef->setFactory($httpDef->getFactory(), [new Statement(UrlScript::class, [$config->url])]);

You must preserver arguments order/position.

[mabar]

I am worried it would still break due to possibly different constructor

[jfilla]

I was thinking about that too, but i didnt like the idea of preserving argument positions. Original idea was cleaner for me.

[jfilla]

What about now? I created console requestFactory, which is used only when default requestFactory is used. Exception is thown on custom requestFactory implementation and console.url set. Console.url in combination with custom requestFactory is in my opinion useless, url should be set by custom requestFactory itself.

[mabar]

Looks way better. I will give it some final touches later today before release. Thanks

[f3l1x]

Hey @jfilla, I want to assure you we'll handle this issue. We didn't have a time yet.

[f3l1x]

Thank you @jfilla ❤️