jenkins-bot has submitted this change and it was merged.

Change subject: configureHightingForSource expects an options field
......................................................................


configureHightingForSource expects an options field

When running without the experimental highlighter we end up raising an
undefined index warning because this options key only gets set in the
branch that deals with the experimental highlighter. This key is not
supported by the fvh, so don't provide it.

Add's some basic unit tests to ensure the config didn't change, beyond
stripping the options key, due to this adjustment. Also add's an
exception when trying to use highlight regex with fvh. This was already
unsupported, it just errored out in less obvious ways.

Change-Id: I26d6419341bec5d11a318bc50c07e18e57c7af1e
---
M includes/Search/ResultsType.php
A tests/unit/Search/ResultsTypeTest.php
2 files changed, 284 insertions(+), 20 deletions(-)

Approvals:
  Cindy-the-browser-test-bot: Looks good to me, but someone else must approve
  DCausse: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/includes/Search/ResultsType.php b/includes/Search/ResultsType.php
index 05dd22c..7f79961 100644
--- a/includes/Search/ResultsType.php
+++ b/includes/Search/ResultsType.php
@@ -377,33 +377,65 @@
                        'post_tags' => array( Searcher::HIGHLIGHT_POST ),
                        'fields' => array(),
                );
+
                if ( count( $highlightSource ) ) {
+                       if ( !$wgCirrusSearchUseExperimentalHighlighter ) {
+                               throw new \RuntimeException( 'regex is only 
supported with $wgCirrusSearchUseExperimentalHighlighter = true' );
+                       }
                        $config[ 'fields' ][ 'source_text.plain' ] = $text;
                        $this->configureHighlightingForSource( $config, 
$highlightSource );
                        return $config;
                }
+               $experimental = array();
                if ( $this->highlightingConfig & self::HIGHLIGHT_TITLE ) {
                        $config[ 'fields' ][ 'title' ] = $entireValue;
                }
                if ( $this->highlightingConfig & self::HIGHLIGHT_ALT_TITLE ) {
                        $config[ 'fields' ][ 'redirect.title' ] = 
$redirectAndHeading;
-                       $config[ 'fields' ][ 'redirect.title' ][ 'options' ][ 
'skip_if_last_matched' ] = true;
+                       $experimental[ 'fields' ][ 'redirect.title' ][ 
'options' ][ 'skip_if_last_matched' ] = true;
                        $config[ 'fields' ][ 'category' ] = $redirectAndHeading;
-                       $config[ 'fields' ][ 'category' ][ 'options' ][ 
'skip_if_last_matched' ] = true;
+                       $experimental[ 'fields' ][ 'category' ][ 'options' ][ 
'skip_if_last_matched' ] = true;
                        $config[ 'fields' ][ 'heading' ] = $redirectAndHeading;
-                       $config[ 'fields' ][ 'heading' ][ 'options' ][ 
'skip_if_last_matched' ] = true;
+                       $experimental[ 'fields' ][ 'heading' ][ 'options' ][ 
'skip_if_last_matched' ] = true;
                }
                if ( $this->highlightingConfig & self::HIGHLIGHT_SNIPPET ) {
                        $config[ 'fields' ][ 'text' ] = $text;
                        $config[ 'fields' ][ 'auxiliary_text' ] = 
$remainingText;
-                       $config[ 'fields' ][ 'auxiliary_text' ][ 'options' ][ 
'skip_if_last_matched' ] = true;
+                       $experimental[ 'fields' ][ 'auxiliary_text' ][ 
'options' ][ 'skip_if_last_matched' ] = true;
                        if ( $this->highlightingConfig & 
self::HIGHLIGHT_FILE_TEXT ) {
                                $config[ 'fields' ][ 'file_text' ] = 
$remainingText;
-                               $config[ 'fields' ][ 'file_text' ][ 'options' 
][ 'skip_if_last_matched' ] = true;
+                               $experimental[ 'fields' ][ 'file_text' ][ 
'options' ][ 'skip_if_last_matched' ] = true;
                        }
                }
                $config[ 'fields' ] = $this->addMatchedFields( $config[ 
'fields' ] );
+
+               if ( $wgCirrusSearchUseExperimentalHighlighter ) {
+                       $config = $this->arrayMergeRecursive( $config, 
$experimental );
+               }
+
                return $config;
+       }
+
+
+       /**
+        * Behaves like array_merge with recursive descent. Unlike 
array_merge_recursive,
+        * but just like array_merge, this does not convert non-arrays into 
arrays.
+        *
+        * @param array $source
+        * @param array $overrides
+        * @return array
+        */
+       private function arrayMergeRecursive( array $source, array $overrides ) 
{
+               foreach ( $source as $k => $v ) {
+                       if ( isset( $overrides[$k] ) ) {
+                               if ( is_array( $overrides[$k] ) ) {
+                                       $source[$k] = 
$this->arrayMergeRecursive( $v, $overrides[$k] );
+                               } else {
+                                       $source[$k] = $overrides[$k];
+                               }
+                       }
+               }
+               return $source;
        }
 
        /**
@@ -433,7 +465,7 @@
         * @param array &$config
         * @param array $highlightSource
         */
