Issue #3150732 by Dave Reid: Fixed xmlsitemap_xmlsitemap_index_links() does...

Issue #3150732 by Dave Reid: Fixed xmlsitemap_xmlsitemap_index_links() does not subquery against the xmlsitemap table to only query for un-indexed links.

src/XmlSitemapGenerator.php

@@ -526,17 +526,18 @@ class XmlSitemapGenerator implements XmlSitemapGeneratorInterface {
     }
 
     $info = $context['sandbox']['info'];
-    $entity_type = $this->entityTypeManager->getDefinition($entity_type_id);
 
     $query = $this->entityTypeManager->getStorage($entity_type_id)->getQuery();
-    $query->condition($entity_type->getKey('id'), $context['sandbox']['last_id'], '>');
-    if ($entity_type->hasKey('bundle')) {
-      $query->condition($entity_type->getKey('bundle'), $context['sandbox']['bundles'], 'IN');
+    $query->condition($info['entity keys']['id'], $context['sandbox']['last_id'], '>');
+    if (!empty($info['entity keys']['bundle'])) {
+      $query->condition($info['entity keys']['bundle'], $context['sandbox']['bundles'], 'IN');
     }
-    $query->addTag('xmlsitemap_link_bundle_access');
+
+    // Access for entities is checked individually for the anonymous user
+    // when each item is processed. We can skip the access check for the
+    // query.
+    $query->accessCheck(FALSE);
     $query->addTag('xmlsitemap_rebuild');
-    $query->addMetaData('entity_type_id', $entity_type_id);
-    $query->addMetaData('entity_info', $info);
 
     if (!isset($context['sandbox']['max'])) {
       $count_query = clone $query;
@@ -549,7 +550,7 @@ class XmlSitemapGenerator implements XmlSitemapGeneratorInterface {
     }
 
     // PostgreSQL cannot have the ORDERED BY in the count query.
-    $query->sort($entity_type->getKey('id'));
+    $query->sort($info['entity keys']['id']);
 
     // Get batch limit.
     $limit = $this->config->get('batch_limit');

xmlsitemap.module

@@ -21,7 +21,6 @@ use Drupal\Core\Entity\ContentEntityInterface;
 use Drupal\Core\Entity\ContentEntityTypeInterface;
 use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Entity\EntityTypeInterface;
-use Drupal\Core\Entity\Query\QueryInterface;
 use Drupal\Core\File\FileSystemInterface;
 use Drupal\Core\Form\FormStateInterface;
 use Drupal\Core\Language\LanguageInterface;
@@ -105,7 +104,7 @@ function xmlsitemap_hook_info() {
     'xmlsitemap_sitemap_delete',
     'xmlsitemap_sitemap_link_url_options_alter',
     'query_xmlsitemap_generate_alter',
-    'query_xmlsitemap_link_bundle_access_alter',
+    'query_xmlsitemap_index_links_alter',
     'form_xmlsitemap_sitemap_edit_form_alter',
     'xmlsitemap_rebuild_clear',
   ];
@@ -735,6 +734,11 @@ function xmlsitemap_get_link_info($type = NULL, $reset = FALSE) {
             'id' => $entity_type->getKey('id'),
             'bundle' => $entity_type->getKey('bundle'),
           ],
+          'xmlsitemap' => [
+            // Add in the default callbacks for entity types.
+            'process callback' => $entity_type->get('xmlsitemap')['process callback'] ?? 'xmlsitemap_xmlsitemap_process_entity_links',
+            'rebuild callback' => $entity_type->get('xmlsitemap')['process callback'] ?? 'xmlsitemap_rebuild_batch_fetch',
+          ],
         ];
       }
 
@@ -745,12 +749,6 @@ function xmlsitemap_get_link_info($type = NULL, $reset = FALSE) {
           'base table' => FALSE,
           'bundles' => [],
         ];
