Test improvement for import routine.

app/Import/Routine/BunqRoutine.php

@@ -78,13 +78,13 @@ class BunqRoutine implements RoutineInterface
                     $handler->run();
                     $transactions = $handler->getTransactions();
                     // could be that more transactions will arrive in a second run.
-                    if (true === $handler->stillRunning) {
+                    if (true === $handler->isStillRunning()) {
                         Log::debug('Handler indicates that it is still working.');
                         $this->repository->setStatus($this->importJob, 'ready_to_run');
                         $this->repository->setStage($this->importJob, 'go-for-import');
                     }
                     $this->repository->appendTransactions($this->importJob, $transactions);
-                    if (false === $handler->stillRunning) {
+                    if (false === $handler->isStillRunning()) {
                         Log::info('Handler indicates that its done!');
                         $this->repository->setStatus($this->importJob, 'provider_finished');
                         $this->repository->setStage($this->importJob, 'final');
@@ -98,6 +98,8 @@ class BunqRoutine implements RoutineInterface
     }
 
 
+
+
     /**
      * Set the import job.
      *

app/Import/Storage/ImportArrayStorage.php

@@ -46,7 +46,7 @@ use Illuminate\Support\Collection;
 use Log;
 
 /**
- * Creates new transactions based upon arrays. Will first check the array for duplicates.
+ * Creates new transactions based on arrays.
  *
  * Class ImportArrayStorage
  *
@@ -58,11 +58,11 @@ class ImportArrayStorage
     private $checkForTransfers = false;
     /** @var ImportJob The import job */
     private $importJob;
-    /** @var JournalRepositoryInterface */
+    /** @var JournalRepositoryInterface Journal repository for storage. */
     private $journalRepos;
     /** @var ImportJobRepositoryInterface Import job repository */
     private $repository;
-    /** @var Collection The transfers. */
+    /** @var Collection The transfers the user already has. */
     private $transfers;
 
     /**
@@ -152,12 +152,10 @@ class ImportArrayStorage
 
     /**
      * Count the number of transfers in the array. If this is zero, don't bother checking for double transfers.
-     *
-     * @SuppressWarnings(PHPMD.CyclomaticComplexity)
      */
     private function countTransfers(): void
     {
-        Log::debug('Now in count transfers.');
+        Log::debug('Now in countTransfers()');
         /** @var array $array */
         $array = $this->importJob->transactions;
         $count = 0;
@@ -167,17 +165,44 @@ class ImportArrayStorage
                 Log::debug(sprintf('Row #%d is a transfer, increase count to %d', $index + 1, $count));
             }
         }
-        if (0 === $count) {
+        Log::debug(sprintf('Count of transfers in import array is %d.', $count));
-            Log::debug('Count is zero, will not check for duplicate transfers.');
-        }
         if ($count > 0) {
-            Log::debug(sprintf('Count is %d, will check for duplicate transfers.', $count));
             $this->checkForTransfers = true;
-
+            Log::debug('Will check for duplicate transfers.');
             // get users transfers. Needed for comparison.
             $this->getTransfers();
         }
+    }
 
