Rework Upload*Exception classes to implement ILocalizedException
authorCormac Parle <cparle@wikimedia.org>
Mon, 9 Oct 2017 14:26:46 +0000 (15:26 +0100)
committerMatthias Mullie <mmullie@wikimedia.org>
Mon, 6 Nov 2017 12:39:11 +0000 (12:39 +0000)
Bug: T154781
Change-Id: Ia64295d7ea502014586b8b8e3e3f34272b72443c

includes/upload/UploadStash.php
tests/phpunit/includes/upload/UploadStashTest.php

index da896f9..c3d2e4d 100644 (file)
@@ -265,8 +265,7 @@ class UploadStash {
                        // At this point, $error should contain the single "most important"
                        // error, plus any parameters.
                        $errorMsg = array_shift( $error );
-                       throw new UploadStashFileException( "Error storing file in '$path': "
-                               . wfMessage( $errorMsg, $error )->text() );
+                       throw new UploadStashFileException( wfMessage( $errorMsg, $error ) );
                }
                $stashPath = $storeStatus->value;
 
@@ -739,7 +738,27 @@ class UploadStashFile extends UnregisteredLocalFile {
        }
 }
 
-class UploadStashException extends MWException {
+class UploadStashException extends MWException implements ILocalizedException {
+       /** @var string|array|MessageSpecifier */
+       protected $messageSpec;
+
+       /**
+        * @param string|array|MessageSpecifier $messageSpec See Message::newFromSpecifier
+        * @param int $code Exception code
+        * @param Exception|Throwable $previous The previous exception used for the exception chaining.
+        */
+       public function __construct( $messageSpec, $code = 0, $previous = null ) {
+               $this->messageSpec = $messageSpec;
+
+               $msg = $this->getMessageObject()->text();
+               $msg = preg_replace( '!</?(var|kbd|samp|code)>!', '"', $msg );
+               $msg = Sanitizer::stripAllTags( $msg );
+               parent::__construct( $msg, $code, $previous );
+       }
+
+       public function getMessageObject() {
+               return Message::newFromSpecifier( $this->messageSpec );
+       }
 }
 
 class UploadStashFileNotFoundException extends UploadStashException {
index 7c40a2d..d754ba5 100644 (file)
@@ -14,14 +14,13 @@ class UploadStashTest extends MediaWikiTestCase {
        /**
         * @var string
         */
-       private $bug29408File;
+       private $tmpFile;
 
        protected function setUp() {
                parent::setUp();
 
-               // Setup a file for T31408
-               $this->bug29408File = wfTempDir() . '/bug29408';
-               file_put_contents( $this->bug29408File, "\x00" );
+               $this->tmpFile = wfTempDir() . '/' . uniqid();
+               file_put_contents( $this->tmpFile, "\x00" );
 
                self::$users = [
                        'sysop' => new TestUser(
@@ -40,12 +39,12 @@ class UploadStashTest extends MediaWikiTestCase {
        }
 
        protected function tearDown() {
-               if ( file_exists( $this->bug29408File . "." ) ) {
-                       unlink( $this->bug29408File . "." );
+               if ( file_exists( $this->tmpFile . "." ) ) {
+                       unlink( $this->tmpFile . "." );
                }
 
-               if ( file_exists( $this->bug29408File ) ) {
-                       unlink( $this->bug29408File );
+               if ( file_exists( $this->tmpFile ) ) {
+                       unlink( $this->tmpFile );
                }
 
                parent::tearDown();
@@ -61,7 +60,7 @@ class UploadStashTest extends MediaWikiTestCase {
                $stash = new UploadStash( $repo );
 
                // Throws exception caught by PHPUnit on failure
-               $file = $stash->stashFile( $this->bug29408File );
+               $file = $stash->stashFile( $this->tmpFile );
                // We'll never reach this point if we hit T31408
                $this->assertTrue( true, 'Unrecognized file without extension' );
 
@@ -104,4 +103,23 @@ class UploadStashTest extends MediaWikiTestCase {
                $this->assertTrue( UploadFromStash::isValidRequest( $request ) );
        }
 
+       public function testExceptionWhenStoreTempFails() {
+               $mockRepoStoreStatusResult = Status::newFatal( 'TEST_ERROR' );
+               $mockRepo = $this->getMockBuilder( FileRepo::class )
+                       ->disableOriginalConstructor()
+                       ->getMock();
+               $mockRepo->expects( $this->once() )
+                       ->method( 'storeTemp' )
+                       ->willReturn( $mockRepoStoreStatusResult );
+
+               $stash = new UploadStash( $mockRepo );
+               try {
+                       $stash->stashFile( $this->tmpFile );
+                       $this->fail( 'Expected UploadStashFileException not thrown' );
+               } catch ( UploadStashFileException $e ) {
+                       $this->assertInstanceOf( 'ILocalizedException', $e );
+               } catch ( Exception $e ) {
+                       $this->fail( 'Unexpected exception class ' . get_class( $e ) );
+               }
+       }
 }