Merge "Status/StatusValue errors/warnings should be unique"

This commit is contained in:
jenkins-bot 2021-02-06 16:26:36 +00:00 committed by Gerrit Code Review
commit 2c69be166f
2 changed files with 289 additions and 5 deletions

View file

@ -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;

View file

@ -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 );
}
}