From f4e1549201be9b9b09cfac80c96d6425a879dea2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 19:50:25 +0000 Subject: [PATCH 1/2] fix: bound the in-memory attachment ID cache The request-local map in Optml_Attachment_Cache kept every distinct URL looked up during a request, including misses, with no limit. It now keeps at most 2000 URLs and drops the older half when full; dropped URLs are still served from the persistent cache (transient or object cache). Refs #1139 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01MEXWtYdE6e3ALYtqtP8j2p --- inc/attachment_cache.php | 28 +++++++++-- tests/test-attachment-cache.php | 83 +++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 3 deletions(-) create mode 100644 tests/test-attachment-cache.php diff --git a/inc/attachment_cache.php b/inc/attachment_cache.php index 64fa4a623..7aaa777cf 100644 --- a/inc/attachment_cache.php +++ b/inc/attachment_cache.php @@ -5,6 +5,13 @@ */ class Optml_Attachment_Cache { const CACHE_GROUP = 'om_att'; + /** + * Most URLs kept in the in-memory map during one request. + * + * A page with many distinct image URLs would otherwise grow the map for the whole request. + * When it is full, the older half is dropped; those URLs are still in the persistent cache. + */ + const MAX_MEMORY_ENTRIES = 2000; /** * Local cache map. * @@ -32,10 +39,10 @@ public static function get_cached_attachment_id( $url ) { return self::$cache_map[ $cache_key ]; } - $value = wp_using_ext_object_cache() + $value = wp_using_ext_object_cache() ? wp_cache_get( $cache_key, self::CACHE_GROUP ) : get_transient( self::CACHE_GROUP . $cache_key ); - self::$cache_map[ $cache_key ] = $value; + self::remember( $cache_key, $value ); return $value; } @@ -51,7 +58,7 @@ public static function get_cached_attachment_id( $url ) { public static function set_cached_attachment_id( $url, $id ) { $cache_key = self::get_cache_key( $url ); // We cache also in memory to avoid calling DB every time when not using Object Cache. - self::$cache_map[ $cache_key ] = $id; + self::remember( $cache_key, $id ); // If the ID is not found we cache for 10 minutes, otherwise for a week. // We try to reduce the cache time when is not found to // avoid caching for situation when this might be temporary. @@ -61,6 +68,21 @@ public static function set_cached_attachment_id( $url, $id ) { : set_transient( self::CACHE_GROUP . $cache_key, $id, $expiration ); } + /** + * Keep a value in the in-memory map, dropping the older half when it is full. + * + * @param string $cache_key The cache key. + * @param int|bool $value The attachment ID, or false when it is not cached. + * + * @return void + */ + private static function remember( $cache_key, $value ) { + if ( ! isset( self::$cache_map[ $cache_key ] ) && count( self::$cache_map ) >= self::MAX_MEMORY_ENTRIES ) { + self::$cache_map = array_slice( self::$cache_map, (int) ( self::MAX_MEMORY_ENTRIES / 2 ), null, true ); + } + self::$cache_map[ $cache_key ] = $value; + } + /** * Generate cache key for URL. * diff --git a/tests/test-attachment-cache.php b/tests/test-attachment-cache.php new file mode 100644 index 000000000..c0d1681d5 --- /dev/null +++ b/tests/test-attachment-cache.php @@ -0,0 +1,83 @@ + + */ + private function memory_map() { + $property = new ReflectionProperty( Optml_Attachment_Cache::class, 'cache_map' ); + $property->setAccessible( true ); + + return $property->getValue(); + } + + /** + * A page with many distinct image URLs must not grow the in-memory map for the whole request. + */ + public function test_memory_map_is_bounded() { + $limit = Optml_Attachment_Cache::MAX_MEMORY_ENTRIES; + + for ( $i = 0; $i < $limit * 3; $i++ ) { + Optml_Attachment_Cache::get_cached_attachment_id( 'https://example.org/wp-content/uploads/miss-' . $i . '.jpg' ); + Optml_Attachment_Cache::set_cached_attachment_id( 'https://example.org/wp-content/uploads/hit-' . $i . '.jpg', $i + 1 ); + } + + $this->assertLessThanOrEqual( $limit, count( $this->memory_map() ) ); + } + + /** + * The newest URLs stay in memory when the older half is dropped, and the dropped ones are + * still found in the persistent cache. + */ + public function test_dropped_entries_are_still_found_in_the_persistent_cache() { + $limit = Optml_Attachment_Cache::MAX_MEMORY_ENTRIES; + + for ( $i = 0; $i <= $limit; $i++ ) { + Optml_Attachment_Cache::set_cached_attachment_id( 'https://example.org/wp-content/uploads/image-' . $i . '.jpg', $i + 1 ); + } + + $map = $this->memory_map(); + $this->assertCount( (int) ( $limit / 2 ) + 1, $map ); + $this->assertContains( $limit + 1, $map ); + $this->assertNotContains( 1, $map ); + + $this->assertSame( 1, (int) Optml_Attachment_Cache::get_cached_attachment_id( 'https://example.org/wp-content/uploads/image-0.jpg' ) ); + } + + /** + * Updating a URL that is already in memory never drops other entries. + */ + public function test_updating_a_known_url_keeps_the_map() { + $limit = Optml_Attachment_Cache::MAX_MEMORY_ENTRIES; + + for ( $i = 0; $i < $limit; $i++ ) { + Optml_Attachment_Cache::set_cached_attachment_id( 'https://example.org/wp-content/uploads/image-' . $i . '.jpg', $i + 1 ); + } + Optml_Attachment_Cache::set_cached_attachment_id( 'https://example.org/wp-content/uploads/image-0.jpg', 99 ); + + $this->assertCount( $limit, $this->memory_map() ); + $this->assertSame( 99, Optml_Attachment_Cache::get_cached_attachment_id( 'https://example.org/wp-content/uploads/image-0.jpg' ) ); + } +} From cc26c37020ad1df630f80abdfc65320ae3766862 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 20:54:19 +0000 Subject: [PATCH 2/2] Cover the cache-miss path in the memory cap test and fix the value type Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01MEXWtYdE6e3ALYtqtP8j2p --- inc/attachment_cache.php | 4 ++-- tests/test-attachment-cache.php | 11 +++++------ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/inc/attachment_cache.php b/inc/attachment_cache.php index 7aaa777cf..392caa9ca 100644 --- a/inc/attachment_cache.php +++ b/inc/attachment_cache.php @@ -71,8 +71,8 @@ public static function set_cached_attachment_id( $url, $id ) { /** * Keep a value in the in-memory map, dropping the older half when it is full. * - * @param string $cache_key The cache key. - * @param int|bool $value The attachment ID, or false when it is not cached. + * @param string $cache_key The cache key. + * @param int|numeric-string|false $value The attachment ID (a numeric string when read back from a transient), or false when it is not cached. * * @return void */ diff --git a/tests/test-attachment-cache.php b/tests/test-attachment-cache.php index c0d1681d5..67ce1f296 100644 --- a/tests/test-attachment-cache.php +++ b/tests/test-attachment-cache.php @@ -24,7 +24,7 @@ public function tearDown(): void { /** * Read the in-memory map. * - * @return array + * @return array */ private function memory_map() { $property = new ReflectionProperty( Optml_Attachment_Cache::class, 'cache_map' ); @@ -34,17 +34,16 @@ private function memory_map() { } /** - * A page with many distinct image URLs must not grow the in-memory map for the whole request. + * A page with many distinct uncached image URLs must not grow the in-memory map for the whole request. */ - public function test_memory_map_is_bounded() { + public function test_memory_map_is_bounded_on_cache_misses() { $limit = Optml_Attachment_Cache::MAX_MEMORY_ENTRIES; - for ( $i = 0; $i < $limit * 3; $i++ ) { + for ( $i = 0; $i <= $limit; $i++ ) { Optml_Attachment_Cache::get_cached_attachment_id( 'https://example.org/wp-content/uploads/miss-' . $i . '.jpg' ); - Optml_Attachment_Cache::set_cached_attachment_id( 'https://example.org/wp-content/uploads/hit-' . $i . '.jpg', $i + 1 ); } - $this->assertLessThanOrEqual( $limit, count( $this->memory_map() ) ); + $this->assertCount( (int) ( $limit / 2 ) + 1, $this->memory_map() ); } /**