Problem/Motivation

The EntityAdapter class now assumes that all entities have a getIterator() method, which is not documented in the EntityInterface and not provided for Config Entities.

As as result, many Drupal pages (e.g. configure block) return a fatal error due to a missing method on a Config Entity that interacts with that page.

The website encountered an unexpected error. Please try again later.
Error: Call to undefined method Drupal\domain\Entity\Domain::getIterator() in Drupal\Core\Entity\Plugin\DataType\EntityAdapter->getIterator() (line 170 of core/lib/Drupal/Core/Entity/Plugin/DataType/EntityAdapter.php).

Drupal\Core\Entity\Plugin\DataType\EntityAdapter->getIterator() (Line: 146)
Drupal\Core\TypedData\Validation\RecursiveContextualValidator->validateNode(Object, Array, 1) (Line: 99)
Drupal\Core\TypedData\Validation\RecursiveContextualValidator->validate(Object, Array, NULL) (Line: 90)
Drupal\Core\TypedData\Validation\RecursiveValidator->validate(Object, Array) (Line: 285)
Drupal\Core\Plugin\Context\ContextDefinition->isSatisfiedBy(Object) (Line: 46)
Drupal\Core\Plugin\Context\ContextHandler->Drupal\Core\Plugin\Context\{closure}(Object)

Proposed resolution

Rewrite the method to understand that getIterator() is not required.

  /**
   * {@inheritdoc}
   */
  public function getIterator() {
    return isset($this->entity) ? $this->entity->getIterator() : new \ArrayIterator([]);
  }

If we don't make this change, then the EntityInterface and base classes must be updated.

Remaining tasks

Provide a patch.

User interface changes

None.

API changes

None.

Data model changes

None

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

agentrickard created an issue. See original summary.

agentrickard’s picture

Issue summary: View changes
agentrickard’s picture

agentrickard’s picture

Status: Active » Needs review
agentrickard’s picture

Priority: Major » Critical

Bumping to critical since this is a BC break from 8.4 to 8.5, likely introduced by #2671964: ContextHandler cannot validate constraints

tim.plunkett’s picture

+++ b/core/lib/Drupal/Core/Entity/Plugin/DataType/EntityAdapter.php
@@ -167,7 +167,7 @@ public function applyDefaultValue($notify = TRUE) {
+    return (isset($this->entity) && method_exists($this->entity, 'getIterator')) ? $this->entity->getIterator() : new \ArrayIterator([]);

Even simpler:

return $this->entity instanceof \IteratorAggregate ? $this->entity->getIterator() : new \ArrayIterator([]);

PHPStorm flags the code in HEAD:

Method 'getIterator' not found in \Drupal\Core\Entity\EntityInterface|null

#1818574: Support config entities in typed data EntityAdapter seems to suggest that config entities aren't even supported?
And EntityAdapter::get/set link to that issue and have explicit checks for FieldableEntityInterface...
Yet EntityDeriver happily returns all entity types?
That's confusing.

Ideally this would be testable.
Currently \Drupal\Tests\Core\Entity\TypedData\EntityAdapterUnitTest cheats and does

$this->getMockForAbstractClass('\Drupal\Core\Entity\ContentEntityBase',...
agentrickard’s picture

Tim's version of the code also fixes the issue.

I'll see about a test case in a bit.

agentrickard’s picture

Patch for failing test case.

agentrickard’s picture

Patch for working test case.

tim.plunkett’s picture

Status: Needs review » Reviewed & tested by the community

Nice! Assuming #8 fails and #9 passes as expected, this looks good to go.

@agentrickard mentioned that https://www.drupal.org/u/hbensalem is the one who found this first, can a committer add credit for them?

The last submitted patch, 8: 2941457-entityadapter-getiterator-fail.patch, failed testing. View results

alexpott’s picture

Adding issue credit

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed 32157c7c5d to 8.6.x and 8830ce72bb to 8.5.x. Thanks!

  • alexpott committed 32157c7 on 8.6.x
    Issue #2941457 by agentrickard, tim.plunkett, hbensalem: EntityAdapter::...

  • alexpott committed 8830ce7 on 8.5.x
    Issue #2941457 by agentrickard, tim.plunkett, hbensalem: EntityAdapter::...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.