Refactor all classes into services, refactor Batch class and remove most static methods from it

simple_sitemap.module

@@ -4,7 +4,7 @@
  * Main module file containing hooks.
  */
 
-use Drupal\simple_sitemap\Form;
+use Drupal\simple_sitemap\Form\Form;
 
 /**
  * Implements hook_help.
@@ -58,15 +58,16 @@ function simple_sitemap_form_alter(&$form, $form_state, $form_id) {
  */
 function simple_sitemap_entity_form_submit($form, &$form_state) {
 
+  $f = \Drupal::service('simple_sitemap.form')->processForm($form_state);
+
   $values = $form_state->getValues();
 
   // Fix for values appearing in a sub array on a commerce product entity.
   $values = isset($values['simple_sitemap']) ? $values['simple_sitemap'] : $values;
 
   // Only make changes in DB if sitemap settings actually changed.
-  if (Form::valuesChanged($form, $values)) {
+  if ($f->valuesChanged($form, $values)) {
     $generator = \Drupal::service('simple_sitemap.generator');
-    $f = \Drupal::service('simple_sitemap.form')->processForm($form_state);
 
     switch ($f->entityCategory) {
 

simple_sitemap.services.yml

 services:
   simple_sitemap.generator:
     class: Drupal\simple_sitemap\Simplesitemap
-    arguments: ['@config.factory', '@database', '@entity_type.manager']
+    arguments: ['@simple_sitemap.sitemap_generator', '@config.factory', '@database', '@entity_type.manager', '@path.validator']
 
   simple_sitemap.sitemap_generator:
     class: Drupal\simple_sitemap\SitemapGenerator
-    arguments: ['@simple_sitemap.generator', '@database', '@language_manager', '@module_handler']
+    arguments: ['@simple_sitemap.batch', '@database', '@module_handler', '@language_manager']
 
   simple_sitemap.form:
-    class: Drupal\simple_sitemap\Form
-    arguments: ['@simple_sitemap.generator']
+    class: Drupal\simple_sitemap\Form\Form
+    arguments: ['@simple_sitemap.generator', '@current_user']
 
-  simple_sitemap.bundle_url_generator:
-    class: Drupal\simple_sitemap\BatchBundleUrlGenerator
-    arguments: ['@simple_sitemap.generator', '@language_manager', '@entity_type.manager', '@path.validator', '@entity.query']
+  simple_sitemap.batch:
+    class: Drupal\simple_sitemap\Batch\Batch
 
-  simple_sitemap.custom_url_generator:
-    class: Drupal\simple_sitemap\BatchCustomUrlGenerator
-    arguments: ['@simple_sitemap.generator', '@language_manager', '@entity_type.manager', '@path.validator']
+  simple_sitemap.batch_url_generator:
+    class: Drupal\simple_sitemap\Batch\BatchUrlGenerator
+    arguments: ['@simple_sitemap.sitemap_generator', '@language_manager', '@entity_type.manager', '@path.validator', '@entity.query']

src/Batch.php → src/Batch/Batch.php

 <?php
 
-namespace Drupal\simple_sitemap;
+namespace Drupal\simple_sitemap\Batch;
 
-use Drupal\user\Entity\User;
-use Drupal\Core\Url;
-use Drupal\Component\Utility\Html;
-use Drupal\Core\Cache\Cache;
 use Drupal\Core\StringTranslation\StringTranslationTrait;
 
 class Batch {
+
   use StringTranslationTrait;
+
   private $batch;
   private $batchInfo;
 
-
   const BATCH_INIT_MESSAGE = 'Initializing batch...';
   const BATCH_ERROR_MESSAGE = 'An error has occurred. This may result in an incomplete XML sitemap.';
   const BATCH_PROGRESS_MESSAGE = 'Processing @current out of @total link types.';
 
+  /**
+   * Batch constructor.
+   */
   public function __construct() {
     $this->batch = [
       'title' => $this->t('Generating XML sitemap'),
@@ -29,6 +29,9 @@ class Batch {
     ];
   }
 
+  /**
+   * @param $batch_info
+   */
   public function setBatchInfo($batch_info) {
     $this->batchInfo = $batch_info;
   }
@@ -81,26 +84,6 @@ class Batch {
     ];
   }
 
-  /**
-   * Callback function called by the batch API when all operations are finished.
-   *
-   * @see https://api.drupal.org/api/drupal/core!includes!form.inc/group/batch/8
-   */
-  public static function finishGeneration($success, $results, $operations) {
-    if ($success) {
-      $remove_sitemap = empty($results['chunk_count']);
-      if (!empty($results['generate']) || $remove_sitemap) {
-        \Drupal::service('simple_sitemap.sitemap_generator')->generateSitemap($results['generate'], $remove_sitemap);
-      }
-      Cache::invalidateTags(['simple_sitemap']);
-      drupal_set_message(t("The <a href='@url' target='_blank'>XML sitemap</a> has been regenerated for all languages.",
-        ['@url' => $GLOBALS['base_url'] . '/sitemap.xml']));
-    }
-    else {
-      //todo: register error
-    }
-  }
-
   /**
    * Batch callback function which generates urls to entity paths.
    *
@@ -109,7 +92,7 @@ class Batch {
    * @param array &$context
    */
   public static function generateBundleUrls($entity_info, $batch_info, &$context) {
-    \Drupal::service('simple_sitemap.bundle_url_generator')->generateBundleUrls($entity_info, $batch_info, $context);
+    \Drupal::service('simple_sitemap.batch_url_generator')->generateBundleUrls($entity_info, $batch_info, $context);
   }
 
   /**
@@ -120,6 +103,17 @@ class Batch {
    * @param array &$context
    */
   public static function generateCustomUrls($custom_paths, $batch_info, &$context) {
-    \Drupal::service('simple_sitemap.custom_url_generator')->generateCustomUrls($custom_paths, $batch_info, $context);
+    \Drupal::service('simple_sitemap.batch_url_generator')->generateCustomUrls($custom_paths, $batch_info, $context);
+  }
+
+  /**
+   * Callback function called by the batch API when all operations are finished.
+   *
+   * @param $success
+   * @param $results
+   * @param $operations
+   */
+  public static function finishGeneration($success, $results, $operations) {
+    \Drupal::service('simple_sitemap.batch_url_generator')->finishGeneration($success, $results, $operations);
   }
 }

src/Form.php → src/Form/Form.php

 <?php
 
-namespace Drupal\simple_sitemap;
+namespace Drupal\simple_sitemap\Form;
 
 use Drupal\Core\StringTranslation\StringTranslationTrait;
 
 /**
- * Form class.
+ * Class Form
+ * @package Drupal\simple_sitemap\Form
  */
 class Form {
   use StringTranslationTrait;
@@ -15,6 +16,7 @@ class Form {
   const PRIORITY_DIVIDER = 10;
 
   private $generator;
+  private $currentUser;
   private $formState;
 
   public $alteringForm = TRUE;
@@ -38,11 +40,19 @@ class Form {
 
   /**
    * Form constructor.
+   *
+   * @param $generator
+   * @param $current_user
    */
-  public function __construct($generator) {
+  public function __construct($generator, $current_user) {
     $this->generator = $generator;
+    $this->currentUser = $current_user;
   }
 
+  /**
+   * @param $form_state
+   * @return $this
+   */
   public function processForm($form_state) {
     $this->formState = $form_state;
     if (!is_null($this->formState)) {
@@ -52,21 +62,37 @@ class Form {
     return $this;
   }
 
+  /**
+   * @param $entity_category
+   * @return $this
+   */
   public function setEntityCategory($entity_category) {
     $this->entityCategory = $entity_category;
     return $this;
   }
 
+  /**
+   * @param $entity_type_id
+   * @return $this
+   */
   public function setEntityTypeId($entity_type_id) {
     $this->entityTypeId = $entity_type_id;
     return $this;
   }
 
+  /**
+   * @param $bundle_name
+   * @return $this
+   */
   public function setBundleName($bundle_name) {
     $this->bundleName = $bundle_name;
     return $this;
   }
 
+  /**
+   * @param $instance_id
+   * @return $this
+   */
   public function setInstanceId($instance_id) {
     $this->instanceId = $instance_id;
     return $this;
@@ -75,7 +101,7 @@ class Form {
   private function assertAlteringForm() {
 
     // Do not alter the form if user lacks certain permissions.
-    if (!\Drupal::currentUser()->hasPermission('administer sitemap settings'))
+    if (!$this->currentUser->hasPermission('administer sitemap settings'))
       $this->alteringForm = FALSE;
 
     // Do not alter the form if it is irrelevant to sitemap generation.
@@ -93,6 +119,9 @@ class Form {
       $this->alteringForm = FALSE;
   }
 
+  /**
+   * @param $form_fragment
+   */
   public function displayRegenerateNow(&$form_fragment) {
     $form_fragment['simple_sitemap_regenerate_now'] = [
       '#type' => 'checkbox',
@@ -104,7 +133,12 @@ class Form {
       $form_fragment['simple_sitemap_regenerate_now']['#description'] .= '</br>' . $this->t('Otherwise the sitemap will be regenerated on the next cron run.');
     }
   }
-  
+
+  /**
+   * @param $form_fragment
+   * @param bool $multiple
+   * @return $this
+   */
   public function displayEntitySettings(&$form_fragment, $multiple = FALSE) {
     $prefix = $multiple ? $this->entityTypeId . '_' : '';
 
@@ -142,7 +176,7 @@ class Form {
       '#title' => $this->t('Priority'),
       '#description' => $priority_description,
       '#default_value' => $priority,
-      '#options' => self::getPrioritySelectValues(),
+      '#options' => $this->getPrioritySelectValues(),
     ];
     if ($this->entityCategory == 'instance' && isset($bundle_settings['priority'])) {
       $form_fragment[$prefix . 'simple_sitemap_priority']['#options'][(string)$bundle_settings['priority']] .= ' (' . $this->t('Default') . ')';
@@ -230,10 +264,12 @@ class Form {
    * Checks if simple_sitemap values have been changed after submitting the form.
    * To be used in an entity form submit.
    *
+   * @param $form
+   * @param $values
    * @return bool
    *  TRUE if simple_sitemap form values have been altered by the user.
    */
-  public static function valuesChanged($form, $values) {
+  public function valuesChanged($form, $values) { //todo make non-static
     foreach (self::$valuesToCheck as $field_name) {
       if (isset($values[$field_name]) && $values[$field_name] != $form['simple_sitemap'][$field_name]['#default_value']) {
         return TRUE;
@@ -245,21 +281,29 @@ class Form {
   /**
    * Gets the values needed to display the priority dropdown setting.
    *
-   * @return array $options
+   * @return array
    */
-  public static function getPrioritySelectValues() {
+  public function getPrioritySelectValues() {
     $options = [];
     foreach(range(0, self::PRIORITY_HIGHEST) as $value) {
-      $value = self::formatPriority($value / self::PRIORITY_DIVIDER);
+      $value = $this->formatPriority($value / self::PRIORITY_DIVIDER);
       $options[$value] = $value;
     }
     return $options;
   }
 
-  public static function formatPriority($priority) {
+  /**
+   * @param $priority
+   * @return string
+   */
+  public function formatPriority($priority) {
     return number_format((float)$priority, 1, '.', '');
   }
 
+  /**
+   * @param $priority
+   * @return bool
+   */
   public static function isValidPriority($priority) {
     return !is_numeric($priority) || $priority < 0 || $priority > 1 ? FALSE : TRUE;
   }

src/Form/SimplesitemapCustomLinksForm.php

@@ -3,7 +3,6 @@
 namespace Drupal\simple_sitemap\Form;
 
 use Drupal\Core\Form\FormStateInterface;
-use Drupal\simple_sitemap\Form;
 
 /**
  * Class SimplesitemapCustomLinksForm
@@ -26,7 +25,7 @@ class SimplesitemapCustomLinksForm extends SimplesitemapFormBase {
     $setting_string = '';
     foreach ($this->generator->getConfig('custom') as $custom_link) {
       $setting_string .= isset($custom_link['priority'])
-        ? $custom_link['path'] . ' ' . Form::formatPriority($custom_link['priority'])
+        ? $custom_link['path'] . ' ' . $this->form->formatPriority($custom_link['priority'])
         : $custom_link['path'];
       $setting_string .= "\r\n";
     }

src/Simplesitemap.php

@@ -3,6 +3,7 @@
 namespace Drupal\simple_sitemap;
 
 use Drupal\Core\Entity\ContentEntityTypeInterface;
+use Drupal\simple_sitemap\Form\Form;
 
 /**
  * Class Simplesitemap
@@ -10,13 +11,15 @@ use Drupal\Core\Entity\ContentEntityTypeInterface;
  */
 class Simplesitemap {
 
+  private $sitemapGenerator;
   private $configFactory;
-  private $config;
   private $db;
   private $entityTypeManager;
+  private $pathValidator;
   private static $allowed_link_settings = [
     'entity' => ['index', 'priority'],
-    'custom' => ['priority']];
+    'custom' => ['priority']
+  ];
 
   /**
    * Simplesitemap constructor.
@@ -26,14 +29,17 @@ class Simplesitemap {
    * @param \Drupal\Core\Entity\EntityTypeManager $entityTypeManager
    */
   public function __construct(
+    $sitemapGenerator,
     \Drupal\Core\Config\ConfigFactoryInterface $configFactoryInterface,
     $database,
-    \Drupal\Core\Entity\EntityTypeManager $entityTypeManager) {
-
+    \Drupal\Core\Entity\EntityTypeManager $entityTypeManager,
+    $pathValidator
+  ) {
+    $this->sitemapGenerator = $sitemapGenerator;
     $this->configFactory = $configFactoryInterface;
     $this->db = $database;
     $this->entityTypeManager = $entityTypeManager;
-    $this->config = $this->configFactory->get('simple_sitemap.settings');
+    $this->pathValidator = $pathValidator;
   }
 
   /**
@@ -45,7 +51,7 @@ class Simplesitemap {
    *  The requested configuration.
    */
   public function getConfig($key) {
-    return $this->config->get($key);
+    return $this->configFactory->get('simple_sitemap.settings')->get($key);
   }
 
   private function fetchSitemapChunks() {
@@ -67,8 +73,6 @@ class Simplesitemap {
   public function saveConfig($key, $value) {
     $this->configFactory->getEditable('simple_sitemap.settings')
       ->set($key, $value)->save();
-    // Refresh config object after making changes.
-    $this->config = $this->configFactory->get('simple_sitemap.settings');
     return $this;
   }
 
@@ -127,7 +131,7 @@ class Simplesitemap {
    * @return $this
    */
   public function setBundleSettings($entity_type_id, $bundle_name = NULL, $settings) {
-    $bundle_name = is_null($bundle_name) ? $entity_type_id : $bundle_name;
+    $bundle_name = empty($bundle_name) ? $entity_type_id : $bundle_name;
     $entity_types = $this->getConfig('entity_types');
     $this->addLinkSettings('entity', $settings, $entity_types[$entity_type_id][$bundle_name]);
     $this->saveConfig('entity_types', $entity_types);
@@ -175,7 +179,7 @@ class Simplesitemap {
    * @return array|false
    */
   public function getBundleSettings($entity_type_id, $bundle_name = NULL) {
-    $bundle_name = is_null($bundle_name) ? $entity_type_id : $bundle_name;
+    $bundle_name = empty($bundle_name) ? $entity_type_id : $bundle_name;
     $entity_types = $this->getConfig('entity_types');
     if (isset($entity_types[$entity_type_id][$bundle_name])) {
       $settings = $entity_types[$entity_type_id][$bundle_name];
@@ -237,7 +241,7 @@ class Simplesitemap {
    * @return $this
    */
   public function addCustomLink($path, $settings) {
-    if (!\Drupal::service('path.validator')->isValid($path))
+    if (!$this->pathValidator->isValid($path))
       return $this; // todo: log error
     if ($path[0] != '/')
       return $this; // todo: log error
@@ -305,7 +309,7 @@ class Simplesitemap {
       if (in_array($setting_key, self::$allowed_link_settings[$type])) {
         switch($setting_key) {
           case 'priority':
-            if (Form::isValidPriority($setting)) {
+            if (!Form::isValidPriority($setting)) {
               // todo: register error
               continue;
             }
@@ -374,9 +378,10 @@ class Simplesitemap {
    *  This decides how the batch process is to be run.
    */
   public function generateSitemap($from = 'form') {
-    \Drupal::service('simple_sitemap.sitemap_generator')
-    ->setGenerateFrom($from)
-    ->startGeneration();
+    $this->sitemapGenerator
+      ->setGenerator($this)
+      ->setGenerateFrom($from)
+      ->startGeneration();
   }
 
   /**
@@ -389,7 +394,8 @@ class Simplesitemap {
    *  The sitemap index.
    */
   private function getSitemapIndex($chunks) {
-    return \Drupal::service('simple_sitemap.sitemap_generator')
+    return $this->sitemapGenerator
+      ->setGenerator($this)
       ->generateSitemapIndex($chunks);
   }
 

src/SitemapGenerator.php

@@ -16,19 +16,36 @@ class SitemapGenerator {
   const XMLNS_XHTML = 'http://www.w3.org/1999/xhtml';
   const GENERATED_BY = 'Generated by the Simple XML sitemap Drupal module: https://drupal.org/project/simple_sitemap.';
 
-  private $generator;
+  private $batch;
   private $db;
   private $moduleHandler;
   private $defaultLanguageId;
   private $generateFrom = 'form';
   private $isHreflangSitemap;
+  private $generator;
 
-  public function __construct($generator, $database, $language_manager, $module_handler) {
-    $this->generator = $generator;
+  /**
+   * SitemapGenerator constructor.
+   * @param $batch
+   * @param $database
+   * @param $module_handler
+   * @param $language_manager
+   */
+  public function __construct($batch, $database, $module_handler, $language_manager) {
+    $this->batch = $batch;
     $this->db = $database;
+    $this->moduleHandler = $module_handler;
     $this->defaultLanguageId = $language_manager->getDefaultLanguage()->getId();
     $this->isHreflangSitemap = count($language_manager->getLanguages()) > 1;
-    $this->moduleHandler = $module_handler;
+  }
+
+  /**
+   * @param $generator
+   * @return $this
+   */
+  public function setGenerator($generator) {
+    $this->generator = $generator;
+    return $this;
   }
 
   public function setGenerateFrom($from) {
@@ -40,8 +57,7 @@ class SitemapGenerator {
    * Adds all operations to the batch and starts it.
    */
   public function startGeneration() {
-    $batch = new Batch();
-    $batch->setBatchInfo([
+    $this->batch->setBatchInfo([
       'from' => $this->generateFrom,
       'batch_process_limit' => !empty($this->generator->getSetting('batch_process_limit'))
         ? $this->generator->getSetting('batch_process_limit') : NULL,
@@ -51,13 +67,13 @@ class SitemapGenerator {
       'entity_types' => $this->generator->getConfig('entity_types'),
     ]);
     // Add custom link generating operation.
-    $batch->addOperation('generateCustomUrls', $this->getCustomUrlsData());
+    $this->batch->addOperation('generateCustomUrls', $this->getCustomUrlsData());
 
     // Add entity link generating operations.
     foreach($this->getEntityTypeData() as $data) {
-      $batch->addOperation('generateBundleUrls', $data);
+      $this->batch->addOperation('generateBundleUrls', $data);
     }
-    $batch->start();
+    $this->batch->start();
   }
 
   /**