Dan-nl has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/90147


Change subject: corrected issue where batch upload stops after 9th image 
uploaded
......................................................................

corrected issue where batch upload stops after 9th image uploaded

* includes/Adapters/Php/MappingPhpAdapter.php
  - added line breaks to code that is > 100 characters
  - changed use of Title::makeTitleSafe to \GWToolset\getTitle() and 
\GWToolset\stripIllegalTitleChars()

* includes/functions/functions.php
  - adjusted some documentation

* includes/Handlers/UploadHandler.php
  - changed use of Title::makeTitleSafe to \GWToolset\getTitle() and 
\GWToolset\stripIllegalTitleChars()
  - created a getTitle() method for shared use of that logic within the class
  - adjusted some documentation

* includes/Jobs/UploadMetadataJob.php
  - changed use of error_log() to $this->setLastError()
  - re-located validateParams() within the class

* re: ticket #173 in assembla - Batch upload stops after 9th image uploaded

Change-Id: I6dbd89482bdfa9e25339d66d41b42644c6d62994
---
M includes/Adapters/Php/MappingPhpAdapter.php
M includes/Handlers/UploadHandler.php
M includes/Jobs/UploadMetadataJob.php
M includes/functions/functions.php
4 files changed, 102 insertions(+), 67 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/GWToolset 
refs/changes/47/90147/1

diff --git a/includes/Adapters/Php/MappingPhpAdapter.php 
b/includes/Adapters/Php/MappingPhpAdapter.php
index a7c06b6..92d7f2a 100644
--- a/includes/Adapters/Php/MappingPhpAdapter.php
+++ b/includes/Adapters/Php/MappingPhpAdapter.php
@@ -53,20 +53,27 @@
                        );
                }
 
-               // nb: cannot filter the json - might need to test it as valid 
by converting it back and forth with json_decode/encode
+               // nb: cannot filter the json - might need to test it as valid 
by converting it back and
+               // forth with json_decode/encode
                $options['text'] = $options['mapping-json'];
                $options['mapping-user-name'] = $options['user']->getName();
+
                $options['summary'] =
                        wfMessage( 'gwtoolset-create-mapping' )
                                ->params( Config::$name, 
$options['mapping-user-name'] )
                                ->escaped();
-               $options['title'] =
-                       Title::makeTitleSafe(
-                               Config::$metadata_namespace,
+
+               $options['title'] =     \GWToolset\getTitle(
+                       \GWToolset\stripIllegalTitleChars(
                                Config::$metadata_mapping_subpage . '/' .
-                                       $options['mapping-user-name'] . '/' .
-                                       $options['mapping-name'] . '.json'
-                       );
+                               $options['mapping-user-name'] . '/' .
+                               $options['mapping-name'] . '.json',
+                               array( 'allow-subpage' => true )
+                       ),
+                       Config::$metadata_namespace,
+                       array( 'must-be-known' => false )
+               );
+
                $result = $this->saveMapping( $options );
 
                return $result;
@@ -95,7 +102,8 @@
 
                        $Mapping_Page = new WikiPage( 
$options['Metadata-Mapping-Title'] );
                        $result = $Mapping_Page->getContent( Revision::RAW 
)->getNativeData();
-                       $result = str_replace( PHP_EOL, '', $result ); // need 
to remove line breaks from the mapping otherwise the json_decode will error out
+                       // need to remove line breaks from the mapping 
otherwise the json_decode will error out
+                       $result = str_replace( PHP_EOL, '', $result );
                }
 
                return $result;
@@ -122,7 +130,13 @@
                $Mapping_Page = new WikiPage( $options['title'] );
 
                set_error_handler( '\GWToolset\swallowErrors' );
-               $result = $Mapping_Page->doEditContent( $Mapping_Content, 
$options['summary'], 0, false, $options['user'] );
+               $result = $Mapping_Page->doEditContent(
+                       $Mapping_Content,
+                       $options['summary'],
+                       0,
+                       false,
+                       $options['user']
+               );
 
                return $result;
        }
