Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge "RC Filters: Detect filters conflicts to by-pass db query"

Browse source
This commit is contained in:
jenkins-bot 2017-04-19 03:20:22 +00:00 committed by Gerrit Code Review
commit 6c81cf528d
8 changed files with 305 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
includes/changes/ChangesListBooleanFilter.php
View file

@ -223,4 +223,13 @@ class ChangesListBooleanFilter extends ChangesListFilter {
return $output;
}
/**
* @inheritdoc
*/
public function isSelected( FormOptions $opts ) {
return !$opts[ $this->getName() ] &&
array_filter( $this->getSiblings(), function ( $sibling ) use ( $opts ) {
return $opts[ $sibling->getName() ];
} );
}
}

105
includes/changes/ChangesListFilter.php
View file

@ -227,6 +227,7 @@ abstract class ChangesListFilter {
if ( $other instanceof ChangesListFilterGroup ) {
$this->conflictingGroups[] = [
'group' => $other->getName(),
'groupObject' => $other,
'globalDescription' => $globalDescription,
'contextDescription' => $contextDescription,
];
@ -234,6 +235,7 @@ abstract class ChangesListFilter {
$this->conflictingFilters[] = [
'group' => $other->getGroup()->getName(),
'filter' => $other->getName(),
'filterObject' => $other,
'globalDescription' => $globalDescription,
'contextDescription' => $contextDescription,
];
@ -385,6 +387,8 @@ abstract class ChangesListFilter {
);
foreach ( $conflicts as $conflictInfo ) {
unset( $conflictInfo['filterObject'] );
unset( $conflictInfo['groupObject'] );
$output['conflicts'][] = $conflictInfo;
array_push(
$output['messageKeys'],
@ -395,4 +399,105 @@ abstract class ChangesListFilter {
return $output;
}
/**
* Checks whether this filter is selected in the provided options
*
* @param FormOptions $opts
* @return bool
*/
abstract public function isSelected( FormOptions $opts );
/**
* Get groups conflicting with this filter
*
* @return ChangesListFilterGroup[]
*/
public function getConflictingGroups() {
return array_map(
function ( $conflictDesc ) {
return $conflictDesc[ 'groupObject' ];
},
$this->conflictingGroups
);
}
/**
* Get filters conflicting with this filter
*
* @return ChangesListFilter[]
*/
public function getConflictingFilters() {
return array_map(
function ( $conflictDesc ) {
return $conflictDesc[ 'filterObject' ];
},
$this->conflictingFilters
);
}
/**
* Check if the conflict with a group is currently "active"
*
* @param ChangesListFilterGroup $group
* @param FormOptions $opts
* @return bool
*/
public function activelyInConflictWithGroup( ChangesListFilterGroup $group, FormOptions $opts ) {
if ( $group->anySelected( $opts ) && $this->isSelected( $opts ) ) {
/** @var ChangesListFilter $siblingFilter */
foreach ( $this->getSiblings() as $siblingFilter ) {
if ( $siblingFilter->isSelected( $opts ) && !$siblingFilter->hasConflictWithGroup( $group ) ) {
return false;
}
}
return true;
}
return false;
}
private function hasConflictWithGroup( ChangesListFilterGroup $group ) {
return in_array( $group, $this->getConflictingGroups() );
}
/**
* Check if the conflict with a filter is currently "active"
*
* @param ChangesListFilter $filter
* @param FormOptions $opts
* @return bool
*/
public function activelyInConflictWithFilter( ChangeslistFilter $filter, FormOptions $opts ) {
if ( $this->isSelected( $opts ) && $filter->isSelected( $opts ) ) {
/** @var ChangesListFilter $siblingFilter */
foreach ( $this->getSiblings() as $siblingFilter ) {
if (
$siblingFilter->isSelected( $opts ) &&
!$siblingFilter->hasConflictWithFilter( $filter )
) {
return false;
}
}
return true;
}
return false;
}
private function hasConflictWithFilter( ChangeslistFilter $filter ) {
return in_array( $filter, $this->getConflictingFilters() );
}
/**
* Get filters in the same group
*
* @return ChangesListFilter[]
*/
protected function getSiblings() {
return array_filter(
$this->getGroup()->getFilters(),
function ( $filter ) {
return $filter !== $this;
}
);
}
}

47
includes/changes/ChangesListFilterGroup.php
View file

@ -261,6 +261,7 @@ abstract class ChangesListFilterGroup {
if ( $other instanceof ChangesListFilterGroup ) {
$this->conflictingGroups[] = [
'group' => $other->getName(),
'groupObject' => $other,
'globalDescription' => $globalDescription,
'contextDescription' => $contextDescription,
];
@ -268,6 +269,7 @@ abstract class ChangesListFilterGroup {
$this->conflictingFilters[] = [
'group' => $other->getGroup()->getName(),
'filter' => $other->getName(),
'filterObject' => $other,
'globalDescription' => $globalDescription,
'contextDescription' => $contextDescription,
];
@ -390,6 +392,8 @@ abstract class ChangesListFilterGroup {
foreach ( $conflicts as $conflictInfo ) {
$output['conflicts'][] = $conflictInfo;
unset( $conflictInfo['filterObject'] );
unset( $conflictInfo['groupObject'] );
array_push(
$output['messageKeys'],
$conflictInfo['globalDescription'],
@ -399,4 +403,47 @@ abstract class ChangesListFilterGroup {
return $output;
}
/**
* Get groups conflicting with this filter group
*
* @return ChangesListFilterGroup[]
*/
public function getConflictingGroups() {
return array_map(
function ( $conflictDesc ) {
return $conflictDesc[ 'groupObject' ];
},
$this->conflictingGroups
);
}
/**
* Get filters conflicting with this filter group
*
* @return ChangesListFilter[]
*/
public function getConflictingFilters() {
return array_map(
function ( $conflictDesc ) {
return $conflictDesc[ 'filterObject' ];
},
$this->conflictingFilters
);
}
/**
* Check if any filter in this group is selected
*
* @param FormOptions $opts
* @return bool
*/
public function anySelected( FormOptions $opts ) {
return !!count( array_filter(
$this->getFilters(),
function ( ChangesListFilter $filter ) use ( $opts ) {
return $filter->isSelected( $opts );
}
) );
}
}

11
includes/changes/ChangesListStringOptionsFilter.php
View file

@ -14,4 +14,15 @@ class ChangesListStringOptionsFilter extends ChangesListFilter {
public function displaysOnUnstructuredUi() {
return false;
}
/**
* @inheritdoc
*/
public function isSelected( FormOptions $opts ) {
$values = explode(
ChangesListStringOptionsFilterGroup::SEPARATOR,
$opts[ $this->getGroup()->getName() ]
);
return in_array( $this->getName(), $values );
}
}

44
includes/specialpage/ChangesListSpecialPage.php
View file

@ -422,6 +422,50 @@ abstract class ChangesListSpecialPage extends SpecialPage {
];
}
/**
* Check if filters are in conflict and guaranteed to return no results.
*
* @return bool
*/
protected function areFiltersInConflict() {
$opts = $this->getOptions();
/** @var ChangesListFilterGroup $group */
foreach ( $this->getFilterGroups() as $group ) {
if ( $group->getConflictingGroups() ) {
wfLogWarning(
$group->getName() .
" specifies conflicts with other groups but these are not supported yet."
);
}
/** @var ChangesListFilter $conflictingFilter */
foreach ( $group->getConflictingFilters() as $conflictingFilter ) {
if ( $conflictingFilter->activelyInConflictWithGroup( $group, $opts ) ) {
return true;
}
}
/** @var ChangesListFilter $filter */
foreach ( $group->getFilters() as $filter ) {
/** @var ChangesListFilter $conflictingFilter */
foreach ( $filter->getConflictingFilters() as $conflictingFilter ) {
if (
$conflictingFilter->activelyInConflictWithFilter( $filter, $opts ) &&
$filter->activelyInConflictWithFilter( $conflictingFilter, $opts )
) {
return true;
}
}
}
}
return false;
}
/**
* Main execution point
*

4
includes/specials/SpecialRecentchanges.php
View file

@ -268,6 +268,10 @@ class SpecialRecentChanges extends ChangesListSpecialPage {
return false;
}
if ( $this->areFiltersInConflict() ) {
return false;
}
// array_merge() is used intentionally here so that hooks can, should
// they so desire, override the ORDER BY / LIMIT condition(s); prior to
// MediaWiki 1.26 this used to use the plus operator instead, which meant

81
tests/phpunit/includes/specialpage/ChangesListSpecialPageTest.php
View file

@ -801,4 +801,85 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
],
];
}
public function provideGetFilterConflicts() {
return [
[
"parameters" => [],
"expectedConflicts" => false,
],
[
"parameters" => [
"hideliu" => true,
"userExpLevel" => "newcomer",
],
"expectedConflicts" => true,
],
[
"parameters" => [
"hideanons" => true,
"userExpLevel" => "learner",
],
"expectedConflicts" => false,
],
[
"parameters" => [
"hidemajor" => true,
"hidenewpages" => true,
"hidepageedits" => true,
"hidecategorization" => false,
"hidelog" => true,
"hideWikidata" => true,
],
"expectedConflicts" => true,
],
[
"parameters" => [
"hidemajor" => true,
"hidenewpages" => false,
"hidepageedits" => true,
"hidecategorization" => false,
"hidelog" => false,
"hideWikidata" => true,
],
"expectedConflicts" => true,
],
[
"parameters" => [
"hidemajor" => true,
"hidenewpages" => false,
"hidepageedits" => false,
"hidecategorization" => true,
"hidelog" => true,
"hideWikidata" => true,
],
"expectedConflicts" => false,
],
[
"parameters" => [
"hideminor" => true,
"hidenewpages" => true,
"hidepageedits" => true,
"hidecategorization" => false,
"hidelog" => true,
"hideWikidata" => true,
],
"expectedConflicts" => false,
],
];
}
/**
* @dataProvider provideGetFilterConflicts
*/
public function testGetFilterConflicts( $parameters, $expectedConflicts ) {
$context = new RequestContext;
$context->setRequest( new FauxRequest( $parameters ) );
$this->changesListSpecialPage->setContext( $context );
$this->assertEquals(
$expectedConflicts,
$this->changesListSpecialPage->areFiltersInConflict()
);
}
}

4
tests/phpunit/mocks/MockChangesListFilter.php
View file

@ -8,4 +8,8 @@ class MockChangesListFilter extends ChangesListFilter {
'instead of testing the abstract class'
);
}
public function isSelected( FormOptions $opts ) {
return false;
}
}