From 6d050108c20efe3d26b75cfa81ed51364ecd9bd0 Mon Sep 17 00:00:00 2001 From: selul Date: Thu, 4 Dec 2025 14:10:43 +0200 Subject: [PATCH 1/2] Enhance image URL handling when the image is from DAM inside the editor - Added logic to extract the original URL from Optimole URLs in the `get_optimized_image_url` method. - Moved the `is_dam_url` method to the `Optml_Dam_Offload_Utils` trait for better organization. - Updated the `Normalizer` trait to handle specific URL patterns. - Introduced new tests for `get_unoptimized_url` to cover various scenarios, including non-Optimole URLs and edge cases. - Prevented cache clearing actions during tests to avoid errors related to cache compatibility. https://secure.helpscout.net/conversation/3142052654/476408?viewId=2353196 --- inc/app_replacer.php | 3 + inc/traits/dam_offload_utils.php | 86 +++++++++++++---- inc/traits/normalizer.php | 4 + inc/url_replacer.php | 10 -- tests/bootstrap.php | 5 + tests/test-generic.php | 153 +++++++++++++++++++++++++++++++ 6 files changed, 231 insertions(+), 30 deletions(-) diff --git a/inc/app_replacer.php b/inc/app_replacer.php index a76d76546..48be9566e 100644 --- a/inc/app_replacer.php +++ b/inc/app_replacer.php @@ -664,6 +664,9 @@ public function url_has_dam_flag( $url ) { protected function get_optimized_image_url( $url, $width, $height, $resize = [] ) { $width = is_int( $width ) ? $width : 'auto'; $height = is_int( $height ) ? $height : 'auto'; + // If the image is already using Optimole URL, we extract the source to rebuild it. + $url = $this->get_unoptimized_url( $url ); + $optimized_image = Optimole::image( $url, $this->settings->get( 'cache_buster' ) ) ->width( $width ) ->height( $height ); diff --git a/inc/traits/dam_offload_utils.php b/inc/traits/dam_offload_utils.php index 08509975f..ec9e67641 100644 --- a/inc/traits/dam_offload_utils.php +++ b/inc/traits/dam_offload_utils.php @@ -3,6 +3,16 @@ trait Optml_Dam_Offload_Utils { use Optml_Normalizer; + /** + * Check if this contains the DAM flag. + * + * @param string $url The URL to check. + * + * @return bool + */ + private function is_dam_url( $url ) { + return strpos( $url, Optml_Dam::URL_DAM_FLAG ) !== false; + } /** * Checks that the attachment is a DAM image. * @@ -239,6 +249,31 @@ private function is_completed_offload( $id ) { return false; } + /** + * Get the attachment ID from optimole ID. + * + * @param string $optimole_id The optimole ID. + * + * @return int + */ + private function get_attachement_id_from_optimole_id( string $optimole_id ): int { + global $wpdb; + + $id = $wpdb->get_var( + $wpdb->prepare( + " + SELECT post_id + FROM {$wpdb->postmeta} + WHERE meta_key = %s + AND meta_value = %s + LIMIT 1 + ", + Optml_Dam::OM_DAM_IMPORTED_FLAG, + $optimole_id + ) + ); + return empty( $id ) ? 0 : (int) $id; + } /** * Get the attachment ID from URL. * @@ -253,32 +288,43 @@ private function attachment_url_to_post_id( $input_url ) { return (int) $cached; } - $url = $this->strip_image_size( $input_url ); + if ( $this->is_dam_url( $input_url ) ) { + // The DAM are stored as attachments of format /id:/ + $pattern = '#/' . Optml_Media_Offload::KEYS['uploaded_flag'] . '([^/]+)#'; + if ( preg_match( $pattern, $input_url, $m ) ) { + $attachment_id = $this->get_attachement_id_from_optimole_id( $m[1] ); + } else { + $attachment_id = 0; + } + } else { - $attachment_id = attachment_url_to_postid( $url ); + $url = $this->strip_image_size( $input_url ); - if ( $attachment_id === 0 && ! $this->is_scaled_url( $url ) ) { - $scaled_url = $this->get_scaled_url( $url ); + $attachment_id = attachment_url_to_postid( $url ); - $attachment_id = attachment_url_to_postid( $scaled_url ); - } + if ( $attachment_id === 0 && ! $this->is_scaled_url( $url ) ) { + $scaled_url = $this->get_scaled_url( $url ); - /* - * TODO: The logic is a mess, we need to refactor at some point. - * Websites may transition between 'www' subdomains and apex domains, potentially breaking references to hosted images. This can cause issues when attempting to match attachment IDs if images are linked using outdated domains. The logic is checking for alternative domains and consider the use of 'scaled' prefixes in image URLs for large images, which might affect ID matching. - */ - if ( $attachment_id === 0 ) { - if ( strpos( $url, 'www.' ) !== false ) { - $variant_url = str_replace( 'www.', '', $url ); - $attachment_id = attachment_url_to_postid( $variant_url ); - } else { - $variant_url = str_replace( '://', '://www.', $url ); - $attachment_id = attachment_url_to_postid( $variant_url ); + $attachment_id = attachment_url_to_postid( $scaled_url ); } - if ( $attachment_id === 0 && ! $this->is_scaled_url( $variant_url ) ) { - $scaled_url = $this->get_scaled_url( $variant_url ); - $attachment_id = attachment_url_to_postid( $scaled_url ); + /* + * TODO: The logic is a mess, we need to refactor at some point. + * Websites may transition between 'www' subdomains and apex domains, potentially breaking references to hosted images. This can cause issues when attempting to match attachment IDs if images are linked using outdated domains. The logic is checking for alternative domains and consider the use of 'scaled' prefixes in image URLs for large images, which might affect ID matching. + */ + if ( $attachment_id === 0 ) { + if ( strpos( $url, 'www.' ) !== false ) { + $variant_url = str_replace( 'www.', '', $url ); + $attachment_id = attachment_url_to_postid( $variant_url ); + } else { + $variant_url = str_replace( '://', '://www.', $url ); + $attachment_id = attachment_url_to_postid( $variant_url ); + } + if ( $attachment_id === 0 && ! $this->is_scaled_url( $variant_url ) ) { + $scaled_url = $this->get_scaled_url( $variant_url ); + + $attachment_id = attachment_url_to_postid( $scaled_url ); + } } } Optml_Attachment_Cache::set_cached_attachment_id( $input_url, $attachment_id ); diff --git a/inc/traits/normalizer.php b/inc/traits/normalizer.php index e1bd15226..a0c2f7134 100644 --- a/inc/traits/normalizer.php +++ b/inc/traits/normalizer.php @@ -53,6 +53,10 @@ public function get_unoptimized_url( $url ) { } // If the url is an uploaded image, return the url if ( Optml_Media_Offload::is_uploaded_image( $url ) ) { + $pattern = '#/id:([^/]+)/((?:https?|http)://\S+)#'; + if ( preg_match( $pattern, $url, $matches ) ) { + $url = $matches[0]; + } return $url; } diff --git a/inc/url_replacer.php b/inc/url_replacer.php index f6eca2a4b..1540f90c3 100644 --- a/inc/url_replacer.php +++ b/inc/url_replacer.php @@ -409,16 +409,6 @@ private function get_dam_url( Image $image ) { return $url; } - /** - * Check if this contains the DAM flag. - * - * @param string $url The URL to check. - * - * @return bool - */ - private function is_dam_url( $url ) { - return strpos( $url, Optml_Dam::URL_DAM_FLAG ) !== false; - } /** * Check if the URL is offloaded. diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 3e4a50c03..ea7784771 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -66,6 +66,10 @@ function( $message ) { } } require dirname( dirname( __FILE__ ) ) . '/optimole-wp.php'; + + // Prevent cache clearing actions during tests to avoid errors from cache compatibility classes + // This filter prevents optml_settings_updated and optml_clear_cache from being triggered + add_filter( 'optml_dont_trigger_settings_updated', '__return_true' ); } tests_add_filter( 'muplugins_loaded', '_manually_load_plugin' ); @@ -83,6 +87,7 @@ function( $message ) { // Activate Optimole plugin activate_plugin( 'optimole-wp/optimole-wp.php' ); + // Set up the current logged in user global $current_user; diff --git a/tests/test-generic.php b/tests/test-generic.php index 538fcc59a..fef9a2a91 100644 --- a/tests/test-generic.php +++ b/tests/test-generic.php @@ -44,4 +44,157 @@ function test_domain_hash() { $this->assertEquals( $this->to_domain_hash("//www.domain.com/"), $value ); $this->assertNotEquals( $this->to_domain_hash("https://something.com/"), $value ); } + + /** + * Test get_unoptimized_url with non-Optimole URLs. + */ + function test_get_unoptimized_url_non_optimole() { + // Initialize config for testing + $settings = new Optml_Settings(); + $settings->update( 'service_data', [ + 'cdn_key' => 'test123', + 'cdn_secret' => '12345', + 'whitelist' => [ 'example.com', 'example.org' ], + ] ); + Optml_Config::init( [ + 'key' => 'test123', + 'secret' => '12345', + ] ); + + // Non-Optimole URLs should be returned as-is + $url = 'http://example.org/wp-content/uploads/image.jpg'; + $this->assertEquals( $url, $this->get_unoptimized_url( $url ) ); + + $url = 'https://example.com/image.png'; + $this->assertEquals( $url, $this->get_unoptimized_url( $url ) ); + + $url = '/wp-content/uploads/image.jpg'; + $this->assertEquals( $url, $this->get_unoptimized_url( $url ) ); + } + + /** + * Test get_unoptimized_url with Optimole URLs (regular images). + */ + function test_get_unoptimized_url_optimole_regular() { + // Initialize config for testing + $settings = new Optml_Settings(); + $settings->update( 'service_data', [ + 'cdn_key' => 'test123', + 'cdn_secret' => '12345', + 'whitelist' => [ 'example.com', 'example.org' ], + ] ); + Optml_Config::init( [ + 'key' => 'test123', + 'secret' => '12345', + ] ); + + // Optimole URL with regular image - should extract original URL after second 'http' + $optimole_url = 'https://test123.i.optimole.com/cb:test/w:800/h:600/q:mauto/http://example.org/wp-content/uploads/image.jpg'; + $expected = 'http://example.org/wp-content/uploads/image.jpg'; + $this->assertEquals( $expected, $this->get_unoptimized_url( $optimole_url ) ); + + // Optimole URL with https in original + $optimole_url = 'https://test123.i.optimole.com/cb:test/w:800/h:600/q:mauto/https://example.org/wp-content/uploads/image.jpg'; + $expected = 'https://example.org/wp-content/uploads/image.jpg'; + $this->assertEquals( $expected, $this->get_unoptimized_url( $optimole_url ) ); + + // Optimole URL with query parameters + $optimole_url = 'https://test123.i.optimole.com/cb:test/w:800/h:600/q:mauto/http://example.org/wp-content/uploads/image.jpg?param=value'; + $expected = 'http://example.org/wp-content/uploads/image.jpg?param=value'; + $this->assertEquals( $expected, $this->get_unoptimized_url( $optimole_url ) ); + } + + /** + * Test get_unoptimized_url with uploaded images (offloaded images). + */ + function test_get_unoptimized_url_uploaded_image() { + // Initialize config for testing + $settings = new Optml_Settings(); + $settings->update( 'service_data', [ + 'cdn_key' => 'test123', + 'cdn_secret' => '12345', + 'whitelist' => [ 'example.com', 'example.org' ], + ] ); + Optml_Config::init( [ + 'key' => 'test123', + 'secret' => '12345', + ] ); + + // Uploaded image URL with /id: pattern - should extract original URL + $uploaded_url = 'https://test123.i.optimole.com/cb:test/w:800/h:600/q:mauto/id:abc123/http://example.org/wp-content/uploads/image.jpg'; + $expected = '/id:abc123/http://example.org/wp-content/uploads/image.jpg'; + $result = $this->get_unoptimized_url( $uploaded_url ); + $this->assertStringContainsString( '/id:abc123/', $result ); + $this->assertStringContainsString( 'http://example.org/wp-content/uploads/image.jpg', $result ); + + // Uploaded image URL with https in original + $uploaded_url = 'https://test123.i.optimole.com/cb:test/w:800/h:600/q:mauto/id:xyz789/https://example.org/wp-content/uploads/image.jpg'; + $expected = '/id:xyz789/https://example.org/wp-content/uploads/image.jpg'; + $result = $this->get_unoptimized_url( $uploaded_url ); + $this->assertStringContainsString( '/id:xyz789/', $result ); + $this->assertStringContainsString( 'https://example.org/wp-content/uploads/image.jpg', $result ); + } + + /** + * Test get_unoptimized_url edge cases. + */ + function test_get_unoptimized_url_edge_cases() { + // Initialize config for testing + $settings = new Optml_Settings(); + $settings->update( 'service_data', [ + 'cdn_key' => 'test123', + 'cdn_secret' => '12345', + 'whitelist' => [ 'example.com', 'example.org' ], + ] ); + Optml_Config::init( [ + 'key' => 'test123', + 'secret' => '12345', + ] ); + + // Optimole URL without second 'http' - should return as-is + $optimole_url = 'https://test123.i.optimole.com/cb:test/w:800/h:600/q:mauto'; + $this->assertEquals( $optimole_url, $this->get_unoptimized_url( $optimole_url ) ); + + // Empty string + $this->assertEquals( '', $this->get_unoptimized_url( '' ) ); + + // URL with only one 'http' occurrence + $optimole_url = 'https://test123.i.optimole.com/cb:test/w:800/h:600/q:mauto/image.jpg'; + $this->assertEquals( $optimole_url, $this->get_unoptimized_url( $optimole_url ) ); + + // Uploaded image URL without matching pattern - should return as-is + $uploaded_url = 'https://test123.i.optimole.com/cb:test/w:800/h:600/q:mauto/id:abc123/image.jpg'; + $result = $this->get_unoptimized_url( $uploaded_url ); + // Should return the URL as-is since pattern doesn't match + $this->assertEquals( $uploaded_url, $result ); + } + + /** + * Test get_unoptimized_url with custom domain configuration. + */ + function test_get_unoptimized_url_custom_domain() { + // Initialize config with custom domain + $settings = new Optml_Settings(); + $settings->update( 'service_data', [ + 'cdn_key' => 'test123', + 'cdn_secret' => '12345', + 'whitelist' => [ 'example.com', 'example.org' ], + 'domain' => 'cdn.example.com', + 'is_cname_assigned' => 'yes', + ] ); + Optml_Config::init( [ + 'key' => 'test123', + 'secret' => '12345', + 'domain' => 'cdn.example.com', + ] ); + + // Custom domain Optimole URL + $optimole_url = 'https://cdn.example.com/cb:test/w:800/h:600/q:mauto/http://example.org/wp-content/uploads/image.jpg'; + $expected = 'http://example.org/wp-content/uploads/image.jpg'; + $this->assertEquals( $expected, $this->get_unoptimized_url( $optimole_url ) ); + + // Non-Optimole URL should still return as-is + $url = 'http://example.org/wp-content/uploads/image.jpg'; + $this->assertEquals( $url, $this->get_unoptimized_url( $url ) ); + } } From 02ce6f2d18f7cafd0e5be4841d93a789fb370820 Mon Sep 17 00:00:00 2001 From: selul Date: Thu, 4 Dec 2025 21:07:09 +0200 Subject: [PATCH 2/2] Refactor attachment ID retrieval logic in DAM URL handling --- inc/traits/dam_offload_utils.php | 54 +++++++++++++++++--------------- 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/inc/traits/dam_offload_utils.php b/inc/traits/dam_offload_utils.php index ec9e67641..4f4553630 100644 --- a/inc/traits/dam_offload_utils.php +++ b/inc/traits/dam_offload_utils.php @@ -288,43 +288,45 @@ private function attachment_url_to_post_id( $input_url ) { return (int) $cached; } - if ( $this->is_dam_url( $input_url ) ) { + if ( Optml_Media_Offload::is_uploaded_image( $input_url ) ) { // The DAM are stored as attachments of format /id:/ $pattern = '#/' . Optml_Media_Offload::KEYS['uploaded_flag'] . '([^/]+)#'; if ( preg_match( $pattern, $input_url, $m ) ) { $attachment_id = $this->get_attachement_id_from_optimole_id( $m[1] ); - } else { - $attachment_id = 0; + if ( $attachment_id !== 0 ) { + Optml_Attachment_Cache::set_cached_attachment_id( $input_url, $attachment_id ); + + return $attachment_id; + } } - } else { + } - $url = $this->strip_image_size( $input_url ); + $url = $this->strip_image_size( $input_url ); - $attachment_id = attachment_url_to_postid( $url ); + $attachment_id = attachment_url_to_postid( $url ); - if ( $attachment_id === 0 && ! $this->is_scaled_url( $url ) ) { - $scaled_url = $this->get_scaled_url( $url ); + if ( $attachment_id === 0 && ! $this->is_scaled_url( $url ) ) { + $scaled_url = $this->get_scaled_url( $url ); - $attachment_id = attachment_url_to_postid( $scaled_url ); - } + $attachment_id = attachment_url_to_postid( $scaled_url ); + } - /* - * TODO: The logic is a mess, we need to refactor at some point. - * Websites may transition between 'www' subdomains and apex domains, potentially breaking references to hosted images. This can cause issues when attempting to match attachment IDs if images are linked using outdated domains. The logic is checking for alternative domains and consider the use of 'scaled' prefixes in image URLs for large images, which might affect ID matching. - */ - if ( $attachment_id === 0 ) { - if ( strpos( $url, 'www.' ) !== false ) { - $variant_url = str_replace( 'www.', '', $url ); - $attachment_id = attachment_url_to_postid( $variant_url ); - } else { - $variant_url = str_replace( '://', '://www.', $url ); - $attachment_id = attachment_url_to_postid( $variant_url ); - } - if ( $attachment_id === 0 && ! $this->is_scaled_url( $variant_url ) ) { - $scaled_url = $this->get_scaled_url( $variant_url ); + /* + * TODO: The logic is a mess, we need to refactor at some point. + * Websites may transition between 'www' subdomains and apex domains, potentially breaking references to hosted images. This can cause issues when attempting to match attachment IDs if images are linked using outdated domains. The logic is checking for alternative domains and consider the use of 'scaled' prefixes in image URLs for large images, which might affect ID matching. + */ + if ( $attachment_id === 0 ) { + if ( strpos( $url, 'www.' ) !== false ) { + $variant_url = str_replace( 'www.', '', $url ); + $attachment_id = attachment_url_to_postid( $variant_url ); + } else { + $variant_url = str_replace( '://', '://www.', $url ); + $attachment_id = attachment_url_to_postid( $variant_url ); + } + if ( $attachment_id === 0 && ! $this->is_scaled_url( $variant_url ) ) { + $scaled_url = $this->get_scaled_url( $variant_url ); - $attachment_id = attachment_url_to_postid( $scaled_url ); - } + $attachment_id = attachment_url_to_postid( $scaled_url ); } } Optml_Attachment_Cache::set_cached_attachment_id( $input_url, $attachment_id );