jhedstrom / DrupalDriver

A collection of drivers for controlling Drupal.
GNU General Public License v2.0
64 stars 95 forks source link

Drupal 8 entities fail to create if their bundle is brand new #239

Closed sensespidey closed 2 years ago

sensespidey commented 2 years ago

We have a test fixtures module that enables a new entity type (bundle), just before doing a "Given content" step in our Behat suite, and we were finding the test would fail because the new bundle is not yet available via the entity_type.bundle.info service. Even inserting a drush cr in between seemed to have no effect, but addling a line in the Drupal/Driver/Cores/Drupal8::entityCreate() method to explicitly clear the bundle cache resolved the problem.

Specifically, the patched method looks like so:

  public function entityCreate($entity_type, $entity) {
     <snip>

    // Throw an exception if a bundle is specified but does not exist.
    if (isset($entity->$bundle_key) && ($entity->$bundle_key !== NULL)) {
      /** @var \Drupal\Core\Entity\EntityTypeBundleInfo $bundle_info */
      $bundle_info = \Drupal::service('entity_type.bundle.info');

      # Clear bundles cache to ensure new entity type bundles are available.
      $bundle_info->clearCachedBundles();

      $bundles = $bundle_info->getBundleInfo($entity_type);
      if (!in_array($entity->$bundle_key, array_keys($bundles))) {
        #throw new \Exception("Cannot create entity because provided bundle '$entity->$bundle_key' does not exist.");
        throw new \Exception("Cannot create entity because provided bundle ".$entity->$bundle_key." does not exist.");
      }   
    }   
    <snip>

}

Incidentally, we also found the Exception being thrown when this happened was failing apparently because of the way the string was being constructed. It looks like a recurrence of #118, but easily fixed as noted above by using string concatenation instead of attempting to interpolate inline.

pfrenssen commented 2 years ago

I would consider it the responsibility of the calling code to only call this method using a valid entity type. If the calling code creates a new entity type just prior to calling this, then they should ensure that the relevant static caches are cleared.

To me this is not a bug in DrupalDriver, and probably not even a bug in DrupalExtension. It sounds more like an incorrect expectation of how DrupalExtension manages its caches. This can probably be avoided by doing the setup of the test fixtures inside a @Given step so that the entire process is handled inside of DrupalExtension.

sensespidey commented 2 years ago

Ok interesting- it's certainly possible this is the wrong place for this fix. FWIW, the calling code here that was creating a new entity type was simply enabling a module which instantiated the new bundle, and thus it seemed reasonable to expect the relevant caches to be cleared at that point.

Unfortunately the test (in this form) seems to have gone away since, but it was intended to check that the new bundle was available and functional when enabling the new module. The step before the "Given ... content" was literally a "drush en -y ", so aside from the "drush cr" we tried introducing in between, there was no obvious place to clear static caches aside from the test step.

Anyway, since the history of this seems to have disappeared on me, I'm happy to close this ticket without further ado. Thanks for following up!

pfrenssen commented 2 years ago

Possibly those Drush commands are run in a separate PHP process, so it's clearing the static caches of the Drush process, and the ones of the Behat process are left untouched.

I'm happy to hear that the problem is gone, but if it would reappear - instead of using Drush you could try to enable the module inside the Behat process by creating a step like @Given the :module_name module is enabled, and calling ModuleInstaller::install() inside it. This would keep the code in the same PHP process and gives you some possibilities to intervene in the process.