r97701 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r97700‎ | r97701 | r97702 >
Date:07:13, 21 September 2011
Author:aaron
Status:ok (Comments)
Tags:todo 
Comment:
* Fixed bug causing new uploads to give "template/file changes need review" message.
* Fixed bug causing the same thing to happen with new uploads (bug 31056). Works via bypassing findFile() process cache.
Modified paths:
  • /trunk/extensions/FlaggedRevs/dataclasses/FlaggedRevs.class.php (modified) (history)

Diff [purge]

Index: trunk/extensions/FlaggedRevs/dataclasses/FlaggedRevs.class.php
@@ -972,10 +972,11 @@
973973 # If this is an image page, store corresponding file info
974974 $fileData = array( 'name' => null, 'timestamp' => null, 'sha1' => null );
975975 if ( $title->getNamespace() == NS_FILE ) {
976 - # We must use ImagePage process cache on upload or get bitten by slave lag
977 - $file = $article instanceof ImagePage
978 - ? $article->getFile()
979 - : wfFindFile( $title );
 976+ # We must use WikiFilePage process cache on upload or get bitten by slave lag
 977+ $file = ( $article instanceof WikiFilePage || $article instanceof ImagePage )
 978+ ? $article->getFile() // uses up-to-date process cache on new uploads
 979+ : wfFindFile( $title, array( 'bypassCache' => true ) ); // skip cache; bug 31056
 980+ #var_dump( $title ); var_dump( $file ); moo();
980981 if ( is_object( $file ) && $file->exists() ) {
981982 $fileData['name'] = $title->getDBkey();
982983 $fileData['timestamp'] = $file->getTimestamp();

Follow-up revisions

RevisionCommit summaryAuthorDate
r97702Removed debugging code from r97701aaron07:15, 21 September 2011
r97764MFT r97701,r97702aaron20:42, 21 September 2011
r97767Clear findFile() process cache of file moves (issue came up with bug bug 31056)aaron21:32, 21 September 2011
r98759MFT r92930, r96665, r97650, r97701, r97702, r97733, r97794, r97892reedy13:31, 3 October 2011

Comments

#Comment by Brion VIBBER (talk | contribs)   19:12, 21 September 2011

Ok, this looks like:

  • code is in FlaggedRevs::autoReviewEdit() which gets called on various editing activities
  • for image pages, one of those times will be during initial upload
  • we'll probably have looked with wfFindFile during that upload process, leaving a negative or old cache entry

I wonder though if we should instead be clearing that in-process cache? Is its old info left over from earlier in-process lookups and changed by an in-process upload completion?

#Comment by Brion VIBBER (talk | contribs)   20:39, 21 September 2011

Adding a todo tag for possible refactoring per notes above -- this should work ok for now.

Status & tagging log