fix(media): keep cached derivatives a sibling attachment still uses

CHANGELOG.md

@@ -6,6 +6,36 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
 
 ## [Unreleased]
 
+### Fixed
+
+- `StarterBase::cleanup_cached_images()` deleted resizer derivatives that other
+  attachments were still using. The cache under `wp-content/cache/image` is
+  keyed by file, but one file routinely carries several attachment rows: WPML
+  writes one per language, and a duplicate upload can be pointed at a path that
+  already exists. Deleting any one of those rows wiped the shared derivatives,
+  and the site then regenerated them — or, where the encoder was broken, served
+  a damaged image.
+
+  Measured on a five-language site: 5542 files were shared by 25981 attachment
+  rows, so roughly four fifths of the media library could take a live image
+  down with it. The homepage hero lost its derivatives while five attachment
+  rows and the rendered page still referenced them.
+
+  The delete now runs only when no other attachment points at the same
+  `_wp_attached_file`. It fails closed: where that question cannot be answered
+  the files are kept, because a stale derivative is overwritten by the next
+  resize while one deleted in error vanishes from a page still serving it.
+  "Cannot be answered" covers a missing `$wpdb`, an empty `_wp_attached_file`
+  (a filter supplied the path, so siblings have no key to match on), and a
+  failed query — `get_var()` reports an error by returning null, which casts to
+  the same zero a genuine "no siblings" answer gives, so the null and
+  `last_error` are both checked rather than read as a count.
+
+  Not fixed here, and tracked separately: two different files that share a
+  basename across upload-year folders still collide in the flat cache
+  namespace, so deleting one can remove the other's derivative. That needs a
+  cache-naming change, not a guard.
+
 ## [1.42.0] - 2026-08-26
 
 ### Added

src/StarterBase.php

@@ -3877,6 +3877,15 @@ public function cleanup_cached_images( $attachment_id ) {
 			return;
 		}
 
+		// The cache is keyed by file, not by attachment, and one file routinely
+		// carries several attachment rows: WPML writes one per language, and a
+		// duplicate upload can be pointed at a path that already exists. Those
+		// rows share one set of derivatives, so deleting one of them must leave
+		// the files alone.
+		if ( $this->attached_file_is_shared( $attachment_id ) ) {
+			return;
+		}
+
 		// Extract filename without path
 		$filename = basename( $file_path );
 		$path_info = pathinfo( $filename );
@@ -3933,6 +3942,56 @@ public function cleanup_cached_images( $attachment_id ) {
 		}
 	}
 
