Skip to content

Commit

Permalink
Do not cache private wiki completion results
Browse files Browse the repository at this point in the history
Previously, when a user with correct permissions uses completion search on a
private wiki, the results are returned and cached. Since we are on a private wiki,
we don't want to cache results since the content is not accessible to all users.

Now, content that is not accessible to all users will not be cached.

This patch achieves this by setting the appropriate Cache-Control response headers
for the MW REST Search endpoint.

Bug: T292763
Change-Id: I693b4088df9c0520d5238c286312ec52ab273604
  • Loading branch information
Nikki Nikkhoui committed Oct 12, 2021
1 parent 9766170 commit 0b3a4c0
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 7 deletions.
16 changes: 13 additions & 3 deletions includes/Rest/Handler/SearchHandler.php
Expand Up @@ -6,6 +6,7 @@
use InvalidArgumentException;
use ISearchResultSet;
use MediaWiki\Page\ProperPageIdentity;
use MediaWiki\Permissions\PermissionManager;
use MediaWiki\Rest\Handler;
use MediaWiki\Rest\LocalizedHttpException;
use MediaWiki\Rest\Response;
Expand All @@ -32,6 +33,9 @@ class SearchHandler extends Handler {
/** @var SearchEngineConfig */
private $searchEngineConfig;

/** @var PermissionManager */
private $permissionManager;

/**
* Search page body and titles.
*/
Expand Down Expand Up @@ -73,14 +77,17 @@ class SearchHandler extends Handler {
* @param Config $config
* @param SearchEngineFactory $searchEngineFactory
* @param SearchEngineConfig $searchEngineConfig
* @param PermissionManager $permissionManager
*/
public function __construct(
Config $config,
SearchEngineFactory $searchEngineFactory,
SearchEngineConfig $searchEngineConfig
SearchEngineConfig $searchEngineConfig,
PermissionManager $permissionManager
) {
$this->searchEngineFactory = $searchEngineFactory;
$this->searchEngineConfig = $searchEngineConfig;
$this->permissionManager = $permissionManager;

// @todo Avoid injecting the entire config, see T246377
$this->completionCacheExpiry = $config->get( 'SearchSuggestCacheExpiry' );
Expand Down Expand Up @@ -327,14 +334,17 @@ public function execute() {
$this->buildDescriptionsFromPageIdentities( $pageIdentities ),
$this->buildThumbnailsFromPageIdentities( $pageIdentities )
);

$response = $this->getResponseFactory()->createJson( [ 'pages' => $result ] );

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 );
if ( $this->permissionManager->isEveryoneAllowed( 'read' ) ) {
$response->setHeader( 'Cache-Control', 'public, max-age=' . $this->completionCacheExpiry );
} else {
$response->setHeader( 'Cache-Control', 'no-store, max-age=0' );
}
}

return $response;
Expand Down
6 changes: 4 additions & 2 deletions includes/Rest/coreRoutes.json
Expand Up @@ -49,7 +49,8 @@
"services": [
"MainConfig",
"SearchEngineFactory",
"SearchEngineConfig"
"SearchEngineConfig",
"PermissionManager"
],
"mode": "fulltext"
},
Expand All @@ -59,7 +60,8 @@
"services": [
"MainConfig",
"SearchEngineFactory",
"SearchEngineConfig"
"SearchEngineConfig",
"PermissionManager"
],
"mode": "completion"
},
Expand Down
36 changes: 34 additions & 2 deletions tests/phpunit/unit/includes/Rest/Handler/SearchHandlerTest.php
Expand Up @@ -6,6 +6,7 @@
use InvalidArgumentException;
use Language;
use MediaWiki\Page\PageIdentity;
use MediaWiki\Permissions\PermissionManager;
use MediaWiki\Rest\Handler\SearchHandler;
use MediaWiki\Rest\LocalizedHttpException;
use MediaWiki\Rest\RequestData;
Expand Down Expand Up @@ -40,14 +41,16 @@ class SearchHandlerTest extends \MediaWikiUnitTestCase {
* @param SearchResultSet|Status $titleResult
* @param SearchResultSet|Status $textResult
* @param SearchSuggestionSet|null $completionResult
* @param PermissionManager|null $permissionManager
*
* @return SearchHandler
*/
private function newHandler(
$query,
$titleResult,
$textResult,
$completionResult = null
$completionResult = null,
$permissionManager = null
) {
$config = new HashConfig( [
'SearchType' => 'test',
Expand All @@ -61,6 +64,13 @@ private function newHandler(
$hookContainer = $this->createHookContainer();
$searchEngineConfig = new \SearchEngineConfig( $config, $language, $hookContainer, [] );

if ( !$permissionManager ) {
$permissionManager = $this->createMock( PermissionManager::class );
$permissionManager->method( 'isEveryoneAllowed' )
->with( 'read' )
->willReturn( true );
}

/** @var SearchEngine|MockObject $searchEngine */
$this->searchEngine = $this->createMock( SearchEngine::class );
$this->searchEngine->method( 'searchTitle' )
Expand All @@ -84,7 +94,8 @@ private function newHandler(
return new SearchHandler(
$config,
$searchEngineFactory,
$searchEngineConfig
$searchEngineConfig,
$permissionManager
);
}

Expand Down Expand Up @@ -204,6 +215,27 @@ public function testExecuteCompletionSearch() {
$this->assertSame( 'Frobnitz', $data['pages'][1]['excerpt'] );
}

public function testCompletionSearchNotCachedForPublicPages() {
$titleResults = new MockSearchResultSet( [] );
$textResults = new MockSearchResultSet( [] );
$completionResults = new SearchSuggestionSet( [] );

$query = 'foo';
$request = new RequestData( [ 'queryParams' => [ 'q' => $query ] ] );

$permissionManager = $this->createMock( PermissionManager::class );
$permissionManager->method( 'isEveryoneAllowed' )
->with( 'read' )
->willReturn( false );

$handler = $this->newHandler( $query, $titleResults, $textResults, $completionResults, $permissionManager );
$config = [ 'mode' => SearchHandler::COMPLETION_MODE ];

$response = $this->executeHandler( $handler, $request, $config );
$this->assertSame( 'no-store, max-age=0', $response->getHeaderLine( 'Cache-Control' ) );
$this->assertSame( 200, $response->getStatusCode() );
}

public function testExecute_limit() {
$titleResults = new MockSearchResultSet( [
$this->makeMockSearchResult( 'Foo' ),
Expand Down

0 comments on commit 0b3a4c0

Please sign in to comment.