User "Catrope" changed the status of MediaWiki.r87402. Old Status: new New Status: fixme
User "Catrope" also posted a comment on MediaWiki.r87402. Full URL: https://secure.wikimedia.org/wikipedia/mediawiki/wiki/Special:Code/MediaWiki/87402#c16571 Commit summary: Added caching of latest highs/lows in high/low populating maintenance script; Added data-fetching logic for getDailyHighsAndLows() in special page (including cachability); Change how the title is generated in renderDailyHighsAndLows() to use page id Comment: <pre> + if ( $cache != false && $cache = -1 ) { </pre> Assignment to -1 gone in r87429. <pre> + 'afshl_ts = ' . $row->afshl_ts, </pre> This is insecure (no quoting or escaping), use <code>array( 'afshl_ts' => $row->afshl_ts )</code> . Also occurs in the maintenance script. <pre> + $wgMemc->set( $key, $result, 86400 ); </pre> This stores a ResultWrapper object in the cache, which scares me. Told Arthur about this on IRC. <code>$highs_lows</code> should be cached instead. <pre> + if ( is_a( $wgMemc, 'EmptyBagOStuff' )) { + $this->output( "Object caching not available.\n" ); + $this->output( "Done.\n" ); + return; + } </pre> Eww; but I guess it's sort of warranted here. <pre> + $wgMemc->delete( $key ); </pre> This is unnecessary. <code>set()</code> will overwrite an existing value just fine. It will probably result in bugs or race conditions because the old value will be briefly unavailable. _______________________________________________ MediaWiki-CodeReview mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
