Skip to content

Commit

Permalink
Revert "Revert "Search: Allow sync manager queue to account for multi…
Browse files Browse the repository at this point in the history 
…site context"" (#6009)

* Revert "Revert "Search: Allow sync manager queue to account for multisite con…"

This reverts commit fddd9d9.

* Point EP towards test branch

* Use develop branch now merged
  • Loading branch information
@rebeccahum
rebeccahum authored Nov 25, 2024
1 parent fddd9d9 commit cb8e520
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 54 deletions.
2 changes: 1 addition & 1 deletion search/elasticpress
Submodule elasticpress updated 22 files
+1 −3 includes/classes/Indexable/Post/SyncManager.php
+6 −6 includes/classes/Indexable/Term/SyncManager.php
+3 −2 includes/classes/Indexable/User/SyncManager.php
+94 −38 includes/classes/SyncManager.php
+1 −1 tests/php/TestAdminNotices.php
+1 −1 tests/php/TestFeatureActivation.php
+1 −1 tests/php/TestInstaller.php
+1 −1 tests/php/TestScreen.php
+1 −1 tests/php/TestStats.php
+1 −1 tests/php/TestUtils.php
+1 −1 tests/php/features/TestComments.php
+1 −1 tests/php/features/TestProtectedContent.php
+1 −1 tests/php/features/TestRelatedPosts.php
+1 −1 tests/php/features/TestSearch.php
+1 −1 tests/php/features/TestSearchOrdering.php
+1 −1 tests/php/features/TestSynonyms.php
+1 −1 tests/php/features/TestWeighting.php
+1 −1 tests/php/features/TestWooCommerce.php
+6 −6 tests/php/indexables/TestComment.php
+8 −8 tests/php/indexables/TestPost.php
+5 −5 tests/php/indexables/TestTerm.php
+4 −4 tests/php/indexables/TestUser.php
10 changes: 5 additions & 5 deletions search/includes/classes/class-queue.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1028,19 +1028,19 @@ public function intercept_ep_sync_manager_indexing( $bail, $sync_manager, $index
return $bail;
}

if ( empty( $sync_manager->sync_queue ) ) {
if ( empty( $sync_manager->get_sync_queue() ) ) {
return $bail;
}

$this->queue_objects( array_keys( $sync_manager->sync_queue ), $indexable_slug );
$this->queue_objects( array_keys( $sync_manager->get_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() ) {
$this->cron->schedule_batch_job();
}

// Empty out the queue now that we've queued those items up
$sync_manager->sync_queue = [];
$sync_manager->reset_sync_queue();

return true;
}
Expand Down Expand Up @@ -1081,12 +1081,12 @@ public function ratelimit_indexing( $bail, $sync_manager, $indexable_slug ) {
return $bail;
}

if ( empty( $sync_manager->sync_queue ) ) {
if ( empty( $sync_manager->get_sync_queue() ) ) {
return $bail;
}

// Increment first to prevent overrunning ratelimiting
$increment = count( $sync_manager->sync_queue );
$increment = count( $sync_manager->get_sync_queue() );
$index_count_in_period = static::index_count_incr( $increment );

// If indexing operation ratelimiting is hit, queue index operations
Expand Down
4 changes: 2 additions & 2 deletions search/includes/classes/class-versioning.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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->sync_queue ) ) {
if ( empty( $inactive_versions ) || empty( $sync_manager->get_sync_queue() ) ) {
return $bail;
}

Expand All @@ -856,7 +856,7 @@ public function filter__pre_ep_index_sync_queue( $bail, $sync_manager, $indexabl
'index_version' => $version['number'],
);

foreach ( $sync_manager->sync_queue as $object_id => $value ) {
foreach ( $sync_manager->get_sync_queue() as $object_id => $value ) {
/**
* This filter is documented in Versioning::replicate_queued_objects_to_other_versions
*/
Expand Down
91 changes: 50 additions & 41 deletions tests/search/includes/classes/test-class-queue.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ class Queue_Test extends WP_UnitTestCase {
/** @var Queue */
private $queue;

/** @var SyncManager */
private $sync_manager;

public function setUp(): void {
parent::setUp();

Expand All @@ -43,6 +46,9 @@ 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 {
Expand Down Expand Up @@ -302,18 +308,18 @@ public function test_checkout_jobs() {
}

public function test_offload_indexing_to_queue() {
$mock_sync_manager = (object) array( 'sync_queue' => [ 1, 2, 3 ] );
$this->add_posts_to_queue( range( 1, 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, $mock_sync_manager, 'post' );
$current_bail = apply_filters( 'pre_ep_index_sync_queue', false, $this->sync_manager, 'post' );

$this->assertFalse( $current_bail );

// Offload indexing
$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, $mock_sync_manager, 'post' );
$current_bail = apply_filters( 'pre_ep_index_sync_queue', false, $this->sync_manager, 'post' );

$this->assertTrue( $current_bail );
}
Expand Down Expand Up @@ -473,16 +479,12 @@ public function test_process_jobs() {
}

public function test_intercept_ep_sync_manager_indexing() {
$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->add_posts_to_queue( [ 1, 2, 1000 ] );

$this->queue->intercept_ep_sync_manager_indexing( false, $mock_sync_manager, 'post' );
$this->queue->intercept_ep_sync_manager_indexing( false, $this->sync_manager, 'post' );

// And the SyncManager's queue should have been emptied
$this->assertEmpty( $mock_sync_manager->sync_queue );
$this->assertEmpty( $this->sync_manager->get_sync_queue() );
}

public function test_get_jobs_by_range() {
Expand Down Expand Up @@ -716,11 +718,8 @@ 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() {
$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' );
$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' );
}

/**
Expand All @@ -734,11 +733,8 @@ 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, $sync_manager, 'post' );
$this->queue->ratelimit_indexing( true, $this->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' );
}
Expand All @@ -751,16 +747,15 @@ public function test_ratelimit_indexing_queue_should_be_empty_if_no_ratelimiting

$table_name = $this->queue->schema->get_table_name();

$sync_manager = new stdClass();
$sync_manager->sync_queue = range( 3, 9 );
$this->add_posts_to_queue( range( 3, 9 ) );

$this->queue::$max_indexing_op_count = PHP_INT_MAX; // Ensure ratelimiting is disabled

$this->queue->ratelimit_indexing( true, $sync_manager, 'post' );
$this->queue->ratelimit_indexing( true, $this->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 ( $sync_manager->sync_queue as $object_id ) {
foreach ( $this->sync_manager->get_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'",
Expand All @@ -771,13 +766,16 @@ 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" );
}

$sync_manager->sync_queue = range( 10, 20 );
$this->sync_manager->reset_sync_queue();

$post_ids = range( 10, 20 );
$this->add_posts_to_queue( $post_ids );

$this->queue->ratelimit_indexing( true, $sync_manager, 'post' );
$this->queue->ratelimit_indexing( true, $this->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 ( $sync_manager->sync_queue as $object_id ) {
foreach ( $this->sync_manager->get_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'",
Expand All @@ -797,21 +795,22 @@ public function test_ratelimit_indexing_queue_should_be_populated_if_ratelimitin

$table_name = $this->queue->schema->get_table_name();

$sync_manager = new stdClass();
$sync_manager->sync_queue = [ 1 ];
$this->add_posts_to_queue( [ 1 ] );

$this->queue->offload_indexing_to_queue();
$current_bail = apply_filters( 'pre_ep_index_sync_queue', false, $sync_manager, 'post' );
$current_bail = apply_filters( 'pre_ep_index_sync_queue', false, $this->sync_manager, 'post' );
$this->assertTrue( $current_bail );

$sync_manager->sync_queue = range( 3, 9 );
$post_ids = range( 3, 9 );
$this->add_posts_to_queue( $post_ids );

$this->queue::$max_indexing_op_count = 0; // Ensure ratelimiting is enabled

$this->queue->ratelimit_indexing( true, $sync_manager, 'post' );
$this->queue->ratelimit_indexing( true, $this->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 ( $sync_manager->sync_queue as $object_id ) {
foreach ( $this->sync_manager->get_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'",
Expand All @@ -822,13 +821,14 @@ 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" );
}

$sync_manager->sync_queue = range( 10, 20 );
$post_ids = range( 10, 20 );
$this->add_posts_to_queue( $post_ids );

$this->queue->ratelimit_indexing( true, $sync_manager, 'post' );
$this->queue->ratelimit_indexing( true, $this->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 ( $sync_manager->sync_queue as $object_id ) {
foreach ( $this->sync_manager->get_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'",
Expand Down Expand Up @@ -866,15 +866,15 @@ public function test__ratelimit_indexing__handles_start_correctly() {
$this->anything()
);

$sync_manager = new stdClass();
$sync_manager->sync_queue = range( 3, 9 );
$post_ids = range( 3, 9 );
$this->add_posts_to_queue( $post_ids );

$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, $sync_manager, 'post' );
$partially_mocked_queue->ratelimit_indexing( true, $this->sync_manager, 'post' );
}

public function test__ratelimit_indexing__clears_start_correctly() {
Expand All @@ -887,10 +887,10 @@ public function test__ratelimit_indexing__clears_start_correctly() {

$partially_mocked_queue->expects( $this->once() )->method( 'clear_index_limiting_start_timestamp' );

$sync_manager = new stdClass();
$sync_manager->sync_queue = range( 3, 9 );
$post_ids = range( 3, 9 );
$this->add_posts_to_queue( $post_ids );

$partially_mocked_queue->ratelimit_indexing( true, $sync_manager, 'post' );
$partially_mocked_queue->ratelimit_indexing( true, $this->sync_manager, 'post' );
}

/**
Expand Down Expand Up @@ -1318,6 +1318,15 @@ 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.
*/
Expand Down
13 changes: 8 additions & 5 deletions tests/search/includes/classes/test-class-versioning.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1334,11 +1334,14 @@ public function test_replicate_indexed_objects_to_other_versions() {
$sync_manager = $indexable->sync_manager;

// Fake some changed posts
$sync_manager->sync_queue = array(
1 => true,
2 => true,
3 => true,
);
$sync_manager->sync_queue = [
get_current_blog_id() =>
[
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' );
Expand Down

0 comments on commit cb8e520

Please sign in to comment.