r60222 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r60221‎ | r60222 | r60223 >
Date:01:08, 19 December 2009
Author:tparscal
Status:resolved (Comments)
Tags:
Comment:
Added memcache usage to reduce queries to once-per-minute. Refactored all the code to be cleaner and hopefully more performant.
Modified paths:
  • /trunk/extensions/ContributionReporting/ContributionReporting.i18n.php (modified) (history)
  • /trunk/extensions/ContributionReporting/FundraiserStatistics_body.php (modified) (history)

Diff [purge]

Index: trunk/extensions/ContributionReporting/FundraiserStatistics_body.php
@@ -11,20 +11,62 @@
1212 /* Functions */
1313
1414 public function __construct() {
15 - // Initialize special page
1615 parent::__construct( 'FundraiserStatistics' );
17 - // Internationalization
1816 wfLoadExtensionMessages( 'ContributionReporting' );
1917 }
2018
2119 public function execute( $sub ) {
22 - global $wgRequest, $wgOut, $wgUser, $wgLang, $wgScriptPath;
23 - global $egFundraiserStatisticsFundraisers;
24 - // Begins ouput
 20+ global $wgRequest, $wgOut, $wgUser, $wgLang, $wgScriptPath, $egFundraiserStatisticsFundraisers;
 21+
 22+ /* Configuration (this isn't totally static data, some of it gets built on the fly) */
 23+
 24+ $charts = array(
 25+ 'totals' => array(
 26+ 'data' => array(),
 27+ 'index' => 1,
 28+ 'query' => 'dailyTotalMax',
 29+ 'precision' => 2,
 30+ 'label' => 'fundraiserstats-total',
 31+ 'max' => 0,
 32+ ),
 33+ 'contributions' => array(
 34+ 'data' => array(),
 35+ 'index' => 2,
 36+ 'query' => 'contributionsMax',
 37+ 'precision' => 0,
 38+ 'label' => 'fundraiserstats-contributions',
 39+ 'max' => 0,
 40+ ),
 41+ 'averages' => array(
 42+ 'data' => array(),
 43+ 'index' => 3,
 44+ 'query' => 'averagesMax',
 45+ 'precision' => 2,
 46+ 'label' => 'fundraiserstats-avg',
 47+ 'max' => 0,
 48+ ),
 49+ 'maximums' => array(
 50+ 'data' => array(),
 51+ 'index' => 4,
 52+ 'query' => 'maximumsMax',
 53+ 'precision' => 2,
 54+ 'label' => 'fundraiserstats-max',
 55+ 'max' => 0,
 56+ ),
 57+ 'ytd' => array(
 58+ 'data' => array(),
 59+ 'index' => 5,
 60+ 'query' => 'yearlyTotalMax',
 61+ 'precision' => 2,
 62+ 'label' => 'fundraiserstats-ytd',
 63+ 'max' => 0,
 64+ ),
 65+ );
 66+
 67+ /* Setup */
 68+
2569 $this->setHeaders();
26 - // Adds JavaScript
2770 $wgOut->addScriptFile( $wgScriptPath . '/extensions/ContributionReporting/FundraiserStatistics.js' );
28 - // Adds CSS
2971 $wgOut->addLink(
3072 array(
3173 'rel' => 'stylesheet',
@@ -32,109 +74,84 @@
3375 'href' => $wgScriptPath . '/extensions/ContributionReporting/FundraiserStatistics.css',
3476 )
3577 );
36 - // Creates arrays that describe the charts and make places where the generated HTML will be stored
37 - $sources = array(
38 - 'totals' => 1,
39 - 'contributions' => 2,
40 - 'averages' => 3,
41 - 'maximums' => 4,
42 - );
43 - $charts = array(
44 - 'totals' => array(),
45 - 'contributions' => array(),
46 - 'averages' => array(),
47 - 'maximums' => array(),
48 - );
49 - $htmlViews = '';
50 - $htmlLegend = '';
51 - // Gets todays date in a format similar to the dates from the database for easy comparison
52 - $today = strtotime( date( 'M j Y' ) );
53 - // Calculates maximum value of all days in all fundraisers
54 - $max = array( 0, 0, 0, 0, 0 );
 78+
 79+ /* Display */
 80+
 81+ // Chart maximums
5582 foreach ( $egFundraiserStatisticsFundraisers as $fundraiser ) {
56 - $fundraiserMax = $this->getContributionsMax( $fundraiser['start'], $fundraiser['end'] );
57 - if ( $fundraiserMax > $max[$sources['contributions']] ) {
58 - $max[$sources['contributions']] = $fundraiserMax;
 83+ foreach ( $charts as $name => $chart ) {
 84+ $chartMax = $this->query( $charts[$name]['query'], $fundraiser['start'], $fundraiser['end'] );
 85+ if ( $chartMax > $charts[$name]['max'] ) {
 86+ $charts[$name]['max'] = $chartMax;
 87+ }
5988 }
60 - $fundraiserMax = $this->getDailyTotalMax( $fundraiser['start'], $fundraiser['end'] );
61 - if ( $fundraiserMax > $max[$sources['totals']] ) {
62 - $max[$sources['totals']] = $fundraiserMax;
63 - }
64 - $fundraiserMax = $this->getAveragesMax( $fundraiser['start'], $fundraiser['end'] );
65 - if ( $fundraiserMax > $max[$sources['averages']] ) {
66 - $max[$sources['averages']] = $fundraiserMax;
67 - }
68 - $fundraiserMax = $this->getMaximumsMax( $fundraiser['start'], $fundraiser['end'] );
69 - if ( $fundraiserMax > $max[$sources['maximums']] ) {
70 - $max[$sources['maximums']] = $fundraiserMax;
71 - }
7289 }
73 - // Builds the various HTML components
 90+ // Scale factors
 91+ foreach ( $charts as $name => $chart ) {
 92+ $charts[$name]['factor'] = $factor = 300 / $chart['max'];
 93+ }
 94+ // HTML-time!
7495 $view = 0;
 96+ $htmlViews = '';
7597 foreach ( $egFundraiserStatisticsFundraisers as $fundraiser ) {
76 - $htmlLegend .= Xml::element( 'div',
77 - array( 'class' => "fundraiserstats-legend-{$fundraiser['id']}" ),
78 - $fundraiser['title']
79 - );
80 - $days = $this->getDailyTotals( $fundraiser['start'], $fundraiser['end'] );
81 - foreach( $sources as $chart => $source ) {
 98+ $days = $this->query( 'dailyTotals', $fundraiser['start'], $fundraiser['end'] );
 99+ foreach ( $charts as $name => $chart ) {
82100 $column = 0;
83 - $factor = 300 / $max[$source];
84 - // Build bars for chart
85101 foreach( $days as $i => $day ) {
86 - $height = $factor * $day[$source];
87 - if ( !isset( $charts[$chart][$column] ) ) {
88 - $charts[$chart][$column] = '';
 102+ if ( !isset( $charts[$name]['data'][$column] ) ) {
 103+ $charts[$name]['data'][$column] = '';
89104 }
 105+ $height = $chart['factor'] * $day[$chart['index']];
90106 $attributes = array(
91107 'style' => "height:{$height}px",
92108 'class' => "fundraiserstats-bar-{$fundraiser['id']}",
93109 'onMouseOver' => "replaceView( 'fundraiserstats-view-box-{$view}' )"
94110 );
95 - $charts[$chart][$column] .= Xml::tags( 'td',
96 - array( 'valign' => 'bottom' ),
97 - Xml::element( 'div', $attributes, '', false )
 111+ $charts[$name]['data'][$column] .= Xml::tags(
 112+ 'td', array( 'valign' => 'bottom' ), Xml::element( 'div', $attributes, '', false )
98113 );
99 - // Build detail view for the day
100 - $tdLabelAttributes = array( 'width' => '16%', 'nowrap' => 'nowrap' );
101 - $tdValueAttributes = array( 'width' => '16%', 'nowrap' => 'nowrap', 'align' => 'right' );
102 - $htmlViews .= Xml::tags( 'div',
 114+ $htmlView = Xml::openElement( 'tr' );
 115+ $count = 0;
 116+ foreach ( $charts as $subchart ) {
 117+ $htmlView .= Xml::element(
 118+ 'td', array( 'width' => '16%', 'nowrap' => 'nowrap' ), wfMsg( $subchart['label'] )
 119+ );
 120+ $htmlView .= Xml::element(
 121+ 'td',
 122+ array( 'width' => '16%', 'nowrap' => 'nowrap', 'align' => 'right' ),
 123+ $wgLang->formatNum( number_format( $day[$subchart['index']], $subchart['precision'] ) )
 124+ );
 125+ if ( ++$count % 3 == 0 ) {
 126+ $htmlView .= Xml::closeElement( 'tr' ) . Xml::openElement( 'tr' );
 127+ }
 128+ }
 129+ $htmlView .= Xml::closeElement( 'tr' );
 130+ $htmlViews .= Xml::tags(
 131+ 'div',
103132 array(
104133 'id' => 'fundraiserstats-view-box-' . $view,
105134 'class' => 'fundraiserstats-view-box',
106135 'style' => 'display: ' . ( $view == 0 ? 'block' : 'none' )
107136 ),
108 - Xml::tags( 'table',
109 - array(
110 - 'cellpadding' => 10,
111 - 'cellspacing' => 0,
112 - 'border' => 0,
113 - 'width' => '100%'
114 - ),
115 - Xml::tags( 'tr', null,
116 - Xml::tags( 'td',
117 - array( 'colspan' => 4 ),
118 - Xml::element( 'h3', null,
 137+ Xml::tags(
 138+ 'table',
 139+ array( 'cellpadding' => 10, 'cellspacing' => 0, 'border' => 0, 'width' => '100%' ),
 140+ Xml::tags(
 141+ 'tr',
 142+ null,
 143+ Xml::tags(
 144+ 'td',
 145+ array( 'colspan' => 6 ),
 146+ Xml::element( 'h3', array( 'style' => 'float:right;color:gray;' ), $day[0] ) .
 147+ Xml::element(
 148+ 'h3',
 149+ array( 'style' => 'float:left;color:black;' ),
119150 wfMsg( 'fundraiserstats-day', $i + 1, $fundraiser['title'] )
120 - )
 151+ ) .
 152+ Xml::element( 'div', array( 'style' => 'clear:both;' ), '', false )
121153 )
122154 ) .
123 - Xml::tags( 'tr', null,
124 - Xml::element( 'td', $tdLabelAttributes, wfMsg( 'fundraiserstats-date' ) ) .
125 - Xml::element( 'td', $tdValueAttributes, $day[0] ) .
126 - Xml::element( 'td', $tdLabelAttributes, wfMsg( 'fundraiserstats-total' ) ) .
127 - Xml::element( 'td', $tdValueAttributes, $wgLang->formatNum( $day[1] ) ) .
128 - Xml::element( 'td', $tdLabelAttributes, wfMsg( 'fundraiserstats-max' ) ) .
129 - Xml::element( 'td', $tdValueAttributes, $wgLang->formatNum( number_format( $day[4], 2 ) ) )
130 - ) .
131 - Xml::tags( 'tr', null,
132 - Xml::element( 'td', $tdLabelAttributes, wfMsg( 'fundraiserstats-contributions' ) ) .
133 - Xml::element( 'td', $tdValueAttributes, $wgLang->formatNum( number_format( $day[2] ) ) ) .
134 - Xml::element( 'td', $tdLabelAttributes, wfMsg( 'fundraiserstats-avg' ) ) .
135 - Xml::element( 'td', $tdValueAttributes, $wgLang->formatNum( number_format( $day[3], 2 ) ) ) .
136 - Xml::element( 'td', $tdLabelAttributes, wfMsg( 'fundraiserstats-ytd' ) ) .
137 - Xml::element( 'td', $tdValueAttributes, $wgLang->formatNum( number_format( $day[5], 2 ) ) )
138 - )
 155+ $htmlView
139156 )
140157 );
141158 $column++;
@@ -142,11 +159,12 @@
143160 }
144161 }
145162 }
146 - // Show bar graphs
 163+ // Tabs
147164 $first = true;
148165 $htmlCharts = Xml::openElement( 'div', array( 'class' => 'fundraiserstats-chart-tabs' ) );
149166 foreach ( $charts as $chart => $columns ) {
150 - $htmlCharts .= Xml::tags( 'div',
 167+ $htmlCharts .= Xml::tags(
 168+ 'div',
151169 array(
152170 'id' => "fundraiserstats-chart-{$chart}-tab",
153171 'class' => 'fundraiserstats-chart-tab-' . ( $first ? 'current' : 'normal' ),
@@ -157,133 +175,138 @@
158176 $first = false;
159177 }
160178 $htmlCharts .= Xml::closeElement( 'div' );
 179+ // Charts
161180 $first = true;
162 - foreach ( $charts as $chart => $columns ) {
163 - $htmlCharts .= Xml::tags( 'div',
 181+ foreach ( $charts as $name => $chart ) {
 182+ $htmlCharts .= Xml::tags(
 183+ 'div',
164184 array(
165 - 'id' => "fundraiserstats-chart-{$chart}",
 185+ 'id' => "fundraiserstats-chart-{$name}",
166186 'class' => 'fundraiserstats-chart',
167187 'style' => 'display:' . ( $first ? 'block' : 'none' )
168188 ),
169 - Xml::tags( 'table',
170 - array(
171 - 'cellpadding' => 0,
172 - 'cellspacing' => 0,
173 - 'border' => 0
174 - ),
175 - Xml::tags( 'tr', null,
176 - implode( $columns )
177 - )
 189+ Xml::tags(
 190+ 'table',
 191+ array( 'cellpadding' => 0, 'cellspacing' => 0, 'border' => 0 ),
 192+ Xml::tags( 'tr', null, implode( $chart['data'] ) )
178193 )
179194 );
180195 $first = false;
181196 }
182 - // Show views
183 - $htmlOut = Xml::tags( 'table',
184 - array(
185 - 'cellpadding' => 0,
186 - 'cellspacing' => 0,
187 - 'border' => 0
188 - ),
189 - Xml::tags( 'tr', null,
190 - Xml::tags( 'td', null,
191 - $htmlCharts
192 - )
193 - ) .
194 - Xml::tags( 'tr', null,
195 - Xml::tags( 'td', null, $htmlViews )
 197+ // Output
 198+ $wgOut->addHTML(
 199+ Xml::tags(
 200+ 'table',
 201+ array(
 202+ 'cellpadding' => 0,
 203+ 'cellspacing' => 0,
 204+ 'border' => 0
 205+ ),
 206+ Xml::tags( 'tr', null, Xml::tags( 'td', null, $htmlCharts ) ) .
 207+ Xml::tags( 'tr', null, Xml::tags( 'td', null, $htmlViews ) )
196208 )
197209 );
198 - $wgOut->addHTML( $htmlOut );
199210 }
200211
201 - /* Query Functions */
 212+ /* Private Functions */
202213
203 - public function getDailyTotals( $start, $end ) {
204 - $dbr = efContributionReportingConnection();
205 - $res = $dbr->select( 'public_reporting',
206 - array(
207 - "FROM_UNIXTIME(received, '%Y-%m-%d')",
208 - 'sum(converted_amount)',
209 - 'count(*)',
210 - 'avg(converted_amount)',
211 - 'max(converted_amount)',
212 - ),
213 - $this->getConditions( $dbr, $start, $end ),
214 - __METHOD__,
215 - array(
216 - 'ORDER BY' => 'received',
217 - 'GROUP BY' => "FROM_UNIXTIME(received, '%Y-%m-%d')"
218 - )
219 - );
220 - $totals = array();
221 - $ytd = 0;
222 - while ( $row = $dbr->fetchRow( $res ) ) {
223 - $row[] = $ytd += $row[1];
224 - $totals[] = $row;
 214+ private function query( $type, $start, $end ) {
 215+ global $wgMemc, $egFundraiserStatisticsMinimum, $egFundraiserStatisticsMaximum;
 216+
 217+ // Try cache - key exipires once per minute
 218+ $key = wfMemcKey( 'fundraiserstatistics', $type, $start, $end, date( 'YmdHi' ) );
 219+ $cache = $wgMemc->get( $key );
 220+ if ( $cache != false && $cache != -1 ) {
 221+ return $cache;
225222 }
226 - return $totals;
227 - }
228 -
229 - public function getDailyTotalMax( $start, $end ) {
 223+ // Use database
230224 $dbr = efContributionReportingConnection();
231 - return $dbr->selectField( 'public_reporting',
232 - array( 'sum(converted_amount) as sum' ),
233 - $this->getConditions( $dbr, $start, $end ),
234 - __METHOD__,
235 - array(
236 - 'ORDER BY' => 'sum DESC',
237 - 'GROUP BY' => "FROM_UNIXTIME(received, '%Y-%m-%d')",
238 - )
239 - );
240 - }
241 -
242 - public function getContributionsMax( $start, $end ) {
243 - $dbr = efContributionReportingConnection();
244 - return $dbr->selectField( 'public_reporting',
245 - array( 'count(converted_amount) as sum' ),
246 - $this->getConditions( $dbr, $start, $end ),
247 - __METHOD__,
248 - array(
249 - 'ORDER BY' => 'sum DESC',
250 - 'GROUP BY' => "FROM_UNIXTIME(received, '%Y-%m-%d')",
251 - )
252 - );
253 - }
254 -
255 - public function getAveragesMax( $start, $end ) {
256 - $dbr = efContributionReportingConnection();
257 - return $dbr->selectField( 'public_reporting',
258 - array( 'avg(converted_amount) as sum' ),
259 - $this->getConditions( $dbr, $start, $end ),
260 - __METHOD__,
261 - array(
262 - 'ORDER BY' => 'sum DESC',
263 - 'GROUP BY' => "FROM_UNIXTIME(received, '%Y-%m-%d')",
264 - )
265 - );
266 - }
267 -
268 - public function getMaximumsMax( $start, $end ) {
269 - $dbr = efContributionReportingConnection();
270 - return $dbr->selectField( 'public_reporting',
271 - array( 'max(converted_amount) as sum' ),
272 - $this->getConditions( $dbr, $start, $end ),
273 - __METHOD__,
274 - array(
275 - 'ORDER BY' => 'sum DESC',
276 - 'GROUP BY' => "FROM_UNIXTIME(received, '%Y-%m-%d')",
277 - )
278 - );
279 - }
280 -
281 - protected function getConditions( $dbr, $start, $end ) {
282 - global $egFundraiserStatisticsMinimum, $egFundraiserStatisticsMaximum;
283 - return array(
 225+ $conditions = array(
284226 'received >= ' . $dbr->addQuotes( wfTimestamp( TS_UNIX, strtotime( $start ) ) ),
285227 'received <= ' . $dbr->addQuotes( wfTimestamp( TS_UNIX, strtotime( $end ) + 24 * 60 * 60 ) ),
286228 'converted_amount >= ' . $egFundraiserStatisticsMinimum,
287229 'converted_amount <= ' . $egFundraiserStatisticsMaximum
288230 );
 231+ switch ( $type ) {
 232+ case 'dailyTotals':
 233+ $select = $dbr->select( 'public_reporting',
 234+ array(
 235+ "FROM_UNIXTIME(received, '%Y-%m-%d')",
 236+ 'sum(converted_amount)',
 237+ 'count(*)',
 238+ 'avg(converted_amount)',
 239+ 'max(converted_amount)',
 240+ ),
 241+ $conditions,
 242+ __METHOD__,
 243+ array(
 244+ 'ORDER BY' => 'received',
 245+ 'GROUP BY' => "FROM_UNIXTIME(received, '%Y-%m-%d')"
 246+ )
 247+ );
 248+ $result = array();
 249+ $ytd = 0;
 250+ while ( $row = $dbr->fetchRow( $select ) ) {
 251+ $row[] = $ytd += $row[1]; // YTD
 252+ $result[] = $row;
 253+ }
 254+ break;
 255+ case 'dailyTotalMax':
 256+ $result = $dbr->selectField( 'public_reporting',
 257+ array( 'sum(converted_amount) as sum' ),
 258+ $conditions,
 259+ __METHOD__,
 260+ array(
 261+ 'ORDER BY' => 'sum DESC',
 262+ 'GROUP BY' => "FROM_UNIXTIME(received, '%Y-%m-%d')",
 263+ )
 264+ );
 265+ break;
 266+ case 'yearlyTotalMax':
 267+ $result = $dbr->selectField( 'public_reporting',
 268+ array( 'sum(converted_amount) as sum' ),
 269+ $conditions,
 270+ __METHOD__
 271+ );
 272+ break;
 273+ case 'contributionsMax':
 274+ $result = $dbr->selectField( 'public_reporting',
 275+ array( 'count(converted_amount) as sum' ),
 276+ $conditions,
 277+ __METHOD__,
 278+ array(
 279+ 'ORDER BY' => 'sum DESC',
 280+ 'GROUP BY' => "FROM_UNIXTIME(received, '%Y-%m-%d')",
 281+ )
 282+ );
 283+ break;
 284+ case 'averagesMax':
 285+ $result = $dbr->selectField( 'public_reporting',
 286+ array( 'avg(converted_amount) as sum' ),
 287+ $conditions,
 288+ __METHOD__,
 289+ array(
 290+ 'ORDER BY' => 'sum DESC',
 291+ 'GROUP BY' => "FROM_UNIXTIME(received, '%Y-%m-%d')",
 292+ )
 293+ );
 294+ break;
 295+ case 'maximumsMax':
 296+ $result = $dbr->selectField( 'public_reporting',
 297+ array( 'max(converted_amount) as sum' ),
 298+ $conditions,
 299+ __METHOD__,
 300+ array(
 301+ 'ORDER BY' => 'sum DESC',
 302+ 'GROUP BY' => "FROM_UNIXTIME(received, '%Y-%m-%d')",
 303+ )
 304+ );
 305+ break;
 306+ }
 307+ if ( isset( $result ) ) {
 308+ $wgMemc->set( $key, $result );
 309+ return $result;
 310+ }
 311+ return null;
289312 }
290313 }
Index: trunk/extensions/ContributionReporting/ContributionReporting.i18n.php
@@ -91,6 +91,7 @@
9292 'fundraiserstats-tab-contributions' => 'Number of contributions',
9393 'fundraiserstats-tab-averages' => 'Averages (USD)',
9494 'fundraiserstats-tab-maximums' => 'Maximums (USD)',
 95+ 'fundraiserstats-tab-ytd' => 'Year-to-date (USD)',
9596
9697 'specialpages-group-contribution' => 'Contributions/Fundraiser',
9798 );

Comments

#Comment by Tim Starling (talk | contribs)   06:46, 12 January 2010

If you want to recache it every minute then call $wgMemc->set() with $expiry=60, don't pollute memcached with useless old data. Remember that you're sharing a cache pool with the whole site: when you add loads of useless rubbish to it, you cause valuable data to be evicted.

#Comment by Tim Starling (talk | contribs)   06:48, 12 January 2010

BTW there's no such word as "performant". Maybe you mean "faster".

#Comment by Trevor Parscal (WMF) (talk | contribs)   19:05, 12 January 2010

At the time I wrote this, I asked around for advice and then acted on that. One issue with what you are suggesting is that it will only invalidate 60 seconds from when someone hits the page, not every 60 seconds of actual time. However, I'm sure this can be done better than I've done it however and I am very interested in any help you are willing to give.

Also, are you sure performant is not a word? http://en.wiktionary.org/wiki/performant

#Comment by Catrope (talk | contribs)   19:34, 12 January 2010

The "60 seconds between hits vs. 60 seconds of actual time" thing is not an issue for most conventional uses of caching. Unless you're like writing to the database every 60 seconds or have another pressing need to stick to the clock, the distinction is so minor (especially if there's either very many or very few hits) that it's very much worth sacrificing.

#Comment by Tim Starling (talk | contribs)   22:59, 12 January 2010

That's what you get when you look up words in the dictionary that anyone can edit.

#Comment by Trevor Parscal (WMF) (talk | contribs)   19:49, 12 January 2010

I'm sure you're right. I've fixed this up using Tim's suggestion in r60982.

Status & tagging log