Bug 39509: Function for running legacy hooks.
authordaniel <daniel.kinzler@wikimedia.de>
Mon, 20 Aug 2012 17:28:20 +0000 (19:28 +0200)
committerdaniel <daniel.kinzler@wikimedia.de>
Mon, 20 Aug 2012 20:00:52 +0000 (22:00 +0200)
ContentHandler::runLegacyHooks can be used to run hooks that don't
supprot Content objects yet. runLegacyHooks will issue a warning and take
case of serialization/unserialization of the content as appropriate.

Changeset 2: rebased.

Change-Id: I31109061110f87c38bdeebf30d520c8e1241bb29

includes/Article.php
includes/ContentHandler.php
includes/EditPage.php
includes/WikiPage.php
includes/diff/DifferenceEngine.php
tests/phpunit/includes/ContentHandlerTest.php

index dc6294f..f791edf 100644 (file)
@@ -391,7 +391,7 @@ class Article extends Page {
                $content = $this->fetchContentObject();
 
                $this->mContent = ContentHandler::getContentText( $content ); #@todo: get rid of mContent everywhere!
-               wfRunHooks( 'ArticleAfterFetchContent', array( &$this, &$this->mContent ) ); #BC cruft, deprecated!
+               ContentHandler::runLegacyHooks( 'ArticleAfterFetchContent', array( &$this, &$this->mContent ) );
 
                wfProfileOut( __METHOD__ );
 
@@ -679,10 +679,16 @@ class Article extends Page {
                                                wfDebug( __METHOD__ . ": showing CSS/JS source\n" );
                                                $this->showCssOrJsPage();
                                                $outputDone = true;
-                                       } elseif( !wfRunHooks( 'ArticleContentViewCustom', array( $this->fetchContentObject(), $this->getTitle(), $outputPage ) ) ) {
+                                       } elseif( !wfRunHooks( 'ArticleContentViewCustom',
+                                                                                       array( $this->fetchContentObject(), $this->getTitle(),
+                                                                                                       $outputPage ) ) ) {
+
                                                # Allow extensions do their own custom view for certain pages
                                                $outputDone = true;
-                                       } elseif( Hooks::isRegistered( 'ArticleViewCustom' ) && !wfRunHooks( 'ArticleViewCustom', array( $this->fetchContent(), $this->getTitle(), $outputPage ) ) ) { #FIXME: fetchContent() is deprecated!
+                                       } elseif( !ContentHandler::runLegacyHooks( 'ArticleViewCustom',
+                                                                                                                               array( $this->fetchContentObject(), $this->getTitle(),
+                                                                                                                                               $outputPage ) ) ) {
+
                                                # Allow extensions do their own custom view for certain pages
                                                $outputDone = true;
                                        } else {
@@ -693,7 +699,8 @@ class Article extends Page {
                                                        # Viewing a redirect page (e.g. with parameter redirect=no)
                                                        $outputPage->addHTML( $this->viewRedirect( $rt ) );
                                                        # Parse just to get categories, displaytitle, etc.
-                                                       $this->mParserOutput = $content->getParserOutput( $this->getTitle(), $oldid, $parserOptions, false );
+                                                       $this->mParserOutput = $content->getParserOutput( $this->getTitle(), $oldid,
+                                                                                                                                                               $parserOptions, false );
                                                        $outputPage->addParserOutputNoText( $this->mParserOutput );
                                                        $outputDone = true;
                                                }
@@ -830,7 +837,7 @@ class Article extends Page {
                }
 
                // Give hooks a chance to customise the output
-               if ( !Hooks::isRegistered('ShowRawCssJs') || wfRunHooks( 'ShowRawCssJs', array( $this->fetchContent(), $this->getTitle(), $wgOut ) ) ) { #FIXME: fetchContent() is deprecated
+               if ( ContentHandler::runLegacyHooks( 'ShowRawCssJs', array( $this->fetchContentObject(), $this->getTitle(), $wgOut ) ) ) {
                        $po = $this->mContentObject->getParserOutput( $this->getTitle() );
                        $wgOut->addHTML( $po->getText() );
                }
@@ -1704,8 +1711,10 @@ class Article extends Page {
         * @return ParserOutput or false if the given revsion ID is not found
         */
        public function getParserOutput( $oldid = null, User $user = null ) {
+               //XXX: bypasses mParserOptions and thus setParserOptions()
+
                $user = is_null( $user ) ? $this->getContext()->getUser() : $user;
-               $parserOptions = $this->mPage->makeParserOptions( $user ); //XXX: bypasses mParserOptions and thus setParserOptions()
+               $parserOptions = $this->mPage->makeParserOptions( $user );
 
                return $this->mPage->getParserOutput( $parserOptions, $oldid );
        }
index e460eee..a0a5552 100644 (file)
@@ -825,6 +825,62 @@ abstract class ContentHandler {
        public function supportsSections() {
                return false;
        }
+
+       /**
+        * Call a legacy hook that uses text instead of Content objects.
+        * Will log a warning when a matching hook function is registered.
+        * If the textual representation of the content is changed by the
+        * hook function, a new Content object is constructed from the new
+        * text.
+        *
+        * @param $event String: event name
+        * @param $args Array: parameters passed to hook functions
+        *
+        * @return Boolean True if no handler aborted the hook
+        */
+       public static function runLegacyHooks( $event, $args = array() ) {
+               if ( !Hooks::isRegistered( $event ) ) {
+                       return true; // nothing to do here
+               }
+
+               wfWarn( "Using obsolete hook $event" );
+
+               // convert Content objects to text
+               $contentObjects = array();
+               $contentTexts = array();
+
+               foreach ( $args as $k => $v ) {
+                       if ( $v instanceof Content ) {
+                               /* @var Content $v */
+
+                               $contentObjects[$k] = $v;
+
+                               $v = $v->serialize();
+                               $contentTexts[ $k ] = $v;
+                               $args[ $k ] = $v;
+                       }
+               }
+
+               // call the hook functions
+               $ok = wfRunHooks( $event, $args );
+
+               // see if the hook changed the text
+               foreach ( $contentTexts as $k => $orig ) {
+                       /* @var Content $content */
+
+                       $modified = $args[ $k ];
+                       $content = $contentObjects[$k];
+
+                       if ( $modified !== $orig ) {
+                               // text was changed, create updated Content object
+                               $content = $content->getContentHandler()->unserializeContent( $modified );
+                       }
+
+                       $args[ $k ] = $content;
+               }
+
+               return $ok;
+       }
 }
 
 /**
index 3e38fb8..09e5f15 100644 (file)
@@ -1457,8 +1457,10 @@ class EditPage {
                                }
 
                                // Run post-section-merge edit filter
-                               if ( !wfRunHooks( 'EditFilterMerged', array( $this, $content->serialize( $this->content_format ), &$this->hookError, $this->summary ) )
-                                               || !wfRunHooks( 'EditFilterMergedContent', array( $this, $content, &$this->hookError, $this->summary ) ) ) {
+                               $hook_args = array( $this, $content, &$this->hookError, $this->summary );
+
+                               if ( !ContentHandler::runLegacyHooks( 'EditFilterMerged', $hook_args )
+                                               || !wfRunHooks( 'EditFilterMergedContent', $hook_args ) ) {
                                        # Error messages etc. could be handled within the hook...
                                        $status->fatal( 'hookaborted' );
                                        $status->value = self::AS_HOOK_ERROR;
@@ -2547,18 +2549,7 @@ HTML
                                                                                        $this->section, $textboxContent,
                                                                                        $this->summary, $this->edittime );
 
-               # hanlde legacy text-based hook
-               $newtext_orig = $newContent->serialize( $this->content_format );
-               $newtext = $newtext_orig; #clone
-               wfRunHooks( 'EditPageGetDiffText', array( $this, &$newtext ) );
-
-               if ( $newtext != $newtext_orig ) {
-                                               #if the hook changed the text, create a new Content object accordingly.
-                                               #XXX: handle parse errors ?
-                                               $newContent = ContentHandler::makeContent( $newtext, $this->getTitle(),
-                                                                                                                                       $newContent->getModel() );
-               }
-
+               ContentHandler::runLegacyHooks( 'EditPageGetDiffText', array( $this, &$newContent ) );
                wfRunHooks( 'EditPageGetDiffContent', array( $this, &$newContent ) );
 
                $popts = ParserOptions::newFromUserAndLang( $wgUser, $wgContLang );
@@ -2862,17 +2853,9 @@ HTML
                                        $content = $content->addSectionHeader( $this->summary );
                                }
 
-                               $toparse_orig = $content->serialize( $this->content_format );
-                               $toparse = $toparse_orig;
-                               wfRunHooks( 'EditPageGetPreviewText', array( $this, &$toparse ) );
-
-                               if ( $toparse !== $toparse_orig ) {
-                                       #hook changed the text, create new Content object
-                                       $content = ContentHandler::makeContent( $toparse, $this->getTitle(),
-                                                                                                                       $this->content_model, $this->content_format );
-                               }
-
-                               wfRunHooks( 'EditPageGetPreviewContent', array( $this, &$content ) );
+                               $hook_args = array( $this, &$content );
+                               ContentHandler::runLegacyHooks( 'EditPageGetPreviewText', $hook_args );
+                               wfRunHooks( 'EditPageGetPreviewContent', $hook_args );
 
                                $parserOptions->enableLimitReport();
 
index a4afda7..0318f2d 100644 (file)
@@ -1623,24 +1623,13 @@ class WikiPage extends Page implements IDBAccessObject {
 
                $flags = $this->checkFlags( $flags );
 
-               # call legacy hook
-               $hook_ok = wfRunHooks( 'ArticleContentSave', array( &$this, &$user, &$content, &$summary,
-                       $flags & EDIT_MINOR, null, null, &$flags, &$status ) );
+               # handle hook
+               $hook_args = array( &$this, &$user, &$content, &$summary,
+                                                       $flags & EDIT_MINOR, null, null, &$flags, &$status );
 
-               if ( $hook_ok && Hooks::isRegistered( 'ArticleSave' ) ) { # avoid serialization overhead if the hook isn't present
-                       $content_text = $content->serialize();
-                       $txt = $content_text; # clone
+               if ( !wfRunHooks( 'ArticleContentSave', $hook_args )
+                       || !ContentHandler::runLegacyHooks( 'ArticleSave', $hook_args ) ) {
 
-                       $hook_ok = wfRunHooks( 'ArticleSave', array( &$this, &$user, &$txt, &$summary,
-                               $flags & EDIT_MINOR, null, null, &$flags, &$status ) ); #TODO: survey extensions using this hook
-
-                       if ( $txt !== $content_text ) {
-                               # if the text changed, unserialize the new version to create an updated Content object.
-                               $content = $content->getContentHandler()->unserializeContent( $txt );
-                       }
-               }
-
-               if ( !$hook_ok ) {
                        wfDebug( __METHOD__ . ": ArticleSave or ArticleSaveContent hook aborted save!\n" );
 
                        if ( $status->isOK() ) {
@@ -1869,11 +1858,11 @@ class WikiPage extends Page implements IDBAccessObject {
                        # Update links, etc.
                        $this->doEditUpdates( $revision, $user, array( 'created' => true ) );
 
-                       wfRunHooks( 'ArticleInsertComplete', array( &$this, &$user, $serialized, $summary,
-                               $flags & EDIT_MINOR, null, null, &$flags, $revision ) );
+                       $hook_args = array( &$this, &$user, $content, $summary,
+                                                               $flags & EDIT_MINOR, null, null, &$flags, $revision );
 
-                       wfRunHooks( 'ArticleContentInsertComplete', array( &$this, &$user, $content, $summary,
-                               $flags & EDIT_MINOR, null, null, &$flags, $revision ) );
+                       ContentHandler::runLegacyHooks( 'ArticleInsertComplete', $hook_args );
+                       wfRunHooks( 'ArticleContentInsertComplete', $hook_args );
                }
 
                # Do updates right now unless deferral was requested
@@ -1884,11 +1873,11 @@ class WikiPage extends Page implements IDBAccessObject {
                // Return the new revision (or null) to the caller
                $status->value['revision'] = $revision;
 
-               wfRunHooks( 'ArticleSaveComplete', array( &$this, &$user, $serialized, $summary,
-                       $flags & EDIT_MINOR, null, null, &$flags, $revision, &$status, $baseRevId ) );
+               $hook_args = array( &$this, &$user, $content, $summary,
+                                                       $flags & EDIT_MINOR, null, null, &$flags, $revision, &$status, $baseRevId );
 
-               wfRunHooks( 'ArticleContentSaveComplete', array( &$this, &$user, $content, $summary,
-                       $flags & EDIT_MINOR, null, null, &$flags, $revision, &$status, $baseRevId ) );
+               ContentHandler::runLegacyHooks( 'ArticleSaveComplete', $hook_args );
+               wfRunHooks( 'ArticleContentSaveComplete', $hook_args );
 
                # Promote user to any groups they meet the criteria for
                $user->addAutopromoteOnceGroups( 'onEdit' );
index f76d9f0..3a624a8 100644 (file)
@@ -510,13 +510,12 @@ class DifferenceEngine extends ContextSource {
                        $out->setRevisionTimestamp( $this->mNewRev->getTimestamp() );
                        $out->setArticleFlag( true );
 
-                       #NOTE: only needed for B/C: custom rendering of JS/CSS via hook
+                       // NOTE: only needed for B/C: custom rendering of JS/CSS via hook
                        if ( $this->mNewPage->isCssJsSubpage() || $this->mNewPage->isCssOrJsPage() ) {
                                // Stolen from Article::view --AG 2007-10-11
                                // Give hooks a chance to customise the output
                                // @TODO: standardize this crap into one function
-                               if ( !Hook::isRegistered( 'ShowRawCssJs' )
-                                       || wfRunHooks( 'ShowRawCssJs', array( ContentHandler::getContentText( $this->mNewContent ), $this->mNewPage, $out ) ) ) {
+                               if ( ContentHandler::runLegacyHooks( 'ShowRawCssJs', array( $this->mNewContent, $this->mNewPage, $out ) ) ) {
                                        // NOTE: deprecated hook, B/C only
                                        // use the content object's own rendering
                                        $po = $this->mContentObject->getParserOutput();
@@ -524,8 +523,7 @@ class DifferenceEngine extends ContextSource {
                                }
                        } elseif( !wfRunHooks( 'ArticleContentViewCustom', array( $this->mNewContent, $this->mNewPage, $out ) ) ) {
                                // Handled by extension
-                       } elseif( Hooks::isRegistered( 'ArticleViewCustom' )
-                                       && !wfRunHooks( 'ArticleViewCustom', array( ContentHandler::getContentText( $this->mNewContent ), $this->mNewPage, $out ) ) ) {
+                       } elseif( !ContentHandler::runLegacyHooks( 'ArticleViewCustom', array( $this->mNewContent, $this->mNewPage, $out ) ) ) {
                                // NOTE: deprecated hook, B/C only
                                // Handled by extension
                        } else {
index afeddf8..01bba65 100644 (file)
@@ -236,6 +236,26 @@ class ContentHandlerTest extends MediaWikiTestCase {
        public function testSupportsSections() {
                $this->markTestIncomplete( "not yet implemented" );
        }
+
+       public function testRunLegacyHooks() {
+               Hooks::register( 'testRunLegacyHooks', __CLASS__ . '::dummyHookHandler' );
+
+               $content = new WikitextContent( 'test text' );
+               $ok = ContentHandler::runLegacyHooks( 'testRunLegacyHooks', array( 'foo', &$content, 'bar' ) );
+
+               $this->assertTrue( $ok, "runLegacyHooks should have returned true" );
+               $this->assertEquals( "TEST TEXT", $content->getNativeData() );
+       }
+
+       public static function dummyHookHandler( $foo, &$text, $bar ) {
+               if ( $text === null || $text === false ) {
+                       return false;
+               }
+
+               $text = strtoupper( $text );
+
+               return true;
+       }
 }
 
 class DummyContentHandlerForTesting extends ContentHandler {
@@ -389,4 +409,3 @@ class DummyContentForTesting extends AbstractContent {
                return new ParserOutput( $this->getNativeData() );
        }
 }
-