Change private members to protected, improve type hinting, clean up

simple_sitemap.module

@@ -25,8 +25,11 @@ function simple_sitemap_help($route_name, RouteMatchInterface $route_match) {
  */
 function simple_sitemap_form_alter(&$form, FormStateInterface $form_state, $form_id) {
 
+  /**
+   * @var Drupal\simple_sitemap\Form\FormHelper $f
+   */
   $f = \Drupal::service('simple_sitemap.form_helper')->processForm($form_state);
-  if (!$f->alteringForm) {
+  if (!$f->alteringForm()) {
     return;
   }
 
@@ -34,12 +37,12 @@ function simple_sitemap_form_alter(&$form, FormStateInterface $form_state, $form
     '#type' => 'details',
     '#group' => isset($form['additional_settings']) ? 'additional_settings' : 'advanced',
     '#title' => t('Simple XML sitemap'),
-    '#description' => $f->entityCategory == 'instance' ? t('Settings for this entity can be overridden here.') : '',
+    '#description' => $f->getEntityCategory() == 'instance' ? t('Settings for this entity can be overridden here.') : '',
   ];
 
   // Attach some js magic to forms.
   // todo: JS not working on comment entity form, hence disabling.
-  if ($f->entityTypeId != 'comment' || $f->entityCategory != 'instance') {
+  if ($f->getEntityTypeId() != 'comment' || $f->getEntityCategory() != 'instance') {
     $form['#attached']['library'][] = 'simple_sitemap/form';
   }
 
@@ -73,6 +76,9 @@ function simple_sitemap_form_alter(&$form, FormStateInterface $form_state, $form
  */
 function simple_sitemap_entity_form_submit($form, FormStateInterface &$form_state) {
 
+  /**
+   * @var Drupal\simple_sitemap\Form\FormHelper $f
+   */
   $f = \Drupal::service('simple_sitemap.form_helper')->processForm($form_state);
 
   $values = $form_state->getValues();
@@ -82,24 +88,34 @@ function simple_sitemap_entity_form_submit($form, FormStateInterface &$form_stat
 
   // Only make changes in DB if sitemap settings actually changed.
   if ($f->valuesChanged($form, $values)) {
+
+    /**
+     * @var \Drupal\simple_sitemap\Simplesitemap $generator
+     */
     $generator = \Drupal::service('simple_sitemap.generator');
 
-    switch ($f->entityCategory) {
+    switch ($f->getEntityCategory()) {
 
       case 'bundle':
-        $f->bundleName = !empty($f->bundleName) ? $f->bundleName : $f->getFormEntityId();
-        $generator->setBundleSettings($f->entityTypeId, $f->bundleName, [
-          'index' => $values['simple_sitemap_index_content'],
-          'priority' => $values['simple_sitemap_priority']
-        ]);
+        $generator->setBundleSettings(
+          $f->getEntityTypeId(),
+          !empty($f->getBundleName()) ? $f->getBundleName() : $f->getFormEntityId(),
+          [
+            'index' => $values['simple_sitemap_index_content'],
+            'priority' => $values['simple_sitemap_priority']
+          ]
+        );
         break;
 
       case 'instance':
-        $f->instanceId = !empty($f->instanceId) ? $f->instanceId : $f->getFormEntityId();
-        $generator->setEntityInstanceSettings($f->entityTypeId, $f->instanceId, [
-          'index' => $values['simple_sitemap_index_content'],
-          'priority' => $values['simple_sitemap_priority']
-        ]);
+        $generator->setEntityInstanceSettings(
+          $f->getEntityTypeId(),
+          !empty($f->getInstanceId()) ? $f->getInstanceId() : $f->getFormEntityId(),
+          [
+            'index' => $values['simple_sitemap_index_content'],
+            'priority' => $values['simple_sitemap_priority']
+          ]
+        );
         break;
     }
 
@@ -114,6 +130,10 @@ function simple_sitemap_entity_form_submit($form, FormStateInterface &$form_stat
  * Implements hook_cron.
  */
 function simple_sitemap_cron() {
+
+  /**
+   * @var \Drupal\simple_sitemap\Simplesitemap $generator
+   */
   $generator = \Drupal::service('simple_sitemap.generator');
   if ($generator->getSetting('cron_generate')) {
     $generator->generateSitemap('backend');
@@ -128,8 +148,14 @@ function simple_sitemap_cron() {
  * @param \Drupal\Core\Entity\EntityInterface $entity
  */
 function simple_sitemap_entity_delete(EntityInterface $entity) {
-  \Drupal::service('simple_sitemap.generator')
-    ->removeEntityInstanceSettings($entity->getEntityTypeId(), $entity->id());
+
+  /**
+   * @var \Drupal\simple_sitemap\Simplesitemap $generator
+   */
+  $generator = \Drupal::service('simple_sitemap.generator');
+  $generator->removeEntityInstanceSettings(
+    $entity->getEntityTypeId(), $entity->id()
+  );
 }
 
 /**
@@ -161,6 +187,10 @@ function simple_sitemap_menu_delete(MenuInterface $menu) {
  * @param string $bundle
  */
 function simple_sitemap_delete_bundle_config($entity_type_id, $bundle) {
+
+  /**
+   * @var \Drupal\simple_sitemap\Simplesitemap $generator
+   */
   $generator = \Drupal::service('simple_sitemap.generator');
   $deleted_bundle_settings = $generator->getBundleSettings($entity_type_id, $bundle);
   if ($deleted_bundle_settings !== FALSE) {

src/Batch/Batch.php

@@ -14,12 +14,12 @@ class Batch {
   /**
    * @var array
    */
-  private $batch;
+  protected $batch;
 
   /**
    * @var array
    */
-  private $batchInfo;
+  protected $batchInfo;
 
   const BATCH_TITLE = 'Generating XML sitemap';
   const BATCH_INIT_MESSAGE = 'Initializing batch...';

src/Batch/BatchUrlGenerator.php

@@ -233,7 +233,7 @@ class BatchUrlGenerator {
         'lastmod' => method_exists($entity, 'getChangedTime') ? date_iso8601($entity->getChangedTime()) : NULL,
         'priority' => isset($custom_path['priority']) ? $custom_path['priority'] : NULL,
       ];
-      if (!is_null($entity)) {
+      if (NULL !== $entity) {
         $path_data['entity_info'] = ['entity_type' => $entity->getEntityTypeId(), 'id' => $entity->id()];
       }
       $this->addUrlVariants($url_object, $path_data, $entity);
@@ -265,7 +265,7 @@ class BatchUrlGenerator {
    * @param $entity_info
    * @return mixed
    */
-  private function getBatchIterationEntities($entity_info) {
+  protected function getBatchIterationEntities($entity_info) {
     $query = $this->entityQuery->get($entity_info['entity_type_name']);
 
     if (!empty($entity_info['keys']['id'])) {
@@ -294,9 +294,9 @@ class BatchUrlGenerator {
 
   /**
    * @param array $custom_paths
-   * @return mixed
+   * @return array
    */
-  private function getBatchIterationCustomPaths(array $custom_paths) {
+  protected function getBatchIterationCustomPaths(array $custom_paths) {
 
     if ($this->needsInitialization()) {
       $this->initializeBatch(count($custom_paths));
@@ -314,14 +314,14 @@ class BatchUrlGenerator {
    * @param $path_data
    * @param $entity
    */
-  private function addUrlVariants($url_object, $path_data, $entity) {
+  protected function addUrlVariants($url_object, $path_data, $entity) {
     $alternate_urls = [];
 
-    $translation_languages = !is_null($entity) && $this->batchInfo['skip_untranslated']
+    $translation_languages = NULL !== $entity && $this->batchInfo['skip_untranslated']
       ? $entity->getTranslationLanguages() : $this->languages;
 
     // Entity is not translated.
-    if (!is_null($entity) && isset($translation_languages['und'])) {
+    if (NULL !== $entity && isset($translation_languages['und'])) {
       if ($url_object->access($this->anonUser)) {
         $url_object->setOption('language', $this->languages[$this->defaultLanguageId]);
         $alternate_urls[$this->defaultLanguageId] = $this->replaceBaseUrlWithCustom($url_object->toString());
@@ -329,7 +329,7 @@ class BatchUrlGenerator {
     }
     else {
       // Including only translated variants of entity.
-      if (!is_null($entity) && $this->batchInfo['skip_untranslated']) {
+      if (NULL !== $entity && $this->batchInfo['skip_untranslated']) {
         foreach ($translation_languages as $language) {
           $translation = $entity->getTranslation($language->getId());
           if ($translation->access('view', $this->anonUser)) {
@@ -429,7 +429,7 @@ class BatchUrlGenerator {
    * @param $url_object
    * @return object|null
    */
-  private function getEntityFromUrlObject($url_object) {
+  protected function getEntityFromUrlObject($url_object) {
     $route_parameters = $url_object->getRouteParameters();
     return !empty($route_parameters) && $this->entityTypeManager
       ->getDefinition($entity_type_id = key($route_parameters), FALSE)
@@ -438,7 +438,11 @@ class BatchUrlGenerator {
       : NULL;
   }
 
-  private function replaceBaseUrlWithCustom($url) {
+  /**
+   * @param string $url
+   * @return string
+   */
+  protected function replaceBaseUrlWithCustom($url) {
     return !empty($this->batchInfo['base_url'])
       ? str_replace($GLOBALS['base_url'], $this->batchInfo['base_url'], $url)
       : $url;

src/EntityHelper.php

@@ -15,7 +15,7 @@ class EntityHelper {
   /**
    * @var \Drupal\Core\Entity\EntityTypeManagerInterface
    */
-  private $entityTypeManager;
+  protected $entityTypeManager;
 
   /**
    * EntityHelper constructor.

src/Form/FormHelper.php

@@ -22,56 +22,56 @@ class FormHelper {
   /**
    * @var \Drupal\simple_sitemap\Simplesitemap
    */
-  private $generator;
+  protected $generator;
 
   /**
    * @var \Drupal\simple_sitemap\EntityHelper
    */
-  private $entityHelper;
+  protected $entityHelper;
 
   /**
    * @var \Drupal\Core\Session\AccountProxyInterface
    */
-  private $currentUser;
+  protected $currentUser;
 
   /**
    * @var \Drupal\Core\Form\FormState
    */
-  private $formState;
+  protected $formState;
 
   /**
    * @var bool
    */
-  public $alteringForm = TRUE;
+  protected $alteringForm = TRUE;
 
   /**
    * @var string|null
    */
-  public $entityCategory = NULL;
+  protected $entityCategory = NULL;
 
   /**
    * @var string
    */
-  public $entityTypeId;
+  protected $entityTypeId;
 
   /**
    * @var string
    */
-  public $bundleName;
+  protected $bundleName;
 
   /**
    * @var string
    */
-  public $instanceId;
+  protected $instanceId;
 
-  private static $allowedFormOperations = [
+  protected static $allowedFormOperations = [
     'default',
     'edit',
     'add',
     'register',
   ];
 
-  private static $valuesToCheck = [
+  protected static $valuesToCheck = [
     'simple_sitemap_index_content',
     'simple_sitemap_priority',
     'simple_sitemap_regenerate_now',
@@ -99,13 +99,18 @@ class FormHelper {
    */
   public function processForm(FormState $form_state) {
     $this->formState = $form_state;
-    if (!is_null($this->formState)) {
-      $this->getEntityDataFromFormEntity();
-      $this->assertAlteringForm();
-    }
+    $this->getEntityDataFromFormEntity();
+    $this->assertAlteringForm();
     return $this;
   }
 
+  /**
+   * @return bool
+   */
+  public function alteringForm() {
+    return $this->alteringForm;
+  }
+
   /**
    * @param string $entity_category
    * @return $this
@@ -116,14 +121,28 @@ class FormHelper {
   }
 
   /**
-   * @param string $entity_type_id
-   * @return $this
+   * @return null|string
    */
+  public function getEntityCategory() {
+    return $this->entityCategory;
+  }
+
+  /**
+ * @param string $entity_type_id
+ * @return $this
+ */
   public function setEntityTypeId($entity_type_id) {
     $this->entityTypeId = $entity_type_id;
     return $this;
   }
 
+  /**
+   * @return string
+   */
+  public function getEntityTypeId() {
+    return $this->entityTypeId;
+  }
+
   /**
    * @param string $bundle_name
    * @return $this
@@ -133,6 +152,13 @@ class FormHelper {
     return $this;
   }
 
+  /**
+   * @return string
+   */
+  public function getBundleName() {
+    return $this->bundleName;
+  }
+
   /**
    * @param string $instance_id
    * @return $this
@@ -142,10 +168,17 @@ class FormHelper {
     return $this;
   }
 
+  /**
+   * @return string
+   */
+  public function getInstanceId() {
+    return $this->instanceId;
+  }
+
   /**
    *
    */
-  private function assertAlteringForm() {
+  protected function assertAlteringForm() {
 
     // Do not alter the form if user lacks certain permissions.
     if (!$this->currentUser->hasPermission('administer sitemap settings')) {
@@ -153,19 +186,19 @@ class FormHelper {
     }
 
     // Do not alter the form if it is irrelevant to sitemap generation.
-    elseif (empty($this->entityCategory)) {
+    elseif (empty($this->getEntityCategory())) {
       $this->alteringForm = FALSE;
     }
 
     // Do not alter the form if entity is not enabled in sitemap settings.
-    elseif (!$this->generator->entityTypeIsEnabled($this->entityTypeId)) {
+    elseif (!$this->generator->entityTypeIsEnabled($this->getEntityTypeId())) {
       $this->alteringForm = FALSE;
     }
 
     // Do not alter the form, if sitemap is disabled for the entity type of this
     // entity instance.
-    elseif ($this->entityCategory == 'instance'
-      && !$this->generator->bundleIsIndexed($this->entityTypeId, $this->bundleName)) {
+    elseif ($this->getEntityCategory() == 'instance'
+      && !$this->generator->bundleIsIndexed($this->getEntityTypeId(), $this->getBundleName())) {
       $this->alteringForm = FALSE;
     }
   }
@@ -191,34 +224,34 @@ class FormHelper {
    * @return $this
    */
   public function displayEntitySettings(&$form_fragment, $multiple = FALSE) {
-    $prefix = $multiple ? $this->entityTypeId . '_' : '';
+    $prefix = $multiple ? $this->getEntityTypeId() . '_' : '';
 
-    if ($this->entityCategory == 'instance') {
-      $bundle_settings = $this->generator->getBundleSettings($this->entityTypeId, $this->bundleName);
-      $settings = !is_null($this->instanceId) ? $this->generator->getEntityInstanceSettings($this->entityTypeId, $this->instanceId) : $bundle_settings;
+    if ($this->getEntityCategory() == 'instance') {
+      $bundle_settings = $this->generator->getBundleSettings($this->getEntityTypeId(), $this->getBundleName());
+      $settings = NULL !== $this->getInstanceId() ? $this->generator->getEntityInstanceSettings($this->getEntityTypeId(), $this->getInstanceId()) : $bundle_settings;
     }
     else {
-      $settings = $this->generator->getBundleSettings($this->entityTypeId, $this->bundleName);
+      $settings = $this->generator->getBundleSettings($this->getEntityTypeId(), $this->getBundleName());
     }
     $index = isset($settings['index']) ? $settings['index'] : 0;
     $priority = isset($settings['priority']) ? $settings['priority'] : self::PRIORITY_DEFAULT;
-    $bundle_name = !empty($this->bundleName) ? $this->bundleName : $this->t('undefined');
+    $bundle_name = !empty($this->getBundleName()) ? $this->getBundleName() : $this->t('undefined');
 
     if (!$multiple) {
       $form_fragment[$prefix . 'simple_sitemap_index_content'] = [
         '#type' => 'radios',
         '#default_value' => $index,
         '#options' => [
-          0 => $this->entityCategory == 'instance' ? $this->t('Do not index this @bundle entity', ['@bundle' => $bundle_name]) : $this->t('Do not index entities of this type'),
-          1 => $this->entityCategory == 'instance' ? $this->t('Index this @bundle entity', ['@bundle' => $bundle_name]) : $this->t('Index entities of this type'),
+          0 => $this->getEntityCategory() == 'instance' ? $this->t('Do not index this @bundle entity', ['@bundle' => $bundle_name]) : $this->t('Do not index entities of this type'),
+          1 => $this->getEntityCategory() == 'instance' ? $this->t('Index this @bundle entity', ['@bundle' => $bundle_name]) : $this->t('Index entities of this type'),
         ],
       ];
-      if ($this->entityCategory == 'instance' && isset($bundle_settings['index'])) {
+      if ($this->getEntityCategory() == 'instance' && isset($bundle_settings['index'])) {
         $form_fragment[$prefix . 'simple_sitemap_index_content']['#options'][$bundle_settings['index']] .= ' <em>(' . $this->t('Default') . ')</em>';
       }
     }
 
-    if ($this->entityCategory == 'instance') {
+    if ($this->getEntityCategory() == 'instance') {
       $priority_description = $this->t('The priority this @bundle entity will have in the eyes of search engine bots.', ['@bundle' => $bundle_name]);
     }
     else {
@@ -231,7 +264,7 @@ class FormHelper {
       '#default_value' => $priority,
       '#options' => $this->getPrioritySelectValues(),
     ];
-    if ($this->entityCategory == 'instance' && isset($bundle_settings['priority'])) {
+    if ($this->getEntityCategory() == 'instance' && isset($bundle_settings['priority'])) {
       $form_fragment[$prefix . 'simple_sitemap_priority']['#options'][$this->formatPriority($bundle_settings['priority'])] .= ' (' . $this->t('Default') . ')';
     }
     return $this;
@@ -244,38 +277,38 @@ class FormHelper {
    * @return bool
    *   TRUE if this is a bundle or bundle instance form, FALSE otherwise.
    */
-  private function getEntityDataFromFormEntity() {
+  protected function getEntityDataFromFormEntity() {
     $form_entity = $this->getFormEntity();
     if ($form_entity !== FALSE) {
       $entity_type_id = $form_entity->getEntityTypeId();
       $sitemap_entity_types = $this->entityHelper->getSitemapEntityTypes();
       if (isset($sitemap_entity_types[$entity_type_id])) {
-        $this->entityCategory = 'instance';
+        $this->setEntityCategory('instance');
       }
       else {
         foreach ($sitemap_entity_types as $sitemap_entity) {
           if ($sitemap_entity->getBundleEntityType() == $entity_type_id) {
-            $this->entityCategory = 'bundle';
+            $this->setEntityCategory('bundle');
             break;
           }
         }
       }
 
       // Menu fix.
-      $this->entityCategory = is_null($this->entityCategory) && $entity_type_id == 'menu' ? 'bundle' : $this->entityCategory;
+      $this->setEntityCategory(NULL === $this->getEntityCategory() && $entity_type_id == 'menu' ? 'bundle' : $this->getEntityCategory());
 
-      switch ($this->entityCategory) {
+      switch ($this->getEntityCategory()) {
         case 'bundle':
-          $this->entityTypeId = $this->entityHelper->getBundleEntityTypeId($form_entity);
-          $this->bundleName = $form_entity->id();
-          $this->instanceId = NULL;
+          $this->setEntityTypeId($this->entityHelper->getBundleEntityTypeId($form_entity));
+          $this->setBundleName($form_entity->id());
+          $this->setInstanceId(NULL);
           break;
 
         case 'instance':
-          $this->entityTypeId = $entity_type_id;
-          $this->bundleName = $this->entityHelper->getEntityInstanceBundleName($form_entity);
+          $this->setEntityTypeId($entity_type_id);
+          $this->setBundleName($this->entityHelper->getEntityInstanceBundleName($form_entity));
           // New menu link's id is '' instead of NULL, hence checking for empty.
-          $this->instanceId = !empty($form_entity->id()) ? $form_entity->id() : NULL;
+          $this->setInstanceId(!empty($form_entity->id()) ? $form_entity->id() : NULL);
           break;
 
         default:
@@ -293,9 +326,9 @@ class FormHelper {
    *   Entity or FALSE if non-existent or if form operation is
    *   'delete'.
    */
-  private function getFormEntity() {
+  protected function getFormEntity() {
     $form_object = $this->formState->getFormObject();
-    if (!is_null($form_object)
+    if (NULL !== $form_object
       && method_exists($form_object, 'getEntity')
       && in_array($form_object->getOperation(), self::$allowedFormOperations)) {
       return $form_object->getEntity();

src/Form/SimplesitemapCustomLinksForm.php

@@ -97,7 +97,7 @@ class SimplesitemapCustomLinksForm extends SimplesitemapFormBase {
    * @param $custom_links_string
    * @return array
    */
-  private function getCustomLinksFromString($custom_links_string) {
+  protected function getCustomLinksFromString($custom_links_string) {
     // Unify newline characters and explode into array.
     $custom_links_string_lines = explode("\n", str_replace("\r\n", "\n", $custom_links_string));
     // Remove empty values and whitespaces from array.

src/Form/SimplesitemapSettingsForm.php

@@ -14,7 +14,7 @@ class SimplesitemapSettingsForm extends SimplesitemapFormBase {
   /**
    * @var array
    */
-  private $formSettings = [
+  protected $formSettings = [
     'max_links',
     'cron_generate',
     'remove_duplicates',

src/Simplesitemap.php

@@ -19,42 +19,42 @@ class Simplesitemap {
   /**
    * @var \Drupal\simple_sitemap\SitemapGenerator
    */
-  private $sitemapGenerator;
+  protected $sitemapGenerator;
 
   /**
    * @var \Drupal\simple_sitemap\EntityHelper
    */
-  private $entityHelper;
+  protected $entityHelper;
 
   /**
    * @var \Drupal\Core\Config\ConfigFactory
    */
-  private $configFactory;
+  protected $configFactory;
 
   /**
    * @var \Drupal\Core\Database\Connection
    */
-  private $db;
+  protected $db;
 
   /**
    * @var \Drupal\Core\Entity\Query\QueryFactory
    */
-  private $entityQuery;
+  protected $entityQuery;
 
   /**
    * @var \Drupal\Core\Entity\EntityTypeManagerInterface
    */
-  private $entityTypeManager;
+  protected $entityTypeManager;
 
   /**
    * @var \Drupal\Core\Path\PathValidator
    */
-  private $pathValidator;
+  protected $pathValidator;
 
   /**
    * @var array
    */
-  private static $allowed_link_settings = [
+  protected static $allowed_link_settings = [
     'entity' => ['index', 'priority'],
     'custom' => ['priority'],
   ];
@@ -95,7 +95,7 @@ class Simplesitemap {
    *
    * @return string
    */
-  private function fetchSitemapChunks() {
+  protected function fetchSitemapChunks() {
     return $this->db
       ->query("SELECT * FROM {simple_sitemap}")
       ->fetchAllAssoc('id');
@@ -185,6 +185,8 @@ class Simplesitemap {
     if (isset($sitemap_entity_types[$entity_type_id])) {
       $entity_type = $sitemap_entity_types[$entity_type_id];
       $keys = $entity_type->getKeys();
+
+      // Menu fix.
       $keys['bundle'] = $entity_type_id == 'menu_link_content' ? 'menu_name' : $keys['bundle'];
 
       $query = $this->entityQuery->get($entity_type_id);
@@ -240,7 +242,7 @@ class Simplesitemap {
    *  False if entity type does not exist.
    */
   public function getBundleSettings($entity_type_id = NULL, $bundle_name = NULL) {
-    if (null !== $entity_type_id) {
+    if (NULL !== $entity_type_id) {
       $bundle_name = empty($bundle_name) ? $entity_type_id : $bundle_name;
       $settings = $this->configFactory
         ->get("simple_sitemap.bundle_settings.$entity_type_id.$bundle_name")
@@ -352,7 +354,7 @@ class Simplesitemap {
   public function removeEntityInstanceSettings($entity_type_id, $entity_ids = NULL) {
     $query = $this->db->delete('simple_sitemap_entity_overrides')
       ->condition('entity_type', $entity_type_id);
-    if (!is_null($entity_ids)) {
+    if (NULL !== $entity_ids) {
       $entity_ids = !is_array($entity_ids) ? [$entity_ids] : $entity_ids;
       $query->condition('entity_id', $entity_ids, 'IN');
     }
@@ -421,7 +423,7 @@ class Simplesitemap {