From 524aabea1d8e7f01ec9e9a2b0f46c8fb156aa0b6 Mon Sep 17 00:00:00 2001 From: Pavel Dohnal Date: Tue, 5 Sep 2017 08:36:46 +0100 Subject: [PATCH 01/12] Split users sync into multiple queries [MAILPOET-1073] --- lib/Segments/WP.php | 57 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 52 insertions(+), 5 deletions(-) diff --git a/lib/Segments/WP.php b/lib/Segments/WP.php index eceaecccca..7120eeb652 100644 --- a/lib/Segments/WP.php +++ b/lib/Segments/WP.php @@ -81,6 +81,54 @@ class WP { static function synchronizeUsers() { // get wordpress users list $wp_segment = Segment::getWPSegment(); + $s = microtime(true); + + // insert all wordpress users from wp table + Subscriber::raw_execute(' + INSERT IGNORE INTO wp_mailpoet_subscribers(wp_user_id, email, status, created_at) + SELECT wu.id, wu.user_email, "subscribed", CURRENT_TIMESTAMP() FROM wp_users wu + LEFT JOIN wp_mailpoet_subscribers mps ON wu.id = mps.wp_user_id + WHERE mps.wp_user_id IS NULL + '); + + // update first name + Subscriber::raw_execute(' + UPDATE wp_mailpoet_subscribers + JOIN wp_usermeta ON wp_mailpoet_subscribers.wp_user_id = wp_usermeta.user_id AND meta_key = "first_name" + SET first_name = meta_value + WHERE wp_mailpoet_subscribers.first_name = "" + AND wp_mailpoet_subscribers.wp_user_id IS NOT NULL + '); + + // update last name + Subscriber::raw_execute(' + UPDATE wp_mailpoet_subscribers + JOIN wp_usermeta ON wp_mailpoet_subscribers.wp_user_id = wp_usermeta.user_id AND meta_key = "last_name" + SET last_name = meta_value + WHERE wp_mailpoet_subscribers.last_name = "" + AND wp_mailpoet_subscribers.wp_user_id IS NOT NULL + '); + + // use display name if first name is missing + Subscriber::raw_execute(' + UPDATE wp_mailpoet_subscribers + JOIN wp_users ON wp_mailpoet_subscribers.wp_user_id = wp_users.id + SET first_name = display_name + WHERE wp_mailpoet_subscribers.first_name = "" + AND wp_mailpoet_subscribers.wp_user_id IS NOT NULL + '); + + // insert users to the wp users list + Subscriber::raw_execute(sprintf(' + INSERT IGNORE INTO wp_mailpoet_subscriber_segment(subscriber_id, segment_id, created_at) + SELECT mps.id, "%s", CURRENT_TIMESTAMP() FROM wp_mailpoet_subscribers mps + WHERE mps.wp_user_id IS NOT NULL + ', $wp_segment->id)); + + $e = microtime(true) - $s; + + file_put_contents('/tmp/whole', $e); + $s = microtime(true); // fetch all wp users id $wp_users = \get_users(array( @@ -88,11 +136,6 @@ class WP { 'fields' => 'ID' )); - // update data for each wp user - foreach($wp_users as $wp_user_id) { - static::synchronizeUser($wp_user_id); - } - // remove orphaned wp segment subscribers (not having a matching wp user id), // e.g. if wp users were deleted directly from the database $wp_segment->subscribers() @@ -100,7 +143,11 @@ class WP { ->findResultSet() ->set('wp_user_id', null) ->delete(); + $e = microtime(true) - $s; + + file_put_contents('/tmp/whole-e', $e); return true; } } + From 8757598a2d3602406e725a6ba0802feba8834b5b Mon Sep 17 00:00:00 2001 From: Pavel Dohnal Date: Tue, 5 Sep 2017 11:44:24 +0100 Subject: [PATCH 02/12] Add index [MAILPOET-1073] --- lib/Config/Migrator.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Config/Migrator.php b/lib/Config/Migrator.php index b8342b4cd3..a1ca8c33a0 100644 --- a/lib/Config/Migrator.php +++ b/lib/Config/Migrator.php @@ -172,7 +172,8 @@ class Migrator { 'deleted_at TIMESTAMP NULL,', 'unconfirmed_data longtext,', 'PRIMARY KEY (id),', - 'UNIQUE KEY email (email)' + 'UNIQUE KEY email (email)', + 'KEY wp_user_id (wp_user_id)', ); return $this->sqlify(__FUNCTION__, $attributes); } From 91e8461cac6e06dc8af84c346ed22554615d0022 Mon Sep 17 00:00:00 2001 From: Pavel Dohnal Date: Tue, 5 Sep 2017 11:45:00 +0100 Subject: [PATCH 03/12] Clean the code [MAILPOET-1073] --- lib/Segments/WP.php | 130 +++++++++++++++++++++++++++----------------- 1 file changed, 79 insertions(+), 51 deletions(-) diff --git a/lib/Segments/WP.php b/lib/Segments/WP.php index 7120eeb652..7fb675b250 100644 --- a/lib/Segments/WP.php +++ b/lib/Segments/WP.php @@ -79,56 +79,15 @@ class WP { } static function synchronizeUsers() { - // get wordpress users list $wp_segment = Segment::getWPSegment(); - $s = microtime(true); - // insert all wordpress users from wp table - Subscriber::raw_execute(' - INSERT IGNORE INTO wp_mailpoet_subscribers(wp_user_id, email, status, created_at) - SELECT wu.id, wu.user_email, "subscribed", CURRENT_TIMESTAMP() FROM wp_users wu - LEFT JOIN wp_mailpoet_subscribers mps ON wu.id = mps.wp_user_id - WHERE mps.wp_user_id IS NULL - '); - - // update first name - Subscriber::raw_execute(' - UPDATE wp_mailpoet_subscribers - JOIN wp_usermeta ON wp_mailpoet_subscribers.wp_user_id = wp_usermeta.user_id AND meta_key = "first_name" - SET first_name = meta_value - WHERE wp_mailpoet_subscribers.first_name = "" - AND wp_mailpoet_subscribers.wp_user_id IS NOT NULL - '); - - // update last name - Subscriber::raw_execute(' - UPDATE wp_mailpoet_subscribers - JOIN wp_usermeta ON wp_mailpoet_subscribers.wp_user_id = wp_usermeta.user_id AND meta_key = "last_name" - SET last_name = meta_value - WHERE wp_mailpoet_subscribers.last_name = "" - AND wp_mailpoet_subscribers.wp_user_id IS NOT NULL - '); - - // use display name if first name is missing - Subscriber::raw_execute(' - UPDATE wp_mailpoet_subscribers - JOIN wp_users ON wp_mailpoet_subscribers.wp_user_id = wp_users.id - SET first_name = display_name - WHERE wp_mailpoet_subscribers.first_name = "" - AND wp_mailpoet_subscribers.wp_user_id IS NOT NULL - '); - - // insert users to the wp users list - Subscriber::raw_execute(sprintf(' - INSERT IGNORE INTO wp_mailpoet_subscriber_segment(subscriber_id, segment_id, created_at) - SELECT mps.id, "%s", CURRENT_TIMESTAMP() FROM wp_mailpoet_subscribers mps - WHERE mps.wp_user_id IS NOT NULL - ', $wp_segment->id)); - - $e = microtime(true) - $s; - - file_put_contents('/tmp/whole', $e); - $s = microtime(true); + self::updateSubscribersEmails(); + self::insertSubscribers(); + self::updateFirstNames(); + self::updateLastNames(); + self::updateFristNameIfMissing(); + self::insertUsersToSegment($wp_segment); + self::removeFromTrash(); // fetch all wp users id $wp_users = \get_users(array( @@ -143,11 +102,80 @@ class WP { ->findResultSet() ->set('wp_user_id', null) ->delete(); - $e = microtime(true) - $s; - - file_put_contents('/tmp/whole-e', $e); return true; } + + private static function updateSubscribersEmails() { + $subscribers_table = Subscriber::$_table; + Subscriber::raw_execute(sprintf(' + UPDATE IGNORE %s + JOIN wp_users ON %s.wp_user_id = wp_users.id + SET email = user_email + WHERE %s.wp_user_id IS NOT NULL + ', $subscribers_table, $subscribers_table, $subscribers_table)); + } + + private static function insertSubscribers() { + $subscribers_table = Subscriber::$_table; + Subscriber::raw_execute(sprintf(' + INSERT IGNORE INTO %s(wp_user_id, email, status, created_at) + SELECT wu.id, wu.user_email, "subscribed", CURRENT_TIMESTAMP() FROM wp_users wu + LEFT JOIN %s mps ON wu.id = mps.wp_user_id + WHERE mps.wp_user_id IS NULL + ', $subscribers_table, $subscribers_table)); + } + + private static function updateFirstNames() { + $subscribers_table = Subscriber::$_table; + Subscriber::raw_execute(sprintf(' + UPDATE %s + JOIN wp_usermeta ON %s.wp_user_id = wp_usermeta.user_id AND meta_key = "first_name" + SET first_name = meta_value + WHERE %s.first_name = "" + AND %s.wp_user_id IS NOT NULL + ', $subscribers_table, $subscribers_table, $subscribers_table, $subscribers_table)); + } + + private static function updateLastNames() { + $subscribers_table = Subscriber::$_table; + Subscriber::raw_execute(sprintf(' + UPDATE %s + JOIN wp_usermeta ON %s.wp_user_id = wp_usermeta.user_id AND meta_key = "last_name" + SET last_name = meta_value + WHERE %s.last_name = "" + AND %s.wp_user_id IS NOT NULL + ', $subscribers_table, $subscribers_table, $subscribers_table, $subscribers_table)); + } + + private static function updateFristNameIfMissing() { + $subscribers_table = Subscriber::$_table; + Subscriber::raw_execute(sprintf(' + UPDATE %s + JOIN wp_users ON %s.wp_user_id = wp_users.id + SET first_name = display_name + WHERE %s.first_name = "" + AND %s.wp_user_id IS NOT NULL + ', $subscribers_table, $subscribers_table, $subscribers_table, $subscribers_table)); + } + + private static function insertUsersToSegment($wp_segment) { + $subscribers_table = Subscriber::$_table; + $wp_mailpoet_subscriber_segment_table = SubscriberSegment::$_table; + Subscriber::raw_execute(sprintf(' + INSERT IGNORE INTO %s(subscriber_id, segment_id, created_at) + SELECT mps.id, "%s", CURRENT_TIMESTAMP() FROM %s mps + WHERE mps.wp_user_id IS NOT NULL + ', $wp_mailpoet_subscriber_segment_table, $wp_segment->id, $subscribers_table)); + } + + private static function removeFromTrash() { + $subscribers_table = Subscriber::$_table; + Subscriber::raw_execute(sprintf(' + UPDATE %s + SET deleted_at = NULL + WHERE %s.wp_user_id IS NOT NULL + ', $subscribers_table, $subscribers_table)); + } } From 922d2b4b5fa435095fe4e494401cd23ff89fc480 Mon Sep 17 00:00:00 2001 From: Pavel Dohnal Date: Tue, 5 Sep 2017 12:03:37 +0100 Subject: [PATCH 04/12] Fix migration [MAILPOET-1073] --- lib/Config/Migrator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Config/Migrator.php b/lib/Config/Migrator.php index a1ca8c33a0..42bcbc1dc1 100644 --- a/lib/Config/Migrator.php +++ b/lib/Config/Migrator.php @@ -172,7 +172,7 @@ class Migrator { 'deleted_at TIMESTAMP NULL,', 'unconfirmed_data longtext,', 'PRIMARY KEY (id),', - 'UNIQUE KEY email (email)', + 'UNIQUE KEY email (email),', 'KEY wp_user_id (wp_user_id)', ); return $this->sqlify(__FUNCTION__, $attributes); From 784a80d1a5d256641dc8d51c9c6524400dfec5d2 Mon Sep 17 00:00:00 2001 From: Pavel Dohnal Date: Wed, 6 Sep 2017 10:33:07 +0100 Subject: [PATCH 05/12] Use ->prefix instead of wp_ [MAILPOET-1073] --- lib/Segments/WP.php | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/lib/Segments/WP.php b/lib/Segments/WP.php index 7fb675b250..68a3a5685d 100644 --- a/lib/Segments/WP.php +++ b/lib/Segments/WP.php @@ -107,56 +107,61 @@ class WP { } private static function updateSubscribersEmails() { + global $wpdb; $subscribers_table = Subscriber::$_table; Subscriber::raw_execute(sprintf(' UPDATE IGNORE %s - JOIN wp_users ON %s.wp_user_id = wp_users.id + JOIN %susers as wu ON %s.wp_user_id = wu.id SET email = user_email WHERE %s.wp_user_id IS NOT NULL - ', $subscribers_table, $subscribers_table, $subscribers_table)); + ', $subscribers_table, $wpdb->prefix, $subscribers_table, $subscribers_table)); } private static function insertSubscribers() { + global $wpdb; $subscribers_table = Subscriber::$_table; Subscriber::raw_execute(sprintf(' INSERT IGNORE INTO %s(wp_user_id, email, status, created_at) - SELECT wu.id, wu.user_email, "subscribed", CURRENT_TIMESTAMP() FROM wp_users wu + SELECT wu.id, wu.user_email, "subscribed", CURRENT_TIMESTAMP() FROM %susers wu LEFT JOIN %s mps ON wu.id = mps.wp_user_id WHERE mps.wp_user_id IS NULL - ', $subscribers_table, $subscribers_table)); + ', $subscribers_table, $wpdb->prefix, $subscribers_table)); } private static function updateFirstNames() { + global $wpdb; $subscribers_table = Subscriber::$_table; Subscriber::raw_execute(sprintf(' UPDATE %s - JOIN wp_usermeta ON %s.wp_user_id = wp_usermeta.user_id AND meta_key = "first_name" + JOIN %susermeta as wpum ON %s.wp_user_id = wpum.user_id AND meta_key = "first_name" SET first_name = meta_value WHERE %s.first_name = "" AND %s.wp_user_id IS NOT NULL - ', $subscribers_table, $subscribers_table, $subscribers_table, $subscribers_table)); + ', $subscribers_table, $wpdb->prefix, $subscribers_table, $subscribers_table, $subscribers_table)); } private static function updateLastNames() { + global $wpdb; $subscribers_table = Subscriber::$_table; Subscriber::raw_execute(sprintf(' UPDATE %s - JOIN wp_usermeta ON %s.wp_user_id = wp_usermeta.user_id AND meta_key = "last_name" + JOIN %susermeta as wpum ON %s.wp_user_id = wpum.user_id AND meta_key = "last_name" SET last_name = meta_value WHERE %s.last_name = "" AND %s.wp_user_id IS NOT NULL - ', $subscribers_table, $subscribers_table, $subscribers_table, $subscribers_table)); + ', $subscribers_table, $wpdb->prefix, $subscribers_table, $subscribers_table, $subscribers_table)); } private static function updateFristNameIfMissing() { + global $wpdb; $subscribers_table = Subscriber::$_table; Subscriber::raw_execute(sprintf(' UPDATE %s - JOIN wp_users ON %s.wp_user_id = wp_users.id + JOIN %susers wu ON %s.wp_user_id = wu.id SET first_name = display_name WHERE %s.first_name = "" AND %s.wp_user_id IS NOT NULL - ', $subscribers_table, $subscribers_table, $subscribers_table, $subscribers_table)); + ', $subscribers_table, $wpdb->prefix, $subscribers_table, $subscribers_table, $subscribers_table)); } private static function insertUsersToSegment($wp_segment) { From 8a362f49f876d3f8152cf09fd9c77141535aff9b Mon Sep 17 00:00:00 2001 From: Pavel Dohnal Date: Thu, 7 Sep 2017 13:41:11 +0100 Subject: [PATCH 06/12] Add test for users synchronisation [MAILPOET-1073] --- .../unit/Segments/WPSynchronizeUsersTest.php | 164 ++++++++++++++++++ 1 file changed, 164 insertions(+) create mode 100644 tests/unit/Segments/WPSynchronizeUsersTest.php diff --git a/tests/unit/Segments/WPSynchronizeUsersTest.php b/tests/unit/Segments/WPSynchronizeUsersTest.php new file mode 100644 index 0000000000..0c12287561 --- /dev/null +++ b/tests/unit/Segments/WPSynchronizeUsersTest.php @@ -0,0 +1,164 @@ +insertUser(); + $this->insertUser(); + WP::synchronizeUsers(); + $subscribersCount = $this->getSubscribersCount(); + expect($subscribersCount)->equals(2); + } + + function testItSynchronizeNewUsers() { + $this->insertUser(); + $this->insertUser(); + WP::synchronizeUsers(); + $this->insertUser(); + WP::synchronizeUsers(); + $subscribersCount = $this->getSubscribersCount(); + expect($subscribersCount)->equals(3); + } + + function testItSynchronizeEmails() { + $id = $this->insertUser(); + WP::synchronizeUsers(); + $this->updateWPUserEmail($id,'user-sync-test-xx@email.com'); + WP::synchronizeUsers(); + $subscriber = Subscriber::where('wp_user_id', $id)->findOne(); + expect($subscriber->email)->equals('user-sync-test-xx@email.com'); + } + + function testItSynchronizeFirstNames() { + $id = $this->insertUser(); + WP::synchronizeUsers(); + update_user_meta($id, 'first_name', 'First name'); + WP::synchronizeUsers(); + $subscriber = Subscriber::where('wp_user_id', $id)->findOne(); + expect($subscriber->first_name)->equals('First name'); + } + + function testItSynchronizeLastNames() { + $id = $this->insertUser(); + WP::synchronizeUsers(); + update_user_meta($id, 'last_name', 'Last name'); + WP::synchronizeUsers(); + $subscriber = Subscriber::where('wp_user_id', $id)->findOne(); + expect($subscriber->last_name)->equals('Last name'); + } + + function testItSynchronizeFirstNamesUsingDisplayName() { + $id = $this->insertUser(); + WP::synchronizeUsers(); + $this->updateWPUserDisplayName($id, 'First name'); + WP::synchronizeUsers(); + $subscriber = Subscriber::where('wp_user_id', $id)->findOne(); + expect($subscriber->first_name)->equals('First name'); + } + + function testItSynchronizeSegment() { + $this->insertUser(); + $this->insertUser(); + $this->insertUser(); + WP::synchronizeUsers(); + $subscribers = Segment::getWPSegment()->subscribers()->whereIn('wp_user_id', $this->userIds); + expect($subscribers->count())->equals(3); + } + + function testItRemovesUsersFromTrash() { + $id = $this->insertUser(); + WP::synchronizeUsers(); + $subscriber = Subscriber::where("wp_user_id", $id)->findOne(); + $subscriber->deleted_at = Carbon::now(); + $subscriber->save(); + WP::synchronizeUsers(); + $subscriber = Subscriber::where("wp_user_id", $id)->findOne(); + expect($subscriber->deleted_at)->null(); + } + + function testItRemovesOrphanedSubscribers() { + $this->insertUser(); + $id = $this->insertUser(); + WP::synchronizeUsers(); + $this->deleteWPUser($id); + WP::synchronizeUsers(); + $subscribers = Segment::getWPSegment()->subscribers()->whereIn('wp_user_id', $this->userIds); + expect($subscribers->count())->equals(1); + } + + function _before() { + \ORM::raw_execute('TRUNCATE ' . Segment::$_table); + global $wpdb; + $db = \ORM::getDb(); + $db->exec(sprintf(' + DELETE FROM + %susers + WHERE + user_email LIKE "user-sync-test%%" + ', $wpdb->prefix)); + } + + private function getSubscribersCount() { + return Subscriber::whereIn("wp_user_id", $this->userIds)->count(); + } + + private function insertUser() { + global $wpdb; + $db = \ORM::getDb(); + $db->exec(sprintf(' + INSERT INTO + %susers(user_login, user_email) + VALUES + (CONCAT("user-sync-test", rand()), CONCAT("user", rand(), "@example.com"))', $wpdb->prefix)); + $id = $db->lastInsertId(); + $this->userIds[] = $id; + return $id; + } + + private function updateWPUserEmail($id, $email) { + global $wpdb; + $db = \ORM::getDb(); + $db->exec(sprintf(' + UPDATE + %susers + SET user_email = "%s" + WHERE + id = %s + ', $wpdb->prefix, $email, $id)); + } + + private function updateWPUserDisplayName($id, $name) { + global $wpdb; + $db = \ORM::getDb(); + $db->exec(sprintf(' + UPDATE + %susers + SET display_name = "%s" + WHERE + id = %s + ', $wpdb->prefix, $name, $id)); + } + + private function deleteWPUser($id) { + global $wpdb; + $db = \ORM::getDb(); + $db->exec(sprintf(' + DELETE FROM + %susers + WHERE + id = %s + ', $wpdb->prefix, $id)); + } + +} \ No newline at end of file From c2789cdac3bfd48e1db6f62bc023ace03166927f Mon Sep 17 00:00:00 2001 From: Pavel Dohnal Date: Thu, 7 Sep 2017 13:42:00 +0100 Subject: [PATCH 07/12] Don't load all ids to memory, PHP could crash [MAILPOET-1073] --- lib/Segments/WP.php | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/lib/Segments/WP.php b/lib/Segments/WP.php index 68a3a5685d..b025508d18 100644 --- a/lib/Segments/WP.php +++ b/lib/Segments/WP.php @@ -79,29 +79,15 @@ class WP { } static function synchronizeUsers() { - $wp_segment = Segment::getWPSegment(); self::updateSubscribersEmails(); self::insertSubscribers(); + self::removeFromTrash(); self::updateFirstNames(); self::updateLastNames(); self::updateFristNameIfMissing(); - self::insertUsersToSegment($wp_segment); - self::removeFromTrash(); - - // fetch all wp users id - $wp_users = \get_users(array( - 'count_total' => false, - 'fields' => 'ID' - )); - - // remove orphaned wp segment subscribers (not having a matching wp user id), - // e.g. if wp users were deleted directly from the database - $wp_segment->subscribers() - ->whereNotIn('wp_user_id', $wp_users) - ->findResultSet() - ->set('wp_user_id', null) - ->delete(); + self::insertUsersToSegment(); + self::removeOrphanedSubscribers(); return true; } @@ -164,7 +150,8 @@ class WP { ', $subscribers_table, $wpdb->prefix, $subscribers_table, $subscribers_table, $subscribers_table)); } - private static function insertUsersToSegment($wp_segment) { + private static function insertUsersToSegment() { + $wp_segment = Segment::getWPSegment(); $subscribers_table = Subscriber::$_table; $wp_mailpoet_subscriber_segment_table = SubscriberSegment::$_table; Subscriber::raw_execute(sprintf(' @@ -182,5 +169,18 @@ class WP { WHERE %s.wp_user_id IS NOT NULL ', $subscribers_table, $subscribers_table)); } + + private static function removeOrphanedSubscribers() { + // remove orphaned wp segment subscribers (not having a matching wp user id), + // e.g. if wp users were deleted directly from the database + global $wpdb; + $subscribers_table = Subscriber::$_table; + Subscriber::raw_execute(sprintf(' + UPDATE %s as wpms + LEFT JOIN %susers as wu ON wpms.wp_user_id = wu.id + SET wp_user_id = NULL + WHERE wu.ID IS NULL; + ', $subscribers_table, $wpdb->prefix)); + } } From e86b7804797f8d883dd1ced7b8441bfab54a38c6 Mon Sep 17 00:00:00 2001 From: Pavel Dohnal Date: Thu, 7 Sep 2017 14:19:33 +0100 Subject: [PATCH 08/12] Fix build` [MAILPOET-1073] --- tests/unit/Segments/WPSynchronizeUsersTest.php | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/unit/Segments/WPSynchronizeUsersTest.php b/tests/unit/Segments/WPSynchronizeUsersTest.php index 0c12287561..c073d36f85 100644 --- a/tests/unit/Segments/WPSynchronizeUsersTest.php +++ b/tests/unit/Segments/WPSynchronizeUsersTest.php @@ -34,7 +34,7 @@ class WPSynchronizeUsersTest extends \MailPoetTest { function testItSynchronizeEmails() { $id = $this->insertUser(); WP::synchronizeUsers(); - $this->updateWPUserEmail($id,'user-sync-test-xx@email.com'); + $this->updateWPUserEmail($id, 'user-sync-test-xx@email.com'); WP::synchronizeUsers(); $subscriber = Subscriber::where('wp_user_id', $id)->findOne(); expect($subscriber->email)->equals('user-sync-test-xx@email.com'); @@ -118,9 +118,13 @@ class WPSynchronizeUsersTest extends \MailPoetTest { $db = \ORM::getDb(); $db->exec(sprintf(' INSERT INTO - %susers(user_login, user_email) + %susers(user_login, user_email, user_registered) VALUES - (CONCAT("user-sync-test", rand()), CONCAT("user", rand(), "@example.com"))', $wpdb->prefix)); + ( + CONCAT("user-sync-test", rand()), + CONCAT("user", rand(), "@example.com"), + "2017-01-02 12:31:12" + )', $wpdb->prefix)); $id = $db->lastInsertId(); $this->userIds[] = $id; return $id; From 272f552f3cf07c046c0904c8dfc3ca731d0d3d5d Mon Sep 17 00:00:00 2001 From: Pavel Dohnal Date: Mon, 11 Sep 2017 09:22:18 +0100 Subject: [PATCH 09/12] Clean data befre and after test run [MAILPOET-1073] --- tests/unit/Segments/WPSynchronizeUsersTest.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/unit/Segments/WPSynchronizeUsersTest.php b/tests/unit/Segments/WPSynchronizeUsersTest.php index c073d36f85..594ffede6a 100644 --- a/tests/unit/Segments/WPSynchronizeUsersTest.php +++ b/tests/unit/Segments/WPSynchronizeUsersTest.php @@ -98,6 +98,14 @@ class WPSynchronizeUsersTest extends \MailPoetTest { } function _before() { + $this->cleanData(); + } + + function _after() { + $this->cleanData(); + } + + private function cleanData() { \ORM::raw_execute('TRUNCATE ' . Segment::$_table); global $wpdb; $db = \ORM::getDb(); From eef969439b2c7a88847243c8e5be45ba9c5afcef Mon Sep 17 00:00:00 2001 From: Pavel Dohnal Date: Mon, 11 Sep 2017 09:22:40 +0100 Subject: [PATCH 10/12] Code comment [MAILPOET-1073] --- tests/unit/Segments/WPSynchronizeUsersTest.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/unit/Segments/WPSynchronizeUsersTest.php b/tests/unit/Segments/WPSynchronizeUsersTest.php index 594ffede6a..e6dee46ad6 100644 --- a/tests/unit/Segments/WPSynchronizeUsersTest.php +++ b/tests/unit/Segments/WPSynchronizeUsersTest.php @@ -121,6 +121,13 @@ class WPSynchronizeUsersTest extends \MailPoetTest { return Subscriber::whereIn("wp_user_id", $this->userIds)->count(); } + /** + * Insert a user without invoking wp hooks. + * Those tests are testing user synchronisation, so we need data in wp_users table which has not been synchronised to + * mailpoet database yet. We cannot use wp_insert_user functions because they would do the sync on insert. + * + * @return string + */ private function insertUser() { global $wpdb; $db = \ORM::getDb(); From 607bf51b37d803c68bef0f53650d065e67312e81 Mon Sep 17 00:00:00 2001 From: Pavel Dohnal Date: Mon, 11 Sep 2017 09:28:39 +0100 Subject: [PATCH 11/12] Merge two test cases for one class [MAILPOET-1073] --- .../unit/Segments/WPSynchronizeUsersTest.php | 183 --------------- tests/unit/Segments/WPTest.php | 215 ++++++++++++++---- 2 files changed, 177 insertions(+), 221 deletions(-) delete mode 100644 tests/unit/Segments/WPSynchronizeUsersTest.php diff --git a/tests/unit/Segments/WPSynchronizeUsersTest.php b/tests/unit/Segments/WPSynchronizeUsersTest.php deleted file mode 100644 index e6dee46ad6..0000000000 --- a/tests/unit/Segments/WPSynchronizeUsersTest.php +++ /dev/null @@ -1,183 +0,0 @@ -insertUser(); - $this->insertUser(); - WP::synchronizeUsers(); - $subscribersCount = $this->getSubscribersCount(); - expect($subscribersCount)->equals(2); - } - - function testItSynchronizeNewUsers() { - $this->insertUser(); - $this->insertUser(); - WP::synchronizeUsers(); - $this->insertUser(); - WP::synchronizeUsers(); - $subscribersCount = $this->getSubscribersCount(); - expect($subscribersCount)->equals(3); - } - - function testItSynchronizeEmails() { - $id = $this->insertUser(); - WP::synchronizeUsers(); - $this->updateWPUserEmail($id, 'user-sync-test-xx@email.com'); - WP::synchronizeUsers(); - $subscriber = Subscriber::where('wp_user_id', $id)->findOne(); - expect($subscriber->email)->equals('user-sync-test-xx@email.com'); - } - - function testItSynchronizeFirstNames() { - $id = $this->insertUser(); - WP::synchronizeUsers(); - update_user_meta($id, 'first_name', 'First name'); - WP::synchronizeUsers(); - $subscriber = Subscriber::where('wp_user_id', $id)->findOne(); - expect($subscriber->first_name)->equals('First name'); - } - - function testItSynchronizeLastNames() { - $id = $this->insertUser(); - WP::synchronizeUsers(); - update_user_meta($id, 'last_name', 'Last name'); - WP::synchronizeUsers(); - $subscriber = Subscriber::where('wp_user_id', $id)->findOne(); - expect($subscriber->last_name)->equals('Last name'); - } - - function testItSynchronizeFirstNamesUsingDisplayName() { - $id = $this->insertUser(); - WP::synchronizeUsers(); - $this->updateWPUserDisplayName($id, 'First name'); - WP::synchronizeUsers(); - $subscriber = Subscriber::where('wp_user_id', $id)->findOne(); - expect($subscriber->first_name)->equals('First name'); - } - - function testItSynchronizeSegment() { - $this->insertUser(); - $this->insertUser(); - $this->insertUser(); - WP::synchronizeUsers(); - $subscribers = Segment::getWPSegment()->subscribers()->whereIn('wp_user_id', $this->userIds); - expect($subscribers->count())->equals(3); - } - - function testItRemovesUsersFromTrash() { - $id = $this->insertUser(); - WP::synchronizeUsers(); - $subscriber = Subscriber::where("wp_user_id", $id)->findOne(); - $subscriber->deleted_at = Carbon::now(); - $subscriber->save(); - WP::synchronizeUsers(); - $subscriber = Subscriber::where("wp_user_id", $id)->findOne(); - expect($subscriber->deleted_at)->null(); - } - - function testItRemovesOrphanedSubscribers() { - $this->insertUser(); - $id = $this->insertUser(); - WP::synchronizeUsers(); - $this->deleteWPUser($id); - WP::synchronizeUsers(); - $subscribers = Segment::getWPSegment()->subscribers()->whereIn('wp_user_id', $this->userIds); - expect($subscribers->count())->equals(1); - } - - function _before() { - $this->cleanData(); - } - - function _after() { - $this->cleanData(); - } - - private function cleanData() { - \ORM::raw_execute('TRUNCATE ' . Segment::$_table); - global $wpdb; - $db = \ORM::getDb(); - $db->exec(sprintf(' - DELETE FROM - %susers - WHERE - user_email LIKE "user-sync-test%%" - ', $wpdb->prefix)); - } - - private function getSubscribersCount() { - return Subscriber::whereIn("wp_user_id", $this->userIds)->count(); - } - - /** - * Insert a user without invoking wp hooks. - * Those tests are testing user synchronisation, so we need data in wp_users table which has not been synchronised to - * mailpoet database yet. We cannot use wp_insert_user functions because they would do the sync on insert. - * - * @return string - */ - private function insertUser() { - global $wpdb; - $db = \ORM::getDb(); - $db->exec(sprintf(' - INSERT INTO - %susers(user_login, user_email, user_registered) - VALUES - ( - CONCAT("user-sync-test", rand()), - CONCAT("user", rand(), "@example.com"), - "2017-01-02 12:31:12" - )', $wpdb->prefix)); - $id = $db->lastInsertId(); - $this->userIds[] = $id; - return $id; - } - - private function updateWPUserEmail($id, $email) { - global $wpdb; - $db = \ORM::getDb(); - $db->exec(sprintf(' - UPDATE - %susers - SET user_email = "%s" - WHERE - id = %s - ', $wpdb->prefix, $email, $id)); - } - - private function updateWPUserDisplayName($id, $name) { - global $wpdb; - $db = \ORM::getDb(); - $db->exec(sprintf(' - UPDATE - %susers - SET display_name = "%s" - WHERE - id = %s - ', $wpdb->prefix, $name, $id)); - } - - private function deleteWPUser($id) { - global $wpdb; - $db = \ORM::getDb(); - $db->exec(sprintf(' - DELETE FROM - %susers - WHERE - id = %s - ', $wpdb->prefix, $id)); - } - -} \ No newline at end of file diff --git a/tests/unit/Segments/WPTest.php b/tests/unit/Segments/WPTest.php index 8a45c6c2f8..da1d2d9b7f 100644 --- a/tests/unit/Segments/WPTest.php +++ b/tests/unit/Segments/WPTest.php @@ -1,55 +1,194 @@ wp_user_1 = $this->createWPUser('phoenix_test_user'); - $this->wp_user_2 = $this->createWPUser('phoenix_test_user2'); - $this->wp_segment = Segment::getWPSegment(); +use Carbon\Carbon; +use MailPoet\Models\Segment; +use MailPoet\Models\Subscriber; +use MailPoet\Segments\WP; + +class WPTest extends \MailPoetTest { + + private $userIds = array(); + + function testItSynchronizeUsers() { + $this->insertUser(); + $this->insertUser(); + WP::synchronizeUsers(); + $subscribersCount = $this->getSubscribersCount(); + expect($subscribersCount)->equals(2); + } + + function testItSynchronizeNewUsers() { + $this->insertUser(); + $this->insertUser(); + WP::synchronizeUsers(); + $this->insertUser(); + WP::synchronizeUsers(); + $subscribersCount = $this->getSubscribersCount(); + expect($subscribersCount)->equals(3); + } + + function testItSynchronizeEmails() { + $id = $this->insertUser(); + WP::synchronizeUsers(); + $this->updateWPUserEmail($id, 'user-sync-test-xx@email.com'); + WP::synchronizeUsers(); + $subscriber = Subscriber::where('wp_user_id', $id)->findOne(); + expect($subscriber->email)->equals('user-sync-test-xx@email.com'); + } + + function testItSynchronizeFirstNames() { + $id = $this->insertUser(); + WP::synchronizeUsers(); + update_user_meta($id, 'first_name', 'First name'); + WP::synchronizeUsers(); + $subscriber = Subscriber::where('wp_user_id', $id)->findOne(); + expect($subscriber->first_name)->equals('First name'); + } + + function testItSynchronizeLastNames() { + $id = $this->insertUser(); + WP::synchronizeUsers(); + update_user_meta($id, 'last_name', 'Last name'); + WP::synchronizeUsers(); + $subscriber = Subscriber::where('wp_user_id', $id)->findOne(); + expect($subscriber->last_name)->equals('Last name'); + } + + function testItSynchronizeFirstNamesUsingDisplayName() { + $id = $this->insertUser(); + WP::synchronizeUsers(); + $this->updateWPUserDisplayName($id, 'First name'); + WP::synchronizeUsers(); + $subscriber = Subscriber::where('wp_user_id', $id)->findOne(); + expect($subscriber->first_name)->equals('First name'); + } + + function testItSynchronizeSegment() { + $this->insertUser(); + $this->insertUser(); + $this->insertUser(); + WP::synchronizeUsers(); + $subscribers = Segment::getWPSegment()->subscribers()->whereIn('wp_user_id', $this->userIds); + expect($subscribers->count())->equals(3); + } + + function testItRemovesUsersFromTrash() { + $id = $this->insertUser(); + WP::synchronizeUsers(); + $subscriber = Subscriber::where("wp_user_id", $id)->findOne(); + $subscriber->deleted_at = Carbon::now(); + $subscriber->save(); + WP::synchronizeUsers(); + $subscriber = Subscriber::where("wp_user_id", $id)->findOne(); + expect($subscriber->deleted_at)->null(); } function testItSynchronizesDeletedWPUsersUsingHooks() { - expect($this->getWPSegmentSubscribers()->count())->equals(2); - wp_delete_user($this->wp_user_1->ID); - expect($this->getWPSegmentSubscribers()->count())->equals(1); - } - - function testItForciblySynchronizesDeletedWPUsers() { - global $wpdb; - expect($this->getWPSegmentSubscribers()->count())->equals(2); - // Remove a WP user directly from the database - \ORM::for_table($wpdb->prefix . 'users') - ->where('id', $this->wp_user_2->ID) - ->deleteMany(); + $id = $this->insertUser(); + $this->insertUser(); WP::synchronizeUsers(); - expect($this->getWPSegmentSubscribers()->count())->equals(1); + $subscribersCount = $this->getSubscribersCount(); + expect($subscribersCount)->equals(2); + wp_delete_user($id); + $subscribersCount = $this->getSubscribersCount(); + expect($subscribersCount)->equals(1); } - private function getWPSegmentSubscribers() { - return $this->wp_segment->subscribers() - ->whereIn( - 'wp_user_id', - array( - $this->wp_user_1->ID, - $this->wp_user_2->ID - ) - ); + function testItRemovesOrphanedSubscribers() { + $this->insertUser(); + $id = $this->insertUser(); + WP::synchronizeUsers(); + $this->deleteWPUser($id); + WP::synchronizeUsers(); + $subscribers = Segment::getWPSegment()->subscribers()->whereIn('wp_user_id', $this->userIds); + expect($subscribers->count())->equals(1); } - private function createWPUser($login) { - $WP_user = wp_create_user($login, 'pass', $login . '@example.com'); - $WP_user = get_user_by('login', $login); - return $WP_user; + function _before() { + $this->cleanData(); } function _after() { - \ORM::raw_execute('TRUNCATE ' . Segment::$_table); - wp_delete_user($this->wp_user_1->ID); - wp_delete_user($this->wp_user_2->ID); + $this->cleanData(); } -} + + private function cleanData() { + \ORM::raw_execute('TRUNCATE ' . Segment::$_table); + global $wpdb; + $db = \ORM::getDb(); + $db->exec(sprintf(' + DELETE FROM + %susers + WHERE + user_email LIKE "user-sync-test%%" + ', $wpdb->prefix)); + } + + private function getSubscribersCount() { + return Subscriber::whereIn("wp_user_id", $this->userIds)->count(); + } + + /** + * Insert a user without invoking wp hooks. + * Those tests are testing user synchronisation, so we need data in wp_users table which has not been synchronised to + * mailpoet database yet. We cannot use wp_insert_user functions because they would do the sync on insert. + * + * @return string + */ + private function insertUser() { + global $wpdb; + $db = \ORM::getDb(); + $db->exec(sprintf(' + INSERT INTO + %susers(user_login, user_email, user_registered) + VALUES + ( + CONCAT("user-sync-test", rand()), + CONCAT("user", rand(), "@example.com"), + "2017-01-02 12:31:12" + )', $wpdb->prefix)); + $id = $db->lastInsertId(); + $this->userIds[] = $id; + return $id; + } + + private function updateWPUserEmail($id, $email) { + global $wpdb; + $db = \ORM::getDb(); + $db->exec(sprintf(' + UPDATE + %susers + SET user_email = "%s" + WHERE + id = %s + ', $wpdb->prefix, $email, $id)); + } + + private function updateWPUserDisplayName($id, $name) { + global $wpdb; + $db = \ORM::getDb(); + $db->exec(sprintf(' + UPDATE + %susers + SET display_name = "%s" + WHERE + id = %s + ', $wpdb->prefix, $name, $id)); + } + + private function deleteWPUser($id) { + global $wpdb; + $db = \ORM::getDb(); + $db->exec(sprintf(' + DELETE FROM + %susers + WHERE + id = %s + ', $wpdb->prefix, $id)); + } + +} \ No newline at end of file From 6d36d67a60d04813b14091fac1b4c80b0db6dc90 Mon Sep 17 00:00:00 2001 From: Pavel Dohnal Date: Mon, 11 Sep 2017 10:09:32 +0100 Subject: [PATCH 12/12] Delete orphaned subscribers instead of updating their id [MAILPOET-1073] --- lib/Segments/WP.php | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/Segments/WP.php b/lib/Segments/WP.php index b025508d18..44152419ef 100644 --- a/lib/Segments/WP.php +++ b/lib/Segments/WP.php @@ -174,13 +174,14 @@ class WP { // remove orphaned wp segment subscribers (not having a matching wp user id), // e.g. if wp users were deleted directly from the database global $wpdb; - $subscribers_table = Subscriber::$_table; - Subscriber::raw_execute(sprintf(' - UPDATE %s as wpms - LEFT JOIN %susers as wu ON wpms.wp_user_id = wu.id - SET wp_user_id = NULL - WHERE wu.ID IS NULL; - ', $subscribers_table, $wpdb->prefix)); + + Subscriber::table_alias('wpms') + ->leftOuterJoin($wpdb->prefix . 'users', array('wpms.wp_user_id', '=', 'wu.id'), 'wu') + ->whereNull('wu.id') + ->findResultSet() + ->set('wp_user_id', null) + ->delete(); + } }