jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/325784 )
Change subject: Make SiteLinkConflictLookup return ItemIds instead of numbers
......................................................................
Make SiteLinkConflictLookup return ItemIds instead of numbers
This does not get rid of the fact that Item IDs are numeric, but this
patch moves this fact into the (only) implementation. It's much closer
to the actual SQL query now. It's an implementation detail now.
This is related to all the other attempts in the past years and weeks
to get rid of numeric IDs.
Change-Id: I4f79544f1318dd32509611fa2fb64bf5d0c0729e
---
M repo/includes/Store/SiteLinkConflictLookup.php
M repo/includes/Store/Sql/SqlSiteLinkConflictLookup.php
M repo/includes/Validators/SiteLinkUniquenessValidator.php
M repo/tests/phpunit/includes/ChangeOp/ChangeOpTestMockProvider.php
M repo/tests/phpunit/includes/Store/Sql/SqlSiteLinkConflictLookupTest.php
5 files changed, 35 insertions(+), 62 deletions(-)
Approvals:
Daniel Kinzler: Looks good to me, approved
Aleksey Bekh-Ivanov (WMDE): Looks good to me, but someone else must approve
jenkins-bot: Verified
diff --git a/repo/includes/Store/SiteLinkConflictLookup.php
b/repo/includes/Store/SiteLinkConflictLookup.php
index 6980a92..fb770df 100644
--- a/repo/includes/Store/SiteLinkConflictLookup.php
+++ b/repo/includes/Store/SiteLinkConflictLookup.php
@@ -20,11 +20,11 @@
* currently in the store. The array is empty if there are no such
conflicts.
*
* The items in the return array are arrays with the following elements:
- * - int itemId Numeric (unprefixed) item id
+ * - ItemId itemId
* - string siteId
* - string sitePage
*
- * @since 0.1
+ * @since 0.5
*
* @param Item $item
* @param Database|null $db The database object to use (optional).
@@ -33,7 +33,7 @@
* also be used for saving. This will preserve transactional
integrity
* and avoid race conditions.
*
- * @return array[]
+ * @return array[] An array of arrays, each with the keys "siteId",
"itemId" and "sitePage".
*/
public function getConflictsForItem( Item $item, Database $db = null );
diff --git a/repo/includes/Store/Sql/SqlSiteLinkConflictLookup.php
b/repo/includes/Store/Sql/SqlSiteLinkConflictLookup.php
index 78a213c..1c96dde 100644
--- a/repo/includes/Store/Sql/SqlSiteLinkConflictLookup.php
+++ b/repo/includes/Store/Sql/SqlSiteLinkConflictLookup.php
@@ -5,6 +5,7 @@
use Database;
use DBAccessBase;
use Wikibase\DataModel\Entity\Item;
+use Wikibase\DataModel\Entity\ItemId;
use Wikibase\DataModel\SiteLink;
use Wikibase\Repo\Store\SiteLinkConflictLookup;
@@ -24,13 +25,13 @@
* @param Item $item
* @param Database|null $db
*
- * @return array[]
+ * @return array[] An array of arrays, each with the keys "siteId",
"itemId" and "sitePage".
*/
public function getConflictsForItem( Item $item, Database $db = null ) {
$siteLinks = $item->getSiteLinkList();
if ( $siteLinks->isEmpty() ) {
- return array();
+ return [];
}
if ( $db ) {
@@ -60,23 +61,23 @@
$conflictingLinks = $dbr->select(
'wb_items_per_site',
- array(
+ [
'ips_site_id',
'ips_site_page',
'ips_item_id',
- ),
+ ],
"($anyOfTheLinks) AND ips_item_id != " .
(int)$item->getId()->getNumericId(),
__METHOD__
);
- $conflicts = array();
+ $conflicts = [];
foreach ( $conflictingLinks as $link ) {
- $conflicts[] = array(
+ $conflicts[] = [
'siteId' => $link->ips_site_id,
- 'itemId' => (int)$link->ips_item_id,
+ 'itemId' => ItemId::newFromNumber(
$link->ips_item_id ),
'sitePage' => $link->ips_site_page,
- );
+ ];
}
if ( !$db ) {
diff --git a/repo/includes/Validators/SiteLinkUniquenessValidator.php
b/repo/includes/Validators/SiteLinkUniquenessValidator.php
index b014aad..6c35943 100644
--- a/repo/includes/Validators/SiteLinkUniquenessValidator.php
+++ b/repo/includes/Validators/SiteLinkUniquenessValidator.php
@@ -48,7 +48,6 @@
$conflicts =
$this->siteLinkConflictLookup->getConflictsForItem( $entity, $db );
- /* @var ItemId $ignoreConflictsWith */
foreach ( $conflicts as $conflict ) {
$errors[] = $this->getConflictError( $conflict
);
}
@@ -65,15 +64,13 @@
* @return Error
*/
private function getConflictError( array $conflict ) {
- $entityId = ItemId::newFromNumber( $conflict['itemId'] );
-
return new UniquenessViolation(
- $entityId,
+ $conflict['itemId'],
'SiteLink conflict',
'sitelink-conflict',
array(
new SiteLink( $conflict['siteId'],
$conflict['sitePage'] ),
- $entityId,
+ $conflict['itemId'],
)
);
}
diff --git a/repo/tests/phpunit/includes/ChangeOp/ChangeOpTestMockProvider.php
b/repo/tests/phpunit/includes/ChangeOp/ChangeOpTestMockProvider.php
index 6be53fc..36c4e13 100644
--- a/repo/tests/phpunit/includes/ChangeOp/ChangeOpTestMockProvider.php
+++ b/repo/tests/phpunit/includes/ChangeOp/ChangeOpTestMockProvider.php
@@ -16,6 +16,7 @@
use ValueValidators\ValueValidator;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\Item;
+use Wikibase\DataModel\Entity\ItemId;
use Wikibase\DataModel\Entity\PropertyId;
use Wikibase\DataModel\Services\Lookup\PropertyDataTypeLookup;
use Wikibase\DataModel\Services\Statement\GuidGenerator;
@@ -461,55 +462,29 @@
}
/**
- * @see SiteLinkLookup::getConflictsForItem
- *
- * The items in the return array are arrays with the following elements:
- * - integer itemId
- * - string siteId
- * - string sitePage
- *
- * @param Item $item
- *
- * @return array
- */
- public function getSiteLinkConflictsForItem( Item $item ) {
- $conflicts = array();
-
- foreach ( $item->getSiteLinkList()->toArray() as $link ) {
- $page = $link->getPageName();
- $site = $link->getSiteId();
-
- if ( $page === 'DUPE' ) {
- //NOTE: some tests may rely on these exact
values!
- $conflicts[] = array(
- 'itemId' => 666,
- 'siteId' => $site,
- 'sitePage' => $page
- );
- }
- }
-
- return $conflicts;
- }
-
- /**
- * @param array|null $returnValue
- *
* @return SiteLinkConflictLookup
*/
- public function getMockSiteLinkConflictLookup( $returnValue = null ) {
- if ( is_array( $returnValue ) ) {
- $getConflictsForItem = function() use ( $returnValue ) {
- return $returnValue;
- };
- } else {
- $getConflictsForItem = array( $this,
'getSiteLinkConflictsForItem' );
- }
-
+ public function getMockSiteLinkConflictLookup() {
$mock = $this->getMock( SiteLinkConflictLookup::class );
+
$mock->expects( PHPUnit_Framework_TestCase::any() )
->method( 'getConflictsForItem' )
- ->will( PHPUnit_Framework_TestCase::returnCallback(
$getConflictsForItem ) );
+ ->will( PHPUnit_Framework_TestCase::returnCallback(
function ( Item $item ) {
+ $conflicts = [];
+
+ foreach ( $item->getSiteLinkList()->toArray()
as $link ) {
+ if ( $link->getPageName() === 'DUPE' ) {
+ $conflicts[] = [
+ 'siteId' =>
$link->getSiteId(),
+ 'itemId' => new ItemId(
'Q666' ),
+ 'sitePage' =>
$link->getPageName(),
+ ];
+ }
+ }
+
+ return $conflicts;
+ } ) );
+
return $mock;
}
diff --git
a/repo/tests/phpunit/includes/Store/Sql/SqlSiteLinkConflictLookupTest.php
b/repo/tests/phpunit/includes/Store/Sql/SqlSiteLinkConflictLookupTest.php
index 7a51435..9f5dc7f 100644
--- a/repo/tests/phpunit/includes/Store/Sql/SqlSiteLinkConflictLookupTest.php
+++ b/repo/tests/phpunit/includes/Store/Sql/SqlSiteLinkConflictLookupTest.php
@@ -52,12 +52,12 @@
$expected = array(
array(
'siteId' => 'enwiki',
- 'itemId' => 9,
+ 'itemId' => new ItemId( 'Q9' ),
'sitePage' => 'Kitten'
)
);
- $this->assertSame(
+ $this->assertEquals(
$expected,
$siteLinkConflictLookup->getConflictsForItem(
$this->getItem( 'Kitten' ) )
);
--
To view, visit https://gerrit.wikimedia.org/r/325784
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I4f79544f1318dd32509611fa2fb64bf5d0c0729e
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: Addshore <[email protected]>
Gerrit-Reviewer: Aleksey Bekh-Ivanov (WMDE) <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Jakob <[email protected]>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: WMDE-leszek <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
