Allow for missing link setting arguments in API functions, add tests and update hook

simple_sitemap.install

@@ -323,3 +323,37 @@ function simple_sitemap_update_8207() {
     ]
   );
 }
+
+/**
+ * Adding changefreq setting to all existing bundle and custom link settings.
+ */
+function simple_sitemap_update_8208() {
+
+  // Update existing bundle settings.
+  $config_factory = \Drupal::service('config.factory');
+  $entity_types = $config_factory->listAll('simple_sitemap.bundle_settings.');
+
+  foreach ($entity_types as $entity_type) {
+    $config = $config_factory->get($entity_type)->get();
+    if (!isset($config['changefreq'])) {
+      $config_factory->getEditable($entity_type)
+        ->setData($config + ['changefreq' => ''])
+        ->save();
+    }
+  }
+
+  // Update existing entity override data.
+  $results = \Drupal::database()->select('simple_sitemap_entity_overrides', 'o')
+    ->fields('o', ['id', 'inclusion_settings'])
+    ->execute()->fetchAll(\PDO::FETCH_OBJ);
+
+  foreach ($results as $row) {
+    $settings = unserialize($row->inclusion_settings);
+    if (!isset($settings['changefreq'])) {
+      \Drupal::database()->update('simple_sitemap_entity_overrides')
+        ->fields(['inclusion_settings' => serialize($settings + ['changefreq' => '']),])
+        ->condition('id', $row->id)
+        ->execute();
+    }
+  }
+}

simple_sitemap.permissions.yml

 administer sitemap settings:
   title: 'Administer sitemap settings'
-  description: 'Administer Simple XML sitemap settings, alter inclusion, priority and changefreq of content and generate the sitemap on demand.'
+  description: 'Administer Simple XML sitemap settings, alter inclusion settings of content and generate the sitemap on demand.'
   restrict access: false

src/Form/FormHelper.php

@@ -66,7 +66,7 @@ class FormHelper {
     'register',
   ];
 
