MDL-53508 search: Improve highlighting and move to text fields
authorEric Merrill <merrill@oakland.edu>
Wed, 23 Mar 2016 12:55:48 +0000 (08:55 -0400)
committerEric Merrill <merrill@oakland.edu>
Wed, 23 Mar 2016 12:55:48 +0000 (08:55 -0400)
lang/en/search.php
search/classes/document.php
search/engine/solr/classes/document.php
search/engine/solr/classes/engine.php
search/engine/solr/classes/schema.php
search/engine/solr/tests/engine_test.php
search/templates/result.mustache

index 49d741a..84bab68 100644 (file)
@@ -67,6 +67,7 @@ $string['ittook'] = 'It took';
 $string['next'] = 'Next';
 $string['noindexmessage'] = 'Admin: There appears to be no search index. Please';
 $string['noresults'] = 'No results';
+$string['notitle'] = 'No title';
 $string['normalsearch'] = 'Normal search';
 $string['openedon'] = 'opened on';
 $string['optimize'] = 'Optimize';
index 9d47ab9..89f0d83 100644 (file)
@@ -91,12 +91,12 @@ class document implements \renderable, \templatable {
             'indexed' => true
         ),
         'title' => array(
-            'type' => 'string',
+            'type' => 'text',
             'stored' => true,
             'indexed' => true
         ),
         'content' => array(
-            'type' => 'string',
+            'type' => 'text',
             'stored' => true,
             'indexed' => true
         ),
@@ -148,12 +148,12 @@ class document implements \renderable, \templatable {
             'indexed' => false
         ),
         'description1' => array(
-            'type' => 'string',
+            'type' => 'text',
             'stored' => true,
             'indexed' => true
         ),
         'description2' => array(
-            'type' => 'string',
+            'type' => 'text',
             'stored' => true,
             'indexed' => true
         ),
@@ -305,6 +305,19 @@ class document implements \renderable, \templatable {
         return $string;
     }
 
