Merge "SearchHandler: emit Cache-Control header."

includes/Rest/Handler/SearchHandler.php

@@ -2,6 +2,7 @@
 
 namespace MediaWiki\Rest\Handler;
 
+use Config;
 use InvalidArgumentException;
 use ISearchResultSet;
 use MediaWiki\Permissions\PermissionManager;
@@ -70,11 +71,21 @@ class SearchHandler extends Handler {
 	private const OFFSET = 0;
 
 	/**
+	 * Expiry time for use as max-age value in the cache-control header
+	 * of completion search responses.
+	 * @see $wgSearchSuggestCacheExpiry
+	 * @var int|null
+	 */
+	private $completionCacheExpiry;
+
+	/**
+	 * @param Config $config
 	 * @param PermissionManager $permissionManager
 	 * @param SearchEngineFactory $searchEngineFactory
 	 * @param SearchEngineConfig $searchEngineConfig
 	 */
 	public function __construct(
+		Config $config,
 		PermissionManager $permissionManager,
 		SearchEngineFactory $searchEngineFactory,
 		SearchEngineConfig $searchEngineConfig
@@ -83,8 +94,11 @@ class SearchHandler extends Handler {
 		$this->searchEngineFactory = $searchEngineFactory;
 		$this->searchEngineConfig = $searchEngineConfig;
 
-		// @todo Inject this, when there is a good way to do that
+		// @todo Inject this, when there is a good way to do that, see T239753
 		$this->user = RequestContext::getMain()->getUser();
+
+		// @todo Avoid injecting the entire config, see T246377
+		$this->completionCacheExpiry = $config->get( 'SearchSuggestCacheExpiry' );
 	}
 
 	public function init(
@@ -191,8 +205,8 @@ class SearchHandler extends Handler {
 	private function buildOutputFromSuggestions( array $suggestions ) {
 		$pages = [];
 		$foundPageIds = [];
-		foreach ( $suggestions as $suggestion ) {
-			$title = $suggestion->getSuggestedTitle();
+		foreach ( $suggestions as $sugg ) {
+			$title = $sugg->getSuggestedTitle();
 			if ( $title && $title->exists() ) {
 				$pageID = $title->getArticleID();
 				if ( !isset( $foundPageIds[$pageID] ) &&
@@ -202,7 +216,7 @@ class SearchHandler extends Handler {
 						'id' => $pageID,
 						'key' => $title->getPrefixedDBkey(),
 						'title' => $title->getPrefixedText(),
-						'excerpt' => $suggestion->getText() ?: null,
+						'excerpt' => $sugg->getText() ?: null,
 					];
 					$pages[] = $page;
 					$foundPageIds[$pageID] = true;
@@ -250,7 +264,16 @@ class SearchHandler extends Handler {
 	public function execute() {
 		$searchEngine = $this->createSearchEngine();
 		$results = $this->doSearch( $searchEngine );
-		return $this->getResponseFactory()->createJson( [ 'pages' => $results ] );
+		$response = $this->getResponseFactory()->createJson( [ 'pages' => $results ] );
+
+		if ( $this->mode === self::COMPLETION_MODE && $this->completionCacheExpiry ) {
+			// Type-ahead completion matches should be cached by the client and
+			// in the CDN, especially for short prefixes.
+			// See also $wgSearchSuggestCacheExpiry and ApiOpenSearch
+			$response->setHeader( 'Cache-Control', 'public, max-age=' . $this->completionCacheExpiry );
+		}
+
+		return $response;
 	}
 
 	public function getParamSettings() {

includes/Rest/coreDevelopmentRoutes.json

@@ -3,6 +3,7 @@
 		"path": "/coredev/v0/search/page",
 		"class": "MediaWiki\\Rest\\Handler\\SearchHandler",
 		"services": [
+			"MainConfig",
 			"PermissionManager",
 			"SearchEngineFactory",
 			"SearchEngineConfig"
@@ -13,6 +14,7 @@
 		"path": "/coredev/v0/search/title",
 		"class": "MediaWiki\\Rest\\Handler\\SearchHandler",
 		"services": [
+			"MainConfig",
 			"PermissionManager",
 			"SearchEngineFactory",
 			"SearchEngineConfig"

tests/api-testing/REST/Search.js

@@ -27,7 +27,7 @@ describe( 'Search', () => {
 			assert.deepEqual( noResultsResponse, body );
 		} );
 		it( 'should return array of pages when there is only a text match', async () => {
-			const { body } = await client.get( `/search/page?q=${searchTerm}` );
+			const { body, headers } = await client.get( `/search/page?q=${searchTerm}` );
 			assert.lengthOf( body.pages, 1 );
 			const returnPage = body.pages[ 0 ];
 			assert.nestedProperty( returnPage, 'title' );
@@ -35,6 +35,13 @@ describe( 'Search', () => {
 			assert.nestedProperty( returnPage, 'key' );
 			assert.nestedProperty( returnPage, 'excerpt' );
 			assert.include( returnPage.excerpt, `<span class='searchmatch'>${searchTerm}</span>` );
+
+			// full-text search should not have cache-control
+			if ( headers[ 'cache-control' ] ) {
+				assert.notMatch( headers[ 'cache-control' ], /\bpublic\b/ );
+				assert.notMatch( headers[ 'cache-control' ], /\bmax-age=0*[1-9]\b/ );
+				assert.notMatch( headers[ 'cache-control' ], /\bs-maxage=0*[1-9]\b/ );
+			}
 		} );
 		it( 'should return array of pages when there is only title match', async () => {
 			const { body } = await client.get( `/search/page?q=${pageWithBothTerms}` );
@@ -90,13 +97,18 @@ describe( 'Search', () => {
 			assert.lengthOf( body.pages, 0 );
 		} );
 		it( 'should return array of pages when there is a title match', async () => {
-			const { body } = await client.get( `/search/title?q=${pageWithBothTerms}` );
+			const { body, headers } = await client.get( `/search/title?q=${pageWithBothTerms}` );
 			assert.lengthOf( body.pages, 1 );
 			const returnPage = body.pages[ 0 ];
 			assert.nestedProperty( returnPage, 'title' );
 			assert.nestedProperty( returnPage, 'id' );
 			assert.nestedProperty( returnPage, 'key' );
 			assert.nestedProperty( returnPage, 'excerpt' );
+
+			// completion search should encourage caching
+			assert.nestedProperty( headers, 'cache-control' );
+			assert.match( headers[ 'cache-control' ], /\bpublic\b/ );
+			assert.match( headers[ 'cache-control' ], /\bmax-age=[1-9]\d*/ );
 		} );
 		it( 'should return two pages when both pages match', async () => {
 			const { body } = await client.get( `/search/title?q=${sharedTitleTerm}` );

tests/phpunit/unit/includes/Rest/Handler/SearchHandlerTest.php

@@ -51,6 +51,7 @@ class SearchHandlerTest extends \MediaWikiUnitTestCase {
 			'SearchType' => 'test',
 			'SearchTypeAlternatives' => [],
 			'NamespacesToBeSearchedDefault' => [ NS_MAIN => true ],
+			'SearchSuggestCacheExpiry' => 1200,
 		] );
 
 		/** @var Language|MockObject $language */
@@ -87,6 +88,7 @@ class SearchHandlerTest extends \MediaWikiUnitTestCase {
 			->willReturn( $this->searchEngine );
 
 		return new SearchHandler(
+			$config,
 			$permissionManager,
 			$searchEngineFactory,
 			$searchEngineConfig
@@ -189,7 +191,14 @@ class SearchHandlerTest extends \MediaWikiUnitTestCase {
 
 		$handler = $this->newHandler( $query, $titleResults, $textResults, $completionResults );
 		$config = [ 'mode' => SearchHandler::COMPLETION_MODE ];
-		$data = $this->executeHandlerAndGetBodyData( $handler, $request, $config );
+		$response = $this->executeHandler( $handler, $request, $config );
+
+		$this->assertSame( 200, $response->getStatusCode() );
+		$this->assertSame( 'application/json', $response->getHeaderLine( 'Content-Type' ) );
+		$this->assertSame( 'public, max-age=1200', $response->getHeaderLine( 'Cache-Control' ) );
+
+		$data = json_decode( $response->getBody(), true );
+		$this->assertIsArray( $data, 'Body must be a JSON array' );
 
 		$this->assertArrayHasKey( 'pages', $data );
 		$this->assertCount( 2, $data['pages'] );