Issue #2896546 - move global_push_limit and pull_max_queue_size to config

- update a few broken unit tests

config/schema/salesforce.schema.yml

@@ -2,6 +2,12 @@ salesforce.settings:
   type: config_object
   label: 'Salesforce Settings'
   mapping:
+    global_push_limit:
+      type: integer
+      label: 'Salesforce push queue global limit'
+    pull_max_queue_size:
+      type: integer
+      label: 'Salesforce pull queue max size'
     standalone:
       type: boolean
       label: 'Provide standalone push queue processing endpoint.'

modules/salesforce_pull/salesforce_pull.install

@@ -7,19 +7,11 @@
 
 use Drupal\salesforce_pull\QueueHandler;
 
- /**
-  * Implements hook_install().
-  */
-function salesforce_pull_install() {
-  \Drupal::state()->set('salesforce.pull_max_queue_size', QueueHandler::PULL_MAX_QUEUE_SIZE);
-}
-
 /**
  * Implements hook_uninstall().
  */
 function salesforce_pull_uninstall() {
   $delete = [
-    'salesforce.pull_max_queue_size',
     'salesforce.sobject_pull_info',
   ];
   \Drupal::state()->deleteMultiple($delete);
@@ -69,4 +61,15 @@ function salesforce_pull_update_8001() {
   \Drupal::state()->set('salesforce_pull_last_delete', $delete);
   \Drupal::state()->set('salesforce_pull_last_sync', $sync);
   \Drupal::state()->deleteMultiple(array_keys($kv));
+}
+
+/**
+ * Moves global push limit out of state into config
+ */
+function salesforce_push_update_3() {
+  $config = \Drupal::configFactory()->getEditable('salesforce.settings');
+  $config
+    ->set('pull_max_queue_size', \Drupal::state()->get('salesforce.pull_max_queue_size'))
+    ->save();
+  \Drupal::state()->delete('salesforce.pull_max_queue_size');
 }
\ No newline at end of file

modules/salesforce_pull/salesforce_pull.services.yml

 services:
   salesforce_pull.queue_handler:
     class: Drupal\salesforce_pull\QueueHandler
-    arguments: ['@salesforce.client', '@entity_type.manager', '@queue.database', '@state', '@event_dispatcher', '@datetime.time']
+    arguments: ['@salesforce.client', '@entity_type.manager', '@queue.database', '@config.factory', '@event_dispatcher', '@datetime.time']
 
   salesforce_pull.delete_handler:
     class: Drupal\salesforce_pull\DeleteHandler

modules/salesforce_pull/src/QueueHandler.php

@@ -4,7 +4,7 @@ namespace Drupal\salesforce_pull;
 
 use Drupal\Core\Entity\EntityTypeManagerInterface;
 use Drupal\Core\Queue\QueueDatabaseFactory;
-use Drupal\Core\State\StateInterface;
+use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Core\Utility\Error;
 use Drupal\salesforce\Event\SalesforceErrorEvent;
 use Drupal\salesforce\Event\SalesforceEvents;
@@ -43,9 +43,9 @@ class QueueHandler {
   protected $mappings;
 
   /**
-   * @var \Drupal\Core\State\StateInterface
+   * @var \Drupal\Core\Config\Config
    */
-  protected $state;
+  protected $config;
 
   /**
    * @var \Symfony\Component\HttpFoundation\Request
@@ -64,10 +64,10 @@ class QueueHandler {
    * @param EventDispatcherInterface $event_dispatcher
    */
 
-  public function __construct(RestClientInterface $sfapi, EntityTypeManagerInterface $entity_type_manager, QueueDatabaseFactory $queue_factory, StateInterface $state, EventDispatcherInterface $event_dispatcher, TimeInterface $time) {
+  public function __construct(RestClientInterface $sfapi, EntityTypeManagerInterface $entity_type_manager, QueueDatabaseFactory $queue_factory, ConfigFactoryInterface $config, EventDispatcherInterface $event_dispatcher, TimeInterface $time) {
     $this->sfapi = $sfapi;
     $this->queue = $queue_factory->get('cron_salesforce_pull');
-    $this->state = $state;
+    $this->config = $config->get('salesforce.settings');
     $this->eventDispatcher = $event_dispatcher;
     $this->time = $time;
     $this->mappings = $entity_type_manager
@@ -87,11 +87,11 @@ class QueueHandler {
   public function getUpdatedRecords() {
     // Avoid overloading the processing queue and pass this time around if it's
     // over a configurable limit.
-    if ($this->queue->numberOfItems() > $this->state->get('salesforce.pull_max_queue_size', self::PULL_MAX_QUEUE_SIZE)) {
+    if ($this->queue->numberOfItems() > $this->config->get('pull_max_queue_size', self::PULL_MAX_QUEUE_SIZE)) {
       $message = 'Pull Queue contains %noi items, exceeding the max size of %max items. Pull processing will be blocked until the number of items in the queue is reduced to below the max size.';
       $args = [
         '%noi' => $this->queue->numberOfItems(),
-        '%max' => $this->state->get('salesforce.pull_max_queue_size', self::PULL_MAX_QUEUE_SIZE),
+        '%max' => $this->config->get('pull_max_queue_size', self::PULL_MAX_QUEUE_SIZE),
       ];
       $this->eventDispatcher->dispatch(SalesforceEvents::NOTICE, new SalesforceNoticeEvent(NULL, $message, $args));
       return FALSE;

modules/salesforce_pull/tests/src/Unit/QueueHandlerTest.php

@@ -6,6 +6,8 @@ use Drupal\Core\Entity\EntityTypeManagerInterface;
 use Drupal\Core\Queue\QueueDatabaseFactory;
 use Drupal\Core\Queue\QueueInterface;
 use Drupal\Core\State\StateInterface;
+use Drupal\Core\Config\Config;
+use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Tests\UnitTestCase;
 use Drupal\salesforce\Rest\RestClientInterface;
 use Drupal\salesforce\SelectQuery;
@@ -94,10 +96,18 @@ class QueueHandlerTest extends UnitTestCase {
     $this->etm = $prophecy->reveal();
 
 
+    // mock config
+    $prophecy = $this->prophesize(Config::CLASS);
+    $prophecy->get('pull_max_queue_size', Argument::any())->willReturn(QueueHandler::PULL_MAX_QUEUE_SIZE);
+    $config = $prophecy->reveal();
+
+    $prophecy = $this->prophesize(ConfigFactoryInterface::CLASS);
+    $prophecy->get('salesforce.settings')->willReturn($config);
+    $this->configFactory = $prophecy->reveal();
+
     // mock state
     $prophecy = $this->prophesize(StateInterface::CLASS);
     $prophecy->get('salesforce.sobject_pull_info', Argument::any())->willReturn(['default' => ['last_pull_timestamp' => '0']]);
-    $prophecy->get('salesforce.pull_max_queue_size', Argument::any())->willReturn(QueueHandler::PULL_MAX_QUEUE_SIZE);
     $prophecy->set('salesforce.sobject_pull_info', Argument::any())->willReturn(null);
     $this->state = $prophecy->reveal();
 
@@ -110,7 +120,7 @@ class QueueHandlerTest extends UnitTestCase {
 
     $this->qh = $this->getMockBuilder(QueueHandler::CLASS)
       ->setMethods(['parseUrl'])
-      ->setConstructorArgs([$this->sfapi, $this->etm, $this->queue_factory, $this->state, $this->ed, $this->time])
+      ->setConstructorArgs([$this->sfapi, $this->etm, $this->queue_factory, $this->configFactory, $this->ed, $this->time])
       ->getMock();
     $this->qh->expects($this->any())
       ->method('parseUrl')
@@ -148,7 +158,7 @@ class QueueHandlerTest extends UnitTestCase {
 
     $this->qh = $this->getMockBuilder(QueueHandler::CLASS)
       ->setMethods(['parseUrl'])
-      ->setConstructorArgs([$this->sfapi, $this->etm, $this->queue_factory, $this->state, $this->ed, $this->time])
+      ->setConstructorArgs([$this->sfapi, $this->etm, $this->queue_factory, $this->configFactory, $this->ed, $this->time])
       ->getMock();
     $this->qh->expects($this->any())
       ->method('parseUrl')

modules/salesforce_push/salesforce_push.install

@@ -7,7 +7,6 @@ use Drupal\salesforce_push\PushQueue;
  */
 function salesforce_push_install() {
   \Drupal::state()->set('salesforce.mapping_push_limit', PushQueue::MAPPING_CRON_PUSH_LIMIT);
-  \Drupal::state()->set('salesforce.global_push_limit', PushQueue::DEFAULT_GLOBAL_LIMIT);
   \Drupal::state()->set('salesforce.push_queue_processor', PushQueue::DEFAULT_QUEUE_PROCESSOR);
   \Drupal::state()->set('salesforce.push_queue_max_fails', PushQueue::DEFAULT_MAX_FAILS);
 }
@@ -44,4 +43,15 @@ function salesforce_push_update_2() {
   \Drupal::state()->set('salesforce.global_push_limit', PushQueue::DEFAULT_GLOBAL_LIMIT);
 
   \Drupal::state()->delete('salesforce.push_limit');
+}
+
+/**
+ * Moves global push limit out of state into config
+ */
+function salesforce_push_update_3() {
+  $config = \Drupal::configFactory()->getEditable('salesforce.settings');
+  $config
+    ->set('global_push_limit', \Drupal::state()->get('salesforce.global_push_limit'))
+    ->save();
+  \Drupal::state()->delete('salesforce.global_push_limit');
 }
\ No newline at end of file

modules/salesforce_push/salesforce_push.services.yml

@@ -5,4 +5,4 @@ services:
 
   queue.salesforce_push:
     class: Drupal\salesforce_push\PushQueue
-    arguments: ['@database', '@state', '@plugin.manager.salesforce_push_queue_processor', '@entity_type.manager', '@event_dispatcher', '@datetime.time']
+    arguments: ['@database', '@state', '@plugin.manager.salesforce_push_queue_processor', '@entity_type.manager', '@event_dispatcher', '@datetime.time', '@config.factory']

modules/salesforce_push/src/PushQueue.php

@@ -2,6 +2,8 @@
 
 namespace Drupal\salesforce_push;
 
+use Drupal\Component\Datetime\TimeInterface;
+use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Core\Database\Connection;
 use Drupal\Core\Database\Query\Merge;
 use Drupal\Core\Entity\EntityInterface;
@@ -12,12 +14,11 @@ use Drupal\Core\Queue\SuspendQueueException;
 use Drupal\Core\State\StateInterface;
 use Drupal\salesforce\EntityNotFoundException;
 use Drupal\salesforce\Event\SalesforceErrorEvent;
-use Drupal\salesforce\Event\SalesforceNoticeEvent;
-use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Drupal\salesforce\Event\SalesforceEvents;
-use Drupal\Component\Datetime\TimeInterface;
-use Symfony\Component\DependencyInjection\ContainerInterface;
+use Drupal\salesforce\Event\SalesforceNoticeEvent;
 use Drupal\salesforce_mapping\Entity\SalesforceMappingInterface;
+use Symfony\Component\DependencyInjection\ContainerInterface;
+use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 
 /**
  * Salesforce push queue.
@@ -69,13 +70,18 @@ class PushQueue extends DatabaseQueue implements PushQueueInterface {
    */
   protected $time;
 
+  /**
+   * @var Drupal\Core\Config\Config
+   */
+  protected $config;
+
   /**
    * Constructs a \Drupal\Core\Queue\DatabaseQueue object.
    *
    * @param \Drupal\Core\Database\Connection $connection
    *   The Connection object containing the key-value tables.
    */
-  public function __construct(Connection $connection, StateInterface $state, PushQueueProcessorPluginManager $queue_manager, EntityTypeManagerInterface $entity_manager, EventDispatcherInterface $event_dispatcher, TimeInterface $time) {
+  public function __construct(Connection $connection, StateInterface $state, PushQueueProcessorPluginManager $queue_manager, EntityTypeManagerInterface $entity_manager, EventDispatcherInterface $event_dispatcher, TimeInterface $time, ConfigFactoryInterface $config) {
     $this->connection = $connection;
     $this->state = $state;
     $this->queueManager = $queue_manager;
@@ -85,7 +91,8 @@ class PushQueue extends DatabaseQueue implements PushQueueInterface {
     $this->eventDispatcher = $event_dispatcher;
     $this->time = $time;
 
-    $this->global_limit = $state->get('salesforce.global_push_limit', static::DEFAULT_GLOBAL_LIMIT);
+    $this->config = $config->get('salesforce.settings');
+    $this->global_limit = $this->config->get('global_push_limit', static::DEFAULT_GLOBAL_LIMIT);
     $this->max_fails = $state->get('salesforce.push_queue_max_fails', static::DEFAULT_MAX_FAILS);
   }
 
@@ -96,7 +103,8 @@ class PushQueue extends DatabaseQueue implements PushQueueInterface {
       $container->get('plugin.manager.salesforce_push_queue_processor'),
       $container->get('entity_type.manager'),
       $container->get('event_dispatcher'),
-      $container->get('datetime.time')
+      $container->get('datetime.time'),
+      $container->get('config.factory')
     );
   }
 
@@ -354,6 +362,7 @@ class PushQueue extends DatabaseQueue implements PushQueueInterface {
     while (TRUE) {
       // Claim as many items as we can from this queue and advance our counter. If this queue is empty, move to the next mapping.
       $items = $this->claimItems($mapping->push_limit, $mapping->push_retries);
+      print_r($items);
       if (empty($items)) {
         $mapping->setLastPushTime($this->time->getRequestTime());
         return $i;

modules/salesforce_push/tests/src/Unit/PushQueueTest.php

@@ -2,7 +2,12 @@
 
 namespace Drupal\Tests\salesforce_push\Unit;
 
+use Drupal\Component\Datetime\TimeInterface;
+use Drupal\Core\Config\Config;
+use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Core\Database\Connection;
+use Drupal\Core\Database\Query\Update;
+use Drupal\Core\Database\StatementInterface;
 use Drupal\Core\DependencyInjection\ContainerBuilder;
 use Drupal\Core\Entity\EntityManagerInterface;
 use Drupal\Core\Entity\EntityTypeManagerInterface;
@@ -11,14 +16,12 @@ use Drupal\Core\State\StateInterface;
 use Drupal\Core\StringTranslation\TranslationInterface;
 use Drupal\Tests\UnitTestCase;
 use Drupal\salesforce_mapping\Entity\SalesforceMappingInterface;
-use Drupal\salesforce_push\PushQueue;
-use Symfony\Component\EventDispatcher\EventDispatcherInterface;
-use Drupal\salesforce_push\PushQueueProcessorPluginManager;
-use Drupal\Core\Database\StatementInterface;
-use Drupal\Core\Database\Query\Update;
 use Drupal\salesforce_mapping\SalesforceMappingStorage;
+use Drupal\salesforce_push\PushQueue;
 use Drupal\salesforce_push\PushQueueProcessorInterface;
-use Drupal\Component\Datetime\TimeInterface;
+use Drupal\salesforce_push\PushQueueProcessorPluginManager;
+use Symfony\Component\EventDispatcher\EventDispatcherInterface;
+use Prophecy\Argument;
 
 /**
  * Test Object instantitation.
@@ -70,6 +73,15 @@ class PushQueueTest extends UnitTestCase {
       ->with($this->equalTo('salesforce_mapped_object'))
       ->willReturn($this->mapped_object_storage);
 
+    // mock config
+    $prophecy = $this->prophesize(Config::CLASS);
+    $prophecy->get('global_push_limit', Argument::any())->willReturn(PushQueue::DEFAULT_GLOBAL_LIMIT);
+    $config = $prophecy->reveal();
+
+    $prophecy = $this->prophesize(ConfigFactoryInterface::CLASS);
+    $prophecy->get('salesforce.settings')->willReturn($config);
+    $this->configFactory = $prophecy->reveal();
+
     $container = new ContainerBuilder();
     $container->set('database', $this->database);
     $container->set('state', $this->state);
@@ -79,6 +91,7 @@ class PushQueueTest extends UnitTestCase {
     $container->set('entity.manager', $this->entity_manager);
     $container->set('plugin.manager.salesforce_push_queue_processor', $this->push_queue_processor_plugin_manager);
     $container->set('datetime.time', $this->time);
+    $container->set('config.factory', $this->configFactory);
     \Drupal::setContainer($container);
   }
 
@@ -130,7 +143,7 @@ class PushQueueTest extends UnitTestCase {
   /**
    * @covers ::processQueues
    */
-  public function testProcessQueues() {
+  public function testProcessQueue() {
     $items = [1, 2, 3];
     $mapping1 = $this->getMock(SalesforceMappingInterface::CLASS);
     $mapping1->expects($this->any())
@@ -142,11 +155,6 @@ class PushQueueTest extends UnitTestCase {
     $mapping1->push_limit = 1;
     $mapping1->push_retries = 1;
 
-    $mappings =
-    $this->mapping_storage->expects($this->once())
-      ->method('loadPushMappings')
-      ->willReturn([$mapping1]);
-
     $this->worker = $this->getMock(PushQueueProcessorInterface::class);
     $this->worker->expects($this->once())
       ->method('process')
@@ -155,15 +163,17 @@ class PushQueueTest extends UnitTestCase {
       ->method('createInstance')
       ->willReturn($this->worker);
 
-    $this->queue = $this->getMock(PushQueue::class, ['claimItems', 'setName'], [$this->database, $this->state, $this->push_queue_processor_plugin_manager, $this->entityTypeManager, $this->eventDispatcher, $this->time]);
-    $this->queue->expects($this->once())
+    $this->queue = $this->getMock(PushQueue::class, ['claimItems', 'setName'], [$this->database, $this->state, $this->push_queue_processor_plugin_manager, $this->entityTypeManager, $this->eventDispatcher, $this->time, $this->configFactory]);
+
+    // I don't know why at(1) works.
+    $this->queue->expects($this->at(1))
       ->method('claimItems')
       ->willReturn($items);
     $this->queue->expects($this->once())
       ->method('setName')
       ->willReturn(NULL);
 
-    $this->queue->processQueues();
+    $this->assertEquals(3, $this->queue->processQueue($mapping1));
 
   }
 

src/Form/SettingsForm.php

@@ -5,7 +5,6 @@ namespace Drupal\salesforce\Form;
 use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Core\Form\ConfigFormBase;
 use Drupal\Core\Form\FormStateInterface;
-use Drupal\Core\State\StateInterface;
 use Drupal\salesforce\Rest\RestClientInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
@@ -30,13 +29,6 @@ class SettingsForm extends ConfigFormBase {
    */
   protected $eventDispatcher;
 
-  /**
-   * The state keyvalue collection.
-   *
-   * @var \Drupal\Core\State\StateInterface
-   */
-  protected $state;
-
   /**
    * Constructs a \Drupal\system\ConfigFormBase object.
    *
@@ -44,13 +36,10 @@ class SettingsForm extends ConfigFormBase {
    *   The factory for configuration objects.
    * @param \Drupal\salesforce\Rest\RestClientInterface $salesforce_client
    *   The factory for configuration objects.
-   * @param \Drupal\Core\State\StateInterface $state
-   *   The state keyvalue collection to use.
    */
-  public function __construct(ConfigFactoryInterface $config_factory, RestClientInterface $salesforce_client, StateInterface $state, EventDispatcherInterface $event_dispatcher) {
+  public function __construct(ConfigFactoryInterface $config_factory, RestClientInterface $salesforce_client, EventDispatcherInterface $event_dispatcher) {
     parent::__construct($config_factory);
     $this->sf_client = $salesforce_client;
-    $this->state = $state;
     $this->eventDispatcher = $event_dispatcher;
   }
 
@@ -61,7 +50,6 @@ class SettingsForm extends ConfigFormBase {
     return new static(
       $container->get('config.factory'),
       $container->get('salesforce.client'),
-      $container->get('state'),
       $container->get('event_dispatcher')
     );
   }
@@ -110,12 +98,12 @@ class SettingsForm extends ConfigFormBase {
       ],
     ];
 
-    $form['push_limit'] = [
+    $form['global_push_limit'] = [
       '#title' => $this->t('Global push limit'),
       '#type' => 'number',
       '#description' => $this->t('Set the maximum number of records to be processed during each push queue process. Enter 0 for no limit.'),
       '#required' => TRUE,
-      '#default_value' => $config->get('push_limit'),
+      '#default_value' => $config->get('global_push_limit'),
       '#min' => 0,
     ];
 
@@ -171,7 +159,7 @@ class SettingsForm extends ConfigFormBase {
     $config = $this->config('salesforce.settings');
     $config->set('show_all_objects', $form_state->getValue('show_all_objects'));
     $config->set('standalone', $form_state->getValue('standalone'));
-    $config->set('push_limit', $form_state->getValue('push_limit'));
+    $config->set('global_push_limit', $form_state->getValue('global_push_limit'));
     $config->set('pull_max_queue_size', $form_state->getValue('pull_max_queue_size'));
     $use_latest = $form_state->getValue('use_latest');
     $config->set('use_latest', $use_latest);