From 446fb431a9aeb1538d7108b5ffca34d7d81ee3d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C4=9Bj=20Such=C3=A1nek?= Date: Sun, 16 Aug 2020 19:14:22 +0200 Subject: [PATCH] [changetags] Backend support for excluded tag filter This patch adds support in the backend but not the frontend. Bug: T119072 Bug: T174349 Change-Id: I05d20818108ef26cb5dfd4fdf0e76f6054f7b178 --- includes/changetags/ChangeTags.php | 52 ++++++++++++------- .../includes/changetags/ChangeTagsTest.php | 26 +++++++++- 2 files changed, 56 insertions(+), 22 deletions(-) diff --git a/includes/changetags/ChangeTags.php b/includes/changetags/ChangeTags.php index fa52ca65c1f..b0ff7e6ebd7 100644 --- a/includes/changetags/ChangeTags.php +++ b/includes/changetags/ChangeTags.php @@ -878,23 +878,24 @@ class ChangeTags { * Handles selecting tags, and filtering. * Needs $tables to be set up properly, so we can figure out which join conditions to use. * - * WARNING: If $filter_tag contains more than one tag, this function will add DISTINCT, - * which may cause performance problems for your query unless you put the ID field of your - * table at the end of the ORDER BY, and set a GROUP BY equal to the ORDER BY. For example, - * if you had ORDER BY foo_timestamp DESC, you will now need GROUP BY foo_timestamp, foo_id - * ORDER BY foo_timestamp DESC, foo_id DESC. + * WARNING: If $filter_tag contains more than one tag and $exclude is false, this function + * will add DISTINCT, which may cause performance problems for your query unless you put + * the ID field of your table at the end of the ORDER BY, and set a GROUP BY equal to the + * ORDER BY. For example, if you had ORDER BY foo_timestamp DESC, you will now need + * GROUP BY foo_timestamp, foo_id ORDER BY foo_timestamp DESC, foo_id DESC. * * @param string|array &$tables Table names, see Database::select * @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 string|array &$options Options, see Database::select - * @param string|array $filter_tag Tag(s) to select on + * @param string|array $filter_tag Tag(s) to select on (OR) + * @param bool $exclude If true, exclude tag(s) from $filter_tag (NOR) * * @throws MWException When unable to determine appropriate JOIN condition for tagging */ public static function modifyDisplayQuery( &$tables, &$fields, &$conds, - &$join_conds, &$options, $filter_tag = '' + &$join_conds, &$options, $filter_tag = '', bool $exclude = false ) { $useTagFilter = MediaWikiServices::getInstance()->getMainConfig()->get( 'UseTagFilter' ); @@ -923,29 +924,40 @@ class ChangeTags { // Somebody wants to filter on a tag. // Add an INNER JOIN on change_tag $tagTable = self::getDisplayTableName(); - $tables[] = $tagTable; - $join_conds[$tagTable] = [ 'JOIN', $join_cond ]; $filterTagIds = []; $changeTagDefStore = MediaWikiServices::getInstance()->getChangeTagDefStore(); foreach ( (array)$filter_tag as $filterTagName ) { try { $filterTagIds[] = $changeTagDefStore->getId( $filterTagName ); } catch ( NameTableAccessException $exception ) { - // Return nothing. - $conds[] = '0=1'; - break; } } - if ( $filterTagIds !== [] ) { - $conds['ct_tag_id'] = $filterTagIds; - } + if ( $exclude ) { + if ( $filterTagIds !== [] ) { + $tables[] = $tagTable; + $join_conds[$tagTable] = [ + 'LEFT JOIN', + [ $join_cond, 'ct_tag_id' => $filterTagIds ] + ]; + $conds[] = "$tagTable.ct_tag_id IS NULL"; + } + } else { + $tables[] = $tagTable; + $join_conds[$tagTable] = [ 'JOIN', $join_cond ]; + if ( $filterTagIds !== [] ) { + $conds['ct_tag_id'] = $filterTagIds; + } else { + // all tags were invalid, return nothing + $conds[] = '0=1'; + } - if ( - is_array( $filter_tag ) && count( $filter_tag ) > 1 && - !in_array( 'DISTINCT', $options ) - ) { - $options[] = 'DISTINCT'; + if ( + is_array( $filter_tag ) && count( $filter_tag ) > 1 && + !in_array( 'DISTINCT', $options ) + ) { + $options[] = 'DISTINCT'; + } } } } diff --git a/tests/phpunit/includes/changetags/ChangeTagsTest.php b/tests/phpunit/includes/changetags/ChangeTagsTest.php index 8d231543b14..aa3adad6db6 100644 --- a/tests/phpunit/includes/changetags/ChangeTagsTest.php +++ b/tests/phpunit/includes/changetags/ChangeTagsTest.php @@ -40,7 +40,8 @@ class ChangeTagsTest extends MediaWikiIntegrationTestCase { $filter_tag, $useTags, $avoidReopeningTables, - $modifiedQuery + $modifiedQuery, + $exclude = false ) { $this->setMwGlobals( 'wgUseTagFilter', $useTags ); @@ -66,7 +67,8 @@ class ChangeTagsTest extends MediaWikiIntegrationTestCase { $origQuery['conds'], $origQuery['join_conds'], $origQuery['options'], - $filter_tag + $filter_tag, + $exclude ); if ( !isset( $modifiedQuery['exception'] ) ) { $this->assertArrayEquals( @@ -275,6 +277,26 @@ class ChangeTagsTest extends MediaWikiIntegrationTestCase { 'options' => [ 'ORDER BY' => 'rc_timestamp DESC', 'DISTINCT' ], ] ], + 'recentchanges query with exclusive 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 + false, // not avoiding reopening tables + [ + 'tables' => [ 'recentchanges', 'change_tag' ], + 'fields' => [ 'rc_id', 'rc_timestamp', 'ts_tags' => $groupConcats['recentchanges'] ], + 'conds' => [ "rc_timestamp > '20170714183203'", 'change_tag.ct_tag_id IS NULL' ], + 'join_conds' => [ 'change_tag' => [ 'LEFT JOIN', [ 'ct_rc_id=rc_id', 'ct_tag_id' => [ 1, 2 ] ] ] ], + 'options' => [ 'ORDER BY' => 'rc_timestamp DESC' ], + ], + true // exclude + ], 'recentchanges query with multiple tag filter that already has DISTINCT' => [ [ 'tables' => [ 'recentchanges' ],