+    /**
+     * Formats a text value for the search engine.
+     *
+     * Search engines may overwrite this method to apply restrictions, like limiting the size.
+     * The default behaviour is just returning the string.
+     *
+     * @param string $text
+     * @return string
+     */
+    public static function format_text_for_engine($text) {
+        return $text;
+    }
+
     /**
      * Returns a timestamp from the value stored in the search engine.
      *
@@ -411,9 +424,13 @@ class document implements \renderable, \templatable {
                 // Overwrite the timestamp with the engine dependant format.
                 $data[$fieldname] = static::format_time_for_engine($data[$fieldname]);
             } else if ($field['type'] === 'string') {
-                // Overwrite the timestamp with the engine dependant format.
+                // Overwrite the string with the engine dependant format.
                 $data[$fieldname] = static::format_string_for_engine($data[$fieldname]);
+            } else if ($field['type'] === 'text') {
+                // Overwrite the text with the engine dependant format.
+                $data[$fieldname] = static::format_text_for_engine($data[$fieldname]);
             }
+
         }
 
         foreach (static::$optionalfields as $fieldname => $field) {
@@ -424,8 +441,11 @@ class document implements \renderable, \templatable {
                 // Overwrite the timestamp with the engine dependant format.
                 $data[$fieldname] = static::format_time_for_engine($data[$fieldname]);
             } else if ($field['type'] === 'string') {
-                // Overwrite the timestamp with the engine dependant format.
+                // Overwrite the string with the engine dependant format.
                 $data[$fieldname] = static::format_string_for_engine($data[$fieldname]);
+            } else if ($field['type'] === 'text') {
+                // Overwrite the text with the engine dependant format.
+                $data[$fieldname] = static::format_text_for_engine($data[$fieldname]);
             }
         }
 
@@ -444,14 +464,14 @@ class document implements \renderable, \templatable {
      * @return array
      */
     public function export_for_template(\renderer_base $output) {
-
         list($componentname, $areaname) = \core_search\manager::extract_areaid_parts($this->get('areaid'));
 
+        $title = $this->is_set('title') ? $this->format_text($this->get('title')) : '';
         $data = [
             'courseurl' => new \moodle_url('/course/view.php?id=' . $this->get('courseid')),
             'coursefullname' => format_string($this->get('coursefullname'), true, array('context' => $this->get('contextid'))),
             'modified' => userdate($this->get('modified')),
-            'title' => format_string($this->get('title'), true, array('context' => $this->get('contextid'))),
+            'title' => ($title !== '') ? $title : get_string('notitle', 'search'),
             'docurl' => $this->get_doc_url(),
             'content' => $this->is_set('content') ? $this->format_text($this->get('content')) : null,
             'contexturl' => $this->get_context_url(),
index 20c1f3e..43137ff 100644 (file)
@@ -72,6 +72,41 @@ class document extends \core_search\document {
      * @return int
      */
     protected function get_text_format() {
-        return FORMAT_MARKDOWN;
+        return FORMAT_HTML;
+    }
+
+    /**
+     * Formats a text string coming from the search engine.
+     *
+     * @param  string $text Text to format
+     * @return string HTML text to be renderer
+     */
+    protected function format_text($text) {
+        // Since we allow output for highlighting, we need to encode html entities.
+        // This ensures plaintext html chars don't become valid html.
+        $out = s($text);
+
+        $startcount = 0;
+        $endcount = 0;
+
+        // Remove end/start pairs that span a few common seperation characters. Allows us to highlight phrases instead of words.
+        $regex = '|'.engine::HIGHLIGHT_END.'([ .,-]{0,3})'.engine::HIGHLIGHT_START.'|';
+        $out = preg_replace($regex, '$1', $out);
+
+        // Now replace our start and end highlight markers.
+        $out = str_replace(engine::HIGHLIGHT_START, '<span class="highlight">', $out, $startcount);
+        $out = str_replace(engine::HIGHLIGHT_END, '</span>', $out, $endcount);
+
+        // This makes sure any highlight tags are balanced, incase truncation or the highlight text contained our markers.
+        while ($startcount > $endcount) {
+            $out .= '</span>';
+            $endcount++;
+        }
+        while ($startcount < $endcount) {
+            $out = '<span class="highlight">' . $out;
+            $endcount++;
+        }
+
+        return parent::format_text($out);
     }
 }
index f67c86a..3a07f6b 100644 (file)
@@ -46,9 +46,19 @@ class engine extends \core_search\engine {
     const AUTOCOMMIT_WITHIN = 15000;
 
     /**
-     * @var int Highlighting fragsize.
+     * Highlighting fragsize. Slightly larger than output size (500) to allow for ... appending.
      */
-    const FRAG_SIZE = 500;
+    const FRAG_SIZE = 510;
+
+    /**
+     * Marker for the start of a highlight.
+     */
+    const HIGHLIGHT_START = '@@HI_S@@';
+
+    /**
+     * Marker for the end of a highlight.
+     */
+    const HIGHLIGHT_END = '@@HI_E@@';
 
     /**
      * @var \SolrClient
@@ -58,7 +68,7 @@ class engine extends \core_search\engine {
     /**
      * @var array Fields that can be highlighted.
      */
-    protected $highlightfields = array('content', 'description1', 'description2');
+    protected $highlightfields = array('title', 'content', 'description1', 'description2');
 
     /**
      * Prepares a Solr query, applies filters and executes it returning its results.
@@ -161,8 +171,9 @@ class engine extends \core_search\engine {
             $query->addHighlightField($field);
         }
         $query->setHighlightFragsize(static::FRAG_SIZE);
-        $query->setHighlightSimplePre('__');
-        $query->setHighlightSimplePost('__');
+        $query->setHighlightSimplePre(self::HIGHLIGHT_START);
+        $query->setHighlightSimplePost(self::HIGHLIGHT_END);
+        $query->setHighlightMergeContiguous(true);
 
         $query->setQuery($q);
 
index 5adff6d..b023970 100644 (file)
@@ -177,7 +177,7 @@ class schema {
             $params = array(
                 'add-field' => array(
                     'name' => $fieldname,
-                    'type' => $data['type'],
+                    'type' => ($data['type'] === 'text' ? 'text_general' : $data['type']),
                     'stored' => $data['stored'],
                     'multiValued' => false,
                     'indexed' => $data['indexed']
@@ -242,7 +242,8 @@ class schema {
                         throw new \moodle_exception('errorcreatingschema', 'search_solr', '',
                             get_string('schemafieldautocreated', 'search_solr', $fieldname));
 
-                    } else if ($results->field->type !== $data['type'] ||
+                    } else if (($results->field->type !== $data['type'] &&
+                                ($data['type'] !== 'text' || $results->field->type !== 'text_general')) ||
                                 $results->field->multiValued !== false ||
                                 $results->field->indexed !== $data['indexed'] ||
                                 $results->field->stored !== $data['stored']) {
index e99b829..5e91f82 100644 (file)
@@ -252,4 +252,27 @@ class search_solr_engine_testcase extends advanced_testcase {
         $this->assertEquals(0, $results[0]->get('owneruserid'));
         $this->assertEquals($originalid, $results[0]->get('id'));
     }
+
+    public function test_highlight() {
+        global $PAGE;
+
+        $this->search->index();
+
+        $querydata = new stdClass();
+        $querydata->q = 'message';
+
+        $results = $this->search->search($querydata);
+        $this->assertCount(2, $results);
+
+        $result = reset($results);
+
+        $regex = '|'.\search_solr\engine::HIGHLIGHT_START.'message'.\search_solr\engine::HIGHLIGHT_END.'|';
+        $this->assertRegExp($regex, $result->get('content'));
+
+        $searchrenderer = $PAGE->get_renderer('core_search');
+        $exported = $result->export_for_template($searchrenderer);
+
+        $regex = '|<span class="highlight">message</span>|';
+        $this->assertRegExp($regex, $exported['content']);
+    }
 }
index 6d39ae8..8840cfd 100644 (file)
@@ -53,7 +53,7 @@
 }}
 <div class="result">
     <h4 class="result-title">
-        <a href="{{{docurl}}}">{{title}}</a>
+        <a href="{{{docurl}}}">{{{title}}}</a>
     </h4>
     {{#content}}
         <div class="result-content">{{{content}}}</div>