Merge pull request #31 from discoverygarden/fix/per-26

PER-26: Rework querying.

.github/workflows/phpunit.yml

@@ -10,3 +10,10 @@ jobs:
   PHPUnit:
     uses: discoverygarden/phpunit-action/.github/workflows/phpunit.yml@v1
     secrets: inherit
+    with:
+      composer_patches: |-
+        {
+          "discoverygarden/islandora_hierarchical_access": {
+            "dependent work from dependency": "https://github.com/discoverygarden/islandora_hierarchical_access/pull/19.patch"
+          }
+        }

embargo.module

@@ -55,25 +55,7 @@ function embargo_file_download($uri) {
 function embargo_query_node_access_alter(AlterableInterface $query) {
   /** @var \Drupal\embargo\Access\QueryTagger $tagger */
   $tagger = \Drupal::service('embargo.query_tagger');
-  $tagger->tagAccess($query, 'node');
-}
-
-/**
- * Implements hook_query_TAG_alter() for `media_access` tagged queries.
- */
-function embargo_query_media_access_alter(AlterableInterface $query) {
-  /** @var \Drupal\embargo\Access\QueryTagger $tagger */
-  $tagger = \Drupal::service('embargo.query_tagger');
-  $tagger->tagAccess($query, 'media');
-}
-
-/**
- * Implements hook_query_TAG_alter() for `file_access` tagged queries.
- */
-function embargo_query_file_access_alter(AlterableInterface $query) {
-  /** @var \Drupal\embargo\Access\QueryTagger $tagger */
-  $tagger = \Drupal::service('embargo.query_tagger');
-  $tagger->tagAccess($query, 'file');
+  $tagger->tagNode($query);
 }
 
 /**

embargo.services.yml

@@ -31,3 +31,10 @@ services:
       - '@url_generator'
     tags:
       - { name: 'event_subscriber' }
+  embargo.event_subscriber.islandora_hierarchical_access:
+    class: Drupal\embargo\EventSubscriber\IslandoraHierarchicalAccessEventSubscriber
+    factory: [null, 'create']
+    arguments:
+      - '@service_container'
+    tags:
+      - { name: 'event_subscriber' }

src/Access/QueryTagger.php

@@ -8,60 +8,20 @@
 use Drupal\Core\Datetime\DateFormatterInterface;
 use Drupal\Core\Entity\EntityTypeManagerInterface;
 use Drupal\Core\Session\AccountProxyInterface;
-use Drupal\datetime\Plugin\Field\FieldType\DateTimeItemInterface;
+use Drupal\embargo\EmbargoExistenceQueryTrait;
 use Drupal\embargo\EmbargoInterface;
 use Drupal\islandora_hierarchical_access\Access\QueryConjunctionTrait;
-use Drupal\islandora_hierarchical_access\LUTGeneratorInterface;
+use Drupal\islandora_hierarchical_access\TaggedTargetsTrait;
 use Symfony\Component\HttpFoundation\RequestStack;
 
 /**
  * Handles tagging entity queries with access restrictions for embargoes.
  */
 class QueryTagger {
 
+  use EmbargoExistenceQueryTrait;
   use QueryConjunctionTrait;
-
-  /**
-   * The current user.
-   *
-   * @var \Drupal\Core\Session\AccountProxyInterface
-   */
-  protected $user;
-
-  /**
-   * The IP of the request.
-   *
-   * @var string
-   */
-  protected $currentIp;
-
-  /**
-   * Instance of a Drupal database connection.
-   *
-   * @var \Drupal\Core\Database\Connection
-   */
-  protected $database;
-
-  /**
-   * The entity type manager.
-   *
-   * @var \Drupal\Core\Entity\EntityTypeManagerInterface
-   */
-  protected $entityTypeManager;
-
-  /**
-   * Time service.
-   *
-   * @var \Drupal\Component\Datetime\TimeInterface
-   */
-  protected TimeInterface $time;
-
-  /**
-   * Date formatter service.
-   *
-   * @var \Drupal\Core\Datetime\DateFormatterInterface
-   */
-  protected DateFormatterInterface $dateFormatter;
+  use TaggedTargetsTrait;
 
   /**
    * Constructor.
@@ -87,127 +47,30 @@ public function __construct(
    *
    * @param \Drupal\Core\Database\Query\SelectInterface $query
    *   The query being executed.
-   * @param string $type
-   *   Either "node" or "file".
    */
-  public function tagAccess(SelectInterface $query, string $type) {
-    if (!in_array($type, ['node', 'media', 'file'])) {
-      throw new \InvalidArgumentException("Unrecognized type '$type'.");
+  public function tagNode(SelectInterface $query) : void {
+    if ($query->hasTag('islandora_hierarchical_access_subquery')) {
+      // Being run as a subquery: We do not want to touch it as we expect our
+      // IslandoraHierarchicalAccessEventSubscriber class to deal with it.
+      return;
     }
-    elseif ($this->user->hasPermission('bypass embargo access')) {
+    if ($this->user->hasPermission('bypass embargo access')) {
       return;
     }
+    $type = 'node';
 
     static::conjunctionQuery($query);
 
-    /** @var \Drupal\Core\Entity\Sql\SqlEntityStorageInterface $storage */
-    $storage = $this->entityTypeManager->getStorage($type);
-    $tables = $storage->getTableMapping()->getTableNames();
+    $tagged_table_aliases = $query->getMetaData('embargo_tagged_table_aliases') ?? [];
 
-    foreach ($query->getTables() as $info) {
-      if ($info['table'] instanceof SelectInterface) {
-        continue;
-      }
-      elseif (in_array($info['table'], $tables)) {
-        $key = (strpos($info['table'], "{$type}__") === 0) ? 'entity_id' : (substr($type, 0, 1) . "id");
-        $alias = $info['alias'];
+    $target_aliases = $this->getTaggingTargets($query, $tagged_table_aliases, $type);
 
-        $to_apply = $query;
-        if ($info['join type'] == 'LEFT') {
-          $to_apply = $query->orConditionGroup()
-            ->isNull("{$alias}.{$key}");
-          $query->condition($to_apply);
-        }
-        if ($type === 'node') {
-          $to_apply->condition("{$alias}.{$key}", $this->buildInaccessibleEmbargoesCondition(), 'NOT IN');
-        }
-        elseif ($type === 'media') {
-          $to_apply->condition("{$alias}.{$key}", $this->buildInaccessibleFileCondition('mid'), 'NOT IN');
-        }
-        elseif ($type === 'file') {
-          $to_apply->condition("{$alias}.{$key}", $this->buildInaccessibleFileCondition('fid'), 'NOT IN');
-        }
-        else {
-          throw new \InvalidArgumentException("Invalid type '$type'.");
-        }
-      }
-    }
-  }
-
-  /**
-   * Builds the condition for file-typed embargoes that are inaccessible.
-   *
-   * @param string $lut_column
-   *   The particular column of the LUT to return, as file embargoes apply to
-   *   media ('mid') as well as files ('fid').
-   *
-   * @return \Drupal\Core\Database\Query\SelectInterface
-   *   The sub-query to be used that results in all file IDs that cannot be
-   *   accessed.
-   */
-  protected function buildInaccessibleFileCondition(string $lut_column) {
-    $query = $this->database->select('embargo', 'e');
-    $lut_alias = $query->join(LUTGeneratorInterface::TABLE_NAME, 'lut', '%alias.nid = e.embargoed_node');
-    return $query
-      ->fields($lut_alias, [$lut_column])
-      ->condition('lut.nid', $this->buildAccessibleEmbargoesQuery(EmbargoInterface::EMBARGO_TYPE_FILE), 'NOT IN');
-  }
-
-  /**
-   * Get query to select accessible embargoed entities.
-   *
-   * @param int $type
-   *   The type of embargo, expected to be one of:
-   *   - EmbargoInterface::EMBARGO_TYPE_NODE; or,
-   *   - EmbargoInterface::EMBARGO_TYPE_FILE.
-   *
-   * @return \Drupal\Core\Database\Query\SelectInterface
-   *   A query returning things that should not be inaccessible.
-   */
-  protected function buildAccessibleEmbargoesQuery($type) : SelectInterface {
-    $query = $this->database->select('embargo', 'e')
-      ->fields('e', ['embargoed_node']);
-
-    // Things are visible if...
-    $group = $query->orConditionGroup()
-      // The selected embargo entity does not apply to the given type; or...
-      ->condition('e.embargo_type', $type, '!=');
-
-    $group->condition($query->andConditionGroup()
-      // ... a scheduled embargo...
-      ->condition('e.expiration_type', EmbargoInterface::EXPIRATION_TYPE_SCHEDULED)
-      // ... has a date in the past.
-      ->condition('e.expiration_date', $this->dateFormatter->format($this->time->getRequestTime(), 'custom', DateTimeItemInterface::DATE_STORAGE_FORMAT), '<')
-    );
-
-    // ... the incoming IP is in an exempt range; or...
-    /** @var \Drupal\embargo\IpRangeStorageInterface $storage */
-    $storage = $this->entityTypeManager->getStorage('embargo_ip_range');
-    $applicable_ip_ranges = $storage->getApplicableIpRanges($this->currentIp);
-    if (!empty($applicable_ip_ranges)) {
-      $group->condition('e.exempt_ips', array_keys($applicable_ip_ranges), 'IN');
+    if (empty($target_aliases)) {
+      return;
     }
 
-    // ... the specific user is exempted from the embargo.
-    $user_alias = $query->leftJoin('embargo__exempt_users', 'u', 'e.id = %alias.entity_id');
-    $group->condition("{$user_alias}.exempt_users_target_id", $this->user->id());
-
-    $query->condition($group);
-
-    return $query;
-  }
-
-  /**
-   * Builds the condition for embargoes that are inaccessible.
-   *
-   * @return \Drupal\Core\Database\Query\SelectInterface
-   *   The sub-query to be used that results in embargoed_node IDs that
-   *   cannot be accessed.
-   */
-  protected function buildInaccessibleEmbargoesCondition() : SelectInterface {
-    return $this->database->select('embargo', 'ein')
-      ->condition('ein.embargoed_node', $this->buildAccessibleEmbargoesQuery(EmbargoInterface::EMBARGO_TYPE_NODE), 'NOT IN')
-      ->fields('ein', ['embargoed_node']);
+    $query->addMetaData('embargo_tagged_table_aliases', $tagged_table_aliases);
+    $this->applyExistenceQuery($query, $target_aliases, [EmbargoInterface::EMBARGO_TYPE_NODE]);
   }
 
 }