Issue #2365585 by yched: FieldItemList::filterEmptyItems() renumbers deltas but does not update the Items

core/lib/Drupal/Core/Config/Schema/Sequence.php

@@ -89,4 +89,13 @@ class Sequence extends ArrayElement implements ListInterface {
     return $this;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function filter($callback) {
+    $this->value = array_filter($this->value, $callback);
+    unset($this->elements);
+    return $this;
+  }
+
 }

core/lib/Drupal/Core/Field/FieldItemList.php

@@ -101,11 +101,9 @@ class FieldItemList extends ItemList implements FieldItemListInterface {
    * {@inheritdoc}
    */
   public function filterEmptyItems() {
-    if (isset($this->list)) {
+    $this->filter(function ($item) {
-      $this->list = array_values(array_filter($this->list, function($item) {
+      return !$item->isEmpty();
-        return !$item->isEmpty();
+    });
-      }));
-    }
   }
 
   /**

core/lib/Drupal/Core/TypedData/ListInterface.php

@@ -80,4 +80,16 @@ interface ListInterface extends TraversableTypedDataInterface, \ArrayAccess, \Co
    */
   public function first();
 
+  /**
+   * Filters the items in the list using a custom callback.
+   *
+   * @param callable $callback
+   *   The callback to use for filtering. Like with array_filter(), the
+   *   callback is called for each item in the list. Only items for which the
+   *   callback returns TRUE are preserved.
+   *
+   * @return $this
+   */
+  public function filter($callback);
+
 }

core/lib/Drupal/Core/TypedData/Plugin/DataType/ItemList.php

@@ -229,6 +229,33 @@ class ItemList extends TypedData implements \IteratorAggregate, ListInterface {
     return TRUE;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function filter($callback) {
+    if (isset($this->list)) {
+      $removed = FALSE;
+      // Apply the filter, detecting if some items were actually removed.
+      $this->list = array_filter($this->list, function ($item) use ($callback, &$removed) {
+        if (call_user_func($callback, $item)) {
+          return TRUE;
+        }
+        else {
+          $removed = TRUE;
+        }
+      });
+      if ($removed) {
+        // Rekey the array using array_values().
+        $this->list = array_values($this->list);
+        // Manually update each item's delta.
+        foreach ($this->list as $delta => $item) {
+          $item->setContext($delta, $this);
+        }
+      }
+    }
+    return $this;
+  }
+
   /**
    * Implements \Drupal\Core\TypedData\ListInterface::onChange().
    */

core/modules/system/src/Tests/Entity/EntityFieldTest.php

@@ -265,6 +265,14 @@ class EntityFieldTest extends EntityUnitTestBase  {
     $this->assertEqual(count(iterator_to_array($entity->name->getIterator())), count($entity->name), format_string('%entity_type: Count matches iterator count.', array('%entity_type' => $entity_type)));
     $this->assertTrue($entity->name->getValue() === array(0 => array('value' => NULL)), format_string('%entity_type: Name field value contains a NULL value.', array('%entity_type' => $entity_type)));
 
+    // Test using filterEmptyItems().
+    $entity->name = array(NULL, 'foo');
+    $this->assertEqual(count($entity->name), 2, format_string('%entity_type: List has 2 items.', array('%entity_type' => $entity_type)));
+    $entity->name->filterEmptyItems();
+    $this->assertEqual(count($entity->name), 1, format_string('%entity_type: The empty item was removed.', array('%entity_type' => $entity_type)));
+    $this->assertEqual($entity->name[0]->value, 'foo', format_string('%entity_type: The items were renumbered.', array('%entity_type' => $entity_type)));
+    $this->assertEqual($entity->name[0]->getName(), 0, format_string('%entity_type: The deltas were updated in the items.', array('%entity_type' => $entity_type)));
+
     // Test removing all list items by assigning an empty array.
     $entity->name = array();
     $this->assertIdentical(count($entity->name), 0, format_string('%entity_type: Name field contains no items.', array('%entity_type' => $entity_type)));

core/modules/system/src/Tests/TypedData/TypedDataTest.php

@@ -11,6 +11,7 @@ use Drupal\Core\Field\BaseFieldDefinition;
 use Drupal\Core\TypedData\DataDefinition;
 use Drupal\Core\TypedData\ListDataDefinition;
 use Drupal\Core\TypedData\MapDataDefinition;
+use Drupal\Core\TypedData\TypedDataInterface;
 use Drupal\simpletest\DrupalUnitTestBase;
 use Drupal\Core\Datetime\DrupalDateTime;
 
@@ -318,7 +319,7 @@ class TypedDataTest extends DrupalUnitTestBase {
     // Test iterating.
     $count = 0;
     foreach ($typed_data as $item) {
-      $this->assertTrue($item instanceof \Drupal\Core\TypedData\TypedDataInterface);
+      $this->assertTrue($item instanceof TypedDataInterface);
       $count++;
     }
     $this->assertEqual($count, 3);
@@ -394,6 +395,43 @@ class TypedDataTest extends DrupalUnitTestBase {
     }
   }
 
+  /**
+   * Tests the filter() method on typed data lists.
+   */
+  public function testTypedDataListsFilter() {
+    // Check that an all-pass filter leaves the list untouched.
+    $value = array('zero', 'one');
+    $typed_data = $this->createTypedData(ListDataDefinition::create('string'), $value);
+    $typed_data->filter(function(TypedDataInterface $item) {
+      return TRUE;
+    });
+    $this->assertEqual($typed_data->count(), 2);
+    $this->assertEqual($typed_data[0]->getValue(), 'zero');
+    $this->assertEqual($typed_data[0]->getName(), 0);
+    $this->assertEqual($typed_data[1]->getValue(), 'one');
+    $this->assertEqual($typed_data[1]->getName(), 1);
+
+    // Check that a none-pass filter empties the list.
+    $value = array('zero', 'one');
+    $typed_data = $this->createTypedData(ListDataDefinition::create('string'), $value);
+    $typed_data->filter(function(TypedDataInterface $item) {
+      return FALSE;
+    });
+    $this->assertEqual($typed_data->count(), 0);
+
+    // Check that filtering correctly renumbers elements.
+    $value = array('zero', 'one', 'two');
+    $typed_data = $this->createTypedData(ListDataDefinition::create('string'), $value);
+    $typed_data->filter(function(TypedDataInterface $item) {
+      return $item->getValue() !== 'one';
+    });
+    $this->assertEqual($typed_data->count(), 2);
+    $this->assertEqual($typed_data[0]->getValue(), 'zero');
+    $this->assertEqual($typed_data[0]->getName(), 0);
+    $this->assertEqual($typed_data[1]->getValue(), 'two');
+    $this->assertEqual($typed_data[1]->getName(), 1);
+  }
+
   /**
    * Tests using a typed data map.
    */