Catrope has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/365430 )
Change subject: Add unit tests for ChangeTags::modifyDisplayQuery()
......................................................................
Add unit tests for ChangeTags::modifyDisplayQuery()
The rest of ChangeTags.php should have tests too, but this is a start.
Also fix up ChangeTags::modifyDisplayQuery() to work when strings
are passed for certain parameters, which its documentation claims
it supports but which leads to errors in practice.
Change-Id: I2fe61bcb716a369a14a45c2843817a6557d44f7c
---
M includes/changetags/ChangeTags.php
A tests/phpunit/includes/changetags/ChangeTagsTest.php
2 files changed, 257 insertions(+), 6 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/30/365430/1
diff --git a/includes/changetags/ChangeTags.php
b/includes/changetags/ChangeTags.php
index 14c2012..e1f83d5 100644
--- a/includes/changetags/ChangeTags.php
+++ b/includes/changetags/ChangeTags.php
@@ -653,7 +653,7 @@
* @param string|array $fields Fields used in query, see
Database::select
* @param string|array $conds Conditions used in query, see
Database::select
* @param array $join_conds Join conditions, see Database::select
- * @param array $options Options, see Database::select
+ * @param string|array $options Options, see Database::select
* @param bool|string|array $filter_tag Tag(s) to select on
*
* @throws MWException When unable to determine appropriate JOIN
condition for tagging
@@ -667,18 +667,19 @@
}
// Figure out which ID field to use
- if ( in_array( 'recentchanges', $tables ) ) {
+ if ( in_array( 'recentchanges', (array)$tables ) ) {
$join_cond = 'ct_rc_id=rc_id';
- } elseif ( in_array( 'logging', $tables ) ) {
+ } elseif ( in_array( 'logging', (array)$tables ) ) {
$join_cond = 'ct_log_id=log_id';
- } elseif ( in_array( 'revision', $tables ) ) {
+ } elseif ( in_array( 'revision', (array)$tables ) ) {
$join_cond = 'ct_rev_id=rev_id';
- } elseif ( in_array( 'archive', $tables ) ) {
+ } elseif ( in_array( 'archive', (array)$tables ) ) {
$join_cond = 'ct_rev_id=ar_rev_id';
} else {
throw new MWException( 'Unable to determine appropriate
JOIN condition for tagging.' );
}
+ $fields = (array)$fields;
$fields['ts_tags'] = wfGetDB( DB_REPLICA
)->buildGroupConcatField(
',', 'change_tag', 'ct_tag', $join_cond
);
@@ -687,10 +688,13 @@
// Somebody wants to filter on a tag.
// Add an INNER JOIN on change_tag
+ $tables = (array)$tables;
$tables[] = 'change_tag';
$join_conds['change_tag'] = [ 'INNER JOIN', $join_cond
];
+ $conds = (array)$conds;
$conds['ct_tag'] = $filter_tag;
- if ( count( $filter_tag ) > 1 && !in_array( 'DISTINCT',
$options ) ) {
+ if ( count( $filter_tag ) > 1 && !in_array( 'DISTINCT',
(array)$options ) ) {
+ $options = (array)$options;
$options[] = 'DISTINCT';
}
}
diff --git a/tests/phpunit/includes/changetags/ChangeTagsTest.php
b/tests/phpunit/includes/changetags/ChangeTagsTest.php
new file mode 100644
index 0000000..2b5f665
--- /dev/null
+++ b/tests/phpunit/includes/changetags/ChangeTagsTest.php
@@ -0,0 +1,247 @@
+<?php
+
+/**
+ * @covers ChangeTags
+ */
+class ChangeTagsTest extends MediaWikiTestCase {
+
+ // TODO only modifyDisplayQuery is tested, nothing else is
+
+ /** @dataProvider provideModifyDisplayQuery */
+ public function testModifyDisplayQuery( $origQuery, $filter_tag,
$useTags, $modifiedQuery ) {
+ $this->setMwGlobals( 'wgUseTagFilter', $useTags );
+ // HACK resolve deferred group concats (see comment in
provideModifyDisplayQuery)
+ if ( isset( $modifiedQuery['fields']['ts_tags'] ) ) {
+ $modifiedQuery['fields']['ts_tags'] =
call_user_func_array(
+ [ wfGetDB( DB_REPLICA ),
'buildGroupConcatField' ],
+ $modifiedQuery['fields']['ts_tags']
+ );
+ }
+ if ( isset( $modifiedQuery['exception'] ) ) {
+ $this->setExpectedException(
$modifiedQuery['exception'] );
+ }
+ ChangeTags::modifyDisplayQuery(
+ $origQuery['tables'],
+ $origQuery['fields'],
+ $origQuery['conds'],
+ $origQuery['join_conds'],
+ $origQuery['options'],
+ $filter_tag
+ );
+ if ( !isset( $modifiedQuery['exception'] ) ) {
+ $this->assertArrayEquals(
+ $modifiedQuery,
+ $origQuery,
+ /* ordered = */ false,
+ /* named = */ true
+ );
+ }
+ }
+
+ public function provideModifyDisplayQuery() {
+ // HACK if we call $dbr->buildGroupConcatField() now, it will
return the wrong table names
+ // We have to have the test runner call it instead
+ $groupConcats = [
+ 'recentchanges' => [ ',', 'change_tag', 'ct_tag',
'ct_rc_id=rc_id' ],
+ 'logging' => [ ',', 'change_tag', 'ct_tag',
'ct_log_id=log_id' ],
+ 'revision' => [ ',', 'change_tag', 'ct_tag',
'ct_rev_id=rev_id' ],
+ 'archive' => [ ',', 'change_tag', 'ct_tag',
'ct_rev_id=ar_rev_id' ],
+ ];
+
+ return [
+ 'simple recentchanges query' => [
+ [
+ 'tables' => [ 'recentchanges' ],
+ 'fields' => [ 'rc_id', 'rc_timestamp' ],
+ 'conds' => [ "rc_timestamp >
'20170714183203'" ],
+ 'join_conds' => [],
+ 'options' => [ 'ORDER BY' =>
'rc_timestamp DESC' ],
+ ],
+ '', // no tag filter
+ true, // tag filtering enabled
+ [
+ 'tables' => [ 'recentchanges' ],
+ 'fields' => [ 'rc_id', 'rc_timestamp',
'ts_tags' => $groupConcats['recentchanges'] ],
+ 'conds' => [ "rc_timestamp >
'20170714183203'" ],
+ 'join_conds' => [],
+ 'options' => [ 'ORDER BY' =>
'rc_timestamp DESC' ],
+ ]
+ ],
+ 'simple query with strings' => [
+ [
+ 'tables' => 'recentchanges',
+ 'fields' => 'rc_id',
+ 'conds' => "rc_timestamp >
'20170714183203'",
+ 'join_conds' => [],
+ 'options' => 'ORDER BY rc_timestamp
DESC',
+ ],
+ '', // no tag filter
+ true, // tag filtering enabled
+ [
+ 'tables' => 'recentchanges',
+ 'fields' => [ 'rc_id', 'ts_tags' =>
$groupConcats['recentchanges'] ],
+ 'conds' => "rc_timestamp >
'20170714183203'",
+ 'join_conds' => [],
+ 'options' => 'ORDER BY rc_timestamp
DESC',
+ ]
+ ],
+ 'recentchanges query with single tag filter' => [
+ [
+ 'tables' => [ 'recentchanges' ],
+ 'fields' => [ 'rc_id', 'rc_timestamp' ],
+ 'conds' => [ "rc_timestamp >
'20170714183203'" ],
+ 'join_conds' => [],
+ 'options' => [ 'ORDER BY' =>
'rc_timestamp DESC' ],
+ ],
+ 'foo',
+ true, // tag filtering enabled
+ [
+ 'tables' => [ 'recentchanges',
'change_tag' ],
+ 'fields' => [ 'rc_id', 'rc_timestamp',
'ts_tags' => $groupConcats['recentchanges'] ],
+ 'conds' => [ "rc_timestamp >
'20170714183203'", 'ct_tag' => 'foo' ],
+ 'join_conds' => [ 'change_tag' => [
'INNER JOIN', 'ct_rc_id=rc_id' ] ],
+ 'options' => [ 'ORDER BY' =>
'rc_timestamp DESC' ],
+ ]
+ ],
+ 'logging query with single tag filter and strings' => [
+ [
+ 'tables' => 'logging',
+ 'fields' => 'log_id',
+ 'conds' => "log_timestamp >
'20170714183203'",
+ 'join_conds' => [],
+ 'options' => 'ORDER BY log_timestamp
DESC',
+ ],
+ 'foo',
+ true, // tag filtering enabled
+ [
+ 'tables' => [ 'logging', 'change_tag' ],
+ 'fields' => [ 'log_id', 'ts_tags' =>
$groupConcats['logging'] ],
+ 'conds' => [ "log_timestamp >
'20170714183203'", 'ct_tag' => 'foo' ],
+ 'join_conds' => [ 'change_tag' => [
'INNER JOIN', 'ct_log_id=log_id' ] ],
+ 'options' => 'ORDER BY log_timestamp
DESC',
+ ]
+ ],
+ 'revision query with single tag filter' => [
+ [
+ 'tables' => [ 'revision' ],
+ 'fields' => [ 'rev_id', 'rev_timestamp'
],
+ 'conds' => [ "rev_timestamp >
'20170714183203'" ],
+ 'join_conds' => [],
+ 'options' => [ 'ORDER BY' =>
'rev_timestamp DESC' ],
+ ],
+ 'foo',
+ true, // tag filtering enabled
+ [
+ 'tables' => [ 'revision', 'change_tag'
],
+ 'fields' => [ 'rev_id',
'rev_timestamp', 'ts_tags' => $groupConcats['revision'] ],
+ 'conds' => [ "rev_timestamp >
'20170714183203'", 'ct_tag' => 'foo' ],
+ 'join_conds' => [ 'change_tag' => [
'INNER JOIN', 'ct_rev_id=rev_id' ] ],
+ 'options' => [ 'ORDER BY' =>
'rev_timestamp DESC' ],
+ ]
+ ],
+ 'archive query with single tag filter' => [
+ [
+ 'tables' => [ 'archive' ],
+ 'fields' => [ 'ar_id', 'ar_timestamp' ],
+ 'conds' => [ "ar_timestamp >
'20170714183203'" ],
+ 'join_conds' => [],
+ 'options' => [ 'ORDER BY' =>
'ar_timestamp DESC' ],
+ ],
+ 'foo',
+ true, // tag filtering enabled
+ [
+ 'tables' => [ 'archive', 'change_tag' ],
+ 'fields' => [ 'ar_id', 'ar_timestamp',
'ts_tags' => $groupConcats['archive'] ],
+ 'conds' => [ "ar_timestamp >
'20170714183203'", 'ct_tag' => 'foo' ],
+ 'join_conds' => [ 'change_tag' => [
'INNER JOIN', 'ct_rev_id=ar_rev_id' ] ],
+ 'options' => [ 'ORDER BY' =>
'ar_timestamp DESC' ],
+ ]
+ ],
+ 'unsupported table name throws exception (even without
tag filter)' => [
+ [
+ 'tables' => [ 'foobar' ],
+ 'fields' => [ 'fb_id', 'fb_timestamp' ],
+ 'conds' => [ "fb_timestamp >
'20170714183203'" ],
+ 'join_conds' => [],
+ 'options' => [ 'ORDER BY' =>
'fb_timestamp DESC' ],
+ ],
+ '',
+ true, // tag filtering enabled
+ [ 'exception' => MWException::class ]
+ ],
+ 'tag filter ignored when tag filtering is disabled' => [
+ [
+ 'tables' => [ 'archive' ],
+ 'fields' => [ 'ar_id', 'ar_timestamp' ],
+ 'conds' => [ "ar_timestamp >
'20170714183203'" ],
+ 'join_conds' => [],
+ 'options' => [ 'ORDER BY' =>
'ar_timestamp DESC' ],
+ ],
+ 'foo',
+ false, // tag filtering disabled
+ [
+ 'tables' => [ 'archive' ],
+ 'fields' => [ 'ar_id', 'ar_timestamp',
'ts_tags' => $groupConcats['archive'] ],
+ 'conds' => [ "ar_timestamp >
'20170714183203'" ],
+ 'join_conds' => [],
+ 'options' => [ 'ORDER BY' =>
'ar_timestamp DESC' ],
+ ]
+ ],
+ 'recentchanges query with multiple tag filter' => [
+ [
+ 'tables' => [ 'recentchanges' ],
+ 'fields' => [ 'rc_id', 'rc_timestamp' ],
+ 'conds' => [ "rc_timestamp >
'20170714183203'" ],
+ 'join_conds' => [],
+ 'options' => [ 'ORDER BY' =>
'rc_timestamp DESC' ],
+ ],
+ [ 'foo', 'bar' ],
+ true, // tag filtering enabled
+ [
+ 'tables' => [ 'recentchanges',
'change_tag' ],
+ 'fields' => [ 'rc_id', 'rc_timestamp',
'ts_tags' => $groupConcats['recentchanges'] ],
+ 'conds' => [ "rc_timestamp >
'20170714183203'", 'ct_tag' => [ 'foo', 'bar' ] ],
+ 'join_conds' => [ 'change_tag' => [
'INNER JOIN', 'ct_rc_id=rc_id' ] ],
+ 'options' => [ 'ORDER BY' =>
'rc_timestamp DESC', 'DISTINCT' ],
+ ]
+ ],
+ 'recentchanges query with multiple tag filter that
already has DISTINCT' => [
+ [
+ 'tables' => [ 'recentchanges' ],
+ 'fields' => [ 'rc_id', 'rc_timestamp' ],
+ 'conds' => [ "rc_timestamp >
'20170714183203'" ],
+ 'join_conds' => [],
+ 'options' => [ 'DISTINCT', 'ORDER BY'
=> 'rc_timestamp DESC' ],
+ ],
+ [ 'foo', 'bar' ],
+ true, // tag filtering enabled
+ [
+ 'tables' => [ 'recentchanges',
'change_tag' ],
+ 'fields' => [ 'rc_id', 'rc_timestamp',
'ts_tags' => $groupConcats['recentchanges'] ],
+ 'conds' => [ "rc_timestamp >
'20170714183203'", 'ct_tag' => [ 'foo', 'bar' ] ],
+ 'join_conds' => [ 'change_tag' => [
'INNER JOIN', 'ct_rc_id=rc_id' ] ],
+ 'options' => [ 'DISTINCT', 'ORDER BY'
=> 'rc_timestamp DESC' ],
+ ]
+ ],
+ 'recentchanges query with multiple tag filter with
strings' => [
+ [
+ 'tables' => 'recentchanges',
+ 'fields' => 'rc_id',
+ 'conds' => "rc_timestamp >
'20170714183203'",
+ 'join_conds' => [],
+ 'options' => 'ORDER BY rc_timestamp
DESC',
+ ],
+ [ 'foo', 'bar' ],
+ true, // tag filtering enabled
+ [
+ 'tables' => [ 'recentchanges',
'change_tag' ],
+ 'fields' => [ 'rc_id', 'ts_tags' =>
$groupConcats['recentchanges'] ],
+ 'conds' => [ "rc_timestamp >
'20170714183203'", 'ct_tag' => [ 'foo', 'bar' ] ],
+ 'join_conds' => [ 'change_tag' => [
'INNER JOIN', 'ct_rc_id=rc_id' ] ],
+ 'options' => [ 'ORDER BY rc_timestamp
DESC', 'DISTINCT' ],
+ ]
+ ],
+ ];
+ }
+
+}
--
To view, visit https://gerrit.wikimedia.org/r/365430
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2fe61bcb716a369a14a45c2843817a6557d44f7c
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Catrope <r...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
