Merge pull request #2040 from Automattic/add/validate_content_cron

Add validate content cron
Automattic · Mar 24, 2021 · 7f8b387 · 7f8b387
2 parents 1520b3d + da58b9f
commit 7f8b387
Show file tree

Hide file tree

Showing 5 changed files with 108 additions and 31 deletions.
diff --git a/lib/feature/class-feature.php b/lib/feature/class-feature.php
@@ -20,6 +20,7 @@ class Feature {
 		// https://github.com/Automattic/vip-go-mu-plugins/tree/master/vip-jetpack/connection-pilot
 		'jetpack-cxn-pilot' => 0.25,
 		'search_indexable_settings_auto_heal' => 0,
+		'search_content_validation_and_auto_heal_cron_job' => 0.1,
 	);
 
 	public static $site_id = false;

diff --git a/search/includes/classes/class-health.php b/search/includes/classes/class-health.php
@@ -255,9 +255,48 @@ public static function validate_index_posts_count( $options = array() ) {
 	/**
 	 * Validate DB and ES index post content
 	 *
+	 * ## OPTIONS
+	 *
+	 * [inspect]
+	 * : Optional gives more verbose output for index inconsistencies
+	 *
+	 * [start_post_id=<int>]
+	 * : Optional starting post id (defaults to 1)
+	 *
+	 * [last_post_id=<int>]
+	 * : Optional last post id to check
+	 *
+	 * [batch_size=<int>]
+	 * : Optional batch size
+	 *
+	 * [max_diff_size=<int>]
+	 * : Optional max count of diff before exiting
+	 *
+	 * [do_not_heal]
+	 * : Optional Don't try to correct inconsistencies
+	 *
+	 * [silent]
+	 * : Optional silences all non-error output except for the final results
+	 *
+	 * [force_parallel_execution]
+	 * : Optional Force execution even if the process is already ongoing
+	 *
+	 *
+	 * @param array $options list of options
+	 *
+	 *
 	 * @return array Array containing counts and ids of posts with inconsistent content
 	 */
-	public function validate_index_posts_content( $start_post_id, $last_post_id, $batch_size, $max_diff_size, $silent, $inspect, $do_not_heal, $force_parallel_execution = false ) {
+	public function validate_index_posts_content( $options ) {
+		$start_post_id = $options['start_post_id'] ?? 1;
+		$last_post_id = $options['last_post_id'] ?? null;
+		$batch_size = $options['batch_size'] ?? null;
+		$max_diff_size = $options['max_diff_size'] ?? null;
+		$silent = isset( $options['silent'] );
+		$inspect = isset( $options['inspect'] );
+		$do_not_heal = isset( $options['do_not_heal'] );
+		$force_parallel_execution = isset( $options['force_parallel_execution'] );
+
 		$process_parallel_execution_lock = ! $force_parallel_execution;
 		// We only work with process if we can guarantee no parallel execution and user did't pick specific start_post_id to avoid unexpected overwriting of that.
 		$track_process = ( ! $start_post_id || 1 === $start_post_id ) && ! $force_parallel_execution;

diff --git a/search/includes/classes/class-healthjob.php b/search/includes/classes/class-healthjob.php
@@ -13,6 +13,11 @@ class HealthJob {
 	 */
 	const CRON_EVENT_NAME = 'vip_search_healthcheck';
 
+	/**
+	 * The name of the scheduled cron event to run the validate contnets check
+	 */
+	const CRON_EVENT_VALIDATE_CONTENT_NAME = 'vip_search_health_validate_content';
+
 	/**
 	 * Custom cron interval name
 	 */
@@ -60,6 +65,7 @@ public function __construct( \Automattic\VIP\Search\Search $search ) {
 	public function init() {
 		// We always add this action so that the job can unregister itself if it no longer should be running
 		add_action( self::CRON_EVENT_NAME, [ $this, 'check_health' ] );
+		add_action( self::CRON_EVENT_VALIDATE_CONTENT_NAME, [ $this, 'validate_contents' ] );
 
 		if ( ! $this->is_enabled() ) {
 			return;
@@ -80,6 +86,9 @@ public function schedule_job() {
 		if ( ! wp_next_scheduled( self::CRON_EVENT_NAME ) ) {
 			wp_schedule_event( time(), self::CRON_INTERVAL_NAME, self::CRON_EVENT_NAME );
 		}
+		if ( ! wp_next_scheduled( self::CRON_EVENT_VALIDATE_CONTENT_NAME ) ) {
+			wp_schedule_event( time(), 'weekly', self::CRON_EVENT_VALIDATE_CONTENT_NAME );
+		}
 	}
 
 	/**
@@ -91,6 +100,9 @@ public function disable_job() {
 		if ( wp_next_scheduled( self::CRON_EVENT_NAME ) ) {
 			wp_clear_scheduled_hook( self::CRON_EVENT_NAME );
 		}
+		if ( wp_next_scheduled( self::CRON_EVENT_VALIDATE_CONTENT_NAME ) ) {
+			wp_clear_scheduled_hook( self::CRON_EVENT_VALIDATE_CONTENT_NAME );
+		}
 	}
 
 	/**
@@ -115,6 +127,38 @@ public function filter_cron_schedules( $schedule ) {
 		return $schedule;
 	}
 
+	/**
+	 * Check index health
+	 */
+	public function validate_contents() {
+		// Check if job has been disabled
+		if ( ! $this->is_enabled() ) {
+			$this->disable_job();
+
+			return;
+		}
+
+		if ( ! \Automattic\VIP\Feature::is_enabled( 'search_content_validation_and_auto_heal_cron_job' ) ) {
+			return;
+		}
+
+		// Don't run the checks if the index is not built.
+		if ( \ElasticPress\Utils\is_indexing() || ! \ElasticPress\Utils\get_last_sync() ) {
+			return;
+		}
+
+		$results = $this->health->validate_index_posts_content( [ 'silent' => true ] );
+
+		if ( is_wp_error( $results ) ) {
+			$message = 'Cron validate-contentes error: ' . $results->get_error_message();
+			$this->send_alert( '#vip-go-es-alerts', $message, 2 );
+		} else if ( ! empty( $results ) ) {
+			$message = 'Cron validate-contentes: Autohealing executed: ' . print_r( $results, true );
+			$this->send_alert( '#vip-go-es-alerts', $message, 2 );
+		}
+
+	}
+
 	/**
 	 * Check index health
 	 */

diff --git a/search/includes/classes/commands/class-healthcommand.php b/search/includes/classes/commands/class-healthcommand.php
@@ -256,13 +256,13 @@ private function render_results( array $results ) {
 	 * default: csv
 	 * ---
 	 *
-	 * [--do-not-heal]
+	 * [--do_not_heal]
 	 * : Optional Don't try to correct inconsistencies
 	 *
 	 * [--silent]
 	 * : Optional silences all non-error output except for the final results
 	 *
-	 * [--force-parallel-execution]
+	 * [--force_parallel_execution]
 	 * : Optional Force execution even if the process is already ongoing
 	 *
 	 * ## EXAMPLES
@@ -273,16 +273,7 @@ private function render_results( array $results ) {
 	public function validate_contents( $args, $assoc_args ) {
 		$health = new \Automattic\VIP\Search\Health( \Automattic\VIP\Search\Search::instance() );
 
-		$results = $health->validate_index_posts_content(
-			$assoc_args['start_post_id'] ?? 1,
-			$assoc_args['last_post_id'] ?? null,
-			$assoc_args['batch_size'] ?? null,
-			$assoc_args['max_diff_size'] ?? null,
-			isset( $assoc_args['silent'] ),
-			isset( $assoc_args['inspect'] ),
-			isset( $assoc_args['do-not-heal'] ),
-			isset( $assoc_args['force-parallel-execution'] )
-		);
+		$results = $health->validate_index_posts_content( $assoc_args );
 
 		if ( is_wp_error( $results ) ) {
 			$diff = $results->get_error_data( 'diff' );

diff --git a/tests/search/includes/classes/test-class-health.php b/tests/search/includes/classes/test-class-health.php
@@ -481,9 +481,11 @@ public function test_validate_index_posts_content__should_set_and_clear_lock() {
 	}
 
 	public function test_validate_index_posts_content__should_set_and_clear_last_processed() {
-		$first_post_id = 1;
-		$last_post_id = 100;
-		$batch_size = 50;
+		$options = [
+			'start_post_id' => 1,
+			'last_post_id' => 100,
+			'batch_size' => 50,
+		];
 		$patrtially_mocked_health = $this->getMockBuilder( \Automattic\VIP\Search\Health::class )
 			->setMethods( [ 'update_validate_content_process', 'remove_validate_content_process', 'validate_index_posts_content_batch' ] )
 			->disableOriginalConstructor()
@@ -503,12 +505,12 @@ public function test_validate_index_posts_content__should_set_and_clear_last_pro
 
 		$patrtially_mocked_health->expects( $this->exactly( 2 ) )
 			->method( 'update_validate_content_process' )
-			->withConsecutive( [ $first_post_id ], [ $first_post_id + $batch_size ] );
+			->withConsecutive( [ $options['start_post_id'] ], [ $options['start_post_id'] + $options['batch_size'] ] );
 
 		$patrtially_mocked_health->expects( $this->once() )->method( 'remove_validate_content_process' );
 
 
-		$patrtially_mocked_health->validate_index_posts_content( $first_post_id, $last_post_id, $batch_size, null, false, false, false );
+		$patrtially_mocked_health->validate_index_posts_content( $options );
 	}
 
 	public function test_validate_index_posts_content__should_not_interact_with_process_if_paralel_run() {
@@ -534,8 +536,7 @@ public function test_validate_index_posts_content__should_not_interact_with_proc
 		$patrtially_mocked_health->expects( $this->never() )->method( 'remove_validate_content_process' );
 
 
-		$allow_running_in_parallel = true;
-		$patrtially_mocked_health->validate_index_posts_content( 1, null, null, null, false, false, false, $allow_running_in_parallel );
+		$patrtially_mocked_health->validate_index_posts_content( [ 'force_parallel_execution' => true ] );
 	}
 
 	public function test_validate_index_posts_content__should_not_interact_with_process_if_non_default_start_id_is_sent_in() {
@@ -561,13 +562,12 @@ public function test_validate_index_posts_content__should_not_interact_with_proc
 		$patrtially_mocked_health->expects( $this->never() )->method( 'remove_validate_content_process' );
 
 
-		$start_post_id = 25;
-		$patrtially_mocked_health->validate_index_posts_content( $start_post_id, null, null, null, false, false, false );
+		$patrtially_mocked_health->validate_index_posts_content( [ 'start_post_id' => 25 ] );
 	}
 
 	public function test_validate_index_posts_content__pick_up_after_interuption() {
 		$interrupted_post_id = 5;
-		$first_post_id = 1;
+		$start_post_id = 1;
 		$patrtially_mocked_health = $this->getMockBuilder( \Automattic\VIP\Search\Health::class )
 			->setMethods( [ 'update_validate_content_process', 'remove_validate_content_process', 'get_validate_content_abandoned_process', 'validate_index_posts_content_batch' ] )
 			->disableOriginalConstructor()
@@ -591,12 +591,12 @@ public function test_validate_index_posts_content__pick_up_after_interuption() {
 			->with( $this->anything(), $interrupted_post_id, $this->anything(), $this->anything() );
 
 
-		$patrtially_mocked_health->validate_index_posts_content( $first_post_id, null, null, null, false, false, false );
+		$patrtially_mocked_health->validate_index_posts_content( $start_post_id, null, null, null, false, false, false );
 	}
 
 	public function test_validate_index_posts_content__do_not_pick_up_after_interuption_when_running_in_parallel() {
 		$interrupted_post_id = 5;
-		$first_post_id = 1;
+		$start_post_id = 1;
 		$patrtially_mocked_health = $this->getMockBuilder( \Automattic\VIP\Search\Health::class )
 			->setMethods( [ 'update_validate_content_process', 'remove_validate_content_process', 'get_validate_content_abandoned_process', 'validate_index_posts_content_batch' ] )
 			->disableOriginalConstructor()
@@ -617,15 +617,17 @@ public function test_validate_index_posts_content__do_not_pick_up_after_interupt
 
 		$patrtially_mocked_health->expects( $this->once() )
 			->method( 'validate_index_posts_content_batch' )
-			->with( $this->anything(), $first_post_id, $this->anything(), $this->anything() );
+			->with( $this->anything(), $start_post_id, $this->anything(), $this->anything() );
 
-		$allow_running_in_parallel = true;
-		$patrtially_mocked_health->validate_index_posts_content( $first_post_id, null, null, null, false, false, false, $allow_running_in_parallel );
+		$patrtially_mocked_health->validate_index_posts_content( [
+			'start_post_id' => $start_post_id,
+			'force_parallel_execution' => true,
+		] );
 	}
 
 	public function test_validate_index_posts_content__do_not_pick_up_after_interuption_when_non_default_start_post_id() {
 		$interrupted_post_id = 5;
-		$first_post_id = 2;
+		$start_post_id = 2;
 		$patrtially_mocked_health = $this->getMockBuilder( \Automattic\VIP\Search\Health::class )
 			->setMethods( [ 'update_validate_content_process', 'remove_validate_content_process', 'get_validate_content_abandoned_process', 'validate_index_posts_content_batch' ] )
 			->disableOriginalConstructor()
@@ -646,9 +648,9 @@ public function test_validate_index_posts_content__do_not_pick_up_after_interupt
 
 		$patrtially_mocked_health->expects( $this->once() )
 			->method( 'validate_index_posts_content_batch' )
-			->with( $this->anything(), $first_post_id, $this->anything(), $this->anything() );
+			->with( $this->anything(), $start_post_id, $this->anything(), $this->anything() );
 
-		$patrtially_mocked_health->validate_index_posts_content( $first_post_id, null, null, null, false, false, false );
+		$patrtially_mocked_health->validate_index_posts_content( [ 'start_post_id' => $start_post_id ] );
 	}
 
 	public function get_index_settings_diff_for_indexable_data() {