+    /**
+     * @param int   $index
+     * @param array $transaction
+     *
+     * @return bool
+     * @throws FireflyException
+     */
+    private function duplicateDetected(int $index, array $transaction): bool
+    {
+        $hash       = $this->getHash($transaction);
+        $existingId = $this->hashExists($hash);
+        if (null !== $existingId) {
+            $message = sprintf('Row #%d ("%s") could not be imported. It already exists.', $index, $transaction['description']);
+            $this->logDuplicateObject($transaction, $existingId);
+            $this->repository->addErrorMessage($this->importJob, $message);
+
+            return true;
+        }
+
+        // do transfer detection:
+        if ($this->checkForTransfers && $this->transferExists($transaction)) {
+            $message = sprintf('Row #%d ("%s") could not be imported. Such a transfer already exists.', $index, $transaction['description']);
+            $this->logDuplicateTransfer($transaction);
+            $this->repository->addErrorMessage($this->importJob, $message);
+
+            return true;
+        }
+
+        return false;
     }
 
     /**
@@ -397,34 +422,17 @@ class ImportArrayStorage
         $count   = \count($array);
         $toStore = [];
 
-        Log::debug(sprintf('Now in store(). Count of items is %d', $count));
+        Log::debug(sprintf('Now in store(). Count of items is %d.', $count));
 
+        /*
+         * Detect duplicates in initial array:
+         */
         foreach ($array as $index => $transaction) {
             Log::debug(sprintf('Now at item %d out of %d', $index + 1, $count));
-            $hash       = $this->getHash($transaction);
+            if ($this->duplicateDetected($index, $transaction)) {
-            $existingId = $this->hashExists($hash);
-            if (null !== $existingId) {
-                $this->logDuplicateObject($transaction, $existingId);
-                $this->repository->addErrorMessage(
-                    $this->importJob, sprintf(
-                                        'Row #%d ("%s") could not be imported. It already exists.',
-                                        $index, $transaction['description']
-                                    )
-                );
                 continue;
             }
-            if ($this->checkForTransfers && $this->transferExists($transaction)) {
+            $transaction['importHashV2'] = $this->getHash($transaction);
-                $this->logDuplicateTransfer($transaction);
-                $this->repository->addErrorMessage(
-                    $this->importJob, sprintf(
-                                        'Row #%d ("%s") could not be imported. Such a transfer already exists.',
-                                        $index,
-                                        $transaction['description']
-                                    )
-                );
-                continue;
-            }
-            $transaction['importHashV2'] = $hash;
             $toStore[]                   = $transaction;
         }
         $count = \count($toStore);
@@ -438,31 +446,8 @@ class ImportArrayStorage
         // now actually store them:
         $collection = new Collection;
         foreach ($toStore as $index => $store) {
-
             // do duplicate detection again!
-            $hash       = $this->getHash($store);
+            if ($this->duplicateDetected($index, $store)) {
-            $existingId = $this->hashExists($hash);
-            if (null !== $existingId) {
-                $this->logDuplicateObject($store, $existingId);
-                $this->repository->addErrorMessage(
-                    $this->importJob, sprintf(
-                                        'Row #%d ("%s") could not be imported. It already exists.',
-                                        $index, $store['description']
-                                    )
-                );
-                continue;
-            }
-
-            // do transfer detection again!
-            if ($this->checkForTransfers && $this->transferExists($store)) {
-                $this->logDuplicateTransfer($store);
-                $this->repository->addErrorMessage(
-                    $this->importJob, sprintf(
-                                        'Row #%d ("%s") could not be imported. Such a transfer already exists.',
-                                        $index,
-                                        $store['description']
-                                    )
-                );
                 continue;
             }
 

app/Support/Import/Routine/Bunq/StageImportDataHandler.php

@@ -80,6 +80,14 @@ class StageImportDataHandler
         return $this->transactions;
     }
 
+    /**
+     * @return bool
+     */
+    public function isStillRunning(): bool
+    {
+        return $this->stillRunning;
+    }
+
     /**
      *
      * @throws FireflyException

tests/Unit/Import/Routine/BunqRoutineTest.php

@@ -71,13 +71,59 @@ class BunqRoutineTest extends TestCase
 
         $repository->shouldReceive('setUser')->once();
         $repository->shouldReceive('setStatus')->withArgs([Mockery::any(), 'running']);
+        $repository->shouldReceive('setStatus')->withArgs([Mockery::any(), 'provider_finished']);
+        $repository->shouldReceive('setStage')->withArgs([Mockery::any(), 'final']);
+        $repository->shouldReceive('appendTransactions')->withArgs([Mockery::any(), ['a' => 'c']])->once();
+
         $handler->shouldReceive('setImportJob')->once();
         $handler->shouldReceive('run')->once();
         $handler->shouldReceive('getTransactions')->once()->andReturn(['a' => 'c']);
-        //$repository->shouldReceive('setStatus')->withArgs([Mockery::any(), 'provider_finished'])->once();
+        $handler->shouldReceive('isStillRunning')->andReturn(false);
-        //$repository->shouldReceive('setStage')->withArgs([Mockery::any(), 'final'])->once();
+        $routine = new BunqRoutine;
+        $routine->setImportJob($job);
+        try {
+            $routine->run();
+        } catch (FireflyException $e) {
+            $this->assertFalse(true, $e->getMessage());
+        }
+
+    }
+
+    /**
+     * @covers \FireflyIII\Import\Routine\BunqRoutine
+     */
+    public function testRunImportStillRunning(): void
+    {
+        $job                = new ImportJob;
+        $job->user_id       = $this->user()->id;
+        $job->key           = 'brY_' . random_int(1, 10000);
+        $job->status        = 'ready_to_run';
+        $job->stage         = 'go-for-import';
+        $job->provider      = 'bunq';
+        $job->file_type     = '';
+        $job->configuration = [];
+        $job->save();
+
+        // mock stuff:
+        $repository = $this->mock(ImportJobRepositoryInterface::class);
+        $handler    = $this->mock(StageImportDataHandler::class);
+
+
+
+        $handler->shouldReceive('setImportJob')->once();
+        $handler->shouldReceive('run')->once();
+        $handler->shouldReceive('getTransactions')->once()->andReturn(['a' => 'c']);
+        $handler->shouldReceive('isStillRunning')->andReturn(true);
+
+        $repository->shouldReceive('setStatus')->withArgs([Mockery::any(), 'ready_to_run']);
+        $repository->shouldReceive('setStage')->withArgs([Mockery::any(), 'go-for-import']);
+
+        $repository->shouldReceive('setUser')->once();
+        $repository->shouldReceive('setStatus')->withArgs([Mockery::any(), 'running']);
         $repository->shouldReceive('appendTransactions')->withArgs([Mockery::any(), ['a' => 'c']])->once();
 
+
+
         $routine = new BunqRoutine;
         $routine->setImportJob($job);
         try {
@@ -88,6 +134,7 @@ class BunqRoutineTest extends TestCase
 
     }
 
+
     /**
      * @covers \FireflyIII\Import\Routine\BunqRoutine
      */