From 1ded833de19f27d2247053897b8655f9c39de76e Mon Sep 17 00:00:00 2001 From: Erik Bernhardson Date: Wed, 7 Oct 2015 14:03:15 -0700 Subject: [PATCH] Implement ApiPageSet::setRedirectMergePolicy() This allows generator implementations to define how generator data about a redirect gets merged into the redirect destination. This does not change any defaults, only modules that explicitly set a merge policy will have a change in their results. This functionality is initially used for the search api modules to retain search positions within the final result set. Bug: T92796 Change-Id: If2f49e0fc3176288c95e870240754ee320a6bf91 --- RELEASE-NOTES-1.27 | 3 + includes/api/ApiPageSet.php | 55 +++++++++++++ includes/api/ApiQueryPrefixSearch.php | 6 ++ includes/api/ApiQuerySearch.php | 6 ++ tests/phpunit/includes/api/ApiPageSetTest.php | 78 +++++++++++++++++++ 5 files changed, 148 insertions(+) create mode 100644 tests/phpunit/includes/api/ApiPageSetTest.php diff --git a/RELEASE-NOTES-1.27 b/RELEASE-NOTES-1.27 index c96eed2fde..d1c62e468c 100644 --- a/RELEASE-NOTES-1.27 +++ b/RELEASE-NOTES-1.27 @@ -41,6 +41,9 @@ production. === Action API changes in 1.27 === * Added list=allrevisions. * generator=recentchanges now has the option to generate revids. +* ApiPageSet::setRedirectMergePolicy() was added. This allows generator + modules to define how generator data for a redirect source gets merged + into the redirect destination. === Action API internal changes in 1.27 === diff --git a/includes/api/ApiPageSet.php b/includes/api/ApiPageSet.php index 17c148e25f..c6abf40e00 100644 --- a/includes/api/ApiPageSet.php +++ b/includes/api/ApiPageSet.php @@ -79,6 +79,8 @@ class ApiPageSet extends ApiBase { private $mRequestedPageFields = array(); /** @var int */ private $mDefaultNamespace = NS_MAIN; + /** @var callable|null */ + private $mRedirectMergePolicy; /** * Add all items from $values into the result @@ -1197,6 +1199,29 @@ class ApiPageSet extends ApiBase { $this->mGeneratorData[$ns][$dbkey] = $data; } + /** + * Controls how generator data about a redirect source is merged into + * the generator data for the redirect target. When not set no data + * is merged. Note that if multiple titles redirect to the same target + * the order of operations is undefined. + * + * Example to include generated data from redirect in target, prefering + * the data generated for the destination when there is a collision: + * @code + * $pageSet->setRedirectMergePolicy( function( array $current, array $new ) { + * return $current + $new; + * } ); + * @endcode + * + * @param callable|null $callable Recieves two array arguments, first the + * generator data for the redirect target and second the generator data + * for the redirect source. Returns the resulting generator data to use + * for the redirect target. + */ + public function setRedirectMergePolicy( $callable ) { + $this->mRedirectMergePolicy = $callable; + } + /** * Populate the generator data for all titles in the result * @@ -1270,6 +1295,36 @@ class ApiPageSet extends ApiBase { } } } + + // Merge data generated about redirect titles into the redirect destination + if ( $this->mRedirectMergePolicy ) { + foreach ( $this->mResolvedRedirectTitles as $titleFrom ) { + $dest = $titleFrom; + while ( isset( $this->mRedirectTitles[$dest->getPrefixedText()] ) ) { + $dest = $this->mRedirectTitles[$dest->getPrefixedText()]; + } + $fromNs = $titleFrom->getNamespace(); + $fromDBkey = $titleFrom->getDBkey(); + $toPageId = $dest->getArticleID(); + if ( isset( $data[$toPageId] ) && + isset( $this->mGeneratorData[$fromNs][$fromDBkey] ) + ) { + // It is necesary to set both $data and add to $result, if an ApiResult, + // to ensure multiple redirects to the same destination are all merged. + $data[$toPageId] = call_user_func( + $this->mRedirectMergePolicy, + $data[$toPageId], + $this->mGeneratorData[$fromNs][$fromDBkey] + ); + if ( $result instanceof ApiResult ) { + if ( !$result->addValue( $path, $toPageId, $data[$toPageId], ApiResult::OVERRIDE ) ) { + return false; + } + } + } + } + } + return true; } diff --git a/includes/api/ApiQueryPrefixSearch.php b/includes/api/ApiQueryPrefixSearch.php index 8eb644fcf9..25ff07c3f6 100644 --- a/includes/api/ApiQueryPrefixSearch.php +++ b/includes/api/ApiQueryPrefixSearch.php @@ -48,6 +48,12 @@ class ApiQueryPrefixSearch extends ApiQueryGeneratorBase { $searcher = new TitlePrefixSearch; $titles = $searcher->searchWithVariants( $search, $limit + 1, $namespaces, $offset ); if ( $resultPageSet ) { + $resultPageSet->setRedirectMergePolicy( function( array $current, array $new ) { + if ( !isset( $current['index'] ) || $new['index'] < $current['index'] ) { + $current['index'] = $new['index']; + } + return $current; + } ); if ( count( $titles ) > $limit ) { $this->setContinueEnumParameter( 'offset', $offset + $params['limit'] ); array_pop( $titles ); diff --git a/includes/api/ApiQuerySearch.php b/includes/api/ApiQuerySearch.php index b866f43e09..32607a5921 100644 --- a/includes/api/ApiQuerySearch.php +++ b/includes/api/ApiQuerySearch.php @@ -278,6 +278,12 @@ class ApiQuerySearch extends ApiQueryGeneratorBase { ), 'p' ); } } else { + $resultPageSet->setRedirectMergePolicy( function ( $current, $new ) { + if ( !isset( $current['index'] ) || $new['index'] < $current['index'] ) { + $current['index'] = $new['index']; + } + return $current; + } ); $resultPageSet->populateFromTitles( $titles ); $offset = $params['offset'] + 1; foreach ( $titles as $index => $title ) { diff --git a/tests/phpunit/includes/api/ApiPageSetTest.php b/tests/phpunit/includes/api/ApiPageSetTest.php new file mode 100644 index 0000000000..d2a4162a4b --- /dev/null +++ b/tests/phpunit/includes/api/ApiPageSetTest.php @@ -0,0 +1,78 @@ + array( + null, + array() + ), + + 'A simple merge policy adds the redirect data in' => array( + function( $current, $new ) { + if ( !isset( $current['index'] ) || $new['index'] < $current['index'] ) { + $current['index'] = $new['index']; + } + return $current; + }, + array( 'index' => 1 ), + ), + ); + } + + /** + * @dataProvider provideRedirectMergePolicy + */ + public function testRedirectMergePolicyWithArrayResult( $mergePolicy, $expect ) { + list( $target, $pageSet ) = $this->createPageSetWithRedirect(); + $pageSet->setRedirectMergePolicy( $mergePolicy ); + $result = array( + $target->getArticleID() => array() + ); + $pageSet->populateGeneratorData( $result ); + $this->assertEquals( $expect, $result[$target->getArticleID()] ); + } + + /** + * @dataProvider provideRedirectMergePolicy + */ + public function testRedirectMergePolicyWithApiResult( $mergePolicy, $expect ) { + list( $target, $pageSet ) = $this->createPageSetWithRedirect(); + $pageSet->setRedirectMergePolicy( $mergePolicy ); + $result = new ApiResult( false ); + $result->addValue( null, 'pages', array( + $target->getArticleID() => array() + ) ); + $pageSet->populateGeneratorData( $result, array( 'pages' ) ); + $this->assertEquals( + $expect, + $result->getResultData( array( 'pages', $target->getArticleID() ) ) + ); + } + + protected function createPageSetWithRedirect() { + $target = Title::makeTitle( NS_MAIN, 'UTRedirectTarget' ); + $sourceA = Title::makeTitle( NS_MAIN, 'UTRedirectSourceA' ); + $sourceB = Title::makeTitle( NS_MAIN, 'UTRedirectSourceB' ); + self::editPage( 'UTRedirectTarget', 'api page set test' ); + self::editPage( 'UTRedirectSourceA', '#REDIRECT [[UTRedirectTarget]]' ); + self::editPage( 'UTRedirectSourceB', '#REDIRECT [[UTRedirectTarget]]' ); + + $request = new FauxRequest( array( 'redirects' => 1 ) ); + $context = new RequestContext(); + $context->setRequest( $request ); + $main = new ApiMain( $context ); + $pageSet = new ApiPageSet( $main ); + + $pageSet->setGeneratorData( $sourceA, array( 'index' => 1 ) ); + $pageSet->setGeneratorData( $sourceB, array( 'index' => 3 ) ); + $pageSet->populateFromTitles( array( $sourceA, $sourceB ) ); + + return array( $target, $pageSet ); + } +} -- 2.20.1