Implement ApiPageSet::setRedirectMergePolicy()
authorErik Bernhardson <ebernhardson@wikimedia.org>
Wed, 7 Oct 2015 21:03:15 +0000 (14:03 -0700)
committerErik Bernhardson <ebernhardson@wikimedia.org>
Thu, 8 Oct 2015 18:36:02 +0000 (11:36 -0700)
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
includes/api/ApiPageSet.php
includes/api/ApiQueryPrefixSearch.php
includes/api/ApiQuerySearch.php
tests/phpunit/includes/api/ApiPageSetTest.php [new file with mode: 0644]

index c96eed2..d1c62e4 100644 (file)
@@ -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 ===
 
index 17c148e..c6abf40 100644 (file)
@@ -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;
        }
 
index 8eb644f..25ff07c 100644 (file)
@@ -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 );
index b866f43..32607a5 100644 (file)
@@ -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 (file)
index 0000000..d2a4162
--- /dev/null
@@ -0,0 +1,78 @@
+<?php
+
+/**
+ * @group API
+ * @group medium
+ * @group Database
+ */
+class ApiPageSetTest extends ApiTestCase {
+       public static function provideRedirectMergePolicy() {
+               return array(
+                       'By default nothing is merged' => 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 );
+       }
+}