From e95b988baf281576bc07272a62c126e6e2db14b3 Mon Sep 17 00:00:00 2001 From: Rebecca Hum Date: Mon, 25 Nov 2024 11:42:18 -0500 Subject: [PATCH] =?UTF-8?q?Revert=20"Search:=20Allow=20sync=20manager=20qu?= =?UTF-8?q?eue=20to=20account=20for=20multisite=20context=20(#5=E2=80=A6"?= =?UTF-8?q?=20(#6005)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 23b2fac0174c718a716e10dfde7d3bfd92706edd. --- search/elasticpress | 2 +- search/includes/classes/class-queue.php | 10 +- search/includes/classes/class-versioning.php | 4 +- .../includes/classes/test-class-queue.php | 91 +++++++++---------- .../classes/test-class-versioning.php | 13 +-- 5 files changed, 54 insertions(+), 66 deletions(-) diff --git a/search/elasticpress b/search/elasticpress index ecfb7bddc8..1a6b4cf442 160000 --- a/search/elasticpress +++ b/search/elasticpress @@ -1 +1 @@ -Subproject commit ecfb7bddc877d2326659cf8d03dcf3ddfb7faa55 +Subproject commit 1a6b4cf4428feab097c49d3dfcfbf6dcff4d400b diff --git a/search/includes/classes/class-queue.php b/search/includes/classes/class-queue.php index 75a3b2e6f8..b8527e529f 100644 --- a/search/includes/classes/class-queue.php +++ b/search/includes/classes/class-queue.php @@ -1028,11 +1028,11 @@ public function intercept_ep_sync_manager_indexing( $bail, $sync_manager, $index return $bail; } - if ( empty( $sync_manager->get_sync_queue() ) ) { + if ( empty( $sync_manager->sync_queue ) ) { return $bail; } - $this->queue_objects( array_keys( $sync_manager->get_sync_queue() ), $indexable_slug ); + $this->queue_objects( array_keys( $sync_manager->sync_queue ), $indexable_slug ); // If indexing operations are NOT currently ratelimited, queue up a cron event to process these immediately. if ( ! static::is_indexing_ratelimited() ) { @@ -1040,7 +1040,7 @@ public function intercept_ep_sync_manager_indexing( $bail, $sync_manager, $index } // Empty out the queue now that we've queued those items up - $sync_manager->reset_sync_queue(); + $sync_manager->sync_queue = []; return true; } @@ -1081,12 +1081,12 @@ public function ratelimit_indexing( $bail, $sync_manager, $indexable_slug ) { return $bail; } - if ( empty( $sync_manager->get_sync_queue() ) ) { + if ( empty( $sync_manager->sync_queue ) ) { return $bail; } // Increment first to prevent overrunning ratelimiting - $increment = count( $sync_manager->get_sync_queue() ); + $increment = count( $sync_manager->sync_queue ); $index_count_in_period = static::index_count_incr( $increment ); // If indexing operation ratelimiting is hit, queue index operations diff --git a/search/includes/classes/class-versioning.php b/search/includes/classes/class-versioning.php index d56e5ffbd4..5362b0b40d 100644 --- a/search/includes/classes/class-versioning.php +++ b/search/includes/classes/class-versioning.php @@ -845,7 +845,7 @@ public function filter__pre_ep_index_sync_queue( $bail, $sync_manager, $indexabl $inactive_versions = $this->get_inactive_versions( $indexable ); // If there are no inactive versions or nothing in the queue, we can just skip - if ( empty( $inactive_versions ) || empty( $sync_manager->get_sync_queue() ) ) { + if ( empty( $inactive_versions ) || empty( $sync_manager->sync_queue ) ) { return $bail; } @@ -856,7 +856,7 @@ public function filter__pre_ep_index_sync_queue( $bail, $sync_manager, $indexabl 'index_version' => $version['number'], ); - foreach ( $sync_manager->get_sync_queue() as $object_id => $value ) { + foreach ( $sync_manager->sync_queue as $object_id => $value ) { /** * This filter is documented in Versioning::replicate_queued_objects_to_other_versions */ diff --git a/tests/search/includes/classes/test-class-queue.php b/tests/search/includes/classes/test-class-queue.php index 98d6f59433..deae3fc9cf 100644 --- a/tests/search/includes/classes/test-class-queue.php +++ b/tests/search/includes/classes/test-class-queue.php @@ -21,9 +21,6 @@ class Queue_Test extends WP_UnitTestCase { /** @var Queue */ private $queue; - /** @var SyncManager */ - private $sync_manager; - public function setUp(): void { parent::setUp(); @@ -46,9 +43,6 @@ public function setUp(): void { $this->queue->empty_queue(); add_filter( 'ep_do_intercept_request', [ $this, 'filter_index_exists_request_ok' ], PHP_INT_MAX, 5 ); - - $indexable = Indexables::factory()->get( 'post' ); - $this->sync_manager = $indexable->sync_manager; } public function tearDown(): void { @@ -308,10 +302,10 @@ public function test_checkout_jobs() { } public function test_offload_indexing_to_queue() { - $this->add_posts_to_queue( range( 1, 3 ) ); + $mock_sync_manager = (object) array( 'sync_queue' => [ 1, 2, 3 ] ); // Make sure we're not already bailing on EP indexing, otherwise the test isn't doing anything - $current_bail = apply_filters( 'pre_ep_index_sync_queue', false, $this->sync_manager, 'post' ); + $current_bail = apply_filters( 'pre_ep_index_sync_queue', false, $mock_sync_manager, 'post' ); $this->assertFalse( $current_bail ); @@ -319,7 +313,7 @@ public function test_offload_indexing_to_queue() { $this->queue->offload_indexing_to_queue(); // Now the filter should return true to bail early from EP indexing - $current_bail = apply_filters( 'pre_ep_index_sync_queue', false, $this->sync_manager, 'post' ); + $current_bail = apply_filters( 'pre_ep_index_sync_queue', false, $mock_sync_manager, 'post' ); $this->assertTrue( $current_bail ); } @@ -479,12 +473,16 @@ public function test_process_jobs() { } public function test_intercept_ep_sync_manager_indexing() { - $this->add_posts_to_queue( [ 1, 2, 1000 ] ); + $post_ids = array( 1, 2, 1000 ); + + $mock_sync_manager = (object) array( + 'sync_queue' => array_fill_keys( $post_ids, true ), // EP stores in id => true format + ); - $this->queue->intercept_ep_sync_manager_indexing( false, $this->sync_manager, 'post' ); + $this->queue->intercept_ep_sync_manager_indexing( false, $mock_sync_manager, 'post' ); // And the SyncManager's queue should have been emptied - $this->assertEmpty( $this->sync_manager->get_sync_queue() ); + $this->assertEmpty( $mock_sync_manager->sync_queue ); } public function test_get_jobs_by_range() { @@ -718,8 +716,11 @@ public function test__ratelimit_indexing_should_pass_bail_if_not_post() { * Ensure that the value passed into the filter is returned if the sync queue is empty */ public function test__ratelimit_indexing_should_pass_bail_if_sync_queue_empty() { - $this->assertTrue( $this->queue->ratelimit_indexing( true, $this->sync_manager, 'post' ), 'should return true since true was passed in' ); - $this->assertFalse( $this->queue->ratelimit_indexing( false, $this->sync_manager, 'post' ), 'should return false since false was passed in' ); + $sync_manager = new stdClass(); + $sync_manager->sync_queue = array(); + + $this->assertTrue( $this->queue->ratelimit_indexing( true, $sync_manager, 'post' ), 'should return true since true was passed in' ); + $this->assertFalse( $this->queue->ratelimit_indexing( false, $sync_manager, 'post' ), 'should return false since false was passed in' ); } /** @@ -733,8 +734,11 @@ public function test_ratelimit_indexing_cache_count_should_not_exist_onload() { * Ensure that the count in the cache doesn't exist if the ratelimit_indexing returns early */ public function test_ratelimit_indexing_cache_count_should_not_exists_if_early_return() { + $sync_manager = new stdClass(); + $sync_manager->sync_queue = array(); + $this->queue->ratelimit_indexing( true, '', 'hippo' ); - $this->queue->ratelimit_indexing( true, $this->sync_manager, 'post' ); + $this->queue->ratelimit_indexing( true, $sync_manager, 'post' ); $this->assertFalse( wp_cache_get( $this->queue::INDEX_COUNT_CACHE_KEY, $this->queue::INDEX_COUNT_CACHE_GROUP ), 'indexing ops count shouldn\'t exist if function calls all returned early' ); } @@ -747,15 +751,16 @@ public function test_ratelimit_indexing_queue_should_be_empty_if_no_ratelimiting $table_name = $this->queue->schema->get_table_name(); - $this->add_posts_to_queue( range( 3, 9 ) ); + $sync_manager = new stdClass(); + $sync_manager->sync_queue = range( 3, 9 ); $this->queue::$max_indexing_op_count = PHP_INT_MAX; // Ensure ratelimiting is disabled - $this->queue->ratelimit_indexing( true, $this->sync_manager, 'post' ); + $this->queue->ratelimit_indexing( true, $sync_manager, 'post' ); $this->assertEquals( 7, wp_cache_get( $this->queue::INDEX_COUNT_CACHE_KEY, $this->queue::INDEX_COUNT_CACHE_GROUP ), 'indexing ops count should be 7' ); - foreach ( $this->sync_manager->get_sync_queue() as $object_id ) { + foreach ( $sync_manager->sync_queue as $object_id ) { $results = $wpdb->get_results( $wpdb->prepare( "SELECT * FROM `{$table_name}` WHERE `object_id` = %d AND `object_type` = 'post' AND `status` = 'queued'", @@ -766,16 +771,13 @@ public function test_ratelimit_indexing_queue_should_be_empty_if_no_ratelimiting $this->assertCount( 0, $results, "should be 0 occurrences of post id #$object_id in queue table" ); } - $this->sync_manager->reset_sync_queue(); - - $post_ids = range( 10, 20 ); - $this->add_posts_to_queue( $post_ids ); + $sync_manager->sync_queue = range( 10, 20 ); - $this->queue->ratelimit_indexing( true, $this->sync_manager, 'post' ); + $this->queue->ratelimit_indexing( true, $sync_manager, 'post' ); $this->assertEquals( 18, wp_cache_get( $this->queue::INDEX_COUNT_CACHE_KEY, $this->queue::INDEX_COUNT_CACHE_GROUP ), 'indexing ops count should be 18' ); - foreach ( $this->sync_manager->get_sync_queue() as $object_id ) { + foreach ( $sync_manager->sync_queue as $object_id ) { $results = $wpdb->get_results( $wpdb->prepare( "SELECT * FROM `{$table_name}` WHERE `object_id` = %d AND `object_type` = 'post' AND `status` = 'queued'", @@ -795,22 +797,21 @@ public function test_ratelimit_indexing_queue_should_be_populated_if_ratelimitin $table_name = $this->queue->schema->get_table_name(); - $this->add_posts_to_queue( [ 1 ] ); + $sync_manager = new stdClass(); + $sync_manager->sync_queue = [ 1 ]; $this->queue->offload_indexing_to_queue(); - $current_bail = apply_filters( 'pre_ep_index_sync_queue', false, $this->sync_manager, 'post' ); + $current_bail = apply_filters( 'pre_ep_index_sync_queue', false, $sync_manager, 'post' ); $this->assertTrue( $current_bail ); - $post_ids = range( 3, 9 ); - $this->add_posts_to_queue( $post_ids ); - + $sync_manager->sync_queue = range( 3, 9 ); $this->queue::$max_indexing_op_count = 0; // Ensure ratelimiting is enabled - $this->queue->ratelimit_indexing( true, $this->sync_manager, 'post' ); + $this->queue->ratelimit_indexing( true, $sync_manager, 'post' ); $this->assertEquals( 7, wp_cache_get( $this->queue::INDEX_COUNT_CACHE_KEY, $this->queue::INDEX_COUNT_CACHE_GROUP ), 'indexing ops count should be 7' ); - foreach ( $this->sync_manager->get_sync_queue() as $object_id ) { + foreach ( $sync_manager->sync_queue as $object_id ) { $results = $wpdb->get_results( $wpdb->prepare( "SELECT * FROM `{$table_name}` WHERE `object_id` = %d AND `object_type` = 'post' AND `status` = 'queued'", @@ -821,14 +822,13 @@ public function test_ratelimit_indexing_queue_should_be_populated_if_ratelimitin $this->assertCount( 1, $results, "should be 1 occurrence of post id #$object_id in queue table" ); } - $post_ids = range( 10, 20 ); - $this->add_posts_to_queue( $post_ids ); + $sync_manager->sync_queue = range( 10, 20 ); - $this->queue->ratelimit_indexing( true, $this->sync_manager, 'post' ); + $this->queue->ratelimit_indexing( true, $sync_manager, 'post' ); $this->assertEquals( 18, wp_cache_get( $this->queue::INDEX_COUNT_CACHE_KEY, $this->queue::INDEX_COUNT_CACHE_GROUP ), 'indexing ops count should be 18' ); - foreach ( $this->sync_manager->get_sync_queue() as $object_id ) { + foreach ( $sync_manager->sync_queue as $object_id ) { $results = $wpdb->get_results( $wpdb->prepare( "SELECT * FROM `{$table_name}` WHERE `object_id` = %d AND `object_type` = 'post' AND `status` = 'queued'", @@ -866,15 +866,15 @@ public function test__ratelimit_indexing__handles_start_correctly() { $this->anything() ); - $post_ids = range( 3, 9 ); - $this->add_posts_to_queue( $post_ids ); + $sync_manager = new stdClass(); + $sync_manager->sync_queue = range( 3, 9 ); $partially_mocked_queue::$max_indexing_op_count = 0; // Ensure ratelimiting is enabled $partially_mocked_queue->expects( $this->once() )->method( 'handle_index_limiting_start_timestamp' ); $partially_mocked_queue->expects( $this->once() )->method( 'maybe_alert_for_prolonged_index_limiting' ); - $partially_mocked_queue->ratelimit_indexing( true, $this->sync_manager, 'post' ); + $partially_mocked_queue->ratelimit_indexing( true, $sync_manager, 'post' ); } public function test__ratelimit_indexing__clears_start_correctly() { @@ -887,10 +887,10 @@ public function test__ratelimit_indexing__clears_start_correctly() { $partially_mocked_queue->expects( $this->once() )->method( 'clear_index_limiting_start_timestamp' ); - $post_ids = range( 3, 9 ); - $this->add_posts_to_queue( $post_ids ); + $sync_manager = new stdClass(); + $sync_manager->sync_queue = range( 3, 9 ); - $partially_mocked_queue->ratelimit_indexing( true, $this->sync_manager, 'post' ); + $partially_mocked_queue->ratelimit_indexing( true, $sync_manager, 'post' ); } /** @@ -1318,15 +1318,6 @@ public function filter_index_exists_request_bad( $request, $query, $args, $failu return $request; } - /** - * Helper function for adding an array of post objects to the sync manager queue. - */ - protected function add_posts_to_queue( $post_ids ) { - foreach ( $post_ids as $post_id ) { - $this->sync_manager->add_to_queue( $post_id ); - } - } - /** * Helper function for accessing protected methods. */ diff --git a/tests/search/includes/classes/test-class-versioning.php b/tests/search/includes/classes/test-class-versioning.php index 703e18eb7d..99fdb07360 100644 --- a/tests/search/includes/classes/test-class-versioning.php +++ b/tests/search/includes/classes/test-class-versioning.php @@ -1334,14 +1334,11 @@ public function test_replicate_indexed_objects_to_other_versions() { $sync_manager = $indexable->sync_manager; // Fake some changed posts - $sync_manager->sync_queue = [ - get_current_blog_id() => - [ - 1 => true, - 2 => true, - 3 => true, - ], - ]; + $sync_manager->sync_queue = array( + 1 => true, + 2 => true, + 3 => true, + ); // Then fire pre_ep_index_sync_queue to simulate EP performing indexing $result = apply_filters( 'pre_ep_index_sync_queue', false, $sync_manager, 'post' );