Create a way to add steps to configuration sync

Looking at this one now.

In fact alexpott is working on this.

Only relatively minor things, so leaving as needs review.

1. +++ b/core/lib/Drupal/Core/Config/ConfigImporter.php
   @@ -436,36 +450,208 @@ public function getUnprocessedExtensions($type) {
   +   * @param string|callable $operation
   +   *   The operation to call. Either a method on the ConfigImporter class or a
   +   *   callable.
   

   So in my local version of this, I made it a callable all the time for simplicity. So config importer would just return callables also in its operations generator.

2. +++ b/core/lib/Drupal/Core/Config/ConfigImporter.php
   @@ -436,36 +450,208 @@ public function getUnprocessedExtensions($type) {
   +    $presync = $this->moduleHandler->invokeAll('config_import_presync_operation');
   +    $postsync = $this->moduleHandler->invokeAll('config_import_postsync_operation');
   

   operations? there may be multiple

3. +++ b/core/lib/Drupal/Core/Config/ConfigImporter.php
   @@ -436,36 +450,208 @@ public function getUnprocessedExtensions($type) {
   +   * Processes extensions as a batch operation.
   +   *
   +   * @param array $context.
   +   *   The batch context.
   ...
   +   * Processes configuration as a batch operation.
   +   *
   +   * @param array $context.
   +   *   The batch context.
   ...
   +   * Finishes the batch.
   +   *
   +   * @param array $context.
   +   *   The batch context.
   
   +++ b/core/modules/config/tests/config_import_test/config_import_test.module
   @@ -4,3 +4,41 @@
   + * @param array $context
   + *   The batch context.
   ...
   + * @param array $context
   + *   The batch context.
   

   It may or may not be a batch :) It is a semi-batch... Would not be correct to say it is a batch AFAIS.

4. +++ b/core/modules/config/tests/config_import_test/config_import_test.module
   @@ -4,3 +4,41 @@
   + * Implements pre configuration synchronization step for testing.
   ...
   + * Implements post configuration synchronization step for testing.
   

   *Operations* not step.

Is that $this lost due to batching? How is that not the same problem for anybody altering the steps then? Not the same problem there?

Do we expect/advise people to use global functions then? Which sounds like the only remaining use case for callables then no? Looks like we should remove support for that then to direct people to the right thing and avoid them doing callables with instances of objects.

1. +++ b/core/lib/Drupal/Core/Config/ConfigImporter.php
   @@ -436,36 +450,206 @@ public function getUnprocessedExtensions($type) {
   +   * @param $context
   

   typehint

2. +++ b/core/lib/Drupal/Core/Config/ConfigImporter.php
   @@ -436,36 +450,206 @@ public function getUnprocessedExtensions($type) {
   +      throw new ConfigImporterException(String::format('Operation %operation can not be called,', array('%operation', $sync_step)));
   

   The exception message shouldn't end in punctuation. Also, this assumes that $sync_step is a string, when it could be a malformed callable (or anything, really)

3. +++ b/core/lib/Drupal/Core/Config/ConfigImporter.php
   @@ -436,36 +450,206 @@ public function getUnprocessedExtensions($type) {
   +   * @throws ConfigImporterException
   

   Here (and elsewhere), full class name please

4. +++ b/core/lib/Drupal/Core/Config/ConfigImporter.php
   @@ -436,36 +450,206 @@ public function getUnprocessedExtensions($type) {
   +    $presync = $this->moduleHandler->invokeAll('config_import_presync_steps');
   +    $postsync = $this->moduleHandler->invokeAll('config_import_postsync_steps');
   +
   +    return array_merge($presync, $sync_steps, $postsync, array('finish'));
   

   These hooks need *.api.php entries.

   Also, I checked install_tasks (which this is similar to), and it includes an alter. I think allowing an alter of the merged list would be a good idea.

5. +++ b/core/lib/Drupal/Core/Config/ConfigImporter.php
   @@ -436,36 +450,206 @@ public function getUnprocessedExtensions($type) {
   +   * @return array|bool
   ...
   +   * @return array|bool
   

   I think we've tried to avoid/remove these, and return empty arrays. Accidentally foreach-ing over FALSE is never fun

6. +++ b/core/modules/config/lib/Drupal/config/Form/ConfigSync.php
   @@ -278,18 +278,20 @@ public function submitForm(array &$form, array &$form_state) {
   -  public static function processBatch(BatchConfigImporter $config_importer, $operation, &$context) {
   +  public static function processBatch(ConfigImporter $config_importer, $sync_step, &$context) {
   

   These renames are fine, but seem rather superfluous.

Addressing #9, array('Classname', 'static_method') and some_global_function would both be fine. We could try protecting against instance methods...
if (is_array($step) && is_object($step[0])) { throw new \Exception(); }
or something...

@timplunkett: re 4, the idea was not to allow removal of the crucial built-in steps which is why the pre/post steps hooks were added. Otherwise the full alter hook could be the only one and we would be done :)

All right, changes look good to me. Not entirely happy with now having 3 new hooks for this (we could just have one alter that would do the same thing?). But I don't feel strong about it. I do see the usecase of having the pre/post hooks which are safe and the alter hook which can then alter everything and knows all the *added* items are in, so we have different hooks for adding and then for possibly doing dangerous things. I don't feel strong either way.

Resolved issues raised by @timplunkett. I think we can refine this in the future as needed, but is a great first step.

Added a draft change notice at https://drupal.org/node/2239929

Updates look good and reflect my preference from #13. I rewrote the change notice based on the changes :)

1. +++ b/core/lib/Drupal/Core/Config/ConfigImporter.php
   @@ -436,36 +450,207 @@ public function getUnprocessedExtensions($type) {
   +        do {
   +          $this->doSyncStep($step, $context);
   +        } while ($context['finished'] < 1);
   

   Hm. We're performing all of the entity CRUD operations for all config of a single "step" ("operation"?) in one go...?

   What if the import involves to install 50 modules? (plus configuration?)

2. +++ b/core/lib/Drupal/Core/Config/ConfigImporter.php
   @@ -436,36 +450,207 @@ public function getUnprocessedExtensions($type) {
   +      $context['message'] = t('Synchronising extensions: @op @name.', array('@op' => $operation['op'], '@name' => $operation['name']));
   ...
   +    $context['message'] = t('Finalising configuration synchronisation.');
   

   s/s/z/ - American English

Cool beans.

The only thing I found a bit weird was that the example code referenced procedural functions vs. more "Drupal Eight-y" OO code. But Alex explained that the whole Batch API atm is highly procedural-focused and we've never gotten around to modernizing the code. There's some issue for this but I can't find it right now.

Otherwise, the code is nice and easy to read and well-tested. Committed and pushed to 8.x. Thanks!