Skip to content

Commit f61d0e3

Browse files
clueclue
committed
Improve FiberHandler to only wrap in promise when using await() 
1 parent 40c6d6e commit f61d0e3

File tree

4 files changed

+113
-216
lines changed

4 files changed

+113
-216
lines changed

‎src/FiberHandler.php

Lines changed: 26 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
use Psr\Http\Message\ResponseInterface;
66
use Psr\Http\Message\ServerRequestInterface;
77
use React\Promise\Deferred;
8-
use React\Promise\Promise;
98
use React\Promise\PromiseInterface;
109

1110
/**
@@ -22,50 +21,40 @@
2221
class FiberHandler
2322
{
2423
/**
25-
* @return PromiseInterface<ResponseInterface,void>
26-
* Returns a promise that is fulfilled with a `ResponseInterface` on
27-
* success. This method never throws or resolves a rejected promise.
28-
* If the request can not be routed or the handler fails, it will be
29-
* turned into a valid error response before returning.
24+
* @return ResponseInterface|PromiseInterface<ResponseInterface,void>|\Generator
25+
* Returns a `ResponseInterface` from the next request handler in the
26+
* chain. If the next request handler returns immediately, this method
27+
* will return immediately. If the next request handler suspends the
28+
* fiber (see `await()`), this method will return a `PromiseInterface`
29+
* that is fulfilled with a `ResponseInterface` when the fiber is
30+
* terminated successfully. If the next request handler returns a
31+
* promise, this method will return a promise that follows its
32+
* resolution. If the next request handler returns a Generator-based
33+
* coroutine, this method returns a `Generator`. This method never
34+
* throws or resolves a rejected promise. If the handler fails, it will
35+
* be turned into a valid error response before returning.
3036
* @throws void
3137
*/
32-
public function __invoke(ServerRequestInterface $request, callable $next): PromiseInterface
38+
public function __invoke(ServerRequestInterface $request, callable $next): mixed
3339
{
34-
return new Promise(function ($resolve) use ($next, $request) {
35-
$fiber = new \Fiber(function () use ($resolve, $next, $request) {
36-
$response = $next($request);
37-
if ($response instanceof \Generator) {
38-
$response = $this->coroutine($response);
39-
}
40+
$deferred = null;
41+
$fiber = new \Fiber(function () use ($request, $next, &$deferred) {
42+
$response = $next($request);
43+
assert($response instanceof ResponseInterface || $response instanceof PromiseInterface || $response instanceof \Generator);
4044

41-
$resolve($response);
42-
});
43-
$fiber->start();
44-
});
45-
}
46-
47-
private function coroutine(\Generator $generator): PromiseInterface
48-
{
49-
$next = null;
50-
$deferred = new Deferred();
51-
$next = function () use ($generator, &$next, $deferred) {
52-
if (!$generator->valid()) {
53-
$deferred->resolve($generator->getReturn());
54-
return;
45+
if ($deferred !== null) {
46+
$deferred->resolve($response);
5547
}
5648

57-
$promise = $generator->current();
58-
$promise->then(function ($value) use ($generator, $next) {
59-
$generator->send($value);
60-
$next();
61-
}, function ($reason) use ($generator, $next) {
62-
$generator->throw($reason);
63-
$next();
64-
});
65-
};
49+
return $response;
50+
});
6651

67-
$next();
52+
$fiber->start();
53+
if ($fiber->isTerminated()) {
54+
return $fiber->getReturn();
55+
}
6856

57+
$deferred = new Deferred();
6958
return $deferred->promise();
7059
}
7160
}

‎tests/AppMiddlewareTest.php

Lines changed: 12 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ public function testMapMethodWithMiddlewareAddsGivenMethodsOnRouter()
171171

