jenkins-bot has submitted this change and it was merged.

Change subject: Drop dead and redundant special page code
......................................................................


Drop dead and redundant special page code

* SpecialPage::execute() is not supposed to return anything.
  Warning, this possibly causes the only WikibaseQuery special page to
  fail, please merge https://github.com/wmde/WikibaseQuery/pull/60 too!
* Drop redundant SpecialItemResolver class, all methods and properties
  do exist in the base class. Warning, please have a look at the
  message used in the ...->outputHeader( ... ) call, it changes from
  "<name>-summary" to "wikibase-<name>-summary" now. But none of these
  messages exist for these 3 special pages so it should not make a
  difference.
* Special page names are canonical, never multi-byte, right? No need
  to use Language::lc(), strtolower() is enough.
* Drop unused $subPage property.
* Add some docs.
* Fix wrong visibility level for getGroupName().

Change-Id: Iff0728ccaef50ec41dd7a58a431aee6ceec2a493
---
M client/includes/specials/SpecialUnconnectedPages.php
M repo/includes/specials/SpecialDispatchStats.php
M repo/includes/specials/SpecialEntitiesWithoutPage.php
M repo/includes/specials/SpecialEntityData.php
M repo/includes/specials/SpecialGoToLinkedPage.php
M repo/includes/specials/SpecialItemByTitle.php
M repo/includes/specials/SpecialItemDisambiguation.php
D repo/includes/specials/SpecialItemResolver.php
M repo/includes/specials/SpecialItemsWithoutSitelinks.php
M repo/includes/specials/SpecialListDatatypes.php
M repo/includes/specials/SpecialMergeItems.php
M repo/includes/specials/SpecialModifyEntity.php
M repo/includes/specials/SpecialMyLanguageFallbackChain.php
M repo/includes/specials/SpecialNewEntity.php
M repo/includes/specials/SpecialWikibasePage.php
M repo/includes/specials/SpecialWikibaseQueryPage.php
16 files changed, 73 insertions(+), 143 deletions(-)

Approvals:
  Hoo man: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/client/includes/specials/SpecialUnconnectedPages.php 
b/client/includes/specials/SpecialUnconnectedPages.php
index 455ad48..460472f 100644
--- a/client/includes/specials/SpecialUnconnectedPages.php
+++ b/client/includes/specials/SpecialUnconnectedPages.php
@@ -53,11 +53,18 @@
                parent::__construct( 'UnconnectedPages' );
        }
 
