Slightly improve degroup action, remove Phan suppressions

Try to get the groups from the var first, and compute them if they don't
exist. Use getEffectiveGroups instead of getGroups as it's done when
setting the lazy var loader. Avoid a pointless array_intersect within an
array_diff. Remove Phan @suppress and add docblock to make it pass.

Change-Id: I49ec6a1264b767cefea55df66ef3b02d4f443b57
This commit is contained in:
Daimona Eaytoy 2019-01-27 13:52:41 +01:00
parent 5b4ea18045
commit 451666272e
3 changed files with 33 additions and 27 deletions

View file

@ -1704,11 +1704,17 @@ class AbuseFilter {
break;
case 'degroup':
if ( !$user->isAnon() ) {
// Remove all groups from the user.
$groups = $user->getGroups();
// Make sure that the stored var dump contains user groups, since we may
// need them if reverting this degroup via Special:AbuseFilter/revert
$vars->setVar( 'user_groups', $groups );
// Pull the groups from the VariableHolder, so that they will always be computed.
// This allow us to pull the groups from the VariableHolder to undo the degroup
// via Special:AbuseFilter/revert.
$groups = $vars->getVar( 'user_groups' );
if ( $groups->type !== AFPData::DARRAY ) {
// Somehow, the variable wasn't set
$groups = $user->getEffectiveGroups();
$vars->setVar( 'user_groups', $groups );
} else {
$groups = $groups->toNative();
}
foreach ( $groups as $group ) {
$user->removeGroup( $group );

View file

@ -42,27 +42,28 @@ class AbuseFilterVariableHolder {
/**
* Get a variable from the current object
*
* @param string $variable
* @param string $varName The variable name
* @return AFPData
*/
public function getVar( $variable ) {
$variable = strtolower( $variable );
if ( $this->mVarsVersion === 1 && in_array( $variable, AbuseFilter::getDeprecatedVariables() ) ) {
public function getVar( $varName ) {
$varName = strtolower( $varName );
if ( $this->mVarsVersion === 1 && in_array( $varName, AbuseFilter::getDeprecatedVariables() ) ) {
// Variables are stored with old names, but the parser has given us
// a new name. Translate it back.
$variable = array_search( $variable, AbuseFilter::getDeprecatedVariables() );
$varName = array_search( $varName, AbuseFilter::getDeprecatedVariables() );
}
if ( isset( $this->mVars[$variable] ) ) {
if ( $this->mVars[$variable] instanceof AFComputedVariable ) {
/** @suppress PhanUndeclaredMethod False positive */
$value = $this->mVars[$variable]->compute( $this );
$this->setVar( $variable, $value );
if ( isset( $this->mVars[$varName] ) ) {
/** @var $variable AFComputedVariable|AFPData */
$variable = $this->mVars[$varName];
if ( $variable instanceof AFComputedVariable ) {
$value = $variable->compute( $this );
$this->setVar( $varName, $value );
return $value;
} elseif ( $this->mVars[$variable] instanceof AFPData ) {
return $this->mVars[$variable];
} elseif ( $variable instanceof AFPData ) {
return $variable;
}
} elseif ( array_key_exists( $variable, AbuseFilter::$disabledVars ) ) {
wfWarn( "Disabled variable $variable requested. Please fix the filter as this will " .
} elseif ( array_key_exists( $varName, AbuseFilter::$disabledVars ) ) {
wfWarn( "Disabled variable $varName requested. Please fix the filter as this will " .
"be removed soon." );
}
return new AFPData();
@ -206,10 +207,12 @@ class AbuseFilterVariableHolder {
'revision-text-by-timestamp'
];
foreach ( $this->mVars as $name => $value ) {
if ( $value instanceof AFComputedVariable &&
in_array( $value->mMethod, $dbTypes )
) {
/** @var AFComputedVariable[] $missingVars */
$missingVars = array_filter( $this->mVars, function ( $el ) {
return ( $el instanceof AFComputedVariable );
} );
foreach ( $missingVars as $name => $value ) {
if ( in_array( $value->mMethod, $dbTypes ) ) {
$value = $value->compute( $this );
$this->setVar( $name, $value );
}

View file

@ -282,10 +282,7 @@ class AbuseFilterViewRevert extends AbuseFilterView {
case 'degroup':
// Pull the user's groups from the vars.
$oldGroups = $result['vars']->getVar( 'user_groups' )->toNative();
$oldGroups = array_diff(
$oldGroups,
array_intersect( $oldGroups, User::getImplicitGroups() )
);
$oldGroups = array_diff( $oldGroups, User::getImplicitGroups() );
$rows = [];
foreach ( $oldGroups as $group ) {