Daniel Kinzler has uploaded a new change for review.
https://gerrit.wikimedia.org/r/97923
Change subject: (bug 57646) Exclude missing entities from JS vars
......................................................................
(bug 57646) Exclude missing entities from JS vars
Referenced items that have been deleted should not be
present in the wbUsedEntities JS variable used by EntityView.
Change-Id: I77af42488b4258ab9eb42682ac10123dd4a1d9ff
---
M lib/includes/store/EntityInfoBuilder.php
M lib/includes/store/sql/SqlEntityInfoBuilder.php
M lib/tests/phpunit/MockRepository.php
M lib/tests/phpunit/MockRepositoryTest.php
M lib/tests/phpunit/store/SqlEntityInfoBuilderTest.php
M repo/includes/EntityView.php
M repo/tests/phpunit/includes/EntityViewTest.php
7 files changed, 235 insertions(+), 16 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/23/97923/1
diff --git a/lib/includes/store/EntityInfoBuilder.php
b/lib/includes/store/EntityInfoBuilder.php
index f084c9b..a73ee9f 100644
--- a/lib/includes/store/EntityInfoBuilder.php
+++ b/lib/includes/store/EntityInfoBuilder.php
@@ -42,4 +42,12 @@
* with the key being the entity's ID. NOTE: This array will be
updated!
*/
public function addDataTypes( array &$entityInfo );
+
+ /**
+ * Removes entries for non-existent Entities from $entityInfo.
+ *
+ * @param array $entityInfo a map of strings to arrays, each array
representing an entity,
+ * with the key being the entity's ID. NOTE: This array will be
updated!
+ */
+ public function removeMissing( array &$entityInfo );
}
diff --git a/lib/includes/store/sql/SqlEntityInfoBuilder.php
b/lib/includes/store/sql/SqlEntityInfoBuilder.php
index b0996b4..f15d0b1 100644
--- a/lib/includes/store/sql/SqlEntityInfoBuilder.php
+++ b/lib/includes/store/sql/SqlEntityInfoBuilder.php
@@ -39,6 +39,11 @@
protected $propertyInfoTable;
/**
+ * @var string
+ */
+ protected $entityPerPageTable;
+
+ /**
* @var EntityIdParser
*/
protected $idParser;
@@ -61,6 +66,7 @@
$this->termTable = 'wb_terms';
$this->propertyInfoTable = 'wb_property_info';
+ $this->entityPerPageTable = 'wb_entity_per_page';
}
/**
@@ -101,7 +107,7 @@
/* @var EntityId $id */
$id = $this->idParser->parse( $prefixedId );
$type = $id->getEntityType();
- $ids[$type][] = $id->getNumericId();
+ $ids[$type][$prefixedId] = $id->getNumericId();
}
return $ids;
@@ -135,9 +141,7 @@
}
/**
- * Adds terms (like labels and/or descriptions and/or aliases) to the
entity records in
- * $entityInfo. If no such terms are found for an entity, the
respective field in the
- * entity records is set to array().
+ * @see EntityInfoBuilder::addTerms()
*
* @param array $entityInfo a map of strings to arrays, each array
representing an entity,
* with the key being the entity's ID. NOTE: This array will be
updated!
@@ -271,8 +275,7 @@
}
/**
- * Adds property data types to the entries in $entityInfo. Missing
Properties
- * will have their datatype field set to null. Other entities remain
unchanged.
+ * @see EntityInfoBuilder::addDataTypes()
*
* @param array $entityInfo a map of strings to arrays, each array
representing an entity,
* with the key being the entity's ID. NOTE: This array will be
updated!
@@ -330,4 +333,69 @@
$entityInfo[$key]['datatype'] = $row->pi_type;
}
}
+
+ /**
+ * Adds property data types to the entries in $entityInfo. Missing
Properties
+ * will have their datatype field set to null. Other entities remain
unchanged.
+ *
+ * @param array $entityInfo a map of strings to arrays, each array
representing an entity,
+ * with the key being the entity's ID. NOTE: This array will be
updated!
+ */
+ public function removeMissing( array &$entityInfo ) {
+ wfProfileIn( __METHOD__ );
+
+ $entityIdsByType = $this->getNumericEntityIds( $entityInfo );
+
+ //NOTE: we make one DB query per entity type, so we can take
advantage of the
+ // database index on the epp_entity_type field.
+ foreach ( $entityIdsByType as $type => $idsForType ) {
+ $pageIds = $this->getPageIdsForEntities( $type,
$idsForType );
+ $missingNumericIds = array_diff( $idsForType,
array_keys( $pageIds ) );
+
+ // get the missing prefixed ids based on the missing
numeric ids
+ $numericToPrefixed = array_flip( $idsForType );
+ $missingPrefixedIds = array_intersect_key(
$numericToPrefixed, array_flip( array_values( $missingNumericIds ) ) );
+
+ // strip missing stuff from $entityInfo
+ $entityInfo = array_diff_key( $entityInfo, array_flip(
$missingPrefixedIds ) );
+ }
+
+ wfProfileOut( __METHOD__ );
+ }
+
+ /**
+ * Creates a mapping from the given entity IDs to the corresponding
page IDs.
+ *
+ * @param string $entityType
+ * @param array $entityIds
+ *
+ * @return array A map of (numeric) entity IDs to page ids.
+ */
+ private function getPageIdsForEntities( $entityType, $entityIds ) {
+ wfProfileIn( __METHOD__ );
+
+ $pageIds = array();
+
+ $dbw = $this->getConnection( DB_SLAVE );
+
+ $res = $dbw->select(
+ $this->entityPerPageTable,
+ array( 'epp_entity_type', 'epp_entity_id',
'epp_page_id' ),
+ array(
+ 'epp_entity_type' => $entityType,
+ 'epp_entity_id' => $entityIds,
+ ),
+ __METHOD__
+ );
+
+ foreach ( $res as $row ) {
+ $pageIds[$row->epp_entity_id] = $row->epp_page_id;
+ }
+
+ $this->releaseConnection( $dbw );
+
+ wfProfileOut( __METHOD__ );
+
+ return $pageIds;
+ }
}
diff --git a/lib/tests/phpunit/MockRepository.php
b/lib/tests/phpunit/MockRepository.php
index 04c1c46..1e85b73 100644
--- a/lib/tests/phpunit/MockRepository.php
+++ b/lib/tests/phpunit/MockRepository.php
@@ -675,6 +675,24 @@
}
/**
+ * Adds property data types to the entries in $entityInfo. Entities
that do not have a data type
+ * remain unchanged.
+ *
+ * @param array $entityInfo a map of strings to arrays, each array
representing an entity,
+ * with the key being the entity's ID. NOTE: This array will be
updated!
+ */
+ public function removeMissing( array &$entityInfo ) {
+ foreach ( array_keys( $entityInfo ) as $key ) {
+ $id = EntityId::newFromPrefixedId( $key );
+ $entity = $this->getEntity( $id );
+
+ if ( !$entity ) {
+ unset( $entityInfo[$key] );
+ }
+ }
+ }
+
+ /**
* @see PropertyDataTypeLookup::getDataTypeIdForProperty()
*
* @since 0.5
diff --git a/lib/tests/phpunit/MockRepositoryTest.php
b/lib/tests/phpunit/MockRepositoryTest.php
index f386c00..ac7b9c0 100644
--- a/lib/tests/phpunit/MockRepositoryTest.php
+++ b/lib/tests/phpunit/MockRepositoryTest.php
@@ -604,4 +604,50 @@
$this->setExpectedException(
'Wikibase\Lib\PropertyNotFoundException' );
$this->repo->getDataTypeIdForProperty( new PropertyId( 'P3645'
) );
}
+
+ public function provideRemoveMissing() {
+ return array(
+ array(
+ array(),
+ array()
+ ),
+
+ array(
+ array(
+ 'Q2' => array( 'id' => 'Q2', 'type' =>
Item::ENTITY_TYPE ),
+ ),
+ array(
+ 'Q2' => array( 'id' => 'Q2', 'type' =>
Item::ENTITY_TYPE ),
+ ),
+ ),
+
+ array(
+ array(
+ 'Q7' => array( 'id' => 'Q7', 'type' =>
Item::ENTITY_TYPE ),
+ ),
+ array()
+ ),
+
+ array(
+ array(
+ 'Q7' => array( 'id' => 'Q7', 'type' =>
Item::ENTITY_TYPE ),
+ 'P7' => array( 'id' => 'P7', 'type' =>
Property::ENTITY_TYPE ),
+ 'Q2' => array( 'id' => 'Q2', 'type' =>
Item::ENTITY_TYPE ),
+ ),
+ array(
+ 'Q2' => array( 'id' => 'Q2', 'type' =>
Item::ENTITY_TYPE ),
+ )
+ ),
+ );
+ }
+
+ /**
+ * @dataProvider provideRemoveMissing
+ */
+ public function testRemoveMissing( array $entityInfo, array $expected =
null ) {
+ $this->setupGetEntities();
+ $this->repo->removeMissing( $entityInfo );
+
+ $this->assertArrayEquals( array_keys( $expected ), array_keys(
$entityInfo ) );
+ }
}
diff --git a/lib/tests/phpunit/store/SqlEntityInfoBuilderTest.php
b/lib/tests/phpunit/store/SqlEntityInfoBuilderTest.php
index 771151c..a65ede0 100644
--- a/lib/tests/phpunit/store/SqlEntityInfoBuilderTest.php
+++ b/lib/tests/phpunit/store/SqlEntityInfoBuilderTest.php
@@ -40,6 +40,7 @@
$this->tablesUsed[] = 'wb_property_info';
$this->tablesUsed[] = 'wb_terms';
+ $this->tablesUsed[] = 'wb_entity_per_page';
$this->insertRows(
'wb_terms',
@@ -79,6 +80,16 @@
array(
array( 2, 'type2', '{"type":"type2"}' ),
array( 3, 'type3', '{"type":"type3"}' ),
+ ) );
+
+ $this->insertRows(
+ 'wb_entity_per_page',
+ array( 'epp_entity_type', 'epp_entity_id',
'epp_page_id' ),
+ array(
+ array( Item::ENTITY_TYPE, 1, 1001 ),
+ array( Item::ENTITY_TYPE, 2, 1002 ),
+ array( Property::ENTITY_TYPE, 2, 2002 ),
+ array( Property::ENTITY_TYPE, 3, 2003 ),
) );
}
@@ -275,4 +286,53 @@
$this->assertArrayEquals( $expectedRecord,
$actualRecord, false, true );
}
}
+
+ public function provideRemoveMissing() {
+ return array(
+ array(
+ array(),
+ array()
+ ),
+
+ array(
+ array(
+ 'Q2' => array( 'id' => 'Q2', 'type' =>
Item::ENTITY_TYPE ),
+ ),
+ array(
+ 'Q2' => array( 'id' => 'Q2', 'type' =>
Item::ENTITY_TYPE ),
+ ),
+ ),
+
+ array(
+ array(
+ 'Q7' => array( 'id' => 'Q7', 'type' =>
Item::ENTITY_TYPE ),
+ ),
+ array()
+ ),
+
+ array(
+ array(
+ 'P2' => array( 'id' => 'P2', 'type' =>
Property::ENTITY_TYPE ),
+ 'Q7' => array( 'id' => 'Q7', 'type' =>
Item::ENTITY_TYPE ),
+ 'P7' => array( 'id' => 'P7', 'type' =>
Property::ENTITY_TYPE ),
+ 'Q2' => array( 'id' => 'Q2', 'type' =>
Item::ENTITY_TYPE ),
+ ),
+ array(
+ 'P2' => array( 'id' => 'P2', 'type' =>
Property::ENTITY_TYPE ),
+ 'Q2' => array( 'id' => 'Q2', 'type' =>
Item::ENTITY_TYPE ),
+ )
+ ),
+ );
+ }
+
+ /**
+ * @dataProvider provideRemoveMissing
+ */
+ public function testRemoveMissing( array $entityInfo, array $expected =
null ) {
+ $builder = $this->newEntityInfoBuilder();
+
+ $builder->removeMissing( $entityInfo );
+
+ $this->assertArrayEquals( array_keys( $expected ), array_keys(
$entityInfo ) );
+ }
}
diff --git a/repo/includes/EntityView.php b/repo/includes/EntityView.php
index d4214f9..ba9f730 100644
--- a/repo/includes/EntityView.php
+++ b/repo/includes/EntityView.php
@@ -785,16 +785,9 @@
//TODO: apply language fallback! Restore fallback test case in
EntityViewTest::provideRegisterJsConfigVars()
$entities = $this->entityInfoBuilder->buildEntityInfo(
$entityIds );
+ $this->entityInfoBuilder->removeMissing( $entities );
$this->entityInfoBuilder->addTerms( $entities, array( 'label',
'description' ), array( $langCode ) );
$this->entityInfoBuilder->addDataTypes( $entities );
-
- // filter non-existing properties
- $entities = array_filter( $entities,
- function ( $entity ) {
- return $entity['type'] !==
Property::ENTITY_TYPE || !empty( $entity['datatype'] );
- }
- );
-
$revisions = $this->attachRevisionInfo( $entities );
wfProfileOut( __METHOD__ );
diff --git a/repo/tests/phpunit/includes/EntityViewTest.php
b/repo/tests/phpunit/includes/EntityViewTest.php
index c91229c..e0e35d9 100644
--- a/repo/tests/phpunit/includes/EntityViewTest.php
+++ b/repo/tests/phpunit/includes/EntityViewTest.php
@@ -415,7 +415,16 @@
ksort( $expected );
ksort( $actual );
- $this->assertEquals( $expected, $actual );
+ $this->assertEquals( array_keys( $expected ), array_keys(
$actual ) );
+
+ foreach ( $expected as $field => $expectedJson ) {
+ $actualJson = $actual[$field];
+
+ $expectedData = json_decode( $expectedJson, true );
+ $actualData = json_decode( $actualJson, true );
+
+ $this->assertEquals( $expectedData, $actualData, $field
);
+ }
}
public function provideRegisterJsConfigVars() {
@@ -428,10 +437,12 @@
$entity->setId( new ItemId( 'Q22' ) );
$q33 = new ItemId( 'Q33' );
+ $q44 = new ItemId( 'Q44' ); // unknown item
$p11 = new PropertyId( 'p11' );
$p77 = new PropertyId( 'p77' ); // unknown property
$entity->addClaim( $this->makeClaim( new PropertyValueSnak(
$p11, new EntityIdValue( $q33 ) ) ) );
+ $entity->addClaim( $this->makeClaim( new PropertyValueSnak(
$p11, new EntityIdValue( $q44 ) ) ) );
$entity->addClaim( $this->makeClaim( new PropertyValueSnak(
$p77, new EntityIdValue( $q33 ) ) ) );
$revision = new EntityRevision( $entity, 1234567,
'20130505333333' );
@@ -486,10 +497,25 @@
),
'type' => 'claim',
),
+ array(
+ 'id' =>
'EntityViewTest$2',
+ 'mainsnak' => array(
+ 'snaktype' =>
'value',
+ 'property' =>
'P11',
+ 'datavalue' =>
array(
+ 'value'
=> array(
+
'entity-type' => 'item',
+
'numeric-id' => 44,
+ ),
+ 'type'
=> 'wikibase-entityid',
+ ),
+ ),
+ 'type' => 'claim',
+ ),
),
'P77' => array(
array(
- 'id' =>
'EntityViewTest$2',
+ 'id' =>
'EntityViewTest$3',
'mainsnak' => array(
'snaktype' =>
'value',
'property' =>
'P77',
--
To view, visit https://gerrit.wikimedia.org/r/97923
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I77af42488b4258ab9eb42682ac10123dd4a1d9ff
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Daniel Kinzler <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
