Fix duplicate assets on DB write fails scenarios - #1182
Merged
gabrielcld2 merged 1 commit intoJun 22, 2026
Conversation
…rite fails When Cloudinary returns `existing: true` (overwrite=false, asset already exists), the plugin retried with a unique suffix, creating a new Cloudinary asset. If the server killed the PHP process after the upload but before saving `_public_id`, WordPress never recorded the asset. Each subsequent autosync cycle repeated this, producing one new Cloudinary duplicate per cycle (102 in the reported case). When WordPress has no `_public_id` for an attachment, the conflicting Cloudinary asset is from a previously failed upload attempt. Overwriting it is safe and avoids creating another suffixed duplicate. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
gabrielcld2
deleted the
fix/duplicate-cloudinary-assets-on-existing-retry
branch
June 24, 2026 10:37
gabrielcld2
pushed a commit
that referenced
this pull request
Aug 27, 2026
- Compare against the same file Api::upload() actually sends, not always the attached file. For images over big_image_size_threshold, get_attached_file() returns the "-scaled" copy while the upload itself sends the unscaled original via wp_get_original_image_path(), so the two sizes never matched and the #1182 crash-recovery path silently stopped working for large images. Extracted the shared resolution into Media::get_upload_file_path(), used by both Api::upload() and the new check, instead of a third inline copy. - Confirm with the response's etag (MD5 of the stored asset) once byte sizes already match, closing the remaining false-positive where two unrelated files coincidentally share a byte count. - Log via Utils::log() when the check bails out for lack of a `bytes` field, so a future API response change doesn't silently resurrect the #1182 duplicate-per-cycle bug. - Mark is_matching_existing_asset() @internal and narrow its docblock to the sync type it actually runs for. Extends the test suite with the scaled-image and etag scenarios.
gabrielcld2
pushed a commit
that referenced
this pull request
Sep 1, 2026
- vip:// paths: skip the etag/md5 hash and rely on byte size alone. Hashing a VIP stream-wrapper path pulls the whole object over the network, and a failed read returns false rather than throwing -- which the etag check would misread as a content mismatch, reintroducing the #1182 duplicate-per-cycle bug on VIP specifically. - Media::get_upload_file_path(): corrected @return to string|false, matching the underlying get_attached_file()/wp_get_original_image_path() core functions. - Byte/etag equality alone isn't ownership: two unrelated attachments can hold byte-identical files that derive the same public ID. Added is_solely_linked_to(), mirroring the guard Delete_Sync::delete_asset() already uses, so the overwrite path now also requires that no other attachment is already tracked as linked to the public ID before treating a collision as this attachment's own orphan. Extends the test suite with the ownership-conflict, vip://, and missing-public_id scenarios (29 tests).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When Cloudinary returns
existing: true(overwrite=false, asset already exists), the plugin retried with a unique suffix, creating a new Cloudinary asset. If the server killed the PHP process after the upload but before saving_public_id, WordPress never recorded the asset. Each subsequent autosync cycle repeated this, producing one new Cloudinary duplicate per cycle (102 in the reported case).When WordPress has no
_public_idfor an attachment, the conflicting Cloudinary asset is from a previously failed upload attempt. Overwriting it is safe and avoids creating another suffixed duplicate.Approach
QA notes
Steps to reproduce
npx wp-env run cli wp eval '$m = get_post_meta(123, "_cloudinary", true); $m["_public_id"] = ""; $m["_sync_signature"] = []; update_post_meta(123, "_cloudinary", $m); echo "Done\n";'npx wp-env run cli wp eval 'Cloudinary\get_plugin_instance()->get_component("sync")->managers["push"]->process_assets(123);'npx wp-env run cli wp eval 'echo get_post_meta(123, "_cloudinary", true)["_public_id"] . "\n";'On
masterbranch: the public_id will have a suffix like _123_xxxxx, meaning the asset is duplicated within Cloudinary's backend.On this branch: the original public_id won't have a suffix, indicating that it was overwritten instead of duplicated.