r47701 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r47700‎ \| r47701 \| r47702 >
Date:	10:53, 23 February 2009
Author:	aaron
Status:	deferred
Tags:
Comment:	* Added htmlspecialchars to some places * Added purge page links * Fixed graph purging * Moved getVoteAggregates() and added docs * Function access tweaks * Avoid $wgTitle usage * Process cache value of time() for consistency
Modified paths:	/trunk/extensions/FlaggedRevs/FlaggedRevs.class.php (modified) (history) /trunk/extensions/FlaggedRevs/language/RatingHistory.i18n.php (modified) (history) /trunk/extensions/FlaggedRevs/specialpages/RatingHistory_body.php (modified) (history) /trunk/extensions/FlaggedRevs/specialpages/ReaderFeedback_body.php (modified) (history)

Diff [purge]

Index: trunk/extensions/FlaggedRevs/FlaggedRevs.class.php
—	—	@@ -1033,6 +1033,118 @@
1034	1034	return $dims;
1035	1035	}
1036	1036
	1037	+ /**
	1038	+ * Get a table of the vote totals for a page
	1039	+ * @param Title $page
	1040	+ * @param int $period, number of days back
	1041	+ * @param array $add, optional vote to add on (used to visually avoid lag)
	1042	+ * @param string $cache, optional param to not use cache
	1043	+ * @returns string HTML table
	1044	+ */
	1045	+ public static function getVoteAggregates( $page, $period, $add = array(), $cache = 'useCache' ) {
	1046	+ global $wgLang, $wgMemc;
	1047	+ if( $period > 93 ) {
	1048	+ return ''; // too big
	1049	+ }
	1050	+ $votes = null;
	1051	+ $now = time();
	1052	+ $key = wfMemcKey( 'flaggedrevs', 'ratingtally', $page->getArticleId(), $period );
	1053	+ // Check cache
	1054	+ if( $cache == 'useCache' ) {
	1055	+ $set = $wgMemc->get($key);
	1056	+ // Cutoff is at the 24 hour mark due to the way the aggregate
	1057	+ // schema groups ratings by date for graphs.
	1058	+ $cache_cutoff = $now - ($now % 86400);
	1059	+ if( is_array($set) && count($set) == 2 ) {
	1060	+ list($val,$time) = $set;
	1061	+ $touched = wfTimestamp( TS_UNIX, RatingHistory::getTouched($page) );
	1062	+ if( $time > $cache_cutoff && $time > $touched ) {
	1063	+ $votes = $val;
	1064	+ }
	1065	+ }
	1066	+ }
	1067	+ // Do query, cache miss
	1068	+ if( !isset($votes) ) {
	1069	+ // Set cutoff time for period
	1070	+ $dbr = wfGetDB( DB_SLAVE );
	1071	+ $cutoff_unixtime = $now - ($period * 24 * 3600);
	1072	+ // Use integral number of days to be consistent with graphs
	1073	+ $cutoff_unixtime = $cutoff_unixtime - ($cutoff_unixtime % 86400);
	1074	+ $cutoff = $dbr->addQuotes( wfTimestamp( TS_MW, $cutoff_unixtime ) );
	1075	+ // Get the first revision possibly voted on in the range
	1076	+ $firstRevTS = $dbr->selectField( 'revision',
	1077	+ 'rev_timestamp',
	1078	+ array( 'rev_page' => $page->getArticleId(), "rev_timestamp <= $cutoff" ),
	1079	+ __METHOD__,
	1080	+ array( 'ORDER BY' => 'rev_timestamp DESC' )
	1081	+ );
	1082	+ // Find average, median, deviation...
	1083	+ $res = $dbr->select( array( 'revision', 'reader_feedback' ),
	1084	+ array( 'rfb_ratings' ),
	1085	+ array( 'rev_page' => $page->getArticleId(),
	1086	+ "rev_id = rfb_rev_id",
	1087	+ "rfb_timestamp >= $cutoff",
	1088	+ // Trigger INDEX usage
	1089	+ "rev_timestamp >= ".$dbr->addQuotes($firstRevTS) ),
	1090	+ __METHOD__,
	1091	+ array( 'USE INDEX' => array('revision' => 'page_timestamp') )
	1092	+ );
	1093	+ $votes = array();
	1094	+ foreach( FlaggedRevs::getFeedbackTags() as $tag => $w ) {
	1095	+ $votes[$tag] = array( 0 => 0, 1 => 0, 2 => 0, 3 => 0, 4 => 0 );
	1096	+ }
	1097	+ // Read votes and tally the numbers
	1098	+ while( $row = $dbr->fetchObject($res) ) {
	1099	+ $dims = FlaggedRevs::expandRatings( $row->rfb_ratings );
	1100	+ foreach( $dims as $tag => $val ) {
	1101	+ if( isset($votes[$tag]) && isset($votes[$tag][$val]) ) {
	1102	+ $votes[$tag][$val]++;
	1103	+ }
	1104	+ }
	1105	+ }
	1106	+ // Tack on $add for display (used to avoid cache/lag)
	1107	+ foreach( $add as $tag => $val ) {
	1108	+ if( isset($votes[$tag]) && isset($votes[$tag][$val]) ) {
	1109	+ $votes[$tag][$val]++;
	1110	+ }
	1111	+ }
	1112	+ $wgMemc->set( $key, array( $votes, $now ), 24*3600 );
	1113	+ }
	1114	+ // Output multi-column list
	1115	+ $html = "<table class='fr_reader_feedback_table' cellspacing='0'><tr>";
	1116	+ foreach( FlaggedRevs::getFeedbackTags() as $tag => $w ) {
	1117	+ // Get tag average...
	1118	+ $dist = isset($votes[$tag]) ? $votes[$tag] : array();
	1119	+ $count = array_sum($dist);
	1120	+ if( $count ) {
	1121	+ $ave = ($dist[0] + 2$dist[1] + 3$dist[2] + 4$dist[3] + 5$dist[4])/$count;
	1122	+ $ave = round($ave,1);
	1123	+ } else {
	1124	+ $ave = '-'; // DIV by zero
	1125	+ }
	1126	+ $html .= '<td align="center"><b>'.wfMsgHtml("readerfeedback-$tag").'</b>  '.
	1127	+ '<sup>('.wfMsgHtml('ratinghistory-ave',$wgLang->formatNum($ave)).')</sup></td>';
	1128	+ }
	1129	+ $html .= '</tr><tr>';
	1130	+ foreach( $votes as $tag => $dist ) {
	1131	+ $html .= '<td><table>';
	1132	+ $html .= '<tr><th align="left">'.wfMsgHtml('ratinghistory-table-rating').'</th>';
	1133	+ for( $i = 1; $i <= 5; $i++ ) {
	1134	+ $html .= "<td align='center' class='fr-rating-option-".($i-1)."'>$i</td>";
	1135	+ }
	1136	+ $html .= '</tr><tr>';
	1137	+ $html .= '<th align="left">'.wfMsgHtml("ratinghistory-table-votes").'</th>';
	1138	+ $html .= '<td align="center">'.$dist[0].'</td>';
	1139	+ $html .= '<td align="center">'.$dist[1].'</td>';
	1140	+ $html .= '<td align="center">'.$dist[2].'</td>';
	1141	+ $html .= '<td align="center">'.$dist[3].'</td>';
	1142	+ $html .= '<td align="center">'.$dist[4].'</td>';
	1143	+ $html .= "</tr></table></td>\n";
	1144	+ }
	1145	+ $html .= '</tr></table>';
	1146	+ return $html;
	1147	+ }
	1148	+
