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.

Sign up for GitHub

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

Jump to bottom

PSR-15 compatibility #253

Closed
wants to merge 3 commits into from
Closed

PSR-15 compatibility #253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bd2c758
PSR-15 compatibility
bwaidelich Mar 27, 2024
cc83db4
Cosmetic tweaks to satisfy PHPStan checks
bwaidelich Mar 27, 2024
681845f
more test coverage
bwaidelich Mar 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
"phpstan/phpstan": "1.10.47 || 1.4.10",
"phpunit/phpunit": "^9.6 || ^7.5",
"psr/container": "^2 || ^1",
"react/promise-timer": "^1.10"
"react/promise-timer": "^1.10",
"psr/http-server-middleware": "^1"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should(tm) only be a requirement for the tests to run, so I added it as dev-dependency

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems about right, we'd rather add a new dev-dependencies instead of a new requirement, as this only affects development and not all users 👍

},
"autoload": {
"psr-4": {
Expand Down
36 changes: 36 additions & 0 deletions docs/api/middleware.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,42 @@ This is commonly used for cache handling and response body transformations (comp
> [Generator-based coroutines](../async/coroutines.md) anymore.
> See [fibers](../async/fibers.md) for more details.

## PSR-15 middleware

Middleware handlers can also be classes implementing the `MiddlewareInterface` from the [PSR-15](https://www.php-fig.org/psr/psr-15/) recommendation:

```php title="src/PsrMiddleware.php"
<?php

namespace Acme\Todo;

use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\MiddlewareInterface;
use Psr\Http\Server\RequestHandlerInterface;
use React\Http\Message\Response;

class PsrMiddleware implements MiddlewareInterface
{
public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
{
return Response::plaintext('Hello from PSR-15');
}
}
```

```php title="public/index.php"
<?php

use Acme\Todo\PsrMiddleware;

// …

$app->get('/user', new PsrMiddleware());
```

This is especially useful in order to integrate one of the many existing PSR-15 implementations as global middleware.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I acknowledge that this is the bare minimum documentation, feel free to amend *g

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a solid start, let's focus on the API first and once we have a feeling what this should look like, we can adjust the documentation accordingly 👍

## Global middleware

Additionally, you can also add middleware to the [`App`](app.md) object itself
Expand Down
42 changes: 23 additions & 19 deletions src/App.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@
namespace FrameworkX;

use FrameworkX\Io\MiddlewareHandler;
use FrameworkX\Io\PsrMiddlewareAdapter;
use FrameworkX\Io\ReactiveHandler;
use FrameworkX\Io\RedirectHandler;
use FrameworkX\Io\RouteHandler;
use FrameworkX\Io\SapiHandler;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\MiddlewareInterface;
use React\Http\Message\Response;
use React\Promise\Deferred;
use React\Promise\PromiseInterface;
Expand Down Expand Up @@ -37,7 +39,7 @@ class App
* $app = new App($middleware1, $middleware2);
* ```
*
* @param callable|class-string ...$middleware
* @param callable|MiddlewareInterface|class-string ...$middleware
*/
public function __construct(...$middleware)
{
Expand Down Expand Up @@ -73,6 +75,8 @@ public function __construct(...$middleware)
if (!$handlers) {
$needsErrorHandler = $needsAccessLog = $container;
}
} elseif ($handler instanceof MiddlewareInterface) {
$handlers[] = new PsrMiddlewareAdapter($handler);
} elseif (!\is_callable($handler)) {
$handlers[] = $container->callable($handler);
} else {
Expand Down Expand Up @@ -118,8 +122,8 @@ public function __construct(...$middleware)

/**
* @param string $route
* @param callable|class-string $handler
* @param callable|class-string ...$handlers
* @param callable|MiddlewareInterface|class-string $handler
* @param callable|MiddlewareInterface|class-string ...$handlers
*/
public function get(string $route, $handler, ...$handlers): void
{
Expand All @@ -128,8 +132,8 @@ public function get(string $route, $handler, ...$handlers): void

/**
* @param string $route
* @param callable|class-string $handler
* @param callable|class-string ...$handlers
* @param callable|MiddlewareInterface|class-string $handler
* @param callable|MiddlewareInterface|class-string ...$handlers
*/
public function head(string $route, $handler, ...$handlers): void
{
Expand All @@ -138,8 +142,8 @@ public function head(string $route, $handler, ...$handlers): void

/**
* @param string $route
* @param callable|class-string $handler
* @param callable|class-string ...$handlers
* @param callable|MiddlewareInterface|class-string $handler
* @param callable|MiddlewareInterface|class-string ...$handlers
*/
public function post(string $route, $handler, ...$handlers): void
{
Expand All @@ -148,8 +152,8 @@ public function post(string $route, $handler, ...$handlers): void

/**
* @param string $route
* @param callable|class-string $handler
* @param callable|class-string ...$handlers
* @param callable|MiddlewareInterface|class-string $handler
* @param callable|MiddlewareInterface|class-string ...$handlers
*/
public function put(string $route, $handler, ...$handlers): void
{
Expand All @@ -158,8 +162,8 @@ public function put(string $route, $handler, ...$handlers): void

/**
* @param string $route
* @param callable|class-string $handler
* @param callable|class-string ...$handlers
* @param callable|MiddlewareInterface|class-string $handler
* @param callable|MiddlewareInterface|class-string ...$handlers
*/
public function patch(string $route, $handler, ...$handlers): void
{
Expand All @@ -168,8 +172,8 @@ public function patch(string $route, $handler, ...$handlers): void

/**
* @param string $route
* @param callable|class-string $handler
* @param callable|class-string ...$handlers
* @param callable|MiddlewareInterface|class-string $handler
* @param callable|MiddlewareInterface|class-string ...$handlers
*/
public function delete(string $route, $handler, ...$handlers): void
{
Expand All @@ -178,8 +182,8 @@ public function delete(string $route, $handler, ...$handlers): void

/**
* @param string $route
* @param callable|class-string $handler
* @param callable|class-string ...$handlers
* @param callable|MiddlewareInterface|class-string $handler
* @param callable|MiddlewareInterface|class-string ...$handlers
*/
public function options(string $route, $handler, ...$handlers): void
{
Expand All @@ -193,8 +197,8 @@ public function options(string $route, $handler, ...$handlers): void

/**
* @param string $route
* @param callable|class-string $handler
* @param callable|class-string ...$handlers
* @param callable|MiddlewareInterface|class-string $handler
* @param callable|MiddlewareInterface|class-string ...$handlers
*/
public function any(string $route, $handler, ...$handlers): void
{
Expand All @@ -205,8 +209,8 @@ public function any(string $route, $handler, ...$handlers): void
*
* @param string[] $methods
* @param string $route
* @param callable|class-string $handler
* @param callable|class-string ...$handlers
* @param callable|MiddlewareInterface|class-string $handler
* @param callable|MiddlewareInterface|class-string ...$handlers
*/
public function map(array $methods, string $route, $handler, ...$handlers): void
{
Expand Down
5 changes: 5 additions & 0 deletions src/Container.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@

namespace FrameworkX;

use FrameworkX\Io\PsrMiddlewareAdapter;
use Psr\Container\ContainerInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\MiddlewareInterface;

/**
* @final
Expand Down Expand Up @@ -83,6 +85,9 @@ public function callable(string $class): callable
$e
);
}
if ($handler instanceof MiddlewareInterface) {
$handler = new PsrMiddlewareAdapter($handler);
}

// Check `$handler` references a class name that is callable, i.e. has an `__invoke()` method.
// This initial version is intentionally limited to checking the method name only.
Expand Down
10 changes: 8 additions & 2 deletions src/Io/MiddlewareHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace FrameworkX\Io;

use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\MiddlewareInterface;

/**
* @internal
Expand All @@ -12,12 +13,17 @@ class MiddlewareHandler
/** @var list<callable> $handlers */
private $handlers;

/** @param list<callable> $handlers */
/** @param list<callable|MiddlewareInterface> $handlers */
public function __construct(array $handlers)
{
assert(count($handlers) >= 2);

$this->handlers = $handlers;
$this->handlers = array_map(function ($handler) {
if ($handler instanceof MiddlewareInterface) {
return new PsrMiddlewareAdapter($handler);
}
return $handler;
}, $handlers);
}

/** @return mixed */
Expand Down
34 changes: 34 additions & 0 deletions src/Io/PsrAwaitRequestHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php

namespace FrameworkX\Io;

use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\RequestHandlerInterface;
use React\Http\Message\Response;
use function React\Async\await;
use function React\Promise\resolve;

/**
* @internal
*/
class PsrAwaitRequestHandler implements RequestHandlerInterface
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is mostly "stolen" from https://github.com/friends-of-reactphp/http-middleware-psr15-adapter and that needs attribution – I'm not sure how you usually handle this? As comment in the class or README?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally we add a comment to the specific class, something like:

# @copyright Copyright (c) 2024 Simon Frings, taken from https://github.com/clue/framework-x/pull/3 with permission

Nearly all of these projects are published under the MIT license, same goes for https://github.com/friends-of-reactphp/http-middleware-psr15-adapter

{

/**
* @var callable|null
*/
private $next;

public function __construct(callable $next = null) {
$this->next = $next;
}

public function handle(ServerRequestInterface $request): ResponseInterface
{
if ($this->next === null) {
return new Response();
}
Comment on lines +29 to +31
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit weird. But there are at least two occurrences of a handler invocation without the next parameter in the framework:

and this was the easiest solution I could think of to avoid these from exploding :)

return await(resolve(($this->next)($request)));
}
}
33 changes: 33 additions & 0 deletions src/Io/PsrMiddlewareAdapter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

namespace FrameworkX\Io;

use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\MiddlewareInterface as PsrMiddlewareInterface;
use React\Promise\PromiseInterface;
use function React\Async\async;

Check failure on line 9 in src/Io/PsrMiddlewareAdapter.php

View workflow job for this annotation

GitHub Actions / PHPStan (PHP 7.4) 

Used function React\Async\async not found.

Check failure on line 9 in src/Io/PsrMiddlewareAdapter.php

View workflow job for this annotation

GitHub Actions / PHPStan (PHP 7.3) 

Used function React\Async\async not found.

Check failure on line 9 in src/Io/PsrMiddlewareAdapter.php

View workflow job for this annotation

GitHub Actions / PHPStan (PHP 7.1) 

Used function React\Async\async not found.

/**
* @internal
*/
class PsrMiddlewareAdapter
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should also be attributed to https://github.com/friends-of-reactphp/http-middleware-psr15-adapter

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as #253 (comment)

{

/**
* @var PsrMiddlewareInterface
*/
private $middleware;

public function __construct(PsrMiddlewareInterface $middleware) {
$this->middleware = $middleware;
}

/** @return PromiseInterface<ResponseInterface> */
public function __invoke(ServerRequestInterface $request, callable $next = null): PromiseInterface
{
return async(function () use ($request, $next) {

Check failure on line 29 in src/Io/PsrMiddlewareAdapter.php

View workflow job for this annotation

GitHub Actions / PHPStan (PHP 7.4) 

Function React\Async\async not found.

Check failure on line 29 in src/Io/PsrMiddlewareAdapter.php

View workflow job for this annotation

GitHub Actions / PHPStan (PHP 7.3) 

Function React\Async\async not found.

Check failure on line 29 in src/Io/PsrMiddlewareAdapter.php

View workflow job for this annotation

GitHub Actions / PHPStan (PHP 7.1) 

Function React\Async\async not found.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

async is not available for PHP < 7.4 but I wonder if that's really needed here.
Couldn't this be:

$deferred = new Deferred();
$deferred->resolve($this->middleware->process($request, new PsrAwaitRequestHandler($next)));
return $deferred->promise();

or even just

return $this->middleware->process($request, new PsrAwaitRequestHandler($next));

since PSR-15 middlewares will most probably be blocking anyways!?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think it's needed and like you said, with our commitment to PHP 7.4 support we can't really use it here for now (except building a workaround for PHP 7.4, but then we can just use the same workaround for all versions).

return $this->middleware->process($request, new PsrAwaitRequestHandler($next));
})($request, $next);
}
}
7 changes: 5 additions & 2 deletions src/Io/RouteHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use FrameworkX\ErrorHandler;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\MiddlewareInterface;
use React\Promise\PromiseInterface;

/**
Expand Down Expand Up @@ -40,8 +41,8 @@ public function __construct(Container $container = null)
/**
* @param string[] $methods
* @param string $route
* @param callable|class-string $handler
* @param callable|class-string ...$handlers
* @param callable|MiddlewareInterface|class-string $handler
* @param callable|MiddlewareInterface|class-string ...$handlers
*/
public function map(array $methods, string $route, $handler, ...$handlers): void
{
Expand All @@ -60,6 +61,8 @@ public function map(array $methods, string $route, $handler, ...$handlers): void
unset($handlers[$i]);
} elseif ($handler instanceof AccessLogHandler || $handler === AccessLogHandler::class) {
throw new \TypeError('AccessLogHandler may currently only be passed as a global middleware');
} elseif ($handler instanceof MiddlewareInterface) {
$handlers[$i] = new PsrMiddlewareAdapter($handler);
} elseif (!\is_callable($handler)) {
$handlers[$i] = $container->callable($handler);
}
Expand Down
Loading
Loading