Don't sync set_object_terms actions that don't update term relationships

[mdbitz]

@jmdodd created a fabulous filter to exclude set_object_terms that were not performing updates.
This patch ensures wp_set_object_terms calls with the same term relations already set on the object will no longer trigger set_object_terms actions to be synced.

Fixes https://wp.me/p9o2xV-12g

Changes proposed in this Pull Request:

• set_object_terms actions where the old and new relationships match will not be synced.

Jetpack product discussion

N/A

Does this pull request change what data or activity we track or use?

NO

Testing instructions:

• Call wp_set_object_terms or wp_set_post_terms on an object/post to add relationships
• Verify Sync actions are queued/sent
• repeat the same call from step Use class_exists() guard for Featured_Content. #1
• Verify no set_object_terms action is queued/sent.

** You can see the unit test for examples.

Proposed changelog entry for your changes:

• Jetpack Sync : Performance update to minimize set_object_terms actions triggered.

[jetpackbot]

Scheduled Jetpack release: October 6, 2020.
Scheduled code freeze: September 29, 2020

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-17171

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Generated by 🚫 dangerJS against cc78c9c

[mdbitz]

E2E Tests were plan related which are not touched in this PR. Merged master to have them re-run. Needs Crew Review.

[leogermani]

Hi @mdbitz, I gave it a go but I need a hand on how to test sync. I'm gonna ask it here because it might be helpful for more people (cc @bisko )

Verify Sync actions are queued/sent

Here's how I was doing it:

• Opened wp shell
• Created the term and called wp_set_post_terms
• Looked at the queue by querying the database: "SELECT * FROM $wpdb->options WHERE option_name like 'jpsq_%' "
• I was able to see the action to create the term and to set the term created.
• I manually deleted the last entry in the queue from the options table
• called wp_set_post_terms again
• checked the queue and verified no action was queued. At this point, I thought things were working as expected.

But then I checked ou master to check the old behavior, and confirm that the action would be queued.

So I followed the same steps again and no action was queued at all. And then all of a sudden I couldn't test it anymore because no actions were being queued. Even if I created a new post, new term, and everything...

What's the best way to test it?

(sorry for the long comment and holding this, but this will be helpful in future PRs - and I volunteer to expand the Field Guide page on this)

[mdbitz]

Hi Leo, you shouldn't have to delete any entries from the queue (options table).

When testing sync I recommend that you always have the test site's jetpack connection sandboxed to your WP.com sandbox. https://wp.me/PCYsg-efV . Defining the JETPACK__SANDBOX_DOMAIN constant. This will have all sync activity go to your WP.com sandbox.

From this point there are quite a few ways to track Sync Activity.

1. You can use Unagi (https://wp.me/PCYsg-lgg) note I don't use this method but it is available and others enjoy it.
2. On your sandbox you can add the following hook to monitor actions 0-sandbox.php. Simply comment out what arguments you want to see in your logs.

add_action( 
	'jetpack_sync_remote_action', 
	function( $trigger, $args, $user_id, $silent, $timestamp, $sent_timestamp, $queue_id, $token, $actor, $queue_size ) 
	{
		//error_log( "-- -- --" );
		//error_log( "Trigger: " . $trigger );
		//error_log( "Sync args: \n" . print_r( $args, true ) );
		//error_log( "User ID: " . $user_id );
		//error_log( "Silent: " . $silent );
		//error_log( "Timestamp: " . $timestamp );
		//error_log( "Sent Timestamp: " . $sent_timestamp );
		//error_log( "Queue ID: " . $queue_id );
		//error_log( "jetpack_sync_remote_action Queue Size: " . $queue_size );
		// error_log( "Token: \n" . print_r( $token, true ) );<br>
		// error_log( "Actor: \n" . print_r( $actor, true ) );<br>
 }, 10, 10 );

3. Use Kibana to view sync activity by looking at the index jetpack-audit-* you can filter by your blog and the post_id to see that the event is only triggered once.

If you do #3 you don't even need to sandbox the site as that is recorded for every request across all servers.

--

If you aren't seeing any items in the options table than most likely they were sent already to WP.com and processed. Sync runs on shutdown so any requests to modify content/view content via web requests will trigger the data being sent.

[leogermani]

Thanks for the info. I made some progress but still can't confirm the original issue on master

On the environment

For some reason, setting define( 'JETPACK__SANDBOX_DOMAIN', 'MY_SLUG.wordpress.com' ); does not work for me. It only works if I edit /etc/hosts inside my docker container and sandbox jetpack.wordpress.com.

Testing

• running on master
• Open a browser window on wp admin dashboard
• Open wp shell
• added filter to 0-sandbox.php and tail -f errors...
• $term = wp_insert_term('testing', 'category');
• refresh browser window
• Check the log appearing on the sandbox: Trigger: jetpack_sync_add_term
• wp_set_post_terms( 1, 8, 'category', false ); (where 8 is the id of the created term)
• refresh browser window
• Check the log appearing on the sandbox: Trigger: set_object_terms
• wp_set_post_terms( 1, 8, 'category', false ); again.
• refresh browser window
• Nothing happens on the sandbox

The sync is only triggered in my sandbox if a new term is created or if the terms attached to a post change.

I also tried using wp_set_post_terms( 1, array( 8 ), 'category', false ); (passing the term id in an array) and even adding more than one term to a post, always with the same result...

But I did try to run the new test on the master branch and confirmed it fails. So I´m probably doing something wrong...

[mdbitz]

Ran some tests on my local test site and the reason for unable to duplicate appears to be that the is_staging_site flag is returning true in cli. This stops actions from going to the sync queue and hence not seeing the duplicate actions.

You can place the wp_set_post_terms in your functions.php or similar file so that it triggers on every request. Then refresh the site a few times and you'll see the same event being queued and sent to WP.com

__

The reason the tests don't have this issue is that we have a specific case to return true in is_sync_enabled for unit tests.

[leogermani]

I was finally able to test it.

The issue was that the Sync server, on WPCOM side, was discarding the requests because it was identifying them as duplicates since they were identical.

So I had to add some additional logging to be able to confirm that, with this branch checked out, no requests were being sent at all.