Commit 34db3c9f authored by Youri van Koppen's avatar Youri van Koppen
Browse files

Issue #3261348 by MegaChriz: Fixed EmptyFeedException in FeedsQueueExecutable... 

Issue #3261348 by MegaChriz: Fixed EmptyFeedException in FeedsQueueExecutable not catched, causing to fill up the log. Don't unlock the feed in case of other exceptions during cron imports - to prevent some simultaneous imports of the same feed.
parent 444bb0c4

src/FeedsQueueExecutable.php

+5 −4
Original line number Diff line number Diff line
@@ -6,7 +6,7 @@ use Drupal\Core\Entity\EntityTypeManagerInterface; 
use Drupal\Core\Messenger\MessengerInterface;

use Drupal\Core\Queue\QueueFactory;

use Drupal\Core\Session\AccountSwitcherInterface;

use Exception;

use Drupal\feeds\Exception\EmptyFeedException;

use Symfony\Component\DependencyInjection\ContainerInterface;

use Symfony\Component\EventDispatcher\EventDispatcherInterface;
@@ -64,13 +64,14 @@ class FeedsQueueExecutable extends FeedsExecutable { 
  /**

   * {@inheritdoc}

   */

  protected function handleException(FeedInterface $feed, $stage, array $params, Exception $exception) {

    $feed->finishImport();



  protected function handleException(FeedInterface $feed, $stage, array $params, \Exception $exception) {

    if ($exception instanceof EmptyFeedException) {

      $feed->finishImport();

      return;

    }



    // On an exception, the queue item remains on the queue so we need to keep

    // the feed locked.

    throw $exception;

  }

src/Plugin/QueueWorker/FeedQueueWorkerBase.php

+5 −4
Original line number Diff line number Diff line
@@ -87,12 +87,13 @@ abstract class FeedQueueWorkerBase extends QueueWorkerBase implements ContainerF 
   * Handles an import exception.

   */

  protected function handleException(FeedInterface $feed, \Exception $exception) {

    if ($exception instanceof EmptyFeedException) {

      $feed->finishImport();

      return;

    }



    if (!$exception instanceof EmptyFeedException) {

    throw $exception;

  }

  }



  /**

   * Safely switches to another account.

tests/src/Functional/CronTest.php

+74 −1
Original line number Diff line number Diff line
@@ -92,7 +92,10 @@ class CronTest extends FeedsBrowserTestBase { 
  public function testImportSourceWithMultipleCronRuns() {

    // Install module that alters how many items can be processed per cron run.

    // By default, the module limits the number of processable items to 5.

    $this->container->get('module_installer')->install(['feeds_test_files', 'feeds_test_multiple_cron_runs']);

    $this->container->get('module_installer')->install([

      'feeds_test_files',

      'feeds_test_multiple_cron_runs',

    ]);

    $this->rebuildContainer();



    // Create a feed type. Do not set a column as unique.
@@ -170,4 +173,74 @@ class CronTest extends FeedsBrowserTestBase { 
    $feed->startCronImport();

  }



  /**

   * Tests that an unchanged feed finishes an import correctly.

   *

   * When a source gets fetched, but it is unchanged, the import gets aborted

   * early. In such case we want that:

   * - the fetch tasks no longer remains on the queue;

   * - the feed gets unlocked so a new import can be done.

   */

  public function testUnchangedFeedImport() {

    // Install module that will throw a 304 when the same data is fetched again.

    $this->container->get('module_installer')->install(['feeds_test_files']);

    $this->rebuildContainer();



    // Create a feed type. Do not set a column as unique.

    $feed_type = $this->createFeedTypeForCsv([

      'guid' => 'GUID',

      'title' => 'Title',

    ], [

      'fetcher' => 'http',

      'fetcher_configuration' => [],

      'mappings' => [

        [

          'target' => 'feeds_item',

          'map' => ['guid' => 'guid'],

        ],

        [

          'target' => 'title',

          'map' => ['value' => 'title'],

        ],

      ],

    ]);

    $queue_name = 'feeds_feed_refresh:' . $feed_type->id();



    // Create a feed that contains 9 items.

    $feed = $this->createFeed($feed_type->id(), [

      'source' => \Drupal::request()->getSchemeAndHttpHost() . '/testing/feeds/nodes.csv',

    ]);



    // Schedule import.

    $feed->startCronImport();

    $this->assertTrue($feed->isLocked());

    $this->assertQueueItemCount(1, $queue_name);



    // Run cron. Nine nodes should be imported.

    $this->cronRun();

    $this->assertNodeCount(9);



    // Check that the queue is empty and that the feed is unlocked.

    $this->assertQueueItemCount(0, $queue_name);

    $feed = $this->reloadEntity($feed);

    $this->assertFalse($feed->isLocked());

    // Unlock the feed manually again, since it still exists in memory.

    // @see \Drupal\Core\Lock\DatabaseLockBackend::acquire()

    $feed->unlock();



    // Schedule another import and run cron again. No nodes should be imported.

    // The total should remain 9.

    $feed->startCronImport();

    $this->assertTrue($feed->isLocked());



    $this->assertQueueItemCount(1, $queue_name);

    $this->cronRun();

    $this->assertNodeCount(9);



    // Check that the queue is empty and that the feed is unlocked.

    $this->assertQueueItemCount(0, $queue_name);

    $feed = $this->reloadEntity($feed);

    $this->assertFalse($feed->isLocked());

  }



}

tests/src/Functional/QueueTest.php

+5 −2
Original line number Diff line number Diff line
@@ -86,6 +86,10 @@ class QueueTest extends FeedsBrowserTestBase { 
    // Assert that 6 nodes have been created.

    $this->assertNodeCount(6);



    // Unlock the feed manually again, since it still exists in memory.

    // @see \Drupal\Core\Lock\DatabaseLockBackend::acquire()

    $feed->unlock();



    // Add feed to queue again but delete the feed before cron has run.

    $feed->startCronImport();

    $feed->delete();
@@ -94,8 +98,7 @@ class QueueTest extends FeedsBrowserTestBase { 
    $this->cronRun();



    // Assert that the queue is empty.

    $queue = \Drupal::service('queue')->get('feeds_feed_refresh:' . $feed_type->id());

    $this->assertEquals(0, $queue->numberOfItems());

    $this->assertQueueItemCount(0, 'feeds_feed_refresh:' . $feed_type->id());

  }



}

tests/src/Traits/FeedsCommonTrait.php

+24 −0
Original line number Diff line number Diff line
@@ -145,6 +145,30 @@ trait FeedsCommonTrait { 
    ]));

  }



  /**

   * Asserts that the given number of queue items exist for the specified queue.

   *

   * @param int $expected

   *   The expected number of queue items.

   * @param string $queue_name

   *   The queue to inspect the number of items for.

   * @param string $message

   *   (optional) The message to assert.

   */

  protected function assertQueueItemCount(int $expected, string $queue_name, string $message = '') {

    if (!$message) {

      $message = '@expected queue items exist on @queue (actual: @count).';

    }



    $queue = $this->container->get('queue')->get($queue_name);

    $item_count = $queue->numberOfItems();

    $this->assertEquals($expected, $item_count, strtr($message, [

      '@expected' => $expected,

      '@queue' => $queue_name,

      '@count' => $item_count,

    ]));

  }



  /**

   * Returns the absolute path to the Drupal root.

   *