adding unit test for elastic pagination

data/SugarBean.php

@@ -5236,7 +5236,7 @@ public function create_index($query)
      * @param string $id
      */
     public function mark_deleted($id)
-    {
+    {        
         global $current_user;
         $date_modified = $GLOBALS['timedate']->nowDb();
         $id = $this->db->quote($id);

include/Exceptions/SuiteException.php

@@ -42,4 +42,8 @@
     die('Not A Valid Entry Point');
 }
 
-class SuiteException extends Exception {}
+class SuiteException extends Exception {
+
+    const NO_ID = 1;
+
+}

include/StateChecker.php

@@ -51,6 +51,8 @@
     die('Not A Valid Entry Point');
 }
 
+include_once __DIR__ . '/StateCheckerDirectoryIterator.php';
+
 /**
  * StateChecker
  *
@@ -325,7 +327,7 @@ protected function getFiles($path = '.')
             throw new StateCheckerException('Real path can not resolved for: ' . $path);
         }
 
-        $objects = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($realpath), RecursiveIteratorIterator::SELF_FIRST);
+        $objects = new RecursiveIteratorIterator(new StateCheckerDirectoryIterator($realpath), RecursiveIteratorIterator::SELF_FIRST);
         $files = [];
         foreach ($objects as $name => $object) {
             if (!$object->isDir() && !$this->isExcludedFile($name)) {

include/StateCheckerDirectoryIterator.php

@@ -0,0 +1,71 @@
+<?php
+/**
+ *
+ * SugarCRM Community Edition is a customer relationship management program developed by
+ * SugarCRM, Inc. Copyright (C) 2004-2013 SugarCRM Inc.
+ *
+ * SuiteCRM is an extension to SugarCRM Community Edition developed by SalesAgility Ltd.
+ * Copyright (C) 2011 - 2018 SalesAgility Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU Affero General Public License version 3 as published by the
+ * Free Software Foundation with the addition of the following permission added
+ * to Section 15 as permitted in Section 7(a): FOR ANY PART OF THE COVERED WORK
+ * IN WHICH THE COPYRIGHT IS OWNED BY SUGARCRM, SUGARCRM DISCLAIMS THE WARRANTY
+ * OF NON INFRINGEMENT OF THIRD PARTY RIGHTS.
+ *
+ * This program 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 Affero General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU Affero General Public License along with
+ * this program; if not, see http://www.gnu.org/licenses or write to the Free
+ * Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301 USA.
+ *
+ * You can contact SugarCRM, Inc. headquarters at 10050 North Wolfe Road,
+ * SW2-130, Cupertino, CA 95014, USA. or at email address contact@sugarcrm.com.
+ *
+ * The interactive user interfaces in modified source and object code versions
+ * of this program must display Appropriate Legal Notices, as required under
+ * Section 5 of the GNU Affero General Public License version 3.
+ *
+ * In accordance with Section 7(b) of the GNU Affero General Public License version 3,
+ * these Appropriate Legal Notices must retain the display of the "Powered by
+ * SugarCRM" logo and "Supercharged by SuiteCRM" logo. If the display of the logos is not
+ * reasonably feasible for technical reasons, the Appropriate Legal Notices must
+ * display the words "Powered by SugarCRM" and "Supercharged by SuiteCRM".
+ */
+
+namespace SuiteCRM;
+
+use InvalidArgumentException;
+use RecursiveDirectoryIterator;
+use RecursiveFilterIterator;
+
+/**
+ * StateCheckerDirectoryIterator
+ *
+ * Readable directory iterator
+ *
+ * @author gyula
+ */
+class StateCheckerDirectoryIterator extends RecursiveFilterIterator
+{
+    public function __construct($path)
+    {
+        if (!$path instanceof RecursiveDirectoryIterator) {
+            if (! is_readable($path) || ! is_dir($path)) {
+                throw new InvalidArgumentException("$path is not a valid directory or not readable");
+            }
+            $path = new RecursiveDirectoryIterator($path, RecursiveDirectoryIterator::SKIP_DOTS);
+        }
+        parent::__construct($path);
+    }
+
+    public function accept()
+    {
+        return $this->current()->isReadable() && $this->current()->isDir();
+    }
+}

lib/Robo/Plugin/Commands/ElasticSearchCommands.php

