Skip to content

Commit 5c956ab

Browse files
outdooracornWMDE bot
authored andcommitted
Make StatementGuid preserve case
Due to the `EntityId` constructor normalizing ids to upper case, `StatementGuid` couldn't guarantee that the statement id is in the correct case as it is stored in the database. This is an issue for the WB REST API as statement ids in the response were always normalized to have upper case entity id prefixes. This commit fixes this by adding an optional `$originalStatementId` parameter to `StatementGuid` for providing the original, non-normalized, statement id and updating `StatementGuidParser` to always use this 3rd parameter. Bug: T354262 Change-Id: Ie2e16a188d9aa329e14a26c4e6bff4b22e796717
1 parent 2b9dbcc commit 5c956ab

2 files changed

Lines changed: 48 additions & 13 deletions

File tree

src/Statement/StatementGuid.php

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace Wikibase\DataModel\Statement;
44

5+
use InvalidArgumentException;
56
use Wikibase\DataModel\Entity\EntityId;
67

78
/**
@@ -25,10 +26,23 @@ class StatementGuid {
2526
private string $guidPart;
2627
private string $serialization;
2728

28-
public function __construct( EntityId $entityId, string $guid ) {
29-
$this->serialization = $entityId->getSerialization() . self::SEPARATOR . $guid;
29+
/**
30+
* @param EntityId $entityId the normalized entity id
31+
* @param string $guidPart the guid part of the statement id that appears after the separator
32+
* @param string|null $originalStatementId the original, non-normalized, statement id.
33+
* Defaults to `null` for compatability.
34+
*/
35+
public function __construct( EntityId $entityId, string $guidPart, string $originalStatementId = null ) {
36+
$constructedStatementId = $entityId->getSerialization() . self::SEPARATOR . $guidPart;
37+
if ( $originalStatementId !== null
38+
&& strtolower( $originalStatementId ) !== strtolower( $constructedStatementId ) ) {
39+
throw new InvalidArgumentException( '$originalStatementId does not match $entityId and/or $guidPart' );
40+
}
41+
42+
// use the original serialization when available to avoid normalizing the entity id prefix
43+
$this->serialization = $originalStatementId ?? $constructedStatementId;
3044
$this->entityId = $entityId;
31-
$this->guidPart = $guid;
45+
$this->guidPart = $guidPart;
3246
}
3347

3448
public function getEntityId(): EntityId {
@@ -43,9 +57,9 @@ public function getGuidPart(): string {
4357
}
4458

4559
/**
46-
* @deprecated The value returned by this method might differ in case from the original, unparsed statement GUID
47-
* (the entity ID part might have been lowercase originally, but is always normalized in the return value here),
48-
* which means that the value should not be compared to other statement GUID serializations,
60+
* If the `$originalStatementId` parameter is not used when constructing the StatementGuid object,
61+
* then this method will return a statement id where the entity id prefix is normalized to upper case.
62+
* This could cause issues when comparing to other statement id serializations,
4963
* e.g. to look up a statement in a StatementList.
5064
*/
5165
public function getSerialization(): string {

tests/unit/Statement/StatementGuidTest.php

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,41 +19,62 @@
1919
class StatementGuidTest extends TestCase {
2020

2121
/**
22-
* @dataProvider provideConstructionData
22+
* @dataProvider provideValidConstructionData
2323
*/
24-
public function testConstructor( EntityId $entityId, string $guid, string $expected ): void {
25-
$statementGuid = new StatementGuid( $entityId, $guid );
24+
public function testConstructor( EntityId $entityId, string $guidPart, ?string $originalSerialization, string $expected ): void {
25+
$statementGuid = new StatementGuid( $entityId, $guidPart, $originalSerialization );
2626

2727
$this->assertSame( $expected, $statementGuid->getSerialization() );
2828
$this->assertEquals( $entityId, $statementGuid->getEntityId() );
29-
$this->assertSame( $guid, $statementGuid->getGuidPart() );
29+
$this->assertSame( $guidPart, $statementGuid->getGuidPart() );
3030
}
3131

32-
public static function provideConstructionData(): array {
32+
public static function provideValidConstructionData(): array {
3333
return [
3434
[
3535
new ItemId( 'q42' ),
3636
'D8404CDA-25E4-4334-AF13-A3290BCD9C0N',
37+
null,
3738
'Q42$D8404CDA-25E4-4334-AF13-A3290BCD9C0N',
3839
],
40+
[
41+
new ItemId( 'q42' ),
42+
'D8404CDA-25E4-4334-AF13-A3290BCD9C0N',
43+
'q42$D8404CDA-25E4-4334-AF13-A3290BCD9C0N',
44+
'q42$D8404CDA-25E4-4334-AF13-A3290BCD9C0N',
45+
],
46+
[
47+
new ItemId( 'Q1234567' ),
48+
'D4FDE516-F20C-4154-ADCE-7C5B609DFDFF',
49+
null,
50+
'Q1234567$D4FDE516-F20C-4154-ADCE-7C5B609DFDFF',
51+
],
3952
[
4053
new ItemId( 'Q1234567' ),
4154
'D4FDE516-F20C-4154-ADCE-7C5B609DFDFF',
4255
'Q1234567$D4FDE516-F20C-4154-ADCE-7C5B609DFDFF',
56+
'Q1234567$D4FDE516-F20C-4154-ADCE-7C5B609DFDFF',
57+
],
58+
[
59+
new ItemId( 'Q1' ),
60+
'foo',
61+
null,
62+
'Q1$foo',
4363
],
4464
[
4565
new ItemId( 'Q1' ),
4666
'foo',
4767
'Q1$foo',
68+
'Q1$foo',
4869
],
4970
];
5071
}
5172

5273
public function provideStatementGuids(): array {
5374
$argLists = [];
5475

55-
foreach ( $this->provideConstructionData() as $data ) {
56-
$argLists[] = [ new StatementGuid( $data[0], $data[1] ) ];
76+
foreach ( $this->provideValidConstructionData() as $data ) {
77+
$argLists[] = [ new StatementGuid( ...$data ) ];
5778
}
5879

5980
return $argLists;

0 commit comments

Comments
 (0)