Make Field Encrypt compatible with Drupal 11.1 by removing eval() and adding a ServiceProvider, use OOP hooks

I don't think this actually needs to be postponed, I'll let you change that though, but just transferring comments here.

Unfortunately dynamically created hooks are unsupported in the new OOP system. It is listed under breaking changes.

You'll want to implement one hook for update and insert like this:

function field_encrypt_entity_insert() {
  if (in_array($entitty_type, \Drupal::state()->get('field_encrypt.entity_types', []) \Drupal::service('field_encrypt.process_entities')->decryptEntity(\$entity);
}

Then you can order that using HMIA normally.

Once you've done that you can remove the eval code and the function checking for eval availability.
You'll also be able to remove the phpcs ignore rule you have to ignore complaints about eval.

Let me know if you have any other questions!

One more note: I think once you do this, you WILL need the patch here: #3484754: Ordering of hooks needs some attention since you will be reordering using extra types.

nicxvan is right: the new system can't support such eval'd code and it's perhaps easiest to decide runtime instead. But what if someone really, really, really wants to define hooks dynamically? It's doable: HookCollectorPass merely automates the following three things when it finds a method marked with the Hook attribute but it's totally doable manually.

1. Register the class as an autowired service (with the classname as the service id):

Drupal\foo\FooHooks:
  class: Drupal\foo\FooHooks
  autowire: true

2. Add a kernel.event_listener tag on the definition:

$definition->addTag('kernel.event_listener', [
  'event' => "drupal_hook.$hook",
  'method' => $method,
  'priority' => $hook_data['priority'],
]);

3. Record which module this listener belongs to in the container argument hook_implementations_map:

$map[$hook][$class][$method] = $hook_data['module'];
// ... later
$container->setParameter('hook_implementations_map', $map);

Note you can add as many hooks as you want on the same class/service and even the same method.

Also note the Hook attribute is not used after this.

So if you do not want to decide run time which entity type should fire your service then you could use a service provider to iterate your entity types and add the right tag directly on the field_encrypt.process_entities service, method decryptEntity and record it in the map:

class FieldEncryptServiceProvider extends ServiceProviderBase {
  public function alter(ContainerBuilder $container) {
    $map = $container->getParameter('hook_implementations_map');
    $definition = $container->findDefinition('field_encrypt.process_entities');
    $class = $definition->getClass();
    $method = 'decryptEntity'; // perhaps this should be a class constant
    foreach ($container->get('state')->get('field_encrypt.entity_types', []) as $entity_type) {
      $hook = "entity_{$entity_type}_insert";
      $definition->addTag('kernel.event_listener', [
        'event' => "drupal_hook.$hook",
        'method' => $method,
        'priority' => 0,
      ]);
      $map[$hook][$class][$method] = 'field_encrypt';
    }
    $container->setParameter('hook_implementations_map', $map);
  }
}

The code is untested but should be close.

Yes this is not easy but this was not among the many goals the hook-as-OOP system needed to solve. It's enough it's doable as it should be very rare this is necessary.

You could keep the current code for pre-Hook codebases and use this for Drupal 11.1 and eventually drop the old one. You could class_exists the HookCollectorPass class in your .module and bail out early if it exists. The service provider doesn't do anything on pre-Hook codebases and also requires nothing from the new codebase so that's safe to keep around. This way one branch will work for all.

Yes sorry if it was not clear.

The explicit reason is because hmia is called during build.

But the question is why are you calling drupal state there.

It is so you can then dynamically generate hook implementations and order them.

If you take one of the two solutions recommended you can just order the specific hooks you create and you don't need the Drupal state call.

FWIW we cannot use the generic hook_entity_insert() here. We need to decrypt the entity at the very beginning of the calls to the hooks and hook_ENTITY_TYPE_insert is called before the generic hooks. That's why all this complexity exists in the first place. Using the eval method increases this module's compatibility with other modules massively as no module expects encrypted data :)

I think #9 might offer a way forward that removes the eval usage and is potentially very interesting.

In that case use priority PHP_INT_MAX.

If you don't mind continuing to document here we'll likely want to clean this up and post it as a guide for advanced usage for any other contrib maintainers that have similar complex needs.

I am using commit ef79e5b425c of oop_hooks_workaround4

And drush updb returns this error:

   [error]  Error: Call to a member function insert() on null in Drupal\Core\Lock\DatabaseLockBackend->acquire() (line 71 of /var/www/html/web/core/lib/Drupal/Core/Lock/DatabaseLockBacken  
  d.php) #0 /var/www/html/web/core/lib/Drupal/Core/Cache/CacheCollector.php(235): Drupal\Core\Lock\DatabaseLockBackend->acquire()                                                            
  #1 /var/www/html/web/core/lib/Drupal/Core/Cache/CacheCollector.php(312): Drupal\Core\Cache\CacheCollector->updateCache()                                                                   
  #2 /var/www/html/web/core/lib/Drupal/Core/DrupalKernel.php(691): Drupal\Core\Cache\CacheCollector->destruct()                                                                              
  #3 /var/www/html/vendor/drush/drush/src/Boot/DrupalBoot8.php(312): Drupal\Core\DrupalKernel->terminate()                                                                                   
  #4 [internal function]: Drush\Boot\DrupalBoot8->terminate()                                                                                                                                
  #5 {main}. 

No idea what is going on, but poking around through intuition and git bisect I found that src/FieldEncryptServiceProvider.php causes the error, and deleting that file makes the error go away, although likely causes other problems because I imagine it is there for a reason.

@ptmkenny unfortunately I don't know any more than I put in my comment. When I ran drush updb and src/FieldEncryptServiceProvider.php was present, I got the error, if I temporarily removed the file I could run drush updb successfully.

On Drupal 11.1.4.

@ptmkenny Yes I am using it to encrypt some node and paragraph fields.

It looks like we have to go one of two ways here. Either to somehow fix state in Drupal core so it can be accessing during an alter implementation. Or we need to stop using state.

If we go for not using state we need to do something like:

• A new table just to store this value. We need something that is available early in the container - unfortunately I don' think we should be accessing entity definitions here otherwise we're going to hit the same problems.
• In the alter read from the table and set this value as a container parameter so all the other places we read from it can use it cheaply.
• In the places where we used to write to state write to the new db table instead.

I think ideally state would work in this situation so might try to find sometime to fix this in core but I feel that we might have to do new table approach for now so that we can be compatible with 11.1 because any fix for state is not going to happen there.

Using a separate key value collection is a good idea that might solve a lot of problems. We will need to change all the calls to state to use the new key value collection and we will need to provide an upgrade path.

This is good to go now. We have test coverage and an update path. We should add some docs to the release notes about removing any manual implementations if you didn't use the eval() way of doing things in 3.x but this approach shows how OOP hooks are great and let's us do interesting things like this.