Converted ApiQueryPageProps to use PageProps; added multi-property query to PageProps.
authorCindy Cicalese <cicalese@mitre.org>
Sun, 14 Feb 2016 21:58:46 +0000 (16:58 -0500)
committerCindy Cicalese <cicalese@mitre.org>
Tue, 16 Feb 2016 18:39:47 +0000 (13:39 -0500)
Change-Id: Icd4540001e044052ae5759c87c8b83a70ab5c30f

includes/PageProps.php
includes/actions/InfoAction.php
includes/api/ApiQueryPageProps.php
tests/phpunit/includes/PagePropsTest.php

index 0a3a324..2d6e535 100644 (file)
@@ -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 );
index a3bd93a..4027b35 100644 (file)
@@ -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];
                }
index 1f992f8..ca85fe5 100644 (file)
@@ -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 );
                }
        }
 
index 9a1f597..e3d69de 100644 (file)
@@ -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" );
        }