Skip to content

Commit

Permalink
!!!TASK: Bootstrap instantiates request handlers internally
Browse files Browse the repository at this point in the history
It seems unnecessarily complex to have code register a request handler
as build instance. Instead it is now registered by class name.
The Flow Bootstrap will then create instances while determining the
best matching request handler. To facilitate this being lazy the
RequestHandlerInterface::getPriority method is now declared static so
that we can decide beforehand if the priority is higher than any
previously found best match.
Depending on possibly request handlers and specifically in all preselected
cases this all reduces the amount of created objects and possibly the
amount of canHandle calls. A microptimization but which affects all
requests. Primary concern was to make registration easier and internalize
instantiation.

As small simplification the
`Bootstrap->setPreselectedRequestHandlerClassName` will also register
the given class name as possible request handler removing one step.

This is breaking if you have your own request handler implementation.
The `getPrioritty` method needs to declare an `int` return type and
must be static. Additionally the registration happens via
`Bootstrap->registerRequestHandlerClassName(YourRequestHandler::class)`.
  • Loading branch information
kitsunet committed Jun 22, 2024
1 parent c7625ec commit de760a5
Show file tree
Hide file tree
Showing 10 changed files with 69 additions and 45 deletions.
2 changes: 1 addition & 1 deletion Neos.Flow/Classes/Cli/CommandRequestHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public function canHandleRequest(): bool
*
* @return integer The priority of the request handler.
*/
public function getPriority(): int
public static function getPriority(): int
{
return 100;
}
Expand Down
2 changes: 1 addition & 1 deletion Neos.Flow/Classes/Cli/SlaveRequestHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public function canHandleRequest(): bool
*
* @return integer The priority of the request handler.
*/
public function getPriority(): int
public static function getPriority(): int
{
return 200;
}
Expand Down
90 changes: 58 additions & 32 deletions Neos.Flow/Classes/Core/Bootstrap.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,28 +31,22 @@
*/
class Bootstrap
{
const RUNLEVEL_COMPILETIME = 'Compiletime';
const RUNLEVEL_RUNTIME = 'Runtime';
public const RUNLEVEL_COMPILETIME = 'Compiletime';
public const RUNLEVEL_RUNTIME = 'Runtime';

/**
* @var ApplicationContext
*/
protected $context;
protected ApplicationContext $context;

/**
* @var array<class-string<RequestHandlerInterface>, RequestHandlerInterface>
* @var array<int, class-string>
*/
protected $requestHandlers = [];
private array $requestHandlerClassNames = [];

/**
* @var class-string<RequestHandlerInterface>
* @var class-string<RequestHandlerInterface>|null
*/
protected $preselectedRequestHandlerClassName;
protected string|null $preselectedRequestHandlerClassName = null;

/**
* @var RequestHandlerInterface
*/
protected $activeRequestHandler;
protected RequestHandlerInterface $activeRequestHandler;

/**
* The same instance like $objectManager, but static, for use in the proxy classes.
Expand Down Expand Up @@ -144,28 +138,33 @@ public function getContext(): ApplicationContext
}

/**
* Registers a request handler which can possibly handle a request.
* All registered request handlers will be queried if they can handle a request
* when the bootstrap's run() method is called.
* Registers a request handler which can possibly handle a request.
* All registered request handlers will be queried if they can handle a request
* when the bootstrap's run() method is called.
*
* @param RequestHandlerInterface $requestHandler
* @param class-string<RequestHandlerInterface> $className
* @return void
* @api
*/
public function registerRequestHandler(RequestHandlerInterface $requestHandler)
public function registerRequestHandlerClassName(string $className): void
{
$this->requestHandlers[get_class($requestHandler)] = $requestHandler;
// No checks at this point in time as we might not be able to autolaod the class yet.
$this->requestHandlerClassNames[] = $className;
}

/**
* Preselects a specific request handler. If such a request handler exists,
* it will be used if it can handle the request – regardless of the priority
* of this or other request handlers.
*
* This will also register the classname as a possible request handler.
*
* @param class-string<RequestHandlerInterface> $className
*/
public function setPreselectedRequestHandlerClassName(string $className)
public function setPreselectedRequestHandlerClassName(string $className): void
{
if (!in_array($className, $this->requestHandlerClassNames, true)) {
$this->registerRequestHandlerClassName($className);
}
$this->preselectedRequestHandlerClassName = $className;
}

Expand All @@ -191,7 +190,7 @@ public function getActiveRequestHandler(): RequestHandlerInterface
* @param RequestHandlerInterface $requestHandler
* @return void
*/
public function setActiveRequestHandler(RequestHandlerInterface $requestHandler)
public function setActiveRequestHandler(RequestHandlerInterface $requestHandler): void
{
$this->activeRequestHandler = $requestHandler;
}
Expand Down Expand Up @@ -404,27 +403,54 @@ public function getObjectManager(): ObjectManagerInterface
*/
protected function resolveRequestHandler(): RequestHandlerInterface
{
if ($this->preselectedRequestHandlerClassName !== null && isset($this->requestHandlers[$this->preselectedRequestHandlerClassName])) {
$requestHandler = $this->requestHandlers[$this->preselectedRequestHandlerClassName];
if ($this->preselectedRequestHandlerClassName !== null) {
$requestHandler = $this->getRequestHandlerInstance($this->preselectedRequestHandlerClassName);
if ($requestHandler->canHandleRequest()) {
return $requestHandler;
}
}

foreach ($this->requestHandlers as $requestHandler) {
/** @var RequestHandlerInterface|null $bestMatchingRequestHandler */
$bestMatchingRequestHandler = null;
$bestMatchingPriority = -9999;
foreach($this->requestHandlerClassNames as $requestHandlerClassName) {
if ($bestMatchingPriority > $requestHandlerClassName::getPriority()) {
continue;
}

$requestHandler = $this->getRequestHandlerInstance($requestHandlerClassName);
if ($requestHandler->canHandleRequest() > 0) {
$priority = $requestHandler->getPriority();
if (isset($suitableRequestHandlers[$priority])) {
$priority = $requestHandler::getPriority();
if ($bestMatchingPriority === $priority) {
throw new FlowException('More than one request handler with the same priority can handle the request, but only one handler may be active at a time!', 1176475350);
}
$suitableRequestHandlers[$priority] = $requestHandler;
$bestMatchingPriority = $priority;
$bestMatchingRequestHandler = $requestHandler;
}
}
if (empty($suitableRequestHandlers)) {

if (is_null($bestMatchingRequestHandler)) {
throw new FlowException('No suitable request handler could be found for the current request. This is most likely a setup-problem, so please check your composer.json and/or try removing Configuration/PackageStates.php', 1464882543);
}
ksort($suitableRequestHandlers);
return array_pop($suitableRequestHandlers);
return $bestMatchingRequestHandler;
}

/**
* @param class-string $className
* @return RequestHandlerInterface
*/
private function getRequestHandlerInstance(string $className): RequestHandlerInterface
{
if (!in_array($className, $this->requestHandlerClassNames, true)) {
throw new FlowException(sprintf('A request handler "%s" was never registered', $className), 1719060769);
}

$requestHandler = new $className($this);
if (!$requestHandler instanceof RequestHandlerInterface) {
throw new FlowException(sprintf('The class "%s" registered as request handler must implement the RequestHandlerInterface interface.', $className), 1719053345);
}

return $requestHandler;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion Neos.Flow/Classes/Core/RequestHandlerInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,5 @@ public function canHandleRequest();
* @return integer The priority of the request handler
* @api
*/
public function getPriority();
public static function getPriority(): int;
}
2 changes: 1 addition & 1 deletion Neos.Flow/Classes/Http/RequestHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public function canHandleRequest()
* @return integer The priority of the request handler.
* @api
*/
public function getPriority()
public static function getPriority(): int
{
return 100;
}
Expand Down
8 changes: 4 additions & 4 deletions Neos.Flow/Classes/Package.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,17 @@ public function boot(Core\Bootstrap $bootstrap)
$context = $bootstrap->getContext();

if (PHP_SAPI === 'cli') {
$bootstrap->registerRequestHandler(new Cli\SlaveRequestHandler($bootstrap));
$bootstrap->registerRequestHandler(new Cli\CommandRequestHandler($bootstrap));
$bootstrap->registerRequestHandlerClassName(Cli\SlaveRequestHandler::class);
$bootstrap->registerRequestHandlerClassName(Cli\CommandRequestHandler::class);
} else {
$bootstrap->registerRequestHandler(new Http\RequestHandler($bootstrap));
$bootstrap->registerRequestHandlerClassName(Http\RequestHandler::class);
}

if ($context->isTesting()) {
// TODO: This is technically not necessary as we can register the request handler in the functional bootstrap
// A future commit will remove this aftter BuildEssentials is adapted
/** @phpstan-ignore-next-line composer doesnt autoload this class */
$bootstrap->registerRequestHandler(new Tests\FunctionalTestRequestHandler($bootstrap));
$bootstrap->registerRequestHandlerClassName(Tests\FunctionalTestRequestHandler::class);
}

$bootstrap->registerCompiletimeCommand('neos.flow:core:*');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public function canHandleRequest(): bool
return true;
}

public function getPriority(): int
public static function getPriority(): int
{
return 0;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public function canHandleRequest(): bool
*
* @return integer The priority of the request handler.
*/
public function getPriority(): int
public static function getPriority(): int
{
return 0;
}
Expand Down
2 changes: 1 addition & 1 deletion Neos.Flow/Tests/FunctionalTestRequestHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public function canHandleRequest()
*
* @return integer The priority of the request handler.
*/
public function getPriority()
public static function getPriority(): int
{
return 0;
}
Expand Down
2 changes: 0 additions & 2 deletions Neos.Flow/Tests/PhpBench/Core/BootstrapBench.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ public function benchBootstrapConstruct(): void
public function benchBootstrapRunWithoutBootSequence(): void
{
$flowBootstrap = new Bootstrap(TestableFramework::getApplicationContext());
$flowBootstrap->registerRequestHandler(new EmptyRequestHandler());
$flowBootstrap->setPreselectedRequestHandlerClassName(EmptyRequestHandler::class);
$flowBootstrap->run();
}
Expand All @@ -57,7 +56,6 @@ public function benchBootstrapRunWithoutBootSequence(): void
public function benchBootstrapRunWithRuntimeBootSequence(): void
{
$flowBootstrap = new Bootstrap(TestableFramework::getApplicationContext());
$flowBootstrap->registerRequestHandler(new RuntimeSequenceInvokingRequestHandler($flowBootstrap));
$flowBootstrap->setPreselectedRequestHandlerClassName(RuntimeSequenceInvokingRequestHandler::class);
$flowBootstrap->run();
}
Expand Down

0 comments on commit de760a5

Please sign in to comment.