-       public function getGroupName() {
+       /**
+        * @see SpecialPage::getGroupName
+        *
+        * @return string
+        */
+       protected function getGroupName() {
                return 'wikibaseclient';
        }
 
        /**
+        * @see SpecialPage::getDescription
+        *
         * @return string
         */
        public function getDescription() {
@@ -73,13 +80,11 @@
        /**
         * @see SpecialPage::execute
         *
-        * @param null|string $subPage
+        * @param string|null $subPage
         */
        public function execute( $subPage ) {
                $this->setHeaders();
-
-               $contLang = $this->getContext()->getLanguage();
-               $this->outputHeader( $contLang->lc( 'wikibase-' . 
$this->getName() ) . '-summary' );
+               $this->outputHeader( 'wikibase-' . strtolower( $this->getName() 
) . '-summary' );
 
                // If the user is authorized, display the page, if not, show an 
error.
                if ( !$this->userCanExecute( $this->getUser() ) ) {
diff --git a/repo/includes/specials/SpecialDispatchStats.php 
b/repo/includes/specials/SpecialDispatchStats.php
index b040cad..050124e 100644
--- a/repo/includes/specials/SpecialDispatchStats.php
+++ b/repo/includes/specials/SpecialDispatchStats.php
@@ -56,6 +56,11 @@
                ) );
        }
 
+       /**
+        * @see SpecialWikibasePage::execute
+        *
+        * @param string|null $subPage
+        */
        public function execute( $subPage ) {
                parent::execute( $subPage );
 
diff --git a/repo/includes/specials/SpecialEntitiesWithoutPage.php 
b/repo/includes/specials/SpecialEntitiesWithoutPage.php
index e66e0ba..9975f0f 100644
--- a/repo/includes/specials/SpecialEntitiesWithoutPage.php
+++ b/repo/includes/specials/SpecialEntitiesWithoutPage.php
@@ -75,11 +75,11 @@
         * @see SpecialWikibasePage::execute
         *
         * @since 0.4
+        *
+        * @param string|null $subPage
         */
        public function execute( $subPage ) {
-               if ( !parent::execute( $subPage ) ) {
-                       return false;
-               }
+               parent::execute( $subPage );
 
                $this->prepareArguments( $subPage );
                $this->setForm();
@@ -87,8 +87,6 @@
                if ( $this->language !== '' ) {
                        $this->showQuery();
                }
-
-               return true;
        }
 
        /**
diff --git a/repo/includes/specials/SpecialEntityData.php 
b/repo/includes/specials/SpecialEntityData.php
index dbc3a06..f9571fb 100644
--- a/repo/includes/specials/SpecialEntityData.php
+++ b/repo/includes/specials/SpecialEntityData.php
@@ -132,14 +132,13 @@
        }
 
        /**
-        * Main method.
+        * @see SpecialWikibasePage::execute
         *
         * @since 0.4
         *
         * @param string|null $subPage
         *
         * @throws HttpError
-        * @return bool
         */
        public function execute( $subPage ) {
                $this->initDependencies();
@@ -148,12 +147,10 @@
                // TODO: Don't do this if HTML is not acceptable according to 
HTTP headers.
                if ( !$this->requestHandler->canHandleRequest( $subPage, 
$this->getRequest() ) ) {
                        $this->showForm();
-                       return true;
+                       return;
                }
 
                $this->requestHandler->handleRequest( $subPage, 
$this->getRequest(), $this->getOutput() );
-
-               return true;
        }
 
        /**
diff --git a/repo/includes/specials/SpecialGoToLinkedPage.php 
b/repo/includes/specials/SpecialGoToLinkedPage.php
index 5320405..3dce249 100644
--- a/repo/includes/specials/SpecialGoToLinkedPage.php
+++ b/repo/includes/specials/SpecialGoToLinkedPage.php
@@ -16,7 +16,7 @@
  * @licence GNU GPL v2+
  * @author Jan Zerebecki
  */
-class SpecialGoToLinkedPage extends SpecialItemResolver {
+class SpecialGoToLinkedPage extends SpecialWikibasePage {
 
        /**
         * @var SiteStore
@@ -29,7 +29,7 @@
        private $siteLinkLookup;
 
        /**
-        * @see SpecialItemResolver::__construct
+        * @see SpecialWikibasePage::__construct
         */
        public function __construct() {
                parent::__construct( 'GoToLinkedPage', '', true );
@@ -108,7 +108,9 @@
        }
 
        /**
-        * @see SpecialItemResolver::execute
+        * @see SpecialWikibasePage::execute
+        *
+        * @param string|null $subPage
         */
        public function execute( $subPage ) {
                parent::execute( $subPage );
diff --git a/repo/includes/specials/SpecialItemByTitle.php 
b/repo/includes/specials/SpecialItemByTitle.php
index 4e1cd2e..d1aa0b9 100644
--- a/repo/includes/specials/SpecialItemByTitle.php
+++ b/repo/includes/specials/SpecialItemByTitle.php
@@ -19,7 +19,7 @@
  * @author Jeroen De Dauw < [email protected] >
  * @author Daniel Kinzler
  */
-class SpecialItemByTitle extends SpecialItemResolver {
+class SpecialItemByTitle extends SpecialWikibasePage {
 
        /**
         * @var EntityTitleLookup
@@ -49,7 +49,7 @@
        private $groups;
 
        /**
-        * @see SpecialItemResolver::__construct
+        * @see SpecialWikibasePage::__construct
         *
         * @since 0.1
         */
@@ -105,9 +105,11 @@
        }
 
        /**
-        * @see SpecialItemResolver::execute
+        * @see SpecialWikibasePage::execute
         *
         * @since 0.1
+        *
+        * @param string|null $subPage
         */
        public function execute( $subPage ) {
                parent::execute( $subPage );
diff --git a/repo/includes/specials/SpecialItemDisambiguation.php 
b/repo/includes/specials/SpecialItemDisambiguation.php
index 3f8c9ef..291c8be 100644
--- a/repo/includes/specials/SpecialItemDisambiguation.php
+++ b/repo/includes/specials/SpecialItemDisambiguation.php
@@ -26,7 +26,7 @@
  * @author Jeroen De Dauw < [email protected] >
  * @author Daniel Kinzler
  */
-class SpecialItemDisambiguation extends SpecialItemResolver {
+class SpecialItemDisambiguation extends SpecialWikibasePage {
 
        /**
         * @var TermIndex
@@ -49,7 +49,7 @@
        private $limit;
 
        /**
-        * @see SpecialItemResolver::__construct
+        * @see SpecialWikibasePage::__construct
         *
         * @since 0.1
         */
@@ -81,14 +81,14 @@
        }
 
        /**
-        * @see SpecialItemResolver::execute
+        * @see SpecialWikibasePage::execute
         *
         * @since 0.1
+        *
+        * @param string|null $subPage
         */
        public function execute( $subPage ) {
-               if ( !parent::execute( $subPage ) ) {
-                       return false;
-               }
+               parent::execute( $subPage );
 
                // Setup
                $request = $this->getRequest();
@@ -123,8 +123,6 @@
                                $this->showNothingFound( $languageCode, $label 
);
                        }
                }
-
-               return true;
        }
 
        /**
diff --git a/repo/includes/specials/SpecialItemResolver.php 
b/repo/includes/specials/SpecialItemResolver.php
deleted file mode 100644
index 60eb3e2..0000000
--- a/repo/includes/specials/SpecialItemResolver.php
+++ /dev/null
@@ -1,64 +0,0 @@
-<?php
-
-namespace Wikibase\Repo\Specials;
-
-/**
- * Base for special pages that resolve certain arguments to an item.
- *
- * @since 0.1
- * @licence GNU GPL v2+
- * @author Jeroen De Dauw < [email protected] >
- */
-abstract class SpecialItemResolver extends SpecialWikibasePage {
-
-       // TODO: would we benefit from using cached page here?
-
-       /**
-        * @see SpecialWikibasePagePage::$subPage
-        *
-        * @since 0.1
-        */
-       public $subPage;
-
-       /**
-        * @see SpecialPage::getDescription
-        *
-        * @since 0.1
-        */
-       public function getDescription() {
-               return $this->msg( 'special-' . strtolower( $this->getName() ) 
)->text();
-       }
-
-       /**
-        * @see SpecialPage::setHeaders
-        *
-        * @since 0.1
-        */
-       public function setHeaders() {
-               $out = $this->getOutput();
-               $out->setArticleRelated( false );
-               $out->setPageTitle( $this->getDescription() );
-       }
-
-       /**
-        * @see SpecialPage::execute
-        *
-        * @since 0.1
-        */
-       public function execute( $subPage ) {
-               $subPage = is_null( $subPage ) ? '' : $subPage;
-               $this->subPage = trim( str_replace( '_', ' ', $subPage ) );
-
-               $this->setHeaders();
-               $this->outputHeader();
-
-               // If the user is authorized, display the page, if not, show an 
error.
-               if ( !$this->userCanExecute( $this->getUser() ) ) {
-                       $this->displayRestrictionError();
-                       return false;
-               }
-
-               return true;
-       }
-
-}
diff --git a/repo/includes/specials/SpecialItemsWithoutSitelinks.php 
b/repo/includes/specials/SpecialItemsWithoutSitelinks.php
index 283022a..9d1d61c 100644
--- a/repo/includes/specials/SpecialItemsWithoutSitelinks.php
+++ b/repo/includes/specials/SpecialItemsWithoutSitelinks.php
@@ -22,16 +22,12 @@
         *
         * @since 0.4
         *
-        * @param string $subPage
-        * @return bool
+        * @param string|null $subPage
         */
        public function execute( $subPage ) {
-               if ( !parent::execute( $subPage ) ) {
-                       return false;
-               }
+               parent::execute( $subPage );
 
                $this->showQuery();
-               return true;
        }
 
        /**
diff --git a/repo/includes/specials/SpecialListDatatypes.php 
b/repo/includes/specials/SpecialListDatatypes.php
index 087256e..2c675ed 100644
--- a/repo/includes/specials/SpecialListDatatypes.php
+++ b/repo/includes/specials/SpecialListDatatypes.php
@@ -21,6 +21,11 @@
                parent::__construct( 'ListDatatypes' );
        }
 
+       /**
+        * @see SpecialWikibasePage::execute
+        *
+        * @param string|null $subPage
+        */
        public function execute( $subPage ) {
                parent::execute( $subPage );
 
diff --git a/repo/includes/specials/SpecialMergeItems.php 
b/repo/includes/specials/SpecialMergeItems.php
index 358750f..216b8cf 100644
--- a/repo/includes/specials/SpecialMergeItems.php
+++ b/repo/includes/specials/SpecialMergeItems.php
@@ -127,13 +127,11 @@
        }
 
        /**
-        * Main method
+        * @see SpecialWikibasePage::execute
         *
         * @since 0.5
         *
-        * @param string $subPage
-        *
-        * @return boolean
+        * @param string|null $subPage
         */
        public function execute( $subPage ) {
                parent::execute( $subPage );
diff --git a/repo/includes/specials/SpecialModifyEntity.php 
b/repo/includes/specials/SpecialModifyEntity.php
index 2900cd7..0c76a92 100644
--- a/repo/includes/specials/SpecialModifyEntity.php
+++ b/repo/includes/specials/SpecialModifyEntity.php
@@ -60,18 +60,14 @@
        }
 
        /**
-        * Main method
+        * @see SpecialWikibasePage::execute
         *
         * @since 0.4
         *
-        * @param string $subPage
-        *
-        * @return boolean
+        * @param string|null $subPage
         */
        public function execute( $subPage ) {
-               if ( !parent::execute( $subPage ) ) {
-                       return false;
-               }
+               parent::execute( $subPage );
 
                $this->checkPermissions();
                $this->checkBlocked();
@@ -114,8 +110,6 @@
                                $this->getOutput()->redirect( $entityUrl );
                        }
                }
-
-               return true;
        }
 
        /**
diff --git a/repo/includes/specials/SpecialMyLanguageFallbackChain.php 
b/repo/includes/specials/SpecialMyLanguageFallbackChain.php
index fd079d9..a61f3be 100644
--- a/repo/includes/specials/SpecialMyLanguageFallbackChain.php
+++ b/repo/includes/specials/SpecialMyLanguageFallbackChain.php
@@ -38,7 +38,12 @@
                $this->factory = 
WikibaseRepo::getDefaultInstance()->getLanguageFallbackChainFactory();
        }
 
-       public function getGroupName() {
+       /**
+        * @see SpecialPage::getGroupName
+        *
+        * @return string
+        */
+       protected function getGroupName() {
                return 'wikibaserepo';
        }
 
@@ -46,7 +51,8 @@
         * @see SpecialPage::getDescription
         *
         * @since 0.4
-        * @return String
+        *
+        * @return string
         */
        public function getDescription() {
                // Message: special-mylanguagefallbackchain
@@ -86,11 +92,11 @@
        }
 
        /**
-        * Main method
+        * @see SpecialPage::execute
         *
         * @since 0.4
         *
-        * @param string $subPage
+        * @param string|null $subPage
         */
        public function execute( $subPage ) {
                $this->setHeaders();
diff --git a/repo/includes/specials/SpecialNewEntity.php 
b/repo/includes/specials/SpecialNewEntity.php
index 74db324..ce25b6f 100644
--- a/repo/includes/specials/SpecialNewEntity.php
+++ b/repo/includes/specials/SpecialNewEntity.php
@@ -55,8 +55,8 @@
        protected $rightsText;
 
        /**
-        * @param $name String: name of the special page, as seen in links and 
URLs
-        * @param $restriction String: user right required, 'createpage' per 
default.
+        * @param string $name Name of the special page, as seen in links and 
URLs.
+        * @param string $restriction User right required, 'createpage' per 
default.
         *
         * @since 0.1
         */
@@ -73,18 +73,14 @@
        }
 
        /**
-        * Main method.
+        * @see SpecialWikibasePage::execute
         *
         * @since 0.1
         *
         * @param string|null $subPage
-        *
-        * @return boolean
         */
        public function execute( $subPage ) {
-               if ( !parent::execute( $subPage ) ) {
-                       return false;
-               }
+               parent::execute( $subPage );
 
                $this->checkPermissions();
                $this->checkBlocked();
@@ -141,8 +137,6 @@
                }
 
                $this->createForm( $this->getLegend(), 
$this->additionalFormElements() );
-
-               return true;
        }
 
        /**
diff --git a/repo/includes/specials/SpecialWikibasePage.php 
b/repo/includes/specials/SpecialWikibasePage.php
index c979b1f..07253be 100644
--- a/repo/includes/specials/SpecialWikibasePage.php
+++ b/repo/includes/specials/SpecialWikibasePage.php
@@ -42,15 +42,11 @@
        }
 
        /**
-        * The subpage, ie the part after Special:PageName/
-        * Empty string if none is provided.
+        * @see SpecialPage::getGroupName
         *
-        * @since 0.1
-        * @var string
+        * @return string
         */
-       public $subPage;
-
-       public function getGroupName() {
+       protected function getGroupName() {
                return 'wikibaserepo';
        }
 
@@ -58,6 +54,8 @@
         * @see SpecialPage::getDescription
         *
         * @since 0.1
+        *
+        * @return string
         */
        public function getDescription() {
                return $this->msg( 'special-' . strtolower( $this->getName() ) 
)->text();
@@ -78,20 +76,17 @@
         * @see SpecialPage::execute
         *
         * @since 0.1
+        *
+        * @param string|null $subPage
         */
        public function execute( $subPage ) {
-               $subPage = is_null( $subPage ) ? '' : $subPage;
-               $this->subPage = trim( str_replace( '_', ' ', $subPage ) );
-
                $this->setHeaders();
-               $contLang = $this->getContext()->getLanguage();
-               $this->outputHeader( $contLang->lc( 'wikibase-' . 
$this->getName() ) . '-summary' );
+               $this->outputHeader( 'wikibase-' . strtolower( $this->getName() 
) . '-summary' );
 
                // If the user is authorized, display the page, if not, show an 
error.
                if ( !$this->userCanExecute( $this->getUser() ) ) {
                        $this->displayRestrictionError();
                }
-               return true;
        }
 
        /**
diff --git a/repo/includes/specials/SpecialWikibaseQueryPage.php 
b/repo/includes/specials/SpecialWikibaseQueryPage.php
index d53361e..9b34df1 100644
--- a/repo/includes/specials/SpecialWikibaseQueryPage.php
+++ b/repo/includes/specials/SpecialWikibaseQueryPage.php
@@ -60,15 +60,14 @@
         * @see SpecialWikibasePage::execute
         *
         * @since 0.5
+        *
+        * @param string|null $subPage
         */
        public function execute( $subPage ) {
-               if( !parent::execute( $subPage ) ) {
-                       return false;
-               }
+               parent::execute( $subPage );
 
                $output = $this->getOutput();
                $output->setSquidMaxage( static::CACHE_TTL_IN_SECONDS );
-               return true;
        }
 
        /**

-- 
To view, visit https://gerrit.wikimedia.org/r/179114
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Iff0728ccaef50ec41dd7a58a431aee6ceec2a493
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: Addshore <[email protected]>
Gerrit-Reviewer: Aude <[email protected]>
Gerrit-Reviewer: Hoo man <[email protected]>
Gerrit-Reviewer: Jeroen De Dauw <[email protected]>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to