EBernhardson has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/326057 )
Change subject: Add warnings for various query parsing failures
......................................................................
Add warnings for various query parsing failures
Bug: T149142
Change-Id: Ia574888bc5ee2669701ead8b2f0aec1d9d5c49ea
---
M i18n/en.json
M i18n/qqq.json
M includes/Query/FileNumericFeature.php
M includes/Query/GeoFeature.php
M includes/Query/InCategoryFeature.php
M includes/Query/LanguageFeature.php
M includes/Query/MoreLikeFeature.php
M includes/Query/RegexInSourceFeature.php
M includes/Search/SearchContext.php
M includes/Searcher.php
M tests/unit/Query/BaseSimpleKeywordFeatureTest.php
M tests/unit/Query/FileFeatureTest.php
M tests/unit/Query/GeoFeatureTest.php
M tests/unit/Query/InCategoryFeatureTest.php
A tests/unit/Query/LanguageFeatureTest.php
M tests/unit/Query/MoreLikeFeatureTest.php
A tests/unit/Query/RegexInSourceFeatureTest.php
17 files changed, 378 insertions(+), 46 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/CirrusSearch
refs/changes/57/326057/1
diff --git a/i18n/en.json b/i18n/en.json
index 389ba76..e83d58e 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -55,5 +55,16 @@
"cirrussearch-completion-profile-classic-pref-name": "Classic prefix
search",
"cirrussearch-completion-profile-classic-pref-desc": "No typo
correction. Matches the beginning of titles.",
"cirrussearch-timed-out": "The search timed out, only partial results
are available.",
- "cirrussearch-regex-timed-out": "The regex search timed out, only
partial results are available. Try simplifying your regular expression to get
complete results."
+ "cirrussearch-regex-timed-out": "The regex search timed out, only
partial results are available. Try simplifying your regular expression to get
complete results.",
+ "cirrussearch-file-numeric-feature-not-a-number": "The search term '$1'
requires numeric values, but '$2' was provided.",
+ "cirrusesarch-file-numeric-feature-multi-argument-w-sign": "The search
term '$1' doesn't allow providing multiple arguments and a sign with '$2'.",
+ "cirrussearch-feature-not-available": "The search keyword '$1' is not
enabled on this wiki.",
+ "cirrussearch-geo-feature-invalid-coordinates": "The geo coordinates
'$2' provided to '$1' could not be parsed.",
+ "cirrussearch-geo-feature-invalid-distance": "The geo distance '$2'
provided to '$1' could not be parsed.",
+ "cirrussearch-geo-feature-unknown-title": "The title '$2' provided to
'$1' does not exist.",
+ "cirrussearch-geo-feature-title-no-coordinates": "The title '$2'
provided to '$1' does not have any coordinates",
+ "cirrussearch-feature-too-many-conditions": "Too many conditions
provided to '$1', truncating to $2.",
+ "cirrussearch-incategory-feature-no-valid-categories": "No valid
categories were provided to '$1'.",
+ "cirrussearch-mlt-feature-no-valid-titles": "No valid titles provided
to '$1'.",
+ "cirrussearch-mlt-not-configured": "The '$1' feature is misconfigured.
Ask admin to fix $wgCirrusSearchMoreLikeThisFields"
}
diff --git a/i18n/qqq.json b/i18n/qqq.json
index 3df6adc..59aa699 100644
--- a/i18n/qqq.json
+++ b/i18n/qqq.json
@@ -65,4 +65,15 @@
"cirrussearch-completion-profile-classic-pref-desc": "Description of
the completion profile classic.",
"cirrussearch-timed-out": "Message displayed to user when the search
query gave up due to timeout before completing.",
"cirrussearch-regex-timed-out": "Message displayed to user when the
regex search query gave up due to timeout before completing."
+ "cirrussearch-file-numeric-feature-not-a-number": "Warning shown to
users when a search keyword for numeric file features used something other than
a number.",
+ "cirrusesarch-file-numeric-feature-multi-argument-w-sign": "Warning
shown to users when a search keyword for numeric file features used multiple
arguments with an invalid qualifier.",
+ "cirrussearch-feature-not-available": "Warning shown to users when an
unavailable search keyword is used.",
+ "cirrussearch-geo-feature-invalid-coordinates": "Warning shown to users
when providing invalid coordinates to a geo keyword.",
+ "cirrussearch-geo-feature-invalid-distance": "Warning shown to users
when providing invalid distance qualifier to a geo keyword.",
+ "cirrussearch-geo-feature-unknown-title": "Warning shown to users when
providing an unknown title to a geo keyword.",
+ "cirrussearch-geo-feature-title-no-coordinates": "Warning shown to
users when providing an article title without coordinates to a geo keyword.",
+ "cirrussearch-feature-too-many-conditions": "Warning shown to users
when more conditions than are allowed were provided to a keyword feature.",
+ "cirrussearch-incategory-feature-no-valid-categories": "Warning shown
to users when no valid categories were found with their incategory keyword.",
+ "cirrussearch-mlt-feature-no-valid-titles": "Warning shown to users
when no valid titles were found with their more like keyword.",
+ "cirrussearch-mlt-not-configured": "Warning shown to users when
issueing a more like this query and the wiki hasn't configured it."
}
diff --git a/includes/Query/FileNumericFeature.php
b/includes/Query/FileNumericFeature.php
index 594e728..a8c3d3f 100644
--- a/includes/Query/FileNumericFeature.php
+++ b/includes/Query/FileNumericFeature.php
@@ -51,15 +51,19 @@
$field = $this->keyTable[$key];
- $sign = $this->extractSign( $value );
+ list( $sign, $number ) = $this->extractSign( $value );
// filesize treats no sign as >, since exact file size matches
make no sense
- if ( !$sign && $key === 'filesize' && strpos( $value, ',' ) ===
false ) {
+ if ( !$sign && $key === 'filesize' && strpos( $number, ',' )
=== false ) {
$sign = 1;
}
+ if ( !$this->validate( $context, $key, $value, $sign, $number )
) {
+ return [ null, false ];
+ }
+
$query =
- $this->buildNumericQuery( $field, $sign, $value, ( $key
=== 'filesize' ) ? 1024 : 1 );
+ $this->buildNumericQuery( $field, $sign, $number, (
$key === 'filesize' ) ? 1024 : 1 );
return [ $query, false ];
}
@@ -68,16 +72,63 @@
* Extract sign prefix which can be < or > or nothing.
* @param $value
* @param int $default
- * @return int 0 is equal, 1 is more, -1 is less
+ * @return array Two element array, first the sign: 0 is equal, 1 is
more, -1 is less,
+ * then the number to be compared.
*/
- protected function extractSign( &$value, $default = 0 ) {
+ protected function extractSign( $value, $default = 0 ) {
if ( $value[0] == '>' || $value[0] == '<' ) {
$sign = ( $value[0] == '>' ) ? 1 : - 1;
- $value = substr( $value, 1 );
+ return [$sign, substr( $value, 1 )];
} else {
- return $default;
+ return [$default, $value];
}
- return $sign;
+ }
+
+ /**
+ * Validate that input arguments will construct a valid query
+ *
+ * @param SearchContext $context
+ * @param string $key The matched query keyword
+ * @param string $value The original value provided by user
+ * @param int $sign
+ * @param string $number
+ * @return bool
+ */
+ protected function validate( SearchContext $context, $key, $value,
$sign, $number ) {
+ $valid = true;
+ if ( $sign && strpos( $number, ',' ) !== false ) {
+ $context->addWarning(
+
'cirrussearch-file-numeric-feature-multi-argument-w-sign',
+ $key,
+ $number
+ );
+ $valid = false;
+ } elseif ( $sign || strpos( $number, ',' ) === false ) {
+ if ( !is_numeric( $number ) ) {
+ $this->nanWarning( $context, $key, $number );
+ $valid = false;
+ }
+ } else {
+ $numbers = explode( ',', $number, 2 );
+ if ( !is_numeric( $numbers[0] ) ) {
+ $this->nanWarning( $context, $key, $numbers[0]
);
+ $valid = false;
+ }
+ if ( !is_numeric( $numbers[1] ) ) {
+ $this->nanWarning( $context, $key, $numbers[1]
);
+ $valid = false;
+ }
+ }
+
+ return $valid;
+ }
+
+ protected function nanWarning( $context, $key, $number ) {
+ $context->addWarning(
+ 'cirrussearch-file-numeric-feature-not-a-number',
+ $key,
+ $number
+ );
}
/**
@@ -90,9 +141,6 @@
*/
protected function buildNumericQuery( $field, $sign, $number,
$multiplier = 1 ) {
if ( $sign ) {
- if ( !is_numeric( $number ) ) {
- return null;
- }
$number = intval( $number );
if ( $sign < 0 ) {
$range = [ 'lte' => $number * $multiplier ];
@@ -102,17 +150,11 @@
return new Query\Range( $field, $range );
} else {
if ( strpos( $number, ',' ) !== false ) {
- $numbers = explode( ',', $number );
- if ( !is_numeric( $numbers[0] ) || !is_numeric(
$numbers[1] ) ) {
- return null;
- }
+ $numbers = explode( ',', $number, 2 );
return new Query\Range( $field, [
'gte' => intval( $numbers[0] ) *
$multiplier,
'lte' => intval( $numbers[1] ) *
$multiplier
] );
- }
- if ( !is_numeric( $number ) ) {
- return null;
}
$query = new Query\Match();
$query->setFieldQuery( $field, (string)( $number *
$multiplier ) );
diff --git a/includes/Query/GeoFeature.php b/includes/Query/GeoFeature.php
index f55fd10..260bff9 100644
--- a/includes/Query/GeoFeature.php
+++ b/includes/Query/GeoFeature.php
@@ -56,16 +56,18 @@
*/
protected function doApply( SearchContext $context, $key, $value,
$quotedValue, $negated ) {
if ( !class_exists( GeoData::class ) ) {
+ $context->addWarning(
'cirrussearch-feature-not-available', $key );
return [ null, false ];
}
if ( substr( $key, -5 ) === 'title' ) {
list( $coord, $radius, $excludeDocId ) =
$this->parseGeoNearbyTitle(
- $context->getConfig(),
+ $context,
+ $key,
$value
);
} else {
- list( $coord, $radius ) = $this->parseGeoNearby( $value
);
+ list( $coord, $radius ) = $this->parseGeoNearby(
$context, $key, $value );
$excludeDocId = '';
}
@@ -87,13 +89,14 @@
* <title>
* <radius>,<title>
*
- * @param SearchConfig $config the Cirrus config object
+ * @param SearchContext $context
+ * @param string $key Key used to trigger feature
* @param string $text user input to parse
* @return array Three member array with Coordinate object, integer
radius
* in meters, and page id to exclude from results.. When invalid the
* Coordinate returned will be null.
*/
- public function parseGeoNearbyTitle( SearchConfig $config, $text ) {
+ public function parseGeoNearbyTitle( SearchContext $context, $key,
$text ) {
$title = Title::newFromText( $text );
if ( $title && $title->exists() ) {
// Default radius if not provided: 5km
@@ -104,24 +107,40 @@
// remaining text is a valid title to use.
$pieces = explode( ',', $text, 2 );
if ( count( $pieces ) !== 2 ) {
+ $context->addWarning(
+
"cirrussearch-geo-feature-invalid-coordinates",
+ $key, $text
+ );
return [ null, 0, '' ];
}
$radius = $this->parseDistance( $pieces[0] );
if ( $radius === null ) {
+ $context->addWarning(
+
"cirrussearch-geo-feature-invalid-distance",
+ $key, $pieces[0]
+ );
return [ null, 0, '' ];
}
$title = Title::newFromText( $pieces[1] );
if ( !$title || !$title->exists() ) {
+ $context->addWarning(
+
"cirrussearch-geo-feature-unknown-title",
+ $key, $pieces[1]
+ );
return [ null, 0, '' ];
}
}
$coord = GeoData::getPageCoordinates( $title );
if ( !$coord ) {
+ $context->addWarning(
+ 'cirrussearch-geo-feature-title-no-coordinates',
+ (string)$title
+ );
return [ null, 0, '' ];
}
- return [ $coord, $radius, $config->makeId(
$title->getArticleID() ) ];
+ return [ $coord, $radius, $context->getConfig()->makeId(
$title->getArticleID() ) ];
}
/**
@@ -131,17 +150,23 @@
* <lat>,<lon>
* <radius>,<lat>,<lon>
*
+ * @param SearchContext $context
+ * @param string $key
* @param string $text
* @return array Two member array with Coordinate object, and integer
radius
* in meters. When invalid the Coordinate returned will be null.
*/
- public function parseGeoNearby( $text ) {
+ public function parseGeoNearby( SearchContext $context, $key, $text ) {
$pieces = explode( ',', $text, 3 );
// Default radius if not provided: 5km
$radius = self::DEFAULT_RADIUS;
if ( count( $pieces ) === 3 ) {
$radius = $this->parseDistance( $pieces[0] );
if ( $radius === null ) {
+ $context->addWarning(
+
'cirrussearch-geo-feature-invalid-distance',
+ $key, $pieces[0]
+ );
return [ null, 0 ];
}
$lat = $pieces[1];
@@ -150,11 +175,19 @@
$lat = $pieces[0];
$lon = $pieces[1];
} else {
+ $context->addWarning(
+ 'cirrussearch-geo-feature-invalid-coordinates',
+ $key, $text
+ );
return [ null, 0 ];
}
$globe = new Globe( self::DEFAULT_GLOBE );
if ( !$globe->coordinatesAreValid( $lat, $lon ) ) {
+ $context->addWarning(
+ 'cirrussearch-geo-feature-invalid-coordinates',
+ $key, $text
+ );
return [ null, 0 ];
}
diff --git a/includes/Query/InCategoryFeature.php
b/includes/Query/InCategoryFeature.php
index 905e608..c07b14d 100644
--- a/includes/Query/InCategoryFeature.php
+++ b/includes/Query/InCategoryFeature.php
@@ -55,14 +55,26 @@
* string.
*/
protected function doApply( SearchContext $context, $key, $value,
$quotedValue, $negated ) {
- $categories = array_slice(
- explode( '|', $value ),
- 0,
- $this->maxConditions
- );
+ $categories = explode( '|', $value );
+ if ( count( $categories ) > $this->maxConditions ) {
+ $context->addWarning(
+ 'cirrussearch-feature-too-many-conditions',
+ $key,
+ $this->maxConditions
+ );
+ $categories = array_slice(
+ $categories,
+ 0,
+ $this->maxConditions
+ );
+ }
$filter = $this->matchPageCategories( $categories );
if ( $filter === null ) {
$context->setResultsPossible( false );
+ $context->addWarning(
+
'cirrussearch-incategory-feature-no-valid-categories',
+ $key
+ );
}
return [ $filter, false ];
diff --git a/includes/Query/LanguageFeature.php
b/includes/Query/LanguageFeature.php
index a4356cc..537ab00 100644
--- a/includes/Query/LanguageFeature.php
+++ b/includes/Query/LanguageFeature.php
@@ -42,7 +42,15 @@
protected function doApply( SearchContext $context, $key, $value,
$quotedValue, $negated ) {
$queries = [];
- $langs = array_slice( explode( ',', $value ), 0,
self::QUERY_LIMIT );
+ $langs = explode( ',', $value );
+ if ( count( $langs ) > self::QUERY_LIMIT ) {
+ $context->addWarning(
+ 'cirrussearch-feature-too-many-conditions',
+ $key,
+ self::QUERY_LIMIT
+ );
+ $langs = array_slice( $langs, 0, self::QUERY_LIMIT );
+ }
foreach ( $langs as $lang ) {
if ( strlen( trim( $lang ) ) > 0 ) {
$query = new \Elastica\Query\Match();
diff --git a/includes/Query/MoreLikeFeature.php
b/includes/Query/MoreLikeFeature.php
index 01837b7..016cfdf 100644
--- a/includes/Query/MoreLikeFeature.php
+++ b/includes/Query/MoreLikeFeature.php
@@ -51,7 +51,7 @@
// of input order when the result would be the same.
if ( $pos === 0 ) {
$titleString = substr( $term, $pos + strlen(
$prefix ) );
- $this->doApply( $context, $titleString,
$options );
+ $this->doApply( $context, $prefix,
$titleString, $options );
return "";
}
@@ -61,14 +61,22 @@
return $term;
}
- private function doApply( SearchContext $context, $titleString,
$options ) {
+ private function doApply( SearchContext $context, $prefix,
$titleString, $options ) {
$titles = $this->collectTitles( $titleString );
if ( !count( $titles ) ) {
+ $context->addWarning(
+ "cirrussearch-mlt-feature-no-valid-titles",
+ substr( $prefix, 0, -1 )
+ );
$context->setResultsPossible( false );
return;
}
$query = $this->buildMoreLikeQuery( $context, $titles, $options
);
if ( $query === null ) {
+ $context->addWarning(
+ "cirrussearch-mlt-not-configured",
+ $prefix
+ );
$context->setResultsPossible( false );
return;
}
diff --git a/includes/Query/RegexInSourceFeature.php
b/includes/Query/RegexInSourceFeature.php
index acd2c9c..5521a05 100644
--- a/includes/Query/RegexInSourceFeature.php
+++ b/includes/Query/RegexInSourceFeature.php
@@ -64,6 +64,10 @@
'/(?<not>-)?insource:\/(?<pattern>(?:[^\\\\\/]|\\\\.)+)\/(?<insensitive>i)? ?/',
function ( $matches ) use ( $context ) {
if ( !$this->enabled ) {
+ $context->addWarning(
+
'cirrussearch-feature-not-available',
+ 'insource regex'
+ );
return '';
}
diff --git a/includes/Search/SearchContext.php
b/includes/Search/SearchContext.php
index 8fe33dc..8e66345 100644
--- a/includes/Search/SearchContext.php
+++ b/includes/Search/SearchContext.php
@@ -180,6 +180,11 @@
private $escaper;
/**
+ * @var array[][] Warnings to be passed into StatusValue::warning()
+ */
+ private $warnings = [];
+
+ /**
* @param SearchConfig $config
* @param int[]|null $namespaces
*/
@@ -679,4 +684,19 @@
public function escaper() {
return $this->escaper;
}
+
+ /**
+ * @param string $message i18n message key
+ */
+ public function addWarning( $message /*, parameters... */ ) {
+ $this->warnings[] = func_get_args();
+ }
+
+ /**
+ * @return array[] Array of arrays. Each sub array is a set of values
+ * suitable for creating an i18n message.
+ */
+ public function getWarnings() {
+ return $this->warnings;
+ }
}
diff --git a/includes/Searcher.php b/includes/Searcher.php
index ec9ebe6..45391f4 100644
--- a/includes/Searcher.php
+++ b/includes/Searcher.php
@@ -365,6 +365,10 @@
}
}
+ foreach ( $this->searchContext->getWarnings() as $warning ) {
+ call_user_func_array( [ $status, 'warning' ], $warning
);
+ }
+
return $status;
}
diff --git a/tests/unit/Query/BaseSimpleKeywordFeatureTest.php
b/tests/unit/Query/BaseSimpleKeywordFeatureTest.php
index 9ee0e83..869c39c 100644
--- a/tests/unit/Query/BaseSimpleKeywordFeatureTest.php
+++ b/tests/unit/Query/BaseSimpleKeywordFeatureTest.php
@@ -11,11 +11,18 @@
* SimpleKeywordFeature
*/
abstract class BaseSimpleKeywordFeatureTest extends CirrusTestCase {
- protected function mockContextExpectingAddFilter( array $expectedQuery
= null ) {
+
+ protected function mockContext() {
$context = $this->getMockBuilder( SearchContext::class )
->disableOriginalConstructor()
->getMock();
+ $context->expects( $this->any() )->method( 'getConfig'
)->willReturn( new SearchConfig() );
+ return $context;
+ }
+
+ protected function mockContextExpectingAddFilter( array $expectedQuery
= null ) {
+ $context = $this->mockContext();
if ( $expectedQuery === null ) {
$context->expects( $this->never() )
->method( 'addFilter' );
@@ -27,8 +34,19 @@
return true;
} ) );
}
- $context->expects( $this->any() )->method( 'getConfig'
)->willReturn( new SearchConfig() );
return $context;
}
+
+ protected function assertWarnings( KeywordFeature $feature, $expected,
$term ) {
+ $warnings = [];
+ $context = $this->mockContext();
+ $context->expects( $this->any() )
+ ->method( 'addWarning' )
+ ->will( $this->returnCallback( function () use (
&$warnings ) {
+ $warnings[] = func_get_args();
+ } ) );
+ $feature->apply( $context, $term );
+ $this->assertEquals( $expected, $warnings );
+ }
}
diff --git a/tests/unit/Query/FileFeatureTest.php
b/tests/unit/Query/FileFeatureTest.php
index de9fc1e..88270a2 100644
--- a/tests/unit/Query/FileFeatureTest.php
+++ b/tests/unit/Query/FileFeatureTest.php
@@ -17,6 +17,15 @@
] ],
'filesize:10',
],
+ 'filesize allows multi-argument' => [
+ [ 'range' => [
+ 'file_size' => [
+ 'gte' => 125952,
+ 'lte' => 328704,
+ ]
+ ] ],
+ 'filesize:123,321',
+ ],
'numeric with with no sign - exact match' => [
[ 'match' => [
'file_bits' => [
@@ -124,4 +133,31 @@
$feature = new FileTypeFeature();
$feature->apply( $context, $term );
}
+
+ public function warningNumericProvider() {
+ return [
+ 'arguments must be numeric' => [
+ [ [
'cirrussearch-file-numeric-feature-not-a-number', 'filebits', 'celery' ] ],
+ 'filebits:celery'
+ ],
+ 'each argument in a multi-value must be a number' => [
+ [
+ [
'cirrussearch-file-numeric-feature-not-a-number', 'fileheight', 'something' ],
+ [
'cirrussearch-file-numeric-feature-not-a-number', 'fileheight', 'voodoo' ]
+ ],
+ 'fileheight:something,voodoo',
+ ],
+ 'multi-argument with a sign is invalid' => [
+ [ [
'cirrussearch-file-numeric-feature-multi-argument-w-sign', 'fileh', '200,400' ]
],
+ 'fileh:>200,400',
+ ],
+ ];
+ }
+
+ /**
+ * @dataProvider warningNumericProvider
+ */
+ public function testWarningNumeric( $expected, $term ) {
+ $this->assertWarnings( new FileNumericFeature(), $expected,
$term );
+ }
}
diff --git a/tests/unit/Query/GeoFeatureTest.php
b/tests/unit/Query/GeoFeatureTest.php
index 126657c..ec846d1 100644
--- a/tests/unit/Query/GeoFeatureTest.php
+++ b/tests/unit/Query/GeoFeatureTest.php
@@ -30,7 +30,7 @@
*
* @group CirrusSearch
*/
-class GeoFeatureTest extends CirrusTestCase {
+class GeoFeatureTest extends BaseSimpleKeywordFeatureTest {
public function parseDistanceProvider() {
return [
@@ -164,7 +164,8 @@
public function testParseGeoNearby( $expected, $value ) {
if ( class_exists( Coord::class ) ) {
$feature = new GeoFeature;
- $result = $feature->parseGeoNearby( $value );
+ $context = $this->mockContextExpectingAddFilter();
+ $result = $feature->parseGeoNearby( $context,
'nearcoord', $value );
if ( $result[0] instanceof Coord ) {
$result[0] = [ 'lat' => $result[0]->lat, 'lon'
=> $result[0]->lon ];
}
@@ -272,21 +273,54 @@
// Inject fake page with comma in it as well
MediaWikiServices::getInstance()->getLinkCache()
->addGoodLinkObj( 1234567, Title::newFromText(
'Washington, D.C.' ) );
-
- $config = $this->getMock( SearchConfig::class );
- $config->expects( $this->any() )
- ->method( 'makeId' )
- ->will( $this->returnCallback( function ( $id ) {
- return $id;
- } ) );
+ $context = $this->mockContextExpectingAddFilter();
// Finally run the test
$feature = new GeoFeature;
- $result = $feature->parseGeoNearbyTitle( $config, $value );
+ $result = $feature->parseGeoNearbyTitle( $context, 'neartitle',
$value );
if ( $result[0] instanceof Coord ) {
$result[0] = [ 'lat' => $result[0]->lat, 'lon' =>
$result[0]->lon ];
}
$this->assertEquals( $expected, $result );
}
+
+ public function geoWarningsProvider() {
+ return [
+ 'coordinates must be two or three pieces' => [
+ [ [
'cirrussearch-geo-feature-invalid-coordinates', 'nearcoord', 'hi' ] ],
+ 'nearcoord:hi',
+ ],
+ 'three piece coordinates must use valid radius with
qualifier' => [
+ [ [
'cirrussearch-geo-feature-invalid-distance', 'nearcoord', '40s' ] ],
+ 'nearcoord:40s,12,21'
+ ],
+ 'coordinates must be valid earth coordinates' => [
+ [ [
'cirrussearch-geo-feature-invalid-coordinates', 'boost-nearcoord', '12345,123'
] ],
+ 'boost-nearcoord:12345,123',
+ ],
+ 'titles must be known' => [
+ [ [ 'cirrussearch-geo-feature-unknown-title',
'neartitle', 'Some unknown page' ] ],
+ 'neartitle:"10km,Some unknown page"',
+ ],
+ 'titles must have coordinates' => [
+ [ [
'cirrussearch-geo-feature-title-no-coordinates', 'Foobar' ] ],
+ 'neartitle:Foobar',
+ ],
+ ];
+ }
+
+ /**
+ * @dataProvider geoWarningsProvider
+ */
+ public function testGeoWarnings( $expected, $term ) {
+ if ( ! class_exists( Coord::class ) ) {
+ $this->markTestSkipped( 'GeoData extension must be
installed' );
+ return;
+ }
+ // Inject fake San Francisco page into LinkCache so it "exists"
+ MediaWikiServices::getInstance()->getLinkCache()
+ ->addGoodLinkObj( 98765, Title::newFromText( 'Foobar' )
);
+ $this->assertWarnings( new GeoFeature(), $expected, $term );
+ }
}
diff --git a/tests/unit/Query/InCategoryFeatureTest.php
b/tests/unit/Query/InCategoryFeatureTest.php
index 774257d..7d6658c 100644
--- a/tests/unit/Query/InCategoryFeatureTest.php
+++ b/tests/unit/Query/InCategoryFeatureTest.php
@@ -157,4 +157,24 @@
->will( $this->returnValue( $db ) );
$this->setService( 'DBLoadBalancer', $lb );
}
+
+ public function testTooManyCategoriesWarning() {
+ $this->assertWarnings(
+ new InCategoryFeature( new \HashConfig( [
+ 'CirrusSearchMaxIncategoryOptions' => 2,
+ ] ) ),
+ [ [ 'cirrussearch-feature-too-many-conditions',
'incategory', 2 ] ],
+ 'incategory:a|b|c'
+ );
+ }
+
+ public function testCategoriesMustExistWarning() {
+ $this->assertWarnings(
+ new InCategoryFeature( new \HashConfig( [
+ 'CirrusSearchMaxIncategoryOptions' => 2,
+ ] ) ),
+ [ [
'cirrussearch-incategory-feature-no-valid-categories', 'incategory' ] ],
+ 'incategory:id:74,id:18'
+ );
+ }
}
diff --git a/tests/unit/Query/LanguageFeatureTest.php
b/tests/unit/Query/LanguageFeatureTest.php
new file mode 100644
index 0000000..795fa40
--- /dev/null
+++ b/tests/unit/Query/LanguageFeatureTest.php
@@ -0,0 +1,27 @@
+<?php
+
+namespace CirrusSearch\Query;
+
+/**
+ * @group CirrusSearch
+ */
+class LanguageFeatureText extends BaseSimpleKeywordFeatureTest {
+
+ public function testTooManyLanguagesWarning() {
+ $this->assertWarnings(
+ new LanguageFeature(),
+ [ [ 'cirrussearch-feature-too-many-conditions',
'inlanguage', LanguageFeature::QUERY_LIMIT ] ],
+ 'inlanguage:' . implode( ',', range(1,40))
+ );
+ }
+
+ public function testCategoriesMustExistWarning() {
+ $this->assertWarnings(
+ new InCategoryFeature( new \HashConfig( [
+ 'CirrusSearchMaxIncategoryOptions' => 2,
+ ] ) ),
+ [ [
'cirrussearch-incategory-feature-no-valid-categories', 'incategory' ] ],
+ 'incategory:id:74,id:18'
+ );
+ }
+}
diff --git a/tests/unit/Query/MoreLikeFeatureTest.php
b/tests/unit/Query/MoreLikeFeatureTest.php
index da9333c..d7484d3 100644
--- a/tests/unit/Query/MoreLikeFeatureTest.php
+++ b/tests/unit/Query/MoreLikeFeatureTest.php
@@ -3,6 +3,7 @@
namespace CirrusSearch\Query;
use CirrusSearch\CirrusTestCase;
+use CirrusSearch\SearchConfig;
use CirrusSearch\Search\SearchContext;
use MediaWiki\MediaWikiServices;
use Title;
@@ -27,7 +28,7 @@
*
* @group CirrusSearch
*/
-class MoreLikeFeatureTest extends CirrusTestCase {
+class MoreLikeFeatureTest extends BaseSimpleKeywordFeatureTest {
public function applyProvider() {
return [
@@ -144,4 +145,24 @@
}
}
}
+
+ public function testWarningsForUnknownPages() {
+ MediaWikiServices::getInstance()->getLinkCache()
+ ->addGoodLinkObj( 12345, Title::newFromText( 'Some
page' ) );
+ $this->assertWarnings(
+ new MoreLikeFeature( new SearchConfig() ),
+ [],
+ 'morelike:Some page'
+ );
+ $this->assertWarnings(
+ new MoreLikeFeature( new SearchConfig() ),
+ [],
+ 'morelike:Some page|Title that doesnt exist'
+ );
+ $this->assertWarnings(
+ new MoreLikeFeature( new SearchConfig() ),
+ [ [ 'cirrussearch-mlt-feature-no-valid-titles',
'morelike' ] ],
+ 'morelike:Title that doesnt exist'
+ );
+ }
}
diff --git a/tests/unit/Query/RegexInSourceFeatureTest.php
b/tests/unit/Query/RegexInSourceFeatureTest.php
new file mode 100644
index 0000000..68b2736
--- /dev/null
+++ b/tests/unit/Query/RegexInSourceFeatureTest.php
@@ -0,0 +1,23 @@
+<?php
+
+namespace CirrusSearch\Query;
+
+use CirrusSearch\SearchConfig;
+use CirrusSearch\Test\HashSearchConfig;
+
+/**
+ * @group CirrusSearch
+ */
+class RegexInSourceFeatureText extends BaseSimpleKeywordFeatureTest {
+
+ public function testGivesWarningIfNotEnabled() {
+ $config = new HashSearchConfig( [
+ 'CirrusSearchEnableRegex' => false,
+ ], [ 'inherit' ] );
+ $this->assertWarnings(
+ new RegexInSourceFeature( $config ),
+ [ [ 'cirrussearch-feature-not-available', 'insource
regex' ] ],
+ 'insource:/abc/'
+ );
+ }
+}
--
To view, visit https://gerrit.wikimedia.org/r/326057
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia574888bc5ee2669701ead8b2f0aec1d9d5c49ea
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/CirrusSearch
Gerrit-Branch: master
Gerrit-Owner: EBernhardson <ebernhard...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