1037	1149	################# Auto-review function #################
1038	1150
1039	1151	/**
Index: trunk/extensions/FlaggedRevs/language/RatingHistory.i18n.php
—	—	@@ -20,6 +20,7 @@
21	21	'ratinghistory-3years' => 'last 3 years',
22	22	'ratinghistory-ave' => 'Avg: $1',
23	23	'ratinghistory-chart' => 'Reader ratings over time',
	24	+ 'ratinghistory-purge' => 'purge cache',
24	25	'ratinghistory-table' => 'Overview of reader ratings',
25	26	'ratinghistory-users' => 'Users who gave ratings',
26	27	'ratinghistory-graph' => '$2 of "$3" ($1 {{PLURAL:$1\|review\|reviews}})',
Index: trunk/extensions/FlaggedRevs/specialpages/RatingHistory_body.php
—	—	@@ -52,6 +52,8 @@
53	53	}
54	54	$this->period = $period;
55	55	$this->dScale = 20;
	56	+
	57	+ $this->now = time(); // one time for whole request
56	58
57	59	$this->showForm();
58	60	$this->showTable();
—	—	@@ -62,6 +64,7 @@
63	65	return; // Client cache fresh and headers sent, nothing more to do
64	66	} else {
65	67	$wgOut->enableClientCache( false ); // don't show stale graphs
	68	+ if( $this->doPurge ) $this->purgePage();
66	69	}
67	70	$this->showGraphs();
68	71	}
—	—	@@ -69,19 +72,21 @@
70	73	protected function showTable() {
71	74	global $wgOut;
72	75	# Show latest month of results
73		~~- $html = self::getVoteAggregates( $this->page, $this->period );~~
	76	+ $html = FlaggedRevs::getVoteAggregates( $this->page, $this->period, array(),
	77	+ $this->doPurge ? 'skipCache' : 'useCache'
	78	+ );
74	79	if( $html ) {
75	80	$wgOut->addHTML( '<h2>'.wfMsgHtml('ratinghistory-table')."</h2>\n".$html );
76	81	}
77	82	}
78	83
79	84	protected function showForm() {
80		~~- global $wgOut, $wgTitle, $wgScript;~~
	85	+ global $wgOut, $wgScript;
81	86	$form = Xml::openElement( 'form', array( 'name' => 'reviewedpages', 'action' => $wgScript, 'method' => 'get' ) );
82	87	$form .= "<fieldset>";
83	88	$form .= "<legend>".wfMsgExt( 'ratinghistory-leg',array('parseinline'),
84	89	$this->page->getPrefixedText() )."</legend>\n";
85		~~- $form .= Xml::hidden( 'title', $wgTitle->getPrefixedDBKey() );~~
	90	+ $form .= Xml::hidden( 'title', $this->getTitle()->getPrefixedDBKey() );
86	91	$form .= Xml::hidden( 'target', $this->page->getPrefixedDBKey() );
87	92	$form .= $this->getPeriodMenu( $this->period );
88	93	$form .= " ".Xml::submitButton( wfMsg( 'go' ) );
—	—	@@ -143,7 +148,7 @@
144	149	$viewLink = "";
145	150	if( $sExt === 'svg' ) {
146	151	$svgUrl = $this->getUrlPath( $tag, 'svg' );
147		~~- $viewLink = " <small>[<a href='".$svgUrl."'>".~~
	152	+ $viewLink = " <small>[<a href='".htmlspecialchars($svgUrl)."'>".
148	153	wfMsgHtml("readerfeedback-svg")."</a>]</small>";
149	154	}
150	155	$html .= "<h3>" . wfMsgHtml("readerfeedback-$tag") . "$viewLink</h3>\n" .
—	—	@@ -166,7 +171,12 @@
167	172	}
168	173	}
169	174	// Add header
170		~~- $wgOut->addHTML( '<h2>' . wfMsgHtml('ratinghistory-chart') . '</h2>' );~~
	175	+ $purgeUrl = $this->getTitle()->getFullUrl( 'target='.$this->page->getPrefixedUrl().
	176	+ "&period={$this->period}&action=purge" );
	177	+ $purgeLink = " <small>[<a href='".htmlspecialchars($purgeUrl)."'>".
	178	+ wfMsgHtml('ratinghistory-purge')."</a>]</small>";
	179	+ // Add header for all the graphs below
	180	+ $wgOut->addHTML( '<h2>' . wfMsgHtml('ratinghistory-chart') . "$purgeLink</h2>\n" );
171	181	if( $data ) {
172	182	// Add legend as needed
173	183	$wgOut->addWikiText( wfMsg('ratinghistory-legend',$this->dScale) );
—	—	@@ -472,7 +482,7 @@
473	483	protected function doQuery( $tag ) {
474	484	// Set cutoff time for period
475	485	$dbr = wfGetDB( DB_SLAVE );
476		~~- $cutoff_unixtime = time() - ($this->period * 24 * 3600);~~
	486	+ $cutoff_unixtime = $this->now - ($this->period * 24 * 3600);
477	487	$cutoff_unixtime = $cutoff_unixtime - ($cutoff_unixtime % 86400);
478	488	$cutoff = $dbr->addQuotes( wfTimestamp( TS_MW, $cutoff_unixtime ) );
479	489	$res = $dbr->select( 'reader_feedback_history',
—	—	@@ -560,28 +570,29 @@
561	571	return $ext;
562	572	}
563	573
564		~~- public function getUserList() {~~
	574	+ protected function getUserList() {
565	575	global $wgMemc;
566	576	if( $this->period > 93 ) {
567	577	return ''; // too big
568	578	}
569		~~- // Check cache~~
570	579	$key = wfMemcKey( 'flaggedrevs', 'ratingusers', $this->page->getArticleId(), $this->period );
571		~~- $set = $wgMemc->get($key);~~
572		~~- // Cutoff is at the 24 hour mark due to the way the aggregate~~
573		~~- // schema groups ratings by date for graphs.~~
574		~~- $now = time();~~
575		~~- $cache_cutoff = $now - ($now % 86400);~~
576		~~- if( is_array($set) && count($set) == 2 ) {~~
577		~~- list($html,$time) = $set;~~
578		~~- $touched = wfTimestamp( TS_UNIX, self::getTouched($this->page) );~~
579		~~- if( $time > $cache_cutoff && $time > $touched ) {~~
580		~~- return $html;~~
	580	+ // Check cache
	581	+ if( !$this->doPurge ) {
	582	+ $set = $wgMemc->get($key);
	583	+ // Cutoff is at the 24 hour mark due to the way the aggregate
	584	+ // schema groups ratings by date for graphs.
	585	+ $cache_cutoff = $this->now - ($this->now % 86400);
	586	+ if( is_array($set) && count($set) == 2 ) {
	587	+ list($html,$time) = $set;
	588	+ $touched = wfTimestamp( TS_UNIX, self::getTouched($this->page) );
	589	+ if( $time > $cache_cutoff && $time > $touched ) {
	590	+ return $html;
	591	+ }
581	592	}
582	593	}
583	594	// Set cutoff time for period
584	595	$dbr = wfGetDB( DB_SLAVE );
585		~~- $cutoff_unixtime = time() - ($this->period * 24 * 3600);~~
	596	+ $cutoff_unixtime = $this->now - ($this->period * 24 * 3600);
586	597	// Use integral number of days to be consistent with graphs
587	598	$cutoff_unixtime = $cutoff_unixtime - ($cutoff_unixtime % 86400);
588	599	$cutoff = $dbr->addQuotes( wfTimestamp( TS_MW, $cutoff_unixtime ) );
—	—	@@ -620,111 +631,10 @@
621	632	}
622	633	}
623	634	$html .= "</tr></table>\n";
624		~~- $wgMemc->set( $key, array( $html, $now ), 24*3600 );~~
	635	+ $wgMemc->set( $key, array( $html, $this->now ), 24*3600 );
625	636	return $html;
626	637	}
627	638
628		~~- public static function getVoteAggregates( $page, $period, $add = array() ) {~~
629		~~- global $wgLang, $wgMemc;~~
630		~~- if( $period > 93 ) {~~
631		~~- return ''; // too big~~
632		~~- }~~
633		~~- // Check cache~~
634		~~- $key = wfMemcKey( 'flaggedrevs', 'ratingtally', $page->getArticleId(), $period );~~
635		~~- $set = $wgMemc->get($key);~~
636		~~- // Cutoff is at the 24 hour mark due to the way the aggregate~~
637		~~- // schema groups ratings by date for graphs.~~
638		~~- $now = time();~~
639		~~- $cache_cutoff = $now - ($now % 86400);~~
640		~~- if( is_array($set) && count($set) == 2 ) {~~
641		~~- list($val,$time) = $set;~~
642		~~- $touched = wfTimestamp( TS_UNIX, self::getTouched($page) );~~
643		~~- if( $time > $cache_cutoff && $time > $touched ) {~~
644		~~- $votes = $val;~~
645		~~- }~~
646		~~- }~~
647		~~- // Do query, cache miss~~
648		~~- if( !isset($votes) ) {~~
649		~~- // Set cutoff time for period~~
650		~~- $dbr = wfGetDB( DB_SLAVE );~~
651		~~- $cutoff_unixtime = $now - ($period * 24 * 3600);~~
652		~~- // Use integral number of days to be consistent with graphs~~
653		~~- $cutoff_unixtime = $cutoff_unixtime - ($cutoff_unixtime % 86400);~~
654		~~- $cutoff = $dbr->addQuotes( wfTimestamp( TS_MW, $cutoff_unixtime ) );~~
655		~~- // Get the first revision possibly voted on in the range~~
656		~~- $firstRevTS = $dbr->selectField( 'revision',~~
657		~~- 'rev_timestamp',~~
658		~~- array( 'rev_page' => $page->getArticleId(), "rev_timestamp <= $cutoff" ),~~
659		~~- __METHOD__,~~
660		~~- array( 'ORDER BY' => 'rev_timestamp DESC' )~~
661		~~- );~~
662		~~- // Find average, median, deviation...~~
663		~~- $res = $dbr->select( array( 'revision', 'reader_feedback' ),~~
664		~~- array( 'rfb_ratings' ),~~
665		~~- array( 'rev_page' => $page->getArticleId(),~~
666		~~- "rev_id = rfb_rev_id",~~
667		~~- "rfb_timestamp >= $cutoff",~~
668		~~- // Trigger INDEX usage~~
669		~~- "rev_timestamp >= ".$dbr->addQuotes($firstRevTS) ),~~
670		~~- __METHOD__,~~
671		~~- array( 'USE INDEX' => array('revision' => 'page_timestamp') )~~
672		~~- );~~
673		~~- $votes = array();~~
674		~~- foreach( FlaggedRevs::getFeedbackTags() as $tag => $w ) {~~
675		~~- $votes[$tag] = array( 0 => 0, 1 => 0, 2 => 0, 3 => 0, 4 => 0 );~~
676		~~- }~~
677		~~- // Read votes and tally the numbers~~
678		~~- while( $row = $dbr->fetchObject($res) ) {~~
679		~~- $dims = FlaggedRevs::expandRatings( $row->rfb_ratings );~~
680		~~- foreach( $dims as $tag => $val ) {~~
681		~~- if( isset($votes[$tag]) && isset($votes[$tag][$val]) ) {~~
682		~~- $votes[$tag][$val]++;~~
683		~~- }~~
684		~~- }~~
685		~~- }~~
686		~~- // Tack on $add for display (used to avoid cache/lag)~~
687		~~- foreach( $add as $tag => $val ) {~~
688		~~- if( isset($votes[$tag]) && isset($votes[$tag][$val]) ) {~~
689		~~- $votes[$tag][$val]++;~~
690		~~- }~~
691		~~- }~~
692		~~- $wgMemc->set( $key, array( $votes, $now ), 24*3600 );~~
693		~~- }~~
694		~~- // Output multi-column list~~
695		~~- $html = "<table class='fr_reader_feedback_table' cellspacing='0'><tr>";~~
696		~~- foreach( FlaggedRevs::getFeedbackTags() as $tag => $w ) {~~
697		~~- // Get tag average...~~
698		~~- $dist = isset($votes[$tag]) ? $votes[$tag] : array();~~
699		~~- $count = array_sum($dist);~~
700		~~- if( $count ) {~~
701		~~- $ave = ($dist[0] + 2$dist[1] + 3$dist[2] + 4$dist[3] + 5$dist[4])/$count;~~
702		~~- $ave = round($ave,1);~~
703		~~- } else {~~
704		~~- $ave = '-'; // DIV by zero~~
705		~~- }~~
706		~~- $html .= '<td align="center"><b>'.wfMsgHtml("readerfeedback-$tag").'</b>  '.~~
707		~~- '<sup>('.wfMsgHtml('ratinghistory-ave',$wgLang->formatNum($ave)).')</sup></td>';~~
708		~~- }~~
709		~~- $html .= '</tr><tr>';~~
710		~~- foreach( $votes as $tag => $dist ) {~~
711		~~- $html .= '<td><table>';~~
712		~~- $html .= '<tr><th align="left">'.wfMsgHtml('ratinghistory-table-rating').'</th>';~~
713		~~- for( $i = 1; $i <= 5; $i++ ) {~~
714		~~- $html .= "<td align='center' class='fr-rating-option-".($i-1)."'>$i</td>";~~
715		~~- }~~
716		~~- $html .= '</tr><tr>';~~
717		~~- $html .= '<th align="left">'.wfMsgHtml("ratinghistory-table-votes").'</th>';~~
718		~~- $html .= '<td align="center">'.$dist[0].'</td>';~~
719		~~- $html .= '<td align="center">'.$dist[1].'</td>';~~
720		~~- $html .= '<td align="center">'.$dist[2].'</td>';~~
721		~~- $html .= '<td align="center">'.$dist[3].'</td>';~~
722		~~- $html .= '<td align="center">'.$dist[4].'</td>';~~
723		~~- $html .= "</tr></table></td>\n";~~
724		~~- }~~
725		~~- $html .= '</tr></table>';~~
726		~~- return $html;~~
727		~~- }~~
728		-
729	639	public function purgePage() {
730	640	global $wgUploadDirectory;
731	641	foreach( FlaggedRevs::getFeedbackTags() as $tag => $weight ) {
—	—	@@ -749,7 +659,7 @@
750	660	* @param string $path, filepath to existing file
751	661	* @returns string
752	662	*/
753		~~- public function fileExpired( $tag, $path ) {~~
	663	+ protected function fileExpired( $tag, $path ) {
754	664	if( $this->doPurge \|\| !file_exists($path) ) {
755	665	return true;
756	666	}
—	—	@@ -760,7 +670,7 @@
761	671	$tagTimestamp = wfTimestamp( TS_UNIX, $tagTimestamp );
762	672	$file_unixtime = filemtime($path);
763	673	# Check max cache time
764		~~- $cutoff_unixtime = time() - (7 * 24 * 3600);~~
	674	+ $cutoff_unixtime = $this->now - (7 * 24 * 3600);
765	675	if( $file_unixtime < $cutoff_unixtime ) {
766	676	$this->purgePage();
767	677	return true;
Index: trunk/extensions/FlaggedRevs/specialpages/ReaderFeedback_body.php
—	—	@@ -165,7 +165,7 @@
166	166	$talk = $form->page->getTalkPage();
167	167
168	168	wfLoadExtensionMessages( 'RatingHistory' );
169		~~- $tallyTable = RatingHistory::getVoteAggregates( $form->page, 31, $form->dims );~~
	169	+ $tallyTable = FlaggedRevs::getVoteAggregates( $form->page, 31, $form->dims );
170	170
171	171	$dbw = wfGetDB( DB_MASTER );
172	172	$dbw->begin();

Status & tagging log

11:03, 25 March 2009 Brion VIBBER (talk | contribs) changed the status of r47701 [removed: new added: deferred]