-       private function configureHighlightingForSource( &$config, 
$highlightSource ) {
+       private function configureHighlightingForSource( array &$config, array 
$highlightSource ) {
                global $wgCirrusSearchRegexMaxDeterminizedStates;
                $patterns = array();
                $locale = null;
@@ -454,22 +486,28 @@
                                'regex_case_insensitive' => 
(boolean)$caseInsensitive,
                                'max_determinized_states' => 
$wgCirrusSearchRegexMaxDeterminizedStates,
                        );
-                       $config[ 'fields' ][ 'source_text.plain' ][ 'options' ] 
= array_merge(
-                               $config[ 'fields' ][ 'source_text.plain' ][ 
'options' ], $options );
-                       return;
-               }
-               $queryStrings = array();
-               foreach ( $highlightSource as $part ) {
-                       if ( isset( $part[ 'query' ] ) ) {
-                               $queryStrings[] = $part[ 'query' ];
+                       if ( isset( 
$config['fields']['source_text.plain']['options'] ) ) {
+                               $config[ 'fields' ][ 'source_text.plain' ][ 
'options' ] = array_merge(
+                                       $config[ 'fields' ][ 
'source_text.plain' ][ 'options' ],
+                                       $options
+                               );
+                       } else {
+                               $config[ 'fields' ][ 'source_text.plain' ][ 
'options' ] = $options;
                        }
-               }
-               if ( count( $queryStrings ) ) {
-                       $bool = new \Elastica\Query\BoolQuery();
-                       foreach ( $queryStrings as $queryString ) {
-                               $bool->addShould( $queryString );
+               } else {
+                       $queryStrings = array();
+                       foreach ( $highlightSource as $part ) {
+                               if ( isset( $part[ 'query' ] ) ) {
+                                       $queryStrings[] = $part[ 'query' ];
+                               }
                        }
-                       $config[ 'fields' ][ 'source_text.plain' ][ 
'highlight_query' ] = $bool->toArray();
+                       if ( count( $queryStrings ) ) {
+                               $bool = new \Elastica\Query\BoolQuery();
+                               foreach ( $queryStrings as $queryString ) {
+                                       $bool->addShould( $queryString );
+                               }
+                               $config[ 'fields' ][ 'source_text.plain' ][ 
'highlight_query' ] = $bool->toArray();
+                       }
                }
        }
 }
