Better catch for long queries, #3903

app/Http/Controllers/Rule/CreateController.php

@@ -32,15 +32,12 @@ use FireflyIII\Models\TransactionJournal;
 use FireflyIII\Repositories\Rule\RuleRepositoryInterface;
 use FireflyIII\Support\Http\Controllers\ModelInformation;
 use FireflyIII\Support\Http\Controllers\RuleManagement;
-use FireflyIII\Support\Search\OperatorQuerySearch;
 use FireflyIII\Support\Search\SearchInterface;
 use Illuminate\Contracts\View\Factory;
 use Illuminate\Http\RedirectResponse;
 use Illuminate\Http\Request;
 use Illuminate\Routing\Redirector;
 use Illuminate\View\View;
-use Log;
-use Throwable;
 
 /**
  * Class CreateController

app/Http/Controllers/Rule/IndexController.php

@@ -25,11 +25,9 @@ namespace FireflyIII\Http\Controllers\Rule;
 use FireflyIII\Http\Controllers\Controller;
 use FireflyIII\Models\Rule;
 use FireflyIII\Models\RuleGroup;
-use FireflyIII\Models\RuleTrigger;
 use FireflyIII\Repositories\Rule\RuleRepositoryInterface;
 use FireflyIII\Repositories\RuleGroup\RuleGroupRepositoryInterface;
 use FireflyIII\Support\Http\Controllers\RuleManagement;
-use FireflyIII\Support\Search\OperatorQuerySearch;
 use FireflyIII\User;
 use Illuminate\Contracts\View\Factory;
 use Illuminate\Http\JsonResponse;
@@ -91,7 +89,7 @@ class IndexController extends Controller
      */
     public function search(Rule $rule): RedirectResponse
     {
-        $route  = route('search.index');
+        $route = route('search.index');
         $query = $this->ruleRepos->getSearchQuery($rule);
         $route = sprintf('%s?%s', $route, http_build_query(['search' => $query, 'rule' => $rule->id]));
 

app/Http/Controllers/SearchController.php

@@ -64,11 +64,12 @@ class SearchController extends Controller
     public function index(Request $request, SearchInterface $searcher)
     {
         // search params:
-        $fullQuery   = (string) $request->get('search');
+        $fullQuery        = (string) $request->get('search');
-        $page        = 0 === (int) $request->get('page') ? 1 : (int) $request->get('page');
+        $page             = 0 === (int) $request->get('page') ? 1 : (int) $request->get('page');
-        $ruleId      = (int) $request->get('rule');
+        $ruleId           = (int) $request->get('rule');
-        $rule        = null;
+        $rule             = null;
-        $ruleChanged = false;
+        $ruleChanged      = false;
+        $longQueryWarning = false;
 
         // find rule, check if query is different, offer to update.
         $ruleRepository = app(RuleRepositoryInterface::class);
@@ -79,7 +80,9 @@ class SearchController extends Controller
                 $ruleChanged = true;
             }
         }
-
+        if (strlen($fullQuery) > 250) {
+            $longQueryWarning = true;
+        }
         // parse search terms:
         $searcher->parseQuery($fullQuery);
 
@@ -89,7 +92,7 @@ class SearchController extends Controller
 
         $subTitle = (string) trans('breadcrumbs.search_result', ['query' => $fullQuery]);
 
