From: Cindy Cicalese Date: Tue, 12 Jan 2016 04:29:48 +0000 (-0500) Subject: Add page_props table access class X-Git-Tag: 1.31.0-rc.0~8329 X-Git-Url: http://git.cyclocoop.org/%7B%24admin_url%7Dcompta/comptes/journal.php?a=commitdiff_plain;h=e48030a7aebb47eeb702d351716ba8304308b02f;p=lhc%2Fweb%2Fwiklou.git Add page_props table access class Bug:T115331 Change-Id: I022b9e3ca47dc63650b8a62260603b0893a80e69 --- diff --git a/autoload.php b/autoload.php index a25b562d08..2e504107f6 100644 --- a/autoload.php +++ b/autoload.php @@ -922,6 +922,7 @@ $wgAutoloadLocalClasses = array( 'PageExists' => __DIR__ . '/maintenance/pageExists.php', 'PageLangLogFormatter' => __DIR__ . '/includes/logging/PageLangLogFormatter.php', 'PageLinkRenderer' => __DIR__ . '/includes/title/PageLinkRenderer.php', + 'PageProps' => __DIR__ . '/includes/PageProps.php', 'PageQueryPage' => __DIR__ . '/includes/specialpage/PageQueryPage.php', 'Pager' => __DIR__ . '/includes/pager/Pager.php', 'ParameterizedPassword' => __DIR__ . '/includes/password/ParameterizedPassword.php', diff --git a/includes/PageProps.php b/includes/PageProps.php new file mode 100644 index 0000000000..0a3a324789 --- /dev/null +++ b/includes/PageProps.php @@ -0,0 +1,264 @@ +cache = new ProcessCacheLRU( self::CACHE_SIZE ); + } + + /** + * 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 + * + * @return array associative array mapping page ID to property value + * + */ + public function getProperty( $titles, $propertyName ) { + $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; + } + } + + if ( $queryIDs != array() ) { + $dbr = wfGetDB( DB_SLAVE ); + $result = $dbr->select( + 'page_props', + array( + 'pp_page', + 'pp_value' + ), + array( + 'pp_page' => $queryIDs, + 'pp_propname' => $propertyName + ), + __METHOD__ + ); + + foreach ( $result as $row ) { + $pageID = $row->pp_page; + $propertyValue = $row->pp_value; + $this->cacheProperty( $pageID, $propertyName, $propertyValue ); + $values[$pageID] = $propertyValue; + } + } + + return $values; + } + + /** + * Get all page property values. + * Given one or more Titles, returns an associative array mapping page + * ID to an associative array mapping property names to property + * values. Pages in the provided set of Titles that do not have any + * 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. An empty array will be returned if no + * matching properties were found. + * + * @param array|Title $titles + * + * @return array associative array mapping page ID to property value array + * + */ + public function getProperties( $titles ) { + $values = array(); + $goodIDs = $this->getGoodIDs( $titles ); + $queryIDs = array(); + foreach ( $goodIDs as $pageID ) { + $pageProperties = $this->getCachedProperties( $pageID ); + if ( $pageProperties === false ) { + $queryIDs[] = $pageID; + } else { + $values[$pageID] = $pageProperties; + } + } + + if ( $queryIDs != array() ) { + $dbr = wfGetDB( DB_SLAVE ); + $result = $dbr->select( + 'page_props', + array( + 'pp_page', + 'pp_propname', + 'pp_value' + ), + array( + 'pp_page' => $queryIDs, + ), + __METHOD__ + ); + + $currentPageID = 0; + $pageProperties = array(); + foreach ( $result as $row ) { + $pageID = $row->pp_page; + if ( $currentPageID != $pageID ) { + if ( $pageProperties != array() ) { + $this->cacheProperties( $currentPageID, $pageProperties ); + $values[$currentPageID] = $pageProperties; + } + $currentPageID = $pageID; + $pageProperties = array(); + } + $pageProperties[$row->pp_propname] = $row->pp_value; + } + if ( $pageProperties != array() ) { + $this->cacheProperties( $pageID, $pageProperties ); + $values[$pageID] = $pageProperties; + } + } + + return $values; + } + + /** + * @param array|Title $titles + * + * @return array array of good page IDs + * + */ + private function getGoodIDs( $titles ) { + $result = array(); + if ( is_array( $titles ) ) { + foreach ( $titles as $title ) { + $pageID = $title->getArticleID(); + if ( $pageID > 0 ) { + $result[] = $pageID; + } + } + } else { + $pageID = $titles->getArticleID(); + if ( $pageID > 0 ) { + $result[] = $pageID; + } + } + return $result; + } + + /** + * Get a property from the cache. + * + * @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 ) ) { + return $this->cache->get( $pageID, $propertyName ); + } + if ( $this->cache->has( 0, $pageID, self::CACHE_TTL ) ) { + $pageProperties = $this->cache->get( 0, $pageID ); + if ( isset( $pageProperties[$propertyName] ) ) { + return $pageProperties[$propertyName]; + } + } + return false; + } + + /** + * 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 ) ) { + return $this->cache->get( 0, $pageID ); + } + return false; + } + + /** + * Save a property to the cache. + * + * @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 ); + } + + /** + * 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 + * + */ + private function cacheProperties( $pageID, $pageProperties ) { + $this->cache->clear( $pageID ); + $this->cache->set( 0, $pageID, $pageProperties ); + } +} diff --git a/includes/actions/InfoAction.php b/includes/actions/InfoAction.php index 7389ae2c02..6f33db723d 100644 --- a/includes/actions/InfoAction.php +++ b/includes/actions/InfoAction.php @@ -203,18 +203,10 @@ class InfoAction extends FormlessAction { $pageCounts = $this->pageCounts( $this->page ); - // Get page properties - $dbr = wfGetDB( DB_SLAVE ); - $result = $dbr->select( - 'page_props', - array( 'pp_propname', 'pp_value' ), - array( 'pp_page' => $id ), - __METHOD__ - ); - $pageProperties = array(); - foreach ( $result as $row ) { - $pageProperties[$row->pp_propname] = $row->pp_value; + $props = PageProps::getInstance()->getProperties( $title ); + if ( isset( $props[$id] ) ) { + $pageProperties = $props[$id]; } // Basic information diff --git a/includes/api/ApiQueryPageProps.php b/includes/api/ApiQueryPageProps.php index 1f992f8ff5..c2a8df7d0f 100644 --- a/includes/api/ApiQueryPageProps.php +++ b/includes/api/ApiQueryPageProps.php @@ -40,63 +40,44 @@ 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'] ); + $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(); - - 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; + if ( $this->params['prop'] ) { + $propnames = $this->params['prop']; + $properties = array(); + foreach ( $propnames as $propname ) { + $values = $pageProps->getProperty( $pages, $propname ); + foreach ( $values as $page => $value ) { + if ( !isset( $properties[$page] ) ) { + $properties[$page] = array(); } - - $props = array(); + $properties[$page][$propname] = $value; } - - $currentPage = $row->pp_page; } - - $props[$row->pp_propname] = $row->pp_value; + } else { + $properties = $pageProps->getProperties( $pages ); } - - if ( count( $props ) ) { - # Add any remaining properties to the results - $this->addPageProps( $result, $currentPage, $props ); + foreach ( $properties as $page => $props ) { + $this->addPageProps( $result, $page, $props ); } } diff --git a/tests/phpunit/includes/PagePropsTest.php b/tests/phpunit/includes/PagePropsTest.php new file mode 100644 index 0000000000..9a1f5973a3 --- /dev/null +++ b/tests/phpunit/includes/PagePropsTest.php @@ -0,0 +1,276 @@ +resetNamespaces(); # reset namespace cache + + if ( !$this->the_properties ) { + $this->the_properties = array( + "property1" => "value1", + "property2" => "value2", + "property3" => "value3", + "property4" => "value4" + ); + } + + if ( !$this->title1 ) { + $page = $this->createPage( + 'PagePropsTest_page_1', + "just a dummy page", + CONTENT_MODEL_WIKITEXT + ); + $this->title1 = $page->getTitle(); + $page1ID = $this->title1->getArticleID(); + $this->setProperties( $page1ID, $this->the_properties ); + } + + if ( !$this->title2 ) { + $page = $this->createPage( + 'PagePropsTest_page_2', + "just a dummy page", + CONTENT_MODEL_WIKITEXT + ); + $this->title2 = $page->getTitle(); + $page2ID = $this->title2->getArticleID(); + $this->setProperties( $page2ID, $this->the_properties ); + } + } + + protected function tearDown() { + global $wgExtraNamespaces, $wgNamespaceContentModels, $wgContentHandlers, $wgContLang; + + parent::tearDown(); + + unset( $wgExtraNamespaces[12312] ); + unset( $wgExtraNamespaces[12313] ); + + unset( $wgNamespaceContentModels[12312] ); + unset( $wgContentHandlers['DUMMY'] ); + + MWNamespace::getCanonicalNamespaces( true ); # reset namespace cache + $wgContLang->resetNamespaces(); # reset namespace cache + } + + /** + * Test getting a single property from a single page. The property was + * set in setUp(). + */ + public function testGetSingleProperty() { + $pageProps = PageProps::getInstance(); + $page1ID = $this->title1->getArticleID(); + $result = $pageProps->getProperty( $this->title1, "property1" ); + $this->assertArrayHasKey( $page1ID, $result, "Found property" ); + $this->assertEquals( $result[$page1ID], "value1", "Get property" ); + } + + /** + * Test getting a single property from multiple pages. The property was + * set in setUp(). + */ + public function testGetSinglePropertyMultiplePages() { + $pageProps = PageProps::getInstance(); + $page1ID = $this->title1->getArticleID(); + $page2ID = $this->title2->getArticleID(); + $titles = array( + $this->title1, + $this->title2 + ); + $result = $pageProps->getProperty( $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 all properties from a single page. The properties were + * set in setUp(). The properties retrieved from the page may include + * additional properties not set in the test case that are added by + * other extensions. Therefore, rather than checking to see if the + * properties that were set in the test case exactly match the + * retrieved properties, we need to check to see if they are a + * subset of the retrieved properties. Since this version of PHPUnit + * does not yet include assertArraySubset(), we needed to code the + * equivalent functionality. + */ + public function testGetAllProperties() { + $pageProps = PageProps::getInstance(); + $page1ID = $this->title1->getArticleID(); + $result = $pageProps->getProperties( $this->title1 ); + $this->assertArrayHasKey( $page1ID, $result, "Found properties" ); + $properties = $result[$page1ID]; + $patched = array_replace_recursive( $properties, $this->the_properties ); + $this->assertEquals( $patched, $properties, "Get all properties" ); + } + + /** + * Test getting all properties from multiple pages. The properties were + * set in setUp(). See getAllProperties() above for more information. + */ + public function testGetAllPropertiesMultiplePages() { + $pageProps = PageProps::getInstance(); + $page1ID = $this->title1->getArticleID(); + $page2ID = $this->title2->getArticleID(); + $titles = array( + $this->title1, + $this->title2 + ); + $result = $pageProps->getProperties( $titles ); + $this->assertArrayHasKey( $page1ID, $result, "Found page 1 properties" ); + $this->assertArrayHasKey( $page2ID, $result, "Found page 2 properties" ); + $properties1 = $result[$page1ID]; + $patched = array_replace_recursive( $properties1, $this->the_properties ); + $this->assertEquals( $patched, $properties1, "Get all properties page 1" ); + $properties2 = $result[$page2ID]; + $patched = array_replace_recursive( $properties2, $this->the_properties ); + $this->assertEquals( $patched, $properties2, "Get all properties page 2" ); + } + + /** + * Test caching when retrieving single properties by getting a property, + * saving a new value for the property, then getting the property + * again. The cached value for the property rather than the new value + * of the property should be returned. + */ + public function testSingleCache() { + $pageProps = PageProps::getInstance(); + $page1ID = $this->title1->getArticleID(); + $value1 = $pageProps->getProperty( $this->title1, "property1" ); + $this->setProperty( $page1ID, "property1", "another value" ); + $value2 = $pageProps->getProperty( $this->title1, "property1" ); + $this->assertEquals( $value1, $value2, "Single cache" ); + } + + /** + * Test caching when retrieving all properties by getting all + * properties, saving a new value for a property, then getting all + * properties again. The cached value for the properties rather than the + * new value of the properties should be returned. + */ + public function testMultiCache() { + $pageProps = PageProps::getInstance(); + $page1ID = $this->title1->getArticleID(); + $properties1 = $pageProps->getProperties( $this->title1 ); + $this->setProperty( $page1ID, "property1", "another value" ); + $properties2 = $pageProps->getProperties( $this->title1 ); + $this->assertEquals( $properties1, $properties2, "Multi Cache" ); + } + + /** + * Test that getting all properties clears the single properties + * that have been cached by getting a property, saving a new value for + * the property, getting all properties (which clears the cached single + * properties), then getting the property again. The new value for the + * property rather than the cached value of the property should be + * returned. + */ + public function testClearCache() { + $pageProps = PageProps::getInstance(); + $page1ID = $this->title1->getArticleID(); + $pageProps->getProperty( $this->title1, "property1" ); + $new_value = "another value"; + $this->setProperty( $page1ID, "property1", $new_value ); + $pageProps->getProperties( $this->title1 ); + $result = $pageProps->getProperty( $this->title1, "property1" ); + $this->assertArrayHasKey( $page1ID, $result, "Found property" ); + $this->assertEquals( $result[$page1ID], "another value", "Clear cache" ); + } + + protected function createPage( $page, $text, $model = null ) { + if ( is_string( $page ) ) { + if ( !preg_match( '/:/', $page ) && + ( $model === null || $model === CONTENT_MODEL_WIKITEXT ) + ) { + $ns = $this->getDefaultWikitextNS(); + $page = MWNamespace::getCanonicalName( $ns ) . ':' . $page; + } + + $page = Title::newFromText( $page ); + } + + if ( $page instanceof Title ) { + $page = new WikiPage( $page ); + } + + if ( $page->exists() ) { + $page->doDeleteArticle( "done" ); + } + + $content = ContentHandler::makeContent( $text, $page->getTitle(), $model ); + $page->doEditContent( $content, "testing", EDIT_NEW ); + + return $page; + } + + protected function setProperties( $pageID, $properties ) { + + $rows = array(); + + foreach ( $properties as $propertyName => $propertyValue ) { + + $row = array( + 'pp_page' => $pageID, + 'pp_propname' => $propertyName, + 'pp_value' => $propertyValue + ); + + $rows[] = $row; + } + + $dbw = wfGetDB( DB_MASTER ); + $dbw->replace( + 'page_props', + array( + array( + 'pp_page', + 'pp_propname' + ) + ), + $rows, + __METHOD__ + ); + } + + protected function setProperty( $pageID, $propertyName, $propertyValue ) { + + $properties = array(); + $properties[$propertyName] = $propertyValue; + + $this->setProperties( $pageID, $properties ); + + } +}