Fatal error: Cannot use object of type ConfigEntityDependency as array

This has to do with a patch that was added to core in #2754477: Unexpected config entity delete due to dependency calculations. I added a question in there to see if the intended result was to return an array of objects or if it was a mistake. I think it's better to fix it in core if it was a mistake before attempting to fix features.

The same code that is in features is also in console, it may also be in other modules so it would be better to fix it in core if possible.

Confirm that with Drupal 8.1.4, features will failed during Drustack installation, because we call to initPackageFromExtension() for rollback configuration manually.

Once patch applied (https://github.com/drustack/drustack-standard/commit/eb30b8b001d36b33d30...), all installation procedure passed. Thank you very much.

Hmmm.... I tried to keep the graph internal to the config dependency manager. Given the complexity of the fixes to it and the criticality when it goes wrong I like to try to keep it this way. Features shouldn't need to access this information - all the dependents are in the list anyway so I'm not sure why it is doing what it is doing. I think we can make a change that is compatible with 8.1.4 and 8.1.3.

As https://www.drupal.org/pift-ci-job/319851 is not failing it looks like we have missing test coverage. Since this involves dependencies, it is vital that there's test coverage.

So on 8.1.3....

$dependency_manager = \Drupal::service('config.manager')->getConfigDependencyManager();
$dependent_list = $dependency_manager->getDependentEntities('config', 'node.type.article');
$dependents = array();
foreach ($dependent_list as $config_name => $item) {
  if (!isset($dependents[$config_name])) {
    $dependents[$config_name] = $config_name;
  }
  // Grab any dependent graph paths.
  if (isset($item['reverse_paths'])) {
    foreach ($item['reverse_paths'] as $dependent_name => $value) {
      if ($value && !isset($dependents[$dependent_name])) {
        $dependents[$dependent_name] = $dependent_name;
      }
    }
  }
}

var_dump(array_keys($dependent_list));

var_dump(array_keys($dependents));

Returns:

array(9) {
  [0]=>
  string(24) "rdf.mapping.node.article"
  [1]=>
  string(35) "field.field.node.article.field_tags"
  [2]=>
  string(36) "field.field.node.article.field_image"
  [3]=>
  string(32) "field.field.node.article.comment"
  [4]=>
  string(29) "field.field.node.article.body"
  [5]=>
  string(44) "core.entity_view_display.node.article.teaser"
  [6]=>
  string(41) "core.entity_view_display.node.article.rss"
  [7]=>
  string(45) "core.entity_view_display.node.article.default"
  [8]=>
  string(45) "core.entity_form_display.node.article.default"
}
array(9) {
  [0]=>
  string(24) "rdf.mapping.node.article"
  [1]=>
  string(35) "field.field.node.article.field_tags"
  [2]=>
  string(45) "core.entity_form_display.node.article.default"
  [3]=>
  string(45) "core.entity_view_display.node.article.default"
  [4]=>
  string(41) "core.entity_view_display.node.article.rss"
  [5]=>
  string(44) "core.entity_view_display.node.article.teaser"
  [6]=>
  string(36) "field.field.node.article.field_image"
  [7]=>
  string(32) "field.field.node.article.comment"
  [8]=>
  string(29) "field.field.node.article.body"
}

So it is the same list.

On HEAD (8.2.x) today...

$dependency_manager = \Drupal::service('config.manager')->getConfigDependencyManager();
$dependent_list = $dependency_manager->getDependentEntities('config', 'node.type.article');

var_dump(array_keys($dependent_list));

array(9) {
  [0]=>
  string(45) "core.entity_view_display.node.article.default"
  [1]=>
  string(41) "core.entity_view_display.node.article.rss"
  [2]=>
  string(44) "core.entity_view_display.node.article.teaser"
  [3]=>
  string(45) "core.entity_form_display.node.article.default"
  [4]=>
  string(29) "field.field.node.article.body"
  [5]=>
  string(35) "field.field.node.article.field_tags"
  [6]=>
  string(24) "rdf.mapping.node.article"
  [7]=>
  string(32) "field.field.node.article.comment"
  [8]=>
  string(36) "field.field.node.article.field_image"
}

So I'm pretty sure all the looping is completely unnecessary.

This must be a critical bug for features.

Even if we reverted the change to return parts of the graph the reverse paths key will contain modules and themes so whatever we do we're going to need to change this code. I think the approach in #6 is the way to go.

So there is very basic test coverage of this functionality but as Features doesn't have daily tests against core we didn't get an early warning this was going to occur.

Without this patch - \Drupal\features_ui\Tests\FeaturesUITest, Drupal\features\Tests\FeaturesBundleUITest, Drupal\features\Tests\FeaturesCreateUITest are failing. With this patch it is green. Therefore I think we have test coverage.

Looks like a file is missing from #11 as I'm not seeing any test.

I looked back at the history on this. The code for the 'reverse_paths' was added by Nedjo from #2579277: Dependents should not include parents. I think maybe things changed since that code in Features was added. Seems that in the past it didn't pick up the dependents of the dependents (thus the looping through reverse_paths). Alex could probably confirm this.

I agree with Alex that we shouldn't be messing inside the config details like this and should just depend on the exposed methods. If getDependentEntities() correctly returns all the dependents now then I'd be really happy to remove the looping complexity from Features.

So, giving a tentative +1 to the patch, but we might need to get a test case from Nedjo that displays the original bug that was being fixed here. I don't see any field_storage being returned in #8 so the bug described in 2579277 doesn't make sense to me now.

@mpotter the empty patch proves that features HEAD is currently broken and is fixed by the patch. And yes the concerns of #2579277: Dependents should not include parents have been dealt with. The field storages are not dependent on the bundles - and actually have never been - that patch could safely make the same change as this patch.

@alexpott I reviewed and tested the patch. Looks like everything is working on my end. Giving it a +1 on my end as well. Again thanks for the quick response.

I enabled daily testing (PHP 5.6) so we can catch stuff like this in the future (thanks for the suggestion Alex!)

And now I understand what you were doing with the testing.

Will commit this and do a release Friday morning (on vacation today). Thanks for the patch and reviews!

Committed to 245b717. Released 3.0-beta6 with this fix.

Thank you for the quick fix and release! I just installed beta 6 and can confirm it fixed this error.

@alexpott, @Gravypower, @mpotter: I've been offfline for a few days. Thanks for fixing this!