From d607421461fdf57c7e1471ad0d2dd288c0ab96e0 Mon Sep 17 00:00:00 2001
From: daniel <daniel.kinzler@wikimedia.de>
Date: Mon, 11 Jun 2012 16:19:39 +0200
Subject: [PATCH] some cleanup in Title and WikiPage

---
 includes/Title.php    | 19 +++++++++++++++----
 includes/WikiPage.php | 16 ++++++++--------
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/includes/Title.php b/includes/Title.php
index 9255cb301c..b0849db73a 100644
--- a/includes/Title.php
+++ b/includes/Title.php
@@ -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 );
 
diff --git a/includes/WikiPage.php b/includes/WikiPage.php
index 5919c41cc0..da1ff2b7f9 100644
--- a/includes/WikiPage.php
+++ b/includes/WikiPage.php
@@ -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 );
 
-- 
2.20.1