Throw an exception when calling MembershipManager::getGroupIds() with an entity which is not group content

src/MembershipManager.php

@@ -214,6 +214,11 @@ public function getGroupIds(EntityInterface $entity, $group_type_id = NULL, $gro
       throw new \InvalidArgumentException('\Drupal\og\MembershipManager::getGroupIds() cannot be used for user entities. Use \Drupal\og\MembershipManager::getUserGroups() instead.');
     }
 
+    // This should only be called on group content types.
+    if (!$this->groupAudienceHelper->hasGroupAudienceField($entity->getEntityTypeId(), $entity->bundle())) {
+      throw new \InvalidArgumentException('Can only retrieve group IDs for group content entities.');
+    }
+
     $cid = [
       __METHOD__,
       $entity->getEntityTypeId(),

src/OgAccess.php

@@ -262,36 +262,37 @@ public function userAccessEntity($operation, EntityInterface $entity, AccountInt
     }
 
     $is_group_content = $this->groupAudienceHelper->hasGroupAudienceField($entity_type_id, $bundle);
-    $cache_tags = $entity_type->getListCacheTags();
-
-    // The entity might be a user or a non-user entity.
-    $groups = $entity->getEntityTypeId() == 'user' ? $this->membershipManager->getUserGroups($entity) : $this->membershipManager->getGroups($entity);
-
-    if ($is_group_content && $groups) {
-      $forbidden = AccessResult::forbidden()->addCacheTags($cache_tags);
-      foreach ($groups as $entity_groups) {
-        foreach ($entity_groups as $group) {
-          // Check if the operation matches a group content entity operation
-          // such as 'create article content'.
-          $operation_access = $this->userAccessGroupContentEntityOperation($operation, $group, $entity, $user);
-
-          if ($operation_access->isAllowed()) {
-            return $operation_access->addCacheTags($cache_tags);
-          }
-
-          // Check if the operation matches a group level operation such as
-          // 'subscribe without approval'.
-          $user_access = $this->userAccess($group, $operation, $user);
-          if ($user_access->isAllowed()) {
-            return $user_access->addCacheTags($cache_tags);
+    if ($is_group_content) {
+      $cache_tags = $entity_type->getListCacheTags();
+
+      // The entity might be a user or a non-user entity.
+      $groups = $entity->getEntityTypeId() == 'user' ? $this->membershipManager->getUserGroups($entity) : $this->membershipManager->getGroups($entity);
+
+      if ($groups) {
+        $forbidden = AccessResult::forbidden()->addCacheTags($cache_tags);
+        foreach ($groups as $entity_groups) {
+          foreach ($entity_groups as $group) {
+            // Check if the operation matches a group content entity operation
+            // such as 'create article content'.
+            $operation_access = $this->userAccessGroupContentEntityOperation($operation, $group, $entity, $user);
+
+            if ($operation_access->isAllowed()) {
+              return $operation_access->addCacheTags($cache_tags);
+            }
+
+            // Check if the operation matches a group level operation such as
+            // 'subscribe without approval'.
+            $user_access = $this->userAccess($group, $operation, $user);
+            if ($user_access->isAllowed()) {
+              return $user_access->addCacheTags($cache_tags);
+            }
+
+            $forbidden->inheritCacheability($user_access);
           }
-
-          $forbidden->inheritCacheability($user_access);
         }
+        return $forbidden;
       }
-      return $forbidden;
-    }
-    if ($is_group_content) {
+
       $result->addCacheTags($cache_tags);
     }
 

src/OgDeleteOrphansBase.php

@@ -39,6 +39,13 @@ abstract class OgDeleteOrphansBase extends PluginBase implements OgDeleteOrphans
    */
   protected $membershipManager;
 
+  /**
+   * The OG group audience helper.
+   *
+   * @var \Drupal\og\OgGroupAudienceHelperInterface
+   */
+  protected $groupAudienceHelper;
+
   /**
    * Constructs an OgDeleteOrphansBase object.
    *
@@ -54,12 +61,15 @@ abstract class OgDeleteOrphansBase extends PluginBase implements OgDeleteOrphans
    *   The queue factory.
    * @param \Drupal\og\MembershipManagerInterface $membership_manager
    *   The OG membership manager service.
+   * @param \Drupal\og\OgGroupAudienceHelperInterface $group_audience_helper
+   *   The OG group audience helper.
    */
-  public function __construct(array $configuration, $plugin_id, $plugin_definition, EntityTypeManagerInterface $entity_type_manager, QueueFactory $queue_factory, MembershipManagerInterface $membership_manager) {
+  public function __construct(array $configuration, $plugin_id, $plugin_definition, EntityTypeManagerInterface $entity_type_manager, QueueFactory $queue_factory, MembershipManagerInterface $membership_manager, OgGroupAudienceHelperInterface $group_audience_helper) {
     parent::__construct($configuration, $plugin_id, $plugin_definition);
     $this->entityTypeManager = $entity_type_manager;
     $this->queueFactory = $queue_factory;
     $this->membershipManager = $membership_manager;
+    $this->groupAudienceHelper = $group_audience_helper;
   }
 
   /**
@@ -72,7 +82,8 @@ public static function create(ContainerInterface $container, array $configuratio
       $plugin_definition,
       $container->get('entity_type.manager'),
       $container->get('queue'),
-      $container->get('og.membership_manager')
+      $container->get('og.membership_manager'),
+      $container->get('og.group_audience_helper')
     );
   }
 
@@ -135,10 +146,18 @@ protected function deleteOrphan($entity_type, $entity_id) {
       return;
     }
 
-    // Only delete content that is fully orphaned, i.e. it is no longer
+    // Only delete group content that is fully orphaned, i.e. it is no longer
     // associated with any groups.
-    $group_count = $this->membershipManager->getGroupCount($entity);
-    if ($group_count == 0) {
+    if ($this->groupAudienceHelper->hasGroupAudienceField($entity->getEntityTypeId(), $entity->bundle())) {
+      // Only do a group count if the entity is actually group content.
+      $group_count = $this->membershipManager->getGroupCount($entity);
+      if ($group_count == 0) {
+        $entity->delete();
+      }
+    }
+    // If the entity is not group content (e.g. an OgMembership entity), just go
+    // ahead and delete it.
+    else {
       $entity->delete();
     }
   }

tests/src/Kernel/Entity/GroupMembershipManagerTest.php

@@ -192,6 +192,35 @@ public function testGetGroupIds($group_type_id, $group_bundle, array $expected)
     }
   }
 
+  /**
+   * Tests that exceptions are thrown when invalid arguments are passed.
+   *
+   * @covers ::getGroupIds
+   * @dataProvider groupContentProvider
+   */
+  public function testGetGroupIdsInvalidArguments() {
+    /** @var \Drupal\og\MembershipManagerInterface $membership_manager */
+    $membership_manager = \Drupal::service('og.membership_manager');
+
+    $test_cases = [
+      // Test that an exception is thrown when passing a User entity.
+      User::create(),
+      // Test that an exception is thrown when passing an entity which is not
+      // group content. We are using one of the test groups for this.
+      $this->groups['node'][0],
+    ];
+
+    foreach ($test_cases as $test_case) {
+      try {
+        $membership_manager->getGroupIds($test_case);
+        $this->fail();
+      }
+      catch (\InvalidArgumentException $e) {
+        // Expected result.
+      }
+    }
+  }
+
   /**
    * Tests that the static cache loads the appropriate group.
    *