Refactor rule processor so it's testable.

app/Handlers/Events/StoredJournalEventHandler.php

@@ -46,7 +46,9 @@ class StoredJournalEventHandler
         $journal = $storedJournalEvent->journal;
 
         // create objects:
+        /** @var RuleGroupRepositoryInterface $ruleGroupRepos */
         $ruleGroupRepos = app(RuleGroupRepositoryInterface::class);
+        $ruleGroupRepos->setUser($journal->user);
         $groups         = $ruleGroupRepos->getActiveGroups($journal->user);
 
         /** @var RuleGroup $group */
@@ -54,7 +56,9 @@ class StoredJournalEventHandler
             $rules = $ruleGroupRepos->getActiveStoreRules($group);
             /** @var Rule $rule */
             foreach ($rules as $rule) {
-                $processor = Processor::make($rule);
+                /** @var Processor $processor */
+                $processor = app(Processor::class);
+                $processor->make($rule);
                 $processor->handleTransactionJournal($journal);
 
                 if ($rule->stop_processing) {

app/Handlers/Events/UpdatedJournalEventHandler.php

@@ -33,19 +33,6 @@ use FireflyIII\TransactionRules\Processor;
  */
 class UpdatedJournalEventHandler
 {
-    /** @var RuleGroupRepositoryInterface The rule group repository */
-    public $repository;
-
-    /**
-     * StoredJournalEventHandler constructor.
-     *
-     * @param RuleGroupRepositoryInterface $ruleGroupRepository
-     */
-    public function __construct(RuleGroupRepositoryInterface $ruleGroupRepository)
-    {
-        $this->repository = $ruleGroupRepository;
-    }
-
     /**
      * This method will check all the rules when a journal is updated.
      *
@@ -58,14 +45,21 @@ class UpdatedJournalEventHandler
     {
         // get all the user's rule groups, with the rules, order by 'order'.
         $journal = $updatedJournalEvent->journal;
-        $groups  = $this->repository->getActiveGroups($journal->user);
+
+        /** @var RuleGroupRepositoryInterface $ruleGroupRepos */
+        $ruleGroupRepos = app(RuleGroupRepositoryInterface::class);
+        $ruleGroupRepos->setUser($journal->user);
+
+        $groups  = $ruleGroupRepos->getActiveGroups($journal->user);
 
         /** @var RuleGroup $group */
         foreach ($groups as $group) {
-            $rules = $this->repository->getActiveUpdateRules($group);
+            $rules = $ruleGroupRepos->getActiveUpdateRules($group);
             /** @var Rule $rule */
             foreach ($rules as $rule) {
-                $processor = Processor::make($rule);
+                /** @var Processor $processor */
+                $processor = app(Processor::class);
+                $processor->make($rule);
                 $processor->handleTransactionJournal($journal);
 
                 if ($rule->stop_processing) {

app/Import/Storage/ImportArrayStorage.php

@@ -135,7 +135,9 @@ class ImportArrayStorage
                 $rules->each(
                     function (Rule $rule) use ($journal) {
                         Log::debug(sprintf('Going to apply rule #%d to journal %d.', $rule->id, $journal->id));
-                        $processor = Processor::make($rule);
+                        /** @var Processor $processor */
+                        $processor = app(Processor::class);
+                        $processor->make($rule);
                         $processor->handleTransactionJournal($journal);
                         if ($rule->stop_processing) {
                             return false;

app/Jobs/CreateRecurringTransactions.php

@@ -157,7 +157,9 @@ class CreateRecurringTransactions implements ShouldQueue
                 $this->rules[$userId]->each(
                     function (Rule $rule) use ($journal) {
                         Log::debug(sprintf('Going to apply rule #%d to journal %d.', $rule->id, $journal->id));
-                        $processor = Processor::make($rule);
+                        /** @var Processor $processor */
+                        $processor = app(Processor::class);
+                        $processor->make($rule);
                         /** @noinspection ExceptionsAnnotatingAndHandlingInspection */
                         $processor->handleTransactionJournal($journal);
                         if ($rule->stop_processing) {

app/Jobs/ExecuteRuleGroupOnExistingTransactions.php

@@ -200,7 +200,10 @@ class ExecuteRuleGroupOnExistingTransactions extends Job implements ShouldQueue
         // Create a list of processors for these rules
         return array_map(
             function ($rule) {
-                return Processor::make($rule);
+                /** @var Processor $processor */
+                $processor = app(Processor::class);
+                $processor->make($rule);
+                return $processor;
             },
             $rules->all()
         );

app/Jobs/ExecuteRuleOnExistingTransactions.php

@@ -162,7 +162,9 @@ class ExecuteRuleOnExistingTransactions extends Job implements ShouldQueue
     {
         // Lookup all journals that match the parameters specified
         $transactions = $this->collectJournals();
-        $processor    = Processor::make($this->rule, true);
+        /** @var Processor $processor */
+        $processor = app(Processor::class);
+        $processor->make($this->rule, true);
         $hits         = 0;
         $misses       = 0;
         $total        = 0;

app/TransactionRules/Processor.php

@@ -38,7 +38,7 @@ use Log;
 /**
  * Class Processor.
  */
-final class Processor
+class Processor
 {
     /** @var Collection Actions to exectute */
     public $actions;
@@ -56,92 +56,12 @@ final class Processor
     /**
      * Processor constructor.
      */
-    private function __construct()
+    public function __construct()
     {
         $this->triggers = new Collection;
         $this->actions  = new Collection;
     }
 
-    /**
-     * This method will make a Processor that will process each transaction journal using the triggers
-     * and actions found in the given Rule.
-     *
-     * @param Rule $rule
-     * @param bool $includeActions
-     *
-     * @return Processor
-     * @throws \FireflyIII\Exceptions\FireflyException
-     */
-    public static function make(Rule $rule, bool $includeActions = null): Processor
-    {
-        $includeActions = $includeActions ?? true;
-        Log::debug(sprintf('Making new rule from Rule %d', $rule->id));
-        Log::debug(sprintf('Rule is strict: %s', var_export($rule->strict, true)));
-        $self         = new self;
-        $self->rule   = $rule;
-        $self->strict = $rule->strict;
-        $triggerSet   = $rule->ruleTriggers()->orderBy('order', 'ASC')->get();
-        /** @var RuleTrigger $trigger */
-        foreach ($triggerSet as $trigger) {
-            Log::debug(sprintf('Push trigger %d', $trigger->id));
-            $self->triggers->push(TriggerFactory::getTrigger($trigger));
-        }
-        if (true === $includeActions) {
-            $self->actions = $rule->ruleActions()->orderBy('order', 'ASC')->get();
-        }
-
-        return $self;
-    }
-
-    /**
-     * This method will make a Processor that will process each transaction journal using the given
-     * trigger (singular!). It can only report if the transaction journal was hit by the given trigger
-     * and will not be able to act on it using actions.
-     *
-     * @param string $triggerName
-     * @param string $triggerValue
-     *
-     * @return Processor
-     *
-     * @throws \FireflyIII\Exceptions\FireflyException
-     */
-    public static function makeFromString(string $triggerName, string $triggerValue): Processor
-    {
-        Log::debug(sprintf('Processor::makeFromString("%s", "%s")', $triggerName, $triggerValue));
-        $self    = new self;
-        $trigger = TriggerFactory::makeTriggerFromStrings($triggerName, $triggerValue, false);
-        $self->triggers->push($trigger);
-
-        return $self;
-    }
-
-    /**
-     * This method will make a Processor that will process each transaction journal using the given
-     * triggers. It can only report if the transaction journal was hit by the given triggers
-     * and will not be able to act on it using actions.
-     *
-     * The given triggers must be in the following format:
-     *
-     * [type => xx, value => yy, stop_processing => bool], [type => xx, value => yy, stop_processing => bool],
-     *
-     * @param array $triggers
-     *
-     * @return Processor
-     *
-     * @throws \FireflyIII\Exceptions\FireflyException
-     */
-    public static function makeFromStringArray(array $triggers): Processor
-    {
-        $self = new self;
-        foreach ($triggers as $entry) {
-            $entry['value'] = $entry['value'] ?? '';
-            $trigger        = TriggerFactory::makeTriggerFromStrings($entry['type'], $entry['value'], $entry['stop_processing']);
-            $self->triggers->push($trigger);
-        }
-
-        return $self;
-    }
-
     /**
      * Return found triggers
      *
@@ -234,6 +154,73 @@ final class Processor
         return false;
     }
 
+    /**
+     * This method will make a Processor that will process each transaction journal using the triggers
+     * and actions found in the given Rule.
+     *
+     * @param Rule $rule
+     * @param bool $includeActions
+     *
+     * @throws \FireflyIII\Exceptions\FireflyException
+     */
+    public function make(Rule $rule, bool $includeActions = null): void
+    {
+        $includeActions = $includeActions ?? true;
+        Log::debug(sprintf('Making new rule from Rule %d', $rule->id));
+        Log::debug(sprintf('Rule is strict: %s', var_export($rule->strict, true)));
+        $this->rule   = $rule;
+        $this->strict = $rule->strict;
+        $triggerSet   = $rule->ruleTriggers()->orderBy('order', 'ASC')->get();
+        /** @var RuleTrigger $trigger */
+        foreach ($triggerSet as $trigger) {
+            Log::debug(sprintf('Push trigger %d', $trigger->id));
+            $this->triggers->push(TriggerFactory::getTrigger($trigger));
+        }
+        if (true === $includeActions) {
+            $this->actions = $rule->ruleActions()->orderBy('order', 'ASC')->get();
+        }
+    }
+
+    /**
+     * This method will make a Processor that will process each transaction journal using the given
+     * trigger (singular!). It can only report if the transaction journal was hit by the given trigger
+     * and will not be able to act on it using actions.
+     *
+     * @param string $triggerName
+     * @param string $triggerValue
+     *
+     * @throws \FireflyIII\Exceptions\FireflyException
+     */
+    public function makeFromString(string $triggerName, string $triggerValue): void
+    {
+        Log::debug(sprintf('Processor::makeFromString("%s", "%s")', $triggerName, $triggerValue));
+        $trigger = TriggerFactory::makeTriggerFromStrings($triggerName, $triggerValue, false);
+        $this->triggers->push($trigger);
+    }
+
+    /**
+     * This method will make a Processor that will process each transaction journal using the given
+     * triggers. It can only report if the transaction journal was hit by the given triggers
+     * and will not be able to act on it using actions.
+     *
+     * The given triggers must be in the following format:
+     *
+     * [type => xx, value => yy, stop_processing => bool], [type => xx, value => yy, stop_processing => bool],
+     *
+     * @param array $triggers
+     *
+     * @throws \FireflyIII\Exceptions\FireflyException
+     */
+    public function makeFromStringArray(array $triggers): void
+    {
+        foreach ($triggers as $entry) {
+            $entry['value'] = $entry['value'] ?? '';
+            $trigger        = TriggerFactory::makeTriggerFromStrings($entry['type'], $entry['value'], $entry['stop_processing']);
+            $this->triggers->push($trigger);
+        }
+
+    }
+
     /**
      * Run the actions
      *

app/TransactionRules/TransactionMatcher.php

@@ -93,8 +93,10 @@ class TransactionMatcher
         }
 
         // Variables used within the loop
-        $processor = Processor::makeFromStringArray($this->triggers);
+        /** @var Processor $processor */
-        $result    = $this->runProcessor($processor);
+        $processor = app(Processor::class);
+        $processor->makeFromStringArray($this->triggers);
+        $result = $this->runProcessor($processor);
 
         // 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

tests/Unit/Handlers/Events/StoredJournalEventHandlerTest.php

@@ -25,8 +25,10 @@ namespace Tests\Unit\Handlers\Events;
 
 
 use FireflyIII\Events\StoredTransactionJournal;
+use FireflyIII\Exceptions\FireflyException;
 use FireflyIII\Handlers\Events\StoredJournalEventHandler;
 use FireflyIII\Repositories\RuleGroup\RuleGroupRepositoryInterface;
+use FireflyIII\TransactionRules\Processor;
 use Log;
 use Tests\TestCase;
 
@@ -51,24 +53,28 @@ class StoredJournalEventHandlerTest extends TestCase
      */
     public function testProcessRules(): void
     {
-//        $ruleGroupRepos = $this->mock(RuleGroupRepositoryInterface::class);
+        $ruleGroupRepos = $this->mock(RuleGroupRepositoryInterface::class);
-//        $journal        = $this->user()->transactionJournals()->inRandomOrder()->first();
+        $processor      = $this->mock(Processor::class);
-//        $piggy          = $this->user()->piggyBanks()->inRandomOrder()->first();
+
-//        $event          = new StoredTransactionJournal($journal, $piggy->id);
+        $journal    = $this->user()->transactionJournals()->inRandomOrder()->first();
-//        $ruleGroups     = $this->user()->ruleGroups()->take(1)->get();
+        $piggy      = $this->user()->piggyBanks()->inRandomOrder()->first();
-//        $rules = $this->user()->rules()->take(1)->get();
+        $event      = new StoredTransactionJournal($journal, $piggy->id);
-//
+        $ruleGroups = $this->user()->ruleGroups()->take(1)->get();
-//        // mock calls:
+        $rules      = $this->user()->rules()->take(1)->get();
-//        $ruleGroupRepos->shouldReceive('setUser')->once();
+
-//        $ruleGroupRepos->shouldReceive('getActiveGroups')->andReturn($ruleGroups)->once();
+        // mock calls:
-//        $ruleGroupRepos->shouldReceive('getActiveStoreRules')->andReturn($rules)->once();
+        $ruleGroupRepos->shouldReceive('setUser')->once();
-//
+        $ruleGroupRepos->shouldReceive('getActiveGroups')->andReturn($ruleGroups)->once();
-//
+        $ruleGroupRepos->shouldReceive('getActiveStoreRules')->andReturn($rules)->once();
-//
+        $processor->shouldReceive('make')->once();
-//        $handler = new StoredJournalEventHandler;
+        $processor->shouldReceive('handleTransactionJournal')->once();
-//        $handler->processRules($event);
-        $this->assertTrue(true);
 
 
+        $handler = new StoredJournalEventHandler;
+        try {
+            $handler->processRules($event);
+        } catch (FireflyException $e) {
+            $this->assertTrue(false, $e->getMessage());
+        }
     }
 }

tests/Unit/Handlers/Events/UpdatedJournalEventHandlerTest.php

@@ -0,0 +1,78 @@
+<?php
+/**
+ * UpdatedJournalEventHandlerTest.php
+ * Copyright (c) 2018 thegrumpydictator@gmail.com
+ *
+ * This file is part of Firefly III.
+ *
+ * Firefly III is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Firefly III is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Firefly III. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+declare(strict_types=1);
+
+namespace Tests\Unit\Handlers\Events;
+
+use FireflyIII\Events\UpdatedTransactionJournal;
+use FireflyIII\Handlers\Events\UpdatedJournalEventHandler;
+use FireflyIII\Repositories\RuleGroup\RuleGroupRepositoryInterface;
+use FireflyIII\TransactionRules\Processor;
+use Log;
+use Tests\TestCase;
+
+/**
+ * Class UpdatedJournalEventHandlerTest
+ */
+class UpdatedJournalEventHandlerTest extends TestCase
+{
+    /**
+     *
+     */
+    public function setUp(): void
+    {
+        parent::setUp();
+        Log::debug(sprintf('Now in %s.', \get_class($this)));
+    }
+
+
+    /**
+     * @covers \FireflyIII\Handlers\Events\UpdatedJournalEventHandler
+     * @covers \FireflyIII\Events\StoredTransactionJournal
+     */
+    public function testProcessRules(): void
+    {
+        $ruleGroupRepos = $this->mock(RuleGroupRepositoryInterface::class);
+        $processor      = $this->mock(Processor::class);
+
+        $journal    = $this->user()->transactionJournals()->inRandomOrder()->first();
+        $event      = new UpdatedTransactionJournal($journal);
+        $ruleGroups = $this->user()->ruleGroups()->take(1)->get();
+        $rules      = $this->user()->rules()->take(1)->get();
+
+        // mock calls:
+        $ruleGroupRepos->shouldReceive('setUser')->once();
+        $ruleGroupRepos->shouldReceive('getActiveGroups')->andReturn($ruleGroups)->once();
+        $ruleGroupRepos->shouldReceive('getActiveUpdateRules')->andReturn($rules)->once();
+        $processor->shouldReceive('make')->once();
+        $processor->shouldReceive('handleTransactionJournal')->once();
+
+
+        $handler = new UpdatedJournalEventHandler;
+        try {
+            $handler->processRules($event);
+        } catch (FireflyException $e) {
+            $this->assertTrue(false, $e->getMessage());
+        }
+    }
+
+}