Turn classes into services and make code more unit testable

simple_sitemap.module

@@ -20,7 +20,7 @@ function simple_sitemap_help($route_name, \Drupal\Core\Routing\RouteMatchInterfa
  * Adds sitemap settings to entity types that are supported via plugins.
  */
 function simple_sitemap_form_alter(&$form, $form_state, $form_id) {
-  $f = new Form($form_state);
+  $f = \Drupal::service('simple_sitemap.form')->processForm($form_state);
   if (!$f->alteringForm)
     return;
 
@@ -66,7 +66,7 @@ function simple_sitemap_entity_form_submit($form, &$form_state) {
   // Only make changes in DB if sitemap settings actually changed.
   if (Form::valuesChanged($form, $values)) {
     $generator = \Drupal::service('simple_sitemap.generator');
-    $f = new Form($form_state);
+    $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']
+    arguments: ['@config.factory', '@database', '@entity_type.manager']
+
+  simple_sitemap.sitemap_generator:
+    class: Drupal\simple_sitemap\SitemapGenerator
+    arguments: ['@simple_sitemap.generator', '@database', '@language_manager', '@module_handler']
+
+  simple_sitemap.form:
+    class: Drupal\simple_sitemap\Form
+    arguments: ['@simple_sitemap.generator']

src/Batch.php

@@ -92,7 +92,7 @@ class Batch {
     if ($success) {
       $remove_sitemap = empty($results['chunk_count']);
       if (!empty($results['generate']) || $remove_sitemap) {
-        SitemapGenerator::generateSitemap($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.",
@@ -312,7 +312,7 @@ class Batch {
       foreach ($chunks as $i => $chunk_links) {
         if (count($chunk_links) == $batch_info['max_links']) {
           $remove_sitemap = empty($context['results']['chunk_count']);
-          SitemapGenerator::generateSitemap($chunk_links, $remove_sitemap);
+          \Drupal::service('simple_sitemap.sitemap_generator')->generateSitemap($chunk_links, $remove_sitemap);
           $context['results']['chunk_count'] = !isset($context['results']['chunk_count']) ? 1 : $context['results']['chunk_count'] + 1;
           $context['results']['generate'] = array_slice($context['results']['generate'], count($chunk_links));
         }

src/Form.php

@@ -9,52 +9,67 @@ use Drupal\Core\StringTranslation\StringTranslationTrait;
  */
 class Form {
   use StringTranslationTrait;
+
   const PRIORITY_DEFAULT = 0.5;
   const PRIORITY_HIGHEST = 10;
   const PRIORITY_DIVIDER = 10;
 
-  public $alteringForm;
-  public $entityCategory;
+  private $generator;
+  private $formState;
+
+  public $alteringForm = TRUE;
+  public $entityCategory = NULL;
   public $entityTypeId;
   public $bundleName;
   public $instanceId;
-  
-  private $formState;
-  private $generator;
 
-  private static $allowedFormOperations = ['default', 'edit', 'add', 'register'];
-  private static $valuesToCheck = ['simple_sitemap_index_content', 'simple_sitemap_priority', 'simple_sitemap_regenerate_now'];
+  private static $allowedFormOperations = [
+    'default',
+    'edit',
+    'add',
+    'register'
+  ];
+
+  private static $valuesToCheck = [
+    'simple_sitemap_index_content',
+    'simple_sitemap_priority',
+    'simple_sitemap_regenerate_now'
+  ];
 
   /**
    * Form constructor.
    */
-  function __construct($form_state = NULL) {
+  function __construct($generator) {
+    $this->generator = $generator;
+  }
 
+  public function processForm($form_state) {
     $this->formState = $form_state;
-    $this->entityCategory = NULL;
-    $this->alteringForm = TRUE;
-    $this->generator = \Drupal::service('simple_sitemap.generator');
-
     if (!is_null($this->formState)) {
       $this->getEntityDataFromFormEntity();
       $this->assertAlteringForm();
     }
+    return $this;
   }
 
   public function setEntityCategory($entity_category) {
     $this->entityCategory = $entity_category;
+    return $this;
   }
 
   public function setEntityTypeId($entity_type_id) {
     $this->entityTypeId = $entity_type_id;
+    return $this;
   }
 
   public function setBundleName($bundle_name) {
     $this->bundleName = $bundle_name;
+    return $this;
   }
 
   public function setInstanceId($instance_id) {
     $this->instanceId = $instance_id;
+    return $this;
   }
 
   private function assertAlteringForm() {
@@ -145,7 +160,7 @@ class Form {
     $form_entity = $this->getFormEntity();
     if ($form_entity !== FALSE) {
       $entity_type_id = $form_entity->getEntityTypeId();
-      $sitemap_entity_types = Simplesitemap::getSitemapEntityTypes();
+      $sitemap_entity_types = $this->generator->getSitemapEntityTypes();
       if (isset($sitemap_entity_types[$entity_type_id])) {
         $this->entityCategory = 'instance';
       }
@@ -163,14 +178,14 @@ class Form {
 
       switch ($this->entityCategory) {
         case 'bundle':
-          $this->entityTypeId = Simplesitemap::getBundleEntityTypeId($form_entity);
+          $this->entityTypeId = $this->generator->getBundleEntityTypeId($form_entity);
           $this->bundleName = $form_entity->id();
           $this->instanceId = NULL;
           break;
 
         case 'instance':
           $this->entityTypeId = $entity_type_id;
-          $this->bundleName = Simplesitemap::getEntityInstanceBundleName($form_entity);
+          $this->bundleName = $this->generator->getEntityInstanceBundleName($form_entity);
           $this->instanceId = !empty($form_entity->id()) ? $form_entity->id() : NULL; // New menu link's id is '' instead of NULL, hence checking for empty.
           break;
 

src/Form/SimplesitemapCustomLinksForm.php

@@ -52,7 +52,7 @@ class SimplesitemapCustomLinksForm extends ConfigFormBase {
       '#description' => $this->t("Please specify drupal internal (relative) paths, one per line. Do not forget to prepend the paths with a '/'. You can optionally add a priority (0.0 - 1.0) by appending it to the path after a space. The home page with the highest priority would be <em>/ 1.0</em>, the contact page with the default priority would be <em>/contact 0.5</em>."),
     ];
 
-    $f = new Form();
+    $f = \Drupal::service('simple_sitemap.form')->processForm($form_state);
     $f->displayRegenerateNow($form['simple_sitemap_custom']);
 
     return parent::buildForm($form, $form_state);

src/Form/SimplesitemapEntitiesForm.php

@@ -43,12 +43,12 @@ class SimplesitemapEntitiesForm extends ConfigFormBase {
     $form['#attached']['drupalSettings']['simple_sitemap'] = ['all_entities' => [], 'atomic_entities' => []];
 
     $entity_type_labels = [];
-    foreach (Simplesitemap::getSitemapEntityTypes() as $entity_type_id => $entity_type) {
+    foreach ($generator->getSitemapEntityTypes() as $entity_type_id => $entity_type) {
       $entity_type_labels[$entity_type_id] = $entity_type->getLabel() ? : $entity_type_id;
     }
     asort($entity_type_labels);
 
-    $f = new Form();
+    $f = \Drupal::service('simple_sitemap.form')->processForm($form_state);
 
     foreach ($entity_type_labels as $entity_type_id => $entity_type_label) {
       $form['simple_sitemap_entities']['entities'][$entity_type_id] = [
@@ -63,12 +63,12 @@ class SimplesitemapEntitiesForm extends ConfigFormBase {
         '#default_value' => $generator->entityTypeIsEnabled($entity_type_id),
       ];
       $form['#attached']['drupalSettings']['simple_sitemap']['all_entities'][] = str_replace('_', '-', $entity_type_id);
-      if (Simplesitemap::entityTypeIsAtomic($entity_type_id)) {
+      if ($generator->entityTypeIsAtomic($entity_type_id)) {
         $form['simple_sitemap_entities']['entities'][$entity_type_id][$entity_type_id . '_enabled']['#description'] = $this->t('Sitemap settings for this entity type can be set below and overridden on its entity pages.');
-        $f->setEntityCategory('bundle');
-        $f->setEntityTypeId($entity_type_id);
-        $f->setBundleName($entity_type_id);
-        $f->displayEntitySettings($form['simple_sitemap_entities']['entities'][$entity_type_id][$entity_type_id . '_settings'], TRUE);
+        $f->setEntityCategory('bundle')
+          ->setEntityTypeId($entity_type_id)
+          ->setBundleName($entity_type_id)
+          ->displayEntitySettings($form['simple_sitemap_entities']['entities'][$entity_type_id][$entity_type_id . '_settings'], TRUE);
         $form['#attached']['drupalSettings']['simple_sitemap']['atomic_entities'][] = str_replace('_', '-', $entity_type_id);
       }
     }
@@ -87,7 +87,7 @@ class SimplesitemapEntitiesForm extends ConfigFormBase {
         $entity_type_id = substr($field_name, 0, -8);
         if ($value) {
           $generator->enableEntityType($entity_type_id);
-          if (Simplesitemap::entityTypeIsAtomic($entity_type_id)) {
+          if ($generator->entityTypeIsAtomic($entity_type_id)) {
             $generator->setBundleSettings($entity_type_id, $entity_type_id, [
                 'index' => TRUE,
                 'priority' => $values[$entity_type_id . '_simple_sitemap_priority']

src/Simplesitemap.php

@@ -3,8 +3,9 @@
 namespace Drupal\simple_sitemap;
 
 use Drupal\Core\Config\ConfigFactoryInterface;
+use Drupal\Core\Database\Database;
 use Drupal\Core\Entity\ContentEntityTypeInterface;
-use Drupal\simple_sitemap\Form;
+use Drupal\Core\Entity\EntityTypeManager;
 
 /**
  * Simplesitemap class.
@@ -13,8 +14,10 @@ use Drupal\simple_sitemap\Form;
  */
 class Simplesitemap {
 
-  private $config_factory;
+  private $configFactory;
   private $config;
+  private $db;
+  private $entityTypeManager;
   private static $allowed_link_settings = [
     'entity' => ['index', 'priority'],
     'custom' => ['priority']];
@@ -22,12 +25,21 @@ class Simplesitemap {
   /**
    * Simplesitemap constructor.
    *
-   * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
+   * @param \Drupal\Core\Config\ConfigFactoryInterface $configFactoryInterface
    *   The config factory from the container.
+   *
+   * @param $database
+   *   The database from the container.
    */
-  function __construct(ConfigFactoryInterface $config_factory) {
-    $this->config_factory = $config_factory;
-    $this->config = $this->config_factory->get('simple_sitemap.settings');
+  function __construct(
+    ConfigFactoryInterface $configFactoryInterface,
+    $database,
+    EntityTypeManager $entityTypeManager) {
+
+    $this->configFactory = $configFactoryInterface;
+    $this->db = $database;
+    $this->entityTypeManager = $entityTypeManager;
+    $this->config = $this->configFactory->get('simple_sitemap.settings');
   }
 
   /**
@@ -43,7 +55,7 @@ class Simplesitemap {
   }
 
   private function fetchSitemapChunks() {
-    return \Drupal::service('database')
+    return $this->db
       ->query("SELECT * FROM {simple_sitemap}")
       ->fetchAllAssoc('id');
   }
@@ -57,10 +69,10 @@ class Simplesitemap {
    *  The configuration to be saved.
    */
   public function saveConfig($key, $value) {
-    $this->config_factory->getEditable('simple_sitemap.settings')
+    $this->configFactory->getEditable('simple_sitemap.settings')
       ->set($key, $value)->save();
     // Refresh config object after making changes.
-    $this->config = $this->config_factory->get('simple_sitemap.settings');
+    $this->config = $this->configFactory->get('simple_sitemap.settings');
   }
 
   /**
@@ -125,20 +137,20 @@ class Simplesitemap {
     $this->saveConfig('entity_types', $entity_types);
   }
 
-  public static function getEntityInstanceBundleName($entity) {
+  public function getEntityInstanceBundleName($entity) {
     return $entity->getEntityTypeId() == 'menu_link_content'
       ? $entity->getMenuName() : $entity->bundle(); // Menu fix.
   }
 
-  public static function getBundleEntityTypeId($entity) {
+  public function getBundleEntityTypeId($entity) {
     return $entity->getEntityTypeId() == 'menu'
       ? 'menu_link_content' : $entity->getEntityType()->getBundleOf(); // Menu fix.
   }
 
   public function setEntityInstanceSettings($entity_type_id, $id, $settings) {
     $entity_types = $this->getConfig('entity_types');
-    $entity = \Drupal::entityTypeManager()->getStorage($entity_type_id)->load($id);
-    $bundle_name = self::getEntityInstanceBundleName($entity);
+    $entity = $this->entityTypeManager->getStorage($entity_type_id)->load($id);
+    $bundle_name = $this->getEntityInstanceBundleName($entity);
     if (isset($entity_types[$entity_type_id][$bundle_name])) {
 
       // Check if overrides are different from bundle setting before saving.
@@ -184,8 +196,8 @@ class Simplesitemap {
 
   public function getEntityInstanceSettings($entity_type_id, $id) {
     $entity_types = $this->getConfig('entity_types');
-    $entity = \Drupal::entityTypeManager()->getStorage($entity_type_id)->load($id);
-    $bundle_name = self::getEntityInstanceBundleName($entity);
+    $entity = $this->entityTypeManager->getStorage($entity_type_id)->load($id);
+    $bundle_name = $this->getEntityInstanceBundleName($entity);
     if (isset($entity_types[$entity_type_id][$bundle_name]['entities'][$id])) {
       return $entity_types[$entity_type_id][$bundle_name]['entities'][$id];
     }
@@ -245,7 +257,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;
             }
@@ -297,7 +309,7 @@ class Simplesitemap {
    *  This decides how the batch process is to be run.
    */
   public function generateSitemap($from = 'form') {
-    $generator = new SitemapGenerator($this);
+    $generator = \Drupal::service('simple_sitemap.sitemap_generator');
     $generator->setGenerateFrom($from);
     $generator->startGeneration();
   }
@@ -312,7 +324,7 @@ class Simplesitemap {
    *  The sitemap index.
    */
   private function getSitemapIndex($chunks) {
-    $generator = new SitemapGenerator($this);
+    $generator = \Drupal::service('simple_sitemap.sitemap_generator');
     return $generator->generateSitemapIndex($chunks);
   }
 
@@ -365,8 +377,8 @@ class Simplesitemap {
    * @return array
    *  Objects of entity types that can be indexed by the sitemap.
    */
-  public static function getSitemapEntityTypes() {
-    $entity_types = \Drupal::entityTypeManager()->getDefinitions();
+  public function getSitemapEntityTypes() {
+    $entity_types = $this->entityTypeManager->getDefinitions();
 
     foreach ($entity_types as $entity_type_id => $entity_type) {
       if (!$entity_type instanceof ContentEntityTypeInterface || !method_exists($entity_type, 'getBundleEntityType')) {
@@ -376,10 +388,10 @@ class Simplesitemap {
     return $entity_types;
   }
 
-  public static function entityTypeIsAtomic($entity_type_id) {
+  public function entityTypeIsAtomic($entity_type_id) {
     if ($entity_type_id == 'menu_link_content') // Menu fix.
       return FALSE;
-    $sitemap_entity_types = self::getSitemapEntityTypes();
+    $sitemap_entity_types = $this->getSitemapEntityTypes();
     if (isset($sitemap_entity_types[$entity_type_id])) {
       $entity_type = $sitemap_entity_types[$entity_type_id];
       if (empty($entity_type->getBundleEntityType())) {

src/SitemapGenerator.php

@@ -15,13 +15,16 @@ class SitemapGenerator {
   const XMLNS_XHTML = 'http://www.w3.org/1999/xhtml';
 
   private $generator;
-  private $links;
-  private $generateFrom;
+  private $db;
+  private $moduleHandler;
+  private $defaultLanguageId;
+  private $generateFrom = 'form';
 
-  function __construct($generator) {
+  function __construct($generator, $database, $language_manager, $module_handler) {
     $this->generator = $generator;
-    $this->links = [];
-    $this->generateFrom = 'form';
+    $this->db = $database;
+    $this->defaultLanguageId = $language_manager->getDefaultLanguage()->getId();
+    $this->moduleHandler = $module_handler;
   }
 
   public function setGenerateFrom($from) {
@@ -70,7 +73,7 @@ class SitemapGenerator {
    */
   private function getEntityTypeData() {
     $data_sets = [];
-    $sitemap_entity_types = Simplesitemap::getSitemapEntityTypes();
+    $sitemap_entity_types = $this->generator->getSitemapEntityTypes();
     $entity_types = $this->generator->getConfig('entity_types');
     foreach($entity_types as $entity_type_name => $bundles) {
       if (isset($sitemap_entity_types[$entity_type_name])) {
@@ -100,18 +103,18 @@ class SitemapGenerator {
    * @param bool $remove_sitemap
    *  Remove old sitemap from database before inserting the new one.
    */
-  public static function generateSitemap($links, $remove_sitemap = FALSE) {
+  public function generateSitemap($links, $remove_sitemap = FALSE) {
     // Invoke alter hook.
-        \Drupal::moduleHandler()->alter('simple_sitemap_links', $links);
+    $this->moduleHandler->alter('simple_sitemap_links', $links);
     $values = [
-      'id' => $remove_sitemap ? 1 : \Drupal::service('database')->query('SELECT MAX(id) FROM {simple_sitemap}')->fetchField() + 1,
-      'sitemap_string' => self::generateSitemapChunk($links),
+      'id' => $remove_sitemap ? 1 : $this->db->query('SELECT MAX(id) FROM {simple_sitemap}')->fetchField() + 1,
+      'sitemap_string' => $this->generateSitemapChunk($links),
       'sitemap_created' => REQUEST_TIME,
     ];
     if ($remove_sitemap) {
-      \Drupal::service('database')->truncate('simple_sitemap')->execute();
+      $this->db->truncate('simple_sitemap')->execute();
     }
-    \Drupal::service('database')->insert('simple_sitemap')->fields($values)->execute();
+    $this->db->insert('simple_sitemap')->fields($values)->execute();
   }
 
   /**
@@ -150,9 +153,7 @@ class SitemapGenerator {
    *
    * @return string sitemap chunk
    */
-  private static function generateSitemapChunk($links) {
-    $default_language_id = \Drupal::languageManager()->getDefaultLanguage()->getId();
-
+  private function generateSitemapChunk($links) {
     $writer = new XMLWriter();
     $writer->openMemory();
     $writer->setIndent(TRUE);
@@ -165,7 +166,7 @@ class SitemapGenerator {
       $writer->startElement('url');
 
       // Adding url to standard language.
-      $writer->writeElement('loc', $link['urls'][$default_language_id]);
+      $writer->writeElement('loc', $link['urls'][$this->defaultLanguageId]);
 
       // Adding alternate urls (other languages) if any.
       if (count($link['urls']) > 1) {