Page MenuHomePhabricator

Remove HitCounters from AbuseFilter and use hooks instead
Closed, ResolvedPublic

Description

includes/AbuseFilter.class.php
// AbuseFilter::$builderValues
// 'article_views' => 'article-views', # May not be enabled, defined in getBuilderValues()
includes/AbuseFilter.class.php
// AbuseFilter::getBuilderValues
global $wgDisableCounters;
if ( !$wgDisableCounters ) {
  $realValues['vars']['article_views'] = 'article-views';
}

Hooks::run( 'AbuseFilter-builder', array( &$realValues ) );
includes/AbuseFilter.class.php
// AbuseFilter::generateTitleVars
if ( method_exists( 'HitCounters\HitCounters', 'getCount' ) ) {
  $vars->setVar( $prefix . '_VIEWS', HitCounters\HitCounters::getCount( $title ) );
}

Hooks::run( 'AbuseFilter-generateTitleVars', [ $vars, $title, $prefix ] );
includes/AbuseFilter.class.php
// AbuseFilter::getStashKey
unset( $inputVars['_VIEWS'] );

Event Timeline

Can you clarify which hooks are supposed to be used instead of calling HitCounter's methods directly?

They are included in the snippets in the task description.

Change 375531 had a related patch set uploaded (by Huji; owner: Huji):
[mediawiki/extensions/AbuseFilter@master] Remove HitCounters from AbuseFilter and use hooks instead

https://gerrit.wikimedia.org/r/375531

Not sure if I still fully understand, but I created a patch to get the ball rolling.

The task is to hook HitCounters to AbuseFilter and remove the relevant code from it.

In other words, there will be one patch in HitCounters which will add handlers for hooks and one in AbuseFilter that will remove anything specific to HC.

Oh, now I understand! And now that you clarified it, it is obvious :) Will work on both patches then.

@matej_suchanek https://gerrit.wikimedia.org/r/#/c/375531 removes the unnecessary code from AbuseFilter. For HitCounters, where do you think is the best place to reference the hooks AbuseFilter-builder and AbuseFilter-generateTitleVars? I am thinking it should be inside ViewCountUpdate::doUpdate() but I might be wrong.

Change 381500 had a related patch set uploaded (by Huji; owner: Huji):
[mediawiki/extensions/HitCounters@master] Call AbuseFilter hooks for its page-views variable

https://gerrit.wikimedia.org/r/381500

@matej_suchanek can you review the two patches again please?

Huji triaged this task as Low priority.Oct 23 2017, 2:12 AM

The AF patch is mostly ready to go but it's polite to submit them together.

For HitCounters, see my review from September 30 (the long comment).

Fantastic. Thanks for the reminder. All issues should be resolved now.

I wonder if this is something that would be useful on Wikimedia sites? PageViewInfo could probably support the information, with some compromises (such as not having any data for the last ~2 days).

Change 381500 merged by jenkins-bot:
[mediawiki/extensions/HitCounters@master] Call AbuseFilter hooks for its page-views variable

https://gerrit.wikimedia.org/r/381500

Change 375531 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Remove HitCounters from AbuseFilter and use hooks instead

https://gerrit.wikimedia.org/r/375531

Huji removed a project: Patch-For-Review.