Skip to content

Commit 712892d

Browse files
committed
refactor(core): improvements
1 parent 08c6140 commit 712892d

File tree

7 files changed

+183
-23
lines changed

7 files changed

+183
-23
lines changed

src/DependencyInjection/SchedulerBundleExtension.php

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@
9494
use SchedulerBundle\Worker\WorkerInterface;
9595
use Symfony\Bundle\FrameworkBundle\Console\Application;
9696
use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument;
97+
use Symfony\Component\DependencyInjection\ChildDefinition;
9798
use Symfony\Component\DependencyInjection\ContainerBuilder;
9899
use Symfony\Component\DependencyInjection\ContainerInterface;
99100
use Symfony\Component\DependencyInjection\Extension\Extension;
@@ -366,18 +367,21 @@ private function registerExpressionFactoryAndPolicies(ContainerBuilder $containe
366367
$container->setAlias(BuilderInterface::class, ExpressionBuilder::class);
367368

368369
$container->register(CronExpressionBuilder::class, CronExpressionBuilder::class)
370+
->addTag('scheduler.expression_builder')
369371
->addTag('container.preload', [
370372
'class' => CronExpressionBuilder::class,
371373
])
372374
;
373375

374376
$container->register(ComputedExpressionBuilder::class, ComputedExpressionBuilder::class)
377+
->addTag('scheduler.expression_builder')
375378
->addTag('container.preload', [
376379
'class' => ComputedExpressionBuilder::class,
377380
])
378381
;
379382

380383
$container->register(FluentExpressionBuilder::class, FluentExpressionBuilder::class)
384+
->addTag('scheduler.expression_builder')
381385
->addTag('container.preload', [
382386
'class' => FluentExpressionBuilder::class,
383387
])
@@ -481,43 +485,50 @@ private function registerBuilders(ContainerBuilder $container): void
481485
])
482486
;
483487

484-
$container->register(CommandBuilder::class, CommandBuilder::class)
488+
$commandBuilderDefinition = new ChildDefinition(AbstractTaskBuilder::class);
489+
$commandBuilderDefinition->setClass(CommandBuilder::class);
490+
$container->setDefinition(CommandBuilder::class, $commandBuilderDefinition)
485491
->setPublic(false)
486492
->addTag('scheduler.task_builder')
487493
->addTag('container.preload', [
488494
'class' => CommandBuilder::class,
489495
])
490496
;
491497

492-
$container->register(HttpBuilder::class, HttpBuilder::class)
498+
$httpBuilderDefinition = new ChildDefinition(AbstractTaskBuilder::class);
499+
$httpBuilderDefinition->setClass(HttpBuilder::class);
500+
$container->setDefinition(HttpBuilder::class, $httpBuilderDefinition)
493501
->setPublic(false)
494502
->addTag('scheduler.task_builder')
495503
->addTag('container.preload', [
496504
'class' => HttpBuilder::class,
497505
])
498506
;
499507

500-
$container->register(NullBuilder::class, NullBuilder::class)
508+
$httpBuilderDefinition = new ChildDefinition(AbstractTaskBuilder::class);
509+
$httpBuilderDefinition->setClass(NullBuilder::class);
510+
$container->setDefinition(NullBuilder::class, $httpBuilderDefinition)
501511
->setPublic(false)
502512
->addTag('scheduler.task_builder')
503513
->addTag('container.preload', [
504514
'class' => NullBuilder::class,
505515
])
506516
;
507517

508-
$container->register(ShellBuilder::class, ShellBuilder::class)
518+
$httpBuilderDefinition = new ChildDefinition(AbstractTaskBuilder::class);
519+
$httpBuilderDefinition->setClass(ShellBuilder::class);
520+
$container->setDefinition(ShellBuilder::class, $httpBuilderDefinition)
509521
->setPublic(false)
510522
->addTag('scheduler.task_builder')
511523
->addTag('container.preload', [
512524
'class' => ShellBuilder::class,
513525
])
514526
;
515527

