From c83dfc44d648a26c2bacf0ca847df44733a577a2 Mon Sep 17 00:00:00 2001 From: James Cole Date: Fri, 28 Apr 2017 20:08:04 +0200 Subject: [PATCH] Update internal filters. --- app/Helpers/Collector/JournalCollector.php | 165 ++++++------------ .../Collector/JournalCollectorInterface.php | 29 ++- app/Helpers/Filter/AmountFilter.php | 14 +- app/Helpers/Filter/EmptyFilter.php | 6 +- app/Helpers/Filter/FilterInterface.php | 3 +- app/Helpers/Filter/InternalTransferFilter.php | 21 ++- app/Helpers/Filter/OpposingAccountFilter.php | 21 ++- app/Helpers/Filter/TransferFilter.php | 3 +- 8 files changed, 116 insertions(+), 146 deletions(-) diff --git a/app/Helpers/Collector/JournalCollector.php b/app/Helpers/Collector/JournalCollector.php index 2e286412da..1aff3caa46 100644 --- a/app/Helpers/Collector/JournalCollector.php +++ b/app/Helpers/Collector/JournalCollector.php @@ -18,12 +18,15 @@ use Carbon\Carbon; use Crypt; use DB; use FireflyIII\Exceptions\FireflyException; +use FireflyIII\Helpers\Filter\FilterInterface; +use FireflyIII\Helpers\Filter\InternalTransferFilter; +use FireflyIII\Helpers\Filter\OpposingAccountFilter; +use FireflyIII\Helpers\Filter\TransferFilter; use FireflyIII\Models\AccountType; use FireflyIII\Models\Budget; use FireflyIII\Models\Category; use FireflyIII\Models\Tag; use FireflyIII\Models\Transaction; -use FireflyIII\Models\TransactionType; use FireflyIII\Repositories\Account\AccountRepositoryInterface; use FireflyIII\User; use Illuminate\Contracts\Encryption\DecryptException; @@ -74,6 +77,9 @@ class JournalCollector implements JournalCollectorInterface private $filterInternalTransfers; /** @var bool */ private $filterTransfers = false; + /** @var array */ + private $filters = [InternalTransferFilter::class]; + /** @var bool */ private $joinedBudget = false; /** @var bool */ @@ -95,6 +101,22 @@ class JournalCollector implements JournalCollectorInterface /** @var User */ private $user; + /** + * @param string $filter + * + * @return JournalCollectorInterface + */ + public function addFilter(string $filter): JournalCollectorInterface + { + $interfaces = class_implements($filter); + if (in_array(FilterInterface::class, $interfaces)) { + Log::debug(sprintf('Enabled filter %s', $filter)); + $this->filters[] = $filter; + } + + return $this; + } + /** * @return int * @throws FireflyException @@ -119,36 +141,6 @@ class JournalCollector implements JournalCollectorInterface return $this->count; } - /** - * @return JournalCollectorInterface - */ - public function disableFilter(): JournalCollectorInterface - { - $this->filterTransfers = false; - - return $this; - } - - /** - * @return JournalCollectorInterface - */ - public function disableInternalFilter(): JournalCollectorInterface - { - $this->filterInternalTransfers = false; - - return $this; - } - - /** - * @return JournalCollectorInterface - */ - public function enableInternalFilter(): JournalCollectorInterface - { - $this->filterInternalTransfers = true; - - return $this; - } - /** * @return Collection */ @@ -157,14 +149,9 @@ class JournalCollector implements JournalCollectorInterface $this->run = true; /** @var Collection $set */ $set = $this->query->get(array_values($this->fields)); - Log::debug(sprintf('Count of set is %d', $set->count())); - $set = $this->filterTransfers($set); - Log::debug(sprintf('Count of set after filterTransfers() is %d', $set->count())); - - // possibly filter "internal" transfers: - $set = $this->filterInternalTransfers($set); - Log::debug(sprintf('Count of set after filterInternalTransfers() is %d', $set->count())); + // run all filters: + $set = $this->filter($set); // loop for decryption. $set->each( @@ -204,6 +191,22 @@ class JournalCollector implements JournalCollectorInterface return $journals; } + /** + * @param string $filter + * + * @return JournalCollectorInterface + */ + public function removeFilter(string $filter): JournalCollectorInterface + { + $key = array_search($filter, $this->filters, true); + if (!($key === false)) { + Log::debug('Removed filter %s', $filter); + unset($this->filters[$key]); + } + + return $this; + } + /** * @param Collection $accounts * @@ -219,6 +222,7 @@ class JournalCollector implements JournalCollectorInterface } if ($accounts->count() > 1) { + $this->addFilter(TransferFilter::class); $this->filterTransfers = true; } @@ -242,6 +246,7 @@ class JournalCollector implements JournalCollectorInterface } if ($accounts->count() > 1) { + $this->addFilter(TransferFilter::class); $this->filterTransfers = true; } @@ -563,79 +568,21 @@ class JournalCollector implements JournalCollectorInterface * * @return Collection */ - private function filterInternalTransfers(Collection $set): Collection + private function filter(Collection $set): Collection { - if ($this->filterInternalTransfers === false) { - Log::debug('Did NO filtering for internal transfers on given set.'); - - return $set; - } - if ($this->joinedOpposing === false) { - Log::info('Cannot filter internal transfers because no opposing information is present.'); - - return $set; - } - - $accountIds = $this->accountIds; - $set = $set->filter( - function (Transaction $transaction) use ($accountIds) { - // both id's in $accountids? - if (in_array($transaction->account_id, $accountIds) && in_array($transaction->opposing_account_id, $accountIds)) { - Log::debug( - sprintf( - 'Transaction #%d has #%d and #%d in set, so removed', - $transaction->id, $transaction->account_id, $transaction->opposing_account_id - ), $accountIds - ); - - return false; - } - - return $transaction; - + // create all possible filters: + $filters = [ + InternalTransferFilter::class => new InternalTransferFilter($this->accountIds), + OpposingAccountFilter::class => new OpposingAccountFilter($this->accountIds), + TransferFilter::class => new TransferFilter, + ]; + Log::debug(sprintf('Will run %d filters on the set.', count($this->filters))); + foreach ($this->filters as $enabled) { + if (isset($filters[$enabled])) { + Log::debug(sprintf('Before filter %s: %d', $enabled, $set->count())); + $set = $filters[$enabled]->filter($set); + Log::debug(sprintf('After filter %s: %d', $enabled, $set->count())); } - ); - - return $set; - } - - /** - * If the set of accounts used by the collector includes more than one asset - * account, chances are the set include double entries: transfers get selected - * on both the source, and then again on the destination account. - * - * This method filters them out by removing transfers that have been selected twice. - * - * @param Collection $set - * - * @return Collection - */ - private function filterTransfers(Collection $set): Collection - { - if ($this->filterTransfers) { - $count = []; - $new = new Collection; - /** @var Transaction $transaction */ - foreach ($set as $transaction) { - if ($transaction->transaction_type_type !== TransactionType::TRANSFER) { - $new->push($transaction); - continue; - } - // make property string: - $journalId = $transaction->transaction_journal_id; - $amount = Steam::positive($transaction->transaction_amount); - $accountIds = [intval($transaction->account_id), intval($transaction->opposing_account_id)]; - sort($accountIds); - $key = $journalId . '-' . join(',', $accountIds) . '-' . $amount; - Log::debug(sprintf('Key is %s', $key)); - if (!isset($count[$key])) { - // not yet counted? add to new set and count it: - $new->push($transaction); - $count[$key] = 1; - } - } - - return $new; } return $set; diff --git a/app/Helpers/Collector/JournalCollectorInterface.php b/app/Helpers/Collector/JournalCollectorInterface.php index 954ae46cfa..3e2c2aa6db 100644 --- a/app/Helpers/Collector/JournalCollectorInterface.php +++ b/app/Helpers/Collector/JournalCollectorInterface.php @@ -28,26 +28,18 @@ use Illuminate\Support\Collection; */ interface JournalCollectorInterface { + /** + * @param string $filter + * + * @return JournalCollectorInterface + */ + public function addFilter(string $filter): JournalCollectorInterface; + /** * @return int */ public function count(): int; - /** - * @return JournalCollectorInterface - */ - public function disableFilter(): JournalCollectorInterface; - - /** - * @return JournalCollectorInterface - */ - public function disableInternalFilter(): JournalCollectorInterface; - - /** - * @return JournalCollectorInterface - */ - public function enableInternalFilter(): JournalCollectorInterface; - /** * @return Collection */ @@ -58,6 +50,13 @@ interface JournalCollectorInterface */ public function getPaginatedJournals(): LengthAwarePaginator; + /** + * @param string $filter + * + * @return JournalCollectorInterface + */ + public function removeFilter(string $filter): JournalCollectorInterface; + /** * @param Collection $accounts * diff --git a/app/Helpers/Filter/AmountFilter.php b/app/Helpers/Filter/AmountFilter.php index 236f114ea4..eb4ab55662 100644 --- a/app/Helpers/Filter/AmountFilter.php +++ b/app/Helpers/Filter/AmountFilter.php @@ -25,19 +25,25 @@ use Log; */ class AmountFilter implements FilterInterface { + /** @var int */ + private $modifier = 0; + + public function __construct(int $modifier) + { + $this->modifier = $modifier; + } /** * @param Collection $set - * @param null $parameters * * @return Collection */ - public function filter(Collection $set, $parameters = null): Collection + public function filter(Collection $set): Collection { return $set->filter( - function (Transaction $transaction) use ($parameters) { + function (Transaction $transaction) { // remove by amount - if (bccomp($transaction->transaction_amount, '0') === $parameters) { + if (bccomp($transaction->transaction_amount, '0') === $this->modifier) { Log::debug(sprintf('Filtered #%d because amount is %f.', $transaction->id, $transaction->transaction_amount)); return null; diff --git a/app/Helpers/Filter/EmptyFilter.php b/app/Helpers/Filter/EmptyFilter.php index a3c3dd20d3..23a4172e88 100644 --- a/app/Helpers/Filter/EmptyFilter.php +++ b/app/Helpers/Filter/EmptyFilter.php @@ -24,13 +24,11 @@ class EmptyFilter implements FilterInterface /** * @param Collection $set - * @param null $parameters * * @return Collection */ - public function filter(Collection $set, $parameters = null): Collection + public function filter(Collection $set): Collection { - // TODO: Implement filter() method. - throw new NotImplementedException; + return $set; } } \ No newline at end of file diff --git a/app/Helpers/Filter/FilterInterface.php b/app/Helpers/Filter/FilterInterface.php index 964d2ac7b2..d2dcb9eda4 100644 --- a/app/Helpers/Filter/FilterInterface.php +++ b/app/Helpers/Filter/FilterInterface.php @@ -18,10 +18,9 @@ interface FilterInterface { /** * @param Collection $set - * @param null $parameters * * @return Collection */ - public function filter(Collection $set, $parameters = null): Collection; + public function filter(Collection $set): Collection; } \ No newline at end of file diff --git a/app/Helpers/Filter/InternalTransferFilter.php b/app/Helpers/Filter/InternalTransferFilter.php index 87d6d750d0..7e8d71588e 100644 --- a/app/Helpers/Filter/InternalTransferFilter.php +++ b/app/Helpers/Filter/InternalTransferFilter.php @@ -26,27 +26,38 @@ use Log; */ class InternalTransferFilter implements FilterInterface { + /** @var array */ + private $accounts = []; + + /** + * InternalTransferFilter constructor. + * + * @param array $accounts + */ + public function __construct(array $accounts) + { + $this->accounts = $accounts; + } /** * @param Collection $set - * @param null $parameters * * @return Collection */ - public function filter(Collection $set, $parameters = null): Collection + public function filter(Collection $set): Collection { return $set->filter( - function (Transaction $transaction) use ($parameters) { + function (Transaction $transaction) { if (is_null($transaction->opposing_account_id)) { return $transaction; } // both id's in $parameters? - if (in_array($transaction->account_id, $parameters) && in_array($transaction->opposing_account_id, $parameters)) { + if (in_array($transaction->account_id, $this->accounts) && in_array($transaction->opposing_account_id, $this->accounts)) { Log::debug( sprintf( 'Transaction #%d has #%d and #%d in set, so removed', $transaction->id, $transaction->account_id, $transaction->opposing_account_id - ), $parameters + ), $this->accounts ); return false; diff --git a/app/Helpers/Filter/OpposingAccountFilter.php b/app/Helpers/Filter/OpposingAccountFilter.php index 0d21356c1d..69a1567943 100644 --- a/app/Helpers/Filter/OpposingAccountFilter.php +++ b/app/Helpers/Filter/OpposingAccountFilter.php @@ -26,21 +26,32 @@ use Log; */ class OpposingAccountFilter implements FilterInterface { + /** @var array */ + private $accounts = []; + + /** + * InternalTransferFilter constructor. + * + * @param array $accounts + */ + public function __construct(array $accounts) + { + $this->accounts = $accounts; + } /** * @param Collection $set - * @param null $parameters * * @return Collection */ - public function filter(Collection $set, $parameters = null): Collection + public function filter(Collection $set): Collection { return $set->filter( - function (Transaction $transaction) use ($parameters) { + function (Transaction $transaction) { $opposing = $transaction->opposing_account_id; // remove internal transfer - if (in_array($opposing, $parameters)) { - Log::debug(sprintf('Filtered #%d because its opposite is in accounts.', $transaction->id), $parameters); + if (in_array($opposing, $this->accounts)) { + Log::debug(sprintf('Filtered #%d because its opposite is in accounts.', $transaction->id), $this->accounts); return null; } diff --git a/app/Helpers/Filter/TransferFilter.php b/app/Helpers/Filter/TransferFilter.php index f17ae6b399..d33d5e41ed 100644 --- a/app/Helpers/Filter/TransferFilter.php +++ b/app/Helpers/Filter/TransferFilter.php @@ -27,11 +27,10 @@ class TransferFilter implements FilterInterface { /** * @param Collection $set - * @param null $parameters * * @return Collection */ - public function filter(Collection $set, $parameters = null): Collection + public function filter(Collection $set): Collection { $count = []; $new = new Collection;