From 86c08b240167d01217d4cd628e978078b1ed7011 Mon Sep 17 00:00:00 2001 From: Cindy Cicalese Date: Sun, 14 Feb 2016 16:58:46 -0500 Subject: [PATCH] Converted ApiQueryPageProps to use PageProps; added multi-property query to PageProps. Change-Id: Icd4540001e044052ae5759c87c8b83a70ab5c30f --- includes/PageProps.php | 81 ++++++++++++++---------- includes/actions/InfoAction.php | 2 +- includes/api/ApiQueryPageProps.php | 68 +++++++------------- tests/phpunit/includes/PagePropsTest.php | 51 +++++++++++---- 4 files changed, 111 insertions(+), 91 deletions(-) diff --git a/includes/PageProps.php b/includes/PageProps.php index 0a3a324789..2d6e535234 100644 --- a/includes/PageProps.php +++ b/includes/PageProps.php @@ -58,53 +58,76 @@ class PageProps { } /** - * 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. - * - * @param array|Title $titles - * @param string $propertyName + * 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 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 @@ class PageProps { * 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 @@ class PageProps { } /** - * @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 @@ class PageProps { * * @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 @@ class PageProps { * 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 @@ class PageProps { * @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 @@ class PageProps { * 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 a3bd93aa5d..4027b3570b 100644 --- a/includes/actions/InfoAction.php +++ b/includes/actions/InfoAction.php @@ -204,7 +204,7 @@ class InfoAction extends FormlessAction { $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 1f992f8ff5..ca85fe5f6c 100644 --- a/includes/api/ApiQueryPageProps.php +++ b/includes/api/ApiQueryPageProps.php @@ -40,63 +40,41 @@ class ApiQueryPageProps extends ApiQueryBase { public function execute() { # Only operate on existing pages $pages = $this->getPageSet()->getGoodTitles(); - 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'] ); + $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; } - # 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' ); + if ( !count( $pages ) ) { + # Nothing to do + return; } - $res = $this->select( __METHOD__ ); - $currentPage = 0; # Id of the page currently processed + $pageProps = PageProps::getInstance(); $props = array(); $result = $this->getResult(); + if ( $this->params['prop'] ) { + $propnames = $this->params['prop']; + $properties = $pageProps->getProperties( $pages, $propnames ); + } else { + $properties = $pageProps->getAllProperties( $pages ); + } - 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(); - } + ksort( $properties ); - $currentPage = $row->pp_page; + foreach ( $properties as $page => $props ) { + if ( !$this->addPageProps( $result, $page, $props ) ) { + break; } - - $props[$row->pp_propname] = $row->pp_value; - } - - if ( count( $props ) ) { - # Add any remaining properties to the results - $this->addPageProps( $result, $currentPage, $props ); } } diff --git a/tests/phpunit/includes/PagePropsTest.php b/tests/phpunit/includes/PagePropsTest.php index 9a1f5973a3..e3d69deb8a 100644 --- a/tests/phpunit/includes/PagePropsTest.php +++ b/tests/phpunit/includes/PagePropsTest.php @@ -92,7 +92,7 @@ class TestPageProps extends MediaWikiLangTestCase { 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,13 +109,42 @@ class TestPageProps extends MediaWikiLangTestCase { $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" ); + } + /** * Test getting all properties from a single page. The properties were * set in setUp(). The properties retrieved from the page may include @@ -130,7 +159,7 @@ class TestPageProps extends MediaWikiLangTestCase { 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 @@ class TestPageProps extends MediaWikiLangTestCase { $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 @@ class TestPageProps extends MediaWikiLangTestCase { 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 @@ class TestPageProps extends MediaWikiLangTestCase { 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 @@ class TestPageProps extends MediaWikiLangTestCase { 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" ); } -- 2.20.1