r47319 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r47318‎ | r47319 | r47320 >
Date:16:50, 16 February 2009
Author:simetrical
Status:resolved (Comments)
Tags:
Comment:
Partial revert of r41018 "Wrap $log->addEntry() in transaction"

In addition to wrapping the given function call in a transaction, the
commit also (without explanation or apparent purpose) moved a block of
code to a totally different place, which broke it entirely.
Specifically, it moved the update of the category table on article
delete to AFTER THE CATEGORYLINKS ROWS WERE ALREADY DELETED, which meant
that it found the article was in no categories, and article deletions
silently failed to decrement the category count (bug 17155).

Could you PLEASE not commit all sorts of stuff under the guise of
one-line commit messages that don't actually relate to what you did.
Thank you.
Modified paths:
  • /trunk/phase3/includes/Article.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Article.php
——@@ -2549,6 +2549,14 @@
25502550 $dbw->delete( 'langlinks', array( 'll_from' => $id ) );
25512551 $dbw->delete( 'redirect', array( 'rd_from' => $id ) );
25522552 }
 2553+
 2554+ # Fix category table counts
 2555+ $cats = array();
 2556+ $res = $dbw->select( 'categorylinks', 'cl_to', array( 'cl_from' => $id ), __METHOD__ );
 2557+ foreach( $res as $row ) {
 2558+ $cats []= $row->cl_to;
 2559+ }
 2560+ $this->updateCategoryCounts( array(), $cats );
25532561
25542562 # If using cleanup triggers, we can skip some manual deletes
25552563 if( !$dbw->cleanupTriggers() ) {
——@@ -2565,14 +2573,6 @@
25662574
25672575 # Clear caches
25682576 Article::onArticleDelete( $this->mTitle );
2569 -
2570 - # Fix category table counts
2571 - $cats = array();
2572 - $res = $dbw->select( 'categorylinks', 'cl_to', array( 'cl_from' => $id ), __METHOD__ );
2573 - foreach( $res as $row ) {
2574 - $cats []= $row->cl_to;
2575 - }
2576 - $this->updateCategoryCounts( array(), $cats );
25772577
25782578 # Clear the cached article id so the interface doesn't act like we exist
25792579 $this->mTitle->resetArticleID( 0 );
——@@ -2584,7 +2584,7 @@
25852585
25862586 # Make sure logging got through
25872587 $log->addEntry( 'delete', $this->mTitle, $reason, array() );
2588 -
 2588+
25892589 $dbw->commit();
25902590
25912591 return true;

Follow-up revisions

RevisionCommit summaryAuthorDate
r47326Fix r47319: this is reverting the wrong revision (cause was r40912). Move cat...aaron17:57, 16 February 2009

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r41018Wrap $log->addEntry() in transactionaaron23:43, 18 September 2008

Comments

#Comment by Aaron Schulz (talk | contribs)   18:01, 16 February 2009

Moved up further in r47326 to fix the problem

#Comment by Simetrical (talk | contribs)   18:04, 16 February 2009

Thanks.

Status & tagging log