516-
$container->register(ChainedBuilder::class, ChainedBuilder::class)
517-
->setArguments([
518-
new Reference(BuilderInterface::class, ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE),
519-
new TaggedIteratorArgument('scheduler.task_builder'),
520-
])
528+
$httpBuilderDefinition = new ChildDefinition(AbstractTaskBuilder::class);
529+
$httpBuilderDefinition->setClass(ChainedBuilder::class);
530+
$container->setDefinition(ChainedBuilder::class, $httpBuilderDefinition)
531+
->setArgument(1, new TaggedIteratorArgument('scheduler.task_builder'))
521532
->setPublic(false)
522533
->addTag('scheduler.task_builder')
523534
->addTag('container.preload', [

src/EventListener/WorkerLifecycleSubscriber.php

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use SchedulerBundle\Event\WorkerRunningEvent;
1111
use SchedulerBundle\Event\WorkerStartedEvent;
1212
use SchedulerBundle\Event\WorkerStoppedEvent;
13+
use SchedulerBundle\Task\TaskInterface;
1314
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
1415

1516
/**
@@ -30,7 +31,7 @@ public function onWorkerRestarted(WorkerRestartedEvent $event): void
3031

3132
$this->logger->info('The worker has been restarted', [
3233
'failedTasks' => $worker->getFailedTasks()->count(),
33-
'lastExecutedTask' => $worker->getLastExecutedTask()->getName(),
34+
'lastExecutedTask' => $worker->getLastExecutedTask() instanceof TaskInterface ? $worker->getLastExecutedTask()->getName() : null,
3435
]);
3536
}
3637

@@ -40,7 +41,7 @@ public function onWorkerRunning(WorkerRunningEvent $event): void
4041

4142
$this->logger->info('The worker is currently running', [
4243
'failedTasks' => $worker->getFailedTasks()->count(),
43-
'lastExecutedTask' => $worker->getLastExecutedTask()->getName(),
44+
'lastExecutedTask' => $worker->getLastExecutedTask() instanceof TaskInterface ? $worker->getLastExecutedTask()->getName() : null,
4445
'idle' => $event->isIdle(),
4546
]);
4647
}
@@ -51,7 +52,7 @@ public function onWorkerStarted(WorkerStartedEvent $event): void
5152

5253
$this->logger->info('The worker has been started', [
5354
'failedTasks' => $worker->getFailedTasks()->count(),
54-
'lastExecutedTask' => $worker->getLastExecutedTask()->getName(),
55+
'lastExecutedTask' => $worker->getLastExecutedTask() instanceof TaskInterface ? $worker->getLastExecutedTask()->getName() : null,
5556
]);
5657
}
5758

@@ -61,7 +62,7 @@ public function onWorkerStopped(WorkerStoppedEvent $event): void
6162

6263
$this->logger->info('The worker has been stopped', [
6364
'failedTasks' => $worker->getFailedTasks()->count(),
64-
'lastExecutedTask' => $worker->getLastExecutedTask()->getName(),
65+
'lastExecutedTask' => $worker->getLastExecutedTask() instanceof TaskInterface ? $worker->getLastExecutedTask()->getName() : null,
6566
]);
6667
}
6768

src/Middleware/AbstractMiddlewareStack.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
use function array_filter;
99
use function array_replace;
1010
use function array_walk;
11+
use function is_array;
12+
use function iterator_to_array;
1113
use function uasort;
1214

1315
/**
@@ -25,7 +27,7 @@ abstract class AbstractMiddlewareStack implements MiddlewareStackInterface
2527
*/
2628
public function __construct(iterable $stack = [])
2729
{
28-
$this->stack = $stack;
30+
$this->stack = is_array($stack) ? $stack : iterator_to_array($stack, true);
2931
}
3032

3133
/**

src/Serializer/TaskNormalizer.php

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,11 @@ public function normalize($object, string $format = null, array $context = []):
134134
'type' => get_class($innerObject[0]),
135135
],
136136
'tasks' => fn (array $innerObject, ChainedTask $outerObject, string $attributeName, string $format = null, array $context = []): array => array_map(function (TaskInterface $task) use ($format, $context): array {
137-
return $this->normalize($task, $format, $context);
137+
return $this->normalize($task, $format, array_merge($context, [
138+
AbstractNormalizer::IGNORED_ATTRIBUTES => $task instanceof CommandTask ? [] : [
139+
'options',
140+
],
141+
]));
138142
}, $innerObject),
139143
]
140144
];
@@ -143,7 +147,7 @@ public function normalize($object, string $format = null, array $context = []):
143147
'body' => $this->objectNormalizer->normalize($object, $format, array_merge($context, $normalizationCallbacks, $object instanceof CommandTask ? [] : [
144148
AbstractNormalizer::IGNORED_ATTRIBUTES => [
145149
'options',
146-
],
150+
]
147151
])),
148152
self::NORMALIZATION_DISCRIMINATOR => get_class($object),
149153
];
@@ -160,7 +164,7 @@ public function supportsNormalization($data, string $format = null): bool
160164
/**
161165
* {@inheritdoc}
162166
*/
163-
public function denormalize($data, string $type, string $format = null, array $context = [])
167+
public function denormalize($data, string $type, string $format = null, array $context = []): TaskInterface
164168
{
165169
$objectType = $data[self::NORMALIZATION_DISCRIMINATOR];
166170
$body = $data['body'];

tests/DependencyInjection/SchedulerBundleExtensionTest.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,14 +375,17 @@ public function testExpressionFactoryAndPoliciesAreRegistered(): void
375375
self::assertSame(ExpressionBuilder::class, $container->getDefinition(ExpressionBuilder::class)->getTag('container.preload')[0]['class']);
376376

377377
self::assertTrue($container->hasDefinition(CronExpressionBuilder::class));
378+
self::assertTrue($container->getDefinition(CronExpressionBuilder::class)->hasTag('scheduler.expression_builder'));
378379
self::assertTrue($container->getDefinition(CronExpressionBuilder::class)->hasTag('container.preload'));
379380
self::assertSame(CronExpressionBuilder::class, $container->getDefinition(CronExpressionBuilder::class)->getTag('container.preload')[0]['class']);
380381

381382
self::assertTrue($container->hasDefinition(ComputedExpressionBuilder::class));
383+
self::assertTrue($container->getDefinition(CronExpressionBuilder::class)->hasTag('scheduler.expression_builder'));
382384
self::assertTrue($container->getDefinition(ComputedExpressionBuilder::class)->hasTag('container.preload'));
383385
self::assertSame(ComputedExpressionBuilder::class, $container->getDefinition(ComputedExpressionBuilder::class)->getTag('container.preload')[0]['class']);
384386

385387
self::assertTrue($container->hasDefinition(FluentExpressionBuilder::class));
388+
self::assertTrue($container->getDefinition(CronExpressionBuilder::class)->hasTag('scheduler.expression_builder'));
386389
self::assertTrue($container->getDefinition(FluentExpressionBuilder::class)->hasTag('container.preload'));
387390
self::assertSame(FluentExpressionBuilder::class, $container->getDefinition(FluentExpressionBuilder::class)->getTag('container.preload')[0]['class']);
388391

@@ -460,24 +463,32 @@ public function testBuildersAreRegistered(): void
460463

461464
self::assertTrue($container->hasDefinition(CommandBuilder::class));
462465
self::assertFalse($container->getDefinition(CommandBuilder::class)->isPublic());
466+
self::assertCount(1, $container->getDefinition(CommandBuilder::class)->getArguments());
467+
self::assertInstanceOf(Reference::class, $container->getDefinition(CommandBuilder::class)->getArgument(0));
463468
self::assertTrue($container->getDefinition(CommandBuilder::class)->hasTag('scheduler.task_builder'));
464469
self::assertTrue($container->getDefinition(CommandBuilder::class)->hasTag('container.preload'));
465470
self::assertSame(CommandBuilder::class, $container->getDefinition(CommandBuilder::class)->getTag('container.preload')[0]['class']);
466471

467472
self::assertTrue($container->hasDefinition(HttpBuilder::class));
468473
self::assertFalse($container->getDefinition(HttpBuilder::class)->isPublic());
474+
self::assertCount(1, $container->getDefinition(HttpBuilder::class)->getArguments());
475+
self::assertInstanceOf(Reference::class, $container->getDefinition(HttpBuilder::class)->getArgument(0));
469476
self::assertTrue($container->getDefinition(HttpBuilder::class)->hasTag('scheduler.task_builder'));
470477
self::assertTrue($container->getDefinition(HttpBuilder::class)->hasTag('container.preload'));
471478
self::assertSame(HttpBuilder::class, $container->getDefinition(HttpBuilder::class)->getTag('container.preload')[0]['class']);
472479

473480
self::assertTrue($container->hasDefinition(NullBuilder::class));
474481
self::assertFalse($container->getDefinition(NullBuilder::class)->isPublic());
482+
self::assertCount(1, $container->getDefinition(NullBuilder::class)->getArguments());
483+
self::assertInstanceOf(Reference::class, $container->getDefinition(NullBuilder::class)->getArgument(0));
475484
self::assertTrue($container->getDefinition(NullBuilder::class)->hasTag('scheduler.task_builder'));
476485
self::assertTrue($container->getDefinition(NullBuilder::class)->hasTag('container.preload'));
477486
self::assertSame(NullBuilder::class, $container->getDefinition(NullBuilder::class)->getTag('container.preload')[0]['class']);
478487

479488
self::assertTrue($container->hasDefinition(ShellBuilder::class));
480489
self::assertFalse($container->getDefinition(ShellBuilder::class)->isPublic());
490+
self::assertCount(1, $container->getDefinition(ShellBuilder::class)->getArguments());
491+
self::assertInstanceOf(Reference::class, $container->getDefinition(ShellBuilder::class)->getArgument(0));
481492
self::assertTrue($container->getDefinition(ShellBuilder::class)->hasTag('scheduler.task_builder'));
482493
self::assertTrue($container->getDefinition(ShellBuilder::class)->hasTag('container.preload'));
483494
self::assertSame(ShellBuilder::class, $container->getDefinition(ShellBuilder::class)->getTag('container.preload')[0]['class']);

0 commit comments

Comments
 (0)