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

Middleware attribute #93

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 24 additions & 0 deletions src/Attribute/Middleware.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php

declare(strict_types=1);

namespace Yiisoft\Middleware\Dispatcher\Attribute;

use Attribute;

#[Attribute(Attribute::TARGET_FUNCTION | Attribute::TARGET_METHOD | Attribute::IS_REPEATABLE)]
final class Middleware
{
private readonly mixed $definition;

public function __construct(
array|callable|string $definition
) {
$this->definition = $definition;
}

public function getDefinition(): mixed
{
return $this->definition;
}
}
3 changes: 3 additions & 0 deletions src/MiddlewareDispatcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ public function __construct(
private MiddlewareFactory $middlewareFactory,
private ?EventDispatcherInterface $eventDispatcher = null
) {
if ($eventDispatcher !== null && !$middlewareFactory->hasEventDispatcher()) {
Copy link
Member

@vjik vjik Feb 22, 2024

Choose a reason for hiding this comment

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

Suggest major refactoring...

1) Mark MiddlewareFactory as internal
2) Create MiddlewareFactory in constructor and replace private MiddlewareFactory $middlewareFactory, to:

private ContainerInterface $container,
private ?ParametersResolverInterface $parametersResolver = null
  1. Pass currently instance of dispatcher to MiddlewareFactory constructor.

It's allow use MiddlewareDispatcher::withMiddlewares() into MiddlewareFactory.

Copy link
Member Author

Choose a reason for hiding this comment

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

There are no reasons to make MiddlewareFactory internal

Copy link
Member

Choose a reason for hiding this comment

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

Agree with internal. But 2 and 3 is actual. MiddlewareFactory methods withEventDispatcher() and hasEventDispatcher() looks strange.

$this->middlewareFactory = $this->middlewareFactory->withEventDispatcher($eventDispatcher);
}
}

/**
Expand Down
120 changes: 51 additions & 69 deletions src/MiddlewareFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,18 @@

use Closure;
use Psr\Container\ContainerInterface;
use Psr\EventDispatcher\EventDispatcherInterface;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\MiddlewareInterface;
use Psr\Http\Server\RequestHandlerInterface;
use ReflectionClass;
use ReflectionFunction;
use ReflectionParameter;
use Yiisoft\Definitions\ArrayDefinition;
use Yiisoft\Definitions\Exception\InvalidConfigException;
use Yiisoft\Definitions\Helpers\DefinitionValidator;
use Yiisoft\Injector\Injector;
use Yiisoft\Middleware\Dispatcher\Attribute\Middleware;

use function in_array;
use function is_array;
Expand All @@ -29,15 +30,29 @@
*/
final class MiddlewareFactory
{
private ?EventDispatcherInterface $eventDispatcher = null;

/**
* @param ContainerInterface $container Container to use for resolving definitions.
*/
public function __construct(
private readonly ContainerInterface $container,
private readonly ?ParametersResolverInterface $parametersResolver = null
private ContainerInterface $container,
private ?ParametersResolverInterface $parametersResolver = null
Comment on lines +39 to +40
Copy link
Member

Choose a reason for hiding this comment

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

Why drop "readonly"?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was written before the rector changes

) {
}

public function withEventDispatcher(?EventDispatcherInterface $eventDispatcher): self
{
$new = clone $this;
$new->eventDispatcher = $eventDispatcher;
return $new;
}

public function hasEventDispatcher(): bool
{
return $this->eventDispatcher !== null;
}

/**
* @param array|callable|string $middlewareDefinition Middleware definition in one of the following formats:
*
Expand Down Expand Up @@ -167,30 +182,42 @@ private function wrapCallable(array|callable $callable): MiddlewareInterface
return $this->createCallableWrapper($callable);
}

return $this->createActionWrapper($callable[0], $callable[1]);
return $this->createCallableWrapper([$this->container->get($callable[0]), $callable[1]]);
}

private function createCallableWrapper(callable $callback): MiddlewareInterface
{
return new class ($callback, $this->container, $this->parametersResolver) implements MiddlewareInterface {
return new class (
$callback,
$this->container,
$this,
$this->eventDispatcher,
$this->parametersResolver,
) implements MiddlewareInterface {
/** @var callable */
private $callback;
/**
* @var ReflectionParameter[]
* @psalm-var array<string,ReflectionParameter>
*/
private array $callableParameters = [];
public array $middlewares = [];

public function __construct(
callable $callback,
private readonly ContainerInterface $container,
private readonly MiddlewareFactory $middlewareFactory,
private readonly ?EventDispatcherInterface $eventDispatcher,
private readonly ?ParametersResolverInterface $parametersResolver
) {
$this->callback = $callback;
$callback = Closure::fromCallable($callback);

$callableParameters = (new ReflectionFunction($callback))->getParameters();
foreach ($callableParameters as $parameter) {
$reflectionFunction = new ReflectionFunction(Closure::fromCallable($callback));

foreach ($reflectionFunction->getAttributes(Middleware::class) as $attribute) {
$this->middlewares[] = $attribute->newInstance()->getDefinition();
}
foreach ($reflectionFunction->getParameters() as $parameter) {
$this->callableParameters[$parameter->getName()] = $parameter;
}
}
Expand All @@ -207,8 +234,16 @@ public function process(
);
}

/** @var MiddlewareInterface|mixed|ResponseInterface $response */
$response = (new Injector($this->container))->invoke($this->callback, $parameters);
if ($this->middlewares !== []) {
$middlewares = [...$this->middlewares, fn(): mixed => ($this->callback)()];
$middlewareDispatcher = new MiddlewareDispatcher($this->middlewareFactory, $this->eventDispatcher);
/** @psalm-suppress MixedArgumentTypeCoercion */
$middlewareDispatcher = $middlewareDispatcher->withMiddlewares($middlewares);
$response = $middlewareDispatcher->dispatch($request, $handler);
} else {
/** @var MiddlewareInterface|mixed|ResponseInterface $response */
$response = (new Injector($this->container))->invoke($this->callback, $parameters);
}
if ($response instanceof ResponseInterface) {
return $response;
}
Expand All @@ -221,66 +256,13 @@ public function process(

public function __debugInfo(): array
{
return ['callback' => $this->callback];
}
};
}

