diff --git a/includes/libs/StatusValue.php b/includes/libs/StatusValue.php index 5be6b48dfa1..922829c1d23 100644 --- a/includes/libs/StatusValue.php +++ b/includes/libs/StatusValue.php @@ -170,6 +170,54 @@ class StatusValue { $this->value = $value; } + /** + * Add a new error to the error array ($this->errors) if that error is not already in the + * error array. Each error is passed as an array with the following fields: + * + * - type: 'error' or 'warning' + * - message: a string (message key) or MessageSpecifier + * - params: an array of string parameters + * + * If the new error is of type 'error' and it matches an existing error of type 'warning', + * the existing error is upgraded to type 'error'. An error provided as a MessageSpecifier + * will successfully match an error provided as the same string message key and array of + * parameters as separate array elements. + * + * @param array $newError + */ + private function addError( array $newError ) { + if ( $newError[ 'message' ] instanceof MessageSpecifier ) { + $isEqual = function ( $existingError ) use ( $newError ) { + if ( $existingError['message'] instanceof MessageSpecifier ) { + // compare attributes of both MessageSpecifiers + return $newError['message'] == $existingError['message']; + } else { + return $newError['message']->getKey() === $existingError['message'] && + $newError['message']->getParams() === $existingError['params']; + } + }; + } else { + $isEqual = function ( $existingError ) use ( $newError ) { + if ( $existingError['message'] instanceof MessageSpecifier ) { + return $newError['message'] === $existingError['message']->getKey() && + $newError['params'] === $existingError['message']->getParams(); + } else { + return $newError['message'] === $existingError['message'] && + $newError['params'] === $existingError['params']; + } + }; + } + foreach ( $this->errors as $index => $existingError ) { + if ( $isEqual( $existingError ) ) { + if ( $newError[ 'type' ] === 'error' && $existingError[ 'type' ] === 'warning' ) { + $this->errors[ $index ][ 'type' ] = 'error'; + } + return; + } + } + $this->errors[] = $newError; + } + /** * Add a new warning * @@ -177,11 +225,11 @@ class StatusValue { * @param mixed ...$parameters */ public function warning( $message, ...$parameters ) { - $this->errors[] = [ + $this->addError( [ 'type' => 'warning', 'message' => $message, 'params' => $parameters - ]; + ] ); } /** @@ -192,11 +240,11 @@ class StatusValue { * @param mixed ...$parameters */ public function error( $message, ...$parameters ) { - $this->errors[] = [ + $this->addError( [ 'type' => 'error', 'message' => $message, 'params' => $parameters - ]; + ] ); } /** @@ -218,7 +266,9 @@ class StatusValue { * @param bool $overwriteValue Whether to override the "value" member */ public function merge( $other, $overwriteValue = false ) { - $this->errors = array_merge( $this->errors, $other->errors ); + foreach ( $other->errors as $error ) { + $this->addError( $error ); + } $this->ok = $this->ok && $other->ok; if ( $overwriteValue ) { $this->value = $other->value; diff --git a/tests/phpunit/includes/StatusTest.php b/tests/phpunit/includes/StatusTest.php index d9264c10b31..c00eafbbf21 100644 --- a/tests/phpunit/includes/StatusTest.php +++ b/tests/phpunit/includes/StatusTest.php @@ -751,4 +751,238 @@ class StatusTest extends MediaWikiLangTestCase { $status->getWikiText( 'wrap-short', 'wrap-long' ); } + public function provideDuplicates() { + yield [ [ 'foo', 1, 2 ], [ 'foo', 1, 2 ] ]; + $message = new Message( 'foo', [ 1, 2 ] ); + yield[ $message, $message ]; + yield [ $message, array_merge( [ $message->getKey() ], $message->getParams() ) ]; + yield [ array_merge( [ $message->getKey() ], $message->getParams() ), $message ]; + } + + /** + * @dataProvider provideDuplicates + * @covers Status::error + */ + public function testDuplicateError( $error1, $error2 ) { + $status = Status::newGood(); + if ( $error1 instanceof MessageSpecifier ) { + $status->error( $error1 ); + $expected = [ + 'type' => 'error', + 'message' => $error1, + 'params' => [] + ]; + } else { + $status->error( ...$error1 ); + $message1 = $error1[0]; + array_shift( $error1 ); + $expected = [ + 'type' => 'error', + 'message' => $message1, + 'params' => $error1 + ]; + } + if ( $error2 instanceof MessageSpecifier ) { + $status->error( $error2 ); + } else { + $message2 = $error2[0]; + array_shift( $error2 ); + $status->error( $message2, ...$error2 ); + } + $this->assertArrayEquals( [ $expected ], $status->errors ); + } + + /** + * @covers Status::warning + */ + public function testDuplicateWarning() { + $status = Status::newGood(); + $status->warning( 'foo', 1, 2 ); + $status->warning( 'foo', 1, 2 ); + $this->assertArrayEquals( + [ + [ + 'type' => 'warning', + 'message' => 'foo', + 'params' => [ 1, 2 ] + ] + ], + $status->errors + ); + } + + /** + * @covers Status::error + * @covers Status::warning + */ + public function testErrorNotDowngradedToWarning() { + $status = Status::newGood(); + $status->error( 'foo', 1, 2 ); + $status->warning( 'foo', 1, 2 ); + $this->assertArrayEquals( + [ + [ + 'type' => 'error', + 'message' => 'foo', + 'params' => [ 1, 2 ] + ] + ], + $status->errors + ); + } + + /** + * @covers Status::error + * @covers Status::warning + */ + public function testErrorNotDowngradedToWarningMessage() { + $status = Status::newGood(); + $message = new Message( 'foo', [ 1, 2 ] ); + $status->error( $message ); + $status->warning( $message ); + $this->assertArrayEquals( + [ + [ + 'type' => 'error', + 'message' => $message, + 'params' => [] + ] + ], + $status->errors + ); + } + + /** + * @covers Status::error + * @covers Status::warning + */ + public function testWarningUpgradedToError() { + $status = Status::newGood(); + $status->warning( 'foo', 1, 2 ); + $status->error( 'foo', 1, 2 ); + $this->assertArrayEquals( + [ + [ + 'type' => 'error', + 'message' => 'foo', + 'params' => [ 1, 2 ] + ] + ], + $status->errors + ); + } + + /** + * @covers Status::error + * @covers Status::warning + */ + public function testWarningUpgradedToErrorMessage() { + $status = Status::newGood(); + $message = new Message( 'foo', [ 1, 2 ] ); + $status->warning( $message ); + $status->error( $message ); + $this->assertArrayEquals( + [ + [ + 'type' => 'error', + 'message' => $message, + 'params' => [] + ] + ], + $status->errors + ); + } + + /** + * Ensure that two MessageSpecifiers that have the same key and params are considered + * identical even if they are different instances. + * @covers Status::error + */ + public function testCompareMessages() { + $status = Status::newGood(); + $message1 = new Message( 'foo', [ 1, 2 ] ); + $status->error( $message1 ); + $message2 = new Message( 'foo', [ 1, 2 ] ); + $status->error( $message2 ); + $this->assertEquals( count( $status->errors ), 1 ); + } + + /** + * @covers Status::merge + */ + public function testDuplicateMerge() { + $status1 = Status::newGood(); + $status1->error( 'cat', 1, 2 ); + $status1->warning( 'dog', 3, 4 ); + $status2 = Status::newGood(); + $status2->warning( 'cat', 1, 2 ); + $status1->error( 'dog', 3, 4 ); + $status2->warning( 'rabbit', 5, 6 ); + $status1->merge( $status2 ); + $this->assertArrayEquals( + [ + [ + 'type' => 'error', + 'message' => 'cat', + 'params' => [ 1, 2 ] + ], + [ + 'type' => 'error', + 'message' => 'dog', + 'params' => [ 3, 4 ] + ], + [ + 'type' => 'warning', + 'message' => 'rabbit', + 'params' => [ 5, 6 ] + ] + ], + $status1->errors + ); + } + + /** + * @covers Status::error + */ + public function testNotDuplicateIfKeyDiffers() { + $status = Status::newGood(); + $status->error( 'foo', 1, 2 ); + $status->error( 'bar', 1, 2 ); + $this->assertArrayEquals( + [ + [ + 'type' => 'error', + 'message' => 'foo', + 'params' => [ 1, 2 ] + ], + [ + 'type' => 'error', + 'message' => 'bar', + 'params' => [ 1, 2 ] + ] + ], + $status->errors + ); + } + + /** + * @covers Status::error + */ + public function testNotDuplicateIfParamsDiffer() { + $status = Status::newGood(); + $status->error( 'foo', 1, 2 ); + $status->error( 'foo', 3, 4 ); + $this->assertArrayEquals( [ + [ + 'type' => 'error', + 'message' => 'foo', + 'params' => [ 1, 2 ] + ], + [ + 'type' => 'error', + 'message' => 'foo', + 'params' => [ 3, 4 ] + ] + ], $status->errors ); + } }