Issue #3370639 by kala4ek, amateescu: Prevent trashed entities from being returned by entity_load

src/TrashStorageTrait.php

@@ -2,6 +2,9 @@
 
 namespace Drupal\trash;
 
+/**
+ * Provides the ability to soft-delete entities at the storage level.
+ */
 trait TrashStorageTrait {
 
   /**
@@ -28,6 +31,49 @@ trait TrashStorageTrait {
     }
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function buildQuery($ids, $revision_ids = FALSE) {
+    $query = parent::buildQuery($ids, $revision_ids);
+
+    if ($this->getTrashManager()->getTrashContext() !== 'active') {
+      return $query;
+    }
+
+    $table_mapping = $this->getTableMapping();
+    $deleted_column = $table_mapping->getFieldColumnName($this->fieldStorageDefinitions['deleted'], 'value');
+
+    // Ensure that entity_load excludes deleted entities.
+    if ($data_table = $this->getDataTable()) {
+      $query->join($data_table, 'data', "[data].[{$this->idKey}] = [base].[{$this->idKey}]");
+      $query->condition("data.$deleted_column", NULL, 'IS NULL');
+    }
+    else {
+      $query->condition("base.$deleted_column", NULL, 'IS NULL');
+    }
+
+    return $query;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setPersistentCache($entities) {
+    if (!$this->entityType->isPersistentlyCacheable()) {
+      return;
+    }
+
+    // Ensure that deleted entities are never stored in the persistent cache.
+    foreach ($entities as $id => $entity) {
+      if (trash_entity_is_deleted($entity)) {
+        unset($entities[$id]);
+      }
+    }
+
+    parent::setPersistentCache($entities);
+  }
+
   /**
    * {@inheritdoc}
    */

tests/src/Kernel/EntityQueryTest.php

@@ -23,21 +23,25 @@ class EntityQueryTest extends TrashKernelTestBase {
     // Test whether they appear in an entity query.
     $this->assertCount(5, $entity_storage->getQuery()->accessCheck(FALSE)->execute());
 
-    // Delete the first three of them. They should all be individual loadable
-    // but no longer accessible via the entity query.
+    // Delete the first three of them. They should no longer accessible via the
+    // entity query.
     for ($i = 0; $i < 3; $i++) {
       $entities[$i]->delete();
-
-      $this->assertNotNull(TrashTest::load($entities[$i]->id()));
     }
     $this->assertCount(2, $entity_storage->getQuery()->accessCheck(FALSE)->execute());
+
+    // Check that deleted entities can still be retrieved by an entity query if
+    // the trash context is disabled.
+    $result = $this->getTrashManager()->executeInTrashContext('ignore', function () use ($entity_storage) {
+      return $entity_storage->getQuery()->accessCheck(FALSE)->execute();
+    });
+    $this->assertCount(5, $result);
   }
 
   public function testQueryWithDeletedAccess() {
     $entities = [];
     $entity_type_manager = \Drupal::entityTypeManager();
     $entity_storage = $entity_type_manager->getStorage('trash_test_entity');
-    $entity_type = $entity_type_manager->getDefinition('trash_test_entity');
 
     for ($i = 0; $i < 5; $i++) {
       $entity = TrashTest::create();

tests/src/Kernel/TrashKernelTest.php

@@ -11,20 +11,45 @@ use Drupal\trash_test\Entity\TrashTest;
 class TrashKernelTest extends TrashKernelTestBase {
 
   public function testDeletion() {
-    $entity = TrashTest::create([
-    ]);
+    $entity = TrashTest::create();
     assert($entity instanceof ContentEntityInterface);
     $entity->save();
+    $entity_id = $entity->id();
+
+    $this->assertNotNull(TrashTest::load($entity_id));
+    $this->assertTrue($entity->get('deleted')->isEmpty());
+    $this->assertNull($entity->get('deleted')->value);
 
-    $this->assertNotNull(TrashTest::load($entity->id()));
-    $this->assertEquals(0, $entity->get('deleted')->value);
     $entity->delete();
 
-    $entity = TrashTest::load($entity->id());
-    assert($entity instanceof ContentEntityInterface);
+    // Test the default 'active' trash context.
+    $entity = TrashTest::load($entity_id);
+    $this->assertNull($entity, 'Deleted entities can not be loaded in the default (active) trash context.');
 
-    $this->assertNotNull($entity, "Ensure that deleting an entity doesn't actually deletes it.");
+    // Test the 'ignore' trash context.
+    $entity = $this->getTrashManager()->executeInTrashContext('ignore', function () use ($entity_id) {
+      return TrashTest::load($entity_id);
+    });
+    assert($entity instanceof ContentEntityInterface);
+    $this->assertNotNull($entity, 'Deleted entities can still be loaded in the "ignore" trash context.');
     $this->assertEquals(\Drupal::time()->getRequestTime(), $entity->get('deleted')->value);
+
+    $second_entity = TrashTest::create();
+    $second_entity->save();
+    $second_entity_id = $second_entity->id();
+
+    // Test the default 'active' trash context for multiple load.
+    $entities = TrashTest::loadMultiple();
+    $this->assertCount(1, $entities);
+    $this->assertEquals($second_entity_id, $entities[$second_entity_id]->id());
+
+    // Test the 'ignore' trash context  for multiple load.
+    $entities = $this->getTrashManager()->executeInTrashContext('ignore', function () {
+      return TrashTest::loadMultiple();
+    });
+    $this->assertCount(2, $entities);
+    $this->assertEquals($entity_id, $entities[$entity_id]->id());
+    $this->assertEquals($second_entity_id, $entities[$second_entity_id]->id());
   }
 
 }

tests/src/Kernel/TrashKernelTestBase.php

@@ -3,6 +3,7 @@
 namespace Drupal\Tests\trash\Kernel;
 
 use Drupal\KernelTests\KernelTestBase;
+use Drupal\trash\TrashManagerInterface;
 
 abstract class TrashKernelTestBase extends KernelTestBase {
 
@@ -27,4 +28,11 @@ abstract class TrashKernelTestBase extends KernelTestBase {
     $config->save();
   }
 
+  /**
+   * Gets the trash manager.
+   */
+  protected function getTrashManager(): TrashManagerInterface {
+    return \Drupal::service('trash.manager');
+  }
+
 }

tests/src/Kernel/ViewQueryTest.php

@@ -39,12 +39,9 @@ class ViewQueryTest extends TrashKernelTestBase {
     ], ['id' => 'id']);
     $view->destroy();
 
-    // Delete the first three of them. They should all be individual loadable
-    // but not longer accessible via the view.
+    // Delete the first three of them. They should no longer appear in the view.
     for ($i = 0; $i < 3; $i++) {
       $entities[$i]->delete();
-
-      $this->assertNotNull(TrashTest::load($entities[$i]->id()));
     }
 
     $view = Views::getView('trash_test_view');