172172
public function testMiddlewareCallsNextReturnsResponseFromRouter()
173173
{
174-
$app = $this->createAppWithoutFibersOrLogger();
174+
$app = $this->createAppWithoutLogger();
175175

176176
$middleware = function (ServerRequestInterface $request, callable $next) {
177177
return $next($request);
@@ -205,7 +205,7 @@ public function testMiddlewareCallsNextReturnsResponseFromRouter()
205205

206206
public function testMiddlewareCallsNextWithModifiedRequestReturnsResponseFromRouter()
207207
{
208-
$app = $this->createAppWithoutFibersOrLogger();
208+
$app = $this->createAppWithoutLogger();
209209

210210
$middleware = function (ServerRequestInterface $request, callable $next) {
211211
return $next($request->withAttribute('name', 'Alice'));
@@ -239,7 +239,7 @@ public function testMiddlewareCallsNextWithModifiedRequestReturnsResponseFromRou
239239

240240
public function testMiddlewareCallsNextReturnsResponseModifiedInMiddlewareFromRouter()
241241
{
242-
$app = $this->createAppWithoutFibersOrLogger();
242+
$app = $this->createAppWithoutLogger();
243243

244244
$middleware = function (ServerRequestInterface $request, callable $next) {
245245
$response = $next($request);
@@ -366,7 +366,7 @@ public function testMiddlewareCallsNextReturnsCoroutineResponseModifiedInMiddlew
366366

367367
public function testMiddlewareCallsNextWhichThrowsExceptionReturnsInternalServerErrorResponse()
368368
{
369-
$app = $this->createAppWithoutFibersOrLogger();
369+
$app = $this->createAppWithoutLogger();
370370

371371
$middleware = function (ServerRequestInterface $request, callable $next) {
372372
return $next($request);
@@ -397,7 +397,7 @@ public function testMiddlewareCallsNextWhichThrowsExceptionReturnsInternalServer
397397

398398
public function testMiddlewareWhichThrowsExceptionReturnsInternalServerErrorResponse()
399399
{
400-
$app = $this->createAppWithoutFibersOrLogger();
400+
$app = $this->createAppWithoutLogger();
401401

402402
$line = __LINE__ + 2;
403403
$middleware = function (ServerRequestInterface $request, callable $next) {
@@ -426,7 +426,7 @@ public function testMiddlewareWhichThrowsExceptionReturnsInternalServerErrorResp
426426

427427
public function testGlobalMiddlewareCallsNextReturnsResponseFromController()
428428
{
429-
$app = $this->createAppWithoutFibersOrLogger(function (ServerRequestInterface $request, callable $next) {
429+
$app = $this->createAppWithoutLogger(function (ServerRequestInterface $request, callable $next) {
430430
return $next($request);
431431
});
432432

@@ -463,7 +463,7 @@ public function __invoke(ServerRequestInterface $request, callable $next)
463463
}
464464
};
465465

466-
$app = $this->createAppWithoutFibersOrLogger($middleware);
466+
$app = $this->createAppWithoutLogger($middleware);
467467

468468
$app->get('/', function () {
469469
return new Response(
@@ -498,7 +498,7 @@ public function __invoke(ServerRequestInterface $request, callable $next)
498498
}
499499
};
500500

501-
$app = $this->createAppWithoutFibersOrLogger(get_class($middleware));
501+
$app = $this->createAppWithoutLogger(get_class($middleware));
502502

503503
$app->get('/', function () {
504504
return new Response(
@@ -534,7 +534,7 @@ public function __invoke(ServerRequestInterface $request, callable $next)
534534
}
535535
};
536536

537-
$app = $this->createAppWithoutFibersOrLogger(get_class($middleware));
537+
$app = $this->createAppWithoutLogger(get_class($middleware));
538538

539539
$app->get('/', get_class($middleware), function (ServerRequestInterface $request) {
540540
return new Response(
@@ -562,7 +562,7 @@ public function __invoke(ServerRequestInterface $request, callable $next)
562562

563563
public function testGlobalMiddlewareCallsNextWithModifiedRequestWillBeUsedForRouting()
564564
{
565-
$app = $this->createAppWithoutFibersOrLogger(function (ServerRequestInterface $request, callable $next) {
565+
$app = $this->createAppWithoutLogger(function (ServerRequestInterface $request, callable $next) {
566566
return $next($request->withUri($request->getUri()->withPath('/users')));
567567
});
568568

@@ -592,7 +592,7 @@ public function testGlobalMiddlewareCallsNextWithModifiedRequestWillBeUsedForRou
592592

593593
public function testGlobalMiddlewareCallsNextReturnsModifiedResponseWhenModifyingResponseFromRouter()
594594
{
595-
$app = $this->createAppWithoutFibersOrLogger(function (ServerRequestInterface $request, callable $next) {
595+
$app = $this->createAppWithoutLogger(function (ServerRequestInterface $request, callable $next) {
596596
$response = $next($request);
597597
assert($response instanceof ResponseInterface);
598598

@@ -623,7 +623,7 @@ public function testGlobalMiddlewareCallsNextReturnsModifiedResponseWhenModifyin
623623

624624
public function testGlobalMiddlewareReturnsResponseWithoutCallingNextReturnsResponseWithoutCallingRouter()
625625
{
626-
$app = $this->createAppWithoutFibersOrLogger(function () {
626+
$app = $this->createAppWithoutLogger(function () {
627627
return new Response(
628628
200,
629629
[
@@ -807,30 +807,4 @@ private function createAppWithoutLogger(...$middleware): App
807807

808808
return $app;
809809
}
810-
811-
/** @param callable|class-string ...$middleware */
812-
private function createAppWithoutFibersOrLogger(...$middleware): App
813-
{
814-
$app = new App(...$middleware);
815-
816-
$ref = new \ReflectionProperty($app, 'handler');
817-
$ref->setAccessible(true);
818-
$middleware = $ref->getValue($app);
819-
820-
$ref = new \ReflectionProperty($middleware, 'handlers');
821-
$ref->setAccessible(true);
822-
$handlers = $ref->getValue($middleware);
823-
824-
if (PHP_VERSION_ID >= 80100) {
825-
$first = array_shift($handlers);
826-
$this->assertInstanceOf(FiberHandler::class, $first);
827-
}
828-
829-
$first = array_shift($handlers);
830-
$this->assertInstanceOf(AccessLogHandler::class, $first);
831-
832-
$ref->setValue($middleware, $handlers);
833-
834-
return $app;
835-
}
836810
}

0 commit comments

Comments
 (0)