allow usage of simplified method args in WorkflowServiceInner; nitpicks

kaliop-uk · Nov 22, 2022 · 63f1e29 · 63f1e29
1 parent 6ca1fde
commit 63f1e29
Show file tree

Hide file tree

Showing 10 changed files with 79 additions and 36 deletions.
diff --git a/.gitignore b/.gitignore
@@ -7,6 +7,5 @@
 /composer_*.json
 /composer_*.lock
 /phpunit.xml
-/teststack
 /vendor
 /vendor_*
diff --git a/API/Value/WorkflowDefinition.php b/API/Value/WorkflowDefinition.php
@@ -30,7 +30,7 @@ class WorkflowDefinition extends MigrationDefinition
      * @param string $path
      * @param string $rawDefinition
      * @param int $status
-     * @param MigrationStep[]|MigrationStepsCollection $steps
+     * @param \Kaliop\eZMigrationBundle\API\Value\MigrationStep[]|\Kaliop\eZMigrationBundle\API\Collection\MigrationStepsCollection $steps
      * @param string $parsingError
      * @param string $signalName
      * @param string|int|false|null $runAs if false will use the current user; if null will use hardcoded 14; string for login or user id

diff --git a/Command/AbstractCommand.php b/Command/AbstractCommand.php
@@ -10,12 +10,12 @@
 abstract class AbstractCommand extends ContainerAwareCommand
 {
     /**
-     * @var \Kaliop\eZWorkflowEngineBundle\Core\WorkflowService
+     * @var \Kaliop\eZWorkflowEngineBundle\Core\WorkflowServiceFacade
      */
     private $workflowService;
 
     /**
-     * @return \Kaliop\eZWorkflowEngineBundle\Core\WorkflowService
+     * @return \Kaliop\eZWorkflowEngineBundle\Core\WorkflowServiceFacade
      */
     public function getWorkflowService()
     {

diff --git a/Command/CleanupCommand.php b/Command/CleanupCommand.php
@@ -6,7 +6,6 @@
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Input\InputOption;
 use Kaliop\eZMigrationBundle\API\Value\Migration;
-use Kaliop\eZMigrationBundle\API\Value\MigrationDefinition;
 
 /**
  * Command to clean up workflows.
@@ -69,5 +68,7 @@ public function execute(InputInterface $input, OutputInterface $output)
         }
 
         $output->writeln($action . count($toRemove) . " workflows out of $total $label");
+
+        return 0;
     }
 }
diff --git a/Command/GenerateCommand.php b/Command/GenerateCommand.php
@@ -28,7 +28,7 @@ protected function configure()
             ->setHelp(<<<EOT
 The <info>kaliop:workflows:generate</info> command generates a skeleton workflows definition file:
 
-    <info>php ezpublish/console kaliop:workflows:generate bundlename</info>
+    <info>php bin/console kaliop:workflows:generate bundlename</info>
 EOT
             );
     }
@@ -92,6 +92,8 @@ public function execute(InputInterface $input, OutputInterface $output)
         if ($warning != '') {
             $output->writeln("<comment>$warning</comment>");
         }
+
+        return 0;
     }
 
     /**

diff --git a/Command/ResumeCommand.php b/Command/ResumeCommand.php
@@ -4,9 +4,8 @@
 
 use Kaliop\eZMigrationBundle\API\Value\Migration;
 use Symfony\Component\Console\Input\InputInterface;
-use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Input\InputOption;
-use Symfony\Component\Console\Input\InputArgument;
+use Symfony\Component\Console\Output\OutputInterface;
 
 /**
  * Command to resume suspended workflows.
@@ -72,7 +71,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
 
         if (!count($suspendedWorkflows)) {
             $output->writeln('Nothing to do');
-            return;
+            return 0;
         }
 
         // ask user for confirmation to make changes
@@ -111,5 +110,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
         if ($failed) {
             return 2;
         }
+
+        return 0;
     }
 }
diff --git a/Command/StatusCommand.php b/Command/StatusCommand.php
@@ -2,11 +2,11 @@
 
 namespace Kaliop\eZWorkflowEngineBundle\Command;
 
+use Kaliop\eZMigrationBundle\API\Value\Migration;
+use Symfony\Component\Console\Helper\Table;
 use Symfony\Component\Console\Input\InputInterface;
-use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Input\InputOption;
-use Symfony\Component\Console\Helper\Table;
-use Kaliop\eZMigrationBundle\API\Value\Migration;
+use Symfony\Component\Console\Output\OutputInterface;
 
 /**
  * Command to display the defined workflows.
@@ -31,7 +31,7 @@ public function execute(InputInterface $input, OutputInterface $output)
 
         if (!count($workflows)) {
             $output->writeln('<info>No workflows found</info>');
-            return;
+            return 0;
         }
 
         $summary = array(
@@ -114,5 +114,7 @@ public function execute(InputInterface $input, OutputInterface $output)
             ->setHeaders($headers)
             ->setRows($data);
         $table->render();
+
+        return 0;
     }
 }
diff --git a/Core/WorkflowServiceFacade.php b/Core/WorkflowServiceFacade.php
@@ -2,15 +2,15 @@
 
 namespace Kaliop\eZWorkflowEngineBundle\Core;
 
-use Symfony\Component\Config\ConfigCache;
-use Symfony\Component\Config\Resource\FileResource;
-use Psr\Log\LoggerInterface;
-use Kaliop\eZMigrationBundle\Core\MigrationService;
-use Kaliop\eZMigrationBundle\API\Value\MigrationDefinition;
 use Kaliop\eZMigrationBundle\API\Collection\MigrationDefinitionCollection;
 use Kaliop\eZMigrationBundle\API\DefinitionParserInterface;
 use Kaliop\eZMigrationBundle\API\ExecutorInterface;
+use Kaliop\eZMigrationBundle\API\Value\MigrationDefinition;
+use Kaliop\eZMigrationBundle\Core\MigrationService;
 use Kaliop\eZWorkflowEngineBundle\API\Value\WorkflowDefinition;
+use Psr\Log\LoggerInterface;
+use Symfony\Component\Config\ConfigCache;
+use Symfony\Component\Config\Resource\FileResource;
 
 /**
  * @todo add phpdoc to help IDEs
@@ -35,8 +35,6 @@ public function __construct(MigrationService $innerService, $recursionLimit, $ca
     }
 
     /**
-     * q: should this method be moved to WorkflowService instead of the Slot ?
-     *
      * @param string $signalName must use the same format as we extract from signal class names
      * @param array $signalParameters must follow what is found in eZ5 signals
      * @throws \Exception
@@ -85,8 +83,13 @@ public function triggerWorkflow($signalName, array $signalParameters)
 
                     if ($this->logger) $this->logger->debug("Executing workflow '{$workflowDefinition->name}' with parameters: " . preg_replace("/\n+/s", ' ', preg_replace('/^(Array| +|\(|\))/m', '', print_r($workflowParameters, true))));
 
-                    /// @todo allow setting of default lang ?
-                    $this->innerService->executeMigration($wfd, $workflowDefinition->useTransaction, null, $workflowDefinition->runAs, false, null, $workflowParameters);
+                    /// @todo allow setting of default lang, user and userGroup content types ?
+                    $this->innerService->executeMigration($wfd, array(
+                        'useTransaction' => $workflowDefinition->useTransaction,
+                        'adminUserLogin' => $workflowDefinition->runAs,
+                        'workflow' => $workflowParameters,
+                    ));
+
                     self::$workflowExecuting -= 1;
                 } catch (\Exception $e) {
                     self::$workflowExecuting -= 1;

diff --git a/Core/WorkflowServiceInner.php b/Core/WorkflowServiceInner.php
@@ -68,16 +68,20 @@ public function parseMigrationDefinition(MigrationDefinition $migrationDefinitio
      * Reimplemented to add more parameters to be stored in the context
      *
      * @param MigrationDefinition $migrationDefinition
-     * @param bool $useTransaction when set to false, no repo transaction will be used to wrap the migration
-     * @param string $defaultLanguageCode
-     * @param string|int|false|null $adminLogin when false, current user is used; when null, hardcoded admin account
-     * @param $workflowParameters
+     * @param array $migrationContext Supported array keys are: adminUserLogin, defaultLanguageCode, userContentType,
+     *                                userGroupContentType useTransaction, workflow
+     *                                was: bool $useTransaction when set to false, no repo transaction will be used to wrap the migration
+     * @param string $defaultLanguageCode Deprecated - use $migrationContext['defaultLanguageCode']
+     * @param string|int|false|null $adminLogin Deprecated - use $migrationContext['adminLogin']; when false, current user is used; when null, hardcoded admin account
+     * @param bool $force Deprecated - use $migrationContext['forceExecution']; when true, execute a migration if it was already in status DONE or SKIPPED (would throw by default)
+     * @param bool|null $forceSigchildEnabled Deprecated
+     * @param null|array $workflowParameters Deprecated - use $migrationContext['workflow']
      * @throws \Exception
      *
      * @todo treating a null and false $adminLogin values differently is prone to hard-to-track errors.
      *       Shall we use instead -1 to indicate the desire to not-login-as-admin-user-at-all ?
      */
-    public function executeMigration(MigrationDefinition $migrationDefinition, $useTransaction = true,
+    public function executeMigration(MigrationDefinition $migrationDefinition, $migrationContext = true,
         $defaultLanguageCode = null, $adminLogin = null, $force = false, $forceSigchildEnabled = null, $workflowParameters = null)
     {
         if ($migrationDefinition->status == MigrationDefinition::STATUS_TO_PARSE) {
@@ -89,8 +93,21 @@ public function executeMigration(MigrationDefinition $migrationDefinition, $useT
             throw new \Exception("Can not execute migration '{$migrationDefinition->name}': {$migrationDefinition->parsingError}");
         }
 
-        /// @todo add support for setting in $migrationContext a userContentType ?
-        $migrationContext = $this->migrationContextFromParameters($defaultLanguageCode, $adminLogin, $forceSigchildEnabled, $workflowParameters);
+        // BC: handling of legacy method call signature
+        if (!is_array($migrationContext)) {
+            $useTransaction = $migrationContext;
+            $migrationContext = $this->migrationContextFromParameters($defaultLanguageCode, $adminLogin, $forceSigchildEnabled, $workflowParameters);
+        } else {
+            if ($defaultLanguageCode !== null || $adminLogin !== null || $force !== false || $forceSigchildEnabled !== null
+                || $workflowParameters !== null
+            ) {
+                throw new \Exception("Invalid call to executeMigration: argument types mismatch");
+            }
+            $useTransaction = array_key_exists('useTransaction', $migrationContext) ? $migrationContext['useTransaction'] : true;
+            $migrationContext['workflow'] = $this->workflowMigrationContextFromParameters(
+                array_key_exists('workflow', $migrationContext) ? $migrationContext['workflow'] : null
+            );
+        }
 
         // set migration as begun - has to be in own db transaction
         $migration = $this->storageHandler->startMigration($migrationDefinition, $force);
@@ -104,8 +121,8 @@ public function executeMigration(MigrationDefinition $migrationDefinition, $useT
      * @param MigrationDefinition $migrationDefinition
      * @param array $migrationContext
      * @param int $stepOffset
-     * @param bool $useTransaction when set to false, no repo transaction will be used to wrap the migration
-     * @param string|int|false|null $adminLogin used only for committing db transaction if needed. If false or null, hardcoded admin is used
+     * @param bool $useTransaction Deprecated - replaced by $migrationContext['useTransaction']. When set to false, no repo transaction will be used to wrap the migration
+     * @param string|int|false|null $adminLogin Deprecated - $migrationContext['adminLogin']. Used only for committing db transaction if needed. If false or null, hardcoded admin is used
      * @throws \Exception
      */
     protected function executeMigrationInner(Migration $migration, MigrationDefinition $migrationDefinition,
@@ -130,27 +147,33 @@ protected function executeMigrationInner(Migration $migration, MigrationDefiniti
      * @param null $adminLogin
      * @param array $workflowParameters
      * @return array
+     * @deprecated kept for BC
      */
     protected function migrationContextFromParameters($defaultLanguageCode = null, $adminLogin = null, $forceSigchildEnabled = null, $workflowParameters = null)
     {
         $properties = parent::migrationContextFromParameters($defaultLanguageCode, $adminLogin, $forceSigchildEnabled);
 
+        $properties['workflow'] = $this->workflowMigrationContextFromParameters($workflowParameters);
+
+        return $properties;
+    }
+
+    protected function workflowMigrationContextFromParameters($workflowParameters = null)
+    {
         if (!is_array($workflowParameters)) {
             /// @todo log warning
             $workflowParameters = array();
         }
 
         $workflowParameters = array_merge(
+            $workflowParameters,
             array(
                 'original_user' => $this->repository->getCurrentUser()->login,
                 'start_time' => time()
-            ),
-            $workflowParameters
+            )
         );
 
-        $properties['workflow'] = $workflowParameters;
-
-        return $properties;
+        return $workflowParameters;
     }
 
     /**

diff --git a/WHATSNEW.md b/WHATSNEW.md
@@ -1,3 +1,15 @@
+Version 2.1
+===========
+
+* New: command `kaliop:workflows:workflow` learned action `--fail`
+
+* New: command `kaliop:workflows:debug` learned option `--show-path`
+
+* BC change (for developers extending the bundle): `WorflowServiceInner` methods `executeMigration` and `executeMigrationInner`
+  should now be called using a different signature. They do still work with the previous signature, but that usage is
+  considered deprecated
+
+
 Version 2.0
 ===========