Legoktm has uploaded a new change for review. https://gerrit.wikimedia.org/r/160199
Change subject: Get rid of the GadgetPageList class ...................................................................... Get rid of the GadgetPageList class The actual database updates are now taken care of by the DataUpdates. Instead of determineExtension(), we can now just check the page's contentmodel directly. GadgetHooks::cssOrJsPageImport() hasn't actually been needed since the ContentHandler switch, since the DataUpdates already run during imports. Change-Id: Idcbacb6836d0c1fe15bc0f04be5274f015dca0d2 --- M Gadgets.hooks.php M Gadgets.php D backend/GadgetPageList.php M content/GadgetCssContent.php M content/GadgetJsContent.php M content/GadgetScriptDeletionUpdate.php M content/GadgetScriptSecondaryDataUpdate.php M populateGadgetPageList.php A tests/GadgetHooksTest.php A tests/content/GadgetScriptSecondaryDataUpdateTest.php 10 files changed, 112 insertions(+), 106 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Gadgets refs/changes/99/160199/1 diff --git a/Gadgets.hooks.php b/Gadgets.hooks.php index ea02612..c57ca3f 100644 --- a/Gadgets.hooks.php +++ b/Gadgets.hooks.php @@ -43,7 +43,9 @@ */ public static function onContentHandlerDefaultModelFor( Title $title, &$model ) { if ( $title->inNamespace( NS_GADGET ) ) { - switch ( GadgetPageList::determineExtension( $title ) ) { + preg_match( '!\.(css|js)$!u', $title->getText(), $ext ); + $ext = $ext ? $ext[1]: ''; + switch ( $ext ) { case 'js': $model = 'GadgetJs'; return false; @@ -108,17 +110,27 @@ return true; } - public static function onTitleMoveComplete( $oldTitle, $newTitle, $user, $pageid, $redirid ) { - // Delete the old title from the list. Even if it still exists after the move, - // it'll be a redirect and we don't want those in there - GadgetPageList::delete( $oldTitle ); + /** + * Really, this should all be taken care of by core, except move page + * logic is icky, and it doesn't. So until it does, run the appropriate + * updates as necessary. + */ + public static function onTitleMoveComplete( Title $oldTitle, Title $newTitle ) { + if ( $oldTitle->inNamespace( NS_GADGET ) ) { + $delUpd = new GadgetScriptDeletionUpdate( $oldTitle ); + $delUpd->doUpdate(); + } - GadgetPageList::updatePageStatus( $newTitle ); - return true; - } + if ( $newTitle->inNamespace( NS_GADGET ) ) { + if ( $newTitle->hasContentModel( 'GadgetCss' ) ) { + $secUpd = new GadgetScriptSecondaryDataUpdate( $newTitle, 'css' ); + $secUpd->doUpdate(); + } elseif ( $newTitle->hasContentModel( 'GadgetJs' ) ) { + $secUpd = new GadgetScriptSecondaryDataUpdate( $newTitle, 'js' ); + $secUpd->doUpdate(); + } + } - public static function cssOrJsPageImport( $title, $origTitle, $revCount, $sRevCount, $pageInfo ) { - GadgetPageList::updatePageStatus( $title ); return true; } diff --git a/Gadgets.php b/Gadgets.php index 47eec88..762d779 100644 --- a/Gadgets.php +++ b/Gadgets.php @@ -98,7 +98,6 @@ #$wgGroupPermissions['gadgetmanagers']['gadgets-definition-delete'] = true; $wgHooks['AfterImportPage'][] = 'GadgetsHooks::gadgetDefinitionImport'; -$wgHooks['AfterImportPage'][] = 'GadgetsHooks::cssOrJsPageImport'; $wgHooks['ArticleUndelete'][] = 'GadgetsHooks::gadgetDefinitionUndelete'; $wgHooks['BeforePageDisplay'][] = 'GadgetsHooks::onBeforePageDisplay'; $wgHooks['MakeGlobalVariablesScript'][] = 'GadgetsHooks::onMakeGlobalVariablesScript'; @@ -133,7 +132,6 @@ $wgAutoloadClasses['ForeignDBGadgetRepo'] = $dir . 'backend/ForeignDBGadgetRepo.php'; $wgAutoloadClasses['Gadget'] = $dir . 'backend/Gadget.php'; $wgAutoloadClasses['GadgetsHooks'] = $dir . 'Gadgets.hooks.php'; -$wgAutoloadClasses['GadgetPageList'] = $dir . 'backend/GadgetPageList.php'; $wgAutoloadClasses['GadgetRepo'] = $dir . 'backend/GadgetRepo.php'; $wgAutoloadClasses['GadgetRepoFactory'] = $dir . 'backend/GadgetRepoFactory.php'; $wgAutoloadClasses['GadgetResourceLoaderModule'] = $dir . 'backend/GadgetResourceLoaderModule.php'; diff --git a/backend/GadgetPageList.php b/backend/GadgetPageList.php deleted file mode 100644 index 640086d..0000000 --- a/backend/GadgetPageList.php +++ /dev/null @@ -1,85 +0,0 @@ -<?php -/** - * Gadgets extension - lets users select custom javascript gadgets - * - * - * For more info see http://mediawiki.org/wiki/Extension:Gadgets - * - * @file - * @ingroup Extensions - * @author Roan Kattouw - * @copyright © 2011 Roan Kattouw - * @license GNU General Public Licence 2.0 or later - */ - -/** - * Class with static methods for accessing the gadgetpagelist table - */ -class GadgetPageList { - /** - * Determine the extension of a title ('css' or 'js') - * @param Title $title - * @return string The extension of the title, or empty string - */ - public static function determineExtension( Title $title ) { - $m = null; - preg_match( '!\.(css|js)$!u', $title->getText(), $m ); - return isset( $m[1] ) ? $m[1] : ''; - } - - /** - * Get a row for the gadgetpagelist table - * @param Title $title - * @return array Database row - */ - public static function getRowForTitle( Title $title ) { - return array( - 'gpl_extension' => self::determineExtension( $title ), - 'gpl_namespace' => $title->getNamespace(), - 'gpl_title' => $title->getDBKey() - ); - } - - /** - * Update the status of a title, typically called when a title has been - * edited or created. - * - * If $title is a CSS/JS page and not a redirect, it is added to the table. - * If it is a CSS/JS page but is a redirect, it is removed from the table. - * If it's not a CSS/JS page, it's assumed never to have been added to begin with, so nothing happens/ - * @param Title $title - */ - public static function updatePageStatus( Title $title ) { - if ( $title->isCssOrJsPage() || $title->isCssJsSubpage() ) { - if ( $title->isRedirect() ) { - self::delete( $title ); - } else { - self::add( $title ); - } - } - } - - /** - * Add a title to the gadgetpagelist table - * @param Title $title - */ - public static function add( Title $title ) { - $dbw = wfGetDB( DB_MASTER ); - $dbw->insert( 'gadgetpagelist', self::getRowForTitle( $title ), - __METHOD__, array( 'IGNORE' ) - ); - } - - /** - * Delete a title from the gadgetpagelist table - * @param Title $title - */ - public static function delete( Title $title ) { - $dbw = wfGetDB( DB_MASTER ); - $dbw->delete( 'gadgetpagelist', array( - 'gpl_namespace' => $title->getNamespace(), - 'gpl_title' => $title->getDBKey() - ), __METHOD__ - ); - } -} diff --git a/content/GadgetCssContent.php b/content/GadgetCssContent.php index efdfceb..795ae44 100644 --- a/content/GadgetCssContent.php +++ b/content/GadgetCssContent.php @@ -30,7 +30,7 @@ ) { return array_merge( parent::getSecondaryDataUpdates( $title, $old, $recursive, $parserOutput ), - array( new GadgetScriptSecondaryDataUpdate( $title ) ) + array( new GadgetScriptSecondaryDataUpdate( $title, 'css' ) ) ); } } diff --git a/content/GadgetJsContent.php b/content/GadgetJsContent.php index a241d6d..f6fba1e 100644 --- a/content/GadgetJsContent.php +++ b/content/GadgetJsContent.php @@ -30,7 +30,7 @@ ) { return array_merge( parent::getSecondaryDataUpdates( $title, $old, $recursive, $parserOutput ), - array( new GadgetScriptSecondaryDataUpdate( $title ) ) + array( new GadgetScriptSecondaryDataUpdate( $title, 'js' ) ) ); } } diff --git a/content/GadgetScriptDeletionUpdate.php b/content/GadgetScriptDeletionUpdate.php index ec4c98d..d8384b0 100644 --- a/content/GadgetScriptDeletionUpdate.php +++ b/content/GadgetScriptDeletionUpdate.php @@ -10,6 +10,13 @@ } public function doUpdate() { - GadgetPageList::delete( $this->title ); + wfGetDB( DB_MASTER )->delete( + 'gadgetpagelist', + array( + 'gpl_title' => $this->title->getDBkey(), + 'gpl_namespace' => $this->title->getNamespace(), + ), + __METHOD__ + ); } } diff --git a/content/GadgetScriptSecondaryDataUpdate.php b/content/GadgetScriptSecondaryDataUpdate.php index 6bab977..3dc38f8 100644 --- a/content/GadgetScriptSecondaryDataUpdate.php +++ b/content/GadgetScriptSecondaryDataUpdate.php @@ -5,11 +5,31 @@ */ class GadgetScriptSecondaryDataUpdate extends DataUpdate { - public function __construct( Title $title ) { + /** + * @param Title $title + * @param string $type either "css" or "js" + * @throws InvalidArgumentException + */ + public function __construct( Title $title, $type ) { $this->title = $title; + + if ( !in_array( $type, array( 'css', 'js' ) ) ) { + throw new InvalidArgumentException( "$type is not a valid Gadget script type" ); + } + $this->type = $type; } + public function doUpdate() { - GadgetPageList::updatePageStatus( $this->title ); + wfGetDB( DB_MASTER )->insert( + 'gadgetpagelist', + array( + 'gpl_title' => $this->title->getDBkey(), + 'gpl_namespace' => $this->title->getNamespace(), + 'gpl_extension' => $this->type, + ), + __METHOD__, + array( 'IGNORE' ) + ); } } \ No newline at end of file diff --git a/populateGadgetPageList.php b/populateGadgetPageList.php index 659221f..54581d4 100644 --- a/populateGadgetPageList.php +++ b/populateGadgetPageList.php @@ -50,11 +50,21 @@ $gplRows = array(); foreach ( $res as $row ) { $title = Title::newFromRow( $row ); - if ( $title->hasContentModel( 'GadgetJs' ) - || $title->hasContentModel( 'GadgetCss' ) - ) { - $gplRows[] = GadgetPageList::getRowForTitle( $title ); + $ext = false; + if ( $title->hasContentModel( 'GadgetJs' ) ) { + $ext = 'js'; + } elseif ( $title->hasContentModel( 'GadgetCss' ) ) { + $ext = 'css'; } + + if ( $ext ) { + $gplRows[] = array( + 'gpl_extension' => $ext, + 'gpl_title' => $title->getDBkey(), + 'gpl_namespace' => $title->getNamespace(), + ); + } + $lastPageID = intval( $row->page_id ); } $dbr->freeResult( $res ); diff --git a/tests/GadgetHooksTest.php b/tests/GadgetHooksTest.php new file mode 100644 index 0000000..da4dd50 --- /dev/null +++ b/tests/GadgetHooksTest.php @@ -0,0 +1,29 @@ +<?php + +/** + * @group Gadgets + * @group Extensions + */ +class GadgetHooksTest extends MediaWikiTestCase { + + /** + * @covers GadgetsHooks::onContentHandlerDefaultModelFor + * @dataProvider provideOnContentHandlerDefaultModelFor + */ + public function testOnContentHandlerDefaultModelFor( $text, $expected, $desc ) { + $title = Title::newFromText( $text ); + $model = null; + GadgetsHooks::onContentHandlerDefaultModelFor( $title, $model ); + $this->assertEquals( $expected, $model, $desc ); + } + + public static function provideOnContentHandlerDefaultModelFor() { + return array( + array( 'Gadget:Foo.js', 'GadgetJs', 'Gadget page with a .js extension' ), + array( 'Gadget:Foo.css', 'GadgetCss', 'Gadget page with a .css extension' ), + array( 'Gadget:Foo', null, 'Gadget page with no extension' ), + array( 'Gadget:Foo.ext', null, 'Gadget page with a non-css/js extension'), + array( 'Main Page', null, 'Mainspace page' ), + ); + } +} \ No newline at end of file diff --git a/tests/content/GadgetScriptSecondaryDataUpdateTest.php b/tests/content/GadgetScriptSecondaryDataUpdateTest.php new file mode 100644 index 0000000..01e4ac5 --- /dev/null +++ b/tests/content/GadgetScriptSecondaryDataUpdateTest.php @@ -0,0 +1,15 @@ +<?php + +class GadgetScriptSecondaryDataUpdateTest extends MediaWikiTestCase { + + /** + * @covers GadgetScriptSecondaryDataUpdate::__construct + */ + public function testConstructor() { + $title = $this->getMock( 'Title' ); + $upd = new GadgetScriptSecondaryDataUpdate( $title, 'css' ); + $this->assertInstanceOf( 'GadgetScriptSecondaryDataUpdate', $upd ); + $this->setExpectedException( 'InvalidArgumentException' ); + new GadgetScriptSecondaryDataUpdate( $title, 'notcssnorjs' ); + } +} \ No newline at end of file -- To view, visit https://gerrit.wikimedia.org/r/160199 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Idcbacb6836d0c1fe15bc0f04be5274f015dca0d2 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Gadgets Gerrit-Branch: RL2 Gerrit-Owner: Legoktm <legoktm.wikipe...@gmail.com> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits