Consolidate CLI functionality into a service

Fixed the link.

Tentative start to this approach.

This patch provides a service for drush commands based on the approach linked in the summary. It provides a service with name migrate_tools.cli in the class Drupal\migrate_tools\MigrateToolsCliService.

This class has stub methods for each of the commands provided by migrate tools. The method for import() is implemented, and called from the Drush 9 command class. This can run a migration in this way, but does not output to the screen as I haven't passed through the drush logger yet.

Next steps should be to pass through the drush logger for drush 9, and get the same service method called from drush 8. Then fill in the other methods. Then profit.

#3 was missing the constructor to inject the Migration Plugin Manager to the CLI service. This fixes that, and passes the logger Drush injects to the Command class to the CLI service before calling import(). So we get the same output to the screen as we did before refactoring to a service.

Some preliminary suggestions:

1. +++ b/src/MigrateToolsCliService.php
   @@ -0,0 +1,301 @@
   +    array $options = [
   +      'all' => NULL,
   +      'group' => NULL,
   +      'tag' => NULL,
   +      'limit' => NULL,
   +      'feedback' => NULL,
   +      'idlist' => NULL,
   +      'update' => NULL,
   +      'force' => NULL,
   +      'execute-dependencies' => NULL,
   +    ]
   

   Can we do array merging after passing this into import? I really don't like to see functions names span across multiple lines.

2. +++ b/src/MigrateToolsCliService.php
   @@ -0,0 +1,301 @@
   +    $filter['migration_group'] = $options['group'] ? explode(
   +      ',',
   +      $options['group']
   +    ) : [];
   +    $filter['migration_tags'] = $options['tag'] ? explode(
   +      ',',
   +      $options['tag']
   +    ) : [];
   

   I find it easier to read if these are on a single line.

3. +++ b/src/MigrateToolsCliService.php
   @@ -0,0 +1,301 @@
   +              $configured_id = (in_array(
   +                $search_value,
   +                $configured_values
   +              )) ? $search_value : 'default';
   ...
   +                if (empty($migration_ids) || in_array(
   +                    Unicode::strtolower($id),
   +                    $migration_ids
   +                  )) {
   

   Same here.

4. +++ b/src/MigrateToolsCliService.php
   @@ -0,0 +1,301 @@
   +        dt(
   +          '!name Migration - !count failed.',
   +          ['!name' => $migration_id, '!count' => $count]
   +        )
   

   Same line maybe?

The split lines are just inherited from the equivalent existing code in the MigrateToolsCommands class, so I left them as they were so it was easier to eyeball any differences. But that doesn't mean we can't fix them as we move them, and I agree that they are better unsplit.

Just addressing the points from #5 - still lots to do.

@Eli-T, can you elaborate on 'still lots to do'? Maybe even some small steps or pieces. I'd like to help but not sure what needs to be done next here.

@quietone that's awesome! I've just updated the issue summary in the standard template so we can clearly detail the approach I was taking and where it was up to.

The tasks struck off in the summary reflect the status of the patch in #7.

I started to work on the next task - getting Drush 8 to run an import through the service - work in progress in patch uploaded in this comment.
The migrate() method on the MigrateToolsCliService is set to be used by Drush 8, but the MigrateToolsCliService class is attempted to be constructed with the Drush 9 logger, which is NULL, so it falls over.

So next step is to either look at that, or flesh out the other methods in Drush 9 land.

Hope that makes sense, if not, hit me up in Slack and I'll try to clarify.

@Eli-T, thanks for the details. It is late so I opted to keep it simple and used the existing migrate-import changes to convert stop, resetStatus, messages and fieldSource.

With the work going into https://github.com/drush-ops/drush/issues/2926 and more specifically the plan in https://github.com/drush-ops/drush/pull/3402... I not sure we need to keep working on things here.

Thanks for the heads up @heddn - I agree, there is little point working on this whilst it looks like it will be removed. Changing status to postponed for now.

This ticket was posponed pending on a possible drush upstrean inclusion, but the it was decided that will not be the case by drush maintainers.

We no longer have 2 code streams for drush commands. So I think this issue has served its purpose.