Skip to content

Commit d838a40

Browse files
dbudbu
authored
Merge pull request #127 from php-http/final-redirect-plugin
Make RedirectPlugin final
2 parents 18e36cc + 8616247 commit d838a40

File tree

2 files changed

+141
-105
lines changed

2 files changed

+141
-105
lines changed

‎spec/Plugin/RedirectPluginSpec.php

Lines changed: 132 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -73,74 +73,75 @@ public function it_redirects_on_302(
7373
$finalPromise->wait()->shouldReturn($finalResponse);
7474
}
7575

76-
public function it_use_storage_on_301(UriInterface $uri, UriInterface $uriRedirect, RequestInterface $request, RequestInterface $modifiedRequest)
77-
{
78-
$this->beAnInstanceOf(RedirectPluginStub::class);
79-
$this->beConstructedWith($uriRedirect, '/original', '301');
80-
81-
$next = function () {
82-
throw new \Exception('Must not be called');
83-
};
84-
85-
$request->getUri()->willReturn($uri);
86-
$uri->__toString()->willReturn('/original');
87-
$request->withUri($uriRedirect)->willReturn($modifiedRequest);
88-
89-
$modifiedRequest->getUri()->willReturn($uriRedirect);
90-
$modifiedRequest->getMethod()->willReturn('GET');
91-
92-
$uriRedirect->__toString()->willReturn('/redirect');
93-
94-
$this->handleRequest($request, $next, PluginStub::first());
95-
}
96-
97-
public function it_stores_a_301(
76+
public function it_use_storage_on_301(
9877
UriInterface $uri,
9978
UriInterface $uriRedirect,
10079
RequestInterface $request,
101-
ResponseInterface $responseRedirect,
10280
RequestInterface $modifiedRequest,
10381
ResponseInterface $finalResponse,
104-
Promise $promise
82+
ResponseInterface $redirectResponse
10583
) {
106-
$this->beAnInstanceOf(RedirectPluginStub::class);
107-
$this->beConstructedWith($uriRedirect, '', '301');
108-
10984
$request->getUri()->willReturn($uri);
110-
$uri->__toString()->willReturn('/301-url');
111-
112-
$responseRedirect->getStatusCode()->willReturn('301');
113-
$responseRedirect->hasHeader('Location')->willReturn(true);
114-
$responseRedirect->getHeaderLine('Location')->willReturn('/redirect');
115-
85+
$uri->__toString()->willReturn('/original');
11686
$uri->withPath('/redirect')->willReturn($uriRedirect);
117-
$uriRedirect->withFragment('')->willReturn($uriRedirect);
11887
$uriRedirect->withQuery('')->willReturn($uriRedirect);
119-
88+
$uriRedirect->withFragment('')->willReturn($uriRedirect);
12089
$request->withUri($uriRedirect)->willReturn($modifiedRequest);
12190

12291
$modifiedRequest->getUri()->willReturn($uriRedirect);
12392
$modifiedRequest->getMethod()->willReturn('GET');
12493

12594
$uriRedirect->__toString()->willReturn('/redirect');
12695

127-
$next = function (RequestInterface $receivedRequest) use ($request, $responseRedirect) {
128-
if (Argument::is($request->getWrappedObject())->scoreArgument($receivedRequest)) {
129-
return new HttpFulfilledPromise($responseRedirect->getWrappedObject());
130-
}
131-
};
96+
$finalResponse->getStatusCode()->willReturn(200);
13297

133-
$first = function (RequestInterface $receivedRequest) use ($modifiedRequest, $promise) {
134-
if (Argument::is($modifiedRequest->getWrappedObject())->scoreArgument($receivedRequest)) {
135-
return $promise->getWrappedObject();
98+
$redirectResponse->getStatusCode()->willReturn(301);
99+
$redirectResponse->hasHeader('Location')->willReturn(true);
100+
$redirectResponse->getHeaderLine('Location')->willReturn('/redirect');
101+
102+
$nextCalled = false;
103+
$next = function (RequestInterface $request) use (&$nextCalled, $finalResponse, $redirectResponse): Promise {
104+
switch ($request->getUri()) {
105+
case '/original':
106+
if ($nextCalled) {
107+
throw new \Exception('Must only be called once');
108+
}
109+
$nextCalled = true;
110+
111+
return new HttpFulfilledPromise($redirectResponse->getWrappedObject());
112+
case '/redirect':
113+
114+
return new HttpFulfilledPromise($finalResponse->getWrappedObject());
115+
default:
116+
throw new \Exception('Test setup error with request uri '.$request->getUri());
136117
}
137118
};
119+
$first = $this->buildFirst($modifiedRequest, $next);
138120

139-
$promise->getState()->willReturn(Promise::FULFILLED);
140-
$promise->wait()->shouldBeCalled()->willReturn($finalResponse);
121+
$this->handleRequest($request, $next, $first);
141122

123+
// rebuild first as this is expected to be called again
124+
$first = $this->buildFirst($modifiedRequest, $next);
125+
// next should not be called again
142126
$this->handleRequest($request, $next, $first);
143-
$this->hasStorage('/301-url')->shouldReturn(true);
127+
}
128+
129+
private function buildFirst(RequestInterface $modifiedRequest, callable $next): callable
130+
{
131+
$redirectPlugin = $this;
132+
$firstCalled = false;
133+
134+
return function (RequestInterface $request) use (&$modifiedRequest, $redirectPlugin, $next, &$firstCalled) {
135+
if ($firstCalled) {
136+
throw new \Exception('Only one restart expected');
137+
}
138+
$firstCalled = true;
139+
if ($modifiedRequest->getWrappedObject() !== $request) {
140+
//throw new \Exception('Redirection failed');
141+
}
142+
143+
return $redirectPlugin->getWrappedObject()->handleRequest($request, $next, $this);
144+
};
144145
}
145146

146147
public function it_replace_full_url(
@@ -359,35 +360,100 @@ public function it_clears_headers(
359360
$this->handleRequest($request, $next, $first);
360361
}
361362

362-
public function it_throws_circular_redirection_exception(UriInterface $uri, UriInterface $uriRedirect, RequestInterface $request, ResponseInterface $responseRedirect, RequestInterface $modifiedRequest)
363-
{
364-
$first = function () {};
363+
/**
364+
* This is the "redirection does not redirect case.
365+
*/
366+
public function it_throws_circular_redirection_exception_on_redirect_that_does_not_change_url(
367+
UriInterface $redirectUri,
368+
RequestInterface $request,
369+
ResponseInterface $redirectResponse
370+
) {
371+
$redirectResponse->getStatusCode()->willReturn(302);
372+
$redirectResponse->hasHeader('Location')->willReturn(true);
373+
$redirectResponse->getHeaderLine('Location')->willReturn('/redirect');
365374

366-
$this->beAnInstanceOf(RedirectPluginStubCircular::class);
367-
$this->beConstructedWith(spl_object_hash((object) $first));
375+
$next = function () use ($redirectResponse): Promise {
376+
return new HttpFulfilledPromise($redirectResponse->getWrappedObject());
377+
};
368378

369-
$request->getUri()->willReturn($uri);
370-
$uri->__toString()->willReturn('/original');
379+
$first = function () {
380+
throw new \Exception('First should never be called');
381+
};
371382

372-
$responseRedirect->getStatusCode()->willReturn('302');
373-
$responseRedirect->hasHeader('Location')->willReturn(true);
374-
$responseRedirect->getHeaderLine('Location')->willReturn('/redirect');
383+
$request->getUri()->willReturn($redirectUri);
384+
$redirectUri->__toString()->willReturn('/redirect');
375385

376-
$uri->withPath('/redirect')->willReturn($uriRedirect);
377-
$uriRedirect->withFragment('')->willReturn($uriRedirect);
378-
$uriRedirect->withQuery('')->willReturn($uriRedirect);
386+
$redirectUri->withPath('/redirect')->willReturn($redirectUri);
387+
$redirectUri->withFragment('')->willReturn($redirectUri);
388+
$redirectUri->withQuery('')->willReturn($redirectUri);
379389

380-
$request->withUri($uriRedirect)->willReturn($modifiedRequest);
381-
$modifiedRequest->getUri()->willReturn($uriRedirect);
382-
$uriRedirect->__toString()->willReturn('/redirect');
383-
$modifiedRequest->getMethod()->willReturn('GET');
390+
$request->withUri($redirectUri)->willReturn($request);
391+
$redirectUri->__toString()->willReturn('/redirect');
392+
$request->getMethod()->willReturn('GET');
384393

385-
$next = function (RequestInterface $receivedRequest) use ($request, $responseRedirect) {
386-
if (Argument::is($request->getWrappedObject())->scoreArgument($receivedRequest)) {
387-
return new HttpFulfilledPromise($responseRedirect->getWrappedObject());
394+
$promise = $this->handleRequest($request, $next, $first);
395+
$promise->shouldReturnAnInstanceOf(HttpRejectedPromise::class);
396+
$promise->shouldThrow(CircularRedirectionException::class)->duringWait();
397+
}
398+
399+
/**
400+
* This is a redirection flipping back and forth between two paths.
401+
*
402+
* There could be a larger loop but the logic in the plugin stays the same with as many redirects as needed.
403+
*/
404+
public function it_throws_circular_redirection_exception_on_alternating_redirect(
405+
UriInterface $uri,
406+
UriInterface $redirectUri,
407+
RequestInterface $request,
408+
ResponseInterface $redirectResponse1,
409+
ResponseInterface $redirectResponse2,
410+
RequestInterface $modifiedRequest
411+
) {
412+
$redirectResponse1->getStatusCode()->willReturn(302);
413+
$redirectResponse1->hasHeader('Location')->willReturn(true);
414+
$redirectResponse1->getHeaderLine('Location')->willReturn('/redirect');
415+
416+
$redirectResponse2->getStatusCode()->willReturn(302);
417+
$redirectResponse2->hasHeader('Location')->willReturn(true);
418+
$redirectResponse2->getHeaderLine('Location')->willReturn('/original');
419+
420+
$next = function (RequestInterface $currentRequest) use ($request, $redirectResponse1, $redirectResponse2): Promise {
421+
return ($currentRequest === $request->getWrappedObject())
422+
? new HttpFulfilledPromise($redirectResponse1->getWrappedObject())
423+
: new HttpFulfilledPromise($redirectResponse2->getWrappedObject())
424+
;
425+
};
426+
427+
$redirectPlugin = $this;
428+
$firstCalled = false;
429+
$first = function (RequestInterface $request) use (&$firstCalled, $redirectPlugin, $next, &$first) {
430+
if ($firstCalled) {
431+
throw new \Exception('only one redirect expected');
388432
}
433+
$firstCalled = true;
434+
435+
return $redirectPlugin->getWrappedObject()->handleRequest($request, $next, $first);
389436
};
390437

438+
$request->getUri()->willReturn($uri);
439+
$uri->__toString()->willReturn('/original');
440+
441+
$modifiedRequest->getUri()->willReturn($redirectUri);
442+
$redirectUri->__toString()->willReturn('/redirect');
443+
444+
$uri->withPath('/redirect')->willReturn($redirectUri);
445+
$redirectUri->withFragment('')->willReturn($redirectUri);
446+
$redirectUri->withQuery('')->willReturn($redirectUri);
447+
448+
$redirectUri->withPath('/original')->willReturn($uri);
449+
$uri->withFragment('')->willReturn($uri);
450+
$uri->withQuery('')->willReturn($uri);
451+
452+
$request->withUri($redirectUri)->willReturn($modifiedRequest);
453+
$request->getMethod()->willReturn('GET');
454+
$modifiedRequest->withUri($uri)->willReturn($request);
455+
$modifiedRequest->getMethod()->willReturn('GET');
456+
391457
$promise = $this->handleRequest($request, $next, $first);
392458
$promise->shouldReturnAnInstanceOf(HttpRejectedPromise::class);
393459
$promise->shouldThrow(CircularRedirectionException::class)->duringWait();
@@ -440,33 +506,3 @@ public function it_redirects_http_to_https(
440506
$finalPromise->wait()->shouldReturn($finalResponse);
441507
}
442508
}
443-
444-
class RedirectPluginStub extends RedirectPlugin
445-
{
446-
public function __construct(UriInterface $uri, $storedUrl, $status, array $config = [])
447-
{
448-
parent::__construct($config);
449-
450-
$this->redirectStorage[$storedUrl] = [
451-
'uri' => $uri,
452-
'status' => $status,
453-
];
454-
}
455-
456-
public function hasStorage($url)
457-
{
458-
return isset($this->redirectStorage[$url]);
459-
}
460-
}
461-
462-
class RedirectPluginStubCircular extends RedirectPlugin
463-
{
464-
public function __construct($chainHash)
465-
{
466-
$this->circularDetection = [
467-
$chainHash => [
468-
'/redirect',
469-
],
470-
];
471-
}
472-
}

‎src/Plugin/RedirectPlugin.php

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,14 @@
1818
*
1919
* @author Joel Wurtz <[email protected]>
2020
*/
21-
class RedirectPlugin implements Plugin
21+
final class RedirectPlugin implements Plugin
2222
{
2323
/**
2424
* Rule on how to redirect, change method for the new request.
2525
*
2626
* @var array
2727
*/
28-
protected $redirectCodes = [
28+
private $redirectCodes = [
2929
300 => [
3030
'switch' => [
3131
'unless' => ['GET', 'HEAD'],
@@ -79,26 +79,26 @@ class RedirectPlugin implements Plugin
7979
* false will ditch all previous headers
8080
* string[] will keep only headers with the specified names
8181
*/
82-
protected $preserveHeader;
82+
private $preserveHeader;
8383

8484
/**
8585
* Store all previous redirect from 301 / 308 status code.
8686
*
8787
* @var array
8888
*/
89-
protected $redirectStorage = [];
89+
private $redirectStorage = [];
9090

9191
/**
9292
* Whether the location header must be directly used for a multiple redirection status code (300).
9393
*
9494
* @var bool
9595
*/
96-
protected $useDefaultForMultiple;
96+
private $useDefaultForMultiple;
9797

9898
/**
9999
* @var array
100100
*/
101-
protected $circularDetection = [];
101+
private $circularDetection = [];
102102

103103
/**
104104
* @param array $config {
@@ -143,7 +143,7 @@ public function handleRequest(RequestInterface $request, callable $next, callabl
143143
return $first($redirectRequest);
144144
}
145145

146-
return $next($request)->then(function (ResponseInterface $response) use ($request, $first) {
146+
return $next($request)->then(function (ResponseInterface $response) use ($request, $first): ResponseInterface {
147147
$statusCode = $response->getStatusCode();
148148

149149
if (!array_key_exists($statusCode, $this->redirectCodes)) {
@@ -171,7 +171,7 @@ public function handleRequest(RequestInterface $request, callable $next, callabl
171171
];
172172
}
173173

174-
// Call redirect request in synchrone
174+
// Call redirect request synchronously
175175
$redirectPromise = $first($redirectRequest);
176176

177177
return $redirectPromise->wait();
@@ -187,7 +187,7 @@ public function handleRequest(RequestInterface $request, callable $next, callabl
187187
*
188188
* @return MessageInterface|RequestInterface
189189
*/
190-
protected function buildRedirectRequest(RequestInterface $request, UriInterface $uri, $statusCode)
190+
private function buildRedirectRequest(RequestInterface $request, UriInterface $uri, $statusCode)
191191
{
192192
$request = $request->withUri($uri);
193193

0 commit comments

Comments
 (0)