Refactor to decorate ModuleHandler instead of HMIA, to make it work in Drupal 10 with Drush 13

Moving some description here, from duplicate issue.

In CheckCircularReferencesPass.php line 81:

  Circular reference detected for service "migrate_boost.module_handler", path: "migrate_boost.module_handler -> config.factory -> config.typed -> migrate_boost.module_handler".

Current code produces this error. I tried uninstalling and pre patch, and installing with patch with the same result.

Same in the title, drupal 10 and drush 13

Sorry, I applied it against 1.x, disregard my comment

I saw the same problem in 2.x

The MR still has this alter() static method call which doesn't exist any more:

function migrate_boost_module_implements_alter(&$implementations, $hook) {
  MigrateBoost::alter($implementations, $hook);
}

What's the plan for this?

MigrateBoost::isBoostActive() seems to be always not set. And the drush command doesn't seem to be available.
I think the command is missing a services yaml like:

  migrate_boost.commands:
    class: Drupal\migrate_boost\Commands\MigrateBoostCommands
    tags:
      - { name: drush.command }

and I think the namespace for the command should be
namespace Drupal\migrate_boost\Commands; instead of namespace Drupal\migrate_boost\Drush\Commands;

Thoughts?

RE #11, this is the error, I didn't make a commit because I don't have an idea what is supposed to happen with that, I suspect remove migrate_boost_module_implements_alter?

Error: Call to undefined method Drupal\migrate_boost\MigrateBoost::alter() in /var/www/html/public/modules/contrib/migrate_boost/migrate_boost.module on line 14 #0 /var/www/html/public/core/lib/Drupal/Core/Extension/ModuleHandler.php(552): migrate_boost_module_implements_alter()

Re #12, based on the Issue Summary goal of “let’s also remove the dependency on Drush,” I’m assuming we can remove the command entirely.

Since we are decorating the module handler, we also don’t need migrate_boost_module_implements_alter().

I’ll go ahead and remove them as I’m just trying to get this working. Let me know if I’ve overstepped in my haste!

Thanks @nicxvan, I am not sure what HMIA is, I am guessing it's related to inheritance or something but google/ChatGPT didn't turn up good results.

Ah a Drupal acronym I've never heard of, thanks!

Ok this is working for me now. My last commit got rid of isHookDisabled() calls without the new $module arg because it wasn't available without implementing getImplementationInfo() and buildImplementationInfo() and verifyImplementations() which isn't the end of the world but getting pretty complicated and likely not needed.

The alter methods are tricky, because they call some of the decorated methods inside themselves. It’s almost better to just leave them alone.

Thanks for the review!

I’ll look at it tomorrow when I am in.

@nicxvan I had a look, trying it out IRL right now. The code looks good to me, going to mark this RTBC from my end as it does what is on the Tin ;)

Providing static patch for the time being.

Seems to work great! Currently, in the midst of migrating a large Drupal 6 site. If anything goes wrong related to this module, I'll comment here again!

Note, that the introduction of the new Decorator leads to an error when migrating d6 nodes. But that is unrelated to this module but rather a drupal core issue. I just created an issue + fix here: #3512815: "core/modules/node/src/Plugin/migrate/source/d6/Node.php" "__construct" method uses variable of type "ModuleHandler" instead of "ModuleHandlerInterface".

Otherwise, RTBC +1!

Confirming #26!

FYI, #3512815: "core/modules/node/src/Plugin/migrate/source/d6/Node.php" "__construct" method uses variable of type "ModuleHandler" instead of "ModuleHandlerInterface" was merged into core! So there is nothing holding back this issue!

Aw man I was using this on Drupal 10 🥺

I am quite sure I got this working in 10.x, but I put some time into making it work (whole time above was testing against 10.x ^^). Can I use #3520832: Consider adding Drupal 10 support back to version 2 as my other issue?