Issue #1854912 by seanB, bojanz, dermario, fubhy, jhedstrom: Don't assume that...

Issue #1854912 by seanB, bojanz, dermario, fubhy, jhedstrom: Don't assume that the array keys are entity id's in EntityStorageBase::delete()

core/lib/Drupal/Core/Entity/EntityStorageBase.php

@@ -351,20 +351,26 @@ public function delete(array $entities) {
       return;
     }
 
+    // Ensure that the entities are keyed by ID.
+    $keyed_entities = [];
+    foreach ($entities as $entity) {
+      $keyed_entities[$entity->id()] = $entity;
+    }
+
     // Allow code to run before deleting.
     $entity_class = $this->entityClass;
-    $entity_class::preDelete($this, $entities);
-    foreach ($entities as $entity) {
+    $entity_class::preDelete($this, $keyed_entities);
+    foreach ($keyed_entities as $entity) {
       $this->invokeHook('predelete', $entity);
     }
 
     // Perform the delete and reset the static cache for the deleted entities.
-    $this->doDelete($entities);
-    $this->resetCache(array_keys($entities));
+    $this->doDelete($keyed_entities);
+    $this->resetCache(array_keys($keyed_entities));
 
     // Allow code to run after deleting.
-    $entity_class::postDelete($this, $entities);
-    foreach ($entities as $entity) {
+    $entity_class::postDelete($this, $keyed_entities);
+    foreach ($keyed_entities as $entity) {
       $this->invokeHook('delete', $entity);
     }
   }

core/lib/Drupal/Core/Entity/KeyValueStore/KeyValueEntityStorage.php

@@ -146,10 +146,7 @@ public function deleteRevision($revision_id) {
    * {@inheritdoc}
    */
   public function doDelete($entities) {
-    $entity_ids = array();
-    foreach ($entities as $entity) {
-      $entity_ids[] = $entity->id();
-    }
+    $entity_ids = array_keys($entities);
     $this->keyValueStore->deleteMultiple($entity_ids);
   }
 

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

@@ -95,6 +95,31 @@ protected function assertCRUD($entity_type, UserInterface $user1) {
     if ($revision_table = $definition->getRevisionTable()) {
       $this->assertEqual(0, db_query('SELECT COUNT(*) FROM {' . $revision_table . '}')->fetchField(), 'Data table was emptied');
     }
+
+    // Test deleting a list of entities not indexed by entity id.
+    $entities = array();
+    $entity = entity_create($entity_type, array('name' => 'test', 'user_id' => $user1->id()));
+    $entity->save();
+    $entities['test'] = $entity;
+    $entity = entity_create($entity_type, array('name' => 'test2', 'user_id' => $user1->id()));
+    $entity->save();
+    $entities['test2'] = $entity;
+    $controller = \Drupal::entityManager()->getStorage($entity_type);
+    $controller->delete($entities);
+
+    // Verify that entities got deleted.
+    $all = entity_load_multiple($entity_type);
+    $this->assertTrue(empty($all), format_string('%entity_type: Deleted all entities.', array('%entity_type' => $entity_type)));
+
+    // Verify that all data got deleted from the tables.
+    $definition = \Drupal::entityManager()->getDefinition($entity_type);
+    $this->assertEqual(0, db_query('SELECT COUNT(*) FROM {' . $definition->getBaseTable() . '}')->fetchField(), 'Base table was emptied');
+    if ($data_table = $definition->getDataTable()) {
+      $this->assertEqual(0, db_query('SELECT COUNT(*) FROM {' . $data_table . '}')->fetchField(), 'Data table was emptied');
+    }
+    if ($revision_table = $definition->getRevisionTable()) {
+      $this->assertEqual(0, db_query('SELECT COUNT(*) FROM {' . $revision_table . '}')->fetchField(), 'Data table was emptied');
+    }
   }
 
   /**