From dfa9e537b330185f4f90da9b3a5d0ea4aa6b95f3 Mon Sep 17 00:00:00 2001 From: James Cole Date: Thu, 30 Aug 2018 19:12:52 +0200 Subject: [PATCH] Improve the bunq routine so it will keep looping when faced with default PHP time out settings (30 seconds). --- app/Import/Routine/BunqRoutine.php | 15 +- .../ImportJob/ImportJobRepository.php | 27 +++ .../ImportJobRepositoryInterface.php | 10 + .../Routine/Bunq/StageImportDataHandler.php | 186 ++++++++++++++---- public/js/ff/import/status_v2.js | 2 +- 5 files changed, 193 insertions(+), 47 deletions(-) diff --git a/app/Import/Routine/BunqRoutine.php b/app/Import/Routine/BunqRoutine.php index 192ff26b4b..c53c75a720 100644 --- a/app/Import/Routine/BunqRoutine.php +++ b/app/Import/Routine/BunqRoutine.php @@ -77,10 +77,19 @@ class BunqRoutine implements RoutineInterface $handler->setImportJob($this->importJob); $handler->run(); $transactions = $handler->getTransactions(); + // could be that more transactions will arrive in a second run. + if (true === $handler->stillRunning) { + 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) { + Log::info('Handler indicates that its done!'); + $this->repository->setStatus($this->importJob, 'provider_finished'); + $this->repository->setStage($this->importJob, 'final'); + } - $this->repository->setTransactions($this->importJob, $transactions); - $this->repository->setStatus($this->importJob, 'provider_finished'); - $this->repository->setStage($this->importJob, 'final'); return; } diff --git a/app/Repositories/ImportJob/ImportJobRepository.php b/app/Repositories/ImportJob/ImportJobRepository.php index 120e612f40..7b65e7f5cd 100644 --- a/app/Repositories/ImportJob/ImportJobRepository.php +++ b/app/Repositories/ImportJob/ImportJobRepository.php @@ -73,6 +73,33 @@ class ImportJobRepository implements ImportJobRepositoryInterface return $job; } + /** + * Append transactions to array instead of replacing them. + * + * @param ImportJob $job + * @param array $transactions + * + * @return ImportJob + */ + public function appendTransactions(ImportJob $job, array $transactions): ImportJob + { + Log::debug(sprintf('Now in appendTransactions(%s)', $job->key)); + $existingTransactions = $job->transactions; + if (!\is_array($existingTransactions)) { + $existingTransactions = []; + } + $new = array_merge($existingTransactions, $transactions); + Log::debug(sprintf('Old transaction count: %d', \count($existingTransactions))); + Log::debug(sprintf('To be added transaction count: %d', \count($transactions))); + Log::debug(sprintf('New count: %d', \count($new))); + $job->transactions = $new; + + + $job->save(); + + return $job; + } + /** * @param string $importProvider * diff --git a/app/Repositories/ImportJob/ImportJobRepositoryInterface.php b/app/Repositories/ImportJob/ImportJobRepositoryInterface.php index 42248dd461..11db5d8186 100644 --- a/app/Repositories/ImportJob/ImportJobRepositoryInterface.php +++ b/app/Repositories/ImportJob/ImportJobRepositoryInterface.php @@ -45,6 +45,16 @@ interface ImportJobRepositoryInterface */ public function addErrorMessage(ImportJob $job, string $error): ImportJob; + /** + * Append transactions to array instead of replacing them. + * + * @param ImportJob $job + * @param array $transactions + * + * @return ImportJob + */ + public function appendTransactions(ImportJob $job, array $transactions): ImportJob; + /** * @param string $importProvider * diff --git a/app/Support/Import/Routine/Bunq/StageImportDataHandler.php b/app/Support/Import/Routine/Bunq/StageImportDataHandler.php index 6e26db17ef..13de86e355 100644 --- a/app/Support/Import/Routine/Bunq/StageImportDataHandler.php +++ b/app/Support/Import/Routine/Bunq/StageImportDataHandler.php @@ -44,6 +44,11 @@ use Log; */ class StageImportDataHandler { + private const DOWNLOAD_BACKWARDS = 1; + private const DOWNLOAD_FORWARDS = 2; + + /** @var bool */ + public $stillRunning; /** @var AccountFactory */ private $accountFactory; /** @var AccountRepositoryInterface */ @@ -54,9 +59,18 @@ class StageImportDataHandler private $jobConfiguration; /** @var ImportJobRepositoryInterface */ private $repository; + /** @var float */ + private $timeStart; /** @var array */ private $transactions; + public function __construct() + { + $this->stillRunning = true; + $this->timeStart = microtime(true); + + } + /** * @codeCoverageIgnore * @return array @@ -272,6 +286,40 @@ class StageImportDataHandler throw new FireflyException('The bunq API context is unexpectedly empty.'); // @codeCoverageIgnore } + /** + * Get the direction in which we must download. + * + * @param int $bunqAccountId + * + * @return int + */ + private function getDirection(int $bunqAccountId): int + { + Log::debug(sprintf('Now in getDirection(%d)', $bunqAccountId)); + + // if oldest transaction ID is 0, AND the newest transaction is 0 + // we don't know about this account, so we must go backward in time. + $oldest = \Preferences::getForUser($this->importJob->user, sprintf('bunq-oldest-transaction-%d', $bunqAccountId), 0); + $newest = \Preferences::getForUser($this->importJob->user, sprintf('bunq-newest-transaction-%d', $bunqAccountId), 0); + + if (0 === $oldest->data && 0 === $newest->data) { + Log::debug(sprintf('Oldest tranaction ID is %d and newest tranasction ID is %d, so go backwards.', $oldest->data, $newest->data)); + + return self::DOWNLOAD_BACKWARDS; + } + + // if newest is not zero but oldest is zero, go forward. + if(0 === $oldest->data && 0 !== $newest->data) { + Log::debug(sprintf('Oldest tranaction ID is %d and newest tranasction ID is %d, so go backwards.', $oldest->data, $newest->data)); + + return self::DOWNLOAD_FORWARDS; + } + + Log::debug(sprintf('Oldest tranaction ID is %d and newest tranasction ID is %d, so go backwards.', $oldest->data, $newest->data)); + + return self::DOWNLOAD_BACKWARDS; + } + /** * @param int $accountId * @@ -300,89 +348,128 @@ class StageImportDataHandler */ private function getTransactionsFromBunq(int $bunqAccountId, LocalAccount $localAccount): array { - Log::debug('Now in getTransactionsFromBunq(%d).'); + Log::debug(sprintf('Now in getTransactionsFromBunq(%d).', $bunqAccountId)); - // what was the last transaction we grabbed from bunq? - $return = []; - $preferenceName = sprintf('bunq-last-transaction-%d', $bunqAccountId); - $transactionPref = \Preferences::getForUser($this->importJob->user, $preferenceName, 0); - $transactionId = (int)$transactionPref->data; - - Log::debug(sprintf('ID of latest transaction is #%d', $transactionId)); - - if (0 === $transactionId) { - Log::debug('Its zero so we go back in time.'); - // we go back into the past, way until the system says there is no more. - $return = $this->goBackInTime($bunqAccountId, $localAccount); + $direction = $this->getDirection($bunqAccountId); + $return = []; + if (self::DOWNLOAD_BACKWARDS === $direction) { + // go back either from NULL or from ID. + // ID is the very last transaction downloaded from bunq. + $preference = \Preferences::getForUser($this->importJob->user, sprintf('bunq-oldest-transaction-%d', $bunqAccountId), 0); + $transactionId = 0 === $preference->data ? null : $preference->data; + $return = $this->goBackInTime($bunqAccountId, $localAccount, $transactionId); } - if (0 !== $transactionId) { + if (self::DOWNLOAD_FORWARDS === $direction) { + // go forward from ID. There is no NULL, young padawan $return = $this->goForwardInTime($bunqAccountId, $localAccount); - // work my way forward. } return $return; } /** + * This method downloads the transactions from bunq going back in time. Assuming bunq + * is fairly consistent with the transactions it provides through the API, the method + * will store both the highest and the lowest transaction ID downloaded in this manner. + * + * The highest transaction ID is used to continue forward in time. The lowest is used to continue + * even further back in time. + * + * The lowest transaction ID can also be given to this method as a parameter (as $startTransaction). + * * @param int $bunqAccountId * @param LocalAccount $localAccount + * @param int $startTransaction * * @return array * @throws FireflyException */ - private function goBackInTime(int $bunqAccountId, LocalAccount $localAccount): array + private function goBackInTime(int $bunqAccountId, LocalAccount $localAccount, int $startTransaction = null): array { - Log::debug('Now in goBackInTime().'); - $hasMoreTransactions = true; - $olderId = null; - $count = 0; - $return = []; - $veryFirstTransaction = null; + Log::debug(sprintf('Now in goBackInTime(#%d, #%s, #%s).', $bunqAccountId, $localAccount->id, $startTransaction)); + $hasMoreTransactions = true; + $olderId = $startTransaction; + $oldestTransaction = null; + $newestTransaction = null; + $count = 0; + $return = []; - // loop die loop! - while ($hasMoreTransactions && $count < 50) { + /* + * Do a loop during which we run: + */ + while ($hasMoreTransactions && $this->timeRunning() < 25) { Log::debug(sprintf('Now in loop #%d', $count)); + Log::debug(sprintf('Now running for %s seconds.', $this->timeRunning())); + + /* + * Send request to bunq. + */ /** @var Payment $paymentRequest */ $paymentRequest = app(Payment::class); - $response = $paymentRequest->listing($bunqAccountId, ['count' => 100, 'older_id' => $olderId]); + $params = ['count' => 107, 'older_id' => $olderId]; + $response = $paymentRequest->listing($bunqAccountId, $params); $pagination = $response->getPagination(); + Log::debug('Params for the request to bunq are: ', $params); + /* * If pagination is not null, we can go back even further. */ if (null !== $pagination) { $olderId = $pagination->getOlderId(); - Log::debug(sprintf('Pagination object is not null, olderID is "%s"', $olderId)); + Log::debug(sprintf('Pagination object is not null, new olderID is "%s"', $olderId)); } - Log::debug('Now looping results...'); + + /* + * Loop the results from bunq + */ + Log::debug('Now looping results from bunq...'); /** @var BunqPayment $payment */ - foreach ($response->getValue() as $payment) { - $return[] = $this->convertPayment($payment, $bunqAccountId, $localAccount); - - // store the very first transaction ID for this particular account. - if (null === $veryFirstTransaction) { - $veryFirstTransaction = $payment->getId(); - } + foreach ($response->getValue() as $index => $payment) { + $return[] = $this->convertPayment($payment, $bunqAccountId, $localAccount); + $paymentId = $payment->getId(); + /* + * If oldest and newest transaction are null, they have to be set: + */ + $oldestTransaction = $oldestTransaction ?? $paymentId; + $newestTransaction = $newestTransaction ?? $paymentId; + /* + * Then, overwrite if appropriate + */ + $oldestTransaction = $paymentId < $oldestTransaction ? $paymentId : $oldestTransaction; + $newestTransaction = $paymentId > $newestTransaction ? $paymentId : $newestTransaction; } + + /* + * After the loop, check if Firefly III must loop again. + */ Log::debug(sprintf('Count of result is now %d', \count($return))); $count++; if (null === $olderId) { Log::debug('Older ID is NULL, so stop looping cause we are done!'); $hasMoreTransactions = false; + $this->stillRunning = false; + /* + * We no longer care about the oldest transaction ID: + */ + $oldestTransaction = 0; } if (null === $pagination) { Log::debug('No pagination object, stop looping.'); $hasMoreTransactions = false; + $this->stillRunning = false; + /* + * We no longer care about the oldest transaction ID: + */ + $oldestTransaction = 0; } sleep(1); + + } - Log::debug(sprintf('Done with looping. Final loop count is %d, first transaction is %d', $count, $veryFirstTransaction)); - if (null !== $veryFirstTransaction) { - Log::debug('Very first transaction is not null, so set the preference!'); - $preferenceName = sprintf('bunq-last-transaction-%d', $bunqAccountId); - $pref = \Preferences::setForUser($this->importJob->user, $preferenceName, $veryFirstTransaction); - Log::debug(sprintf('Preference set to: %s', $pref->data)); - } + // store newest and oldest tranasction ID to be used later: + \Preferences::setForUser($this->importJob->user, sprintf('bunq-oldest-transaction-%d', $bunqAccountId), $oldestTransaction); + \Preferences::setForUser($this->importJob->user, sprintf('bunq-newest-transaction-%d', $bunqAccountId), $newestTransaction); return $return; } @@ -408,11 +495,12 @@ class StageImportDataHandler $newerId = (int)$transactionPref->data; // loop die loop! - while ($hasMoreTransactions && $count < 50) { + while ($hasMoreTransactions && $this->timeRunning() < 25) { Log::debug(sprintf('Now in loop #%d', $count)); + Log::debug(sprintf('Now running for %s seconds.', $this->timeRunning())); /** @var Payment $paymentRequest */ $paymentRequest = app(Payment::class); - $params = ['count' => 100, 'newer_id' => $newerId]; + $params = ['count' => 107, 'newer_id' => $newerId]; $response = $paymentRequest->listing($bunqAccountId, $params); $pagination = $response->getPagination(); Log::debug('Submit payment request with params', $params); @@ -436,10 +524,12 @@ class StageImportDataHandler if (null === $newerId) { Log::debug('Newer ID is NULL, so stop looping cause we are done!'); $hasMoreTransactions = false; + $this->stillRunning = false; } if (null === $pagination) { Log::debug('No pagination object, stop looping.'); $hasMoreTransactions = false; + $this->stillRunning = false; } sleep(1); } @@ -453,4 +543,14 @@ class StageImportDataHandler return $return; } + + /** + * @return float + */ + private function timeRunning(): float + { + $time_end = microtime(true); + + return $time_end - $this->timeStart; + } } diff --git a/public/js/ff/import/status_v2.js b/public/js/ff/import/status_v2.js index d5cafb34a7..080c447793 100644 --- a/public/js/ff/import/status_v2.js +++ b/public/js/ff/import/status_v2.js @@ -25,7 +25,7 @@ var jobRunRoutineStarted = false; var jobStorageRoutineStarted = false; var checkInitialInterval = 1000; var checkNextInterval = 500; -var maxLoops = 60; +var maxLoops = 65536; var totalLoops = 0; var startCount = 0; var jobFailed = false;