-  protected static $changefreqSelectValues = [
+  protected static $changefreqValues = [
     'always',
     'hourly',
     'daily',
@@ -425,7 +425,7 @@ class FormHelper {
    */
   public function getChangefreqSelectValues() {
     $options = ['' => t('- Not specified -')];
-    foreach (self::$changefreqSelectValues as $setting) {
+    foreach (self::$changefreqValues as $setting) {
       $options[$setting] = t($setting);
     }
     return $options;
@@ -435,7 +435,7 @@ class FormHelper {
    * @return array
    */
   public static function getChangefreqOptions() {
-    return self::$changefreqSelectValues;
+    return self::$changefreqValues;
   }
 
   /**
@@ -459,6 +459,6 @@ class FormHelper {
    * @return bool
    */
   public static function isValidChangefreq($changefreq) {
-    return in_array($changefreq, self::$changefreqSelectValues);
+    return in_array($changefreq, self::$changefreqValues);
   }
 }

src/Form/SimplesitemapCustomLinksForm.php

@@ -32,7 +32,7 @@ class SimplesitemapCustomLinksForm extends SimplesitemapFormBase {
     $form['simple_sitemap_custom']['custom_links'] = [
       '#type' => 'textarea',
       '#title' => $this->t('Relative Drupal paths'),
-      '#default_value' => $this->customLinksToString($this->generator->getCustomLinks()),
+      '#default_value' => $this->customLinksToString($this->generator->getCustomLinks(FALSE)),
       '#description' => $this->t("Please specify drupal internal (relative) paths, one per line. Do not forget to prepend the paths with a '/'.<br/>Optionally link priority <em>(0.0 - 1.0)</em> can be added by appending it after a space.<br/> Optionally link change frequency <em>(always / hourly / daily / weekly / monthly / yearly / never)</em> can be added by appending it after a space.<br/><br/><strong>Examples:</strong><br/><em>/ 1.0 daily</em> -> home page with the highest priority and daily change frequency<br/><em>/contact</em> -> contact page with the default priority and no change frequency information."),
     ];
 

src/Simplesitemap.php

@@ -45,6 +45,11 @@ class Simplesitemap {
    */
   protected $pathValidator;
 
+  /**
+   * @var \Drupal\Core\Datetime\DateFormatter
+   */
+  protected $dateFormatter;
+
   /**
    * @var array
    */
@@ -54,8 +59,9 @@ class Simplesitemap {
   ];
 
   protected static $link_setting_defaults = [
-    'index' => 0,
+    'index' => 1,
     'priority' => 0.5,
+    'changefreq' => '',
   ];
 
   /**
@@ -292,21 +298,24 @@ class Simplesitemap {
    *   Example: ['index' => TRUE, 'priority' => 0.5, 'changefreq' => 'never'].
    *
    * @return $this
+   *
+   * @todo: enableEntityType automatically
    */
-  public function setBundleSettings($entity_type_id, $bundle_name = NULL, $settings) {
-
+  public function setBundleSettings($entity_type_id, $bundle_name = NULL, $settings = []) {
     $bundle_name = empty($bundle_name) ? $entity_type_id : $bundle_name;
 
+    $old_settings = $this->getBundleSettings($entity_type_id, $bundle_name);
+    $settings = !empty($old_settings) ? array_merge($old_settings, $settings) : $this->supplementDefaultSettings('entity', $settings);
+
+    $bundle_settings = $this->configFactory
+      ->getEditable("simple_sitemap.bundle_settings.$entity_type_id.$bundle_name");
     foreach($settings as $setting_key => $setting) {
-      if ($setting_key == 'index') {
+      if ($setting_key === 'index') {
         $setting = intval($setting);
       }
-      $this->configFactory
-        ->getEditable("simple_sitemap.bundle_settings.$entity_type_id.$bundle_name")
-        ->set($setting_key, $setting)
-        ->save();
+      $bundle_settings->set($setting_key, $setting);
     }
-    //todo: Use addLinkSettings()?
+    $bundle_settings->save();
 
     // Delete entity overrides which are identical to new bundle setting.
     $sitemap_entity_types = $this->entityHelper->getSupportedEntityTypes();
@@ -323,9 +332,6 @@ class Simplesitemap {
       }
       $entity_ids = $query->execute();
 
-      $bundle_settings = $this->configFactory
-        ->get("simple_sitemap.bundle_settings.$entity_type_id.$bundle_name");
-
       $query = $this->db->select('simple_sitemap_entity_overrides', 'o')
         ->fields('o', ['id', 'inclusion_settings'])
         ->condition('o.entity_type', $entity_type_id);
@@ -333,22 +339,26 @@ class Simplesitemap {
         $query->condition('o.entity_id', $entity_ids, 'IN');
       }
 
+      $delete_instances = [];
       foreach($query->execute()->fetchAll() as $result) {
         $delete = TRUE;
         $instance_settings = unserialize($result->inclusion_settings);
         foreach ($instance_settings as $setting_key => $instance_setting) {
-          if ($instance_setting != $bundle_settings->get($setting_key)) {
+          if ($instance_setting != $settings[$setting_key]) {
             $delete = FALSE;
             break;
           }
         }
         if ($delete) {
+          $delete_instances[] = $result->id;
+        }
+      }
+      if (!empty($delete_instances)) {
         $this->db->delete('simple_sitemap_entity_overrides')
-            ->condition('id', $result->id)
+          ->condition('id', $delete_instances, 'IN')
           ->execute();
       }
     }
-    }
     else {
       //todo: log error
     }
@@ -375,25 +385,22 @@ class Simplesitemap {
       $bundle_settings = $this->configFactory
         ->get("simple_sitemap.bundle_settings.$entity_type_id.$bundle_name")
         ->get();
-      return !empty($bundle_settings)
-        ? $this->supplementDefaultSettings('entity', $bundle_settings)
-        : FALSE;
+      return !empty($bundle_settings) ? $bundle_settings : FALSE;
     }
     else {
       $config_names = $this->configFactory->listAll("simple_sitemap.bundle_settings.");
       $all_settings = [];
       foreach($config_names as $config_name) {
         $config_name_parts = explode('.', $config_name);
-        $all_settings[$config_name_parts[2]][$config_name_parts[3]] = $this->supplementDefaultSettings(
-          'entity',
-          $this->configFactory->get($config_name)->get()
-        );
+        $all_settings[$config_name_parts[2]][$config_name_parts[3]] = $this->configFactory->get($config_name)->get();
       }
       return $all_settings;
     }
   }
 
   /**
+   * Supplements all missing link setting with default values.
+   *
    * @param string $type
    * @param array $settings
    * @return array
@@ -401,7 +408,7 @@ class Simplesitemap {
   protected function supplementDefaultSettings($type, $settings) {
     foreach(self::$allowed_link_settings[$type] as $allowed_link_setting) {
       if (!isset($settings[$allowed_link_setting])
-        && isset($link_setting_defaults[$allowed_link_setting])) {
+        && isset(self::$link_setting_defaults[$allowed_link_setting])) {
         $settings[$allowed_link_setting] = self::$link_setting_defaults[$allowed_link_setting];
       }
     }
@@ -418,19 +425,16 @@ class Simplesitemap {
    * @return $this
    */
   public function setEntityInstanceSettings($entity_type_id, $id, $settings) {
-
     $entity = $this->entityTypeManager->getStorage($entity_type_id)->load($id);
-    $bundle_name = $this->entityHelper->getEntityInstanceBundleName($entity);
-    $bundle_settings = $this->configFactory
-      ->get("simple_sitemap.bundle_settings.$entity_type_id.$bundle_name")
-      ->get();
-
+    $bundle_settings = $this->getBundleSettings(
+      $entity_type_id, $this->entityHelper->getEntityInstanceBundleName($entity)
+    );
     if (!empty($bundle_settings)) {
 
       // Check if overrides are different from bundle setting before saving.
       $override = FALSE;
       foreach ($settings as $key => $setting) {
-        if ($setting != $bundle_settings[$key]) {
+        if (!isset($bundle_settings[$key]) || $setting != $bundle_settings[$key]) {
           $override = TRUE;
           break;
         }
@@ -444,8 +448,7 @@ class Simplesitemap {
           ->fields([
             'entity_type' => $entity_type_id,
             'entity_id' => $id,
-            'inclusion_settings' => serialize($settings),
-          ])
+            'inclusion_settings' => serialize(array_merge($bundle_settings, $settings)),])
           ->execute();
       }
       // Else unset override.
@@ -477,7 +480,7 @@ class Simplesitemap {
       ->fetchField();
 
     if (!empty($results)) {
-      return $this->supplementDefaultSettings('entity', unserialize($results));
+      return unserialize($results);
     }
     else {
       $entity = $this->entityTypeManager->getStorage($entity_type_id)
@@ -544,7 +547,7 @@ class Simplesitemap {
    *
    * @todo Validate $settings and throw exceptions
    */
-  public function addCustomLink($path, $settings) {
+  public function addCustomLink($path, $settings = []) {
     if (!$this->pathValidator->isValid($path)) {
       // todo: log error.
       return $this;
@@ -554,9 +557,9 @@ class Simplesitemap {
       return $this;
     }
 
-    $custom_links = $this->getCustomLinks();
+    $custom_links = $this->getCustomLinks(FALSE);
     foreach ($custom_links as $key => $link) {
-      if ($link['path'] == $path) {
+      if ($link['path'] === $path) {
         $link_key = $key;
         break;
       }
@@ -571,12 +574,20 @@ class Simplesitemap {
   /**
    * Returns an array of custom paths and their sitemap settings.
    *
+   * @param bool $supplement_default_settings
    * @return array
    */
-  public function getCustomLinks() {
+  public function getCustomLinks($supplement_default_settings = TRUE) {
     $custom_links = $this->configFactory
       ->get('simple_sitemap.custom')
       ->get('links');
+
+    if ($supplement_default_settings) {
+      foreach ($custom_links as $i => $link_settings) {
+        $custom_links[$i] = $this->supplementDefaultSettings('custom', $link_settings);
+      }
+    }
+
     return $custom_links !== NULL ? $custom_links : [];
   }
 
@@ -590,7 +601,7 @@ class Simplesitemap {
   public function getCustomLink($path) {
     foreach ($this->getCustomLinks() as $key => $link) {
       if ($link['path'] === $path) {
-        return $this->supplementDefaultSettings('custom', $link);
+        return $link;
       }
     }
     return FALSE;
@@ -604,7 +615,7 @@ class Simplesitemap {
    * @return $this
    */
   public function removeCustomLink($path) {
-    $custom_links = $this->getCustomLinks();
+    $custom_links = $this->getCustomLinks(FALSE);
     foreach ($custom_links as $key => $link) {
       if ($link['path'] === $path) {
         unset($custom_links[$key]);

src/Tests/SimplesitemapTest.php

@@ -46,7 +46,7 @@ class SimplesitemapTest extends WebTestBase {
   }
 
   /**
-   * Verify sitemap.xml has the link to the front page with priority '1.0' after first generation.
+   * Verify sitemap.xml has the link to the front page after first generation.
    */
   public function testInitialGeneration() {
     $this->generator->generateSitemap('nobatch');
@@ -54,20 +54,73 @@ class SimplesitemapTest extends WebTestBase {
     $this->assertRaw('urlset');
     $this->assertText($GLOBALS['base_url']);
     $this->assertText('1.0');
+    $this->assertText('daily');
+  }
+
+  /**
+   * Test adding a custom link to the sitemap.
+   */
+  public function testAddCustomLink() {
+    $this->generator->addCustomLink('/node/' . $this->node->id(), ['priority' => 0.2, 'changefreq' => 'monthly'])
+      ->generateSitemap('nobatch');
+
+    $this->drupalGet('sitemap.xml');
+    $this->assertText('node/' . $this->node->id());
+    $this->assertText('0.2');
+    $this->assertText('monthly');
   }
 
   /**
    *
    */
+  public function testAddCustomLinkDefaults() {
+    $this->generator->removeCustomLinks()
+      ->addCustomLink('/node/' . $this->node->id())
+      ->generateSitemap('nobatch');
+
+    $this->drupalGet('sitemap.xml');
+    $this->assertText('node/' . $this->node->id());
+    $this->assertText('0.5');
+    $this->assertNoRaw('changefreq');
+  }
+
+  /**
+   * Test removing custom links from the sitemap.
+   */
+  public function testRemoveCustomLink() {
+    $this->generator->addCustomLink('/node/' . $this->node->id())
+      ->removeCustomLink('/node/' . $this->node->id())
+      ->generateSitemap('nobatch');
+
+    $this->drupalGet('sitemap.xml');
+    $this->assertNoText('node/' . $this->node->id());
+  }
+
+  /**
+   * Test removing all custom paths from the sitemap settings.
+   */
+  public function testRemoveCustomLinks() {
+    $this->generator->removeCustomLinks()
+      ->generateSitemap('nobatch');
+
+    $this->drupalGet('sitemap.xml');
+    $this->assertNoText($GLOBALS['base_url']);
+  }
+
+  /**
+   * Tests setting
+   */
   public function testSetBundleSettings() {
 
     // Index new bundle.
-    $this->generator->setBundleSettings('node', 'page', ['index' => TRUE, 'priority' => 0.5])
+    $this->generator->removeCustomLinks()
+      ->setBundleSettings('node', 'page', ['index' => TRUE, 'priority' => 0.5, 'changefreq' => 'hourly'])
       ->generateSitemap('nobatch');
 
     $this->drupalGet('sitemap.xml');
     $this->assertText('node/' . $this->node->id());
     $this->assertText('0.5');
+    $this->assertText('hourly');
 
     // Only change bundle priority.
     $this->generator->setBundleSettings('node', 'page', ['priority' => 0.9])
@@ -78,21 +131,52 @@ class SimplesitemapTest extends WebTestBase {
     $this->assertNoText('0.5');
     $this->assertText('0.9');
 
-    // Set bundle 'index' setting to 0.
-    $this->generator->setBundleSettings('node', 'page', ['index' => 0])
+    // Only change bundle changefreq.
+    $this->generator->setBundleSettings('node', 'page', ['changefreq' => 'daily'])
+      ->generateSitemap('nobatch');
+
+    $this->drupalGet('sitemap.xml');
+    $this->assertText('node/' . $this->node->id());
+    $this->assertNoText('hourly');
+    $this->assertText('daily');
+
+    // Remove changefreq setting.
+    $this->generator->setBundleSettings('node', 'page', ['changefreq' => ''])
+      ->generateSitemap('nobatch');
+
+    $this->drupalGet('sitemap.xml');
+    $this->assertText('node/' . $this->node->id());
+    $this->assertNoRaw('changefreq');
+    $this->assertNoText('daily');
+
+    // Set bundle 'index' setting to false.
+    $this->generator->setBundleSettings('node', 'page', ['index' => FALSE])
       ->generateSitemap('nobatch');
 
     $this->drupalGet('sitemap.xml');
     $this->assertNoText('node/' . $this->node->id());
-    $this->assertNoText('0.5');
-    $this->assertNoText('0.9');
+  }
+
+  /**
+   *
+   */
+  public function testSetBundleSettingsDefaults() {
+
+    $this->generator->setBundleSettings('node', 'page')
+      ->removeCustomLinks()
+      ->generateSitemap('nobatch');
+
+    $this->drupalGet('sitemap.xml');
+    $this->assertText('node/' . $this->node->id());
+    $this->assertText('0.5');
+    $this->assertNoRaw('changefreq');
   }
 
   /**
    * Test cacheability of the response.
    */
   public function testCacheability() {
-    $this->generator->setBundleSettings('node', 'page', ['index' => TRUE, 'priority' => 0.5])
+    $this->generator->setBundleSettings('node', 'page')
       ->generateSitemap('nobatch');
 
     // Verify the cache was flushed and node is in the sitemap.
@@ -108,23 +192,27 @@ class SimplesitemapTest extends WebTestBase {
 
   /**
    * Test overriding of bundle settings for a single entity.
+   *
+   * @todo Test if overrides are removed if bundle settings are identical.
    */
   public function testSetEntityInstanceSettings() {
-    $this->generator->setBundleSettings('node', 'page', ['index' => TRUE, 'priority' => 0.5])
-      ->setEntityInstanceSettings('node', $this->node->id(), ['index' => TRUE, 'priority' => 0.1])
+    $this->generator->setBundleSettings('node', 'page')
+      ->removeCustomLinks()
+      ->setEntityInstanceSettings('node', $this->node->id(), ['priority' => 0.1, 'changefreq' => 'never'])
       ->generateSitemap('nobatch');
 
     $this->drupalGet('sitemap.xml');
     $this->assertText('node/' . $this->node->id());
-    $this->assertText('0.5');
     $this->assertText('0.1');
+    $this->assertText('never');
   }
 
   /**
    * Test disabling sitemap support for an entity type.
    */
   public function testDisableEntityType() {
-    $this->generator->setBundleSettings('node', 'page', ['index' => TRUE, 'priority' => 0.5])
+    $this->generator->setBundleSettings('node', 'page')
+      ->removeCustomLinks()
       ->disableEntityType('node');
 
     $this->drupalLogin($this->privilegedUser);
@@ -135,7 +223,6 @@ class SimplesitemapTest extends WebTestBase {
 
     $this->drupalGet('sitemap.xml');
     $this->assertNoText('node/' . $this->node->id());
-    $this->assertNoText('0.5');
   }
 
   /**
@@ -143,8 +230,9 @@ class SimplesitemapTest extends WebTestBase {
    */
   public function testEnableEntityType() {
     $this->generator->disableEntityType('node')
+      ->removeCustomLinks()
       ->enableEntityType('node')
-      ->setBundleSettings('node', 'page', ['index' => TRUE, 'priority' => 0.5]);
+      ->setBundleSettings('node', 'page');
 
     $this->drupalLogin($this->privilegedUser);
     $this->drupalGet('admin/structure/types/manage/page');
@@ -154,25 +242,13 @@ class SimplesitemapTest extends WebTestBase {
 
     $this->drupalGet('sitemap.xml');
     $this->assertText('node/' . $this->node->id());
-    $this->assertText('0.5');
-  }
-
-  /**
-   * Test removing all custom paths from the sitemap settings.
-   */
-  public function testRemoveCustomLinks() {
-    $this->generator->removeCustomLinks()
-      ->generateSitemap('nobatch');
-
-    $this->drupalGet('sitemap.xml');
-    $this->assertNoText($GLOBALS['base_url']);
   }
 
   /**
    * Test sitemap index.
    */
   public function testSitemapIndex() {
-    $this->generator->setBundleSettings('node', 'page', ['index' => TRUE, 'priority' => 0.5])
+    $this->generator->setBundleSettings('node', 'page')
       ->saveSetting('max_links', 1)
       ->removeCustomLinks()
       ->generateSitemap('nobatch');
@@ -194,7 +270,7 @@ class SimplesitemapTest extends WebTestBase {
    * Test setting the base URL.
    */
   public function testSetBaseUrl() {
-    $this->generator->setBundleSettings('node', 'page', ['index' => TRUE, 'priority' => 0.5])
+    $this->generator->setBundleSettings('node', 'page')
       ->saveSetting('base_url', 'http://base_url_test')
       ->generateSitemap('nobatch');
 
@@ -206,7 +282,7 @@ class SimplesitemapTest extends WebTestBase {
    * Test setting the base URL in the sitemap index.
    */
   public function testSetBaseUrlInSitemapIndex() {
-    $this->generator->setBundleSettings('node', 'page', ['index' => TRUE, 'priority' => 0.5])
+    $this->generator->setBundleSettings('node', 'page')
       ->saveSetting('max_links', 1)
       ->saveSetting('base_url', 'http://base_url_test')
       ->generateSitemap('nobatch');
@@ -214,30 +290,4 @@ class SimplesitemapTest extends WebTestBase {
     $this->drupalGet('sitemap.xml');
     $this->assertText('http://base_url_test');
   }
-
-  /**
-   * Test adding a custom link to the sitemap.
-   */
-  public function testAddCustomLink() {
-    $this->generator->addCustomLink('/node/' . $this->node->id(), ['priority' => 0.2])
-      ->generateSitemap('nobatch');
-
-    $this->drupalGet('sitemap.xml');
-    $this->assertText('node/' . $this->node->id());
-    $this->assertText('0.2');
-  }
-
-  /**
-   * Test removing custom links from the sitemap.
-   */
-  public function testRemoveCustomLink() {
-    $this->generator->addCustomLink('/node/' . $this->node->id(), ['priority' => 0.2])
-      ->removeCustomLink('/node/' . $this->node->id())
-      ->generateSitemap('nobatch');
-
-    $this->drupalGet('sitemap.xml');
-    $this->assertNoText('node/' . $this->node->id());
-    $this->assertNoText('0.2');
-  }
-
 }