@@ -118,12 +118,7 @@ public function elasticIndex($differential = 1, $searchdefs = 0)
     {
         $this->bootstrap();
 
-        $indexer = new ElasticSearchIndexer();
-        $indexer->setDifferentialIndexing($differential);
-        if ($searchdefs) {
-            $indexer->setDocumentifier(new SearchDefsDocumentifier());
-        }
-        $indexer->index();
+        ElasticSearchIndexer::repairElasticsearchIndex($differential, $searchdefs);
     }
 
     /**

lib/Search/ElasticSearch/ElasticSearchHooks.php

@@ -44,7 +44,9 @@
 }
 
 use InvalidArgumentException;
+use LoggerManager;
 use SugarBean;
+use SuiteCRM\Search\Exceptions\SearchException;
 use SuiteCRM\Utility\SuiteLogger;
 use Throwable;
 
@@ -99,6 +101,8 @@ private function reIndexSafe(SugarBean $bean)
     {
         try {
             $this->reIndex($bean);
+        } catch (SearchException $exception) {
+            $this->handleError($exception);
         } catch (\Exception $exception) {
             $this->handleError($exception);
         } catch (\Throwable $throwable) {
@@ -114,20 +118,25 @@ private function reIndexSafe(SugarBean $bean)
     private function reIndex(SugarBean $bean)
     {
         if (ElasticSearchIndexer::isEnabled() === false) {
-            return;
+            throw new SearchException(
+                'Elasticsearch trying to re-indexing a bean but indexer is disabled in configuration.',
+                SearchException::ES_DISABLED
+            );
         }
 
         $this->bean = $bean;
 
         $this->getIndexer();
 
-        if ($this->isBlacklisted()) {
-            return;
+        if (!$this->isBlacklisted()) {
+            $this->correctAction();
+            $this->performAction($bean);
+        } else {
+            LoggerManager::getLogger()->warn(
+                'Elasticsearch trying to re-indexing a bean but this module is blacklisted: ' .
+                $bean->module_name
+            );
         }
-
-        $this->correctAction();
-
-        $this->performAction($bean);
     }
 
     /**
@@ -192,4 +201,4 @@ private function handleError($exception)
         $logger->error($message);
         $logger->error($exception);
     }
-}
+}

lib/Search/ElasticSearch/ElasticSearchIndexer.php

@@ -566,4 +566,18 @@ private function getModuleLastIndexed($module)
 
         return $meta['last_index'];
     }
+
+    /**
+     * 
+     * @param bool $differential
+     * @param int $searchdefs
+     */
+    public static function repairElasticsearchIndex($differential = true, $searchdefs = 0) {
+        $indexer = new ElasticSearchIndexer();
+        $indexer->setDifferentialIndexing($differential);
+        if ($searchdefs) {
+            $indexer->setDocumentifier(new SearchDefsDocumentifier());
+        }
+        $indexer->index();
+    }
 }

lib/Search/Exceptions/SearchException.php

@@ -50,5 +50,7 @@ class SearchException extends \RuntimeException
 {
 
     const ZERO_SIZE = 100;
+    const ES_DISABLED = 101;
+    const ES_MODULE_BLACKLISTED = 102;
 
 }

lib/Search/SearchResults.php

@@ -118,6 +118,15 @@ public function getHitsAsBeans()
         foreach ($hits as $module => $beans) {
             foreach ((array)$beans as $bean) {
                 $obj = BeanFactory::getBean($module, $bean);
+
+                // if a search found a bean but suitecrm does not, it could happens 
+                // maybe the bean is deleted but elsasticsearch is not re-indexing yet.
+                // so at this point we trying to rebuild the index and try again to get bean:
+                if (!$obj) {                    
+                    ElasticSearch\ElasticSearchIndexer::repairElasticsearchIndex();
+                    $obj = BeanFactory::getBean($module, $bean);
+                }
+
                 if (!$obj) {
                     throw new Exception('Error retrieveing bean: ' . $module . ' [' . $bean . ']');
                 }

modules/Administration/repairDatabase.php

@@ -38,6 +38,8 @@
  * display the words "Powered by SugarCRM" and "Supercharged by SuiteCRM".
  */
 
+use SuiteCRM\Search\ElasticSearch\ElasticSearchIndexer;
+
 if (!defined('sugarEntry') || !sugarEntry) {
     die('Not A Valid Entry Point');
 }
@@ -95,6 +97,7 @@
             }
 
             echo "<h3>{$mod_strings['LBL_REPAIR_DATABASE_SYNCED']}</h3>";
+            ElasticSearchIndexer::repairElasticsearchIndex();
         }
     } else {
         if (!$export && empty($_REQUEST['repair_silent'])) {
@@ -173,6 +176,7 @@
                 echo $ss->fetch('modules/Administration/templates/RepairDatabase.tpl');
             } else {
                 echo "<h3>{$mod_strings['LBL_REPAIR_DATABASE_SYNCED']}</h3>";
+                ElasticSearchIndexer::repairElasticsearchIndex();
             }
         }
     }

