some cleanup in Title and WikiPage
authordaniel <daniel.kinzler@wikimedia.de>
Mon, 11 Jun 2012 14:19:39 +0000 (16:19 +0200)
committerdaniel <daniel.kinzler@wikimedia.de>
Mon, 11 Jun 2012 14:19:39 +0000 (16:19 +0200)
includes/Title.php
includes/WikiPage.php

index 9255cb3..b0849db 100644 (file)
@@ -416,7 +416,6 @@ class Title {
         * @param $text String Text with possible redirect
         * @return Array of Titles, with the destination last
         * @deprecated since 1.WD, use Content::getRedirectChain instead.
-        * @todo: migrate this logic into WikitextContent!
         */
        public static function newFromRedirectArray( $text ) {
                $content = ContentHandler::makeContent( $text, null, CONTENT_MODEL_WIKITEXT );
@@ -2838,7 +2837,11 @@ class Title {
 
                $linkCache = LinkCache::singleton();
                $cached = $linkCache->getGoodLinkFieldObj( $this, 'redirect' );
-               assert( $cached !== null ); # assert the assumption that the cache actually knows about this title #XXX breaks stuff #TODO: use exception
+               if ( $cached === null ) { # check the assumption that the cache actually knows about this title
+                       # XXX: this does apparently happen, see https://bugzilla.wikimedia.org/show_bug.cgi?id=37209
+                       #      as a stop gap, perhaps log this, but don't throw an exception?
+                       throw new MWException( "LinkCache doesn't currently know about this title: " . $this->getPrefixedDBkey() );
+               }
 
                $this->mRedirect = (bool)$cached;
 
@@ -2862,7 +2865,11 @@ class Title {
                }
                $linkCache = LinkCache::singleton();
                $cached = $linkCache->getGoodLinkFieldObj( $this, 'length' );
-               assert( $cached !== null ); # assert the assumption that the cache actually knows about this title #TODO: use exception
+               if ( $cached === null ) { # check the assumption that the cache actually knows about this title
+                       # XXX: this does apparently happen, see https://bugzilla.wikimedia.org/show_bug.cgi?id=37209
+                       #      as a stop gap, perhaps log this, but don't throw an exception?
+                       throw new MWException( "LinkCache doesn't currently know about this title: " . $this->getPrefixedDBkey() );
+               }
 
                $this->mLength = intval( $cached );
 
@@ -2885,7 +2892,11 @@ class Title {
                }
                $linkCache = LinkCache::singleton();
                $cached = $linkCache->getGoodLinkFieldObj( $this, 'revision' );
-               assert( $cached !== null ); # assert the assumption that the cache actually knows about this title #TODO: use exception
+               if ( $cached === null ) { # check the assumption that the cache actually knows about this title
+                       # XXX: this does apparently happen, see https://bugzilla.wikimedia.org/show_bug.cgi?id=37209
+                       #      as a stop gap, perhaps log this, but don't throw an exception?
+                       throw new MWException( "LinkCache doesn't currently know about this title: " . $this->getPrefixedDBkey() );
+               }
 
                $this->mLatestID = intval( $cached );
 
index 5919c41..da1ff2b 100644 (file)
@@ -1617,7 +1617,7 @@ class WikiPage extends Page {
                $hook_ok = wfRunHooks( 'ArticleContentSave', array( &$this, &$user, &$content, &$summary,
                        $flags & EDIT_MINOR, null, null, &$flags, &$status ) );
 
-               if ( $hook_ok && !empty( $wgHooks['ArticleSave'] ) ) { #FIXME: use wfHasHook or whatever. # avoid serialization overhead if the hook isn't present
+               if ( $hook_ok && Hooks::isRegistered( 'ArticleSave' ) ) { # avoid serialization overhead if the hook isn't present
                        $content_text = $content->serialize();
                        $txt = $content_text; # clone
 
@@ -1704,9 +1704,8 @@ class WikiPage extends Page {
                        $changed = !$content->equals( $old_content );
 
                        if ( $changed ) {
-                               // TODO: validate!
-                               if ( $content->isValid() ) {
-                                       #XXX: do it! throw exception??
+                               if ( !$content->isValid() ) {
+                                       throw new MWException( "New content failed validity check!" );
                                }
 
                                $dbw->begin( __METHOD__ );
@@ -1950,8 +1949,9 @@ class WikiPage extends Page {
                $edit->newContent = $content;
                $edit->oldContent = $this->getContent( Revision::RAW );
 
-               $edit->newText = ContentHandler::getContentText( $edit->newContent ); #FIXME: B/C only! don't use this field!
-               $edit->oldText = $edit->oldContent ? ContentHandler::getContentText( $edit->oldContent ) : ''; #FIXME: B/C only! don't use this field!
+               #NOTE: B/C for hooks! don't use these fields!
+               $edit->newText = ContentHandler::getContentText( $edit->newContent );
+               $edit->oldText = $edit->oldContent ? ContentHandler::getContentText( $edit->oldContent ) : '';
 
                $this->mPreparedEdit = $edit;
 
@@ -2071,7 +2071,7 @@ class WikiPage extends Page {
                }
 
                if ( $this->mTitle->getNamespace() == NS_MEDIAWIKI ) {
-                       $msgtext = ContentHandler::getContentText( $content ); #XXX: could skip pseudo-messages like js/css here, based on content model.
+                       $msgtext = $content->getWikitextForTransclusion(); #XXX: could skip pseudo-messages like js/css here, based on content model.
                        if ( $msgtext === false || $msgtext === null ) $msgtext = '';
 
                        MessageCache::singleton()->replace( $shortTitle, $msgtext );
@@ -2128,7 +2128,7 @@ class WikiPage extends Page {
                        'length'     => $content->getSize(),
                        'comment'    => $comment,
                        'minor_edit' => $minor ? 1 : 0,
-               ) ); #XXX: set the content object
+               ) ); #XXX: set the content object?
                $revision->insertOn( $dbw );
                $this->updateRevisionOn( $dbw, $revision );