Limit Max Window Size (#2051)

* limit_max_window_size

* Update search/includes/classes/class-search.php

Co-authored-by: Rinat K <[email protected]>

* adds more comments

* Add index.max_result_window to the monitored / healed index settings list

* Whitespace

Co-authored-by: Nick Daugherty <[email protected]>
Co-authored-by: Rinat K <[email protected]>

Author: 3 people

search/includes/classes/class-health.php

@@ -26,11 +26,13 @@ class Health {
 		'time',
 	);
 	const INDEX_SETTINGS_HEALTH_MONITORED_KEYS = array(
+		'index.max_result_window',
 		'index.number_of_replicas',
 		'index.number_of_shards',
 		'index.routing.allocation.include.dc',
 	);
 	const INDEX_SETTINGS_HEALTH_AUTO_HEAL_KEYS = array(
+		'index.max_result_window',
 		'index.number_of_replicas',
 		'index.routing.allocation.include.dc',
 	);

search/includes/classes/class-search.php

@@ -11,6 +11,7 @@ class Search {
 	public const QUERY_INTEGRATION_FORCE_ENABLE_KEY = 'vip-search-enabled';
 	public const SEARCH_ALERT_SLACK_CHAT = '#vip-go-es-alerts';
 	public const SEARCH_ALERT_LEVEL = 2; // Level 2 = 'alert'
+	public const MAX_RESULT_WINDOW = 9000;
 	/**
 	 * Empty for now. Will flesh out once migration path discussions are underway and/or the same meta are added to the filter across many
 	 * sites.
@@ -484,6 +485,14 @@ protected function setup_hooks() {
 
 		add_filter( 'vip_search_post_meta_allow_list', array( $this, 'filter__vip_search_post_meta_allow_list_defaults' ) );
 
+		// Limit the max result window on index settings
+		add_filter( 'ep_max_result_window', [ $this, 'limit_max_result_window' ], PHP_INT_MAX );
+		add_filter( 'ep_term_max_result_window', [ $this, 'limit_max_result_window' ], PHP_INT_MAX );
+		add_filter( 'ep_user_max_result_window', [ $this, 'limit_max_result_window' ], PHP_INT_MAX );
+
+		// Limit the max result window on query arguments
+		add_filter( 'ep_max_results_window', [ $this, 'limit_max_result_window' ], PHP_INT_MAX );
+
 		add_action( 'after_setup_theme', array( $this, 'apply_settings' ), PHP_INT_MAX ); // Try to apply Search settings after other actions in this hook.
 	}
 
@@ -1637,7 +1646,7 @@ public function filter__ep_prepare_meta_allowed_protected_keys( $keys, $post ) {
 
 	/**
 	 * Hooks into the ep_$indexable_mapping hooks to add things like allocation rules
-	 * 
+	 *
 	 * Note that this hook receives the mapping and settings together
 	 */
 	public function filter__ep_indexable_mapping( $mapping ) {
@@ -1802,4 +1811,8 @@ public function is_protected_content_enabled() {
 
 		return $protected_content_feature->is_active();
 	}
+
+	public function limit_max_result_window( $current_value ) {
+		return min( $current_value, self::MAX_RESULT_WINDOW );
+	}
 }

tests/search/includes/classes/test-class-health.php

@@ -672,11 +672,13 @@ public function get_index_settings_diff_for_indexable_data() {
 				array(
 					'index.number_of_shards' => 1,
 					'index.number_of_replicas' => 2,
+					'index.max_result_window' => 9000,
 				),
 				// Desired index settings from ElasticPress
 				array(
 					'index.number_of_shards' => 1,
 					'index.number_of_replicas' => 2,
+					'index.max_result_window' => 9000,
 				),
 				// Options
 				array(),
@@ -729,12 +731,14 @@ public function get_index_settings_diff_for_indexable_data() {
 					'index.number_of_shards' => 1,
 					'index.number_of_replicas' => 2,
 					'foo' => 'bar',
+					'index.max_result_window' => '1000000',
 				),
 				// Desired index settings from ElasticPress
 				array(
 					'index.number_of_shards' => 1,
 					'index.number_of_replicas' => 1,
 					'foo' => 'baz',
+					'index.max_result_window' => 9000,
 				),
 				// Options
 				array(),
@@ -744,6 +748,10 @@ public function get_index_settings_diff_for_indexable_data() {
 						'expected' => 1,
 						'actual' => 2,
 					),
+					'index.max_result_window' => array(
+						'expected' => 9000,
+						'actual' => 1000000,
+					),
 				),
 			),
 			// Diff expected, mismatched settings with specific index version
@@ -815,6 +823,7 @@ public function heal_index_settings_for_indexable_data() {
 				array(
 					'index.number_of_shards' => 1,
 					'index.number_of_replicas' => 2,
+					'index.max_result_window' => 9000,
 				),
 				// Options
 				array(),
@@ -825,6 +834,7 @@ public function heal_index_settings_for_indexable_data() {
 				array(
 					'index.number_of_shards' => 1,
 					'index.number_of_replicas' => 1,
+					'index.max_result_window' => 9000,
 					'foo' => 'baz',
 				),
 				// Options
@@ -836,6 +846,7 @@ public function heal_index_settings_for_indexable_data() {
 				array(
 					'index.number_of_shards' => 1,
 					'index.number_of_replicas' => 1,
+					'index.max_result_window' => 9000,
 					'foo' => 'baz',
 				),
 				// Options
@@ -1003,12 +1014,14 @@ public function get_index_settings_diff_data() {
 				array(
 					'number_of_shards' => 1,
 					'number_of_replicas' => 2,
+					'max_result_window' => '1000000',
 					'foo' => 'bar',
 				),
 				// Desired index settings from ElasticPress
 				array(
 					'number_of_shards' => 1,
 					'number_of_replicas' => 1,
+					'max_result_window' => 9000,
 					'foo' => 'baz',
 				),
 				// Expected diff
@@ -1017,6 +1030,10 @@ public function get_index_settings_diff_data() {
 						'expected' => 1,
 						'actual' => 2,
 					),
+					'max_result_window' => array(
+						'expected' => 9000,
+						'actual' => '1000000',
+					),
 					'foo' => array(
 						'expected' => 'baz',
 						'actual' => 'bar',

tests/search/test-class-search.php

@@ -2986,6 +2986,29 @@ public function test__maybe_enable_ep_query_logging_debug_bar_and_qm_enabled() {
 		$this->assertTrue( defined( 'WP_EP_DEBUG' ) );
 		$this->assertTrue( WP_EP_DEBUG );
 	}
+	public function limit_max_result_window_data() {
+		return [
+			[
+				'input' => 500,
+				'expected' => 500,
+			],
+			[
+				'input' => 10000,
+				'expected' => 9000,
+			],
+		];
+	}
+
+	/**
+	 * @dataProvider limit_max_result_window_data
+	 */
+	public function test__limit_max_result_window( $input, $expected ) {
+		$es = new \Automattic\VIP\Search\Search();
+
+		$result = $es->limit_max_result_window( $input );
+
+		$this->assertEquals( $expected, $result );
+	}
 
 	/**
 	 * Helper function for accessing protected methods.