jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/342845 )
Change subject: Handle duplicate files gracefully
......................................................................
Handle duplicate files gracefully
Bug: T160166
Change-Id: I351049b934f23817f41903292b8c75e69e352286
---
M extension.json
M i18n/en.json
M i18n/qqq.json
A src/Generic/Data/FileRevisions.php
M src/Generic/Data/ImportDetails.php
A src/Generic/Services/DuplicateFileRevisionChecker.php
M src/MediaWiki/ApiDetailRetriever.php
M src/ServiceWiring.php
M src/SpecialImportFile.php
A tests/Generic/Data/FileRevisionsTest.php
10 files changed, 214 insertions(+), 14 deletions(-)
Approvals:
WMDE-Fisch: Looks good to me, approved
jenkins-bot: Verified
diff --git a/extension.json b/extension.json
index bc83f2c..0d7c69c 100644
--- a/extension.json
+++ b/extension.json
@@ -28,10 +28,12 @@
"FileImporter\\Generic\\Services\\Importer":
"src/Generic/Services/Importer.php",
"FileImporter\\Generic\\Services\\HttpRequestExecutor":
"src/Generic/Services/HttpRequestExecutor.php",
"FileImporter\\Generic\\Services\\RevisionModifier":
"src/Generic/Services/RevisionModifier.php",
+
"FileImporter\\Generic\\Services\\DuplicateFileRevisionChecker":
"src/Generic/Services/DuplicateFileRevisionChecker.php",
"FileImporter\\Generic\\Data\\ImportTransformations":
"src/Generic/Data/ImportTransformations.php",
"FileImporter\\Generic\\Data\\ImportDetails":
"src/Generic/Data/ImportDetails.php",
"FileImporter\\Generic\\Data\\TargetUrl":
"src/Generic/Data/TargetUrl.php",
"FileImporter\\Generic\\Data\\FileRevision":
"src/Generic/Data/FileRevision.php",
+ "FileImporter\\Generic\\Data\\FileRevisions":
"src/Generic/Data/FileRevisions.php",
"FileImporter\\Generic\\Data\\TextRevision":
"src/Generic/Data/TextRevision.php",
"FileImporter\\MediaWiki\\ApiDetailRetriever":
"src/MediaWiki/ApiDetailRetriever.php",
"FileImporter\\MediaWiki\\HttpApiLookup":
"src/MediaWiki/HttpApiLookup.php",
diff --git a/i18n/en.json b/i18n/en.json
index bb18904..bb868d8 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -9,6 +9,8 @@
"fileimporter-specialpage": "Import file",
"fileimporter-cantparseurl": "Can't parse the given URL",
"fileimporter-cantimporturl": "Can't import the given URL",
+ "fileimporter-duplicatefilesdetected" : "The file you are currently
trying to import already exists on this wiki.",
+ "fileimporter-duplicatefilesdetected-prefix" : "Duplicates",
"fileimporter-exampleprefix": "Example",
"fileimporter-textrevisionsprefix": "Text Revisions",
"fileimporter-filerevisionsprefix": "File Revisions",
diff --git a/i18n/qqq.json b/i18n/qqq.json
index 04f3a17..28c178a 100644
--- a/i18n/qqq.json
+++ b/i18n/qqq.json
@@ -10,6 +10,8 @@
"fileimporter-specialpage": "Title for the File Import special page.",
"fileimporter-cantparseurl": "Error message shown on the special page
when the URL entered can not be parsed.",
"fileimporter-cantimporturl": "Error message shown on the special page
when the URL entered can not be imported from.",
+ "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.",
"fileimporter-exampleprefix": "Prefix for the example URL contained
within the URL text box on the special page.\n{{Identical|Example}}",
"fileimporter-textrevisionsprefix": "Prefix for the number of text
revisions to be imported on the special page.",
"fileimporter-filerevisionsprefix": "Prefix for the number of file
revisions to be imported on the special page.",
diff --git a/src/Generic/Data/FileRevisions.php
b/src/Generic/Data/FileRevisions.php
new file mode 100644
index 0000000..2b59f95
--- /dev/null
+++ b/src/Generic/Data/FileRevisions.php
@@ -0,0 +1,53 @@
+<?php
+
+namespace FileImporter\Generic\Data;
+
+use Wikimedia\Assert\Assert;
+
+class FileRevisions {
+
+ /**
+ * @var FileRevision[]
+ */
+ private $fileRevisions;
+
+ private $latestKey = null;
+
+ /**
+ * @param FileRevision[] $fileRevisions
+ */
+ public function __construct( array $fileRevisions ) {
+ Assert::parameterElementType( FileRevision::class,
$fileRevisions, '$fileRevisions' );
+ $this->fileRevisions = $fileRevisions;
+ }
+
+ /**
+ * @return FileRevision[]
+ */
+ public function toArray() {
+ return $this->fileRevisions;
+ }
+
+ /**
+ * @return FileRevision|null
+ */
+ public function getLatest() {
+ if ( $this->latestKey === null ) {
+ $this->calculateLatestKey();
+ }
+
+ return $this->latestKey !== null ?
$this->fileRevisions[$this->latestKey] : null;
+ }
+
+ private function calculateLatestKey() {
+ $latestTimestamp = 0;
+ foreach ( $this->fileRevisions as $key => $fileRevision ) {
+ $fileTimestamp = strtotime( $fileRevision->getField(
'timestamp' ) );
+ if ( $latestTimestamp < $fileTimestamp ) {
+ $latestTimestamp = $fileTimestamp;
+ $this->latestKey = $key;
+ }
+ }
+ }
+
+}
diff --git a/src/Generic/Data/ImportDetails.php
b/src/Generic/Data/ImportDetails.php
index 1482a3a..16b0359 100644
--- a/src/Generic/Data/ImportDetails.php
+++ b/src/Generic/Data/ImportDetails.php
@@ -2,9 +2,6 @@
namespace FileImporter\Generic\Data;
-use FileImporter\Generic\Data\FileRevision;
-use FileImporter\Generic\Data\TargetUrl;
-use FileImporter\Generic\Data\TextRevision;
use Wikimedia\Assert\Assert;
class ImportDetails {
@@ -30,7 +27,7 @@
private $textRevisions;
/**
- * @var FileRevision[]
+ * @var FileRevisions
*/
private $fileRevisions;
@@ -39,19 +36,18 @@
* @param string $titleText
* @param string $imageDisplayUrl
* @param TextRevision[] $textRevisions
- * @param FileRevision[] $fileRevisions
+ * @param FileRevisions $fileRevisions
*/
public function __construct(
TargetUrl $targetUrl,
$titleText,
$imageDisplayUrl,
array $textRevisions,
- array $fileRevisions
+ FileRevisions $fileRevisions
) {
Assert::parameterType( 'string', $titleText, '$titleText' );
Assert::parameterType( 'string', $imageDisplayUrl,
'$imageDisplayUrl' );
Assert::parameterElementType( TextRevision::class,
$textRevisions, '$textRevisions' );
- Assert::parameterElementType( FileRevision::class,
$fileRevisions, '$fileRevisions' );
$this->targetUrl = $targetUrl;
$this->titleText = $titleText;
@@ -94,14 +90,14 @@
$hashes = [
sha1( $this->targetUrl->getUrl() ),
sha1( count( $this->getTextRevisions() ) ),
- sha1( count( $this->getFileRevisions() ) ),
+ sha1( count( $this->getFileRevisions()->toArray() ) ),
];
foreach ( $this->getTextRevisions() as $textRevision ) {
$hashes[] = $textRevision->getField( 'sha1' );
}
- foreach ( $this->getFileRevisions() as $fileRevision ) {
+ foreach ( $this->getFileRevisions()->toArray() as $fileRevision
) {
$hashes[] = $fileRevision->getField( 'sha1' );
}
diff --git a/src/Generic/Services/DuplicateFileRevisionChecker.php
b/src/Generic/Services/DuplicateFileRevisionChecker.php
new file mode 100644
index 0000000..ca01f58
--- /dev/null
+++ b/src/Generic/Services/DuplicateFileRevisionChecker.php
@@ -0,0 +1,60 @@
+<?php
+
+namespace FileImporter\Generic\Services;
+
+use File;
+use FileImporter\Generic\Data\FileRevision;
+use LocalRepo;
+use Wikimedia\Assert\Assert;
+
+/**
+ * Class that can be used to check if a FileRevision already exists on the
current wiki.
+ * Only current / latest and non deleted files are checked.
+ */
+class DuplicateFileRevisionChecker {
+
+ /**
+ * @var LocalRepo
+ */
+ private $localRepo;
+
+ public function __construct( LocalRepo $localRepo ) {
+ $this->localRepo = $localRepo;
+ }
+
+ /**
+ * @param FileRevision $fileRevision
+ *
+ * @return File[] array of matched files
+ */
+ public function findDuplicates( FileRevision $fileRevision ) {
+ $files = $this->localRepo->findBySha1( $fileRevision->getField(
'sha1' ) );
+ $files = $this->removeIgnoredFiles( $files );
+
+ return $files;
+ }
+
+ /**
+ * This removed removes files that are either old or deleted.
+ *
+ * @param File[] $files
+ *
+ * @return File[]
+ */
+ private function removeIgnoredFiles( array $files ) {
+ $wantedFiles = [];
+
+ foreach ( $files as $file ) {
+ if (
+ $file->isOld() ||
+ $file->isDeleted( File::DELETED_FILE )
+ ) {
+ continue;
+ }
+ $wantedFiles[] = $file;
+ }
+
+ return $wantedFiles;
+ }
+
+}
diff --git a/src/MediaWiki/ApiDetailRetriever.php
b/src/MediaWiki/ApiDetailRetriever.php
index db178b0..80c31a0 100644
--- a/src/MediaWiki/ApiDetailRetriever.php
+++ b/src/MediaWiki/ApiDetailRetriever.php
@@ -2,6 +2,7 @@
namespace FileImporter\MediaWiki;
+use FileImporter\Generic\Data\FileRevisions;
use FileImporter\Generic\Exceptions\HttpRequestException;
use FileImporter\Generic\Exceptions\ImportException;
use FileImporter\Generic\Data\FileRevision;
@@ -126,7 +127,7 @@
$importDetails = new ImportDetails(
$targetUrl,
$normalizationData['to'],
- $fileRevisions[0]->getField( 'thumburl' ),
+ $fileRevisions->getLatest()->getField( 'thumburl' ),
$textRevisions,
$fileRevisions
);
@@ -137,14 +138,21 @@
/**
* @param array $imageInfo
*
- * @return FileRevision[]
+ * @return FileRevisions
*/
private function getFileRevisionsFromImageInfo( array $imageInfo ) {
$revisions = [];
foreach ( $imageInfo as $revisionInfo ) {
+ /**
+ * Convert from API sha1 format to DB sha1 format.
+ * The conversion can be se inside ApiQueryImageInfo.
+ * - API sha1 format is base 16 padded to 40 chars
+ * - DB sha1 format is base 36 padded to 31 chars
+ */
+ $revisionInfo['sha1'] = \Wikimedia\base_convert(
$revisionInfo['sha1'], 16, 36, 31 );
$revisions[] = new FileRevision( $revisionInfo );
}
- return $revisions;
+ return new FileRevisions( $revisions );
}
/**
diff --git a/src/ServiceWiring.php b/src/ServiceWiring.php
index 38a1e02..4030ea5 100644
--- a/src/ServiceWiring.php
+++ b/src/ServiceWiring.php
@@ -3,10 +3,12 @@
namespace FileImporter;
use FileImporter\Generic\Services\DispatchingDetailRetriever;
+use FileImporter\Generic\Services\DuplicateFileRevisionChecker;
use FileImporter\Generic\Services\HttpRequestExecutor;
use MediaWiki\Logger\LoggerFactory;
use MediaWiki\MediaWikiServices;
use Psr\Log\LoggerInterface;
+use RepoGroup;
return [
@@ -29,6 +31,11 @@
return $service;
},
+ 'FileImporterDuplicateFileRevisionChecker' => function(
MediaWikiServices $services ) {
+ $localRepo = RepoGroup::singleton()->getLocalRepo();
+ return new DuplicateFileRevisionChecker( $localRepo );
+ },
+
// MediaWiki
'FileImporterMediaWikiHttpApiLookup' => function( MediaWikiServices
$services ) {
diff --git a/src/SpecialImportFile.php b/src/SpecialImportFile.php
index 8b94d0e..08a7d0c 100644
--- a/src/SpecialImportFile.php
+++ b/src/SpecialImportFile.php
@@ -2,9 +2,11 @@
namespace FileImporter;
+use File;
use FileImporter\Generic\Data\ImportTransformations;
use FileImporter\Generic\Data\ImportDetails;
use FileImporter\Generic\Services\DetailRetriever;
+use FileImporter\Generic\Services\DuplicateFileRevisionChecker;
use FileImporter\Generic\Services\Importer;
use FileImporter\Generic\Data\TargetUrl;
use Html;
@@ -49,7 +51,15 @@
$this->showUrlEntryPage();
} else {
$importDetails = $detailRetriever->getImportDetails(
$targetUrl );
- if ( $wasPosted ) {
+ /** @var DuplicateFileRevisionChecker
$duplicateFileChecker */
+ $duplicateFileChecker = MediaWikiServices::getInstance()
+ ->getService(
'FileImporterDuplicateFileRevisionChecker' );
+ $duplicateFiles = $duplicateFileChecker->findDuplicates(
+ $importDetails->getFileRevisions()->getLatest()
+ );
+ if ( !empty( $duplicateFiles ) ) {
+ $this->showDuplicateFilesDetectedMessage(
$duplicateFiles );
+ } elseif ( $wasPosted ) {
$this->doImport( $importDetails );
} else {
$this->showImportPage( $importDetails );
@@ -97,6 +107,20 @@
$this->showWarningMessage( ( new Message(
'fileimporter-cantimporturl' ) )->plain() );
}
+ /**
+ * @param File[] $duplicateFiles
+ */
+ private function showDuplicateFilesDetectedMessage( array
$duplicateFiles ) {
+ $this->showWarningMessage(
+ ( new Message( 'fileimporter-duplicatefilesdetected' )
)->plain()
+ );
+ $duplicatesMessage = ( new Message(
'fileimporter-duplicatefilesdetected-prefix' ) )->plain();
+ $this->getOutput()->addWikiText( '\'\'\'' . $duplicatesMessage
. '\'\'\'' );
+ foreach ( $duplicateFiles as $file ) {
+ $this->getOutput()->addWikiText( '* [[:' .
$file->getTitle() . ']]' );
+ }
+ }
+
private function showWarningMessage( $message ) {
$this->getOutput()->addHTML(
Html::rawElement(
@@ -140,6 +164,7 @@
$importDetails->getTitleText()
)
);
+
$out->addHTML(
Html::element(
'p',
@@ -153,7 +178,7 @@
'p',
[],
( new Message(
'fileimporter-filerevisionsprefix' ) )->plain() . ': ' .
- count(
$importDetails->getFileRevisions() )
+ count(
$importDetails->getFileRevisions()->toArray() )
)
);
$out->addHTML(
diff --git a/tests/Generic/Data/FileRevisionsTest.php
b/tests/Generic/Data/FileRevisionsTest.php
new file mode 100644
index 0000000..9365aa3
--- /dev/null
+++ b/tests/Generic/Data/FileRevisionsTest.php
@@ -0,0 +1,45 @@
+<?php
+
+namespace FileImporter\Generic\Data\Test;
+
+use FileImporter\Generic\Data\FileRevision;
+use FileImporter\Generic\Data\FileRevisions;
+use PHPUnit_Framework_TestCase;
+
+class FileRevisionsTest extends PHPUnit_Framework_TestCase {
+
+ private function getMockFileRevision( $timestamp ) {
+ $mock = $this->getMockBuilder( FileRevision::class )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $mock->expects( $this->any() )
+ ->method( 'getField' )
+ ->with( 'timestamp' )
+ ->will( $this->returnValue( $timestamp ) );
+ return $mock;
+ }
+
+ public function provideGetLatest() {
+ $firstFileRevision = $this->getMockFileRevision(
'2013-11-18T13:19:01Z' );
+ $secondFileRevision = $this->getMockFileRevision(
'2014-11-18T13:19:01Z' );
+ $thirdFileRevision = $this->getMockFileRevision(
'2015-11-18T13:19:01Z' );
+ return [
+ [ [], null ],
+ [ [ $firstFileRevision ], $firstFileRevision ],
+ [ [ $secondFileRevision ], $secondFileRevision ],
+ [ [ $firstFileRevision, $secondFileRevision ],
$secondFileRevision ],
+ [ [ $secondFileRevision, $firstFileRevision ],
$secondFileRevision ],
+ [ [ $secondFileRevision, $firstFileRevision,
$thirdFileRevision ], $thirdFileRevision ],
+ [ [ $thirdFileRevision, $firstFileRevision,
$secondFileRevision ], $thirdFileRevision ],
+ ];
+ }
+
+ /**
+ * @dataProvider provideGetLatest
+ */
+ public function testGetLatest( array $fileRevisions, $expected ) {
+ $fileRevisionsObject = new FileRevisions( $fileRevisions );
+ $this->assertSame( $expected, $fileRevisionsObject->getLatest()
);
+ }
+
+}
--
To view, visit https://gerrit.wikimedia.org/r/342845
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I351049b934f23817f41903292b8c75e69e352286
Gerrit-PatchSet: 8
Gerrit-Project: mediawiki/extensions/FileImporter
Gerrit-Branch: master
Gerrit-Owner: Addshore <[email protected]>
Gerrit-Reviewer: Andrew-WMDE <[email protected]>
Gerrit-Reviewer: Siebrand <[email protected]>
Gerrit-Reviewer: Tobias Gritschacher <[email protected]>
Gerrit-Reviewer: WMDE-Fisch <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
