From 16079f8b039349a5188c696a9b153bcdcb57dbf4 Mon Sep 17 00:00:00 2001 From: Matthew Denton Date: Thu, 4 Nov 2021 13:27:19 -0400 Subject: [PATCH 1/6] Full Sync :: limit to contributors and above --- projects/packages/sync/changelog/fix-full-sync-user_level | 4 ++++ projects/packages/sync/src/modules/class-users.php | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 projects/packages/sync/changelog/fix-full-sync-user_level diff --git a/projects/packages/sync/changelog/fix-full-sync-user_level b/projects/packages/sync/changelog/fix-full-sync-user_level new file mode 100644 index 0000000000000..74ea48be65b9b --- /dev/null +++ b/projects/packages/sync/changelog/fix-full-sync-user_level @@ -0,0 +1,4 @@ +Significance: patch +Type: changed + +Full Sync : limit included users to contributors and above (based on wp_user_limit) diff --git a/projects/packages/sync/src/modules/class-users.php b/projects/packages/sync/src/modules/class-users.php index b4356f00f012b..c53cfc5e5b831 100644 --- a/projects/packages/sync/src/modules/class-users.php +++ b/projects/packages/sync/src/modules/class-users.php @@ -670,7 +670,7 @@ public function estimate_full_sync_actions( $config ) { public function get_where_sql( $config ) { global $wpdb; - $query = "meta_key = '{$wpdb->prefix}capabilities'"; + $query = "meta_key = '{$wpdb->prefix}user_level' AND meta_value > 0"; // The $config variable is a list of user IDs to sync. if ( is_array( $config ) ) { From 14045ff865098a51baedfda05fb9a5cae00d4804 Mon Sep 17 00:00:00 2001 From: Matthew Denton Date: Thu, 4 Nov 2021 14:04:08 -0400 Subject: [PATCH 2/6] Unit Tests update --- .../changelog/fix-full-sync-user_level | 4 ++++ ...st_class.jetpack-sync-full-immediately.php | 19 ++++++++++++------- 2 files changed, 16 insertions(+), 7 deletions(-) create mode 100644 projects/plugins/jetpack/changelog/fix-full-sync-user_level diff --git a/projects/plugins/jetpack/changelog/fix-full-sync-user_level b/projects/plugins/jetpack/changelog/fix-full-sync-user_level new file mode 100644 index 0000000000000..ffe8bddf4ba81 --- /dev/null +++ b/projects/plugins/jetpack/changelog/fix-full-sync-user_level @@ -0,0 +1,4 @@ +Significance: patch +Type: compat + +Unit Tests : Update Full Sync tests to align with limitation on users that are synced. diff --git a/projects/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-full-immediately.php b/projects/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-full-immediately.php index 07e097f9f9191..2cf420190ce61 100644 --- a/projects/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-full-immediately.php +++ b/projects/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-full-immediately.php @@ -312,10 +312,15 @@ function test_full_sync_sends_all_term_relationships_with_previous_interval_end( ) ); } + /** + * Full Sync is limited to contributor and above users based on wp_user_level. + * This test verifies only contributors are sent. + */ function test_full_sync_sends_all_users() { - $first_user_id = $this->factory->user->create(); + $this->factory->user->create( array( 'role' => 'subscriber' ) ); + $first_user_id = $this->factory->user->create( array( 'role' => 'contributor' ) ); for ( $i = 0; $i < 9; $i += 1 ) { - $user_id = $this->factory->user->create(); + $user_id = $this->factory->user->create( array( 'role' => 'contributor' ) ); } update_user_meta( $user_id, 'locale', 'en_GB' ); @@ -1049,8 +1054,8 @@ function test_full_sync_can_sync_individual_users() { function test_full_sync_doesnt_send_deleted_posts() { // previously, the behavior was to send false or throw errors - we // should actively detect false values and remove them - $keep_post_id = $this->factory->post->create(); - $delete_post_id = $this->factory->post->create(); + $keep_post_id = $this->factory->post->create( array( 'role' => 'editor' ) ); + $delete_post_id = $this->factory->post->create( array( 'role' => 'editor' ) ); $this->full_sync->start(); @@ -1090,8 +1095,8 @@ function test_full_sync_doesnt_send_deleted_users() { // previously, the behavior was to send false or throw errors - we // should actively detect false values and remove them - $keep_user_id = $this->factory->user->create(); - $delete_user_id = $this->factory->user->create(); + $keep_user_id = $this->factory->user->create( array( 'role' => 'contributor' ) ); + $delete_user_id = $this->factory->user->create( array( 'role' => 'contributor' ) ); $this->full_sync->start(); @@ -1147,7 +1152,7 @@ function test_initial_sync_doesnt_sync_subscribers() { } $this->full_sync->start(); $this->sender->do_full_sync(); - $this->assertEquals( 13, $this->server_replica_storage->user_count() ); + $this->assertEquals( 3, $this->server_replica_storage->user_count() ); $this->server_replica_storage->reset(); $this->assertEquals( 0, $this->server_replica_storage->user_count() ); $user_ids = Modules::get_module( 'users' )->get_initial_sync_user_config(); From 929482f417fd83c112d7588dab05b67ca38db1ce Mon Sep 17 00:00:00 2001 From: Matthew Denton Date: Thu, 4 Nov 2021 14:17:51 -0400 Subject: [PATCH 3/6] Another test that needed role specified. --- .../php/sync/test_class.jetpack-sync-full-immediately.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/projects/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-full-immediately.php b/projects/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-full-immediately.php index 2cf420190ce61..a1cde983eb58d 100644 --- a/projects/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-full-immediately.php +++ b/projects/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-full-immediately.php @@ -1031,9 +1031,9 @@ function test_full_sync_can_sync_individual_comments() { } function test_full_sync_can_sync_individual_users() { - $sync_user_id = $this->factory->user->create(); - $sync_user_id_2 = $this->factory->user->create(); - $no_sync_user_id = $this->factory->user->create(); + $sync_user_id = $this->factory->user->create( array( 'role' => 'editor' ) ); + $sync_user_id_2 = $this->factory->user->create( array( 'role' => 'editor' ) ); + $this->factory->user->create( array( 'role' => 'editor' ) ); $this->full_sync->start( array( 'users' => array( $sync_user_id, $sync_user_id_2 ) ) ); $this->sender->do_full_sync(); From a5b222598cafdd0787d5cd8db3fb08003960340d Mon Sep 17 00:00:00 2001 From: Matthew Denton Date: Thu, 4 Nov 2021 15:00:23 -0400 Subject: [PATCH 4/6] multisite test update --- .../php/sync/test_class.jetpack-sync-full-immediately.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/projects/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-full-immediately.php b/projects/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-full-immediately.php index a1cde983eb58d..74986bee7954d 100644 --- a/projects/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-full-immediately.php +++ b/projects/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-full-immediately.php @@ -380,8 +380,8 @@ function test_full_sync_sends_only_current_blog_users_in_multisite() { // let's create some users on the other blog switch_to_blog( $other_blog_id ); - $mu_blog_user_id = $this->factory->user->create(); - $added_mu_blog_user_id = $this->factory->user->create(); + $mu_blog_user_id = $this->factory->user->create( array( 'role' => 'contributor' ) ); + $added_mu_blog_user_id = $this->factory->user->create( array( 'role' => 'contributor' ) ); restore_current_blog(); // add one of the users to our current blog From 95574926b2460ce7d95d4146d6a15fed0d9c6893 Mon Sep 17 00:00:00 2001 From: Matthew Denton Date: Thu, 4 Nov 2021 15:19:04 -0400 Subject: [PATCH 5/6] Legacy Full Sync Unit Tests --- .../php/sync/test_class.jetpack-sync-full.php | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/projects/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-full.php b/projects/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-full.php index ece74276f5f06..52ca0c015f9b8 100644 --- a/projects/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-full.php +++ b/projects/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-full.php @@ -445,9 +445,10 @@ function test_full_sync_sends_all_term_relationships_with_previous_interval_end( } function test_full_sync_sends_all_users() { - $first_user_id = $this->factory->user->create(); + $this->factory->user->create( array( 'role' => 'subscriber' ) ); + $first_user_id = $this->factory->user->create( array( 'role' => 'contributor' ) ); for ( $i = 0; $i < 9; $i += 1 ) { - $user_id = $this->factory->user->create(); + $user_id = $this->factory->user->create( array( 'role' => 'contributor' ) ); } update_user_meta( $user_id, 'locale', 'en_GB' ); @@ -474,7 +475,7 @@ function test_full_sync_sends_previous_interval_end_for_users() { Settings::update_settings( array( 'max_queue_size_full_sync' => 1, 'max_enqueue_full_sync' => 10 ) ); for ( $i = 0; $i < 45; $i += 1 ) { - $user_ids[] = $this->factory->user->create(); + $this->factory->user->create( array( 'role' => 'contributor' ) ); } // The first event is for full sync start. @@ -1276,9 +1277,9 @@ function test_full_sync_can_sync_individual_comments() { } function test_full_sync_can_sync_individual_users() { - $sync_user_id = $this->factory->user->create(); - $sync_user_id_2 = $this->factory->user->create(); - $no_sync_user_id = $this->factory->user->create(); + $sync_user_id = $this->factory->user->create( array( 'role' => 'editor' ) ); + $sync_user_id_2 = $this->factory->user->create( array( 'role' => 'editor' ) ); + $this->factory->user->create( array( 'role' => 'editor' ) ); $this->full_sync->start( array( 'users' => array( $sync_user_id, $sync_user_id_2) ) ); $this->sender->do_full_sync(); @@ -1338,13 +1339,13 @@ function test_full_sync_doesnt_send_deleted_comments() { function test_full_sync_doesnt_send_deleted_users() { - $user_counts = count_users(); + $user_counts = count_users(); $existing_user_count = $user_counts['total_users']; // previously, the behavior was to send false or throw errors - we // should actively detect false values and remove them - $keep_user_id = $this->factory->user->create(); - $delete_user_id = $this->factory->user->create(); + $keep_user_id = $this->factory->user->create( array( 'role' => 'contributor' ) ); + $delete_user_id = $this->factory->user->create( array( 'role' => 'contributor' ) ); $this->full_sync->start(); @@ -1499,7 +1500,7 @@ function test_initial_sync_doesnt_sync_subscribers() { } $this->full_sync->start(); $this->sender->do_full_sync(); - $this->assertEquals( 13, $this->server_replica_storage->user_count() ); + $this->assertEquals( 3, $this->server_replica_storage->user_count() ); $this->server_replica_storage->reset(); $this->assertEquals( 0, $this->server_replica_storage->user_count() ); $user_ids = Modules::get_module( 'users' )->get_initial_sync_user_config(); From 95a2900dfb26b36de49c8186658bfab130841d5b Mon Sep 17 00:00:00 2001 From: Matthew Denton Date: Thu, 4 Nov 2021 15:28:35 -0400 Subject: [PATCH 6/6] duplicate tests, should purge legacy --- .../php/sync/test_class.jetpack-sync-full-immediately.php | 2 +- .../jetpack/tests/php/sync/test_class.jetpack-sync-full.php | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/projects/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-full-immediately.php b/projects/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-full-immediately.php index 74986bee7954d..afc7bfed7a1eb 100644 --- a/projects/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-full-immediately.php +++ b/projects/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-full-immediately.php @@ -368,7 +368,7 @@ function test_full_sync_sends_only_current_blog_users_in_multisite() { $original_blog_id = get_current_blog_id(); - $user_id = $this->factory->user->create(); + $user_id = $this->factory->user->create( array( 'role' => 'contributor' ) ); // NOTE this is necessary because WPMU causes certain assumptions about transients // to be wrong, and tests to explode. @see: https://github.com/sheabunge/WordPress/commit/ff4f1bb17095c6af8a0f35ac304f79074f3c3ff6 diff --git a/projects/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-full.php b/projects/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-full.php index 52ca0c015f9b8..35750b6baa7ec 100644 --- a/projects/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-full.php +++ b/projects/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-full.php @@ -530,7 +530,7 @@ function test_full_sync_sends_only_current_blog_users_in_multisite() { $original_blog_id = get_current_blog_id(); - $user_id = $this->factory->user->create(); + $user_id = $this->factory->user->create( array( 'role' => 'contributor' ) ); // NOTE this is necessary because WPMU causes certain assumptions about transients // to be wrong, and tests to explode. @see: https://github.com/sheabunge/WordPress/commit/ff4f1bb17095c6af8a0f35ac304f79074f3c3ff6 @@ -542,8 +542,8 @@ function test_full_sync_sends_only_current_blog_users_in_multisite() { // let's create some users on the other blog switch_to_blog( $other_blog_id ); - $mu_blog_user_id = $this->factory->user->create(); - $added_mu_blog_user_id = $this->factory->user->create(); + $mu_blog_user_id = $this->factory->user->create( array( 'role' => 'contributor' ) ); + $added_mu_blog_user_id = $this->factory->user->create( array( 'role' => 'contributor' ) ); restore_current_blog(); // add one of the users to our current blog