From ca22c35aaa9631709ad02fe4770520aad250d715 Mon Sep 17 00:00:00 2001 From: Johannes Wachter Date: Fri, 12 May 2017 07:42:21 +0200 Subject: [PATCH 1/7] improved interface for storage --- src/Entity/TaskExecutionRepository.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Entity/TaskExecutionRepository.php b/src/Entity/TaskExecutionRepository.php index bf897e9..95ca443 100644 --- a/src/Entity/TaskExecutionRepository.php +++ b/src/Entity/TaskExecutionRepository.php @@ -129,14 +129,15 @@ public function findByTaskUuid($taskUuid) /** * {@inheritdoc} */ - public function findScheduled() + public function findNextScheduled(\DateTime $dateTime = null) { $query = $this->createQueryBuilder('e') ->innerJoin('e.task', 't') ->where('e.status = :status') ->andWhere('e.scheduleTime < :date') - ->setParameter('date', new \DateTime()) + ->setParameter('date', $dateTime ?: new \DateTime()) ->setParameter('status', TaskStatus::PLANNED) + ->setMaxResults(1) ->getQuery(); return $query->getResult(); From 375053163d07a293ad3fc6b94e1503eee435aeed Mon Sep 17 00:00:00 2001 From: Johannes Wachter Date: Mon, 15 May 2017 09:14:46 +0200 Subject: [PATCH 2/7] added lock to config --- composer.json | 3 +- src/DependencyInjection/Configuration.php | 50 +++++++++++++++- src/DependencyInjection/TaskExtension.php | 58 ++++++++++++++++++- src/Entity/TaskExecutionRepository.php | 6 +- src/Locking/NullLock.php | 53 +++++++++++++++++ src/Resources/config/locking/null.xml | 8 +++ src/Resources/config/locking/services.xml | 13 +++++ src/Resources/config/locking/storages.xml | 12 ++++ src/Resources/config/scheduler.xml | 1 + .../Entity/TaskExecutionRepositoryTest.php | 8 +-- .../Handler/TaskHandlerFactoryTest.php | 6 +- tests/app/config/config.yml | 6 ++ 12 files changed, 209 insertions(+), 15 deletions(-) create mode 100644 src/Locking/NullLock.php create mode 100644 src/Resources/config/locking/null.xml create mode 100644 src/Resources/config/locking/services.xml create mode 100644 src/Resources/config/locking/storages.xml diff --git a/composer.json b/composer.json index 37c18da..d65508e 100644 --- a/composer.json +++ b/composer.json @@ -11,9 +11,10 @@ ], "require": { "php": "~5.5 || ~7.0", - "php-task/php-task": "dev-develop", + "php-task/php-task": "dev-feature/lock", "symfony/http-kernel": "^2.6 || ^3.0", "symfony/dependency-injection": "^2.6 || ^3.0", + "symfony/expression-language": "^2.6 || ^3.0", "symfony/config": "^2.6 || ^3.0", "symfony/console": "^2.6 || ^3.0", "doctrine/orm": "^2.5" diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 7e8ba92..65b4d70 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -19,6 +19,19 @@ */ class Configuration implements ConfigurationInterface { + /** + * @var string[] + */ + private $lockingStorageAliases = []; + + /** + * @param \string[] $lockingStorageAliases + */ + public function __construct(array $lockingStorageAliases) + { + $this->lockingStorageAliases = $lockingStorageAliases; + } + /** * {@inheritdoc} */ @@ -43,10 +56,29 @@ public function getConfigTreeBuilder() ->arrayNode('run') ->addDefaultsIfNotSet() ->children() - ->enumNode('mode') - ->values(['off', 'listener']) - ->defaultValue('off') + ->enumNode('mode')->values(['off', 'listener'])->defaultValue('off')->end() + ->end() + ->end() + ->arrayNode('locking') + ->canBeEnabled() + ->addDefaultsIfNotSet() + ->children() + ->enumNode('storage') + ->values(array_keys($this->lockingStorageAliases)) + ->defaultValue('file') + ->end() + ->arrayNode('storages') + ->addDefaultsIfNotSet() + ->children() + ->arrayNode('file') + ->addDefaultsIfNotSet() + ->children() + ->scalarNode('directory')->defaultValue(sys_get_temp_dir() . '/task')->end() + ->end() + ->end() + ->end() ->end() + ->enumNode('mode')->values(['off', 'listener'])->defaultValue('off')->end() ->end() ->end() ->arrayNode('system_tasks') @@ -63,4 +95,16 @@ public function getConfigTreeBuilder() return $treeBuilder; } + + /** + * Returns id for given storage-alias. + * + * @param string $alias + * + * @return string + */ + public function getLockingStorageId($alias) + { + return $this->lockingStorageAliases[$alias]; + } } diff --git a/src/DependencyInjection/TaskExtension.php b/src/DependencyInjection/TaskExtension.php index c22ec65..579979f 100644 --- a/src/DependencyInjection/TaskExtension.php +++ b/src/DependencyInjection/TaskExtension.php @@ -12,6 +12,7 @@ namespace Task\TaskBundle\DependencyInjection; use Symfony\Component\Config\FileLocator; +use Symfony\Component\Config\Loader\LoaderInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\Definition; @@ -31,23 +32,29 @@ class TaskExtension extends Extension */ public function load(array $configs, ContainerBuilder $container) { - $configuration = new Configuration(); + $configuration = $this->getConfiguration($configs, $container); $config = $this->processConfiguration($configuration, $configs); $container->setParameter('task.system_tasks', $config['system_tasks']); - $container->setParameter('task.storage', $config['storage']); + + $container->setAlias('task.lock.storage', $configuration->getLockingStorageId($config['locking']['storage'])); + foreach (array_keys($config['locking']['storages']) as $key) { + $container->setParameter('task.lock.storages.' . $key, $config['locking']['storages'][$key]); + } $loader = new Loader\XmlFileLoader($container, new FileLocator(__DIR__ . '/../Resources/config')); $loader->load(sprintf('storage/%s.xml', $config['storage'])); $loader->load('task_event_listener.xml'); $loader->load('scheduler.xml'); $loader->load('command.xml'); + $loader->load('locking/services.xml'); if ($config['run']['mode'] === 'listener') { $loader->load('listener.xml'); } $this->loadDoctrineAdapter($config['adapters']['doctrine'], $container); + $this->loadLockingComponent($config['locking'], $loader); } /** @@ -70,4 +77,51 @@ private function loadDoctrineAdapter(array $config, ContainerBuilder $container) $container->setDefinition('task.adapter.doctrine.execution_listener', $definition); } } + + /** + * Load services for locking component. + * + * @param array $config + * @param LoaderInterface $loader + */ + private function loadLockingComponent(array $config, LoaderInterface $loader) + { + if (!$config['enabled']) { + return $loader->load('locking/null.xml'); + } + + $loader->load('locking/services.xml'); + } + + /** + * Find storage aliases and related ids. + * + * @param ContainerBuilder $container + * + * @return array + */ + private function getLockingStorageAliases(ContainerBuilder $container) + { + $taggedServices = $container->findTaggedServiceIds('task.lock.storage'); + + $result = []; + foreach ($taggedServices as $id => $tags) { + foreach ($tags as $tag) { + $result[$tag['alias']] = $id; + } + } + + return $result; + } + + /** + * {@inheritdoc} + */ + public function getConfiguration(array $config, ContainerBuilder $container) + { + $loader = new Loader\XmlFileLoader($container, new FileLocator(__DIR__ . '/../Resources/config')); + $loader->load('locking/storages.xml'); + + return new Configuration($this->getLockingStorageAliases($container)); + } } diff --git a/src/Entity/TaskExecutionRepository.php b/src/Entity/TaskExecutionRepository.php index 95ca443..61f0049 100644 --- a/src/Entity/TaskExecutionRepository.php +++ b/src/Entity/TaskExecutionRepository.php @@ -140,6 +140,10 @@ public function findNextScheduled(\DateTime $dateTime = null) ->setMaxResults(1) ->getQuery(); - return $query->getResult(); + try { + return $query->getSingleResult(); + } catch (NoResultException $exception) { + return null; + } } } diff --git a/src/Locking/NullLock.php b/src/Locking/NullLock.php new file mode 100644 index 0000000..9fafaf4 --- /dev/null +++ b/src/Locking/NullLock.php @@ -0,0 +1,53 @@ + + + + + + diff --git a/src/Resources/config/locking/services.xml b/src/Resources/config/locking/services.xml new file mode 100644 index 0000000..4544787 --- /dev/null +++ b/src/Resources/config/locking/services.xml @@ -0,0 +1,13 @@ + + + + + + + + + + + diff --git a/src/Resources/config/locking/storages.xml b/src/Resources/config/locking/storages.xml new file mode 100644 index 0000000..99f988b --- /dev/null +++ b/src/Resources/config/locking/storages.xml @@ -0,0 +1,12 @@ + + + + + parameter('task.lock.storages.file')['directory'] + + + + + diff --git a/src/Resources/config/scheduler.xml b/src/Resources/config/scheduler.xml index 983dd6a..aba9b4a 100644 --- a/src/Resources/config/scheduler.xml +++ b/src/Resources/config/scheduler.xml @@ -19,6 +19,7 @@ + diff --git a/tests/Functional/Entity/TaskExecutionRepositoryTest.php b/tests/Functional/Entity/TaskExecutionRepositoryTest.php index f5cb54e..707a7b9 100644 --- a/tests/Functional/Entity/TaskExecutionRepositoryTest.php +++ b/tests/Functional/Entity/TaskExecutionRepositoryTest.php @@ -154,10 +154,8 @@ public function testFindScheduledPast() $execution = $this->save($task, new \DateTime('-1 hour')); - $result = $this->taskExecutionRepository->findScheduled(); - - $this->assertCount(1, $result); - $this->assertEquals($execution->getUuid(), $result[0]->getUuid()); + $result = $this->taskExecutionRepository->findNextScheduled(); + $this->assertEquals($execution->getUuid(), $result->getUuid()); } public function testFindScheduledFuture() @@ -167,7 +165,7 @@ public function testFindScheduledFuture() $this->save($task, new \DateTime('+1 hour')); - $this->assertEmpty($this->taskExecutionRepository->findScheduled()); + $this->assertNull($this->taskExecutionRepository->findNextScheduled()); } /** diff --git a/tests/Functional/Handler/TaskHandlerFactoryTest.php b/tests/Functional/Handler/TaskHandlerFactoryTest.php index 5510b6c..df20342 100644 --- a/tests/Functional/Handler/TaskHandlerFactoryTest.php +++ b/tests/Functional/Handler/TaskHandlerFactoryTest.php @@ -12,7 +12,6 @@ namespace Task\TaskBundle\Tests\Functional\Handler; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; -use Task\Handler\TaskHandlerNotExistsException; use Task\TaskBundle\Handler\TaskHandlerFactory; use Task\TaskBundle\Tests\Functional\TestHandler; @@ -39,10 +38,11 @@ public function testCreate() $this->assertInstanceOf(TestHandler::class, $this->taskHandlerFactory->create(TestHandler::class)); } + /** + * @expectedException \Task\Handler\TaskHandlerNotExistsException + */ public function testCreateNotExists() { - $this->setExpectedException(TaskHandlerNotExistsException::class); - $this->taskHandlerFactory->create(\stdClass::class); } } diff --git a/tests/app/config/config.yml b/tests/app/config/config.yml index c751a29..71793c9 100644 --- a/tests/app/config/config.yml +++ b/tests/app/config/config.yml @@ -1,2 +1,8 @@ parameters: kernel.secret: 12345 + +task: + locking: + storages: + file: + directory: %kernel.cache_dir%/locks From 68f8bc3062dcd15bd1979a6115087ae174e6d5a5 Mon Sep 17 00:00:00 2001 From: Johannes Wachter Date: Mon, 15 May 2017 16:41:31 +0200 Subject: [PATCH 3/7] removed strategy from locking configuration --- src/DependencyInjection/Configuration.php | 4 ++-- src/DependencyInjection/TaskExtension.php | 6 ++++-- src/Locking/NullLock.php | 9 ++++----- src/Resources/config/locking/services.xml | 4 +--- tests/Functional/Command/ScheduleTaskCommandTest.php | 2 +- 5 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 65b4d70..553fd1f 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -67,18 +67,18 @@ public function getConfigTreeBuilder() ->values(array_keys($this->lockingStorageAliases)) ->defaultValue('file') ->end() + ->integerNode('ttl')->defaultValue(600)->end() ->arrayNode('storages') ->addDefaultsIfNotSet() ->children() ->arrayNode('file') ->addDefaultsIfNotSet() ->children() - ->scalarNode('directory')->defaultValue(sys_get_temp_dir() . '/task')->end() + ->scalarNode('directory')->defaultValue('%kernel.cache_dir%/tasks')->end() ->end() ->end() ->end() ->end() - ->enumNode('mode')->values(['off', 'listener'])->defaultValue('off')->end() ->end() ->end() ->arrayNode('system_tasks') diff --git a/src/DependencyInjection/TaskExtension.php b/src/DependencyInjection/TaskExtension.php index 579979f..9452d54 100644 --- a/src/DependencyInjection/TaskExtension.php +++ b/src/DependencyInjection/TaskExtension.php @@ -54,7 +54,7 @@ public function load(array $configs, ContainerBuilder $container) } $this->loadDoctrineAdapter($config['adapters']['doctrine'], $container); - $this->loadLockingComponent($config['locking'], $loader); + $this->loadLockingComponent($config['locking'], $container, $loader); } /** @@ -83,14 +83,16 @@ private function loadDoctrineAdapter(array $config, ContainerBuilder $container) * * @param array $config * @param LoaderInterface $loader + * @param ContainerBuilder $container */ - private function loadLockingComponent(array $config, LoaderInterface $loader) + private function loadLockingComponent(array $config, ContainerBuilder $container, LoaderInterface $loader) { if (!$config['enabled']) { return $loader->load('locking/null.xml'); } $loader->load('locking/services.xml'); + $container->setParameter('task.lock.ttl', $config['ttl']); } /** diff --git a/src/Locking/NullLock.php b/src/Locking/NullLock.php index 9fafaf4..cb0c914 100644 --- a/src/Locking/NullLock.php +++ b/src/Locking/NullLock.php @@ -11,7 +11,6 @@ namespace Task\TaskBundle\Locking; -use Task\Execution\TaskExecutionInterface; use Task\Lock\LockInterface; /** @@ -22,7 +21,7 @@ class NullLock implements LockInterface /** * {@inheritdoc} */ - public function acquire(TaskExecutionInterface $execution) + public function acquire($task) { return true; } @@ -30,7 +29,7 @@ public function acquire(TaskExecutionInterface $execution) /** * {@inheritdoc} */ - public function refresh(TaskExecutionInterface $execution) + public function refresh($task) { return true; } @@ -38,7 +37,7 @@ public function refresh(TaskExecutionInterface $execution) /** * {@inheritdoc} */ - public function release(TaskExecutionInterface $execution) + public function release($task) { return true; } @@ -46,7 +45,7 @@ public function release(TaskExecutionInterface $execution) /** * {@inheritdoc} */ - public function isAcquired(TaskExecutionInterface $execution) + public function isAcquired($task) { return false; } diff --git a/src/Resources/config/locking/services.xml b/src/Resources/config/locking/services.xml index 4544787..25488c8 100644 --- a/src/Resources/config/locking/services.xml +++ b/src/Resources/config/locking/services.xml @@ -3,11 +3,9 @@ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> - - - + %task.lock.ttl% diff --git a/tests/Functional/Command/ScheduleTaskCommandTest.php b/tests/Functional/Command/ScheduleTaskCommandTest.php index 85d1451..1551532 100644 --- a/tests/Functional/Command/ScheduleTaskCommandTest.php +++ b/tests/Functional/Command/ScheduleTaskCommandTest.php @@ -94,7 +94,7 @@ public function testExecuteWithWorkloadAndIntervalAndEndDate() $this->assertEquals(TestHandler::class, $tasks[0]->getHandlerClass()); $this->assertEquals('Test workload 1', $tasks[0]->getWorkload()); $this->assertEquals('0 * * * *', $tasks[0]->getInterval()); - $this->assertEquals($date, $tasks[0]->getLastExecution()); + $this->assertEquals($date, $tasks[0]->getLastExecution(), '', 2); } public function testExecuteWithExecutionDate() From 7dc010cdd5ee18b74566479816a1183f6a95d8d7 Mon Sep 17 00:00:00 2001 From: Johannes Wachter Date: Tue, 16 May 2017 12:59:41 +0200 Subject: [PATCH 4/7] renamed file-lok --- src/Resources/config/locking/storages.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Resources/config/locking/storages.xml b/src/Resources/config/locking/storages.xml index 99f988b..35e2572 100644 --- a/src/Resources/config/locking/storages.xml +++ b/src/Resources/config/locking/storages.xml @@ -3,7 +3,7 @@ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> - + parameter('task.lock.storages.file')['directory'] From 37f1ff2a387d528ce8d949eb33e23c8e2745f62b Mon Sep 17 00:00:00 2001 From: Johannes Wachter Date: Thu, 18 May 2017 08:37:00 +0200 Subject: [PATCH 5/7] added logger to runner --- src/Resources/config/scheduler.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Resources/config/scheduler.xml b/src/Resources/config/scheduler.xml index aba9b4a..9b5c183 100644 --- a/src/Resources/config/scheduler.xml +++ b/src/Resources/config/scheduler.xml @@ -21,6 +21,7 @@ + From 5ef00c2ef62cc2022d426d5e561c50024d451dd4 Mon Sep 17 00:00:00 2001 From: Johannes Wachter Date: Thu, 18 May 2017 09:03:14 +0200 Subject: [PATCH 6/7] added skipped to execution-repository --- src/Entity/TaskExecutionRepository.php | 15 ++++++++++----- .../Entity/TaskExecutionRepositoryTest.php | 10 ++++++++++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/Entity/TaskExecutionRepository.php b/src/Entity/TaskExecutionRepository.php index 61f0049..3bb6db5 100644 --- a/src/Entity/TaskExecutionRepository.php +++ b/src/Entity/TaskExecutionRepository.php @@ -129,19 +129,24 @@ public function findByTaskUuid($taskUuid) /** * {@inheritdoc} */ - public function findNextScheduled(\DateTime $dateTime = null) + public function findNextScheduled(\DateTime $dateTime = null, array $skippedExecutions = []) { - $query = $this->createQueryBuilder('e') + $queryBuilder = $this->createQueryBuilder('e') ->innerJoin('e.task', 't') ->where('e.status = :status') ->andWhere('e.scheduleTime < :date') ->setParameter('date', $dateTime ?: new \DateTime()) ->setParameter('status', TaskStatus::PLANNED) - ->setMaxResults(1) - ->getQuery(); + ->setMaxResults(1); + + $expr = $queryBuilder->expr(); + if (!empty($skippedExecutions)) { + $queryBuilder->andWhere($expr->not($expr->in('e.uuid', ':skipped'))) + ->setParameter('skipped', $skippedExecutions); + } try { - return $query->getSingleResult(); + return $queryBuilder->getQuery()->getSingleResult(); } catch (NoResultException $exception) { return null; } diff --git a/tests/Functional/Entity/TaskExecutionRepositoryTest.php b/tests/Functional/Entity/TaskExecutionRepositoryTest.php index 707a7b9..1716f3a 100644 --- a/tests/Functional/Entity/TaskExecutionRepositoryTest.php +++ b/tests/Functional/Entity/TaskExecutionRepositoryTest.php @@ -168,6 +168,16 @@ public function testFindScheduledFuture() $this->assertNull($this->taskExecutionRepository->findNextScheduled()); } + public function testFindScheduledSkipped() + { + $task = $this->createTask(); + $this->taskRepository->save($task); + + $this->save($task, new \DateTime('+1 hour')); + + $this->assertNull($this->taskExecutionRepository->findNextScheduled()); + } + /** * Save a new execution to database. * From 01f80cb2045b6f4a64816a5299666f5169707761 Mon Sep 17 00:00:00 2001 From: Johannes Wachter Date: Thu, 18 May 2017 09:55:21 +0200 Subject: [PATCH 7/7] fixed composer.json --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index d65508e..a61abd1 100644 --- a/composer.json +++ b/composer.json @@ -11,7 +11,7 @@ ], "require": { "php": "~5.5 || ~7.0", - "php-task/php-task": "dev-feature/lock", + "php-task/php-task": "dev-master", "symfony/http-kernel": "^2.6 || ^3.0", "symfony/dependency-injection": "^2.6 || ^3.0", "symfony/expression-language": "^2.6 || ^3.0",