Jetpack Sync: Only send a single updated_post_meta action per attachment metadata request.

[coder-karen]

Fixes SYNC-166

Prevent updated_post_meta from running multiple times in a single request if the meta key is _wp_attachment_metadata and ensure we send only the final attachment data.

Proposed changes:

• In this PR we are fetching 'fresh' data at send time, to ensure that WPcom receives the latest state of the attachment metadata.
• We are doing this because the update_post_meta action is firing multiple times during one media update (in the case of _wp_attachment_metadata, as it is written incrementally during processing of the attachment), but we know the final order state will be reliable to fetch at send time.
  We are only doing this for the _wp_attachment_metadata post meta at this stage. This could be expanded to include other particularly noisy metas but could be costly if expanded to all (lots of unnecessary DB reading for example).
• For more info on the (new) logic, see the comment at Jetpack Sync: Only send a single updated_post_meta action per attachment metadata request. #45921 (comment)

Other information:

• Have you written new tests for your changes, if applicable?
• Have you checked the E2E test CI results, and verified that your changes do not break them?
• Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

SYNC-166

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

No.

Testing instructions:

To test this out, on a self-hosted site connected to Jetpack, or WoA site, with the Jetpack beta tester plugin installed and this branch applied

• Visit the media library at /wp-admin/upload.php, and upload a media item to the library.
• Check the Jetpack audit logs for your site (eg. 571a247b6623a0aa62165b5e4e2c7b10-logstash - but change the filtered URL), and you should only see one updated_post_meta for the upload with the name wp_attachment_metadata (another should exist too but it is for the _wp_attached_file meta key).
• If you can access the postmeta table in the DB for your site, inspect he wp_attachment_metadata calue for the newly uploaded image.
• Sandbox and visit wpsh, running gpr select meta_value from wp_YOURBLOGID_postmeta where post_id=YOURMEDIAID and meta_key='_wp_attachment_metadata'. You may need to run cl 3000` first so that the serialized value is not truncated.
• Compare that with the value in the database - they should be identical.

Other tests to confirm no issues:

• Upload a pdf and compare the values in the remote vs cache site - should be the same.
• Try uploading images in bulk, for example four at once. The values should exist for all four and match the remote site values.
• Crop an image and save the edits, ensuring the remote and cache sites have the same meta value
• Test hooking into wp_get_attachment_metadata in a code snippet, the new info should be there on the remote site and cache site. Here's a snippet you can add via a code snippets plugin (and to run it, try cropping an existing image): 3999d-pb

[github-actions]

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

• To test on WoA, go to the Plugins menu on a WoA dev site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin (Jetpack or WordPress.com Site Helper), and enable the update/sync-class-posts-attachment-metadata branch.
• To test on Simple, run the following command on your sandbox:

bin/jetpack-downloader test jetpack update/sync-class-posts-attachment-metadata

bin/jetpack-downloader test jetpack-mu-wpcom-plugin update/sync-class-posts-attachment-metadata

Interested in more tips and information?

• In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
• Read more about our development workflow here: PCYsg-eg0-p2
• Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

[github-actions]

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

• ✅ Include a description of your PR changes.
  
• ✅ Add a "[Status]" label (In Progress, Needs Review, ...).
  
• ✅ Add a "[Type]" label (Bug, Enhancement, Janitorial, Task).
  
• ✅ Add testing instructions.
  
• ✅ Specify whether this PR includes any changes to data or privacy.
  
• ✅ Add changelog entries to affected projects
  

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

---

Follow this PR Review Process:

1. Ensure all required checks appearing at the bottom of this PR are passing.
2. Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
3. You can use GitHub's Reviewers functionality to request a review.
4. When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!

[jp-launch-control]

Code Coverage Summary

Coverage changed in 1 file.

File	Coverage	Δ%	Δ Uncovered
projects/packages/sync/src/modules/class-posts.php	19/286 (6.64%)	-0.53%	34 💔

Full summary · PHP report · JS report

_{If appropriate, add one of these labels to override the failing coverage check: Covered by non-unit tests Use to ignore the Code coverage requirement check when E2Es or other non-unit tests cover the code Coverage tests to be added later Use to ignore the Code coverage requirement check when tests will be added in a follow-up PR I don't care about code coverage for this PR Use this label to ignore the check for insufficient code coveage.}

[fgiannar]

Nice work, Karen!

I wonder if we can do even better here and also ensure we don't add those meta to Sync's queue in the first place.

The logic could be similar to #45697, aka:

1. Only add a single updated_post_meta actions to Sync's queue per post_id per request. We can add this logic in filter_meta
2. ensure we fetch the fresh meta before sending via wp_get_attachment_metadata in filter_updated_post_meta_before_send. Pretty much what you already do but without the static logic in drop_stale_attachment_metadata

What do you think?

[coder-karen]

Thanks for looking at this!

I couldn't follow exactly the same approach as with the mentioned PR due to filter interdependencies (eg. with added_post_meta), but after looking at it again I have come up with a different solution.

New changes:

• At enqueue time, dedupe only updated_post_meta per post_id per request.
• At send-time, fetch fresh _wp_attachment_metadata from the DB right before sending, via filter_updated_post_meta_before_send.

I've not made changes in filter_meta, as this is used by both updated, added and deleted actions, and whitelisting logic. I wanted to target only updated_post_meta.

I have added send-time changes for added_post_meta though, as this can fire first with incomplete metadata during image processing - because our new enqueue-time dedupe changed the event dynamics.

Before, we only refreshed updated_post_meta at send-time. added_post_meta passed through unchanged. Because updated_post_meta later overwrote the state, you don't notice the added_post_meta incompleteness.

Now, we dedupe updated_post_meta at enqueue time. That means added_post_meta can be the only or the first carrier in the request. If we don’t refresh that at send-time, the cache can store an early/unfiltered snapshot, and then updated (if it still fires) can add a second final snapshot, resulting in two images stored for the same id on the cache site.

The fix is to refresh added_post_meta at send-time and skip it if clearly incomplete. So: if only added_post_meta fires, we still send a correct, filtered value. If both fire, we don’t send the early incomplete added_post_meta; updated sends the final snapshot.

As for the duplication between the filters, theres no helper for the duplicated logic because sharing a single helper caused timing regressions. Keeping the wrappers separate isolates per-hook behavior.

[fgiannar]

Looks and tests well 👍