http://www.mediawiki.org/wiki/Special:Code/MediaWiki/89764
Revision: 89764
Author: catrope
Date: 2011-06-09 10:14:01 +0000 (Thu, 09 Jun 2011)
Log Message:
-----------
Fix r89277, there were tons of things wrong with it:
* Fix indexing on the stats table
* Use $foo[] instead of array_push()
* Actually insert the rows for problem articles into the DB
* Use the data we just inserted to build the cache entry, instead of
re-querying the data
** Re-querying is generally redundant
** Re-querying from the slave is guaranteed to fail in a replicated environment
with transactions
** Tweaked SpecialArticleFeedback::buildProblems() and buildHighsAndLows() to
accept arrays as well as database results
* Run potentially user-supplied timestamp through Database::timestamp()
* Declare getPage() to return a reference. The $page =& $pages->getPage()
assignment fails with a notice otherwise
* Remove $page['category'] stuff in problems view and make it more like the
highs/lows view instead. We simply don't have that data and it was spewing
notices
* $page['page'] is an ID, not a title
* Cache the results of Title::newFromID() in SpecialArticleFeedback (there's no
built-in caching in Title for this function) and bulk-request arrays of page IDs
* Fix variable name in getProblems(); copypaste mistake was causing it to
always return null when there was a cache entry
* Get the latest timestamp per statistic type, not globally. Queries will fail
for all but the last-populated statistic otherwise
* Kill getRecentLows(), unused, not even referenced in commented-out code
* Make formatNumber() and getCategories(), which claim to be static and are
called statically, actually static
Modified Paths:
--------------
trunk/extensions/ArticleFeedback/SpecialArticleFeedback.php
trunk/extensions/ArticleFeedback/populateAFStatistics.php
trunk/extensions/ArticleFeedback/sql/AddArticleFeedbackStatsTable.sql
Modified: trunk/extensions/ArticleFeedback/SpecialArticleFeedback.php
===================================================================
--- trunk/extensions/ArticleFeedback/SpecialArticleFeedback.php 2011-06-09
09:50:51 UTC (rev 89763)
+++ trunk/extensions/ArticleFeedback/SpecialArticleFeedback.php 2011-06-09
10:14:01 UTC (rev 89764)
@@ -113,12 +113,19 @@
*/
protected function renderDailyHighsAndLows( $pages, $caption ) {
global $wgOut, $wgUser;
+
+ // Pre-fill page ID cache
+ $ids = array();
+ foreach ( $pages as $page ) {
+ $ids[] = $page['page'];
+ }
+ self::populateTitleCache( $ids );
$rows = array();
if ( $pages ) {
foreach ( $pages as $page ) {
$row = array();
- $pageTitle = Title::newFromId( $page['page'] );
+ $pageTitle = self::getTitleFromID(
$page['page'] );
$row['page'] = $wgUser->getSkin()->link(
$pageTitle, $pageTitle->getPrefixedText() );
foreach ( $page['ratings'] as $id => $value ) {
$row[] = array(
@@ -194,32 +201,45 @@
protected function renderProblems() {
global $wgOut, $wgUser, $wgArticleFeedbackRatings;
+
+ $problems = $this->getProblems();
+
+ // Pre-fill page ID cache
+ $ids = array();
+ foreach ( $problems as $page ) {
+ $ids[] = $page['page'];
+ }
+ self::populateTitleCache( $ids );
+
$rows = array();
- foreach ( $this->getProblems() as $page ) {
+ foreach ( $problems as $page ) {
$row = array();
- $pageTitle = Title::newFromText( $page['page'] );
+ $pageTitle = self::getTitleFromID( $page['page'] );
$row['page'] = $wgUser->getSkin()->link( $pageTitle,
$pageTitle->getPrefixedText() );
- foreach ( $wgArticleFeedbackRatings as $category ) {
+ foreach ( $page['ratings'] as $id => $value ) {
$row[] = array(
- 'attr' => in_array( $category,
$page['categories'] )
- ? array(
- 'class' =>
'articleFeedback-table-column-bad',
- 'data-sort-value' => 0
- )
- : array(
- 'class' =>
'articleFeedback-table-column-good',
- 'data-sort-value' => 1
- ),
- 'html' => ' '
+ 'text' => $this->formatNumber( $value ),
+ 'attr' => array(
+ 'class' =>
'articleFeedback-table-column-rating ' .
+
'articleFeedback-table-column-score-' . round( $value )
+ )
);
}
+ $row[] = array(
+ 'text' => $this->formatNumber( $page['average']
),
+ 'attr' => array(
+ 'class' =>
'articleFeedback-table-column-average ' .
+
'articleFeedback-table-column-score-' . round( $page['average'] )
+ )
+ );
$rows[] = $row;
}
$this->renderTable(
wfMsg( 'articleFeedback-table-caption-recentlows' ),
array_merge(
array( wfMsg(
'articleFeedback-table-heading-page' ) ),
- self::getCategories()
+ self::getCategories(),
+ array( wfMsg(
'articleFeedback-table-heading-average' ) )
),
$rows,
'articleFeedback-table-recentlows'
@@ -235,14 +255,15 @@
$key = wfMemcKey( 'article_feedback_stats_problems' );
$cache = $wgMemc->get( $key );
if ( is_array( $cache )) {
- $highs_lows = $cache;
+ $problems = $cache;
} else {
$dbr = wfGetDB( DB_SLAVE );
+ $typeID = self::getStatsTypeId( 'problems' );
// first find the freshest timestamp
$row = $dbr->selectRow(
'article_feedback_stats',
array( 'afs_ts' ),
- "",
+ array( 'afs_stats_type_id' => $typeID ),
__METHOD__,
array( "ORDER BY" => "afs_ts DESC", "LIMIT" =>
1 )
);
@@ -262,7 +283,7 @@
),
array(
'afs_ts' => $row->afs_ts,
- 'afs_stats_type_id' =>
self::getStatsTypeId( 'problems' )
+ 'afs_stats_type_id' => $typeID
),
__METHOD__,
array( "ORDER BY" => "afs_orderable_data" )
@@ -292,11 +313,12 @@
$highs_lows = $cache;
} else {
$dbr = wfGetDB( DB_SLAVE );
+ $typeID = self::getStatsTypeId( 'highs_and_lows' );
// first find the freshest timestamp
$row = $dbr->selectRow(
'article_feedback_stats',
array( 'afs_ts' ),
- "",
+ array( 'afs_stats_type_id' => $typeID ),
__METHOD__,
array( "ORDER BY" => "afs_ts DESC", "LIMIT" =>
1 )
);
@@ -316,7 +338,7 @@
),
array(
'afs_ts' => $row->afs_ts,
- 'afs_stats_type_id' =>
self::getStatsTypeId( 'highs_and_lows' )
+ 'afs_stats_type_id' => $typeID,
),
__METHOD__,
array( "ORDER BY" => "afs_orderable_data" )
@@ -374,16 +396,19 @@
/**
* Build data store of highs/lows for use when rendering table
- * @param object Database result
+ * @param object Database result or array of rows
* @return array
*/
public static function buildHighsAndLows( $result ) {
$highs_lows = array();
foreach ( $result as $row ) {
+ if ( is_array( $row ) ) {
+ $row = (object)$row;
+ }
$highs_lows[] = array(
'page' => $row->afs_page_id,
'ratings' => FormatJson::decode( $row->afs_data
),
- 'average' => $row->afs_orderable_data
+ 'average' => $row->afs_orderable_data
);
}
return $highs_lows;
@@ -391,16 +416,19 @@
/**
* Build data store of problems for use when rendering table
- * @param object Database result
+ * @param object Database result or array of rows
* @return array
*/
public static function buildProblems( $result ) {
$problems = array();
foreach( $result as $row ) {
+ if ( is_array( $row ) ) {
+ $row = (object)$row;
+ }
$problems[] = array(
'page' => $row->afs_page_id,
'ratings' => FormatJson::decode( $row->afs_data
),
- 'average' => $row->afs_orderable_data
+ 'average' => $row->afs_orderable_data
);
}
return $problems;
@@ -463,54 +491,20 @@
);
}
- /**
- * Gets a list of articles which have recently recieved exceptionally
low ratings.
- *
- * - Based on any rating category
- * - Gets up to 100 most recently poorly rated articles
- * - Only consider articles which were rated lower than 3 for 7 out of
the last 10 ratings
- *
- * This data should be updated whenever article ratings are changed,
ideally through a hook
- */
- protected function getRecentLows() {
- return array(
- array(
- 'page' => 'Main Page',
- // List of rating IDs that qualify as recent
lows
- 'categories' => array( 1, 4 ),
- ),
- array(
- 'page' => 'Test Article 1',
- 'categories' => array( 1, 3 ),
- ),
- array(
- 'page' => 'Test Article 2',
- 'categories' => array( 2, 3 ),
- ),
- array(
- 'page' => 'Test Article 3',
- 'categories' => array( 3, 4 ),
- ),
- array(
- 'page' => 'Test Article 4',
- 'categories' => array( 1, 2 ),
- )
- );
- }
-
/* Protected Static Members */
protected static $categories;
+ protected static $titleCache = array();
/* Protected Static Methods */
- protected function formatNumber( $number ) {
+ protected static function formatNumber( $number ) {
global $wgLang;
return $wgLang->formatNum( number_format( $number, 2 ) );
}
- protected function getCategories() {
+ protected static function getCategories() {
global $wgArticleFeedbackRatings;
if ( !isset( self::$categories ) ) {
@@ -518,7 +512,8 @@
$res = $dbr->select(
'article_feedback_ratings',
array( 'aar_id', 'aar_rating' ),
- array( 'aar_id' => $wgArticleFeedbackRatings )
+ array( 'aar_id' => $wgArticleFeedbackRatings ),
+ __METHOD__
);
self::$categories = array();
foreach ( $res as $row ) {
@@ -527,4 +522,20 @@
}
return self::$categories;
}
+
+ protected static function getTitleFromID( $id ) {
+ // There's no caching in Title::newFromId() so we hack our own
around it
+ if ( !isset( self::$titleCache[$id] ) ) {
+ self::$titleCache[$id] = Title::newFromId( $id );
+ }
+ return self::$titleCache[$id];
+ }
+
+ protected static function populateTitleCache( $ids ) {
+ $toQuery = array_diff( $ids, array_keys( self::$titleCache ) );
+ $titles = Title::newFromIds( $toQuery );
+ foreach ( $titles as $title ) {
+ self::$titleCache[$title->getArticleID()] = $title;
+ }
+ }
}
Modified: trunk/extensions/ArticleFeedback/populateAFStatistics.php
===================================================================
--- trunk/extensions/ArticleFeedback/populateAFStatistics.php 2011-06-09
09:50:51 UTC (rev 89763)
+++ trunk/extensions/ArticleFeedback/populateAFStatistics.php 2011-06-09
10:14:01 UTC (rev 89764)
@@ -176,12 +176,12 @@
}
if ( $page->isProblematic() ) {
- array_push( $problems, $page->page_id );
+ $problems[] = $page->page_id;
}
}
// populate stats table with problem articles & associated data
- // fetch stats type id - add stat type if it's non-existant
+ // fetch stats type id - add stat type if it's non-existent
$stats_type_id = SpecialArticleFeedback::getStatsTypeId(
'problems' );
if ( !$stats_type_id ) {
$stats_type_id = $this->addStatType( 'problems' );
@@ -201,28 +201,31 @@
}
$this->output( "Done.\n" );
+ // Insert the problem rows into the database
+ $this->output( "Writing data to article_feedback_stats ...\n" );
+ $rowsInserted = 0;
+ // $rows is gonna be modified by array_splice(), so make a copy
for later use
+ $rowsCopy = $rows;
+ while( $rows ) {
+ $batch = array_splice( $rows, 0,
$this->insert_batch_size );
+ $this->dbw->insert(
+ 'article_feedback_stats',
+ $batch,
+ __METHOD__
+ );
+ $rowsInserted += count( $batch );
+ $this->syncDBs();
+ $this->output( "Inserted " . $rowsInserted . " rows\n"
);
+ }
+ $this->output( "Done.\n" );
+
// populate cache with current problem articles
- // loading data into cache
$this->output( "Caching latest problems (if cache present).\n"
);
- $key = wfMemcKey( 'article_feedback_stats_problems' );
- $result = $this->dbr->select(
- 'article_feedback_stats',
- array(
- 'afs_page_id',
- 'afs_orderable_data',
- 'afs_data'
- ),
- array(
- 'afs_ts' => $cur_ts,
- 'afs_stats_type_id' => $stats_type_id
- ),
- __METHOD__,
- array( "ORDER BY" => "afs_orderable_data" )
- );
// grab the article feedback special page so we can reuse the
data structure building code
// FIXME this logic should not be in the special page class
- $problems = SpecialArticleFeedback::buildProblems( $result );
+ $problems = SpecialArticleFeedback::buildProblems( $rowsCopy );
// stash the data structure in the cache
+ $key = wfMemcKey( 'article_feedback_stats_problems' );
$wgMemc->set( $key, $problems, 86400 );
$this->output( "Done.\n" );
}
@@ -300,6 +303,8 @@
// insert data to db
$this->output( "Writing data to article_feedback_stats ...\n" );
$rowsInserted = 0;
+ // $rows is gonna be modified by array_splice(), so make a copy
for later use
+ $rowsCopy = $rows;
while( $rows ) {
$batch = array_splice( $rows, 0,
$this->insert_batch_size );
$this->dbw->insert(
@@ -316,26 +321,12 @@
// loading data into cache
$this->output( "Caching latest highs/lows (if cache
present).\n" );
$key = wfMemcKey( 'article_feedback_stats_highs_lows' );
- $result = $this->dbr->select(
- 'article_feedback_stats',
- array(
- 'afs_page_id',
- 'afs_orderable_data',
- 'afs_data'
- ),
- array(
- 'afs_ts' => $cur_ts,
- 'afs_stats_type_id' => $stats_type_id
- ),
- __METHOD__,
- array( "ORDER BY" => "afs_orderable_data" )
- );
// grab the article feedback special page so we can reuse the
data structure building code
// FIXME this logic should not be in the special page class
- $highs_lows = SpecialArticleFeedback::buildHighsAndLows(
$result );
+ $highs_lows = SpecialArticleFeedback::buildHighsAndLows(
$rowsCopy );
// stash the data structure in the cache
$wgMemc->set( $key, $highs_lows, 86400 );
- $this->output( "Done\n" );
+ $this->output( "Done\n" );
}
/**
@@ -364,7 +355,7 @@
'aa_page_id',
'aa_rating_value',
),
- array( 'aa_timestamp >= ' . $this->dbr->addQuotes( $ts
)),
+ array( 'aa_timestamp >= ' . $this->dbr->addQuotes(
$this->dbr->timestamp( $ts ) ) ),
__METHOD__,
array()
);
@@ -390,6 +381,7 @@
// fetch the page from the page store referentially so
we can
// perform actions on it that will automagically be
saved in the
// object for easy access later
+
$page =& $pages->getPage( $row->aa_page_id );
// determine the unique hash for a given rating set
(page rev + user identifying info)
@@ -584,7 +576,7 @@
*/
public $pages = array();
- public function getPage( $page_id ) {
+ public function &getPage( $page_id ) {
if ( !isset( $this->pages[ $page_id ] )) {
$this->addPage( $page_id );
}
Modified: trunk/extensions/ArticleFeedback/sql/AddArticleFeedbackStatsTable.sql
===================================================================
--- trunk/extensions/ArticleFeedback/sql/AddArticleFeedbackStatsTable.sql
2011-06-09 09:50:51 UTC (rev 89763)
+++ trunk/extensions/ArticleFeedback/sql/AddArticleFeedbackStatsTable.sql
2011-06-09 10:14:01 UTC (rev 89764)
@@ -9,5 +9,5 @@
-- timestamp of insertion job
afs_ts binary(14) NOT NULL
) /*$wgDBTableOptions*/;
-CREATE UNIQUE INDEX /*i*/ afs_page_ts_type ON /*_*/article_feedback_stats(
afs_page_id, afs_ts, afs_stats_type_id );
-CREATE INDEX /*i*/ afs_ts_avg_overall ON /*_*/article_feedback_stats (afs_ts,
afs_orderable_data);
+CREATE UNIQUE INDEX /*i*/afs_type_ts_page ON
/*_*/article_feedback_stats(afs_stats_type_id, afs_ts, afs_page_id);
+CREATE INDEX /*i*/ afs_type_ts_orderable ON /*_*/article_feedback_stats
(afs_stats_type_id, afs_ts, afs_orderable_data);
_______________________________________________
MediaWiki-CVS mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-cvs