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

Reply via email to