Skip to content

Commit

Permalink
refactor(middleware): improvements on error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
Guikingone committed Aug 18, 2022
1 parent 2f918a2 commit 83d6301
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 15 deletions.
6 changes: 3 additions & 3 deletions src/Middleware/AbstractMiddlewareStack.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,16 @@ protected function runMiddleware(MiddlewareRegistryInterface $middlewareList, Cl

$this->executedMiddleware->attach(object: $middleware);
});
} catch (Throwable $throwable) {
} catch (Throwable) {
$middlewareList->next();
} finally {
foreach ($requiredMiddlewareList as $singleRequiredMiddlewareList) {
if ($this->executedMiddleware->contains(object: $singleRequiredMiddlewareList)) {
continue;
}

$func($singleRequiredMiddlewareList);
}

throw $throwable;
}
}

Expand Down
9 changes: 9 additions & 0 deletions src/Middleware/MiddlewareRegistry.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use function is_array;
use function iterator_to_array;
use function uasort;
use function next;

use const ARRAY_FILTER_USE_BOTH;

Expand Down Expand Up @@ -73,6 +74,14 @@ public function toArray(): array
return $this->middlewareList;
}

/**
* {@inheritdoc}
*/
public function next(): void
{
next(array: $this->middlewareList);
}

/**
* {@inheritdoc}
*/
Expand Down
5 changes: 5 additions & 0 deletions src/Middleware/MiddlewareRegistryInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ public function walk(Closure $func): MiddlewareRegistryInterface;
*/
public function uasort(Closure $func): MiddlewareRegistryInterface;

/**
* Change the internal cursor of the middleware list, mainly used by {@see AbstractMiddlewareStack::runMiddleware()} to iterate over the list when an exception is thrown.
*/
public function next(): void;

/**
* @return array<int, PostExecutionMiddlewareInterface|PreExecutionMiddlewareInterface|PreSchedulingMiddlewareInterface|PostSchedulingMiddlewareInterface|RequiredMiddlewareInterface|OrderedMiddlewareInterface>
*/
Expand Down
6 changes: 3 additions & 3 deletions src/Worker/Worker.php
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,10 @@ protected function handleTask(TaskInterface $task, TaskListInterface $taskList):

$this->taskExecutionTracker->endTracking(task: $task);
$task->setExecutionEndTime(dateTimeImmutable: new DateTimeImmutable());
$task->setLastExecution(dateTimeImmutable: new DateTimeImmutable());
$this->defineTaskExecutionState(task: $task, output: $output);

$this->middlewareStack->runPostExecutionMiddleware(task: $task, worker: $this);
$this->eventDispatcher->dispatch(new TaskExecutedEvent(task: $task, output: $output));
$this->configuration->setLastExecutedTask(lastExecutedTask: $task);

Expand All @@ -304,12 +307,9 @@ protected function handleTask(TaskInterface $task, TaskListInterface $taskList):
$this->getFailedTasks()->add(task: $failedTask);
$this->eventDispatcher->dispatch(event: new TaskFailedEvent(task: $failedTask));
} finally {
$task->setLastExecution(dateTimeImmutable: new DateTimeImmutable());

$this->configuration->setCurrentlyExecutedTask(task: null);
$this->configuration->run(isRunning: false);

$this->middlewareStack->runPostExecutionMiddleware(task: $task, worker: $this);
$this->eventDispatcher->dispatch(event: new WorkerRunningEvent(worker: $this, isIdle: true));
}
}
Expand Down
7 changes: 4 additions & 3 deletions tests/Middleware/WorkerMiddlewareStackTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use SchedulerBundle\Middleware\PostExecutionMiddlewareInterface;
use SchedulerBundle\Middleware\PreExecutionMiddlewareInterface;
use SchedulerBundle\Middleware\WorkerMiddlewareStack;
use SchedulerBundle\Task\NullTask;
use SchedulerBundle\Task\TaskInterface;
use SchedulerBundle\Worker\WorkerInterface;
use Throwable;
Expand All @@ -23,7 +24,7 @@ final class WorkerMiddlewareStackTest extends TestCase
*/
public function testStackCanRunEmptyPreMiddlewareList(): void
{
$task = $this->createMock(TaskInterface::class);
$task = new NullTask(name: 'foo');

$middleware = $this->createMock(PostExecutionMiddlewareInterface::class);
$middleware->expects(self::never())->method('postExecute')->with($task);
Expand All @@ -40,7 +41,7 @@ public function testStackCanRunEmptyPreMiddlewareList(): void
*/
public function testStackCanRunPreMiddlewareList(): void
{
$task = $this->createMock(TaskInterface::class);
$task = new NullTask(name: 'foo');

$middleware = $this->createMock(PreExecutionMiddlewareInterface::class);
$middleware->expects(self::once())->method('preExecute')->with($task);
Expand All @@ -62,7 +63,7 @@ public function testStackCanRunPreMiddlewareList(): void
public function testStackCanRunEmptyPostMiddlewareList(): void
{
$worker = $this->createMock(WorkerInterface::class);
$task = $this->createMock(TaskInterface::class);
$task = new NullTask(name: 'foo');

$middleware = $this->createMock(PreExecutionMiddlewareInterface::class);
$middleware->expects(self::never())->method('preExecute')->with($task);
Expand Down
10 changes: 5 additions & 5 deletions tests/Worker/ExecutionPolicy/ExecutionPolicyRegistryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ final class ExecutionPolicyRegistryTest extends TestCase
{
public function testRegistryCanCount(): void
{
$registry = new ExecutionPolicyRegistry([]);
$registry = new ExecutionPolicyRegistry(policies: []);

self::assertCount(0, $registry);
}

public function testRegistryCannotReturnInvalidPolicy(): void
{
$registry = new ExecutionPolicyRegistry([
$registry = new ExecutionPolicyRegistry(policies: [
new DefaultPolicy(),
]);
self::assertCount(1, $registry);
Expand All @@ -36,7 +36,7 @@ public function testRegistryCannotReturnInvalidPolicy(): void

public function testRegistryCannotReturnMultiplePolicies(): void
{
$registry = new ExecutionPolicyRegistry([
$registry = new ExecutionPolicyRegistry(policies: [
new DefaultPolicy(),
new DefaultPolicy(),
]);
Expand All @@ -50,12 +50,12 @@ public function testRegistryCannotReturnMultiplePolicies(): void

public function testRegistryCanReturnPolicy(): void
{
$registry = new ExecutionPolicyRegistry([
$registry = new ExecutionPolicyRegistry(policies: [
new DefaultPolicy(),
]);
self::assertCount(1, $registry);

$policy = $registry->find('default');
$policy = $registry->find(policy: 'default');
self::assertInstanceOf(DefaultPolicy::class, $policy);
}
}
2 changes: 1 addition & 1 deletion tests/Worker/WorkerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1367,7 +1367,7 @@ public function testWorkerCanRetrieveTasksLazily(): void
]), $eventDispatcher, $lockFactory, $logger);

$configuration = WorkerConfiguration::create();
$configuration->mustRetrieveTasksLazily(true);
$configuration->mustRetrieveTasksLazily(mustRetrieveTasksLazily: true);

$worker->execute($configuration);

Expand Down

0 comments on commit 83d6301

Please sign in to comment.