diff --git a/includes/Handlers/UploadHandler.php 
b/includes/Handlers/UploadHandler.php
index 8faddbf..306c481 100644
--- a/includes/Handlers/UploadHandler.php
+++ b/includes/Handlers/UploadHandler.php
@@ -401,6 +401,28 @@
        }
 
        /**
+        * @param {Array} $options
+        * @throws {MWException}
+        * @return {Title}
+        */
+       protected function getTitle( array &$options ) {
+               $result = \GWToolset\getTitle(
+                       \GWToolset\stripIllegalTitleChars( $options['title'] ),
+                       Config::$mediafile_namespace,
+                       array( 'must-be-known' => false )
+               );
+
+               if ( !( $result instanceof Title ) ) {
+                       throw new MWException(
+                               wfMessage( 'gwtoolset-title-bad' )
+                                       ->params( $options['title'] )->parse()
+                       );
+               }
+
+               return $result;
+       }
+
+       /**
         * retrieves the metadata file via :
         * - a url to the local wiki
         * - or the uploaded file given in the $_POST'ed form
@@ -515,14 +537,17 @@
                $this->augmentAllowedExtensions( 
Config::$accepted_metadata_types );
                WikiChecks::increaseHTTPTimeout( 120 );
 
-               $Metadata_Title =
-                       Title::makeTitleSafe(
-                               Config::$metadata_namespace,
+               $Metadata_Title = \GWToolset\getTitle(
+                       \GWToolset\stripIllegalTitleChars(
                                Config::$metadata_sets_subpage . '/' .
-                                       $this->_User->getName() . '/' .
-                                       $this->_File->pathinfo['filename'] .
-                                       '.' . 
$this->_File->pathinfo['extension']
-                       );
+                               $this->_User->getName() . '/' .
+                               $this->_File->pathinfo['filename'] .
+                               '.' . $this->_File->pathinfo['extension'],
+                               array( 'allow-subpage' => true )
+                       ),
+                       Config::$metadata_namespace,
+                       array( 'must-be-known' => false )
+               );
 
                $text = file_get_contents( $this->_File->tmp_name );
                $Metadata_Content = ContentHandler::makeContent( $text, 
$Metadata_Title );
@@ -620,19 +645,7 @@
                $Status = null;
                WikiChecks::increaseHTTPTimeout();
                $this->validatePageOptions( $options );
-
-               $Title = \GWToolset\getTitle(
-                       \GWToolset\stripIllegalTitleChars( $options['title'] ),
-                       Config::$mediafile_namespace,
-                       array( 'must-be-known' => false )
-               );
-
-               if ( !( $Title instanceof Title ) ) {
-                       throw new MWException(
-                               wfMessage( 'gwtoolset-title-bad' )
-                                       ->params( $options['title'] )->parse()
-                       );
-               }
+               $Title = $this->getTitle( $options );
 
                if ( !$Title->isKnown() ) {
                        $Status = $this->uploadMediaFileViaUploadFromUrl( 
$options, $Title );
@@ -669,11 +682,11 @@
                        return;
                }
 
+               $this->validatePageOptions( $options );
+               $Title = $this->getTitle( $options );
+
                $job = new UploadMediafileJob(
-                       Title::makeTitleSafe(
-                               Config::$mediafile_namespace,
-                               $options['title']
-                       ),
+                       $Title,
                        array(
                                'comment' => $options['comment'],
                                'ignorewarnings' => $options['ignorewarnings'],
@@ -760,19 +773,17 @@
        }
 
        /**
+        * makes sure that the following values are present
+        *   - title
+        *   - ignorewarnings
+        *   - text
+        *   - url-to-the-media-file
+        *
         * @param {array} $options
         * @throws {MWException}
         * @return {void}
         */
        protected function validatePageOptions( array &$options ) {
-               if ( empty( $options['title'] ) ) {
-                       throw new MWException(
-                               wfMessage( 'gwtoolset-developer-issue' )
-                                       ->params( wfMessage( 
'gwtoolset-no-title' )->escaped() )
-                                       ->parse()
-                       );
-               }
-
                if ( !isset( $options['ignorewarnings'] ) ) {
                        throw new MWException(
                                wfMessage( 'gwtoolset-developer-issue' )
@@ -788,6 +799,12 @@
                                        ->params( wfMessage( 
'gwtoolset-no-text' )->escaped() )
                                        ->parse()
                        );
+               }if ( empty( $options['title'] ) ) {
+                       throw new MWException(
+                               wfMessage( 'gwtoolset-developer-issue' )
+                                       ->params( wfMessage( 
'gwtoolset-no-title' )->escaped() )
+                                       ->parse()
+                       );
                }
 
                if ( empty( $options['url-to-the-media-file'] ) ) {
diff --git a/includes/Jobs/UploadMetadataJob.php 
b/includes/Jobs/UploadMetadataJob.php
index aed02a6..1b3c25d 100644
--- a/includes/Jobs/UploadMetadataJob.php
+++ b/includes/Jobs/UploadMetadataJob.php
@@ -44,25 +44,6 @@
        }
 
        /**
-        * @return {bool}
-        */
-       protected function validateParams() {
-               $result = true;
-
-               if ( empty( $this->params['username'] ) ) {
-                       error_log( __METHOD__ . ' : no $this->params[\'user\'] 
provided' . PHP_EOL );
-                       $result = false;
-               }
-
-               if ( empty( $this->params['post'] ) ) {
-                       error_log( __METHOD__ . ' : no $this->params[\'post\'] 
provided' . PHP_EOL );
-                       $result = false;
-               }
-
-               return $result;
-       }
-
-       /**
         * entry point
         * @todo: should $result always be true?
         * @return {bool|array}
@@ -82,7 +63,26 @@
                try {
                        $result = 
$this->_MetadataMappingHandler->processRequest();
                } catch ( MWException $e ) {
-                       error_log( $e->getMessage() );
+                       $this->setLastError( __METHOD__ . ': ' . 
$e->getMessage() );
+               }
+
+               return $result;
+       }
+
+       /**
+        * @return {bool}
+        */
+       protected function validateParams() {
+               $result = true;
+
+               if ( empty( $this->params['username'] ) ) {
+                       $this->setLastError( __METHOD__ . ': no 
$this->params[\'user\'] provided' );
+                       $result = false;
+               }
+
+               if ( empty( $this->params['post'] ) ) {
+                       $this->setLastError( __METHOD__ . ': no 
$this->params[\'post\'] provided' );
+                       $result = false;
                }
 
                return $result;
diff --git a/includes/functions/functions.php b/includes/functions/functions.php
index cc08917..c778330 100644
--- a/includes/functions/functions.php
+++ b/includes/functions/functions.php
@@ -72,10 +72,11 @@
  * attempts to retrieve a wiki title based on a given page title, an
  * optional namespace requirement and whether or not the title must be known
  *
- * @param {string} $page_title
- * @param {int} $namespace
- * @param {array} $options
- *   {boolean} $options['must-be-known']
+ * @param {String} $page_title
+ * @param {Int} $namespace
+ * @param {Array} $options
+ *   {Boolean} $options['must-be-known']
+ *   Whether or not the Title must be known; defaults to true
  *
  * @throws {MWException}
  * @return {null|Title}
@@ -149,8 +150,11 @@
  * @param {string} $title
  *
  * @param {array} $options
- *   {boolean} $options['allow-subpage'] allows for the ‘/’ subpage character
- *    {string} $options['replacement']
+ *   {Boolean} $options['allow-subpage']
+ *   allows for the ‘/’ subpage character
+ *
+ *   {String} $options['replacement']
+ *   the character used to replace illegal characters; defaults to ‘-’
  *
  * @return {string} the string is not filtered
  */

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6dbd89482bdfa9e25339d66d41b42644c6d62994
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/GWToolset
Gerrit-Branch: master
Gerrit-Owner: Dan-nl <[email protected]>

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

Reply via email to