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

Request: added withUrl() #69

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JanTvrdik
Copy link
Contributor

It's often useful (e.g. in tests) to modify URL inside httpRequest. Doing this is really annoying because Request has lot of constructor parameters. The naming (with*) is based on PSR-7.

What do you think about this idea? Tests are currently missing.

@enumag
Copy link
Contributor

enumag commented Aug 9, 2015

Can you show an example of a test where you need this? I never needed anything like it, instead it would be really benefitial for my tests if Request would not be a service (and Response as well for that matter).

Also I certainly wouldn't like this method in the IRequest interface.

@fprochazka
Copy link
Contributor

@JanTvrdik I like the idea, doing this every time is tedious.

@JanTvrdik
Copy link
Contributor Author

instead it would be really benefitial for my tests if Request would not be a service

Feel free to join the relevant discussion.


Can you show an example of a test where you need this?

Sorry, I have a really bad memory. I found the code I have written (6 days ago) and the withUrl method would not help in that case. The reason why I brought it up today is because I was creating a draft for a router where the method would be helpful.

class DevRouter implements Application\IRouter
{
    // *.signaly.cz -> *.signaly.l  
    // *.signaly.cz -> localhost/signaly/www/*
    public function __construct($fromMask, $toMask, Application\IRouter $innerRouter)
    {
        $this->fromMask = $fromMask;
        $this->toMask = $toMask;
        $this->innerRouter = $innerRouter;
    }

    public function match(Nette\Http\IRequest $httpRequest)
    {
        $fixedUrl = $this->transformIn($httpRequest->url);
        $fixedHttpRequest = $httpRequest->withUrl($fixedUrl); // <-- THIS IS THE LINE
        return $this->innerRouter->match($fixedHttpRequest);
    }

    public function constructUrl(Request $appRequest, Nette\Http\Url $refUrl)
    {
        $url = $this->innerRouter->constructUrl($appRequest, $refUrl);
        $fixedUrl = $this->transformOut($url);
        return $fixedUrl;
    }
}

@enumag
Copy link
Contributor

enumag commented Aug 9, 2015

In my opinion you should just write a static helper method cloneRequestWithUrl($request, $url) outside of this class.

@dg dg force-pushed the master branch 7 times, most recently from 1dc599c to f35d20f Compare November 5, 2015 00:41
@dg dg force-pushed the master branch 2 times, most recently from 7981e33 to 3fb64f5 Compare July 21, 2016 12:07
@dg dg force-pushed the master branch 3 times, most recently from b9f06ea to 4f6d6c9 Compare December 20, 2016 14:30
@dg dg force-pushed the master branch 3 times, most recently from 2ff2f22 to 40175d7 Compare January 9, 2017 19:55
@dg dg force-pushed the master branch 5 times, most recently from b031216 to 9979093 Compare January 23, 2017 09:20
@dg dg force-pushed the master branch 4 times, most recently from 2a0c595 to 4ca7b03 Compare January 26, 2023 22:47
@dg dg force-pushed the master branch 6 times, most recently from da24b94 to 540335c Compare March 20, 2023 13:43
@dg dg force-pushed the master branch 2 times, most recently from e7c7e2d to bf945f3 Compare August 5, 2023 19:08
@dg dg force-pushed the master branch 3 times, most recently from 9a14e6e to a20fb8f Compare November 14, 2023 18:31
@dg dg force-pushed the master branch 5 times, most recently from 55488bd to 2042d2e Compare December 11, 2023 13:01
@dg dg force-pushed the master branch 2 times, most recently from 4960652 to 5e67add Compare May 2, 2024 10:56
@dg dg force-pushed the master branch 5 times, most recently from 689f4ae to 33aae19 Compare November 5, 2024 06:45
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

Successfully merging this pull request may close these issues.

5 participants