Issue #2431379 by tim.plunkett, Upchuk, dawehner: LazyPluginCollection should...

Issue #2431379 by tim.plunkett, Upchuk, dawehner: LazyPluginCollection should not implement \Iterator

core/lib/Drupal/Component/Plugin/LazyPluginCollection.php

@@ -12,7 +12,7 @@
  *
  * @ingroup plugin_api
  */
-abstract class LazyPluginCollection implements \Iterator, \Countable {
+abstract class LazyPluginCollection implements \IteratorAggregate, \Countable {
 
   /**
    * Stores all instantiated plugins.
@@ -147,46 +147,12 @@ public function removeInstanceId($instance_id) {
     $this->remove($instance_id);
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  public function current() {
-    return $this->get($this->key());
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function next() {
-    next($this->instanceIDs);
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function key() {
-    return key($this->instanceIDs);
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function valid() {
-    $key = key($this->instanceIDs);
-    // Check the key is valid but also that this key yields a plugin from get().
-    // There can be situations where configuration contains data for a plugin
-    // that cannot be instantiated. In this case, this enables us to skip that
-    // plugin during iteration.
-    // @todo Look at removing when https://drupal.org/node/2080823 has been
-    //   solved.
-    return $key !== NULL && $key !== FALSE && $this->get($key);
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function rewind() {
-    reset($this->instanceIDs);
+  public function getIterator() {
+    $instances = [];
+    foreach ($this->getInstanceIds() as $instance_id) {
+      $instances[$instance_id] = $this->get($instance_id);
+    }
+    return new \ArrayIterator($instances);
   }
 
   /**

core/lib/Drupal/Core/Plugin/DefaultLazyPluginCollection.php

@@ -110,7 +110,6 @@ public function sortHelper($aID, $bID) {
    */
   public function getConfiguration() {
     $instances = array();
-    $this->rewind();
     // Store the current order of the instances.
     $current_order = $this->instanceIDs;
     // Reorder the instances to match the original order, adding new instances

core/modules/views/src/Tests/ViewExecutableTest.php

@@ -8,6 +8,7 @@
 namespace Drupal\views\Tests;
 
 use Drupal\comment\Tests\CommentTestTrait;
+use Drupal\views\Entity\View;
 use Drupal\views\Views;
 use Drupal\views\ViewExecutable;
 use Drupal\views\ViewExecutableFactory;
@@ -436,4 +437,23 @@ public function testValidate() {
     $this->assertNotIdentical($validate, $validate_deleted, 'Master display has not been validated.');
   }
 
+  /**
+   * Tests that nested loops of the display handlers won't break validation.
+   */
+  public function testValidateNestedLoops() {
+    $view = View::create(['id' => 'test_validate_nested_loops']);
+    $executable = $view->getExecutable();
+
+    $executable->newDisplay('display_test');
+    $executable->newDisplay('display_test');
+    $errors = $executable->validate();
+    $total_error_count = array_reduce($errors, function ($carry, $item) {
+      $carry += count($item);
+      return $carry;
+    });
+    // Assert that there were 9 total errors across 3 displays.
+    $this->assertIdentical(9, $total_error_count);
+    $this->assertIdentical(3, count($errors));
+  }
+
 }

core/modules/views/tests/modules/views_test_data/src/Plugin/views/display/DisplayTest.php

@@ -149,4 +149,15 @@ public function calculateDependencies() {
     ];
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function validate() {
+    $errors = parent::validate();
+    foreach ($this->view->displayHandlers as $display_handler) {
+      $errors[] = 'error';
+    }
+    return $errors;
+  }
+
 }