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

Reply via email to