Support hook_post_update_X().

[jhedstrom]

• Resolves Support hook_post_update_X implementations #1602
• See https://www.drupal.org/node/2538108

[weitzman]

Hmmm.

1. I'm not sure why this needs to be added. I fear that we are going to clear caches twice. Does Drupal UI do that?
2. We always clear caches in a separate process now which can be skipped via an option. You can replace drupal_flush_all_caches() with drush_drupal_cache_clear_all().

[jhedstrom]

Yeah, the patch that just landed calls that function between hook_update_N implementations and the new hook:

    if ($post_updates) {
      // Now we rebuild all caches and after that execute the hook_post_update()
      // functions.
      $operations[] = ['drupal_flush_all_caches', []];
      foreach ($post_updates as $function) {
        $operations[] = ['update_invoke_post_update', [$function]];
      }
    }

I will re-work to use drush_drupal_cache_clear_all().

[weitzman]

It would be unicorn awesome if we had tests for updb. Just saying.

[greg-1-anderson]

I have a question of the opposite bent: is it now necessary for use to invariably clear the cache prior to a post-update operation?

Referring to https://www.drupal.org/node/2538108 again, imagine that there is some hook 8001, that makes a schema change, and a follow-on hook 8002 that needs to run as a post-update operation, with all hook options. If hook 8001 makes some change that invalidates caches, then it would be necessary to clear the caches before running hook 8002, or the results of the post-update hook would be unpredictable.

Also, the reason we allowed cache-clearing to be skipped via an option is to allow users to do multiple cache-clearing operations back-to-back, followed by one manual cache-clear at the end. I'm not sure that it is always safe to do that in the case of updatedb, though.

The bugaboo about updatedb functional tests and cache invalidation is that it's hard to know that you've really covered all contingencies; however, I agree that it would be super awesome to have at least one test that adequately covered one use case that was sufficiently complex (and no more so).

Something like:

1. Provide a 'demo' module in three versions:
   a. A base 8.x-1.0 version with no update hooks that initializes some data.
   b. An 8.x-1.1 version with an 8001 update hook, and an 8002 post-update hook
   c. An 8.x-1.2 version with an 8003 update hook.
2. Run two tests, starting with a fresh D8 base install for each one:
   a. Stepwise update:
   - Install base version, test data
   - Update to 8.x-1.1, run updatedb, test data
   - Update to 8.x-1.2, run updatedb, test data
   b. Skipped update:
   - Install base version, test data
   - Update to 8.x-1.2, run updatedb, test data

I think it would be best to just have the three "versions" in three different folders and "update" them via a filesystem copy, not via the pm code. The module should provide an API function that could be called from the test to verify the data; the verify function can be different in each "version".

Not sure what the best update_N function should be; ideally, something that actually used Drupal APIs to set up and alter its schema would be best.

[jhedstrom]

It would be unicorn awesome if we had tests for updb. Just saying.

With the db dumps in D8 for testing, we may be able to extend core's UpdatePathTestBase just enough to test our own updates. We can pick that up in #1453.

[jhedstrom]

Oh, just realized #1453 is closed.

[weitzman]

@jhedstrom - Feel free to merge this when satisfied

[jhedstrom]

Sorry, was on vacation--doing final review now.