Addshore has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/363020 )
Change subject: Allow recovery from invalid chars in title exception
......................................................................
Allow recovery from invalid chars in title exception
Change-Id: Ib99aa3e105370dfc9fb5564d4cf35b254e6b5684
---
M extension.json
M i18n/en.json
M i18n/qqq.json
A src/Exceptions/RecoverableTitleImportException.php
D src/Exceptions/TitleConflictException.php
M src/Exceptions/TitleException.php
A src/Html/RecoverableTitleExceptionPage.php
D src/Html/TitleConflictPage.php
M src/Services/ImportPlanValidator.php
M src/SpecialImportFile.php
M tests/phpunit/Services/ImportPlanValidatorTest.php
11 files changed, 160 insertions(+), 127 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/FileImporter
refs/changes/20/363020/1
diff --git a/extension.json b/extension.json
index 8b0b701..898b298 100644
--- a/extension.json
+++ b/extension.json
@@ -37,9 +37,9 @@
"FileImporter\\Exceptions\\ImportException":
"src/Exceptions/ImportException.php",
"FileImporter\\Exceptions\\InvalidArgumentException":
"src/Exceptions/InvalidArgumentException.php",
"FileImporter\\Exceptions\\LocalizedImportException":
"src/Exceptions/LocalizedImportException.php",
+ "FileImporter\\Exceptions\\RecoverableTitleImportException":
"src/Exceptions/RecoverableTitleImportException.php",
"FileImporter\\Exceptions\\SourceUrlException":
"src/Exceptions/SourceUrlException.php",
"FileImporter\\Exceptions\\TitleException":
"src/Exceptions/TitleException.php",
- "FileImporter\\Exceptions\\TitleConflictException":
"src/Exceptions/TitleConflictException.php",
"FileImporter\\Interfaces\\DetailRetriever":
"src/Interfaces/DetailRetriever.php",
"FileImporter\\Interfaces\\ImportTitleChecker":
"src/Interfaces/ImportTitleChecker.php",
"FileImporter\\Interfaces\\SourceUrlChecker":
"src/Interfaces/SourceUrlChecker.php",
@@ -59,7 +59,7 @@
"FileImporter\\Html\\ImportPreviewPage":
"src/Html/ImportPreviewPage.php",
"FileImporter\\Html\\ImportSuccessPage":
"src/Html/ImportSuccessPage.php",
"FileImporter\\Html\\InputFormPage":
"src/Html/InputFormPage.php",
- "FileImporter\\Html\\TitleConflictPage":
"src/Html/TitleConflictPage.php",
+ "FileImporter\\Html\\RecoverableTitleExceptionPage":
"src/Html/RecoverableTitleExceptionPage.php",
"FileImporter\\Html\\TextRevisionSnippet":
"src/Html/TextRevisionSnippet.php",
"FileImporter\\Remote\\MediaWiki\\ApiDetailRetriever":
"src/Remote/MediaWiki/ApiDetailRetriever.php",
"FileImporter\\Remote\\MediaWiki\\HttpApiLookup":
"src/Remote/MediaWiki/HttpApiLookup.php",
diff --git a/i18n/en.json b/i18n/en.json
index 4eccc05..aaf1655 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -21,6 +21,7 @@
"fileimporter-badimporthash": "Import hash does not match. Please
restart the import.",
"fileimporter-localtitleexists" : "The title you are currently trying
to import to already exists on this wiki.",
"fileimporter-sourcetitleexists" : "The title you are currently trying
to import to already exists on the source wiki.",
+ "fileimporter-illegalfilenamechars" : "The title you are currently
trying to import to contains illegal filename characters.",
"fileimporter-duplicatefilesdetected" : "The file you are currently
trying to import already exists on this wiki.",
"fileimporter-duplicatefilesdetected-prefix" : "Duplicates",
"fileimporter-exampleprefix": "Example",
diff --git a/i18n/qqq.json b/i18n/qqq.json
index ec5c210..0f3c649 100644
--- a/i18n/qqq.json
+++ b/i18n/qqq.json
@@ -23,6 +23,7 @@
"fileimporter-badimporthash": "Error message shown on the special page
when the import hash for the request does not match the expected hash.",
"fileimporter-localtitleexists": "Error message shown on the special
page when the title to be imported to already exists on the local wiki.",
"fileimporter-sourcetitleexists": "Error message shown on the special
page when the title to be imported to already exists on the source wiki.",
+ "fileimporter-illegalfilenamechars" : "Error message shown on the
special page when the title to be imported to contains illegal characters as
defined by MediaWiki.",
"fileimporter-duplicatefilesdetected": "Error message shown on the
special page when the file to be imported has been detected as already existing
on the local wiki.",
"fileimporter-duplicatefilesdetected-prefix": "Prefix for the duplicate
file that has been found.\n{{Identical|Duplicate}}",
"fileimporter-exampleprefix": "Prefix for the example URL contained
within the URL text box on the special page.\n{{Identical|Example}}",
diff --git a/src/Exceptions/RecoverableTitleImportException.php
b/src/Exceptions/RecoverableTitleImportException.php
new file mode 100644
index 0000000..599424a
--- /dev/null
+++ b/src/Exceptions/RecoverableTitleImportException.php
@@ -0,0 +1,32 @@
+<?php
+
+namespace FileImporter\Exceptions;
+
+use FileImporter\Data\ImportPlan;
+use MessageSpecifier;
+
+/**
+ * Exception thrown when an import has an issue with the planned title that
can be
+ * resolved by the user.
+ */
+class RecoverableTitleImportException extends LocalizedImportException {
+
+ /**
+ * @var ImportPlan
+ */
+ private $importPlan;
+
+ /**
+ * @param string|array|MessageSpecifier $messageSpec See
Message::newFromSpecifier
+ * @param ImportPlan $importPlan ImportPlan to recover the import of.
+ */
+ public function __construct( $messageSpec, ImportPlan $importPlan ) {
+ $this->importPlan = $importPlan;
+ parent::__construct( $messageSpec );
+ }
+
+ public function getImportPlan() {
+ return $this->importPlan;
+ }
+
+}
diff --git a/src/Exceptions/TitleConflictException.php
b/src/Exceptions/TitleConflictException.php
deleted file mode 100644
index 1fc7bd4..0000000
--- a/src/Exceptions/TitleConflictException.php
+++ /dev/null
@@ -1,38 +0,0 @@
-<?php
-
-namespace FileImporter\Exceptions;
-
-use FileImporter\Data\ImportPlan;
-
-class TitleConflictException extends ImportException {
-
- const LOCAL_TITLE = 1;
- const REMOTE_TITLE = 2;
-
- private $importPlan;
- private $conflictType;
-
- /**
- * TitleConflictException constructor.
- *
- * @param ImportPlan $importPlan
- * @param int $conflictType Either LOCAL_TITLE or REMOTE_TITLE
- */
- public function __construct( ImportPlan $importPlan, $conflictType ) {
- $this->importPlan = $importPlan;
- $this->conflictType = $conflictType;
- parent::__construct( 'Title conflict detected' );
- }
-
- public function getImportPlan() {
- return $this->importPlan;
- }
-
- /**
- * @return int
- */
- public function getType() {
- return $this->conflictType;
- }
-
-}
diff --git a/src/Exceptions/TitleException.php
b/src/Exceptions/TitleException.php
index 2cdd538..a3240d8 100644
--- a/src/Exceptions/TitleException.php
+++ b/src/Exceptions/TitleException.php
@@ -2,6 +2,36 @@
namespace FileImporter\Exceptions;
+use Throwable;
+
class TitleException extends ImportException {
+ /**
+ * The user could recover this title exception by changing the title.
+ */
+ const IS_RECOVERABLE = 1;
+
+ /**
+ * The user can not recover this title exception by changing the title.
+ */
+ const IS_NOT_RECOVERABLE = 0;
+
+ /**
+ * TitleException constructor.
+ *
+ * @param string $message
+ * @param int $code Either IS_RECOVERABLE or IS_NOT_RECOVERABLE
+ * @param Throwable|null $previous
+ */
+ public function __construct( $message = "", $code =
self::IS_NOT_RECOVERABLE, Throwable $previous = null ) {
+ parent::__construct( $message, $code, $previous );
+ }
+
+ /**
+ * @return bool Can the user recover this TitleException?
+ */
+ public function isRecoverable() {
+ return $this->code === self::IS_RECOVERABLE;
+ }
+
}
diff --git a/src/Html/RecoverableTitleExceptionPage.php
b/src/Html/RecoverableTitleExceptionPage.php
new file mode 100644
index 0000000..5714c80
--- /dev/null
+++ b/src/Html/RecoverableTitleExceptionPage.php
@@ -0,0 +1,55 @@
+<?php
+
+namespace FileImporter\Html;
+
+use FileImporter\Exceptions\RecoverableTitleImportException;
+use Html;
+use SpecialPage;
+
+/**
+ * Html showing an error and the ChangeTitleForm
+ */
+class RecoverableTitleExceptionPage {
+
+ /**
+ * @var SpecialPage
+ */
+ private $specialPage;
+
+ /**
+ * @var RecoverableTitleImportException
+ */
+ private $exception;
+
+ /**
+ * @param SpecialPage $specialPage
+ * @param RecoverableTitleImportException $exception
+ */
+ public function __construct(
+ SpecialPage $specialPage,
+ RecoverableTitleImportException $exception
+ ) {
+ $this->specialPage = $specialPage;
+ $this->exception = $exception;
+ }
+
+ private function getMessageInPageLanguage() {
+ return $this->exception
+ ->getMessageObject()
+ ->inLanguage( $this->specialPage->getLanguage() )
+ ->plain();
+ }
+
+ public function getHtml() {
+ return Html::rawElement(
+ 'div',
+ [ 'class' => 'warningbox' ],
+ Html::element( 'p', [], (
$this->getMessageInPageLanguage() ) )
+ ) .
+ ( new ChangeTitleForm(
+ $this->specialPage,
+ $this->exception->getImportPlan()
+ ) )->getHtml();
+ }
+
+}
diff --git a/src/Html/TitleConflictPage.php b/src/Html/TitleConflictPage.php
deleted file mode 100644
index 2351ccb..0000000
--- a/src/Html/TitleConflictPage.php
+++ /dev/null
@@ -1,57 +0,0 @@
-<?php
-
-namespace FileImporter\Html;
-
-use FileImporter\Data\ImportPlan;
-use Html;
-use Message;
-use SpecialPage;
-
-/**
- * Html showing a title conflict error and ChangeTitleForm
- */
-class TitleConflictPage {
-
- /**
- * @var SpecialPage
- */
- private $specialPage;
-
- /**
- * @var string
- */
- private $warningMessage;
-
- /**
- * @var ImportPlan
- */
- private $importPlan;
-
- /**
- * @param SpecialPage $specialPage
- * @param ImportPlan $importPlan
- * @param string $warningMessage
- */
- public function __construct(
- SpecialPage $specialPage,
- ImportPlan $importPlan,
- $warningMessage
- ) {
- $this->specialPage = $specialPage;
- $this->importPlan = $importPlan;
- $this->warningMessage = $warningMessage;
- }
-
- public function getHtml() {
- return Html::rawElement(
- 'div',
- [ 'class' => 'warningbox' ],
- Html::element( 'p', [], ( new Message(
$this->warningMessage ) )->plain() )
- ) .
- ( new ChangeTitleForm(
- $this->specialPage,
- $this->importPlan
- ) )->getHtml();
- }
-
-}
diff --git a/src/Services/ImportPlanValidator.php
b/src/Services/ImportPlanValidator.php
index 1a0c113..98009d5 100644
--- a/src/Services/ImportPlanValidator.php
+++ b/src/Services/ImportPlanValidator.php
@@ -4,7 +4,7 @@
use FileImporter\Data\ImportPlan;
use FileImporter\Exceptions\DuplicateFilesException;
-use FileImporter\Exceptions\TitleConflictException;
+use FileImporter\Exceptions\RecoverableTitleImportException;
use FileImporter\Exceptions\TitleException;
use FileImporter\Interfaces\ImportTitleChecker;
@@ -37,15 +37,16 @@
*
* @param ImportPlan $importPlan The plan to be validated
*
- * @throws DuplicateFilesException When a file with the same hash is
detected
- * @throws TitleConflictException When either a local or remote title
conflict was detected
+ * @throws TitleException When there is a problem with the planned
title (can't be fixed easily).
+ * @throws DuplicateFilesException When a file with the same hash is
detected locally..
+ * @throws RecoverableTitleImportException When there is a problem with
the title that can be fixed.
*/
public function validate( ImportPlan $importPlan ) {
// Checks the extension doesn't provide easy ways to fix
- $this->runFileTitleCheck( $importPlan );
$this->runFileExtensionCheck( $importPlan );
$this->runDuplicateFilesCheck( $importPlan );
// Checks that can be fixed in the extension
+ $this->runFileTitleCheck( $importPlan );
$this->runLocalTitleConflictCheck( $importPlan );
$this->runRemoteTitleConflictCheck( $importPlan );
}
@@ -53,7 +54,10 @@
private function runFileTitleCheck( ImportPlan $importPlan ) {
$plannedTitleText = $importPlan->getTitle()->getText();
if ( $plannedTitleText != wfStripIllegalFilenameChars(
$plannedTitleText ) ) {
- throw new TitleException( 'The target filename is
invalid' );
+ throw new RecoverableTitleImportException(
+ 'fileimporter-illegalfilenamechars',
+ $importPlan
+ );
}
}
@@ -89,7 +93,10 @@
private function runLocalTitleConflictCheck( ImportPlan $importPlan ) {
if ( $importPlan->getTitle()->exists() ) {
- throw new TitleConflictException( $importPlan,
TitleConflictException::LOCAL_TITLE );
+ throw new RecoverableTitleImportException(
+ 'fileimporter-localtitleexists',
+ $importPlan
+ );
}
}
@@ -104,7 +111,10 @@
$title->getText() !==
$details->getSourceLinkTarget()->getText() &&
!$this->importTitleChecker->importAllowed(
$request->getUrl(), $title->getText() )
) {
- throw new TitleConflictException( $importPlan,
TitleConflictException::REMOTE_TITLE );
+ throw new RecoverableTitleImportException(
+ 'fileimporter-sourcetitleexists',
+ $importPlan
+ );
}
}
diff --git a/src/SpecialImportFile.php b/src/SpecialImportFile.php
index 3ea287e..7fffc29 100644
--- a/src/SpecialImportFile.php
+++ b/src/SpecialImportFile.php
@@ -9,13 +9,14 @@
use FileImporter\Data\SourceUrl;
use FileImporter\Exceptions\DuplicateFilesException;
use FileImporter\Exceptions\ImportException;
-use FileImporter\Exceptions\TitleConflictException;
+use FileImporter\Exceptions\RecoverableTitleImportException;
+use FileImporter\Exceptions\TitleException;
use FileImporter\Html\ChangeTitleForm;
use FileImporter\Html\DuplicateFilesPage;
use FileImporter\Html\ImportPreviewPage;
use FileImporter\Html\ImportSuccessPage;
use FileImporter\Html\InputFormPage;
-use FileImporter\Html\TitleConflictPage;
+use FileImporter\Html\RecoverableTitleExceptionPage;
use FileImporter\Services\DuplicateFileRevisionChecker;
use FileImporter\Services\Importer;
use FileImporter\Services\ImportPlanValidator;
@@ -148,21 +149,10 @@
return;
}
- if ( $exception instanceof TitleConflictException ) {
- switch ( $exception->getType() ) {
- case TitleConflictException::LOCAL_TITLE:
- $titleConflictMessage =
'fileimporter-localtitleexists';
- break;
- case TitleConflictException::REMOTE_TITLE:
- $titleConflictMessage =
'fileimporter-sourcetitleexists';
- break;
- default:
- throw $exception;
- }
- $this->getOutput()->addHTML( ( new TitleConflictPage(
+ if ( $exception instanceof RecoverableTitleImportException ) {
+ $this->getOutput()->addHTML( ( new
RecoverableTitleExceptionPage(
$this,
- $exception->getImportPlan(),
- $titleConflictMessage
+ $exception
) )->getHtml() );
return;
}
@@ -172,7 +162,7 @@
}
/**
- * @throws
DuplicateFilesException|TitleConflictException|ImportException
+ * @throws ImportException
* @return ImportPlan
*/
private function getImportPlan() {
diff --git a/tests/phpunit/Services/ImportPlanValidatorTest.php
b/tests/phpunit/Services/ImportPlanValidatorTest.php
index 5891194..3ded3ca 100644
--- a/tests/phpunit/Services/ImportPlanValidatorTest.php
+++ b/tests/phpunit/Services/ImportPlanValidatorTest.php
@@ -10,7 +10,7 @@
use FileImporter\Data\ImportRequest;
use FileImporter\Data\SourceUrl;
use FileImporter\Exceptions\DuplicateFilesException;
-use FileImporter\Exceptions\TitleConflictException;
+use FileImporter\Exceptions\RecoverableTitleImportException;
use FileImporter\Exceptions\TitleException;
use FileImporter\Interfaces\ImportTitleChecker;
use FileImporter\Services\DuplicateFileRevisionChecker;
@@ -144,7 +144,10 @@
$this->getMockImportTitleChecker( 0, true )
],
'Invalid, title exists on target site' => [
- new TitleConflictException( $emptyPlan, 'Title
conflict detected' ),
+ new RecoverableTitleImportException(
+ 'fileimporter-localtitleexists',
+ $emptyPlan
+ ),
$this->getMockImportPlan(
$this->getMockTitle( 'FinalName.JPG',
true ),
$this->getMockTitle( 'SourceName.JPG',
false )
@@ -153,7 +156,10 @@
$this->getMockImportTitleChecker( 0, true )
],
'Invalid, title exists on source site' => [
- new TitleConflictException( $emptyPlan, 'Title
conflict detected' ),
+ new RecoverableTitleImportException(
+ 'fileimporter-sourcetitleexists',
+ $emptyPlan
+ ),
$this->getMockImportPlan(
$this->getMockTitle( 'FinalName.JPG',
false ),
$this->getMockTitle( 'SourceName.JPG',
false )
@@ -171,7 +177,7 @@
$this->getMockImportTitleChecker( 1, true )
],
'Invalid, No Extension on planned name' => [
- new TitleException( 'The target filename is
invalid' ),
+ new TitleException( 'Planned file name does not
have an extension' ),
$this->getMockImportPlan(
$this->getMockTitle(
'FinalName.JPG/Foo', false ),
$this->getMockTitle( 'SourceName.jpg',
false )
@@ -189,12 +195,15 @@
$this->getMockImportTitleChecker( 0, true )
],
'Invalid, Bad title (includes another namespace)' => [
- new TitleException( 'The target filename is
invalid' ),
+ new RecoverableTitleImportException(
+ 'fileimporter-illegalfilenamechars',
+ $emptyPlan
+ ),
$this->getMockImportPlan(
$this->getMockTitle(
'Talk:FinalName.JPG', false ),
$this->getMockTitle( 'SourceName.JPG',
false )
),
- $this->getMockDuplicateFileRevisionChecker( 0,
0 ),
+ $this->getMockDuplicateFileRevisionChecker( 1,
0 ),
$this->getMockImportTitleChecker( 0, true )
],
];
--
To view, visit https://gerrit.wikimedia.org/r/363020
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib99aa3e105370dfc9fb5564d4cf35b254e6b5684
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/FileImporter
Gerrit-Branch: master
Gerrit-Owner: Addshore <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
