Mattflaschen has uploaded a new change for review.
https://gerrit.wikimedia.org/r/97691
Change subject: New API for selecting a task; related refactoring and minor fm
......................................................................
New API for selecting a task; related refactoring and minor fm
* New API module GettingStartedGetPages for getting one or more pages for a
given task
* Used by special page
* New ArticleFilter for secondary criterion (other than Redis set) that can
allow or veto an article
** Broken out from the special page.
* Some renaming for clarity
* Small README fix about srand.
Bug: 55598
Bug: 55773
Change-Id: Icaf84e75d95bb06d108137221aa13e99003355e1
---
D CategoryRoulette.php
M GettingStarted.php
A PageFilter.php
M README
M SpecialGettingStarted.php
A api/ApiGettingStartedGetPages.php
6 files changed, 230 insertions(+), 148 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/GettingStarted
refs/changes/91/97691/1
diff --git a/CategoryRoulette.php b/CategoryRoulette.php
deleted file mode 100644
index e681c2a..0000000
--- a/CategoryRoulette.php
+++ /dev/null
@@ -1,75 +0,0 @@
-<?php
-
-namespace GettingStarted;
-
-use Title;
-
-/**
- * Helper class for retrieving random pages from a category.
- * See Redis requirements in README.
- *
- * Sample usage:
- * <code>
- * $category = Category::newFromName( 'All_articles_needing_copy_edit' );
- * $roulette = new CategoryRoulette( $category );
- * $pages = $roulette->getRandomArticles( 3 );
- * </code>
- *
- */
-class CategoryRoulette {
-
- const MAX_ATTEMPTS = 100;
-
- /** @var Category **/
- public $category = null;
-
- /**
- * Constructor.
- * @param $category Category.
- */
- public function __construct( $category ) {
- $this->category = $category;
- }
-
- /**
- * Get a random set of $numWanted unique pages in the
- * category. If fewer than $numWanted pages exist in category,
- * return as many as are available. It is up to the caller to decide
- * how to handle the deficit.
- *
- * @param $numWanted int Number of unique pages to get.
- * @return array Set of $numWanted unique pages (or however many
- * were available, if the desired count was not satisfiable).
- */
- public function getRandomArticles( $numWanted ) {
- $key = RedisCategorySync::makeCategoryKey( $this->category );
-
- $redis = RedisCategorySync::getClient();
- if ( !$redis ) {
- wfDebugLog( 'GettingStarted', "Unable to acquire redis
connection.\n" );
- return array();
- }
-
- $articleIDs = array();
- $attempts = 0;
- while ( count( $articleIDs ) < $numWanted ) {
- $attempts++;
- // Sanity check to prevent calling srand too many times
- if ( $attempts >= self::MAX_ATTEMPTS ) {
- wfDebugLog( 'GettingStarted', 'Returning early
after ' . self::MAX_ATTEMPTS . ".\n" );
- return Title::newFromIDs( $articleIDs );
- }
- try {
- $randomID = $redis->sRandMember( $key );
- if ( !in_array( $randomID, $articleIDs, true )
) {
- $articleIDs[] = $randomID;
- }
- } catch ( RedisException $e ) {
- wfDebugLog( 'GettingStarted', 'Redis exception:
' . $e->getMessage() . ". Returning early.\n" );
- return Title::newFromIDs( $articleIDs );
- }
- }
-
- return Title::newFromIDs( $articleIDs );
- }
-}
diff --git a/GettingStarted.php b/GettingStarted.php
index 2f6718d..7b60561 100644
--- a/GettingStarted.php
+++ b/GettingStarted.php
@@ -91,17 +91,22 @@
$wgAutoloadClasses += array(
'GettingStarted\SpecialGettingStarted' => __DIR__ .
'/SpecialGettingStarted.php',
- 'GettingStarted\Hooks' => __DIR__ . '/Hooks.php',
- 'GettingStarted\RedisCategorySync' => __DIR__ .
'/RedisCategorySync.php',
- 'GettingStarted\CategoryRoulette' => __DIR__ .
'/CategoryRoulette.php',
+ 'GettingStarted\Hooks' => __DIR__ . '/Hooks.php',
+ 'GettingStarted\RedisCategorySync' => __DIR__ .
'/RedisCategorySync.php',
+ 'GettingStarted\PageFilter' => __DIR__ . '/PageFilter.php',
+ 'GettingStarted\ApiGettingStartedGetPages' => __DIR__ .
'/api/ApiGettingStartedGetPages.php',
);
$wgExtensionMessagesFiles[ 'GettingStarted' ] = __DIR__ .
'/GettingStarted.i18n.php';
$wgExtensionMessagesFiles[ 'GettingStartedAlias' ] = __DIR__ .
'/GettingStarted.alias.php';
+// Special pages
$wgSpecialPages[ 'GettingStarted' ] = 'GettingStarted\SpecialGettingStarted';
$wgSpecialPageGroups[ 'GettingStarted' ] = 'users';
+// APIs
+$wgAPIModules['gettingstartedgetpages'] =
'GettingStarted\ApiGettingStartedGetPages';
+
// Modules
$gettingStartedModuleInfo = array(
diff --git a/PageFilter.php b/PageFilter.php
new file mode 100644
index 0000000..eaa3520
--- /dev/null
+++ b/PageFilter.php
@@ -0,0 +1,66 @@
+<?php
+
+namespace GettingStarted;
+
+use Title, User;
+
+/**
+ Approve or reject a given page for suitability with GettingStarted.
+*/
+class PageFilter {
+ const MAX_PAGE_LENGTH = 10000;
+
+ /** @var User */
+ protected $user;
+
+ /** @var Title */
+ protected $excludedTitle;
+
+ /**
+ * Constructor.
+ *
+ * @param User $user user object, for permissions checks
+ * @param Title $excludedTitle optional title to exclude, to avoid
consecutive duplicates
+ */
+ public function __construct( User $user, Title $excludedTitle = null ) {
+ $this->user = $user;
+ $this->excludedTitle = $excludedTitle;
+ }
+
+ protected function inExcludedCategories( Title $title ) {
+ global $wgGettingStartedExcludedCategories;
+
+ $articleID = $title->getArticleID();
+
+ $dbr = wfGetDB( DB_SLAVE );
+ foreach( $wgGettingStartedExcludedCategories as $cat ) {
+ $res = $dbr->selectRow( 'categorylinks', '1', array(
+ 'cl_from' => $articleID,
+ 'cl_to' => $cat,
+ ), __METHOD__ );
+
+ if ( $res !== false ) {
+ return true;
+ }
+ }
+
+ return false;
+ }
+
+ public function isAllowedPage( Title $title ) {
+ $length = $title->getLength();
+ $passesExclude = $this->excludedTitle === null ||
+ !$title->equals( $this->excludedTitle );
+ return $length > 0
+ && $length <= self::MAX_PAGE_LENGTH
+ // RedisCategorySync ignores category changes outside
NS_MAIN,
+ // but the API can still access pages outside.
+ && $title->inNamespace( NS_MAIN )
+ && $passesExclude
+ && $title->userCan( 'edit', $this->user )
+ && !$this->inExcludedCategories( $title );
+
+
+ }
+}
+
diff --git a/README b/README
index 87a5127..b9c31d2 100644
--- a/README
+++ b/README
@@ -34,10 +34,6 @@
* phpredis extension installed on the same machine, and Redis installed on some
machine that can be accessed.
-Since CategoryRoulette uses the two-argument version of
-SRANDMEMBER, it requires Redis 2.6+ for both the server and
-the PHP extension.
-
That means phpredis needs to be (re-)compiled against 2.6 if
upgrading from 2.4.
diff --git a/SpecialGettingStarted.php b/SpecialGettingStarted.php
index 02a0f8b..9f11c23 100644
--- a/SpecialGettingStarted.php
+++ b/SpecialGettingStarted.php
@@ -5,34 +5,8 @@
use Html, Linker, SpecialPage, Title;
class SpecialGettingStarted extends SpecialPage {
-
- const MAX_ARTICLE_LENGTH = 10000;
- const MAX_ATTEMPTS = 20; // Somewhat arbitrary for now.
-
public function __construct() {
parent::__construct( 'GettingStarted' );
- }
-
- /**
- * Determines if title is an allowed task, based on user and
configuration.
- *
- * @param Title $title title of article
- * @param User $user user to check
- * @param string|null $excludedPageName page name to be excluded, in in
getPrefixedDBkey() format,
- * or null for no exclusion
- */
- protected function isAllowedArticle( Title $title, \User $user,
$excludedPageName ) {
- $length = $title->getLength();
- $passesExclude = $excludedPageName === null ||
- $title->getPrefixedDBkey() !== $excludedPageName;
- return $length > 0
- && $length <= self::MAX_ARTICLE_LENGTH
- // RedisCategorySync ignores category changes outside
NS_MAIN,
- // but CategoryRoulette can still return pages outside.
- && $title->inNamespace( NS_MAIN )
- && $passesExclude
- && $title->userCan( 'edit', $user )
- && !$this->inExcludedCategories( $title );
}
/**
@@ -61,31 +35,36 @@
* constraints.
*
* @param string $taskName task name
- * @return bool whether they were redirected
+ * @return Title|false page to send user to, or false on failure
*/
public function chooseTitleForTask( $taskName ) {
- global $wgGettingStartedTasks;
+ $request = new \DerivativeRequest(
+ $this->getRequest(),
+ array(
+ 'action' => 'gettingstartedgetpages',
+ 'taskname' => $taskName,
+ 'excludedTitle' => $this->getRequest()->getVal(
'exclude' ),
+ 'count' => 1,
+ ),
+ false // GET request
+ );
- if ( !isset( $wgGettingStartedTasks[$taskName] ) ) {
+ $api = new \ApiMain(
+ $request,
+ false // no writes
+ );
+
+ try {
+ $api->execute();
+ $result = $api->getResult()->getData();
+ if ( isset(
$result['gettingstartedgetpages']['titles'][0] ) ) {
+ return Title::newFromText(
$result['gettingstartedgetpages']['titles'][0] );
+ } else {
+ return false;
+ }
+ } catch ( \UsageException $ex ) {
return false;
}
- $task = $wgGettingStartedTasks[$taskName];
- $taskCategory = \Category::newFromName( $task['category'] );
- $roulette = new CategoryRoulette( $taskCategory );
- $user = $this->getUser();
-
- $excludedPageName = $this->getRequest()->getVal( 'exclude' );
-
- for ( $i = 0; $i < self::MAX_ATTEMPTS; $i++ ) {
- $titles = $roulette->getRandomArticles( 1 );
- if ( count( $titles ) === 1
- && $this->isAllowedArticle( $titles[0], $user,
$excludedPageName )
- ) {
- return $titles[0];
- }
- }
-
- return false;
}
/**
@@ -175,26 +154,6 @@
'ext.gettingstarted',
'ext.gettingstarted.specialPage',
) );
- }
-
- public function inExcludedCategories( Title $title ) {
- global $wgGettingStartedExcludedCategories;
-
- $articleID = $title->getArticleID();
-
- $dbr = wfGetDB( DB_SLAVE );
- foreach( $wgGettingStartedExcludedCategories as $cat ) {
- $res = $dbr->selectRow( 'categorylinks', '1', array(
- 'cl_from' => $articleID,
- 'cl_to' => $cat,
- ), __METHOD__ );
-
- if ( $res !== false ) {
- return true;
- }
- }
-
- return false;
}
/**
diff --git a/api/ApiGettingStartedGetPages.php
b/api/ApiGettingStartedGetPages.php
new file mode 100644
index 0000000..fe6b87f
--- /dev/null
+++ b/api/ApiGettingStartedGetPages.php
@@ -0,0 +1,131 @@
+<?php
+
+namespace GettingStarted;
+
+use ApiBase, Category, Title;
+
+class ApiGettingStartedGetPages extends ApiBase {
+ const MAX_ATTEMPTS = 100;
+
+ public function __construct( $query, $moduleName ) {
+ parent::__construct( $query, $moduleName );
+ }
+
+ public function execute() {
+ global $wgGettingStartedTasks;
+
+ $result = $this->getResult();
+
+ // For PageFilter and specifically userCan( 'edit' )
+ $user = $this->getUser();
+
+ $taskName = $this->getParameter( 'taskname' );
+ $excludedTitle = Title::newFromText( $this->getParameter(
'excludedTitle' ) );
+ $count = $this->getParameter( 'count' );
+
+ if ( !isset( $wgGettingStartedTasks[$taskName] ) ) {
+ $this->dieUsage( 'Invalid value for "taskName"',
'gettingstarted-invalidtaskname' );
+ }
+
+ $category = Category::newFromName(
$wgGettingStartedTasks[$taskName]['category'] );
+ $pageFilter = new PageFilter( $user, $excludedTitle );
+
+ $titles = self::getRandomArticles( $count, $category,
$pageFilter );
+ $data = array(
+ 'titles' => array()
+ );
+ foreach ( $titles as $title ) {
+ $data['titles'][] = $title->getPrefixedText();
+ }
+ $result->setIndexedTagName( $data['titles'], 'title' );
+ $result->addValue( null, $this->getModuleName(), $data );
+ }
+
+ /**
+ * Get a random set of $numWanted unique pages in the
+ * category. If fewer than $numWanted pages exist in category,
+ * return as many as are available. It is up to the caller to decide
+ * how to handle the deficit.
+ *
+ * @param int $numWanted Number of unique pages to get.
+ * @param Category $category category to choose from
+ * @param PageFilter $pageFilter filter than can approve or reject a
page
+ * @return array Set of $numWanted unique Title objects (or however many
+ * were available, if the desired count was not satisfiable).
+ */
+ protected function getRandomArticles( $numWanted, Category $category,
PageFilter $pageFilter ) {
+ $key = RedisCategorySync::makeCategoryKey( $category );
+
+ $redis = RedisCategorySync::getClient();
+ if ( !$redis ) {
+ wfDebugLog( 'GettingStarted', "Unable to acquire redis
connection.\n" );
+ return array();
+ }
+
+ // Map article ID to Title. At the end, we simply return a
non-associative array of Titles.
+ // However, sRandMember can return the same ID more than once.
This allows us to easily remove
+ // avoid these duplicates with array_key_exists.
+ $titles = array();
+
+ $attempts = 0;
+ while ( count( $titles ) < $numWanted ) {
+ $attempts++;
+ // Sanity check to prevent calling srand or filter too
many times
+ if ( $attempts >= self::MAX_ATTEMPTS ) {
+ wfDebugLog( 'GettingStarted', 'Returning early
after ' . self::MAX_ATTEMPTS . ".\n" );
+ return array_values( $titles );
+ }
+ try {
+ $randomArticleID = $redis->sRandMember( $key );
+ if ( !array_key_exists( $randomArticleID,
$titles ) ) {
+ $title = Title::newFromID(
$randomArticleID );
+ if ( $pageFilter->isAllowedPage( $title
) ) {
+ $titles[$randomArticleID] =
$title;
+ }
+ }
+ } catch ( RedisException $e ) {
+ wfDebugLog( 'GettingStarted', 'Redis exception:
' . $e->getMessage() . ". Returning early.\n" );
+ return array_values( $titles );
+ }
+ }
+
+ return array_values( $titles );
+ }
+
+ public function getDescription() {
+ return array(
+ 'This API is for getting one or more GettingStarted
tasks'
+ );
+ }
+
+ public function getParamDescriptions() {
+ return array(
+ 'taskname' => 'Task name',
+ 'excludedTitle' => 'Full title of article',
+ 'count' => 'Requested count; will attempt to fetch this
exact number, but may fetch fewer if no more are found after multiple attempts'
+ );
+ }
+
+ public function getAllowedParams() {
+ return array(
+ 'taskname' => array(
+ ApiBase::PARAM_TYPE => 'string',
+ ApiBase::PARAM_REQUIRED => true,
+ ),
+ 'excludedTitle' => array(
+ ApiBase::PARAM_TYPE => 'string',
+ ApiBase::PARAM_REQUIRED => false,
+ ),
+ 'count' => array(
+ ApiBase::PARAM_TYPE => 'integer',
+ ApiBase::PARAM_REQUIRED => true,
+ ),
+ );
+ }
+
+ public function getExamples() {
+ return array(
+
'api.php?action=gettingstartedgetpages&taskname=copyedit&excludedTitle=Earth&count=1',
+ );
+ }
+}
--
To view, visit https://gerrit.wikimedia.org/r/97691
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icaf84e75d95bb06d108137221aa13e99003355e1
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/GettingStarted
Gerrit-Branch: master
Gerrit-Owner: Mattflaschen <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