-        return view('search.index', compact('query', 'operators', 'page', 'rule', 'fullQuery', 'subTitle', 'ruleId', 'ruleChanged'));
+        return view('search.index', compact('query', 'longQueryWarning', 'operators', 'page', 'rule', 'fullQuery', 'subTitle', 'ruleId', 'ruleChanged'));
     }
 
     /**
@@ -106,6 +109,7 @@ class SearchController extends Controller
         $page      = 0 === (int) $request->get('page') ? 1 : (int) $request->get('page');
 
         $searcher->parseQuery($fullQuery);
+
         $searcher->setPage($page);
         $groups     = $searcher->searchTransactions();
         $hasPages   = $groups->hasPages();

app/Support/Search/OperatorQuerySearch.php

@@ -77,6 +77,7 @@ class OperatorQuerySearch implements SearchInterface
     private float                              $startTime;
     private Collection                         $modifiers; // obsolete
     private Collection                         $operators;
+    private string                             $originalQuery;
 
     /**
      * OperatorQuerySearch constructor.
@@ -90,6 +91,7 @@ class OperatorQuerySearch implements SearchInterface
         $this->page               = 1;
         $this->words              = [];
         $this->limit              = 25;
+        $this->originalQuery      = '';
         $this->validOperators     = array_keys(config('firefly.search.operators'));
         $this->startTime          = microtime(true);
         $this->accountRepository  = app(AccountRepositoryInterface::class);
@@ -143,6 +145,7 @@ class OperatorQuerySearch implements SearchInterface
     public function setPage(int $page): void
     {
         $this->page = $page;
+        $this->collector->setPage($this->page);
     }
 
     /**
@@ -161,22 +164,16 @@ class OperatorQuerySearch implements SearchInterface
     public function parseQuery(string $query)
     {
         Log::debug(sprintf('Now in parseQuery(%s)', $query));
-        $parser      = new QueryParser();
+        $parser              = new QueryParser();
-        $this->query = $parser->parse($query);
+        $this->query         = $parser->parse($query);
-
+        $this->originalQuery = $query;
-        $this->collector = app(GroupCollectorInterface::class);
-        $this->collector->setUser($this->user);
-        $this->collector->setLimit($this->limit)->setPage($this->page);
-        $this->collector->withAccountInformation()->withCategoryInformation()->withBudgetInformation();
 
         Log::debug(sprintf('Found %d node(s)', count($this->query->getNodes())));
-
         foreach ($this->query->getNodes() as $searchNode) {
             $this->handleSearchNode($searchNode);
         }
 
         $this->collector->setSearchWords($this->words);
-
     }
 
     /**
@@ -211,8 +208,12 @@ class OperatorQuerySearch implements SearchInterface
         $this->billRepository->setUser($user);
         $this->categoryRepository->setUser($user);
         $this->budgetRepository->setUser($user);
+        $this->collector = app(GroupCollectorInterface::class);
+        $this->collector->setUser($this->user);
+        $this->collector->withAccountInformation()->withCategoryInformation()->withBudgetInformation();
 
         $this->setLimit((int) app('preferences')->getForUser($user, 'listPageSize', 50)->data);
+
     }
 
     /**
@@ -268,7 +269,7 @@ class OperatorQuerySearch implements SearchInterface
      */
     private function updateCollector(string $operator, string $value): bool
     {
-        Log::debug(sprintf('updateCollector(%s, %s)', $operator, $value));
+        Log::debug(sprintf('updateCollector("%s", "%s")', $operator, $value));
 
         // check if alias, replace if necessary:
         $operator = self::getRootOperator($operator);
@@ -579,7 +580,7 @@ class OperatorQuerySearch implements SearchInterface
      */
     private function searchAccount(string $value, int $searchDirection, int $stringPosition): void
     {
-        Log::debug(sprintf('searchAccount(%s, %d, %d)', $value, $stringPosition, $searchDirection));
+        Log::debug(sprintf('searchAccount("%s", %d, %d)', $value, $stringPosition, $searchDirection));
 
         // search direction (default): for source accounts
         $searchTypes     = [AccountType::ASSET, AccountType::MORTGAGE, AccountType::LOAN, AccountType::DEBT, AccountType::REVENUE];
@@ -767,6 +768,7 @@ class OperatorQuerySearch implements SearchInterface
     public function setLimit(int $limit): void
     {
         $this->limit = $limit;
+        $this->collector->setLimit($this->limit);
     }
 
     /**

app/TransactionRules/Engine/SearchRuleEngine.php

@@ -234,11 +234,11 @@ class SearchRuleEngine implements RuleEngineInterface
         foreach ($rule->ruleTriggers as $ruleTrigger) {
             // if needs no context, value is different:
             $needsContext = config(sprintf('firefly.search.operators.%s.needs_context', $ruleTrigger->trigger_type)) ?? true;
-            if(false === $needsContext) {
+            if (false === $needsContext) {
                 Log::debug(sprintf('SearchRuleEngine:: add a rule trigger: %s:true', $ruleTrigger->trigger_type));
                 $searchArray[$ruleTrigger->trigger_type] = 'true';
             }
-            if(true === $needsContext) {
+            if (true === $needsContext) {
                 Log::debug(sprintf('SearchRuleEngine:: add a rule trigger: %s:"%s"', $ruleTrigger->trigger_type, $ruleTrigger->trigger_value));
                 $searchArray[$ruleTrigger->trigger_type] = sprintf('"%s"', $ruleTrigger->trigger_value);
             }
@@ -249,20 +249,17 @@ class SearchRuleEngine implements RuleEngineInterface
             Log::debug(sprintf('SearchRuleEngine:: add local added operator: %s:"%s"', $operator['type'], $operator['value']));
             $searchArray[$operator['type']] = sprintf('"%s"', $operator['value']);
         }
-        $toJoin = [];
-        foreach ($searchArray as $type => $value) {
-            $toJoin[] = sprintf('%s:%s', $type, $value);
-        }
-
-        $searchQuery = join(' ', $toJoin);
-        Log::debug(sprintf('SearchRuleEngine:: Search strict query for rule #%d ("%s") = %s', $rule->id, $rule->title, $searchQuery));
 
         // build and run the search engine.
         $searchEngine = app(SearchInterface::class);
         $searchEngine->setUser($this->user);
         $searchEngine->setPage(1);
         $searchEngine->setLimit(31337);
-        $searchEngine->parseQuery($searchQuery);
+
+        foreach ($searchArray as $type => $value) {
+            $searchEngine->parseQuery(sprintf('%s:%s', $type, $value));
+        }
+
 
         $result     = $searchEngine->searchTransactions();
         $collection = $result->getCollection();
@@ -286,13 +283,13 @@ class SearchRuleEngine implements RuleEngineInterface
                 Log::debug('Skip trigger type.');
                 continue;
             }
-            $searchArray = [];
+            $searchArray  = [];
             $needsContext = config(sprintf('firefly.search.operators.%s.needs_context', $ruleTrigger->trigger_type)) ?? true;
-            if(false === $needsContext) {
+            if (false === $needsContext) {
                 Log::debug(sprintf('SearchRuleEngine:: non strict, will search for: %s:true', $ruleTrigger->trigger_type));
                 $searchArray[$ruleTrigger->trigger_type] = 'true';
             }
-            if(true === $needsContext) {
+            if (true === $needsContext) {
                 Log::debug(sprintf('SearchRuleEngine:: non strict, will search for: %s:"%s"', $ruleTrigger->trigger_type, $ruleTrigger->trigger_value));
                 $searchArray[$ruleTrigger->trigger_type] = sprintf('"%s"', $ruleTrigger->trigger_value);
             }
@@ -302,21 +299,16 @@ class SearchRuleEngine implements RuleEngineInterface
                 Log::debug(sprintf('SearchRuleEngine:: add local added operator: %s:"%s"', $operator['type'], $operator['value']));
                 $searchArray[$operator['type']] = sprintf('"%s"', $operator['value']);
             }
-            $toJoin = [];
-            foreach ($searchArray as $type => $value) {
-                $toJoin[] = sprintf('%s:%s', $type, $value);
-            }
 
-            $searchQuery = join(' ', $toJoin);
-            Log::debug(sprintf('SearchRuleEngine:: Search strict query for non-strict rule #%d ("%s") = %s', $rule->id, $rule->title, $searchQuery));
-
-            // build and run a search:
             // build and run the search engine.
             $searchEngine = app(SearchInterface::class);
             $searchEngine->setUser($this->user);
             $searchEngine->setPage(1);
             $searchEngine->setLimit(31337);
-            $searchEngine->parseQuery($searchQuery);
+
+            foreach ($searchArray as $type => $value) {
+                $searchEngine->parseQuery(sprintf('%s:%s', $type, $value));
+            }
 
             $result     = $searchEngine->searchTransactions();
             $collection = $result->getCollection();

config/firefly.php

@@ -459,6 +459,7 @@ return [
             'description_ends'     => ['alias' => false, 'needs_context' => true,],
             'description_contains' => ['alias' => false, 'needs_context' => true,],
             'description_is'       => ['alias' => false, 'needs_context' => true,],
+            'description'          => ['alias' => true, 'alias_for' => 'description_contains', 'needs_context' => true,],
 
             'currency_is'         => ['alias' => false, 'needs_context' => true,],
             'foreign_currency_is' => ['alias' => false, 'needs_context' => true,],

resources/lang/en_US/firefly.php

@@ -260,6 +260,7 @@ return [
 
     // search
     'search'                                              => 'Search',
+    'long_query_warning'                                  => 'Your search query is very long, and may not work as expected.',
     'search_query'                                        => 'Query',
     'search_found_transactions'                           => 'Firefly III found :count transaction in :time seconds.|Firefly III found :count transactions in :time seconds.',
     'search_found_more_transactions'                      => 'Firefly III found more than :count transactions in :time seconds.',

resources/views/v1/search/index.twig

@@ -20,7 +20,7 @@
                         <div class="form-group">
                             <label for="query" class="col-sm-1 control-label">{{ 'search_query'|_ }}</label>
                             <div class="col-sm-10">
-                                <input autocomplete="off" type="text" name="search" id="query" value="{{ fullQuery }}" class="form-control"
+                                <input autocomplete="off" maxlength="255" type="text" name="search" id="query" value="{{ fullQuery }}" class="form-control"
                                        placeholder="{{ fullQuery }}">
                             </div>
                         </div>
@@ -37,6 +37,11 @@
                             <input type="hidden" name="rule" value="{{ ruleId }}" />
                         {% endif %}
                     </form>
+                    {% if longQueryWarning %}
+                    <p class="text-danger">
+                        {{ 'long_query_warning'|_ }}
+                    </p>
+                    {% endif %}
                     {% if '' != query %}
                     <p>
                         {{ trans('firefly.search_for_query', {query: query|escape})|raw}}