/**
* @param class-string $class
* @param non-empty-string $method
*/
private function createActionWrapper(string $class, string $method): MiddlewareInterface
{
return new class ($this->container, $this->parametersResolver, $class, $method) implements MiddlewareInterface {
/**
* @var ReflectionParameter[]
* @psalm-var array<string,ReflectionParameter>
*/
private array $actionParameters = [];

public function __construct(
private readonly ContainerInterface $container,
private readonly ?ParametersResolverInterface $parametersResolver,
/** @var class-string */
private readonly string $class,
/** @var non-empty-string */
private readonly string $method
) {
$actionParameters = (new ReflectionClass($this->class))->getMethod($this->method)->getParameters();
foreach ($actionParameters as $parameter) {
$this->actionParameters[$parameter->getName()] = $parameter;
if (is_array($this->callback)
&& isset($this->callback[0], $this->callback[1])
&& is_object($this->callback[0])
) {
return ['callback' => [$this->callback[0]::class, $this->callback[1]]];
}
}

public function process(
ServerRequestInterface $request,
RequestHandlerInterface $handler
): ResponseInterface {
/** @var mixed $controller */
$controller = $this->container->get($this->class);
$parameters = [$request, $handler];
if ($this->parametersResolver !== null) {
$parameters = array_merge(
$parameters,
$this->parametersResolver->resolve($this->actionParameters, $request)
);
}

/** @var mixed|ResponseInterface $response */
$response = (new Injector($this->container))->invoke([$controller, $this->method], $parameters);
if ($response instanceof ResponseInterface) {
return $response;
}

throw new InvalidMiddlewareDefinitionException([$this->class, $this->method]);
}

public function __debugInfo()
{
return [
'callback' => [$this->class, $this->method],
];
return ['callback' => $this->callback];
}
};
}
Expand Down
2 changes: 1 addition & 1 deletion src/MiddlewareStack.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ final class MiddlewareStack implements RequestHandlerInterface
*/
public function __construct(
private readonly array $middlewares,
private RequestHandlerInterface $fallbackHandler,
private readonly RequestHandlerInterface $fallbackHandler,
private readonly ?EventDispatcherInterface $eventDispatcher = null
) {
if ($middlewares === []) {
Expand Down
Loading
Loading