jenkins-bot has submitted this change and it was merged.
Change subject: Converted ApiQueryPageProps to use PageProps; added
multi-property query to PageProps.
......................................................................
Converted ApiQueryPageProps to use PageProps; added multi-property query to
PageProps.
Change-Id: Icd4540001e044052ae5759c87c8b83a70ab5c30f
---
M includes/PageProps.php
M includes/actions/InfoAction.php
M includes/api/ApiQueryPageProps.php
M tests/phpunit/includes/PagePropsTest.php
4 files changed, 114 insertions(+), 94 deletions(-)
Approvals:
Anomie: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/PageProps.php b/includes/PageProps.php
index 0a3a324..2d6e535 100644
--- a/includes/PageProps.php
+++ b/includes/PageProps.php
@@ -58,53 +58,76 @@
}
/**
- * Given one or more Titles and the name of a property, returns an
- * associative array mapping page ID to property value. Pages in the
- * provided set of Titles that do not have a value for the given
- * property will not appear in the returned array. If a single Title
- * is provided, it does not need to be passed in an array, but an array
- * will always be returned. An empty array will be returned if no
- * matching properties were found.
+ * Given one or more Titles and one or more names of properties,
+ * returns an associative array mapping page ID to property value.
+ * Pages in the provided set of Titles that do not have a value for
+ * the given properties will not appear in the returned array. If a
+ * single Title is provided, it does not need to be passed in an array,
+ * but an array will always be returned. If a single property name is
+ * provided, it does not need to be passed in an array. In that case,
+ * an associtive array mapping page ID to property value will be
+ * returned; otherwise, an associative array mapping page ID to
+ * an associative array mapping property name to property value will be
+ * returned. An empty array will be returned if no matching properties
+ * were found.
*
- * @param array|Title $titles
- * @param string $propertyName
- *
+ * @param Title[]|Title $titles
+ * @param string[]|string $propertyNames
* @return array associative array mapping page ID to property value
- *
*/
- public function getProperty( $titles, $propertyName ) {
+ public function getProperties( $titles, $propertyNames ) {
+ if ( is_array( $propertyNames ) ) {
+ $gotArray = true;
+ } else {
+ $propertyNames = array( $propertyNames );
+ $gotArray = false;
+ }
+
$values = array();
$goodIDs = $this->getGoodIDs( $titles );
$queryIDs = array();
foreach ( $goodIDs as $pageID ) {
- $propertyValue = $this->getCachedProperty( $pageID,
$propertyName );
- if ( $propertyValue === false ) {
- $queryIDs[] = $pageID;
- } else {
- $values[$pageID] = $propertyValue;
+ foreach ( $propertyNames as $propertyName ) {
+ $propertyValue = $this->getCachedProperty(
$pageID, $propertyName );
+ if ( $propertyValue === false ) {
+ $queryIDs[] = $pageID;
+ break;
+ } else {
+ if ( $gotArray ) {
+ $values[$pageID][$propertyName]
= $propertyValue;
+ } else {
+ $values[$pageID] =
$propertyValue;
+ }
+ }
}
}
- if ( $queryIDs != array() ) {
+ if ( $queryIDs ) {
$dbr = wfGetDB( DB_SLAVE );
$result = $dbr->select(
'page_props',
array(
'pp_page',
+ 'pp_propname',
'pp_value'
),
array(
'pp_page' => $queryIDs,
- 'pp_propname' => $propertyName
+ 'pp_propname' => $propertyNames
),
__METHOD__
);
foreach ( $result as $row ) {
$pageID = $row->pp_page;
+ $propertyName = $row->pp_propname;
$propertyValue = $row->pp_value;
$this->cacheProperty( $pageID, $propertyName,
$propertyValue );
- $values[$pageID] = $propertyValue;
+ if ( $gotArray ) {
+ $values[$pageID][$propertyName] =
$propertyValue;
+ } else {
+ $values[$pageID] = $propertyValue;
+ }
}
}
@@ -121,12 +144,10 @@
* will always be returned. An empty array will be returned if no
* matching properties were found.
*
- * @param array|Title $titles
- *
+ * @param Title[]|Title $titles
* @return array associative array mapping page ID to property value
array
- *
*/
- public function getProperties( $titles ) {
+ public function getAllProperties( $titles ) {
$values = array();
$goodIDs = $this->getGoodIDs( $titles );
$queryIDs = array();
@@ -178,10 +199,8 @@
}
/**
- * @param array|Title $titles
- *
+ * @param Title[]|Title $titles
* @return array array of good page IDs
- *
*/
private function getGoodIDs( $titles ) {
$result = array();
@@ -206,9 +225,7 @@
*
* @param int $pageID page ID of page being queried
* @param string $propertyName name of property being queried
- *
* @return string|bool property value array or false if not found
- *
*/
private function getCachedProperty( $pageID, $propertyName ) {
if ( $this->cache->has( $pageID, $propertyName, self::CACHE_TTL
) ) {
@@ -227,9 +244,7 @@
* Get properties from the cache.
*
* @param int $pageID page ID of page being queried
- *
* @return string|bool property value array or false if not found
- *
*/
private function getCachedProperties( $pageID ) {
if ( $this->cache->has( 0, $pageID, self::CACHE_TTL ) ) {
@@ -244,7 +259,6 @@
* @param int $pageID page ID of page being cached
* @param string $propertyName name of property being cached
* @param mixed $propertyValue value of property
- *
*/
private function cacheProperty( $pageID, $propertyName, $propertyValue
) {
$this->cache->set( $pageID, $propertyName, $propertyValue );
@@ -254,8 +268,7 @@
* Save properties to the cache.
*
* @param int $pageID page ID of page being cached
- * @param array $pageProperties associative array of page properties to
be cached
- *
+ * @param string[] $pageProperties associative array of page properties
to be cached
*/
private function cacheProperties( $pageID, $pageProperties ) {
$this->cache->clear( $pageID );
diff --git a/includes/actions/InfoAction.php b/includes/actions/InfoAction.php
index a3bd93a..4027b35 100644
--- a/includes/actions/InfoAction.php
+++ b/includes/actions/InfoAction.php
@@ -204,7 +204,7 @@
$pageCounts = $this->pageCounts( $this->page );
$pageProperties = array();
- $props = PageProps::getInstance()->getProperties( $title );
+ $props = PageProps::getInstance()->getAllProperties( $title );
if ( isset( $props[$id] ) ) {
$pageProperties = $props[$id];
}
diff --git a/includes/api/ApiQueryPageProps.php
b/includes/api/ApiQueryPageProps.php
index 1f992f8..ca85fe5 100644
--- a/includes/api/ApiQueryPageProps.php
+++ b/includes/api/ApiQueryPageProps.php
@@ -40,63 +40,41 @@
public function execute() {
# Only operate on existing pages
$pages = $this->getPageSet()->getGoodTitles();
+
+ $this->params = $this->extractRequestParams();
+ if ( $this->params['continue'] ) {
+ $continueValue = intval( $this->params['continue'] );
+ $this->dieContinueUsageIf( strval( $continueValue ) !==
$this->params['continue'] );
+ $filteredPages = array();
+ foreach ( $pages as $id => $page ) {
+ if ( $id >= $continueValue ) {
+ $filteredPages[$id] = $page;
+ }
+ }
+ $pages = $filteredPages;
+ }
+
if ( !count( $pages ) ) {
# Nothing to do
return;
}
- $this->params = $this->extractRequestParams();
-
- $this->addTables( 'page_props' );
- $this->addFields( array( 'pp_page', 'pp_propname', 'pp_value' )
);
- $this->addWhereFld( 'pp_page', array_keys( $pages ) );
-
- if ( $this->params['continue'] ) {
- $this->addWhere( 'pp_page >=' . intval(
$this->params['continue'] ) );
- }
-
- if ( $this->params['prop'] ) {
- $this->addWhereFld( 'pp_propname',
$this->params['prop'] );
- }
-
- # Force a sort order to ensure that properties are grouped by
page
- # But only if pp_page is not constant in the WHERE clause.
- if ( count( $pages ) > 1 ) {
- $this->addOption( 'ORDER BY', 'pp_page' );
- }
-
- $res = $this->select( __METHOD__ );
- $currentPage = 0; # Id of the page currently processed
+ $pageProps = PageProps::getInstance();
$props = array();
$result = $this->getResult();
-
- foreach ( $res as $row ) {
- if ( $currentPage != $row->pp_page ) {
- # Different page than previous row, so add the
properties to
- # the result and save the new page id
-
- if ( $currentPage ) {
- if ( !$this->addPageProps( $result,
$currentPage, $props ) ) {
- # addPageProps() indicated that
the result did not fit
- # so stop adding data. Reset
props so that it doesn't
- # get added again after loop
exit
-
- $props = array();
- break;
- }
-
- $props = array();
- }
-
- $currentPage = $row->pp_page;
- }
-
- $props[$row->pp_propname] = $row->pp_value;
+ if ( $this->params['prop'] ) {
+ $propnames = $this->params['prop'];
+ $properties = $pageProps->getProperties( $pages,
$propnames );
+ } else {
+ $properties = $pageProps->getAllProperties( $pages );
}
- if ( count( $props ) ) {
- # Add any remaining properties to the results
- $this->addPageProps( $result, $currentPage, $props );
+ ksort( $properties );
+
+ foreach ( $properties as $page => $props ) {
+ if ( !$this->addPageProps( $result, $page, $props ) ) {
+ break;
+ }
}
}
diff --git a/tests/phpunit/includes/PagePropsTest.php
b/tests/phpunit/includes/PagePropsTest.php
index 9a1f597..e3d69de 100644
--- a/tests/phpunit/includes/PagePropsTest.php
+++ b/tests/phpunit/includes/PagePropsTest.php
@@ -92,7 +92,7 @@
public function testGetSingleProperty() {
$pageProps = PageProps::getInstance();
$page1ID = $this->title1->getArticleID();
- $result = $pageProps->getProperty( $this->title1, "property1" );
+ $result = $pageProps->getProperties( $this->title1, "property1"
);
$this->assertArrayHasKey( $page1ID, $result, "Found property" );
$this->assertEquals( $result[$page1ID], "value1", "Get
property" );
}
@@ -109,11 +109,40 @@
$this->title1,
$this->title2
);
- $result = $pageProps->getProperty( $titles, "property1" );
+ $result = $pageProps->getProperties( $titles, "property1" );
$this->assertArrayHasKey( $page1ID, $result, "Found page 1
property" );
$this->assertArrayHasKey( $page2ID, $result, "Found page 2
property" );
$this->assertEquals( $result[$page1ID], "value1", "Get property
page 1" );
$this->assertEquals( $result[$page2ID], "value1", "Get property
page 2" );
+ }
+
+ /**
+ * Test getting multiple properties from multiple pages. The properties
+ * were set in setUp().
+ */
+ public function testGetMultiplePropertiesMultiplePages() {
+ $pageProps = PageProps::getInstance();
+ $page1ID = $this->title1->getArticleID();
+ $page2ID = $this->title2->getArticleID();
+ $titles = array(
+ $this->title1,
+ $this->title2
+ );
+ $properties = array(
+ "property1",
+ "property2"
+ );
+ $result = $pageProps->getProperties( $titles, $properties );
+ $this->assertArrayHasKey( $page1ID, $result, "Found page 1
property" );
+ $this->assertArrayHasKey( "property1", $result[$page1ID],
"Found page 1 property 1" );
+ $this->assertArrayHasKey( "property2", $result[$page1ID],
"Found page 1 property 2" );
+ $this->assertArrayHasKey( $page2ID, $result, "Found page 2
property" );
+ $this->assertArrayHasKey( "property1", $result[$page2ID],
"Found page 2 property 1" );
+ $this->assertArrayHasKey( "property2", $result[$page2ID],
"Found page 2 property 2" );
+ $this->assertEquals( $result[$page1ID]["property1"], "value1",
"Get page 1 property 1" );
+ $this->assertEquals( $result[$page1ID]["property2"], "value2",
"Get page 1 property 2" );
+ $this->assertEquals( $result[$page2ID]["property1"], "value1",
"Get page 2 property 1" );
+ $this->assertEquals( $result[$page2ID]["property2"], "value2",
"Get page 2 property 2" );
}
/**
@@ -130,7 +159,7 @@
public function testGetAllProperties() {
$pageProps = PageProps::getInstance();
$page1ID = $this->title1->getArticleID();
- $result = $pageProps->getProperties( $this->title1 );
+ $result = $pageProps->getAllProperties( $this->title1 );
$this->assertArrayHasKey( $page1ID, $result, "Found properties"
);
$properties = $result[$page1ID];
$patched = array_replace_recursive( $properties,
$this->the_properties );
@@ -149,7 +178,7 @@
$this->title1,
$this->title2
);
- $result = $pageProps->getProperties( $titles );
+ $result = $pageProps->getAllProperties( $titles );
$this->assertArrayHasKey( $page1ID, $result, "Found page 1
properties" );
$this->assertArrayHasKey( $page2ID, $result, "Found page 2
properties" );
$properties1 = $result[$page1ID];
@@ -169,9 +198,9 @@
public function testSingleCache() {
$pageProps = PageProps::getInstance();
$page1ID = $this->title1->getArticleID();
- $value1 = $pageProps->getProperty( $this->title1, "property1" );
+ $value1 = $pageProps->getProperties( $this->title1, "property1"
);
$this->setProperty( $page1ID, "property1", "another value" );
- $value2 = $pageProps->getProperty( $this->title1, "property1" );
+ $value2 = $pageProps->getProperties( $this->title1, "property1"
);
$this->assertEquals( $value1, $value2, "Single cache" );
}
@@ -184,9 +213,9 @@
public function testMultiCache() {
$pageProps = PageProps::getInstance();
$page1ID = $this->title1->getArticleID();
- $properties1 = $pageProps->getProperties( $this->title1 );
+ $properties1 = $pageProps->getAllProperties( $this->title1 );
$this->setProperty( $page1ID, "property1", "another value" );
- $properties2 = $pageProps->getProperties( $this->title1 );
+ $properties2 = $pageProps->getAllProperties( $this->title1 );
$this->assertEquals( $properties1, $properties2, "Multi Cache"
);
}
@@ -201,11 +230,11 @@
public function testClearCache() {
$pageProps = PageProps::getInstance();
$page1ID = $this->title1->getArticleID();
- $pageProps->getProperty( $this->title1, "property1" );
+ $pageProps->getProperties( $this->title1, "property1" );
$new_value = "another value";
$this->setProperty( $page1ID, "property1", $new_value );
- $pageProps->getProperties( $this->title1 );
- $result = $pageProps->getProperty( $this->title1, "property1" );
+ $pageProps->getAllProperties( $this->title1 );
+ $result = $pageProps->getProperties( $this->title1, "property1"
);
$this->assertArrayHasKey( $page1ID, $result, "Found property" );
$this->assertEquals( $result[$page1ID], "another value", "Clear
cache" );
}
--
To view, visit https://gerrit.wikimedia.org/r/270644
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Icd4540001e044052ae5759c87c8b83a70ab5c30f
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Cicalese <[email protected]>
Gerrit-Reviewer: Anomie <[email protected]>
Gerrit-Reviewer: Cicalese <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
