jenkins-bot has submitted this change and it was merged.
Change subject: corrected issue where batch upload stops after 9th image
uploaded
......................................................................
corrected issue where batch upload stops after 9th image uploaded
* UploadHandler.php and MappingPhpAdapter.php were still using only
Title::makeTitleSafe(),
which has a problem creating a title like,
'Pantser Rups Tegen Luchtdoelen [PRTL] rupsvoertuig, met twee kanonnen 35 x
228' or
'GWToolset:Metadata Mappings/Dan-nl/Rijksmuseum.json'.
using \GWToolset\getTitle() and \GWToolset\stripIllegalTitleChars() took care
of the issue.
* while troubleshooting this issue i had to watch the job queue in a terminal
window and
noticed that while UploadMetadataJob.php was using error_log() to output any
errors,
Job.php prefers setting $this->setLastError().
* included some cosmetic changes
- added line breaks to code that is > 100 characters
- adjusted some documentation
- re-ordered some code
* 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(-)
Approvals:
BryanDavis: Looks good to me, approved
jenkins-bot: Verified
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: merged
Gerrit-Change-Id: I6dbd89482bdfa9e25339d66d41b42644c6d62994
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/GWToolset
Gerrit-Branch: master
Gerrit-Owner: Dan-nl <[email protected]>
Gerrit-Reviewer: BryanDavis <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
