From 42383f8faf08f6184ca9d455b3bd8469beed26b0 Mon Sep 17 00:00:00 2001 From: Roan Kattouw Date: Fri, 14 Jul 2017 19:39:49 -0700 Subject: [PATCH] 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. Normalize these parameters to arrays. Change-Id: I2fe61bcb716a369a14a45c2843817a6557d44f7c --- includes/changetags/ChangeTags.php | 8 +- .../includes/changetags/ChangeTagsTest.php | 247 ++++++++++++++++++ 2 files changed, 254 insertions(+), 1 deletion(-) create mode 100644 tests/phpunit/includes/changetags/ChangeTagsTest.php diff --git a/includes/changetags/ChangeTags.php b/includes/changetags/ChangeTags.php index feed7885ee..6ea4812e3e 100644 --- a/includes/changetags/ChangeTags.php +++ b/includes/changetags/ChangeTags.php @@ -653,7 +653,7 @@ class ChangeTags { * @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 @@ -666,6 +666,12 @@ class ChangeTags { $filter_tag = $wgRequest->getVal( 'tagfilter' ); } + // Normalize to arrays + $tables = (array)$tables; + $fields = (array)$fields; + $conds = (array)$conds; + $options = (array)$options; + // Figure out which ID field to use if ( in_array( 'recentchanges', $tables ) ) { $join_cond = 'ct_rc_id=rc_id'; diff --git a/tests/phpunit/includes/changetags/ChangeTagsTest.php b/tests/phpunit/includes/changetags/ChangeTagsTest.php new file mode 100644 index 0000000000..723d685627 --- /dev/null +++ b/tests/phpunit/includes/changetags/ChangeTagsTest.php @@ -0,0 +1,247 @@ +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' ], + ] + ], + ]; + } + +} -- 2.20.1