-        if (!isset($info['xmlsitemap']['process callback']) && !empty($info['entity keys'])) {
-          $info['xmlsitemap']['process callback'] = 'xmlsitemap_xmlsitemap_process_entity_links';
-        }
-        if (!isset($info['xmlsitemap']['rebuild callback']) && !empty($info['entity keys']['id'])) {
-          $info['xmlsitemap']['rebuild callback'] = 'xmlsitemap_rebuild_batch_fetch';
-        }
         foreach ($info['bundles'] as $bundle => &$bundle_info) {
           $bundle_info += [
             'xmlsitemap' => [],
@@ -837,18 +835,13 @@ function xmlsitemap_get_link_type_indexed_status($entity_type_id, $bundle = '')
 
   try {
     $query = \Drupal::entityQuery($entity_type_id);
-
-    if ($bundle) {
-      $query->condition($entity_type->getKey('bundle'), $bundle, '=');
+    if ($bundle && $entity_type->hasKey('bundle')) {
+      $query->condition($entity_type->getKey('bundle'), $bundle);
     }
-
-    $query->addTag('xmlsitemap_link_bundle_access');
+    // We are only using this for totals, so we can skip the access check.
+    $query->accessCheck(FALSE);
     $query->addTag('xmlsitemap_link_indexed_status');
-    $query->addMetaData('entity_type_id', $entity_type_id);
-    $query->addMetaData('bundle', $bundle);
-    $query->addMetaData('entity_info', $info);
-    $query->count();
-    $status['total'] = $query->execute();
+    $status['total'] = $query->count()->execute();
     return $status;
   }
   catch (\Exception $e) {
@@ -1628,9 +1621,9 @@ function xmlsitemap_xmlsitemap_index_links($limit) {
   $entity_type_manager = \Drupal::entityTypeManager();
   $entity_types = $entity_type_manager->getDefinitions();
   foreach ($entity_types as $entity_type_id => $entity_type) {
-    // Don't try to loop over non-content entity types or with no bundle
-    // defined.
-    if (!xmlsitemap_is_entity_type_supported($entity_type)) {
+    // If an entity type is not supported it will not have link info.
+    $info = xmlsitemap_get_link_info($entity_type_id);
+    if (empty($info)) {
       continue;
     }
 
@@ -1642,16 +1635,41 @@ function xmlsitemap_xmlsitemap_index_links($limit) {
     try {
       $query = $entity_type_manager->getStorage($entity_type_id)->getQuery();
       $query->range(0, $limit);
-      if ($entity_type->hasKey('bundle')) {
-        $bundle_key = $entity_type->getKey('bundle');
-        $query->condition($bundle_key, $bundles, 'IN');
+      if (!empty($info['entity keys']['bundle'])) {
+        $query->condition($info['entity keys']['bundle'], $bundles, 'IN');
+      }
+
+      // Perform a subquery against the xmlsitemap table to ensure that we are
+      // only looking for items that we have not already indexed.
+      $subquery = \Drupal::database()->select('xmlsitemap', 'x');
+      $subquery->addField('x', 'id');
+      $subquery->condition('type', $entity_type_id);
+      // If the storage for this entity type is against a SQL backend, perform
+      // a direct subquery condition to avoid needing to load all the IDs.
+      if ($query instanceof \Drupal\Core\Entity\Query\Sql\Query) {
+        $query->condition($info['entity keys']['id'], $subquery, 'NOT IN');
       }
-      $ids = $query->execute();
+      else {
+        $query->condition($info['entity keys']['id'], $subquery->execute()->fetchCol(), 'NOT IN');
+      }
+
+      // Access for entities is checked individually for the anonymous user
+      // when each item is processed. We can skip the access check for the
+      // query.
+      $query->accessCheck(FALSE);
+      $query->addTag('xmlsitemap_index_links');
+
+      if ($ids = $query->execute()) {
+        // Chunk the array into batch sizes.
+        $chunks = array_chunk($ids, \Drupal::config('xmlsitemap.settings')->get('batch_limit'));
+        foreach ($chunks as $chunk) {
+          $info['xmlsitemap']['process callback']($entity_type_id, $chunk);
+        }
 
-      // Chunk the array into batch sizes.
-      $chunks = array_chunk($ids, \Drupal::config('xmlsitemap.settings')->get('batch_limit'));
-      foreach ($chunks as $chunk) {
-        xmlsitemap_xmlsitemap_process_entity_links($entity_type_id, $chunk);
+        \Drupal::logger('xmlsitemap')->info('Indexed @count new @type items.', [
+          '@count' => count($ids),
+          '@type' => $entity_type_id
+        ]);
       }
     }
     catch (\Exception $e) {
@@ -2011,24 +2029,6 @@ function xmlsitemap_xmlsitemap_sitemap_operations() {
   return $operations;
 }
 
-/**
- * Implements hook_query_TAG_alter().
- *
- * @todo Check if this query alter is still needed.
- */
-function xmlsitemap_query_xmlsitemap_link_bundle_access_alter(AlterableInterface $query) {
-  if ($query instanceof QueryInterface && $entity_type_id = $query->getMetaData('entity_type_id')) {
-    $entity_type = \Drupal::entityTypeManager()->getDefinition($entity_type_id);
-    if ($entity_type->hasKey('bundle')) {
-      $bundle = $query->getMetaData('bundle');
-      if (empty($bundle)) {
-        $bundle = xmlsitemap_get_link_type_enabled_bundles($entity_type_id);
-      }
-      $query->condition($entity_type->getKey('bundle'), $bundle, is_array($bundle) ? 'IN' : '=');
-    }
-  }
-}
-
 /**
  * XML sitemap link type settings callback for frontpage link entity.
  *