tests/unit/phpunit/lib/Search/UI/SearchResultsControllerTest.php

@@ -54,41 +54,124 @@
  */
 class SearchResultsControllerTest extends StateCheckerPHPUnitTestCaseAbstract {
 
+    public function testDisplayFoundOnePage() {
+                $state = new SuiteCRM\StateSaver();
+        $state->pushTable('accounts');
+        $state->pushTable('accounts_cstm');
+        $state->pushGlobals();
+
+        $ids = [];
+        for ($i=0; $i<15; $i++) {
+            $account = BeanFactory::getBean('Accounts');
+            $account->name = 'test account ' . $i;
+            $ok = $account->save();
+            $this->assertTrue((bool)$ok);
+            $ids[] = $account->id;
+        }
+        $this->assertEquals(15, count($ids));
+
+        $request = [
+            'search-query-string' => 'test account',
+            'query_string' => 'test account',
+            'search-query-size' => 10,
+            'search-query-from' => 0,
+            'search-engine' => 0,
+        ];
+        $query = SearchQuery::fromRequestArray($request);
+        $hits = [
+            'Accounts' => $ids,
+        ];
+        $groupedByModule = true;
+        $searchTime = 0.05;
+        $total = 15;
+        $scores = null;
+        $options = null;
+        $results = new SearchResults($hits, $groupedByModule, $searchTime, $total, $scores, $options);
+        $searchResultsController = new SearchResultsController($query, $results);
+        ob_start();        
+        $searchResultsController->display();
+        $content = ob_get_contents();
+        ob_end_clean();
+        $this->assertContains('Total result(s): 15', $content);
+        $this->assertContains('Page 1 of 2', $content);
+
+        // add 5 more..
+        for ($i=15; $i<20; $i++) {
+            $account = BeanFactory::getBean('Accounts');
+            $account->name = 'test account ' . $i;
+            $ok = $account->save();
+            $this->assertTrue((bool)$ok);
+            $ids[] = $account->id;
+        }
+        $this->assertEquals(20, count($ids));
+
+        $request = [
+            'search-query-string' => 'test account',
+            'query_string' => 'test account',
+            'search-query-size' => 10,
+            'search-query-from' => 10,
+            'search-engine' => 0,
+        ];
+        $query = SearchQuery::fromRequestArray($request);
+        $hits = [
+            'Accounts' => $ids,
+        ];
+        $groupedByModule = true;
+        $searchTime = 0.05;
+        $total = 20;
+        $scores = null;
+        $options = null;
+        $results = new SearchResults($hits, $groupedByModule, $searchTime, $total, $scores, $options);
+        $searchResultsController = new SearchResultsController($query, $results);
+        ob_start();        
+        $searchResultsController->display();
+        $content = ob_get_contents();
+        ob_end_clean();
+        $this->assertContains('Total result(s): 20', $content);
+        $this->assertContains('Page 2 of 2', $content);
+
+        $state->popGlobals();
+        $state->popTable('accounts_cstm');
+        $state->popTable('accounts');
+    }
+
     public function testDisplayFoundOne() {
 
         $state = new SuiteCRM\StateSaver();
         $state->pushTable('accounts');
         $state->pushTable('accounts_cstm');
-
+        $state->pushGlobals();
+
         $account = BeanFactory::getBean('Accounts');
         $account->name = 'test account 1';
         $ok = $account->save();
         $this->assertTrue((bool)$ok);
-        $searchHooks = new \SuiteCRM\Search\ElasticSearch\ElasticSearchHooks();
-        $searchHooks->beanSaved($account, null, null);
-
+
         $request = [
             'search-query-string' => 'test account',
-            //'query_string' => '',
+            'query_string' => 'test account',
             'search-query-size' => 10,
             'search-query-from' => 0,
             'search-engine' => 0,
         ];
         $query = SearchQuery::fromRequestArray($request);
-        $hits = [];
+        $hits = [
+            'Accounts' => [$account->id],
+        ];
         $groupedByModule = true;
-        $searchTime = null;
-        $total = null;
+        $searchTime = 0.05;
+        $total = 1;
         $scores = null;
         $options = null;
         $results = new SearchResults($hits, $groupedByModule, $searchTime, $total, $scores, $options);
         $searchResultsController = new SearchResultsController($query, $results);
         ob_start();        
         $searchResultsController->display();
-        $contents = ob_get_contents();
+        $content = ob_get_contents();
         ob_end_clean();
-        $this->assertContains('test account 1', $contents);
+        $this->assertContains('test account 1', $content);
 
+        $state->popGlobals();
         $state->popTable('accounts_cstm');
         $state->popTable('accounts');
     }