Merge pull request #4 from discoverygarden/main

Sync with main

.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"
+          }
+        }

README.md

@@ -19,25 +19,50 @@ See [the module's docs for more info](modules/migrate_embargoes_to_embargo/READM
 Configuration options can be set at `admin/config/content/embargo`,
 including a contact email and notification message.
 
-Embargoes can be managed at `admin/content/embargo`. 
+Embargoes can be managed at `admin/content/embargo`.
 
 To add an IP range for use on embargoes, navigate to
 `admin/content/embargo/range` and click 'Add IP range'. Ranges
 created via this method can then be used as IP address whitelists when creating
-embargoes. This [CIDR to IPv4 Conversion utility](https://www.ipaddressguide.com/cidr) 
+embargoes. This [CIDR to IPv4 Conversion utility](https://www.ipaddressguide.com/cidr)
 can be helpful in creating valid CIDR IP ranges.
 
+### `search_api` processor(s)
+
+We have multiple `search_api` processors which attempt to constrain search
+results based on the effects of embargoes on the entities represented by search
+results, including:
+
+- `embargo_processor` ("Embargo access (deprecated)")
+  - Adds additional properties to the indexed rows, requiring additional index
+    maintenance on mutation of the entities under consideration, but should
+    theoretically work with any `search_api` backend
+- `embargo_join_process` ("Embargo access, join-wise")
+  - Requires Solr/Solarium-compatible index, and indexing of embargo entities in
+    the same index as the node/media/files to be search, tracking necessary info
+    and performing
+    [Solr joins](https://solr.apache.org/guide/solr/latest/query-guide/join-query-parser.html)
+    to constrain results
+
+Typically, only one should be used in any particular index.
+
 ## Usage
 
 ### Applying an embargo
 
-An embargo can be applied to an existing node by clicking the 
-"Embargoes" tab on a node, or navigating to 
+An embargo can be applied to an existing node by clicking the
+"Embargoes" tab on a node, or navigating to
 `embargoes/node/{node_id}`. From here, an embargo can be applied if it doesn't
 already exist, and existing embargoes can be modified or removed.
 
-## Known Issues
-Embargoed items may show up in search results. To work around this at a cost to performance you can enable access checking in your search views.
+## Known Issues/FAQ
+
+- Embargoed items show up in search results
+  - Enable one of our `search_api` processors to handle applying embargo restrictions.
+- "Embargo access, join-wise" does not show up as an available processor
+  - Ensure embargo entities are being indexed in the given index.
+  - Ensure that eligible node/media/files entities are being indexed in the
+    given index.
 
 ## Troubleshooting/Issues
 

embargo.module

@@ -5,15 +5,18 @@
  * Hook implementations.
  */
 
+use Drupal\Core\Access\AccessResultInterface;
 use Drupal\Core\Database\Query\AlterableInterface;
 use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Session\AccountInterface;
 use Drupal\embargo\EmbargoStorage;
+use Drupal\media\MediaInterface;
+use Drupal\node\NodeInterface;
 
 /**
  * Implements hook_entity_type_alter().
  */
-function embargo_entity_type_alter(array &$entity_types) {
+function embargo_entity_type_alter(array &$entity_types) : void {
   $applicable_entity_types = EmbargoStorage::applicableEntityTypes();
   foreach ($applicable_entity_types as $entity_type_id) {
     $entity_type = &$entity_types[$entity_type_id];
@@ -24,7 +27,7 @@ function embargo_entity_type_alter(array &$entity_types) {
 /**
  * Implements hook_entity_access().
  */
-function embargo_entity_access(EntityInterface $entity, $operation, AccountInterface $account) {
+function embargo_entity_access(EntityInterface $entity, $operation, AccountInterface $account) : AccessResultInterface {
   /** @var \Drupal\embargo\Access\EmbargoAccessCheckInterface $service */
   $service = \Drupal::service('access_check.embargo');
   return $service->access($entity, $account);
@@ -33,7 +36,7 @@ function embargo_entity_access(EntityInterface $entity, $operation, AccountInter
 /**
  * Implements hook_file_download().
  */
-function embargo_file_download($uri) {
+function embargo_file_download($uri) : null|array|int {
   $files = \Drupal::entityTypeManager()
     ->getStorage('file')
     ->loadByProperties(['uri' => $uri]);
@@ -44,39 +47,23 @@ function embargo_file_download($uri) {
       return -1;
     }
   }
-}
 
-/**
- * Implements hook_query_TAG_alter() for `node_access` tagged queries.
- */
-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');
+  return NULL;
 }
 
 /**
- * Implements hook_query_TAG_alter() for `file_access` tagged queries.
+ * Implements hook_query_TAG_alter() for `node_access` tagged queries.
  */
-function embargo_query_file_access_alter(AlterableInterface $query) {
+function embargo_query_node_access_alter(AlterableInterface $query) : void {
   /** @var \Drupal\embargo\Access\QueryTagger $tagger */
   $tagger = \Drupal::service('embargo.query_tagger');
-  $tagger->tagAccess($query, 'file');
+  $tagger->tagNode($query);
 }
 
 /**
  * Implements hook_theme().
  */
-function embargo_theme($existing, $type, $theme, $path) {
+function embargo_theme($existing, $type, $theme, $path) : array {
   return [
     'embargo_ip_access_exemption' => [
       'template' => 'embargo-ip-access-exemption',
@@ -97,3 +84,73 @@ function embargo_theme($existing, $type, $theme, $path) {
     ],
   ];
 }
+
+/**
+ * Implements hook_ENTITY_TYPE_insert() for embargo entities.
+ */
+function embargo_embargo_insert(EntityInterface $entity) : void {
+  /** @var \Drupal\embargo\SearchApiTracker $tracker */
+  $tracker = \Drupal::service('embargo.search_api_tracker_helper');
+  $tracker->track($entity);
+}
+
+/**
+ * Implements hook_ENTITY_TYPE_update() for embargo entities.
+ */
+function embargo_embargo_update(EntityInterface $entity) : void {
+  /** @var \Drupal\embargo\SearchApiTracker $tracker */
+  $tracker = \Drupal::service('embargo.search_api_tracker_helper');
+  $tracker->track($entity);
+}
+
+/**
+ * Implements hook_ENTITY_TYPE_delete() for embargo entities.
+ */
+function embargo_embargo_delete(EntityInterface $entity) : void {
+  /** @var \Drupal\embargo\SearchApiTracker $tracker */
+  $tracker = \Drupal::service('embargo.search_api_tracker_helper');
+  $tracker->track($entity);
+}
+
+/**
+ * Implements hook_ENTITY_TYPE_delete() for node entities.
+ */
+function embargo_node_delete(EntityInterface $entity) : void {
+  assert($entity instanceof NodeInterface);
+  /** @var \Drupal\embargo\SearchApiTracker $tracker */
+  $tracker = \Drupal::service('embargo.search_api_tracker_helper');
+  $tracker->propagateChildren($entity);
+}
+
+/**
+ * Implements hook_ENTITY_TYPE_insert() for media entities.
+ */
+function embargo_media_insert(EntityInterface $entity) : void {
+  assert($entity instanceof MediaInterface);
+
+  /** @var \Drupal\embargo\SearchApiTracker $tracker */
+  $tracker = \Drupal::service('embargo.search_api_tracker_helper');
+  $tracker->mediaWriteReaction($entity);
+}
+
+/**
+ * Implements hook_ENTITY_TYPE_update() for media entities.
+ */
+function embargo_media_update(EntityInterface $entity) : void {
+  assert($entity instanceof MediaInterface);
+
+  /** @var \Drupal\embargo\SearchApiTracker $tracker */
+  $tracker = \Drupal::service('embargo.search_api_tracker_helper');
+  $tracker->mediaWriteReaction($entity);
+}
+
+/**
+ * Implements hook_ENTITY_TYPE_delete() for media entities.
+ */
+function embargo_media_delete(EntityInterface $entity) : void {
+  assert($entity instanceof MediaInterface);
+
+  /** @var \Drupal\embargo\SearchApiTracker $tracker */
+  $tracker = \Drupal::service('embargo.search_api_tracker_helper');
+  $tracker->mediaDeleteReaction($entity);
+}

embargo.services.yml

@@ -18,11 +18,12 @@ services:
       - '@entity_type.manager'
       - '@datetime.time'
       - '@date.formatter'
+      - '@event_dispatcher'
   embargo.route_subscriber:
     class: Drupal\embargo\Routing\EmbargoRouteSubscriber
     arguments: ['@entity_type.manager']
     tags:
-      - { name: event_subscriber }
+      - { name: 'event_subscriber' }
   embargo.ip_range_redirect:
     class: '\Drupal\embargo\EventSubscriber\IpRangeRedirect'
     arguments:
@@ -31,3 +32,40 @@ 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' }
+  embargo.search_api_tracker_helper:
+    class: Drupal\embargo\SearchApiTracker
+    factory: [null, 'create']
+    arguments:
+      - '@service_container'
+  embargo.search_api_solr_join_processor_event_subscriber:
+    class: Drupal\embargo\EventSubscriber\EmbargoJoinProcessorEventSubscriber
+    factory: [null, 'create']
+    arguments:
+      - '@service_container'
+    tags:
+      - { name: 'event_subscriber' }
+  cache_context.ip.embargo_range:
+    class: Drupal\embargo\Cache\Context\IpRangeCacheContext
+    arguments:
+      - '@request_stack'
+      - '@entity_type.manager'
+    tags:
+      - { name: 'cache.context' }
+  embargo.tagging_event_subscriber:
+    class: Drupal\embargo\EventSubscriber\TaggingEventSubscriber
+    tags:
+      - { name: 'event_subscriber' }
+  cache_context.user.embargo__has_exemption:
+    class: Drupal\embargo\Cache\Context\UserExemptedCacheContext
+    arguments:
+      - '@current_user'
+      - '@entity_type.manager'
+    tags:
+      - { name: 'cache.context' }

embargo.views.inc

@@ -0,0 +1,39 @@
+<?php
+
+/**
+ * @file
+ * Views hook implementations.
+ */
+
+/**
+ * Implements hook_views_data_alter().
+ */
+function embargo_views_data_alter(array &$data) {
+
+  $data['node_field_data']['embargo__embargoes'] = [
+    'title' => \t('Embargoes'),
+    'help' => \t('Embargoes applicable to the given node.'),
+    'relationship' => [
+      'base' => 'embargo',
+      'base field' => 'embargoed_node',
+      'field' => 'nid',
+      'id' => 'standard',
+      'label' => \t('Embargoes'),
+    ],
+  ];
+  $data['users_field_data']['embargo__exempt_users'] = [
+    'title' => \t('Embargo exemptions'),
+    'help' => \t('Embargoes for which the given user is specifically exempt.'),
+    'relationship' => [
+      'id' => 'entity_reverse',
+      'field_name' => 'embargo__exempt_users',
+      'entity_type' => 'embargo',
+      'field table' => 'embargo__exempt_users',
+      'field field' => 'exempt_users_target_id',
+      'base' => 'embargo',
+      'base field' => 'id',
+      'label' => \t('Embargo exemptions'),
+    ],
+  ];
+
+}

modules/migrate_embargoes_to_embargo/src/Plugin/migrate/source/Entity.php

@@ -2,13 +2,11 @@
 
 namespace Drupal\migrate_embargoes_to_embargo\Plugin\migrate\source;
 
-use Drupal\migrate\Plugin\migrate\source\SourcePluginBase;
-use Drupal\migrate\Plugin\MigrationInterface;
-
-use Drupal\Core\Entity\EntityTypeManagerInterface;
 use Drupal\Core\Entity\EntityTypeInterface;
+use Drupal\Core\Entity\EntityTypeManagerInterface;
 use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
-
+use Drupal\migrate\Plugin\migrate\source\SourcePluginBase;
+use Drupal\migrate\Plugin\MigrationInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
@@ -42,7 +40,7 @@ public function __construct(
     $plugin_id,
     $plugin_definition,
     MigrationInterface $migration,
-    EntityTypeManagerInterface $entity_type_manager
+    EntityTypeManagerInterface $entity_type_manager,
   ) {
     parent::__construct($configuration, $plugin_id, $plugin_definition, $migration);
 
@@ -58,7 +56,7 @@ public static function create(
     array $configuration,
     $plugin_id,
     $plugin_definition,
-    MigrationInterface $migration = NULL
+    MigrationInterface $migration = NULL,
   ) {
     return new static(
       $configuration,

src/Access/EmbargoAccessCheck.php

@@ -57,24 +57,40 @@ public function __construct(EntityTypeManagerInterface $entity_type_manager, Req
    * {@inheritdoc}
    */
   public function access(EntityInterface $entity, AccountInterface $user) {
+    $type = $this->entityTypeManager->getDefinition('embargo');
+    $state = AccessResult::neutral();
+
+    if ($user->hasPermission('bypass embargo access')) {
+      return $state->setReason('User has embargo bypass permission.')
+        ->addCacheContexts(['user.permissions']);
+    }
+
     /** @var \Drupal\embargo\EmbargoStorage $storage */
     $storage = $this->entityTypeManager->getStorage('embargo');
+    $state->addCacheTags($type->getListCacheTags())
+      ->addCacheContexts($type->getListCacheContexts());
+    $related_embargoes = $storage->getApplicableEmbargoes($entity);
+    if (empty($related_embargoes)) {
+      return $state->setReason('No embargo statements for the given entity.');
+    }
+
+    array_map([$state, 'addCacheableDependency'], $related_embargoes);
+
     $embargoes = $storage->getApplicableNonExemptNonExpiredEmbargoes(
       $entity,
       $this->request->server->get('REQUEST_TIME'),
       $user,
       $this->request->getClientIp()
     );
-    $state = AccessResult::forbiddenIf(
+    return $state->andIf(AccessResult::forbiddenIf(
       !empty($embargoes),
       $this->formatPlural(
         count($embargoes),
         '1 embargo preventing access.',
         '@count embargoes preventing access.'
       )->render()
-    );
-    array_map([$state, 'addCacheableDependency'], $embargoes);
-    return $state;
+    ));
+
   }
 
 }