+	/**
+	 * Whether an attachment other than this one still points at the same file.
+	 *
+	 * Compares `_wp_attached_file` because that is the value siblings share.
+	 * `get_attached_file()` is filtered and absolute, so it is not comparable
+	 * across rows.
+	 *
+	 * Fails closed. When the question cannot be answered the caller keeps the
+	 * cached files: a stale derivative is overwritten by the next resize, while
+	 * one deleted in error vanishes from a page that is still serving it.
+	 *
+	 * @param int $attachment_id The attachment being deleted.
+	 * @return bool True when the file is shared, or when sharing cannot be determined.
+	 */
+	private function attached_file_is_shared( $attachment_id ) {
+		global $wpdb;
+
+		if ( ! $wpdb instanceof \wpdb ) {
+			return true;
+		}
+
+		$relative_path = get_post_meta( (int) $attachment_id, '_wp_attached_file', true );
+
+		// `get_attached_file()` already returned a path, so an empty meta value
+		// means a filter supplied that path (offloaded media). Siblings are keyed
+		// on the meta, so there is nothing left to compare them against.
+		if ( ! is_string( $relative_path ) || '' === $relative_path ) {
+			return true;
+		}
+
+		$shared = $wpdb->get_var(
+			$wpdb->prepare(
+				"SELECT COUNT(*) FROM %i WHERE meta_key = '_wp_attached_file' AND meta_value = %s AND post_id != %d",
+				$wpdb->postmeta,
+				$relative_path,
+				(int) $attachment_id
+			)
+		);
+
+		// A failed query answers nothing, and it does not announce itself: on
+		// error `get_var()` returns null, which casts to the same 0 a genuine
+		// "no siblings" result gives. Reading it that way would delete on any
+		// transient database error, so both signals are checked instead.
+		if ( null === $shared || '' !== $wpdb->last_error ) {
+			return true;
+		}
+
+		return (int) $shared > 0;
+	}
+
 	/**
 	 * Prevent uploading images when a file with the same basename but different extension already exists.
 	 *

tests/Unit/StarterBase/CleanupCachedImagesTest.php

@@ -0,0 +1,223 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tests\Unit\StarterBase;
+
+use Brain\Monkey\Functions;
+use Tests\Unit\StarterBaseTestCase;
+
+/**
+ * `cleanup_cached_images()` keys the resizer cache by file, not by attachment.
+ *
+ * A single file routinely carries more than one attachment row: WPML writes one
+ * per language, and a duplicate upload can be pointed at an existing path. The
+ * derivatives under wp-content/cache/image are shared by all of them, so
+ * deleting one row must not take the other rows' images with it.
+ */
+class CleanupCachedImagesTest extends StarterBaseTestCase {
+
+	private \Parisek\TimberKit\StarterBase $base;
+
+	private string $cache_dir;
+
+	/**
+	 * Paths this test created, so teardown removes its own trace and nothing
+	 * else. WP_CONTENT_DIR is a constant fixed by the bootstrap, so the cache
+	 * directory cannot be varied per run — deleting the tree wholesale would
+	 * take a concurrent run's fixtures (or a developer's scratch files) with it.
+	 *
+	 * @var list<string>
+	 */
+	private array $created = [];
+
+	protected function setUp(): void {
+		parent::setUp();
+		$this->base = $this->createStarterBase();
+
+		$this->cache_dir = WP_CONTENT_DIR . '/cache/image';
+		$this->makeDir( $this->cache_dir . '/900x0-center' );
+		$this->makeDir( $this->cache_dir . '/1439x0-center' );
+
+		$this->stubFilesystem();
+	}
+
+	protected function tearDown(): void {
+		foreach ( array_reverse( $this->created ) as $path ) {
+			if ( is_file( $path ) ) {
+				unlink( $path );
+			} elseif ( is_dir( $path ) ) {
+				@rmdir( $path ); // Only succeeds while empty, which is the intent.
+			}
+		}
+		$this->created = [];
+		unset( $GLOBALS['wpdb'], $GLOBALS['wp_filesystem'] );
+		parent::tearDown();
+	}
+
+	/** Create $path and every missing parent, recording only what did not exist. */
+	private function makeDir( string $path ): void {
+		$missing = [];
+		for ( $dir = $path; ! is_dir( $dir ); $dir = dirname( $dir ) ) {
+			$missing[] = $dir;
+		}
+		mkdir( $path, 0777, true );
+		// Deepest first, so teardown's reverse walk removes children before parents.
+		$this->created = array_merge( $this->created, array_reverse( $missing ) );
+	}
+
+	private function seedFile( string $path ): void {
+		file_put_contents( $path, 'x' );
+		$this->created[] = $path;
+	}
+
+	/** Real unlink through the WP_Filesystem seam the production code uses. */
+	private function stubFilesystem(): void {
+		Functions\when( 'WP_Filesystem' )->justReturn( true );
+		$GLOBALS['wp_filesystem'] = new class {
+			public function exists( string $path ): bool {
+				return file_exists( $path );
+			}
+
+			public function delete( string $path ): bool {
+				return unlink( $path );
+			}
+		};
+	}
+
+	/**
+	 * @param int $siblings Attachment rows OTHER than the one being deleted that
+	 *                      point at the same `_wp_attached_file` value.
+	 */
+	private function stubAttachment( int $attachment_id, string $relative_path, int|null $siblings, string $last_error = '' ): void {
+		Functions\when( 'get_attached_file' )->justReturn( '/var/www/wp-content/uploads/' . $relative_path );
+		Functions\when( 'get_post_meta' )->justReturn( $relative_path );
+
+		$GLOBALS['wpdb'] = new class( $siblings, $last_error ) extends \wpdb {
+			public string $postmeta = 'wp_postmeta';
+
+			public string $last_error = '';
+
+			/** @var list<mixed> Arguments the production code passed to prepare(). */
+			public array $prepare_args = [];
+
+			public string $prepared_query = '';
+
+			public function __construct( private readonly int|null $siblings, string $last_error ) {
+				$this->last_error = $last_error;
+			}
+
+			public function prepare( string $query, mixed ...$args ): string {
+				$this->prepared_query = $query;
+				$this->prepare_args = $args;
+				return $query;
+			}
+
+			public function get_var( string $query ): string|null {
+				return null === $this->siblings ? null : (string) $this->siblings;
+			}
+		};
+	}
+
+	private function seedDerivatives(): void {
+		$this->seedFile( $this->cache_dir . '/900x0-center/homepage-hero-desktop.avif' );
+		$this->seedFile( $this->cache_dir . '/1439x0-center/homepage-hero-desktop.avif' );
+		$this->seedFile( $this->cache_dir . '/900x0-center/unrelated.avif' );
+	}
+
+	public function test_deletes_derivatives_when_no_other_attachment_shares_the_file(): void {
+		$this->seedDerivatives();
+		$this->stubAttachment( 59741, '2026/08/homepage-hero-desktop.webp', siblings: 0 );
+
+		$this->base->cleanup_cached_images( 59741 );
+
+		$this->assertFileDoesNotExist( $this->cache_dir . '/900x0-center/homepage-hero-desktop.avif' );
+		$this->assertFileDoesNotExist( $this->cache_dir . '/1439x0-center/homepage-hero-desktop.avif' );
+		$this->assertFileExists(
+			$this->cache_dir . '/900x0-center/unrelated.avif',
+			'a different basename must never be touched'
+		);
+	}
+
+	public function test_keeps_derivatives_when_another_attachment_shares_the_file(): void {
+		$this->seedDerivatives();
+		// The WPML case: five rows, one file. Deleting one language's row leaves
+		// four rows — and the live page — still rendering these derivatives.
+		$this->stubAttachment( 59741, '2026/08/homepage-hero-desktop.webp', siblings: 4 );
+
+		$this->base->cleanup_cached_images( 59741 );
+
+		$this->assertFileExists( $this->cache_dir . '/900x0-center/homepage-hero-desktop.avif' );
+		$this->assertFileExists( $this->cache_dir . '/1439x0-center/homepage-hero-desktop.avif' );
+	}
+
+	/**
+	 * Without a database the sibling question cannot be answered. Skipping the
+	 * delete leaves a stale file that the next resize overwrites; guessing wrong
+	 * in the other direction destroys an image that is still on the page.
+	 */
+	/**
+	 * A failed query answers nothing. `get_var()` returns null on error, and the
+	 * naive `(int) null > 0` reading of that is indistinguishable from a real
+	 * zero — which would delete the files on any transient database error.
+	 */
+	public function test_keeps_derivatives_when_the_query_fails(): void {
+		$this->seedDerivatives();
+		$this->stubAttachment( 59741, '2026/08/homepage-hero-desktop.webp', siblings: null );
+
+		$this->base->cleanup_cached_images( 59741 );
+
+		$this->assertFileExists( $this->cache_dir . '/900x0-center/homepage-hero-desktop.avif' );
+	}
+
+	public function test_keeps_derivatives_when_the_query_reported_an_error(): void {
+		$this->seedDerivatives();
+		$this->stubAttachment( 59741, '2026/08/homepage-hero-desktop.webp', siblings: 0, last_error: 'MySQL server has gone away' );
+
+		$this->base->cleanup_cached_images( 59741 );
+
+		$this->assertFileExists( $this->cache_dir . '/900x0-center/homepage-hero-desktop.avif' );
+	}
+
+	/**
+	 * An empty `_wp_attached_file` with a non-empty `get_attached_file()` means a
+	 * filter supplied the path (offloaded media). Sharing is then unanswerable.
+	 */
+	public function test_keeps_derivatives_when_the_attached_file_meta_is_empty(): void {
+		$this->seedDerivatives();
+		$this->stubAttachment( 59741, '2026/08/homepage-hero-desktop.webp', siblings: 0 );
+		Functions\when( 'get_post_meta' )->justReturn( '' );
+
+		$this->base->cleanup_cached_images( 59741 );
+
+		$this->assertFileExists( $this->cache_dir . '/900x0-center/homepage-hero-desktop.avif' );
+	}
+
+	/** The stub returns a count whatever the SQL says, so pin the SQL itself. */
+	public function test_asks_the_database_the_question_it_claims_to_ask(): void {
+		$this->seedDerivatives();
+		$this->stubAttachment( 59741, '2026/08/homepage-hero-desktop.webp', siblings: 0 );
+
+		$this->base->cleanup_cached_images( 59741 );
+
+		$wpdb = $GLOBALS['wpdb'];
+		$this->assertStringContainsString( "meta_key = '_wp_attached_file'", $wpdb->prepared_query );
+		$this->assertStringContainsString( 'post_id != %d', $wpdb->prepared_query );
+		$this->assertSame(
+			[ 'wp_postmeta', '2026/08/homepage-hero-desktop.webp', 59741 ],
+			$wpdb->prepare_args,
+			'the table, the shared path and the excluded row must all reach prepare()'
+		);
+	}
+
+	public function test_keeps_derivatives_when_the_sibling_count_is_unavailable(): void {
+		$this->seedDerivatives();
+		Functions\when( 'get_attached_file' )->justReturn( '/var/www/wp-content/uploads/2026/08/homepage-hero-desktop.webp' );
+		Functions\when( 'get_post_meta' )->justReturn( '2026/08/homepage-hero-desktop.webp' );
+		unset( $GLOBALS['wpdb'] );
+
+		$this->base->cleanup_cached_images( 59741 );
+
+		$this->assertFileExists( $this->cache_dir . '/900x0-center/homepage-hero-desktop.avif' );
+	}
+}