Revamped a part of the rule test code.

- clean way of constructing triggers
- processor can be constructed in a number of ways.
- cleaner transaction matcher using collections instead of arrays
This commit is contained in:
James Cole 2016-02-17 21:03:59 +01:00
parent 1cf0125d1b
commit 6c22bad77a
5 changed files with 178 additions and 83 deletions

View File

@ -267,13 +267,23 @@ class RuleController extends Controller
*/
public function testTriggers(TestRuleFormRequest $request)
{
$triggers = [
// build trigger array from response TODO move to function:
$triggers = [];
$data = [
'rule-triggers' => $request->get('rule-trigger'),
'rule-trigger-values' => $request->get('rule-trigger-value'),
'rule-trigger-stop' => $request->get('rule-trigger-stop'),
];
foreach ($data['rule-triggers'] as $index => $triggerType) {
$triggers[] = [
'type' => $triggerType,
'value' => $data['rule-trigger-values'][$index],
'stopProcessing' => intval($data['rule-trigger-stop'][$index]) === 1 ? true : false,
];
}
if (count($triggers['rule-triggers']) == 0) {
if (count($triggers) == 0) {
return Response::json(['html' => '', 'warning' => trans('firefly.warning_no_valid_triggers')]);
}
@ -281,12 +291,15 @@ class RuleController extends Controller
$range = Config::get('firefly.test-triggers.range');
$matcher = new TransactionMatcher;
$matcher->setLimit($limit);
$matcher->setRange($range);
$matcher->setTriggers($triggers);
$matchingTransactions = $matcher->findMatchingTransactions();
// Dispatch the actual work to a matched object
$matchingTransactions
= (new TransactionMatcher($triggers))
->setTransactionLimit($range)
->findMatchingTransactions($limit);
// $matchingTransactions
// = (new TransactionMatcher($triggers))
// ->setTransactionLimit($range)
// ->findMatchingTransactions($limit);
// Warn the user if only a subset of transactions is returned
if (count($matchingTransactions) == $limit) {

View File

@ -16,8 +16,8 @@ use FireflyIII\Models\RuleTrigger;
use FireflyIII\Models\TransactionJournal;
use FireflyIII\Rules\Actions\ActionFactory;
use FireflyIII\Rules\Actions\ActionInterface;
use FireflyIII\Rules\Triggers\AbstractTrigger;
use FireflyIII\Rules\Triggers\TriggerFactory;
use FireflyIII\Rules\Triggers\TriggerInterface;
use Illuminate\Support\Collection;
use Log;
@ -43,7 +43,8 @@ class Processor
*/
private function __construct()
{
$this->triggers = new Collection;
$this->actions = new Collection;
}
/**
@ -53,16 +54,55 @@ class Processor
*/
public static function make(Rule $rule)
{
$self = new self;
$self->rule = $rule;
$self->triggers = $rule->ruleTriggers()->orderBy('order', 'ASC')->get();
$self->actions = $rule->ruleActions()->orderBy('order', 'ASC')->get();
$self = new self;
$self->rule = $rule;
$triggerSet = $rule->ruleTriggers()->orderBy('order', 'ASC')->get();
/** @var RuleTrigger $trigger */
foreach ($triggerSet as $trigger) {
$self->triggers->push(TriggerFactory::getTrigger($trigger));
}
$self->actions = $rule->ruleActions()->orderBy('order', 'ASC')->get();
return $self;
}
/**
* @param string $triggerName
* @param string $triggerValue
*
* @return Processor
*/
public static function makeFromString(string $triggerName, string $triggerValue)
{
$self = new self;
$trigger = TriggerFactory::makeTriggerFromStrings($triggerName, $triggerValue, false);
$self->triggers->push($trigger);
return $self;
}
/**
* @param array $triggers
*
* @return Processor
*/
public static function makeFromStringArray(array $triggers)
{
$self = new self;
foreach ($triggers as $entry) {
$trigger = TriggerFactory::makeTriggerFromStrings($entry['type'], $entry['value'], $entry['stopProcessing']);
$self->triggers->push($trigger);
}
return $self;
}
/**
* @param TransactionJournal $journal
*
* @return bool
*/
public function handleTransactionJournal(TransactionJournal $journal)
{
@ -70,10 +110,15 @@ class Processor
// get all triggers:
$triggered = $this->triggered();
if ($triggered) {
Log::debug('Rule #' . $this->rule->id . ' was triggered. Now process each action.');
$this->actions();
if ($this->actions->count() > 0) {
$this->actions();
}
return true;
}
return false;
}
/**
@ -112,13 +157,11 @@ class Processor
foreach ($this->triggers as $trigger) {
$foundTriggers++;
/** @var TriggerInterface $triggerObject */
$triggerObject = TriggerFactory::getTrigger($trigger);
// no need to keep pushing the journal around!
if ($triggerObject->triggered($this->journal)) {
/** @var AbstractTrigger $trigger */
if ($trigger->triggered($this->journal)) {
$hitTriggers++;
}
if ($trigger->stop_processing) {
if ($trigger->stopProcessing) {
break;
}

View File

@ -11,8 +11,10 @@ declare(strict_types = 1);
namespace FireflyIII\Rules;
use FireflyIII\Models\Rule;
use FireflyIII\Models\TransactionJournal;
use FireflyIII\Models\TransactionType;
use FireflyIII\Repositories\Journal\JournalRepositoryInterface;
use Illuminate\Support\Collection;
/**
* Class TransactionMatcher is used to find a list of
@ -22,11 +24,10 @@ use FireflyIII\Repositories\Journal\JournalRepositoryInterface;
*/
class TransactionMatcher
{
/** @var int Maximum number of transaction to search in (for performance reasons) * */
private $range = 200;
/** @var int */
private $limit = 10;
/** @var int Maximum number of transaction to search in (for performance reasons) * */
private $range = 200;
/** @var array */
private $transactionTypes = [TransactionType::DEPOSIT, TransactionType::WITHDRAWAL, TransactionType::TRANSFER];
/** @var array List of triggers to match */
@ -43,25 +44,18 @@ class TransactionMatcher
{
/** @var JournalRepositoryInterface $repository */
$repository = app('FireflyIII\Repositories\Journal\JournalRepositoryInterface');
// We don't know the number of transaction to fetch from the database, in
// order to return the proper number of matching transactions. Since we don't want
// to fetch all transactions (as the first transactions already match, or the last
// transactions are irrelevant), we will fetch data in pages.
// The optimal pagesize is somewhere between the maximum number of results to be returned
// and the maximum number of transactions to consider.
$pagesize = min($this->range / 2, $maxResults * 2);
$pagesize = min($this->range / 2, $this->limit * 2);
// Variables used within the loop
$numTransactionsProcessed = 0;
$page = 1;
$matchingTransactions = [];
$matchingTransactions = new Collection();
// Flags to indicate the end of the loop
$reachedEndOfList = false;
$foundEnoughTransactions = false;
$searchedEnoughTransactions = false;
$processor = Processor::makeFromStringArray($this->triggers);
// Start a loop to fetch batches of transactions. The loop will finish if:
// - all transactions have been fetched from the database
@ -69,51 +63,44 @@ class TransactionMatcher
// - the maximum number of transactions to search in have been searched
do {
// Fetch a batch of transactions from the database
$offset = $page > 0 ? ($page - 1) * $pagesize : 0;
$transactions = $repository->getJournalsOfTypes($this->transactionTypes, $offset, $page, $pagesize)->getCollection()->all();
$offset = $page > 0 ? ($page - 1) * $pagesize : 0;
$set = $repository->getCollectionOfTypes($this->transactionTypes, $offset, $pagesize);
// Filter transactions that match the rule
$matchingTransactions += array_filter(
$transactions, function ($transaction) {
$processor = new Processor(new Rule, $transaction);
return $processor->isTriggeredBy($this->triggers);
}
// Filter transactions that match the given triggers.
$filtered = $set->filter(
function (TransactionJournal $journal) use ($processor) {
return $processor->handleTransactionJournal($journal);
}
);
// merge:
$matchingTransactions = $matchingTransactions->merge($filtered);
// $matchingTransactions += array_filter(
// $set, function ($transaction) {
// $processor = new Processor(new Rule, $transaction);
//
// return $processor->isTriggeredBy($this->triggers);
// }
// );
// Update counters
$page++;
$numTransactionsProcessed += count($transactions);
$numTransactionsProcessed += count($set);
// Check for conditions to finish the loop
$reachedEndOfList = (count($transactions) < $pagesize);
$foundEnoughTransactions = (count($matchingTransactions) >= $maxResults);
$reachedEndOfList = (count($set) < $pagesize);
$foundEnoughTransactions = (count($matchingTransactions) >= $this->limit);
$searchedEnoughTransactions = ($numTransactionsProcessed >= $this->range);
} while (!$reachedEndOfList && !$foundEnoughTransactions && !$searchedEnoughTransactions);
// If the list of matchingTransactions is larger than the maximum number of results
// (e.g. if a large percentage of the transactions match), truncate the list
$matchingTransactions = array_slice($matchingTransactions, 0, $maxResults);
$matchingTransactions = $matchingTransactions->slice(0, $this->limit);
return $matchingTransactions;
}
/**
* @return int
*/
public function getRange()
{
return $this->range;
}
/**
* @param int $range
*/
public function setRange($range)
{
$this->range = $range;
}
/**
* @return int
*/
@ -130,6 +117,22 @@ class TransactionMatcher
$this->limit = $limit;
}
/**
* @return int
*/
public function getRange()
{
return $this->range;
}
/**
* @param int $range
*/
public function setRange($range)
{
$this->range = $range;
}
/**
* @return array
*/
@ -147,5 +150,4 @@ class TransactionMatcher
}
}

View File

@ -21,6 +21,8 @@ use FireflyIII\Models\TransactionJournal;
*/
class AbstractTrigger
{
/** @var bool */
public $stopProcessing;
/** @var string */
protected $checkValue;
/** @var TransactionJournal */
@ -38,6 +40,20 @@ class AbstractTrigger
}
/**
* @param string $triggerValue
* @param bool $stopProcessing
*
* @return static
*/
public static function makeFromStrings(string $triggerValue, bool $stopProcessing)
{
$self = new static;
$self->triggerValue = $triggerValue;
$self->stopProcessing = $stopProcessing;
return $self;
}
/**
* @param RuleTrigger $trigger
*
@ -45,9 +61,10 @@ class AbstractTrigger
*/
public static function makeFromTrigger(RuleTrigger $trigger)
{
$self = new self;
$self->trigger = $trigger;
$self->triggerValue = $trigger->trigger_value;
$self = new static;
$self->trigger = $trigger;
$self->triggerValue = $trigger->trigger_value;
$self->stopProcessing = $trigger->stop_processing;
return $self;
}
@ -58,10 +75,11 @@ class AbstractTrigger
*/
public static function makeFromTriggerAndJournal(RuleTrigger $trigger, TransactionJournal $journal)
{
$self = new self;
$self->trigger = $trigger;
$self->triggerValue = $trigger->trigger_value;
$self->journal = $journal;
$self = new static;
$self->trigger = $trigger;
$self->triggerValue = $trigger->trigger_value;
$self->stopProcessing = $trigger->stop_processing;
$self->journal = $journal;
}
/**

View File

@ -38,9 +38,40 @@ class TriggerFactory
$class = self::getTriggerClass($triggerType);
$obj = $class::makeFromTriggerValue($trigger->trigger_value);
// this is a massive HACK. TODO.
$obj->databaseObject = $trigger;
return $obj;
}
/**
* @param string $triggerType
* @param string $triggerValue
*
* @return AbstractTrigger
* @throws FireflyException
*/
public static function makeTriggerFromStrings(string $triggerType, string $triggerValue, bool $stopProcessing)
{
/** @var AbstractTrigger $class */
$class = self::getTriggerClass($triggerType);
$obj = $class::makeFromStrings($triggerValue, $stopProcessing);
return $obj;
}
/**
* Returns a map with triggertypes, mapped to the class representing that type
*/
protected static function getTriggerTypes()
{
if (!self::$triggerTypes) {
self::$triggerTypes = Domain::getRuleTriggers();
}
return self::$triggerTypes;
}
/**
* Returns the class name to be used for triggers with the given name
*
@ -49,7 +80,7 @@ class TriggerFactory
* @return TriggerInterface|string
* @throws FireflyException
*/
public static function getTriggerClass(string $triggerType): string
private static function getTriggerClass(string $triggerType): string
{
$triggerTypes = self::getTriggerTypes();
@ -64,16 +95,4 @@ class TriggerFactory
return $class;
}
/**
* Returns a map with triggertypes, mapped to the class representing that type
*/
protected static function getTriggerTypes()
{
if (!self::$triggerTypes) {
self::$triggerTypes = Domain::getRuleTriggers();
}
return self::$triggerTypes;
}
}