Fix empty variants not being deleted on generate and clean up

src/Plugin/simple_sitemap/SitemapGenerator/SitemapGeneratorBase.php

@@ -119,7 +119,6 @@ abstract class SitemapGeneratorBase extends SimplesitemapPluginBase implements S
 
   /**
    * @return bool
-   * @todo: Variant cannot be null
    */
   protected function isDefaultVariant() {
     return $this->sitemapVariant === $this->settings['default_variant'];
@@ -128,7 +127,6 @@ abstract class SitemapGeneratorBase extends SimplesitemapPluginBase implements S
   /**
    * @param array $links
    * @return string
-   * @todo: Variant cannot be null
    */
   abstract protected function getXml(array $links);
 
@@ -146,7 +144,6 @@ abstract class SitemapGeneratorBase extends SimplesitemapPluginBase implements S
    * Returns the sitemap index for all sitemap chunks of this type.
    *
    * @return string
-   * @todo: Variant cannot be null
    */
   protected function getIndexXml(array $chunk_info) {
     $this->writer->openMemory();
@@ -183,12 +180,12 @@ abstract class SitemapGeneratorBase extends SimplesitemapPluginBase implements S
    * @return $this
    */
   public function remove($mode = 'all') {
-    self::removeSitemapVariants($this->sitemapVariant, $mode);
+    self::purgeSitemapVariants($this->sitemapVariant, $mode);
 
     return $this;
   }
 
-  public static function removeSitemapVariants($variants = NULL, $mode = 'all') {
+  public static function purgeSitemapVariants($variants = NULL, $mode = 'all') {
     if (NULL === $variants || !empty((array) $variants)) {
       $delete_query = \Drupal::database()->delete('simple_sitemap');
 
@@ -223,7 +220,6 @@ abstract class SitemapGeneratorBase extends SimplesitemapPluginBase implements S
    * @param array $links
    *   All links with their multilingual versions and settings.
    *
-   * @todo Variant cannot be null
    */
   public function generate(array $links) {
     $highest_id = $this->db->query('SELECT MAX(id) FROM {simple_sitemap}')->fetchField();
@@ -244,7 +240,6 @@ abstract class SitemapGeneratorBase extends SimplesitemapPluginBase implements S
 
   /**
    * @throws \Exception
-   * @todo: Variant cannot be null
    */
   public function generateIndex() {
     if (!empty($chunk_info = $this->getChunkInfo()) && count($chunk_info) > 1) {

src/Queue/QueueWorker.php

@@ -115,7 +115,7 @@ class QueueWorker {
    */
   public function deleteQueue() {
     $this->queue->deleteQueue();
-    SitemapGeneratorBase::removeSitemapVariants(NULL, 'unpublished'); //todo should call the remove() method of every plugin instead?
+    SitemapGeneratorBase::purgeSitemapVariants(NULL, 'unpublished');
     $this->variantProcessedNow = NULL;
     $this->generatorProcessedNow = NULL;
     $this->results = [];
@@ -136,7 +136,7 @@ class QueueWorker {
    * @todo Lock functionality
    */
   public function rebuildQueue($variants = NULL) {
-    $data_sets = [];
+    $all_data_sets = [];
     $sitemap_variants = $this->manager->getSitemapVariants();
     $this->moduleHandler->alter('simple_sitemap_variants', $sitemap_variants);
 
@@ -155,30 +155,38 @@ class QueueWorker {
       // Adding generate_sitemap operations for all data sets.
       foreach ($type_definitions[$type]['urlGenerators'] as $url_generator_id) {
 
-        foreach ($this->manager->getUrlGenerator($url_generator_id)
-                   ->setSitemapVariant($variant_name)
-                   ->getDataSets() as $data_set) {
-
-          $data_sets[] = [
-            'data' => $data_set,
-            'sitemap_variant' => $variant_name,
-            'url_generator' => $url_generator_id,
-            'sitemap_generator' => $type_definitions[$type]['sitemapGenerator'],
-          ];
-
-          if (count($data_sets) === self::REBUILD_QUEUE_CHUNK_ITEM_SIZE) {
-            $this->queueElements($data_sets);
-            $data_sets = [];
+        $data_sets = $this->manager->getUrlGenerator($url_generator_id)
+          ->setSitemapVariant($variant_name)
+          ->getDataSets();
+
+        if (!empty($data_sets)) {
+          $sitemap_variants[$variant_name]['data'] = TRUE;
+          foreach ($data_sets as $data_set) {
+            $all_data_sets[] = [
+              'data' => $data_set,
+              'sitemap_variant' => $variant_name,
+              'url_generator' => $url_generator_id,
+              'sitemap_generator' => $type_definitions[$type]['sitemapGenerator'],
+            ];
+
+            if (count($all_data_sets) === self::REBUILD_QUEUE_CHUNK_ITEM_SIZE) {
+              $this->queueElements($all_data_sets);
+              $all_data_sets = [];
+            }
           }
         }
       }
     }
 
-    if (!empty($data_sets)) {
-      $this->queueElements($data_sets);
+    if (!empty($all_data_sets)) {
+      $this->queueElements($all_data_sets);
     }
     $this->getQueuedElementCount(TRUE);
 
+    // todo: May not be clean to remove sitemap variants data when queuing elements.
+    // Remove all sitemap variant instances where no results have been queued.
+    $this->manager->removeSitemap(array_keys(array_filter($sitemap_variants, function($e) { return empty($e['data']); })));
+
     return $this;
   }
 
@@ -193,6 +201,7 @@ class QueueWorker {
    * @throws \Drupal\Component\Plugin\Exception\PluginException
    *
    * @todo Lock functionality
+   * @todo Does not unpublish sitemaps were variant does not have links anymore.
    */
   public function generateSitemap($from = 'form') {
 

src/Simplesitemap.php

@@ -299,25 +299,9 @@ class Simplesitemap {
   /**
    * @return $this
    * @throws \Drupal\Component\Plugin\Exception\PluginException
-   *
-   * @todo document
    */
   public function removeSitemap() {
-    $saved_variants = $this->manager->getSitemapVariants();
-    $this->moduleHandler->alter('simple_sitemap_variants', $saved_variants);
-    $remove_variants = NULL !== ($variants = $this->getVariants(FALSE))
-      ? array_intersect_key($saved_variants, array_flip((array) $variants))
-      : $saved_variants;
-
-    if (!empty($remove_variants)) {
-      $type_definitions = $this->manager->getSitemapTypes();
-      $this->moduleHandler->alter('simple_sitemap_types', $type_definitions);
-      foreach ($remove_variants as $variant_name => $variant_definition) {
-        $this->manager->getSitemapGenerator($type_definitions[$variant_definition['type']]['sitemapGenerator'])
-          ->setSitemapVariant($variant_name)
-          ->remove();
-      }
-    }
+    $this->manager->removeSitemap($this->getVariants(FALSE));
 
     return $this;
   }
@@ -326,6 +310,8 @@ class Simplesitemap {
    * @param string $from
    * @return $this
    * @throws \Drupal\Component\Plugin\Exception\PluginException
+   *
+   * @todo variants
    */
   public function generateSitemap($from = 'form') {
     switch($from) {

src/SimplesitemapManager.php

@@ -133,7 +133,6 @@ class SimplesitemapManager {
    * @param null $sitemap_type
    * @return array
    *
-   * @todo document
    * @todo translate label
    */
   public function getSitemapVariants($sitemap_type = NULL, $attach_type_info = TRUE) {
@@ -210,9 +209,29 @@ class SimplesitemapManager {
     return $this;
   }
 
+  public function removeSitemap($variant_names = NULL) {
+    $saved_variants = $this->getSitemapVariants();
+    $remove_variants = NULL !== $variant_names && !empty((array) $variant_names)
+      ? array_intersect_key($saved_variants, array_flip((array) $variant_names))
+      : $saved_variants;
+
+    if (!empty($remove_variants)) {
+      $type_definitions = $this->getSitemapTypes();
+      foreach ($remove_variants as $variant_name => $variant_definition) {
+        $this->getSitemapGenerator($type_definitions[$variant_definition['type']]['sitemapGenerator'])
+          ->setSitemapVariant($variant_name)
+          ->remove();
+      }
+    }
+
+    return $this;
+  }
+
   public function removeSitemapVariants($variant_names = NULL) {
     if (NULL === $variant_names || !empty((array) $variant_names)) {
-      SitemapGeneratorBase::removeSitemapVariants($variant_names); //todo should call the remove() method of every plugin instead?
+
+      // Remove sitemap instances.
+      $this->removeSitemap($variant_names);
 
       if (NULL === $variant_names) {
         // Remove all variants and their bundle settings.