diff --git a/tests/unit/Search/ResultsTypeTest.php 
b/tests/unit/Search/ResultsTypeTest.php
new file mode 100644
index 0000000..d0ab69d
--- /dev/null
+++ b/tests/unit/Search/ResultsTypeTest.php
@@ -0,0 +1,226 @@
+<?php
+
+namespace CirrusSearch\Search;
+
+use MediaWikiTestCase;
+
+/**
+ * Test escaping search strings.
+ *
+ * This program 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 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+class ResultsTypeTest extends MediaWikiTestCase {
+       /**
+        * @dataProvider fullTextHighlightingConfigurationTestCases
+        */
+       public function testFullTextHighlightingConfiguration(
+               $highlightingConfig,
+               $useExperimentalHighlighter,
+               array $highlightSource,
+               array $expected
+       ) {
+               $this->setMwGlobals( 
'wgCirrusSearchUseExperimentalHighlighter', $useExperimentalHighlighter );
+               $type = new FullTextResultsType( $highlightingConfig, '' );
+               $this->assertEquals( $expected, 
$type->getHighlightingConfiguration( $highlightSource ) );
+       }
+
+       public static function fullTextHighlightingConfigurationTestCases() {
+               $boostBefore = array(
+                       20 => 2,
+                       50 => 1.8,
+                       200 => 1.5,
+                       1000 => 1.2,
+               );
+
+               return array(
+                       'default configuration' => array(
+                               FullTextResultsType::HIGHLIGHT_ALL,
+                               false,
+                               array(),
+                               array(
+                                       'pre_tags' => array( '<span 
class="searchmatch">' ),
+                                       'post_tags' => array( '</span>' ),
+                                       'fields' => array(
+                                               'title' => array(
+                                                       'number_of_fragments' 
=> 0,
+                                                       'type' => 'fvh',
+                                                       'order' => 'score',
+                                                       'matched_fields' => 
array( 'title', 'title.plain' ),
+                                               ),
+                                               'redirect.title' => array(
+                                                       'number_of_fragments' 
=> 1,
+                                                       'fragment_size' => 
10000,
+                                                       'type' => 'fvh',
+                                                       'order' => 'score',
+                                                       'matched_fields' => 
array( 'redirect.title', 'redirect.title.plain' ),
+                                               ),
+                                               'category' => array(
+                                                       'number_of_fragments' 
=> 1,
+                                                       'fragment_size' => 
10000,
+                                                       'type' => 'fvh',
+                                                       'order' => 'score',
+                                                       'matched_fields' => 
array( 'category', 'category.plain' ),
+                                               ),
+                                               'heading' => array(
+                                                       'number_of_fragments' 
=> 1,
+                                                       'fragment_size' => 
10000,
+                                                       'type' => 'fvh',
+                                                       'order' => 'score',
+                                                       'matched_fields' => 
array( 'heading', 'heading.plain' ),
+                                               ),
+                                               'text' => array(
+                                                       'number_of_fragments' 
=> 1,
+                                                       'fragment_size' => 150,
+                                                       'type' => 'fvh',
+                                                       'order' => 'score',
+                                                       'no_match_size' => 150,
+                                                       'matched_fields' => 
array( 'text', 'text.plain' ),
+                                               ),
+                                               'auxiliary_text' => array(
+                                                       'number_of_fragments' 
=> 1,
+                                                       'fragment_size' => 150,
+                                                       'type' => 'fvh',
+                                                       'order' => 'score',
+                                                       'matched_fields' => 
array( 'auxiliary_text', 'auxiliary_text.plain' ),
+                                               ),
+                                               'file_text' => array(
+                                                       'number_of_fragments' 
=> 1,
+                                                       'fragment_size' => 150,
+                                                       'type' => 'fvh',
+                                                       'order' => 'score',
+                                                       'matched_fields' => 
array( 'file_text', 'file_text.plain' ),
+                                               ),
+                                       ),
+                               )
+                       ),
+                       'default configuration with experimental highlighter' 
=> array(
+                               FullTextResultsType::HIGHLIGHT_ALL,
+                               true,
+                               array(),
+                               array(
+                                       'pre_tags' => array( '<span 
class="searchmatch">' ),
+                                       'post_tags' => array( '</span>' ),
+                                       'fields' => array(
+                                               'title' => array(
+                                                       'number_of_fragments' 
=> 1,
+                                                       'type' => 
'experimental',
+                                                       'matched_fields' => 
array( 'title', 'title.plain' ),
+                                                       'fragmenter' => 'none',
+                                               ),
+                                               'redirect.title' => array(
+                                                       'number_of_fragments' 
=> 1,
+                                                       'type' => 
'experimental',
+                                                       'order' => 'score',
+                                                       'options' => array( 
'skip_if_last_matched' => true ),
+                                                       'matched_fields' => 
array( 'redirect.title', 'redirect.title.plain' ),
+                                                       'fragmenter' => 'none',
+                                               ),
+                                               'category' => array(
+                                                       'number_of_fragments' 
=> 1,
+                                                       'type' => 
'experimental',
+                                                       'order' => 'score',
+                                                       'options' => array( 
'skip_if_last_matched' => true ),
+                                                       'matched_fields' => 
array( 'category', 'category.plain' ),
+                                                       'fragmenter' => 'none',
+                                               ),
+                                               'heading' => array(
+                                                       'number_of_fragments' 
=> 1,
+                                                       'type' => 
'experimental',
+                                                       'order' => 'score',
+                                                       'options' => array( 
'skip_if_last_matched' => true ),
+                                                       'matched_fields' => 
array( 'heading', 'heading.plain' ),
+                                                       'fragmenter' => 'none',
+                                               ),
+                                               'text' => array(
+                                                       'number_of_fragments' 
=> 1,
+                                                       'fragment_size' => 150,
+                                                       'type' => 
'experimental',
+                                                       'options' => array(
+                                                               'top_scoring' 
=> true,
+                                                               'boost_before' 
=> $boostBefore,
+                                                               
'max_fragments_scored' => 5000,
+                                                       ),
+                                                       'no_match_size' => 150,
+                                                       'matched_fields' => 
array( 'text', 'text.plain' ),
+                                                       'fragmenter' => 'scan',
+                                               ),
+                                               'auxiliary_text' => array(
+                                                       'number_of_fragments' 
=> 1,
+                                                       'fragment_size' => 150,
+                                                       'type' => 
'experimental',
+                                                       'options' => array(
+                                                               'top_scoring' 
=> true,
+                                                               'boost_before' 
=> $boostBefore,
+                                                               
'max_fragments_scored' => 5000,
+                                                               
'skip_if_last_matched' => true,
+                                                       ),
+                                                       'matched_fields' => 
array( 'auxiliary_text', 'auxiliary_text.plain' ),
+                                                       'fragmenter' => 'scan',
+                                               ),
+                                               'file_text' => array(
+                                                       'number_of_fragments' 
=> 1,
+                                                       'fragment_size' => 150,
+                                                       'type' => 
'experimental',
+                                                       'options' => array(
+                                                               'top_scoring' 
=> true,
+                                                               'boost_before' 
=> $boostBefore,
+                                                               
'max_fragments_scored' => 5000,
+                                                               
'skip_if_last_matched' => true,
+                                                       ),
+                                                       'matched_fields' => 
array( 'file_text', 'file_text.plain' ),
+                                                       'fragmenter' => 'scan',
+                                               ),
+                                       ),
+                               ),
+                       ),
+                       'source configuration with experimental-highlighter' => 
array(
+                               FullTextResultsType::HIGHLIGHT_ALL,
+                               true,
+                               array(
+                                       array(
+                                               'pattern' => '(some|thing)',
+                                               'locale' => 'testlocale',
+                                               'insensitive' => false,
+                                       ),
+                               ),
+                               array(
+                                       'pre_tags' => array( '<span 
class="searchmatch">' ),
+                                       'post_tags' => array( '</span>' ),
+                                       'fields' => array(
+                                               'source_text.plain' => array(
+                                                       'type' => 
'experimental',
+                                                       'number_of_fragments' 
=> 1,
+                                                       'fragment_size' => 150,
+                                                       'options' => array(
+                                                               'regex' => 
array( '(some|thing)' ),
+                                                               'locale' => 
'testlocale',
+                                                               'regex_flavor' 
=> 'lucene',
+                                                               'skip_query' => 
true,
+                                                               
'regex_case_insensitive' => false,
+                                                               
'max_determinized_states' => 20000,
+                                                               'top_scoring' 
=> true,
+                                                               'boost_before' 
=> $boostBefore,
+                                                               
'max_fragments_scored' => 5000,
+                                                       ),
+                                                       'no_match_size' => 150,
+                                                       'fragmenter' => 'scan',
+                                               ),
+                                       ),
+                               ),
+                       ),
+               );
+       }
+}

-- 
To view, visit https://gerrit.wikimedia.org/r/286722
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I26d6419341bec5d11a318bc50c07e18e57c7af1e
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/CirrusSearch
Gerrit-Branch: master
Gerrit-Owner: EBernhardson <[email protected]>
Gerrit-Reviewer: Cindy-the-browser-test-bot <[email protected]>
Gerrit-Reviewer: DCausse <[email protected]>
Gerrit-Reviewer: Gehel <[email protected]>
Gerrit-Reviewer: Manybubbles <[email protected]>
Gerrit-Reviewer: Smalyshev <[email protected]>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to