Sbisson has uploaded a new change for review. https://gerrit.wikimedia.org/r/323328
Change subject: [WIP] goodfaith filter ...................................................................... [WIP] goodfaith filter Bug: T149853 Depends-On: If47842f3d91c5999a7c5bf25666b967e9b30a6d7 Change-Id: I28c593f34145d566f803d96ff8220fd685c2f695 --- M extension.json M includes/Hooks.php A includes/Range.php M tests/phpunit/includes/HooksTest.php A tests/phpunit/includes/RangeTest.php 5 files changed, 463 insertions(+), 55 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/ORES refs/changes/28/323328/1 diff --git a/extension.json b/extension.json index 9a20026..dc9e8e8 100644 --- a/extension.json +++ b/extension.json @@ -13,6 +13,7 @@ "ORES\\Cache": "includes/Cache.php", "ORES\\Hooks": "includes/Hooks.php", "ORES\\FetchScoreJob": "includes/FetchScoreJob.php", + "ORES\\Range": "includes/Range.php", "ORES\\Scoring": "includes/Scoring.php", "ORES\\ApiQueryORES": "includes/ApiQueryORES.php", "ORES\\ApiHooks": "includes/ApiHooks.php", @@ -143,6 +144,11 @@ "soft": 0.70, "hard": 0.50 }, + "OresGoodfaithThresholds": { + "good" : { "min": 0.35, "max": 1 }, + "maybebad" : { "min": 0, "max": 0.65 }, + "bad" : { "min": 0, "max": 0.15 } + }, "OresEnabledNamespaces": {}, "OresWikiId": null, "OresRevisionsPerBatch": 50, diff --git a/includes/Hooks.php b/includes/Hooks.php index de75b4b..be873a3 100644 --- a/includes/Hooks.php +++ b/includes/Hooks.php @@ -93,25 +93,31 @@ ChangesListSpecialPage $clsp, &$filters ) { - if ( !self::oresEnabled( $clsp->getUser() ) || !self::isModelEnabled( 'damaging' ) ) { + if ( !self::oresEnabled( $clsp->getUser() ) ) { return true; } - switch ( $clsp->getName() ) { - case 'Watchlist': - $default = $clsp->getUser()->getOption( 'oresWatchlistHideNonDamaging' ); - break; - case 'Recentchanges': - $default = $clsp->getUser()->getOption( 'oresRCHideNonDamaging' ); - break; - default: - $default = false; + if ( self::isModelEnabled( 'damaging' ) ) { + switch ( $clsp->getName() ) { + case 'Watchlist': + $default = $clsp->getUser()->getOption( 'oresWatchlistHideNonDamaging' ); + break; + case 'Recentchanges': + $default = $clsp->getUser()->getOption( 'oresRCHideNonDamaging' ); + break; + default: + $default = false; + } + + $filters['hidenondamaging'] = [ + 'msg' => 'ores-damaging-filter', + 'default' => $default, + ]; } - $filters['hidenondamaging'] = [ - 'msg' => 'ores-damaging-filter', - 'default' => $default, - ]; + if ( self::isModelEnabled( 'goodfaith' ) ) { + $filters['goodfaith'] = [ 'msg' => false, 'default' => 'all' ]; + } return true; } @@ -138,24 +144,27 @@ } if ( self::isModelEnabled( 'damaging' ) ) { - $hidenondamaging = $opts->getValue( 'hidenondamaging' ); - self::manipulateQuery( - 'damaging', - $wgUser, - 'rc_this_oldid', - $hidenondamaging, + self::hideDamagingFilter( $tables, $fields, $conds, $query_options, - $join_conds + $join_conds, + $opts->getValue( 'hidenondamaging' ), + 'rc_this_oldid', + $wgUser ); + } - if ( $hidenondamaging ) { - $conds['rc_patrolled'] = 0; - // Performance hack: add STRAIGHT_JOIN (146111) - $query_options[] = 'STRAIGHT_JOIN'; - } + if ( self::isModelEnabled( 'goodfaith' ) ) { + self::goodfaithFilter( + $tables, + $fields, + $conds, + $query_options, + $join_conds, + $opts + ); } return true; @@ -260,18 +269,19 @@ if ( !self::oresEnabled( $pager->getUser() ) ) { return true; } - $request = $pager->getContext()->getRequest(); - self::manipulateQuery( - 'damaging', - $pager->getUser(), - 'rev_id', - $request->getVal( 'hidenondamaging' ), - $query['tables'], - $query['fields'], - $query['conds'], - $query['options'], - $query['join_conds'] - ); + if ( self::isModelEnabled( 'damaging' ) ) { + $request = $pager->getContext()->getRequest(); + self::hideDamagingFilter( + $query['tables'], + $query['fields'], + $query['conds'], + $query['options'], + $query['join_conds'], + $request->getVal( 'hidenondamaging' ), + 'rev_id', + $pager->getUser() + ); + } return true; } @@ -547,29 +557,20 @@ $out->setProperty( 'oresData', $data ); } - private static function manipulateQuery( + private static function joinWithOresTables( $type, - User $user, - $revid_field, $filter, + $revIdField, array &$tables, array &$fields, array &$conds, array &$query_options, array &$join_conds ) { - if ( !self::isModelEnabled( $type ) ) { - return; - } - - $dbr = \wfGetDB( DB_REPLICA ); - $threshold = self::getThreshold( $type, $user ); $tables["ores_${type}_mdl"] = 'ores_model'; $tables["ores_${type}_cls"] = 'ores_classification'; $fields["ores_${type}_score"] = "ores_${type}_cls.oresc_probability"; - // Add user-based threshold - $fields["ores_${type}_threshold"] = $dbr->addQuotes( $threshold ); $join_conds["ores_${type}_mdl"] = [ 'LEFT JOIN', [ "ores_${type}_mdl.oresm_is_current" => 1, @@ -577,17 +578,130 @@ ] ]; $join_conds["ores_${type}_cls"] = [ 'LEFT JOIN', [ "ores_${type}_cls.oresc_model = ores_${type}_mdl.oresm_id", - "$revid_field = ores_${type}_cls.oresc_rev", + "$revIdField = ores_${type}_cls.oresc_rev", "ores_${type}_cls.oresc_class" => 1 ] ]; if ( $filter ) { - // Filter out non-damaging edits. - $conds[] = "ores_${type}_cls.oresc_probability > " . $dbr->addQuotes( $threshold ); // Performance hack: override the LEFT JOINs to be INNER JOINs (T137895) $join_conds["ores_${type}_mdl"][0] = 'INNER JOIN'; $join_conds["ores_${type}_cls"][0] = 'INNER JOIN'; } } + private static function hideDamagingFilter( + array &$tables, + array &$fields, + array &$conds, + array &$query_options, + array &$join_conds, + $hidenondamaging, + $revIdField, + $user + ) { + self::joinWithOresTables( + 'damaging', + $hidenondamaging, + $revIdField, + $tables, + $fields, + $conds, + $query_options, + $join_conds + ); + + $dbr = \wfGetDB( DB_REPLICA ); + // Add user-based threshold + $threshold = self::getThreshold( 'damaging', $user ); + $fields["ores_damaging_threshold"] = $dbr->addQuotes( $threshold ); + + if ( $hidenondamaging ) { + // Filter out non-damaging edits. + $conds[] = "ores_damaging_cls.oresc_probability > " . $dbr->addQuotes( $threshold ); + + $conds['rc_patrolled'] = 0; + + // Performance hack: add STRAIGHT_JOIN (146111) + $query_options[] = 'STRAIGHT_JOIN'; + } + } + + private static function goodfaithFilter( + &$tables, + &$fields, + &$conds, + &$query_options, + &$join_conds, + $opts + ) { + global $wgOresGoodfaithThresholds; + + $goodfaithLevels = explode( ',', strtolower( $opts->getValue( 'goodfaith' ) ) ); + $goodfaithLevels = array_intersect( $goodfaithLevels, [ 'good', 'bad', 'maybebad' ] ); + + self::joinWithOresTables( + 'goodfaith', + count( $goodfaithLevels ), + 'rc_this_oldid', + $tables, + $fields, + $conds, + $query_options, + $join_conds + ); + + if ( $goodfaithLevels ) { + $ranges = [ ]; + foreach ( $goodfaithLevels as $level ) { + $range = new Range( + $wgOresGoodfaithThresholds[$level]['min'], + $wgOresGoodfaithThresholds[$level]['max'] + ); + + $overlap = reset( array_filter( + $ranges, + function ( Range $r ) use ( $range ) { + return $r->overlaps( $range ); + } + ) ); + if ( $overlap ) { + $overlap->combineWith( $range ); + } else { + $ranges[] = $range; + } + } + + $betweenConditions = array_map( + function ( Range $range ) { + $min = $range->getMin(); + $max = $range->getMax(); + return "ores_goodfaith_cls.oresc_probability BETWEEN $min AND $max"; + }, + $ranges + ); + + $conds[] = \wfGetDB( DB_REPLICA )->makeList( $betweenConditions, \IDatabase::LIST_OR ); + } + +// if ( $goodfaithLevels ) { +// $min = null; +// $max = null; +// foreach ( $goodfaithLevels as $level ) { +// if ( $min === null ) { +// $min = $wgOresGoodfaithThresholds[$level]['min']; +// } else { +// $min = min( $wgOresGoodfaithThresholds[$level]['min'], $min ); +// } +// +// if ( $max === null ) { +// $max = $wgOresGoodfaithThresholds[$level]['max']; +// } else { +// $max = max( $wgOresGoodfaithThresholds[$level]['max'], $max ); +// } +// } +// +// $conds[] = "ores_goodfaith_cls.oresc_probability BETWEEN $min AND $max"; +// } + } + } diff --git a/includes/Range.php b/includes/Range.php new file mode 100644 index 0000000..e647432 --- /dev/null +++ b/includes/Range.php @@ -0,0 +1,69 @@ +<?php + +namespace ORES; + +/** + * Represents a range defined by two values: min and max + * + * Class Range + * @package ORES + */ +class Range { + + /** + * @var float + */ + protected $min; + + /** + * @var float + */ + protected $max; + + /** + * Range constructor. + * @param float $min + * @param float $max + */ + public function __construct( $min, $max ) { + if ( $min > $max ) { + throw new \DomainException( '$min must be smaller or equal to $max' ); + } + $this->min = $min; + $this->max = $max; + } + + /** + * @return float + */ + public function getMin() { + return $this->min; + } + + /** + * @return float + */ + public function getMax() { + return $this->max; + } + + /** + * Check if the current range overlaps with the given range. + * + * @param Range $other + * @return bool + */ + public function overlaps( Range $other ) { + return max( $this->getMin(), $other->getMin() ) <= min( $this->getMax(), $other->getMax() ); + } + + /** + * Expands the current range to include the given range. + * + * @param Range $other + */ + public function combineWith( Range $other ) { + $this->min = min( $this->getMin(), $other->getMin() ); + $this->max = max( $this->getMax(), $other->getMax() ); + } +} \ No newline at end of file diff --git a/tests/phpunit/includes/HooksTest.php b/tests/phpunit/includes/HooksTest.php index 9834c1f..204362a 100644 --- a/tests/phpunit/includes/HooksTest.php +++ b/tests/phpunit/includes/HooksTest.php @@ -73,17 +73,25 @@ ORES\Hooks::onChangesListSpecialPageFilters( $clsp, $filters ); $expected = [ - 'hidenondamaging' => [ 'msg' => 'ores-damaging-filter', 'default' => false ] + 'hidenondamaging' => [ 'msg' => 'ores-damaging-filter', 'default' => false ], + 'goodfaith' => [ 'msg' => false, 'default' => 'all' ], ]; $this->assertSame( $expected, $filters ); } - public function testOnChangesListSpecialPageQuery() { - $this->setMwGlobals( 'wgUser', $this->user ); + public function testOnChangesListSpecialPageQuery_hidenondamaging() { + $this->setMwGlobals( [ + 'wgUser' => $this->user, + 'wgOresModels' => [ + 'damaging' => true, + 'goodfaith' => false, + ] + ] ); $opts = new FormOptions(); $opts->add( 'hidenondamaging', true, 2 ); + $opts->add( 'goodfaith', 'all' ); $tables = []; $fields = []; @@ -136,6 +144,147 @@ $this->assertSame( $expected['join_conds'], $join_conds ); } + public function onChangesListSpecialPageQuery_goodfaith_provider() { + return [ + [ 'good', 0.35, 1 ], + [ 'maybebad', 0, 0.65 ], + [ 'bad', 0, 0.15 ], + [ 'good,maybebad', 0, 1 ], + [ 'maybebad,bad', 0, 0.65 ], + [ 'good,maybebad,bad', 0, 1 ], +// [ 'good,bad', 0, 0.15, 0.35, 1 ], // @todo: multi-range + ]; + } + + /** + * @dataProvider onChangesListSpecialPageQuery_goodfaith_provider + */ + public function testOnChangesListSpecialPageQuery_goodfaith ( + $goodfaithValue, + $expectedMin, + $expectedMax + ) { + $this->setMwGlobals( [ + 'wgUser' => $this->user, + 'wgOresModels' => [ + 'damaging' => false, + 'goodfaith' => true, + ] + ] ); + + $opts = new FormOptions(); + + $opts->add( 'hidenondamaging', false ); + $opts->add( 'goodfaith', $goodfaithValue ); + + $tables = []; + $fields = []; + $conds = []; + $query_options = []; + $join_conds = []; + ORES\Hooks::onChangesListSpecialPageQuery( + '', + $tables, + $fields, + $conds, + $query_options, + $join_conds, + $opts + ); + $expected = [ + 'tables' => [ + 'ores_goodfaith_mdl' => 'ores_model', + 'ores_goodfaith_cls' => 'ores_classification' + ], + 'fields' => [ + 'ores_goodfaith_score' => 'ores_goodfaith_cls.oresc_probability', + ], + 'conds' => [ + "(ores_goodfaith_cls.oresc_probability BETWEEN $expectedMin AND $expectedMax)" + ], + 'join_conds' => [ + 'ores_goodfaith_mdl' => [ 'INNER JOIN', + [ + 'ores_goodfaith_mdl.oresm_is_current' => 1, + 'ores_goodfaith_mdl.oresm_name' => 'goodfaith' + ] + ], + 'ores_goodfaith_cls' => [ 'INNER JOIN', + [ + 'ores_goodfaith_cls.oresc_model = ores_goodfaith_mdl.oresm_id', + 'rc_this_oldid = ores_goodfaith_cls.oresc_rev', + 'ores_goodfaith_cls.oresc_class' => 1 + ] + ] + ], + ]; + $this->assertSame( $expected['tables'], $tables ); + $this->assertSame( $expected['fields'], $fields ); + $this->assertSame( $expected['conds'], $conds ); + $this->assertSame( $expected['join_conds'], $join_conds ); + } + + public function testOnChangesListSpecialPageQuery_goodfaith_goodbad () { + $this->setMwGlobals( [ + 'wgUser' => $this->user, + 'wgOresModels' => [ + 'damaging' => false, + 'goodfaith' => true, + ] + ] ); + + $opts = new FormOptions(); + + $opts->add( 'hidenondamaging', false ); + $opts->add( 'goodfaith', 'good,bad' ); + + $tables = []; + $fields = []; + $conds = []; + $query_options = []; + $join_conds = []; + ORES\Hooks::onChangesListSpecialPageQuery( + '', + $tables, + $fields, + $conds, + $query_options, + $join_conds, + $opts + ); + $expected = [ + 'tables' => [ + 'ores_goodfaith_mdl' => 'ores_model', + 'ores_goodfaith_cls' => 'ores_classification' + ], + 'fields' => [ + 'ores_goodfaith_score' => 'ores_goodfaith_cls.oresc_probability', + ], + 'conds' => [ + "(ores_goodfaith_cls.oresc_probability BETWEEN 0.35 AND 1) OR (ores_goodfaith_cls.oresc_probability BETWEEN 0 AND 0.15)", + ], + 'join_conds' => [ + 'ores_goodfaith_mdl' => [ 'INNER JOIN', + [ + 'ores_goodfaith_mdl.oresm_is_current' => 1, + 'ores_goodfaith_mdl.oresm_name' => 'goodfaith' + ] + ], + 'ores_goodfaith_cls' => [ 'INNER JOIN', + [ + 'ores_goodfaith_cls.oresc_model = ores_goodfaith_mdl.oresm_id', + 'rc_this_oldid = ores_goodfaith_cls.oresc_rev', + 'ores_goodfaith_cls.oresc_class' => 1 + ] + ] + ], + ]; + $this->assertSame( $expected['tables'], $tables ); + $this->assertSame( $expected['fields'], $fields ); + $this->assertSame( $expected['conds'], $conds ); + $this->assertSame( $expected['join_conds'], $join_conds ); + } + public function testOnEnhancedChangesListModifyLineDataDamaging() { $row = new \stdClass(); $row->ores_damaging_threshold = 0.2; diff --git a/tests/phpunit/includes/RangeTest.php b/tests/phpunit/includes/RangeTest.php new file mode 100644 index 0000000..40c1f30 --- /dev/null +++ b/tests/phpunit/includes/RangeTest.php @@ -0,0 +1,70 @@ +<?php + +namespace ORES\Tests; + +use ORES; +use ORES\Range; +use PHPUnit_Framework_TestCase; + +/** + * @group ORES + * @covers ORES\Range + */ +class OresRangeTest extends PHPUnit_Framework_TestCase { + public function testConstructor() { + $r = new Range( 1, 2 ); + + $this->assertEquals( 1, $r->getMin() ); + $this->assertEquals( 2, $r->getMax() ); + } + + /** + * @expectedException \DomainException + */ + public function testConstructorEx() { + $r = new Range( 3, 2 ); + } + + public function overlapsProvider() { + return [ + [ [ 1, 3 ], [ 2, 4 ], true ], + [ [ 1, 2 ], [ 2, 4 ], true ], + [ [ 1, 5 ], [ 2, 3 ], true ], + [ [ 1, 1 ], [ 1, 3 ], true ], + [ [ 0.1, 0.35 ], [ 0.29, 1 ], true ], + [ [ 0.1, 0.35 ], [ 0.56, 1 ], false ], + [ [ 1, 2 ], [ 3, 4 ], false ], + ]; + } + + /** + * @dataProvider overlapsProvider + */ + public function testOverlaps( $r1, $r2, $expectedOverlap ) { + $r1 = new Range( $r1[0], $r1[1] ); + $r2 = new Range( $r2[0], $r2[1] ); + $this->assertEquals( $expectedOverlap, $r1->overlaps( $r2 ) ); + $this->assertEquals( $expectedOverlap, $r2->overlaps( $r1 ) ); + } + + public function combineWithProvider() { + return [ + [ [ 1, 2 ], [ 3, 4 ], [ 1, 4 ] ], + [ [ 4, 44 ], [ 11, 88 ], [ 4, 88 ] ], + [ [ 1, 2 ], [ 4, 5 ], [ 1, 5 ] ], + ]; + } + + /** + * @dataProvider combineWithProvider + */ + public function testCombineWith( $r1, $r2, $expectedRange ) { + $r1 = new Range( $r1[0], $r1[1] ); + $r2 = new Range( $r2[0], $r2[1] ); + + $r1->combineWith( $r2 ); + + $this->assertEquals( $expectedRange[0], $r1->getMin() ); + $this->assertEquals( $expectedRange[1], $r1->getMax() ); + } +} -- To view, visit https://gerrit.wikimedia.org/r/323328 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I28c593f34145d566f803d96ff8220fd685c2f695 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/ORES Gerrit-Branch: master Gerrit-Owner: Sbisson <sbis...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits