From 2477e3f479e561902500fe2b5aa600dc7635aba6 Mon Sep 17 00:00:00 2001 From: Mukesh Panchal Date: Wed, 6 Nov 2024 15:11:58 +0530 Subject: [PATCH 1/7] Remove false negative checks --- plugins/webp-uploads/tests/test-image-edit.php | 1 - plugins/webp-uploads/tests/test-load.php | 5 ----- 2 files changed, 6 deletions(-) diff --git a/plugins/webp-uploads/tests/test-image-edit.php b/plugins/webp-uploads/tests/test-image-edit.php index fa4ec310c1..7bfb0b9606 100644 --- a/plugins/webp-uploads/tests/test-image-edit.php +++ b/plugins/webp-uploads/tests/test-image-edit.php @@ -77,7 +77,6 @@ public function test_it_should_restore_the_sources_array_from_the_backup_when_an wp_restore_image( $attachment_id ); - $this->assertImageNotHasSource( $attachment_id, 'image/jpeg' ); $this->assertImageHasSource( $attachment_id, 'image/webp' ); $metadata = wp_get_attachment_metadata( $attachment_id ); diff --git a/plugins/webp-uploads/tests/test-load.php b/plugins/webp-uploads/tests/test-load.php index f70fd19e9f..050b9d8bde 100644 --- a/plugins/webp-uploads/tests/test-load.php +++ b/plugins/webp-uploads/tests/test-load.php @@ -55,7 +55,6 @@ public function test_it_should_not_create_the_original_mime_type_for_jpeg_images // There should be an image_type source, but no JPEG source for the full image. $this->assertImageHasSource( $attachment_id, $mime_type ); - $this->assertImageNotHasSource( $attachment_id, 'image/jpeg' ); $metadata = wp_get_attachment_metadata( $attachment_id ); @@ -263,8 +262,6 @@ public function test_it_should_create_a_webp_version_with_all_the_required_prope $file = get_attached_file( $attachment_id, true ); $dirname = pathinfo( $file, PATHINFO_DIRNAME ); - $this->assertImageNotHasSource( $attachment_id, 'image/jpeg' ); - $this->assertImageHasSource( $attachment_id, 'image/webp' ); $this->assertFileExists( path_join( $dirname, $metadata['sources']['image/webp']['file'] ) ); $this->assertSame( $metadata['sources']['image/webp']['filesize'], wp_filesize( path_join( $dirname, $metadata['sources']['image/webp']['file'] ) ) ); @@ -285,7 +282,6 @@ public function test_it_should_create_the_full_size_images_when_no_size_is_avail $metadata = wp_get_attachment_metadata( $attachment_id ); $this->assertEmpty( $metadata['sizes'] ); - $this->assertImageNotHasSource( $attachment_id, 'image/jpeg' ); $this->assertImageHasSource( $attachment_id, 'image/webp' ); } @@ -706,7 +702,6 @@ static function ( $editors ) { $this->assertArrayHasKey( 'sources', $metadata ); $this->assertIsArray( $metadata['sources'] ); - $this->assertImageNotHasSource( $attachment_id, 'image/jpeg' ); $this->assertImageHasSource( $attachment_id, 'image/' . $image_type ); $this->assertImageNotHasSizeSource( $attachment_id, 'thumbnail', 'image/jpeg' ); From b203c1514b18bd7391c79738d42571be07719509 Mon Sep 17 00:00:00 2001 From: Mukesh Panchal Date: Thu, 7 Nov 2024 10:21:28 +0530 Subject: [PATCH 2/7] Revert "Remove false negative checks" This reverts commit 2477e3f479e561902500fe2b5aa600dc7635aba6. --- plugins/webp-uploads/tests/test-image-edit.php | 1 + plugins/webp-uploads/tests/test-load.php | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/plugins/webp-uploads/tests/test-image-edit.php b/plugins/webp-uploads/tests/test-image-edit.php index 7bfb0b9606..fa4ec310c1 100644 --- a/plugins/webp-uploads/tests/test-image-edit.php +++ b/plugins/webp-uploads/tests/test-image-edit.php @@ -77,6 +77,7 @@ public function test_it_should_restore_the_sources_array_from_the_backup_when_an wp_restore_image( $attachment_id ); + $this->assertImageNotHasSource( $attachment_id, 'image/jpeg' ); $this->assertImageHasSource( $attachment_id, 'image/webp' ); $metadata = wp_get_attachment_metadata( $attachment_id ); diff --git a/plugins/webp-uploads/tests/test-load.php b/plugins/webp-uploads/tests/test-load.php index 050b9d8bde..f70fd19e9f 100644 --- a/plugins/webp-uploads/tests/test-load.php +++ b/plugins/webp-uploads/tests/test-load.php @@ -55,6 +55,7 @@ public function test_it_should_not_create_the_original_mime_type_for_jpeg_images // There should be an image_type source, but no JPEG source for the full image. $this->assertImageHasSource( $attachment_id, $mime_type ); + $this->assertImageNotHasSource( $attachment_id, 'image/jpeg' ); $metadata = wp_get_attachment_metadata( $attachment_id ); @@ -262,6 +263,8 @@ public function test_it_should_create_a_webp_version_with_all_the_required_prope $file = get_attached_file( $attachment_id, true ); $dirname = pathinfo( $file, PATHINFO_DIRNAME ); + $this->assertImageNotHasSource( $attachment_id, 'image/jpeg' ); + $this->assertImageHasSource( $attachment_id, 'image/webp' ); $this->assertFileExists( path_join( $dirname, $metadata['sources']['image/webp']['file'] ) ); $this->assertSame( $metadata['sources']['image/webp']['filesize'], wp_filesize( path_join( $dirname, $metadata['sources']['image/webp']['file'] ) ) ); @@ -282,6 +285,7 @@ public function test_it_should_create_the_full_size_images_when_no_size_is_avail $metadata = wp_get_attachment_metadata( $attachment_id ); $this->assertEmpty( $metadata['sizes'] ); + $this->assertImageNotHasSource( $attachment_id, 'image/jpeg' ); $this->assertImageHasSource( $attachment_id, 'image/webp' ); } @@ -702,6 +706,7 @@ static function ( $editors ) { $this->assertArrayHasKey( 'sources', $metadata ); $this->assertIsArray( $metadata['sources'] ); + $this->assertImageNotHasSource( $attachment_id, 'image/jpeg' ); $this->assertImageHasSource( $attachment_id, 'image/' . $image_type ); $this->assertImageNotHasSizeSource( $attachment_id, 'thumbnail', 'image/jpeg' ); From db72c296a31a6b610880507266634653dad6d3fb Mon Sep 17 00:00:00 2001 From: Mukesh Panchal Date: Thu, 7 Nov 2024 10:32:04 +0530 Subject: [PATCH 3/7] Fix failed unit tests --- plugins/webp-uploads/hooks.php | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/plugins/webp-uploads/hooks.php b/plugins/webp-uploads/hooks.php index 8354eea92c..965fb6b782 100644 --- a/plugins/webp-uploads/hooks.php +++ b/plugins/webp-uploads/hooks.php @@ -52,21 +52,30 @@ * } An array with the updated structure for the metadata before is stored in the database. */ function webp_uploads_create_sources_property( array $metadata, int $attachment_id ): array { - // This should take place only on the JPEG image. + $file = get_attached_file( $attachment_id, true ); + // File does not exist. + if ( false === $file || ! file_exists( $file ) ) { + return $metadata; + } + + /* + * We need to get the MIME type ideally from the file, as WordPress Core may have already altered it. + * The post MIME type is typically not updated during that process. + */ + $filetype = wp_check_filetype( $file ); + if ( isset( $filetype['type'] ) ) { + $mime_type = $filetype['type']; + } else { + $mime_type = get_post_mime_type( $attachment_id ); + } + $valid_mime_transforms = webp_uploads_get_upload_image_mime_transforms(); // Not a supported mime type to create the sources property. - $mime_type = get_post_mime_type( $attachment_id ); if ( ! is_string( $mime_type ) || ! isset( $valid_mime_transforms[ $mime_type ] ) ) { return $metadata; } - $file = get_attached_file( $attachment_id, true ); - // File does not exist. - if ( false === $file || ! file_exists( $file ) ) { - return $metadata; - } - // Make sure the top level `sources` key is a valid array. if ( ! isset( $metadata['sources'] ) || ! is_array( $metadata['sources'] ) ) { $metadata['sources'] = array(); From c19d894a9c95e5337a3df2ba2415f8b3d730b3c1 Mon Sep 17 00:00:00 2001 From: Mukesh Panchal Date: Thu, 7 Nov 2024 11:14:50 +0530 Subject: [PATCH 4/7] Add big threshold check for image --- plugins/webp-uploads/tests/test-load.php | 28 ++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/plugins/webp-uploads/tests/test-load.php b/plugins/webp-uploads/tests/test-load.php index f70fd19e9f..d5f4d4820b 100644 --- a/plugins/webp-uploads/tests/test-load.php +++ b/plugins/webp-uploads/tests/test-load.php @@ -43,9 +43,9 @@ static function ( string $filename ) { /** * Don't create the original mime type for JPEG images. * - * @dataProvider data_provider_supported_image_types + * @dataProvider data_provider_supported_image_types_with_threshold */ - public function test_it_should_not_create_the_original_mime_type_for_jpeg_images( string $image_type ): void { + public function test_it_should_not_create_the_original_mime_type_for_jpeg_images( string $image_type, bool $apply_threshold = false ): void { $mime_type = 'image/' . $image_type; $this->set_image_output_type( $image_type ); if ( ! webp_uploads_mime_type_supported( $mime_type ) ) { @@ -53,6 +53,16 @@ public function test_it_should_not_create_the_original_mime_type_for_jpeg_images } $attachment_id = self::factory()->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/data/images/leaves.jpg' ); + if ( $apply_threshold ) { + // Add threshold to create a `-scaled` output image for testing. + add_filter( + 'big_image_size_threshold', + static function () { + return 850; + } + ); + } + // There should be an image_type source, but no JPEG source for the full image. $this->assertImageHasSource( $attachment_id, $mime_type ); $this->assertImageNotHasSource( $attachment_id, 'image/jpeg' ); @@ -749,6 +759,20 @@ public function data_provider_supported_image_types(): array { ); } + /** + * Data provider for tests returns the supported image types to run the tests with and without threshold check. + * + * @return array> An array of valid image types. + */ + public function data_provider_supported_image_types_with_threshold(): array { + return array( + 'webp' => array( 'webp' ), + 'webp with 850 threshold' => array( 'webp', true ), + 'avif' => array( 'avif' ), + 'avif with 850 threshold' => array( 'avif', true ), + ); + } + /** * Prevent replacing an image if image was uploaded via external source or plugin. * From 616dbea2b4737884eccc9363bc8c1c6b4207f223 Mon Sep 17 00:00:00 2001 From: Mukesh Panchal Date: Thu, 7 Nov 2024 11:37:47 +0530 Subject: [PATCH 5/7] Remove duplicate test --- plugins/webp-uploads/tests/test-load.php | 36 ------------------------ 1 file changed, 36 deletions(-) diff --git a/plugins/webp-uploads/tests/test-load.php b/plugins/webp-uploads/tests/test-load.php index d5f4d4820b..a27d873289 100644 --- a/plugins/webp-uploads/tests/test-load.php +++ b/plugins/webp-uploads/tests/test-load.php @@ -116,42 +116,6 @@ public function test_it_should_create_the_original_mime_type_as_well_with_all_th } } - /** - * Create JPEG and output type for JPEG images, if opted in. - * - * @dataProvider data_provider_supported_image_types - */ - public function test_it_should_create_jpeg_and_webp_for_jpeg_images_if_opted_in( string $image_type ): void { - $mime_type = 'image/' . $image_type; - if ( ! webp_uploads_mime_type_supported( $mime_type ) ) { - $this->markTestSkipped( "Mime type $mime_type is not supported." ); - } - $this->set_image_output_type( $image_type ); - - update_option( 'perflab_generate_webp_and_jpeg', true ); - $attachment_id = self::factory()->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/data/images/leaves.jpg' ); - - // There should be JPEG and mime_type sources for the full image. - $this->assertImageHasSource( $attachment_id, 'image/jpeg' ); - $this->assertImageHasSource( $attachment_id, $mime_type ); - - $metadata = wp_get_attachment_metadata( $attachment_id ); - - // The full image should be a JPEG. - $this->assertArrayHasKey( 'file', $metadata ); - $this->assertStringEndsWith( $metadata['sources']['image/jpeg']['file'], $metadata['file'] ); - $this->assertStringEndsWith( $metadata['sources']['image/jpeg']['file'], get_attached_file( $attachment_id ) ); - - // The post MIME type should be JPEG. - $this->assertSame( 'image/jpeg', get_post_mime_type( $attachment_id ) ); - - // There should be JPEG and WebP sources for all sizes. - foreach ( array_keys( $metadata['sizes'] ) as $size_name ) { - $this->assertImageHasSizeSource( $attachment_id, $size_name, 'image/jpeg' ); - $this->assertImageHasSizeSource( $attachment_id, $size_name, $mime_type ); - } - } - /** * Create JPEG and output format for JPEG images, if perflab_generate_webp_and_jpeg option set. * From 2adce3d72dad0ed67dfe1d76717b15e53644d744 Mon Sep 17 00:00:00 2001 From: Mukesh Panchal Date: Fri, 8 Nov 2024 12:41:18 +0530 Subject: [PATCH 6/7] Update variable name --- plugins/webp-uploads/tests/test-load.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/webp-uploads/tests/test-load.php b/plugins/webp-uploads/tests/test-load.php index a27d873289..c9c8f0d32f 100644 --- a/plugins/webp-uploads/tests/test-load.php +++ b/plugins/webp-uploads/tests/test-load.php @@ -45,7 +45,7 @@ static function ( string $filename ) { * * @dataProvider data_provider_supported_image_types_with_threshold */ - public function test_it_should_not_create_the_original_mime_type_for_jpeg_images( string $image_type, bool $apply_threshold = false ): void { + public function test_it_should_not_create_the_original_mime_type_for_jpeg_images( string $image_type, bool $above_big_image_size = false ): void { $mime_type = 'image/' . $image_type; $this->set_image_output_type( $image_type ); if ( ! webp_uploads_mime_type_supported( $mime_type ) ) { @@ -53,7 +53,7 @@ public function test_it_should_not_create_the_original_mime_type_for_jpeg_images } $attachment_id = self::factory()->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/data/images/leaves.jpg' ); - if ( $apply_threshold ) { + if ( $above_big_image_size ) { // Add threshold to create a `-scaled` output image for testing. add_filter( 'big_image_size_threshold', From 5b027becb22f4dfbb51e3fc9078f1d2872ff9462 Mon Sep 17 00:00:00 2001 From: Mukesh Panchal Date: Fri, 8 Nov 2024 12:54:17 +0530 Subject: [PATCH 7/7] Apply suggestions from code review Co-authored-by: Weston Ruter --- plugins/webp-uploads/hooks.php | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/plugins/webp-uploads/hooks.php b/plugins/webp-uploads/hooks.php index 965fb6b782..07167cfc4f 100644 --- a/plugins/webp-uploads/hooks.php +++ b/plugins/webp-uploads/hooks.php @@ -62,17 +62,16 @@ function webp_uploads_create_sources_property( array $metadata, int $attachment_ * We need to get the MIME type ideally from the file, as WordPress Core may have already altered it. * The post MIME type is typically not updated during that process. */ - $filetype = wp_check_filetype( $file ); - if ( isset( $filetype['type'] ) ) { - $mime_type = $filetype['type']; - } else { - $mime_type = get_post_mime_type( $attachment_id ); + $filetype = wp_check_filetype( $file ); + $mime_type = $filetype['type'] ?? get_post_mime_type( $attachment_id ); + if ( ! is_string( $mime_type ) ) { + return $metadata; } $valid_mime_transforms = webp_uploads_get_upload_image_mime_transforms(); // Not a supported mime type to create the sources property. - if ( ! is_string( $mime_type ) || ! isset( $valid_mime_transforms[ $mime_type ] ) ) { + if ( ! isset( $valid_mime_transforms[ $mime_type ] ) ) { return $metadata; }