User "Aaron Schulz" changed the status of MediaWiki.r88113. Old Status: new New Status: fixme
User "Aaron Schulz" also posted a comment on MediaWiki.r88113. Full URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/88113#c18645 Commit summary: Rewrote the article counting code and related: * (bug 26033, bug 24754) Added $wgArticleCountMethod to have a more flexible way to define which method to use to define if a page is an article or not and deprecated $wgUseCommaCount. There is now a new 'any' method to count any article that is in a content namespace and not a redirect. * (bug 11868) If using links to count articles, Article::isCountable() will now use the ParserOutput to check if there's a link instead of checking for the "[[" string. Changed Article::isCountable() to take a stdObject or false for the first parameters. If false is passed, the result will be based on the current article's state (i.e. database). The only call outside of the Article class is in DeleteAction (including extensions). * Removed this horror of Article::$mGoodAdjustment and Article::$mTotalAdjustment, replaced by the new $created parameter on Article::editUpdates(); simplified Article::createUpdates() * Updated Import.php to take advantage of the new parameter and make a single call to Article::editUpdates() Comment: 1) If you have the 'comma' counting method set, and do something like: <pre> $a = new Article( $title_of_existing_page ); $a->doEdit( ... ); </pre> ...nothing will get called that process caches the text getRawText() needs (I don't see anything that sets contentLoaded yet). Then updateRevisionOn() will get called and a COMMIT done to the DB before $this->editUpdates(). This means that page_latest is already updated (even if we didn't commit, if we happened hit the same DB, a MySQL session will "see" it's own uncommitted changes). editUpdates() will get called and cause some lines of code to be hit: <pre> 1. $good = (int)$this->isCountable( $editInfo ) - (int)$this->isCountable(); 2. $text = $this->getRawText(); [in isCountable] 3. $rev = Revision::newFromTitle( $this->mTitle ); [in getRawText] </pre> ...this will query the DB based on page_latest. This will fetch the ''new'' text, not the ''old'' text. Even when/if this gets the proper old text, it's still an extra hit (to memcached) to get the text. You should pass $oldtext (from <code>$oldtext = $this->getRawText(); // current revision</code>) down to the editUpdates somehow. 2) I grepped and don't see any extensions called editUpdates(). I suggest refactoring the params. I'd combine $minoredit, $created, and $changed into one $flags param using constants instead of booleans. $timestamp_of_pagechange could also be removed for being unused. If we really need b/c, then we can make a doEditUpdates() like this and leave editUpdates() as wrapper around it. _______________________________________________ MediaWiki-CodeReview mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
