From a6f5e077c3c647d018de574e96a91f73724bc3ab Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Sat, 19 Mar 2022 12:39:31 -0600 Subject: [PATCH 01/25] Add class to mimic image operations on the editor. The class is in charge of performing operations to an image as if a user is performing those actions from the image editor but from the BE. This class is a utility class to be used from within the test context to mimic the user behavior of performing edits into an image. --- .../webp-uploads/class-wp-image-edit.php | 167 ++++++++++++++++++ 1 file changed, 167 insertions(+) create mode 100644 tests/testdata/modules/images/webp-uploads/class-wp-image-edit.php diff --git a/tests/testdata/modules/images/webp-uploads/class-wp-image-edit.php b/tests/testdata/modules/images/webp-uploads/class-wp-image-edit.php new file mode 100644 index 0000000000..831c0465de --- /dev/null +++ b/tests/testdata/modules/images/webp-uploads/class-wp-image-edit.php @@ -0,0 +1,167 @@ +attachment_id = $attachment_id; + } + + /** + * Once the object is removed make sure that `history` and `target` are + * removed from the $_REQUEST. + */ + public function __destruct() { + unset( $_REQUEST['history'], $_REQUEST['target'] ); + } + + /** + * Register a change to rotate an image to the right. + * + * @return $this + */ + public function rotate_right() { + $this->changes[] = array( 'r' => -90 ); + + return $this; + } + + /** + * Register a new change to rotate an image to the left. + * + * @return $this + */ + public function rotate_left() { + $this->changes[] = array( 'r' => 90 ); + + return $this; + } + + /** + * Add a new change, to flip an image vertically. + * + * @return $this + */ + public function flip_vertical() { + $this->changes[] = array( 'f' => 1 ); + + return $this; + } + + /** + * Add a new change to the image to flip it right. + * + * @return $this + */ + public function flip_right() { + $this->changes[] = array( 'f' => 2 ); + + return $this; + } + + /** + * Store a crop change for an image. + * + * @param int $width The width of the crop. + * @param int $height The height of the crop. + * @param int $x The X position on the axis where the image would be cropped. + * @param int $y The Y position on the axis where the image would be cropped. + * + * @return $this + */ + public function crop( $width, $height, $x, $y ) { + $this->changes[] = array( + 'c' => array( + 'x' => (int) $x, + 'y' => (int) $y, + 'w' => (int) $width, + 'h' => (int) $height, + ), + ); + + return $this; + } + + /** + * Set the target of the edits to all the image sizes. + * + * @return $this + */ + public function all() { + $this->target = 'all'; + + return $this; + } + + /** + * Set the target of the edit only to the thumbnail image. + * + * @return $this + */ + public function only_thumbnail() { + $this->target = 'thumbnail'; + + return $this; + } + + /** + * Set the target to all image sizes except the thumbnail. + * + * @return $this + */ + public function all_except_thumbnail() { + $this->target = 'nothumb'; + + return $this; + } + + /** + * Setup the $_REQUEST global so `wp_save_image` can process the image with the same editions + * performend into an image as it was performed from the editor. + * + * @see wp_save_image + * + * @return stdClass The operation resulted from calling `wp_save_image` + */ + public function save() { + $_REQUEST['target'] = $this->target; + $_REQUEST['history'] = wp_slash( wp_json_encode( $this->changes ) ); + + if ( ! function_exists( 'wp_save_image' ) ) { + include_once ABSPATH . 'wp-admin/includes/image-edit.php'; + } + + $this->result = wp_save_image( $this->attachment_id ); + + return $this->result; + } + + /** + * Determine if the last operation executed to edit the image was successfully or not. + * + * @return bool whether the operation to save the image was succesfully or not. + */ + public function success() { + return is_object( $this->result ) && property_exists( $this->result, 'msg' ) && property_exists( $this->result, 'thumbnail' ) && 'Image saved' === $this->result->msg; + } +} From 4820501b979d4e710baed19bd75d4597b628562b Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Sat, 19 Mar 2022 12:49:27 -0600 Subject: [PATCH 02/25] Restore and backup image sizes including `sources`. When an image is edited the `sources` properties is going to be backup with the rest of the sizes properties, however that's not the case for the `full` size which is a virtual type and the top level `sources` would be moved into the backup metadata. When the image is restored any edited image available in the sources array would be removed before the restore takes place. The `sources` property for the `full-orig` would be placed at the top level `sources` with the rest of the metadata. --- modules/images/webp-uploads/load.php | 187 ++++++++++++ .../images/webp-uploads/webp-uploads-test.php | 276 +++++++++++++++++- 2 files changed, 461 insertions(+), 2 deletions(-) diff --git a/modules/images/webp-uploads/load.php b/modules/images/webp-uploads/load.php index 81cacbb998..b334ec5078 100644 --- a/modules/images/webp-uploads/load.php +++ b/modules/images/webp-uploads/load.php @@ -683,3 +683,190 @@ function webp_uploads_update_rest_attachment( WP_REST_Response $response, WP_Pos return rest_ensure_response( $data ); } add_filter( 'rest_prepare_attachment', 'webp_uploads_update_rest_attachment', 10, 3 ); + +/** + * Inspect if the current call to `wp_update_attachment_metadata()` was done from within the context + * of an edit to an attachment either restore or other type of edit, in that case we perform operations + * to save the sources properties, specifically for the `full` size image due this is a virtual image size. + * + * @since n.e.x.t + * + * @see wp_update_attachment_metadata() + * + * @param array $data The current metadata of the attachment. + * @param int $attachment_id The ID of the current attachment. + * @return array The updated metadata for the attachment to be stored in the meta table. + */ +function webp_wp_update_attachment_metadata( $data, $attachment_id ) { + + $trace = debug_backtrace( DEBUG_BACKTRACE_IGNORE_ARGS, 10 ); + + foreach ( $trace as $element ) { + if ( ! isset( $element['function'] ) ) { + continue; + } + + switch ( $element['function'] ) { + case 'wp_save_image': + // Right after an image has been edited. + return webp_uploads_backup_sources( $attachment_id, $data ); + case 'wp_restore_image': + // When an image has been restored. + return webp_uploads_restore_image( $attachment_id, $data ); + } + } + + return $data; +} + +add_filter( 'wp_update_attachment_metadata', 'webp_wp_update_attachment_metadata', 10, 2 ); + +/** + * Before saving the metadata of the image store a backup values for the sources and file property + * those files would be used and deleted by the backup mechanism, right after the metadata has + * been updated. It removes the current sources property due once this function is executed + * right after an edit has taken place and the current sources are no longer accurate. + * + * @since n.e.x.t + * + * @param int $attachment_id The ID representing the attachment. + * @param array $data The current metadata of the attachment. + * @return array The updated metadata for the attachment. + */ +function webp_uploads_backup_sources( $attachment_id, $data ) { + if ( ! isset( $data['sources'] ) ) { + return $data; + } + + $data['_sources'] = $data['sources']; + // Remove the current sources as at this point the current values are no longer accurate. + unset( $data['sources'] ); + + return $data; +} + +/** + * Restore an image from the backup sizes, the current hook moves the `sources` from the `full-orig` key into + * the top level `sources` into the metadata, in order to ensure the restore process has a reference to the right + * images. When `IMAGE_EDIT_OVERWRITE` is defined and is truthy the function would loop into the sources array + * and remove any sources that was created as an edited version, the filenames for those cases are constructed out + * of `e-{digits}` where `{digits}` is a number of length 13. + * + * @param int $attachment_id The ID of the attachment. + * @param array $data The current metadata to be stored in the attachment. + * @return array The updated metadata of the attachment. + */ +function webp_uploads_restore_image( $attachment_id, $data ) { + $backup_sizes = get_post_meta( $attachment_id, '_wp_attachment_backup_sizes', true ); + + if ( ! is_array( $backup_sizes ) ) { + return $data; + } + + // If `IMAGE_EDIT_OVERWRITE` is defined and is truthy remove any edited images if present before replacing the metadata. + if ( defined( 'IMAGE_EDIT_OVERWRITE' ) && IMAGE_EDIT_OVERWRITE ) { + $file = get_attached_file( $attachment_id ); + $dirname = pathinfo( $file, PATHINFO_DIRNAME ); + $metadata = wp_get_attachment_metadata( $attachment_id ); + + $sources = isset( $metadata['sources'] ) && is_array( $metadata['sources'] ) ? $metadata['sources'] : array(); + + foreach ( $sources as $mime => $properties ) { + if ( empty( $properties['file'] ) ) { + continue; + } + + // Delete only if it's an edited image. + if ( preg_match( '/-e\d{13}/', $properties['file'] ) ) { + $delete_file = path_join( $dirname, $properties['file'] ); + wp_delete_file( $delete_file ); + } + } + + foreach ( $metadata['sizes'] as $size_name => $properties ) { + if ( ! isset( $properties['sources'] ) || ! is_array( $properties['sources'] ) ) { + continue; + } + + foreach ( $properties['sources'] as $mime => $source_properties ) { + if ( empty( $source_properties['file'] ) ) { + continue; + } + + // Delete only if it's an edited image. + if ( preg_match( '/-e\d{13}/', $source_properties['file'] ) ) { + $delete_file = path_join( $dirname, $source_properties['file'] ); + wp_delete_file( $delete_file ); + } + } + } + } + + if ( isset( $backup_sizes['full-orig']['sources'] ) ) { + $data['sources'] = $backup_sizes['full-orig']['sources']; + } + + return $data; +} + +/** + * Hook fired right after a metadata has been created or updated, this function would look + * specifically only for the key: `_wp_attachment_backup_sizes` which is the one used to + * store all the backup sizes. This hook is in charge of cleaning up the additional meta + * keys stored in the metadata of the attachment and storing the `sources` property in the + * previous full size image due this is not a size we need to move the `sources` from the + * metadata back into the full size similar as how it's done on the rest of the sizes. + * + * @since n.e.x.t + * + * @param int $meta_id The ID of the meta value stored in the DB. + * @param int $attachment_id The ID of the post used for this meta, in our case the attachment ID. + * @param string $meta_name The name of the metadata to be updated. + * @param array $backup_sizes An array with the metadata value in this case the backup sizes. + */ +function webp_updated_postmeta( $meta_id, $attachment_id, $meta_name, $backup_sizes ) { + if ( '_wp_attachment_backup_sizes' !== $meta_name ) { + return; + } + + if ( ! is_array( $backup_sizes ) ) { + return; + } + + $metadata = wp_get_attachment_metadata( $attachment_id ); + // No backup sources exists for the full size. + if ( ! isset( $metadata['_sources'] ) ) { + return; + } + + $target = null; + foreach ( array_keys( $backup_sizes ) as $size_name ) { + // We are only interested in the `full-` sizes. + if ( strpos( $size_name, 'full-' ) === false ) { + continue; + } + // If the target already has the sources attributes find the next one. + if ( isset( $backup_sizes[ $size_name ]['sources'] ) ) { + continue; + } + + $target = $size_name; + } + + if ( null === $target || ! isset( $backup_sizes[ $target ] ) ) { + return; + } + + $updated_backup_sizes = $backup_sizes; + $updated_backup_sizes[ $target ]['sources'] = $metadata['_sources']; + // Prevent infinite loop. + remove_action( 'update_post_meta', 'webp_updated_postmeta' ); + // Store the `sources` property into the full size if present. + update_post_meta( $attachment_id, '_wp_attachment_backup_sizes', $updated_backup_sizes, $backup_sizes ); + // Make sure the metadata no longer has a reference to the _sources property once has been stored. + unset( $metadata['_sources'] ); + wp_update_attachment_metadata( $attachment_id, $metadata ); +} + +add_action( 'added_post_meta', 'webp_updated_postmeta', 10, 4 ); +add_action( 'updated_post_meta', 'webp_updated_postmeta', 10, 4 ); diff --git a/tests/modules/images/webp-uploads/webp-uploads-test.php b/tests/modules/images/webp-uploads/webp-uploads-test.php index 93084b0593..35151afcc0 100644 --- a/tests/modules/images/webp-uploads/webp-uploads-test.php +++ b/tests/modules/images/webp-uploads/webp-uploads-test.php @@ -234,6 +234,7 @@ function () { return array( 'WP_Image_Doesnt_Support_WebP' ); } ); + $result = webp_uploads_generate_image_size( $attachment_id, 'medium', 'image/webp' ); $this->assertTrue( is_wp_error( $result ) ); $this->assertSame( 'image_mime_type_not_supported', $result->get_error_code() ); @@ -748,7 +749,7 @@ function () { $this->assertStringContainsString( $size['width'], $size['sources']['image/webp']['file'] ); $this->assertStringContainsString( $size['height'], $size['sources']['image/webp']['file'] ); $this->assertStringContainsString( - // Remove the extension from the file. + // Remove the extension from the file. substr( $size['sources']['image/webp']['file'], 0, -4 ), $size['sources']['image/jpeg']['file'] ); @@ -765,9 +766,10 @@ public function it_should_transform_jpeg_to_webp_subsizes_using_transform_filter add_filter( 'webp_uploads_upload_image_mime_transforms', - function( $transforms ) { + function ( $transforms ) { // Unset "image/jpeg" mime type for jpeg images. unset( $transforms['image/jpeg'][ array_search( 'image/jpeg', $transforms['image/jpeg'], true ) ] ); + return $transforms; } ); @@ -783,4 +785,274 @@ function( $transforms ) { $this->assertImageNotHasSizeSource( $attachment_id, $size_name, 'image/jpeg' ); } } + + /** + * Backup the sources structure alongside the full size + * + * @test + */ + public function it_should_backup_the_sources_structure_alongside_the_full_size() { + $attachment_id = $this->factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' ); + + $metadata = wp_get_attachment_metadata( $attachment_id ); + $this->assertEmpty( get_post_meta( $attachment_id, '_wp_attachment_backup_sizes', true ) ); + + $editor = new WP_Image_Edit( $attachment_id ); + $editor->rotate_right()->save(); + + // Having a thumbnail ensures the process finished correctly. + $this->assertTrue( $editor->success() ); + + $backup_sizes = get_post_meta( $attachment_id, '_wp_attachment_backup_sizes', true ); + + $this->assertNotEmpty( $backup_sizes ); + $this->assertIsArray( $backup_sizes ); + + foreach ( $backup_sizes as $size => $properties ) { + $size_name = str_replace( '-orig', '', $size ); + $this->assertArrayHasKey( 'sources', $properties ); + + if ( 'full-orig' === $size ) { + $this->assertSame( $metadata['sources'], $properties['sources'] ); + } else { + $this->assertSame( $metadata['sizes'][ $size_name ]['sources'], $properties['sources'] ); + } + } + + $metadata = wp_get_attachment_metadata( $attachment_id ); + + $this->assertArrayNotHasKey( '_sources', $metadata ); + $this->assertArrayNotHasKey( '_file', $metadata ); + } + + /** + * Backup sources from edited images + * + * @test + */ + public function it_should_backup_sources_from_edited_images() { + $attachment_id = $this->factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' ); + $original_metadata = wp_get_attachment_metadata( $attachment_id ); + + $editor = new WP_Image_Edit( $attachment_id ); + $editor->rotate_right()->save(); + $this->assertTrue( $editor->success() ); + + $metadata = wp_get_attachment_metadata( $attachment_id ); + $updated_metadata = $metadata; + $backup_sizes = get_post_meta( $attachment_id, '_wp_attachment_backup_sizes', true ); + $filename = pathinfo( $updated_metadata['file'], PATHINFO_FILENAME ); + + $this->assertArrayHasKey( 'sources', $backup_sizes['full-orig'] ); + $this->assertMatchesRegularExpression( '/-e\d{13}/', $filename ); + // Fake the creation of sources array to the existing metadata. + $updated_metadata['sources'] = array( + get_post_mime_type( $attachment_id ) => $filename, + ); + + foreach ( $updated_metadata['sizes'] as $size_name => $props ) { + $updated_metadata['sizes'][ $size_name ]['sources'] = array( + $props['mime-type'] => $props['file'], + ); + } + + wp_update_attachment_metadata( $attachment_id, $updated_metadata ); + + $editor->rotate_left()->save(); + $this->assertTrue( $editor->success() ); + + $backup_sizes = get_post_meta( $attachment_id, '_wp_attachment_backup_sizes', true ); + + // Make sure the original images were stored in the backup. + foreach ( $backup_sizes as $size_name => $properties ) { + if ( preg_match( '/-\d{13}/', $size_name ) ) { + continue; + } + + $real_name = str_replace( '-orig', '', $size_name ); + if ( 'full' === $real_name ) { + $sources = $original_metadata['sources']; + } else { + $sources = $original_metadata['sizes'][ $real_name ]['sources']; + } + + $this->assertArrayHasKey( 'sources', $properties, "Sources not present in '{$size_name}'" ); + $this->assertSame( $sources, $properties['sources'], "The '{$size_name} is not identical.'" ); + } + + // Make sure that the edited images were stored correctly in the backup. + foreach ( $backup_sizes as $size_name => $properties ) { + // Test only the edited names. + if ( ! preg_match( '/-\d{13}/', $size_name ) ) { + continue; + } + + $real_name = preg_replace( '/-\d{13}/', '', $size_name ); + if ( 'full' === $real_name ) { + $sources = $updated_metadata['sources']; + } else { + $sources = $updated_metadata['sizes'][ $real_name ]['sources']; + } + + $this->assertArrayHasKey( 'sources', $properties, "Sources not present in '{$size_name}'" ); + $this->assertSame( $sources, $properties['sources'], "The '{$size_name} is not identical.'" ); + } + } + + /** + * Store all the information on the original backup key when image edit overwrite is defined + * + * @test + */ + public function it_should_store_all_the_information_on_the_original_backup_key_when_image_edit_overwrite_is_defined() { + define( 'IMAGE_EDIT_OVERWRITE', true ); + + $attachment_id = $this->factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' ); + $original_metadata = wp_get_attachment_metadata( $attachment_id ); + + $editor = new WP_Image_Edit( $attachment_id ); + $editor->rotate_right()->save(); + $this->assertTrue( $editor->success() ); + + $metadata = wp_get_attachment_metadata( $attachment_id ); + $updated_metadata = $metadata; + + // Fake the creation of sources array to the existing metadata. + $updated_metadata['sources'] = array( + get_post_mime_type( $attachment_id ) => pathinfo( $updated_metadata['file'], PATHINFO_FILENAME ), + ); + + foreach ( $updated_metadata['sizes'] as $size_name => $props ) { + $updated_metadata['sizes'][ $size_name ]['sources'] = array( + $props['mime-type'] => $props['file'], + ); + } + + wp_update_attachment_metadata( $attachment_id, $updated_metadata ); + + $editor->rotate_left()->save(); + $this->assertTrue( $editor->success() ); + + $backup_sizes = get_post_meta( $attachment_id, '_wp_attachment_backup_sizes', true ); + + // Make sure the original images were stored in the backup. + foreach ( $backup_sizes as $size_name => $properties ) { + $this->assertDoesNotMatchRegularExpression( '/-\d{13}/', $size_name ); + $this->assertMatchesRegularExpression( '/-orig/', $size_name ); + $this->assertArrayHasKey( 'sources', $properties, "Sources not present in '{$size_name}'" ); + + if ( 'full-orig' === $size_name ) { + $sources = $original_metadata['sources']; + } else { + $name = str_replace( '-orig', '', $size_name ); + $sources = $original_metadata['sizes'][ $name ]['sources']; + } + + $this->assertSame( $sources, $properties['sources'], "The '{$size_name} is not identical.'" ); + } + } + + /** + * Restore the sources array from the backup when an image is edited + * + * @test + */ + public function it_should_restore_the_sources_array_from_the_backup_when_an_image_is_edited() { + $attachment_id = $this->factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' ); + $editor = new WP_Image_Edit( $attachment_id ); + $editor->rotate_right()->save(); + $this->assertTrue( $editor->success() ); + + $backup_sizes = get_post_meta( $attachment_id, '_wp_attachment_backup_sizes', true ); + $this->assertArrayHasKey( 'full-orig', $backup_sizes ); + $this->assertArrayHasKey( 'sources', $backup_sizes['full-orig'] ); + $this->assertIsArray( $backup_sizes['full-orig']['sources'] ); + + wp_restore_image( $attachment_id ); + + $metadata = wp_get_attachment_metadata( $attachment_id ); + $this->assertArrayHasKey( 'sources', $metadata ); + $this->assertSame( $backup_sizes['full-orig']['sources'], $metadata['sources'] ); + + foreach ( $metadata['sizes'] as $size_name => $properties ) { + $this->assertArrayHasKey( 'sources', $backup_sizes[ $size_name . '-orig' ] ); + $this->assertSame( $backup_sizes[ $size_name . '-orig' ]['sources'], $properties['sources'] ); + } + } + + /** + * Delete edited images when image edit overwrite is defined + * + * @test + */ + public function it_should_delete_edited_images_when_image_edit_overwrite_is_defined() { + define( 'IMAGE_EDIT_OVERWRITE', true ); + + $attachment_id = $this->factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' ); + $editor = new WP_Image_Edit( $attachment_id ); + $editor->flip_vertical()->save(); + $this->assertTrue( $editor->success() ); + + $metadata = wp_get_attachment_metadata( $attachment_id ); + $file = get_attached_file( $attachment_id ); + + // Populate the sources array due this one requires: https://github.com/WordPress/performance/issues/158. + $metadata['sources'] = array( + 'image/jpeg' => array( + 'file' => pathinfo( $file, PATHINFO_FILENAME ), + 'filesize' => filesize( $file ), + ), + 'image/webp' => webp_uploads_generate_additional_image_source( + $attachment_id, + array( + 'width' => $metadata['width'], + 'height' => $metadata['height'], + 'crop' => false, + ), + 'image/webp', + str_replace( '.jpeg', '.webp', $file ) + ), + ); + + $directory = pathinfo( $file, PATHINFO_DIRNAME ); + foreach ( wp_get_registered_image_subsizes() as $size_name => $props ) { + if ( ! isset( $metadata['sizes'][ $size_name ] ) ) { + continue; + } + + $metadata['sizes'][ $size_name ]['sources'] = array( + $metadata['sizes'][ $size_name ]['mime-type'] => array( + 'file' => $metadata['sizes'][ $size_name ]['file'], + 'filesize' => filesize( $directory . DIRECTORY_SEPARATOR . $metadata['sizes'][ $size_name ]['file'] ), + ), + 'image/webp' => webp_uploads_generate_additional_image_source( + $attachment_id, + array( + 'width' => $props['width'], + 'height' => $props['height'], + 'crop' => $props['crop'], + ), + 'image/webp' + ), + ); + } + + wp_update_attachment_metadata( $attachment_id, $metadata ); + + $metadata_before_restore = wp_get_attachment_metadata( $attachment_id ); + + wp_restore_image( $attachment_id ); + + $this->assertFileDoesNotExist( $metadata_before_restore['file'] ); + $this->assertFileDoesNotExist( $directory . DIRECTORY_SEPARATOR . $metadata_before_restore['sources']['image/jpeg']['file'] ); + $this->assertFileDoesNotExist( $directory . DIRECTORY_SEPARATOR . $metadata_before_restore['sources']['image/webp']['file'] ); + + $this->assertArrayHasKey( 'sizes', $metadata_before_restore ); + foreach ( $metadata_before_restore['sizes'] as $size_name => $properties ) { + $this->assertArrayHasKey( 'sources', $properties ); + foreach ( $properties['sources'] as $mime => $values ) { + $this->assertFileDoesNotExist( $directory . DIRECTORY_SEPARATOR . $values['file'] ); + } + } + } } From 7e82041726a206909fa9d7aa59520619c9af6de4 Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Wed, 23 Mar 2022 11:37:56 -0600 Subject: [PATCH 03/25] Remove handling of edited images This would be handled once #158 is merged so the current approach can be adjusted accordingly. --- modules/images/webp-uploads/load.php | 26 +-- .../images/webp-uploads/webp-uploads-test.php | 150 ------------------ 2 files changed, 2 insertions(+), 174 deletions(-) diff --git a/modules/images/webp-uploads/load.php b/modules/images/webp-uploads/load.php index fa3e73b5e3..1e9eee081c 100644 --- a/modules/images/webp-uploads/load.php +++ b/modules/images/webp-uploads/load.php @@ -784,30 +784,8 @@ function webp_uploads_restore_image( $attachment_id, $data ) { return $data; } - // If `IMAGE_EDIT_OVERWRITE` is defined and is truthy remove any edited images if present before replacing the metadata. - if ( defined( 'IMAGE_EDIT_OVERWRITE' ) && IMAGE_EDIT_OVERWRITE ) { - $file = get_attached_file( $attachment_id ); - $dirname = pathinfo( $file, PATHINFO_DIRNAME ); - $metadata = wp_get_attachment_metadata( $attachment_id ); - - $sources = isset( $metadata['sources'] ) && is_array( $metadata['sources'] ) ? $metadata['sources'] : array(); - - foreach ( $sources as $mime => $properties ) { - if ( empty( $properties['file'] ) ) { - continue; - } - - // Delete only if it's an edited image. - if ( preg_match( '/-e\d{13}/', $properties['file'] ) ) { - $delete_file = path_join( $dirname, $properties['file'] ); - wp_delete_file( $delete_file ); - } - } - - foreach ( $metadata['sizes'] as $size_name => $properties ) { - if ( ! isset( $properties['sources'] ) || ! is_array( $properties['sources'] ) ) { - continue; - } + // TODO: Handle the case If `IMAGE_EDIT_OVERWRITE` is defined and is truthy remove any edited images if present before replacing the metadata. + // See: https://github.com/WordPress/performance/issues/158. foreach ( $properties['sources'] as $mime => $source_properties ) { if ( empty( $source_properties['file'] ) ) { diff --git a/tests/modules/images/webp-uploads/webp-uploads-test.php b/tests/modules/images/webp-uploads/webp-uploads-test.php index f601d25f9f..7cfdd189e6 100644 --- a/tests/modules/images/webp-uploads/webp-uploads-test.php +++ b/tests/modules/images/webp-uploads/webp-uploads-test.php @@ -850,80 +850,6 @@ public function it_should_backup_the_sources_structure_alongside_the_full_size() $this->assertArrayNotHasKey( '_file', $metadata ); } - /** - * Backup sources from edited images - * - * @test - */ - public function it_should_backup_sources_from_edited_images() { - $attachment_id = $this->factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' ); - $original_metadata = wp_get_attachment_metadata( $attachment_id ); - - $editor = new WP_Image_Edit( $attachment_id ); - $editor->rotate_right()->save(); - $this->assertTrue( $editor->success() ); - - $metadata = wp_get_attachment_metadata( $attachment_id ); - $updated_metadata = $metadata; - $backup_sizes = get_post_meta( $attachment_id, '_wp_attachment_backup_sizes', true ); - $filename = pathinfo( $updated_metadata['file'], PATHINFO_FILENAME ); - - $this->assertArrayHasKey( 'sources', $backup_sizes['full-orig'] ); - $this->assertMatchesRegularExpression( '/-e\d{13}/', $filename ); - // Fake the creation of sources array to the existing metadata. - $updated_metadata['sources'] = array( - get_post_mime_type( $attachment_id ) => $filename, - ); - - foreach ( $updated_metadata['sizes'] as $size_name => $props ) { - $updated_metadata['sizes'][ $size_name ]['sources'] = array( - $props['mime-type'] => $props['file'], - ); - } - - wp_update_attachment_metadata( $attachment_id, $updated_metadata ); - - $editor->rotate_left()->save(); - $this->assertTrue( $editor->success() ); - - $backup_sizes = get_post_meta( $attachment_id, '_wp_attachment_backup_sizes', true ); - - // Make sure the original images were stored in the backup. - foreach ( $backup_sizes as $size_name => $properties ) { - if ( preg_match( '/-\d{13}/', $size_name ) ) { - continue; - } - - $real_name = str_replace( '-orig', '', $size_name ); - if ( 'full' === $real_name ) { - $sources = $original_metadata['sources']; - } else { - $sources = $original_metadata['sizes'][ $real_name ]['sources']; - } - - $this->assertArrayHasKey( 'sources', $properties, "Sources not present in '{$size_name}'" ); - $this->assertSame( $sources, $properties['sources'], "The '{$size_name} is not identical.'" ); - } - - // Make sure that the edited images were stored correctly in the backup. - foreach ( $backup_sizes as $size_name => $properties ) { - // Test only the edited names. - if ( ! preg_match( '/-\d{13}/', $size_name ) ) { - continue; - } - - $real_name = preg_replace( '/-\d{13}/', '', $size_name ); - if ( 'full' === $real_name ) { - $sources = $updated_metadata['sources']; - } else { - $sources = $updated_metadata['sizes'][ $real_name ]['sources']; - } - - $this->assertArrayHasKey( 'sources', $properties, "Sources not present in '{$size_name}'" ); - $this->assertSame( $sources, $properties['sources'], "The '{$size_name} is not identical.'" ); - } - } - /** * Store all the information on the original backup key when image edit overwrite is defined * @@ -1004,80 +930,4 @@ public function it_should_restore_the_sources_array_from_the_backup_when_an_imag $this->assertSame( $backup_sizes[ $size_name . '-orig' ]['sources'], $properties['sources'] ); } } - - /** - * Delete edited images when image edit overwrite is defined - * - * @test - */ - public function it_should_delete_edited_images_when_image_edit_overwrite_is_defined() { - define( 'IMAGE_EDIT_OVERWRITE', true ); - - $attachment_id = $this->factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' ); - $editor = new WP_Image_Edit( $attachment_id ); - $editor->flip_vertical()->save(); - $this->assertTrue( $editor->success() ); - - $metadata = wp_get_attachment_metadata( $attachment_id ); - $file = get_attached_file( $attachment_id ); - - // Populate the sources array due this one requires: https://github.com/WordPress/performance/issues/158. - $metadata['sources'] = array( - 'image/jpeg' => array( - 'file' => pathinfo( $file, PATHINFO_FILENAME ), - 'filesize' => filesize( $file ), - ), - 'image/webp' => webp_uploads_generate_additional_image_source( - $attachment_id, - array( - 'width' => $metadata['width'], - 'height' => $metadata['height'], - 'crop' => false, - ), - 'image/webp', - str_replace( '.jpeg', '.webp', $file ) - ), - ); - - $directory = pathinfo( $file, PATHINFO_DIRNAME ); - foreach ( wp_get_registered_image_subsizes() as $size_name => $props ) { - if ( ! isset( $metadata['sizes'][ $size_name ] ) ) { - continue; - } - - $metadata['sizes'][ $size_name ]['sources'] = array( - $metadata['sizes'][ $size_name ]['mime-type'] => array( - 'file' => $metadata['sizes'][ $size_name ]['file'], - 'filesize' => filesize( $directory . DIRECTORY_SEPARATOR . $metadata['sizes'][ $size_name ]['file'] ), - ), - 'image/webp' => webp_uploads_generate_additional_image_source( - $attachment_id, - array( - 'width' => $props['width'], - 'height' => $props['height'], - 'crop' => $props['crop'], - ), - 'image/webp' - ), - ); - } - - wp_update_attachment_metadata( $attachment_id, $metadata ); - - $metadata_before_restore = wp_get_attachment_metadata( $attachment_id ); - - wp_restore_image( $attachment_id ); - - $this->assertFileDoesNotExist( $metadata_before_restore['file'] ); - $this->assertFileDoesNotExist( $directory . DIRECTORY_SEPARATOR . $metadata_before_restore['sources']['image/jpeg']['file'] ); - $this->assertFileDoesNotExist( $directory . DIRECTORY_SEPARATOR . $metadata_before_restore['sources']['image/webp']['file'] ); - - $this->assertArrayHasKey( 'sizes', $metadata_before_restore ); - foreach ( $metadata_before_restore['sizes'] as $size_name => $properties ) { - $this->assertArrayHasKey( 'sources', $properties ); - foreach ( $properties['sources'] as $mime => $values ) { - $this->assertFileDoesNotExist( $directory . DIRECTORY_SEPARATOR . $values['file'] ); - } - } - } } From a2b82e373bba288029aa9ab3c701cac2148e6085 Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Wed, 23 Mar 2022 12:27:15 -0600 Subject: [PATCH 04/25] Update the `success` logic to handle additional cases. When an image is edited using a target other than the all and the thumbnail make sure the `success` method returns the correct value. --- .../images/webp-uploads/class-wp-image-edit.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/testdata/modules/images/webp-uploads/class-wp-image-edit.php b/tests/testdata/modules/images/webp-uploads/class-wp-image-edit.php index 831c0465de..b3591b0cf0 100644 --- a/tests/testdata/modules/images/webp-uploads/class-wp-image-edit.php +++ b/tests/testdata/modules/images/webp-uploads/class-wp-image-edit.php @@ -162,6 +162,16 @@ public function save() { * @return bool whether the operation to save the image was succesfully or not. */ public function success() { - return is_object( $this->result ) && property_exists( $this->result, 'msg' ) && property_exists( $this->result, 'thumbnail' ) && 'Image saved' === $this->result->msg; + if ( ! is_object( $this->result ) ) { + return false; + } + + $valid_target = true; + // The thumbnail property is only set in `all` and `thumbnail` target. + if ( 'all' === $this->target || 'thumbnail' === $this->target ) { + $valid_target = property_exists( $this->result, 'thumbnail' ); + } + + return property_exists( $this->result, 'msg' ) && $valid_target && 'Image saved' === $this->result->msg; } } From ba0cbdf45808d8844fbd371da66e70b57ff5cdd3 Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Wed, 23 Mar 2022 14:32:48 -0600 Subject: [PATCH 05/25] Update logic to store and restore backup. Due to the only value that is not correctly stored is the top level sources attributes this now is stored in a separate meta value. Stored in `_wp_attachment_backup_sources` Logic was added to handle scenarios where only the thumbnail or all images except the thumbnail are modified. --- modules/images/webp-uploads/load.php | 63 ++++---- .../images/webp-uploads/webp-uploads-test.php | 146 ++++++++++++------ 2 files changed, 123 insertions(+), 86 deletions(-) diff --git a/modules/images/webp-uploads/load.php b/modules/images/webp-uploads/load.php index 1e9eee081c..4542fe5019 100644 --- a/modules/images/webp-uploads/load.php +++ b/modules/images/webp-uploads/load.php @@ -759,8 +759,20 @@ function webp_uploads_backup_sources( $attachment_id, $data ) { return $data; } - $data['_sources'] = $data['sources']; + $target = isset( $_REQUEST['target'] ) ? sanitize_text_field( $_REQUEST['target'] ) : 'all'; + + // When an edit to an image is only applied to a thumbnail there's nothing we need to backu up. + if ( 'thumbnail' === $target ) { + return $data; + } + + // Store the current sources before the current metadata is overwritten. + $sources = get_post_meta( $attachment_id, '_wp_attachment_backup_sources', true ); + $sources = is_array( $sources ) ? $sources : array(); + $sources['_sources'] = $data['sources']; + update_post_meta( $attachment_id, '_wp_attachment_backup_sources', $sources ); // Remove the current sources as at this point the current values are no longer accurate. + // TODO: Requires to be updated from https://github.com/WordPress/performance/issues/158. unset( $data['sources'] ); return $data; @@ -778,32 +790,16 @@ function webp_uploads_backup_sources( $attachment_id, $data ) { * @return array The updated metadata of the attachment. */ function webp_uploads_restore_image( $attachment_id, $data ) { - $backup_sizes = get_post_meta( $attachment_id, '_wp_attachment_backup_sizes', true ); + $backup_sources = get_post_meta( $attachment_id, '_wp_attachment_backup_sources', true ); - if ( ! is_array( $backup_sizes ) ) { + if ( ! is_array( $backup_sources ) || ! isset( $backup_sources['full-orig'] ) || ! is_array( $backup_sources['full-orig'] ) ) { return $data; } // TODO: Handle the case If `IMAGE_EDIT_OVERWRITE` is defined and is truthy remove any edited images if present before replacing the metadata. // See: https://github.com/WordPress/performance/issues/158. - foreach ( $properties['sources'] as $mime => $source_properties ) { - if ( empty( $source_properties['file'] ) ) { - continue; - } - - // Delete only if it's an edited image. - if ( preg_match( '/-e\d{13}/', $source_properties['file'] ) ) { - $delete_file = path_join( $dirname, $source_properties['file'] ); - wp_delete_file( $delete_file ); - } - } - } - } - - if ( isset( $backup_sizes['full-orig']['sources'] ) ) { - $data['sources'] = $backup_sizes['full-orig']['sources']; - } + $data['sources'] = $backup_sources['full-orig']; return $data; } @@ -824,17 +820,15 @@ function webp_uploads_restore_image( $attachment_id, $data ) { * @param array $backup_sizes An array with the metadata value in this case the backup sizes. */ function webp_updated_postmeta( $meta_id, $attachment_id, $meta_name, $backup_sizes ) { + // The backup sources array. if ( '_wp_attachment_backup_sizes' !== $meta_name ) { return; } - if ( ! is_array( $backup_sizes ) ) { - return; - } + $backup_sources = get_post_meta( $attachment_id, '_wp_attachment_backup_sources', true ); + $backup_sources = is_array( $backup_sources ) ? $backup_sources : array(); - $metadata = wp_get_attachment_metadata( $attachment_id ); - // No backup sources exists for the full size. - if ( ! isset( $metadata['_sources'] ) ) { + if ( ! isset( $backup_sources['_sources'] ) ) { return; } @@ -845,26 +839,23 @@ function webp_updated_postmeta( $meta_id, $attachment_id, $meta_name, $backup_si continue; } // If the target already has the sources attributes find the next one. - if ( isset( $backup_sizes[ $size_name ]['sources'] ) ) { + if ( isset( $backup_sources[ $size_name ] ) ) { continue; } $target = $size_name; + break; } - if ( null === $target || ! isset( $backup_sizes[ $target ] ) ) { + if ( null === $target ) { return; } - $updated_backup_sizes = $backup_sizes; - $updated_backup_sizes[ $target ]['sources'] = $metadata['_sources']; - // Prevent infinite loop. - remove_action( 'update_post_meta', 'webp_updated_postmeta' ); + $backup_sources[ $target ] = $backup_sources['_sources']; + // Remove the temporary used space. + unset( $backup_sources['_sources'] ); // Store the `sources` property into the full size if present. - update_post_meta( $attachment_id, '_wp_attachment_backup_sizes', $updated_backup_sizes, $backup_sizes ); - // Make sure the metadata no longer has a reference to the _sources property once has been stored. - unset( $metadata['_sources'] ); - wp_update_attachment_metadata( $attachment_id, $metadata ); + update_post_meta( $attachment_id, '_wp_attachment_backup_sources', $backup_sources ); } add_action( 'added_post_meta', 'webp_updated_postmeta', 10, 4 ); diff --git a/tests/modules/images/webp-uploads/webp-uploads-test.php b/tests/modules/images/webp-uploads/webp-uploads-test.php index 7cfdd189e6..f03c38a2cb 100644 --- a/tests/modules/images/webp-uploads/webp-uploads-test.php +++ b/tests/modules/images/webp-uploads/webp-uploads-test.php @@ -821,6 +821,7 @@ public function it_should_backup_the_sources_structure_alongside_the_full_size() $metadata = wp_get_attachment_metadata( $attachment_id ); $this->assertEmpty( get_post_meta( $attachment_id, '_wp_attachment_backup_sizes', true ) ); + $this->assertEmpty( get_post_meta( $attachment_id, '_wp_attachment_backup_sources', true ) ); $editor = new WP_Image_Edit( $attachment_id ); $editor->rotate_right()->save(); @@ -833,15 +834,21 @@ public function it_should_backup_the_sources_structure_alongside_the_full_size() $this->assertNotEmpty( $backup_sizes ); $this->assertIsArray( $backup_sizes ); + $backup_sources = get_post_meta( $attachment_id, '_wp_attachment_backup_sources', true ); + $this->assertIsArray( $backup_sources ); + $this->assertArrayHasKey( 'full-orig', $backup_sources ); + $this->assertSame( $metadata['sources'], $backup_sources['full-orig'] ); + $this->assertArrayNotHasKey( '_sources', $backup_sources ); + foreach ( $backup_sizes as $size => $properties ) { $size_name = str_replace( '-orig', '', $size ); - $this->assertArrayHasKey( 'sources', $properties ); if ( 'full-orig' === $size ) { - $this->assertSame( $metadata['sources'], $properties['sources'] ); - } else { - $this->assertSame( $metadata['sizes'][ $size_name ]['sources'], $properties['sources'] ); + continue; } + + $this->assertArrayHasKey( 'sources', $properties ); + $this->assertSame( $metadata['sizes'][ $size_name ]['sources'], $properties['sources'] ); } $metadata = wp_get_attachment_metadata( $attachment_id ); @@ -851,83 +858,122 @@ public function it_should_backup_the_sources_structure_alongside_the_full_size() } /** - * Store all the information on the original backup key when image edit overwrite is defined + * Restore the sources array from the backup when an image is edited * * @test */ - public function it_should_store_all_the_information_on_the_original_backup_key_when_image_edit_overwrite_is_defined() { - define( 'IMAGE_EDIT_OVERWRITE', true ); - - $attachment_id = $this->factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' ); - $original_metadata = wp_get_attachment_metadata( $attachment_id ); - - $editor = new WP_Image_Edit( $attachment_id ); + public function it_should_restore_the_sources_array_from_the_backup_when_an_image_is_edited() { + $attachment_id = $this->factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' ); + $metadata = wp_get_attachment_metadata( $attachment_id ); + $editor = new WP_Image_Edit( $attachment_id ); $editor->rotate_right()->save(); $this->assertTrue( $editor->success() ); - $metadata = wp_get_attachment_metadata( $attachment_id ); - $updated_metadata = $metadata; + $backup_sources = get_post_meta( $attachment_id, '_wp_attachment_backup_sources', true ); + $this->assertArrayHasKey( 'full-orig', $backup_sources ); + $this->assertIsArray( $backup_sources['full-orig'] ); + $this->assertSame( $metadata['sources'], $backup_sources['full-orig'] ); + $this->assertArrayNotHasKey( '_sources', $backup_sources['full-orig'] ); - // Fake the creation of sources array to the existing metadata. - $updated_metadata['sources'] = array( - get_post_mime_type( $attachment_id ) => pathinfo( $updated_metadata['file'], PATHINFO_FILENAME ), - ); + wp_restore_image( $attachment_id ); - foreach ( $updated_metadata['sizes'] as $size_name => $props ) { - $updated_metadata['sizes'][ $size_name ]['sources'] = array( - $props['mime-type'] => $props['file'], - ); + $metadata = wp_get_attachment_metadata( $attachment_id ); + $this->assertArrayHasKey( 'sources', $metadata ); + $this->assertSame( $backup_sources['full-orig'], $metadata['sources'] ); + $this->assertSame( $backup_sources, get_post_meta( $attachment_id, '_wp_attachment_backup_sources', true ) ); + + $backup_sizes = get_post_meta( $attachment_id, '_wp_attachment_backup_sizes', true ); + foreach ( $metadata['sizes'] as $size_name => $properties ) { + $this->assertArrayHasKey( 'sources', $backup_sizes[ $size_name . '-orig' ] ); + $this->assertSame( $backup_sizes[ $size_name . '-orig' ]['sources'], $properties['sources'] ); } + } - wp_update_attachment_metadata( $attachment_id, $updated_metadata ); + /** + * Prevent to back up the sources when the sources attributes does not exists + * + * @test + */ + public function it_should_prevent_to_back_up_the_sources_when_the_sources_attributes_does_not_exists() { + // Disable the generation of the sources attributes. + add_filter( 'webp_uploads_upload_image_mime_transforms', '__return_empty_array' ); - $editor->rotate_left()->save(); + $attachment_id = $this->factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' ); + $metadata = wp_get_attachment_metadata( $attachment_id ); + + $this->assertArrayNotHasKey( 'sources', $metadata ); + + $editor = new WP_Image_Edit( $attachment_id ); + $editor->flip_vertical()->save(); $this->assertTrue( $editor->success() ); + $backup_sources = get_post_meta( $attachment_id, '_wp_attachment_backup_sources', true ); + $this->assertEmpty( $backup_sources ); + $backup_sizes = get_post_meta( $attachment_id, '_wp_attachment_backup_sizes', true ); + $this->assertIsArray( $backup_sizes ); - // Make sure the original images were stored in the backup. foreach ( $backup_sizes as $size_name => $properties ) { - $this->assertDoesNotMatchRegularExpression( '/-\d{13}/', $size_name ); - $this->assertMatchesRegularExpression( '/-orig/', $size_name ); - $this->assertArrayHasKey( 'sources', $properties, "Sources not present in '{$size_name}'" ); - - if ( 'full-orig' === $size_name ) { - $sources = $original_metadata['sources']; - } else { - $name = str_replace( '-orig', '', $size_name ); - $sources = $original_metadata['sizes'][ $name ]['sources']; - } - - $this->assertSame( $sources, $properties['sources'], "The '{$size_name} is not identical.'" ); + $this->assertArrayNotHasKey( 'sources', $properties ); } } /** - * Restore the sources array from the backup when an image is edited + * Prevent to backup the full size image if only the thumbnail is edited * * @test */ - public function it_should_restore_the_sources_array_from_the_backup_when_an_image_is_edited() { + public function it_should_prevent_to_backup_the_full_size_image_if_only_the_thumbnail_is_edited() { $attachment_id = $this->factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' ); - $editor = new WP_Image_Edit( $attachment_id ); - $editor->rotate_right()->save(); + $metadata = wp_get_attachment_metadata( $attachment_id ); + $this->assertArrayHasKey( 'sources', $metadata ); + + $editor = new WP_Image_Edit( $attachment_id ); + $editor->flip_vertical()->only_thumbnail()->save(); $this->assertTrue( $editor->success() ); - $backup_sizes = get_post_meta( $attachment_id, '_wp_attachment_backup_sizes', true ); - $this->assertArrayHasKey( 'full-orig', $backup_sizes ); - $this->assertArrayHasKey( 'sources', $backup_sizes['full-orig'] ); - $this->assertIsArray( $backup_sizes['full-orig']['sources'] ); + $backup_sources = get_post_meta( $attachment_id, '_wp_attachment_backup_sources', true ); + $this->assertEmpty( $backup_sources ); - wp_restore_image( $attachment_id ); + $backup_sizes = get_post_meta( $attachment_id, '_wp_attachment_backup_sizes', true ); + $this->assertIsArray( $backup_sizes ); + $this->assertCount( 1, $backup_sizes ); + $this->assertArrayHasKey( 'thumbnail-orig', $backup_sizes ); + $this->assertArrayHasKey( 'sources', $backup_sizes['thumbnail-orig'] ); $metadata = wp_get_attachment_metadata( $attachment_id ); $this->assertArrayHasKey( 'sources', $metadata ); - $this->assertSame( $backup_sizes['full-orig']['sources'], $metadata['sources'] ); + } - foreach ( $metadata['sizes'] as $size_name => $properties ) { - $this->assertArrayHasKey( 'sources', $backup_sizes[ $size_name . '-orig' ] ); - $this->assertSame( $backup_sizes[ $size_name . '-orig' ]['sources'], $properties['sources'] ); + /** + * Backup the image when all images except the thumbnail are updated + * + * @test + */ + public function it_should_backup_the_image_when_all_images_except_the_thumbnail_are_updated() { + $attachment_id = $this->factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' ); + $metadata = wp_get_attachment_metadata( $attachment_id ); + + $editor = new WP_Image_Edit( $attachment_id ); + $editor->rotate_left()->all_except_thumbnail()->save(); + $this->assertTrue( $editor->success() ); + + $backup_sources = get_post_meta( $attachment_id, '_wp_attachment_backup_sources', true ); + $this->assertIsArray( $backup_sources ); + $this->assertArrayHasKey( 'full-orig', $backup_sources ); + $this->assertSame( $metadata['sources'], $backup_sources['full-orig'] ); + + $this->assertArrayNotHasKey( 'sources', wp_get_attachment_metadata( $attachment_id ), 'The sources attributes was not removed from the metadata.' ); + + $backup_sizes = get_post_meta( $attachment_id, '_wp_attachment_backup_sizes', true ); + $this->assertIsArray( $backup_sizes ); + $this->assertArrayNotHasKey( 'thumbnail-orig', $backup_sizes, 'The thumbnail-orig was stored in the back up' ); + + foreach ( $backup_sizes as $size_name => $properties ) { + if ( 'full-orig' === $size_name ) { + continue; + } + $this->assertArrayHasKey( 'sources', $properties, "The '{$size_name}' does not have the sources." ); } } } From 2dd29d844ce9e4de0563100f8ebac2609d3417a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Crisoforo=20Gaspar=20Hern=C3=A1ndez?= Date: Wed, 23 Mar 2022 15:11:02 -0600 Subject: [PATCH 06/25] Remove empty space Co-authored-by: Felix Arntz --- modules/images/webp-uploads/load.php | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/images/webp-uploads/load.php b/modules/images/webp-uploads/load.php index 4542fe5019..53c1761d56 100644 --- a/modules/images/webp-uploads/load.php +++ b/modules/images/webp-uploads/load.php @@ -739,7 +739,6 @@ function webp_wp_update_attachment_metadata( $data, $attachment_id ) { return $data; } - add_filter( 'wp_update_attachment_metadata', 'webp_wp_update_attachment_metadata', 10, 2 ); /** From b16cee1c05a8134be1ff4634355e134676cd46dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Crisoforo=20Gaspar=20Hern=C3=A1ndez?= Date: Wed, 23 Mar 2022 15:12:28 -0600 Subject: [PATCH 07/25] Update prefix on functions Co-authored-by: Felix Arntz --- modules/images/webp-uploads/load.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/images/webp-uploads/load.php b/modules/images/webp-uploads/load.php index 53c1761d56..97130bd475 100644 --- a/modules/images/webp-uploads/load.php +++ b/modules/images/webp-uploads/load.php @@ -718,7 +718,7 @@ function webp_uploads_update_rest_attachment( WP_REST_Response $response, WP_Pos * @param int $attachment_id The ID of the current attachment. * @return array The updated metadata for the attachment to be stored in the meta table. */ -function webp_wp_update_attachment_metadata( $data, $attachment_id ) { +function webp_uploads_update_attachment_metadata( $data, $attachment_id ) { $trace = debug_backtrace( DEBUG_BACKTRACE_IGNORE_ARGS, 10 ); From 8a8e96731ffff6197f44a028d8009d14d292e3de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Crisoforo=20Gaspar=20Hern=C3=A1ndez?= Date: Wed, 23 Mar 2022 15:13:23 -0600 Subject: [PATCH 08/25] Update prefix on functions Co-authored-by: Felix Arntz --- modules/images/webp-uploads/load.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/images/webp-uploads/load.php b/modules/images/webp-uploads/load.php index 97130bd475..1cc5264bdf 100644 --- a/modules/images/webp-uploads/load.php +++ b/modules/images/webp-uploads/load.php @@ -818,7 +818,7 @@ function webp_uploads_restore_image( $attachment_id, $data ) { * @param string $meta_name The name of the metadata to be updated. * @param array $backup_sizes An array with the metadata value in this case the backup sizes. */ -function webp_updated_postmeta( $meta_id, $attachment_id, $meta_name, $backup_sizes ) { +function webp_uploads_updated_postmeta( $meta_id, $attachment_id, $meta_name, $backup_sizes ) { // The backup sources array. if ( '_wp_attachment_backup_sizes' !== $meta_name ) { return; From 322dd3dc701b2f07df2d3517d856c7fe784236cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Crisoforo=20Gaspar=20Hern=C3=A1ndez?= Date: Wed, 23 Mar 2022 15:13:28 -0600 Subject: [PATCH 09/25] Update prefix on functions Co-authored-by: Felix Arntz --- modules/images/webp-uploads/load.php | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/images/webp-uploads/load.php b/modules/images/webp-uploads/load.php index 1cc5264bdf..13335464c6 100644 --- a/modules/images/webp-uploads/load.php +++ b/modules/images/webp-uploads/load.php @@ -856,6 +856,5 @@ function webp_uploads_updated_postmeta( $meta_id, $attachment_id, $meta_name, $b // Store the `sources` property into the full size if present. update_post_meta( $attachment_id, '_wp_attachment_backup_sources', $backup_sources ); } - add_action( 'added_post_meta', 'webp_updated_postmeta', 10, 4 ); add_action( 'updated_post_meta', 'webp_updated_postmeta', 10, 4 ); From 226a5aa851c6d46c4f350e7cc462575b5bccdf86 Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Wed, 23 Mar 2022 15:38:25 -0600 Subject: [PATCH 10/25] Update hooks reference --- modules/images/webp-uploads/load.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/images/webp-uploads/load.php b/modules/images/webp-uploads/load.php index 13335464c6..a08d3be1bc 100644 --- a/modules/images/webp-uploads/load.php +++ b/modules/images/webp-uploads/load.php @@ -739,7 +739,7 @@ function webp_uploads_update_attachment_metadata( $data, $attachment_id ) { return $data; } -add_filter( 'wp_update_attachment_metadata', 'webp_wp_update_attachment_metadata', 10, 2 ); +add_filter( 'wp_update_attachment_metadata', 'webp_uploads_update_attachment_metadata', 10, 2 ); /** * Before saving the metadata of the image store a backup values for the sources and file property @@ -856,5 +856,5 @@ function webp_uploads_updated_postmeta( $meta_id, $attachment_id, $meta_name, $b // Store the `sources` property into the full size if present. update_post_meta( $attachment_id, '_wp_attachment_backup_sources', $backup_sources ); } -add_action( 'added_post_meta', 'webp_updated_postmeta', 10, 4 ); -add_action( 'updated_post_meta', 'webp_updated_postmeta', 10, 4 ); +add_action( 'added_post_meta', 'webp_uploads_updated_postmeta', 10, 4 ); +add_action( 'updated_post_meta', 'webp_uploads_updated_postmeta', 10, 4 ); From 8a809ed3802012c8d9c4e4b6ed88828987647c29 Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Wed, 23 Mar 2022 15:41:18 -0600 Subject: [PATCH 11/25] Backup the data from previous metadata --- modules/images/webp-uploads/load.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/modules/images/webp-uploads/load.php b/modules/images/webp-uploads/load.php index a08d3be1bc..4f80575008 100644 --- a/modules/images/webp-uploads/load.php +++ b/modules/images/webp-uploads/load.php @@ -754,7 +754,9 @@ function webp_uploads_update_attachment_metadata( $data, $attachment_id ) { * @return array The updated metadata for the attachment. */ function webp_uploads_backup_sources( $attachment_id, $data ) { - if ( ! isset( $data['sources'] ) ) { + $metadata = wp_get_attachment_metadata( $attachment_id ); + // Nothing to back up. + if ( ! isset( $metadata['sources'] ) ) { return $data; } @@ -768,7 +770,7 @@ function webp_uploads_backup_sources( $attachment_id, $data ) { // Store the current sources before the current metadata is overwritten. $sources = get_post_meta( $attachment_id, '_wp_attachment_backup_sources', true ); $sources = is_array( $sources ) ? $sources : array(); - $sources['_sources'] = $data['sources']; + $sources['_sources'] = $metadata['sources']; update_post_meta( $attachment_id, '_wp_attachment_backup_sources', $sources ); // Remove the current sources as at this point the current values are no longer accurate. // TODO: Requires to be updated from https://github.com/WordPress/performance/issues/158. From 951c4cad3d00cb80d69570ecc00ed9c68a251d74 Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Sat, 26 Mar 2022 12:31:10 -0600 Subject: [PATCH 12/25] Remove the need of auxilary variable to hold the sources Move the hooks for the update right into the place where they are actually used. --- modules/images/webp-uploads/load.php | 140 +++++++++++------- .../images/webp-uploads/webp-uploads-test.php | 132 ++++++++++++++++- 2 files changed, 210 insertions(+), 62 deletions(-) diff --git a/modules/images/webp-uploads/load.php b/modules/images/webp-uploads/load.php index d7e229657b..0a37c28948 100644 --- a/modules/images/webp-uploads/load.php +++ b/modules/images/webp-uploads/load.php @@ -762,16 +762,44 @@ function webp_uploads_backup_sources( $attachment_id, $data ) { $target = isset( $_REQUEST['target'] ) ? sanitize_text_field( $_REQUEST['target'] ) : 'all'; - // When an edit to an image is only applied to a thumbnail there's nothing we need to backu up. + // When an edit to an image is only applied to a thumbnail there's nothing we need to back up. if ( 'thumbnail' === $target ) { return $data; } - // Store the current sources before the current metadata is overwritten. - $sources = get_post_meta( $attachment_id, '_wp_attachment_backup_sources', true ); - $sources = is_array( $sources ) ? $sources : array(); - $sources['_sources'] = $metadata['sources']; - update_post_meta( $attachment_id, '_wp_attachment_backup_sources', $sources ); + $sources = $metadata['sources']; + // Prevent execution of the callbacks more than once if the callback was already executed. + $executed = false; + + add_action( + 'added_post_meta', + function ( $meta_id, $attachment_id, $meta_name, $backup_sizes ) use ( $sources, &$executed ) { + // The backup sources array. + if ( '_wp_attachment_backup_sizes' !== $meta_name || $executed ) { + return; + } + + $executed = true; + webp_uploads_backup_full_image_sources( $attachment_id, $sources ); + }, + 10, + 4 + ); + + add_action( + 'updated_post_meta', + function ( $meta_id, $attachment_id, $meta_name, $backup_sizes ) use ( $sources, &$executed ) { + // The backup sources array. + if ( '_wp_attachment_backup_sizes' !== $meta_name || $executed ) { + return; + } + $executed = true; + webp_uploads_backup_full_image_sources( $attachment_id, $sources ); + }, + 10, + 4 + ); + // Remove the current sources as at this point the current values are no longer accurate. // TODO: Requires to be updated from https://github.com/WordPress/performance/issues/158. unset( $data['sources'] ); @@ -780,83 +808,83 @@ function webp_uploads_backup_sources( $attachment_id, $data ) { } /** - * Restore an image from the backup sizes, the current hook moves the `sources` from the `full-orig` key into - * the top level `sources` into the metadata, in order to ensure the restore process has a reference to the right - * images. When `IMAGE_EDIT_OVERWRITE` is defined and is truthy the function would loop into the sources array - * and remove any sources that was created as an edited version, the filenames for those cases are constructed out - * of `e-{digits}` where `{digits}` is a number of length 13. + * Stores the provided sources for the attachment ID in the `_wp_attachment_backup_sources` with + * the next available target if target is `null` no source would be stored. * * @param int $attachment_id The ID of the attachment. - * @param array $data The current metadata to be stored in the attachment. - * @return array The updated metadata of the attachment. + * @param array $sources An array with the full sources to be stored on the next available key. */ -function webp_uploads_restore_image( $attachment_id, $data ) { - $backup_sources = get_post_meta( $attachment_id, '_wp_attachment_backup_sources', true ); +function webp_uploads_backup_full_image_sources( $attachment_id, $sources ) { + $target = webp_uploads_get_next_full_size_key_from_backup( $attachment_id ); - if ( ! is_array( $backup_sources ) || ! isset( $backup_sources['full-orig'] ) || ! is_array( $backup_sources['full-orig'] ) ) { - return $data; + if ( null === $target || empty( $sources ) ) { + return; } - // TODO: Handle the case If `IMAGE_EDIT_OVERWRITE` is defined and is truthy remove any edited images if present before replacing the metadata. - // See: https://github.com/WordPress/performance/issues/158. - - $data['sources'] = $backup_sources['full-orig']; - - return $data; + $backup_sources = get_post_meta( $attachment_id, '_wp_attachment_backup_sources', true ); + $backup_sources = is_array( $backup_sources ) ? $backup_sources : array(); + $backup_sources[ $target ] = $sources; + // Store the `sources` property into the full size if present. + update_post_meta( $attachment_id, '_wp_attachment_backup_sources', $backup_sources ); } /** - * Hook fired right after a metadata has been created or updated, this function would look - * specifically only for the key: `_wp_attachment_backup_sizes` which is the one used to - * store all the backup sizes. This hook is in charge of cleaning up the additional meta - * keys stored in the metadata of the attachment and storing the `sources` property in the - * previous full size image due this is not a size we need to move the `sources` from the - * metadata back into the full size similar as how it's done on the rest of the sizes. + * It finds the next available `full-{orig or hash}` key on the images if the name + * has not been used as part of the backup sources it would be used if no size is + * found or backup exists `null` would be returned instead. * - * @since n.e.x.t - * - * @param int $meta_id The ID of the meta value stored in the DB. - * @param int $attachment_id The ID of the post used for this meta, in our case the attachment ID. - * @param string $meta_name The name of the metadata to be updated. - * @param array $backup_sizes An array with the metadata value in this case the backup sizes. + * @param int $attachment_id The ID of the attachment. + * @return null|string The next available full size name. */ -function webp_uploads_updated_postmeta( $meta_id, $attachment_id, $meta_name, $backup_sizes ) { - // The backup sources array. - if ( '_wp_attachment_backup_sizes' !== $meta_name ) { - return; +function webp_uploads_get_next_full_size_key_from_backup( $attachment_id ) { + $backup_sizes = get_post_meta( $attachment_id, '_wp_attachment_backup_sizes', true ); + $backup_sizes = is_array( $backup_sizes ) ? $backup_sizes : array(); + + if ( empty( $backup_sizes ) ) { + return null; } $backup_sources = get_post_meta( $attachment_id, '_wp_attachment_backup_sources', true ); $backup_sources = is_array( $backup_sources ) ? $backup_sources : array(); - - if ( ! isset( $backup_sources['_sources'] ) ) { - return; - } - - $target = null; foreach ( array_keys( $backup_sizes ) as $size_name ) { // We are only interested in the `full-` sizes. if ( strpos( $size_name, 'full-' ) === false ) { continue; } + // If the target already has the sources attributes find the next one. if ( isset( $backup_sources[ $size_name ] ) ) { continue; } - $target = $size_name; - break; + return $size_name; } - if ( null === $target ) { - return; + return null; +} + +/** + * Restore an image from the backup sizes, the current hook moves the `sources` from the `full-orig` key into + * the top level `sources` into the metadata, in order to ensure the restore process has a reference to the right + * images. When `IMAGE_EDIT_OVERWRITE` is defined and is truthy the function would loop into the sources array + * and remove any sources that was created as an edited version, the filenames for those cases are constructed out + * of `e-{digits}` where `{digits}` is a number of length 13. + * + * @param int $attachment_id The ID of the attachment. + * @param array $data The current metadata to be stored in the attachment. + * @return array The updated metadata of the attachment. + */ +function webp_uploads_restore_image( $attachment_id, $data ) { + $backup_sources = get_post_meta( $attachment_id, '_wp_attachment_backup_sources', true ); + + if ( ! is_array( $backup_sources ) || ! isset( $backup_sources['full-orig'] ) || ! is_array( $backup_sources['full-orig'] ) ) { + return $data; } - $backup_sources[ $target ] = $backup_sources['_sources']; - // Remove the temporary used space. - unset( $backup_sources['_sources'] ); - // Store the `sources` property into the full size if present. - update_post_meta( $attachment_id, '_wp_attachment_backup_sources', $backup_sources ); + // TODO: Handle the case If `IMAGE_EDIT_OVERWRITE` is defined and is truthy remove any edited images if present before replacing the metadata. + // See: https://github.com/WordPress/performance/issues/158. + + $data['sources'] = $backup_sources['full-orig']; + + return $data; } -add_action( 'added_post_meta', 'webp_uploads_updated_postmeta', 10, 4 ); -add_action( 'updated_post_meta', 'webp_uploads_updated_postmeta', 10, 4 ); diff --git a/tests/modules/images/webp-uploads/webp-uploads-test.php b/tests/modules/images/webp-uploads/webp-uploads-test.php index 419c407d54..8a68fab092 100644 --- a/tests/modules/images/webp-uploads/webp-uploads-test.php +++ b/tests/modules/images/webp-uploads/webp-uploads-test.php @@ -838,7 +838,6 @@ public function it_should_backup_the_sources_structure_alongside_the_full_size() $this->assertIsArray( $backup_sources ); $this->assertArrayHasKey( 'full-orig', $backup_sources ); $this->assertSame( $metadata['sources'], $backup_sources['full-orig'] ); - $this->assertArrayNotHasKey( '_sources', $backup_sources ); foreach ( $backup_sizes as $size => $properties ) { $size_name = str_replace( '-orig', '', $size ); @@ -852,9 +851,6 @@ public function it_should_backup_the_sources_structure_alongside_the_full_size() } $metadata = wp_get_attachment_metadata( $attachment_id ); - - $this->assertArrayNotHasKey( '_sources', $metadata ); - $this->assertArrayNotHasKey( '_file', $metadata ); } /** @@ -868,12 +864,10 @@ public function it_should_restore_the_sources_array_from_the_backup_when_an_imag $editor = new WP_Image_Edit( $attachment_id ); $editor->rotate_right()->save(); $this->assertTrue( $editor->success() ); - $backup_sources = get_post_meta( $attachment_id, '_wp_attachment_backup_sources', true ); $this->assertArrayHasKey( 'full-orig', $backup_sources ); $this->assertIsArray( $backup_sources['full-orig'] ); $this->assertSame( $metadata['sources'], $backup_sources['full-orig'] ); - $this->assertArrayNotHasKey( '_sources', $backup_sources['full-orig'] ); wp_restore_image( $attachment_id ); @@ -1004,4 +998,130 @@ function () { $this->assertImageHasSizeSource( $attachment_id, 'thumbnail', 'image/jpeg' ); $this->assertImageHasSizeSource( $attachment_id, 'thumbnail', 'image/webp' ); } + + /** + * Not return a target if no backup image exists + * + * @test + */ + public function it_should_not_return_a_target_if_no_backup_image_exists() { + $attachment_id = $this->factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' ); + $this->assertNull( webp_uploads_get_next_full_size_key_from_backup( $attachment_id ) ); + } + + /** + * Return the full-orig target key when only one edit image exists + * + * @test + */ + public function it_should_return_the_full_orig_target_key_when_only_one_edit_image_exists() { + // Remove the filter to prevent the usage of the next target. + remove_filter( 'wp_update_attachment_metadata', 'webp_uploads_update_attachment_metadata' ); + + $attachment_id = $this->factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' ); + $editor = new WP_Image_Edit( $attachment_id ); + $editor->rotate_right()->save(); + + $this->assertTrue( $editor->success() ); + $this->assertSame( 'full-orig', webp_uploads_get_next_full_size_key_from_backup( $attachment_id ) ); + } + + /** + * Return null when looking for a target that is already used + * + * @test + */ + public function it_should_return_null_when_looking_for_a_target_that_is_already_used() { + $attachment_id = $this->factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' ); + $editor = new WP_Image_Edit( $attachment_id ); + $editor->rotate_right()->save(); + + $this->assertTrue( $editor->success() ); + $this->assertNull( webp_uploads_get_next_full_size_key_from_backup( $attachment_id ) ); + } + + /** + * USe the next available hash for the full size image on multiple image edits + * + * @test + */ + public function it_should_u_se_the_next_available_hash_for_the_full_size_image_on_multiple_image_edits() { + $attachment_id = $this->factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' ); + $editor = new WP_Image_Edit( $attachment_id ); + $editor->rotate_right()->save(); + + $this->assertTrue( $editor->success() ); + $this->assertNull( webp_uploads_get_next_full_size_key_from_backup( $attachment_id ) ); + // Remove the filter to prevent the usage of the next target. + remove_filter( 'wp_update_attachment_metadata', 'webp_uploads_update_attachment_metadata' ); + + $editor->rotate_right()->save(); + $this->assertRegExp( '/full-\d{13}/', webp_uploads_get_next_full_size_key_from_backup( $attachment_id ) ); + } + + /** + * Save populate the backup sources with the next target + * + * @test + */ + public function it_should_save_populate_the_backup_sources_with_the_next_target() { + // Remove the filter to prevent the usage of the next target. + remove_filter( 'wp_update_attachment_metadata', 'webp_uploads_update_attachment_metadata' ); + + $attachment_id = $this->factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' ); + $editor = new WP_Image_Edit( $attachment_id ); + $editor->rotate_right()->save(); + $this->assertTrue( $editor->success() ); + + $sources = array( 'image/webp' => 'leafs.webp' ); + webp_uploads_backup_full_image_sources( $attachment_id, $sources ); + + $this->assertSame( array( 'full-orig' => $sources ), get_post_meta( $attachment_id, '_wp_attachment_backup_sources', true ) ); + } + + /** + * Store the metadata on the next available hash + * + * @test + */ + public function it_should_store_the_metadata_on_the_next_available_hash() { + $attachment_id = $this->factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' ); + $editor = new WP_Image_Edit( $attachment_id ); + $editor->rotate_right()->save(); + $this->assertTrue( $editor->success() ); + // Remove the filter to prevent the usage of the next target. + remove_filter( 'wp_update_attachment_metadata', 'webp_uploads_update_attachment_metadata' ); + $editor->rotate_right()->save(); + $this->assertTrue( $editor->success() ); + + $sources = array( 'image/webp' => 'leafs.webp' ); + webp_uploads_backup_full_image_sources( $attachment_id, $sources ); + + $backup_sources = get_post_meta( $attachment_id, '_wp_attachment_backup_sources', true ); + $this->assertIsArray( $backup_sources ); + + $backup_sources_keys = array_keys( $backup_sources ); + $this->assertSame( 'full-orig', reset( $backup_sources_keys ) ); + $this->assertRegExp( '/full-\d{13}/', end( $backup_sources_keys ) ); + $this->assertSame( $sources, end( $backup_sources ) ); + } + + /** + * Prevent to store an empty set of sources + * + * @test + */ + public function it_should_prevent_to_store_an_empty_set_of_sources() { + // Remove the filter to prevent the usage of the next target. + remove_filter( 'wp_update_attachment_metadata', 'webp_uploads_update_attachment_metadata' ); + + $attachment_id = $this->factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' ); + $editor = new WP_Image_Edit( $attachment_id ); + $editor->rotate_right()->save(); + + webp_uploads_backup_full_image_sources( $attachment_id, array() ); + + $this->assertTrue( $editor->success() ); + $this->assertEmpty( get_post_meta( $attachment_id, '_wp_attachment_backup_sources', true ) ); + } } From 61e25e8d89dbd23713e1de51f72409397d2957a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Crisoforo=20Gaspar=20Hern=C3=A1ndez?= Date: Mon, 28 Mar 2022 16:34:27 -0600 Subject: [PATCH 13/25] Add `since` doc block Co-authored-by: Felix Arntz --- modules/images/webp-uploads/load.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/modules/images/webp-uploads/load.php b/modules/images/webp-uploads/load.php index 0a37c28948..603a46eaf3 100644 --- a/modules/images/webp-uploads/load.php +++ b/modules/images/webp-uploads/load.php @@ -811,6 +811,8 @@ function ( $meta_id, $attachment_id, $meta_name, $backup_sizes ) use ( $sources, * Stores the provided sources for the attachment ID in the `_wp_attachment_backup_sources` with * the next available target if target is `null` no source would be stored. * + * @since n.e.x.t + * * @param int $attachment_id The ID of the attachment. * @param array $sources An array with the full sources to be stored on the next available key. */ From 4c26f4f40146f0c3a5464fef116cc9eeca1e4ae1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Crisoforo=20Gaspar=20Hern=C3=A1ndez?= Date: Mon, 28 Mar 2022 16:35:02 -0600 Subject: [PATCH 14/25] Align parameters on the doc block Co-authored-by: Felix Arntz --- modules/images/webp-uploads/load.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/images/webp-uploads/load.php b/modules/images/webp-uploads/load.php index 603a46eaf3..eb4552caa0 100644 --- a/modules/images/webp-uploads/load.php +++ b/modules/images/webp-uploads/load.php @@ -714,7 +714,7 @@ function webp_uploads_update_rest_attachment( WP_REST_Response $response, WP_Pos * * @see wp_update_attachment_metadata() * - * @param array $data The current metadata of the attachment. + * @param array $data The current metadata of the attachment. * @param int $attachment_id The ID of the current attachment. * @return array The updated metadata for the attachment to be stored in the meta table. */ From 973a7b896f4cef89a6b4d74ab71ccd73a80bda28 Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Mon, 28 Mar 2022 16:38:44 -0600 Subject: [PATCH 15/25] Removal of non required parameter The `backup_sizes` parameter was not used so it can be safely removed from the code execution. --- modules/images/webp-uploads/load.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/images/webp-uploads/load.php b/modules/images/webp-uploads/load.php index eb4552caa0..bdb2d77982 100644 --- a/modules/images/webp-uploads/load.php +++ b/modules/images/webp-uploads/load.php @@ -773,7 +773,7 @@ function webp_uploads_backup_sources( $attachment_id, $data ) { add_action( 'added_post_meta', - function ( $meta_id, $attachment_id, $meta_name, $backup_sizes ) use ( $sources, &$executed ) { + function ( $meta_id, $attachment_id, $meta_name ) use ( $sources, &$executed ) { // The backup sources array. if ( '_wp_attachment_backup_sizes' !== $meta_name || $executed ) { return; @@ -783,12 +783,12 @@ function ( $meta_id, $attachment_id, $meta_name, $backup_sizes ) use ( $sources, webp_uploads_backup_full_image_sources( $attachment_id, $sources ); }, 10, - 4 + 3 ); add_action( 'updated_post_meta', - function ( $meta_id, $attachment_id, $meta_name, $backup_sizes ) use ( $sources, &$executed ) { + function ( $meta_id, $attachment_id, $meta_name ) use ( $sources, &$executed ) { // The backup sources array. if ( '_wp_attachment_backup_sizes' !== $meta_name || $executed ) { return; @@ -797,7 +797,7 @@ function ( $meta_id, $attachment_id, $meta_name, $backup_sizes ) use ( $sources, webp_uploads_backup_full_image_sources( $attachment_id, $sources ); }, 10, - 4 + 3 ); // Remove the current sources as at this point the current values are no longer accurate. From cb35f42cdf151ca9423f9e8a7d78c0a6503cdf20 Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Mon, 28 Mar 2022 16:41:44 -0600 Subject: [PATCH 16/25] Align parameters to follow WordPress guidelines --- modules/images/webp-uploads/load.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/images/webp-uploads/load.php b/modules/images/webp-uploads/load.php index bdb2d77982..31db13b662 100644 --- a/modules/images/webp-uploads/load.php +++ b/modules/images/webp-uploads/load.php @@ -750,7 +750,7 @@ function webp_uploads_update_attachment_metadata( $data, $attachment_id ) { * @since n.e.x.t * * @param int $attachment_id The ID representing the attachment. - * @param array $data The current metadata of the attachment. + * @param array $data The current metadata of the attachment. * @return array The updated metadata for the attachment. */ function webp_uploads_backup_sources( $attachment_id, $data ) { @@ -814,7 +814,7 @@ function ( $meta_id, $attachment_id, $meta_name ) use ( $sources, &$executed ) { * @since n.e.x.t * * @param int $attachment_id The ID of the attachment. - * @param array $sources An array with the full sources to be stored on the next available key. + * @param array $sources An array with the full sources to be stored on the next available key. */ function webp_uploads_backup_full_image_sources( $attachment_id, $sources ) { $target = webp_uploads_get_next_full_size_key_from_backup( $attachment_id ); @@ -873,7 +873,7 @@ function webp_uploads_get_next_full_size_key_from_backup( $attachment_id ) { * of `e-{digits}` where `{digits}` is a number of length 13. * * @param int $attachment_id The ID of the attachment. - * @param array $data The current metadata to be stored in the attachment. + * @param array $data The current metadata to be stored in the attachment. * @return array The updated metadata of the attachment. */ function webp_uploads_restore_image( $attachment_id, $data ) { From 7880f6b9504cd1755517c93724c6b5b4f16da33d Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Mon, 28 Mar 2022 16:42:11 -0600 Subject: [PATCH 17/25] Include the `@since` tags on missing functions --- modules/images/webp-uploads/load.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/modules/images/webp-uploads/load.php b/modules/images/webp-uploads/load.php index 31db13b662..a516c858cd 100644 --- a/modules/images/webp-uploads/load.php +++ b/modules/images/webp-uploads/load.php @@ -835,6 +835,8 @@ function webp_uploads_backup_full_image_sources( $attachment_id, $sources ) { * has not been used as part of the backup sources it would be used if no size is * found or backup exists `null` would be returned instead. * + * @since n.e.x.t + * * @param int $attachment_id The ID of the attachment. * @return null|string The next available full size name. */ @@ -868,9 +870,9 @@ function webp_uploads_get_next_full_size_key_from_backup( $attachment_id ) { /** * Restore an image from the backup sizes, the current hook moves the `sources` from the `full-orig` key into * the top level `sources` into the metadata, in order to ensure the restore process has a reference to the right - * images. When `IMAGE_EDIT_OVERWRITE` is defined and is truthy the function would loop into the sources array - * and remove any sources that was created as an edited version, the filenames for those cases are constructed out - * of `e-{digits}` where `{digits}` is a number of length 13. + * images. + * + * @since n.e.x.t * * @param int $attachment_id The ID of the attachment. * @param array $data The current metadata to be stored in the attachment. From ca8d28e8fd19c91734fdd6c5288a857c116361d5 Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Mon, 28 Mar 2022 16:48:40 -0600 Subject: [PATCH 18/25] Move logic of the hook into a variable Reuse the hook as now is stored in a variable, so it can be reused in both hooks. --- modules/images/webp-uploads/load.php | 49 ++++++++++++---------------- 1 file changed, 21 insertions(+), 28 deletions(-) diff --git a/modules/images/webp-uploads/load.php b/modules/images/webp-uploads/load.php index a516c858cd..4a0ed12979 100644 --- a/modules/images/webp-uploads/load.php +++ b/modules/images/webp-uploads/load.php @@ -769,36 +769,29 @@ function webp_uploads_backup_sources( $attachment_id, $data ) { $sources = $metadata['sources']; // Prevent execution of the callbacks more than once if the callback was already executed. - $executed = false; - - add_action( - 'added_post_meta', - function ( $meta_id, $attachment_id, $meta_name ) use ( $sources, &$executed ) { - // The backup sources array. - if ( '_wp_attachment_backup_sizes' !== $meta_name || $executed ) { - return; - } + $has_been_processed = false; - $executed = true; - webp_uploads_backup_full_image_sources( $attachment_id, $sources ); - }, - 10, - 3 - ); + $hook = function ( $meta_id, $post_id, $meta_name ) use ( $attachment_id, $sources, &$has_been_processed ) { + // Make sure this hook is only executed in the same context for the provided $attachment_id. + if ( $post_id !== $attachment_id ) { + return; + } - add_action( - 'updated_post_meta', - function ( $meta_id, $attachment_id, $meta_name ) use ( $sources, &$executed ) { - // The backup sources array. - if ( '_wp_attachment_backup_sizes' !== $meta_name || $executed ) { - return; - } - $executed = true; - webp_uploads_backup_full_image_sources( $attachment_id, $sources ); - }, - 10, - 3 - ); + // This logic should work only if we are looking at the meta key: `_wp_attachment_backup_sizes`. + if ( '_wp_attachment_backup_sizes' !== $meta_name ) { + return; + } + + if ( $has_been_processed ) { + return; + } + + $has_been_processed = true; + webp_uploads_backup_full_image_sources( $post_id, $sources ); + }; + + add_action( 'added_post_meta', $hook, 10, 3 ); + add_action( 'updated_post_meta', $hook, 10, 3 ); // Remove the current sources as at this point the current values are no longer accurate. // TODO: Requires to be updated from https://github.com/WordPress/performance/issues/158. From c381398f6a27839d2bc79c885565268fb2d1f309 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Crisoforo=20Gaspar=20Hern=C3=A1ndez?= Date: Wed, 30 Mar 2022 10:49:18 -0600 Subject: [PATCH 19/25] Add white spaces on tests Co-authored-by: Eugene Manuilov --- tests/modules/images/webp-uploads/webp-uploads-test.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/modules/images/webp-uploads/webp-uploads-test.php b/tests/modules/images/webp-uploads/webp-uploads-test.php index 8a68fab092..4e8fb85e5e 100644 --- a/tests/modules/images/webp-uploads/webp-uploads-test.php +++ b/tests/modules/images/webp-uploads/webp-uploads-test.php @@ -861,9 +861,11 @@ public function it_should_backup_the_sources_structure_alongside_the_full_size() public function it_should_restore_the_sources_array_from_the_backup_when_an_image_is_edited() { $attachment_id = $this->factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' ); $metadata = wp_get_attachment_metadata( $attachment_id ); - $editor = new WP_Image_Edit( $attachment_id ); + + $editor = new WP_Image_Edit( $attachment_id ); $editor->rotate_right()->save(); $this->assertTrue( $editor->success() ); + $backup_sources = get_post_meta( $attachment_id, '_wp_attachment_backup_sources', true ); $this->assertArrayHasKey( 'full-orig', $backup_sources ); $this->assertIsArray( $backup_sources['full-orig'] ); From 93e0d01298d3b1c49e5ac10b0b3e4571c3a70cfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Crisoforo=20Gaspar=20Hern=C3=A1ndez?= Date: Wed, 30 Mar 2022 10:49:24 -0600 Subject: [PATCH 20/25] Add white spaces on tests Co-authored-by: Eugene Manuilov --- tests/modules/images/webp-uploads/webp-uploads-test.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/modules/images/webp-uploads/webp-uploads-test.php b/tests/modules/images/webp-uploads/webp-uploads-test.php index 4e8fb85e5e..bcb6fb081c 100644 --- a/tests/modules/images/webp-uploads/webp-uploads-test.php +++ b/tests/modules/images/webp-uploads/webp-uploads-test.php @@ -1088,9 +1088,11 @@ public function it_should_save_populate_the_backup_sources_with_the_next_target( */ public function it_should_store_the_metadata_on_the_next_available_hash() { $attachment_id = $this->factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' ); - $editor = new WP_Image_Edit( $attachment_id ); + + $editor = new WP_Image_Edit( $attachment_id ); $editor->rotate_right()->save(); $this->assertTrue( $editor->success() ); + // Remove the filter to prevent the usage of the next target. remove_filter( 'wp_update_attachment_metadata', 'webp_uploads_update_attachment_metadata' ); $editor->rotate_right()->save(); From c165a11ef9507d29ad1c9c3fa56b727907ee1643 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Crisoforo=20Gaspar=20Hern=C3=A1ndez?= Date: Wed, 30 Mar 2022 10:54:07 -0600 Subject: [PATCH 21/25] Removal of `sanitize_text_field` due this value is not required. Co-authored-by: Eugene Manuilov --- modules/images/webp-uploads/load.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/images/webp-uploads/load.php b/modules/images/webp-uploads/load.php index 4a0ed12979..3e10e5246c 100644 --- a/modules/images/webp-uploads/load.php +++ b/modules/images/webp-uploads/load.php @@ -760,7 +760,7 @@ function webp_uploads_backup_sources( $attachment_id, $data ) { return $data; } - $target = isset( $_REQUEST['target'] ) ? sanitize_text_field( $_REQUEST['target'] ) : 'all'; + $target = isset( $_REQUEST['target'] ) ? $_REQUEST['target'] : 'all'; // When an edit to an image is only applied to a thumbnail there's nothing we need to back up. if ( 'thumbnail' === $target ) { From 7d96583fdece85d087b4aabda59c5086358ec3d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Crisoforo=20Gaspar=20Hern=C3=A1ndez?= Date: Wed, 30 Mar 2022 10:55:30 -0600 Subject: [PATCH 22/25] Run logic only if was executed when doing ajax. Co-authored-by: Eugene Manuilov --- modules/images/webp-uploads/load.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/modules/images/webp-uploads/load.php b/modules/images/webp-uploads/load.php index 3e10e5246c..6e88271680 100644 --- a/modules/images/webp-uploads/load.php +++ b/modules/images/webp-uploads/load.php @@ -720,6 +720,10 @@ function webp_uploads_update_rest_attachment( WP_REST_Response $response, WP_Pos */ function webp_uploads_update_attachment_metadata( $data, $attachment_id ) { + if ( ! doing_action( 'wp_ajax_image-editor' ) ) { + return; + } + $trace = debug_backtrace( DEBUG_BACKTRACE_IGNORE_ARGS, 10 ); foreach ( $trace as $element ) { From 7a113d31d3b5ff7ae23c754d31e8e28d805069bc Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Wed, 30 Mar 2022 12:06:31 -0600 Subject: [PATCH 23/25] Update code flow for more performant checks Make sure the code performns in a more structured way --- modules/images/webp-uploads/load.php | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/modules/images/webp-uploads/load.php b/modules/images/webp-uploads/load.php index 6e88271680..735297aedd 100644 --- a/modules/images/webp-uploads/load.php +++ b/modules/images/webp-uploads/load.php @@ -758,12 +758,6 @@ function webp_uploads_update_attachment_metadata( $data, $attachment_id ) { * @return array The updated metadata for the attachment. */ function webp_uploads_backup_sources( $attachment_id, $data ) { - $metadata = wp_get_attachment_metadata( $attachment_id ); - // Nothing to back up. - if ( ! isset( $metadata['sources'] ) ) { - return $data; - } - $target = isset( $_REQUEST['target'] ) ? $_REQUEST['target'] : 'all'; // When an edit to an image is only applied to a thumbnail there's nothing we need to back up. @@ -771,6 +765,12 @@ function webp_uploads_backup_sources( $attachment_id, $data ) { return $data; } + $metadata = wp_get_attachment_metadata( $attachment_id ); + // Nothing to back up. + if ( ! isset( $metadata['sources'] ) ) { + return $data; + } + $sources = $metadata['sources']; // Prevent execution of the callbacks more than once if the callback was already executed. $has_been_processed = false; @@ -814,9 +814,12 @@ function webp_uploads_backup_sources( $attachment_id, $data ) { * @param array $sources An array with the full sources to be stored on the next available key. */ function webp_uploads_backup_full_image_sources( $attachment_id, $sources ) { - $target = webp_uploads_get_next_full_size_key_from_backup( $attachment_id ); + if ( empty( $sources ) ) { + return; + } - if ( null === $target || empty( $sources ) ) { + $target = webp_uploads_get_next_full_size_key_from_backup( $attachment_id ); + if ( null === $target ) { return; } @@ -848,13 +851,13 @@ function webp_uploads_get_next_full_size_key_from_backup( $attachment_id ) { $backup_sources = get_post_meta( $attachment_id, '_wp_attachment_backup_sources', true ); $backup_sources = is_array( $backup_sources ) ? $backup_sources : array(); foreach ( array_keys( $backup_sizes ) as $size_name ) { - // We are only interested in the `full-` sizes. - if ( strpos( $size_name, 'full-' ) === false ) { + // If the target already has the sources attributes find the next one. + if ( isset( $backup_sources[ $size_name ] ) ) { continue; } - // If the target already has the sources attributes find the next one. - if ( isset( $backup_sources[ $size_name ] ) ) { + // We are only interested in the `full-` sizes. + if ( strpos( $size_name, 'full-' ) === false ) { continue; } @@ -884,7 +887,6 @@ function webp_uploads_restore_image( $attachment_id, $data ) { // TODO: Handle the case If `IMAGE_EDIT_OVERWRITE` is defined and is truthy remove any edited images if present before replacing the metadata. // See: https://github.com/WordPress/performance/issues/158. - $data['sources'] = $backup_sources['full-orig']; return $data; From e202548ef2aef815e8bd09f21fa4f9a1db94ee2c Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Wed, 30 Mar 2022 12:11:38 -0600 Subject: [PATCH 24/25] Split conditionals into separate statements --- modules/images/webp-uploads/load.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/modules/images/webp-uploads/load.php b/modules/images/webp-uploads/load.php index 735297aedd..a129637ac3 100644 --- a/modules/images/webp-uploads/load.php +++ b/modules/images/webp-uploads/load.php @@ -881,7 +881,11 @@ function webp_uploads_get_next_full_size_key_from_backup( $attachment_id ) { function webp_uploads_restore_image( $attachment_id, $data ) { $backup_sources = get_post_meta( $attachment_id, '_wp_attachment_backup_sources', true ); - if ( ! is_array( $backup_sources ) || ! isset( $backup_sources['full-orig'] ) || ! is_array( $backup_sources['full-orig'] ) ) { + if ( ! is_array( $backup_sources ) ) { + return $data; + } + + if ( ! isset( $backup_sources['full-orig'] ) || ! is_array( $backup_sources['full-orig'] ) ) { return $data; } From 188c7df084dac80504c939061b74b4e0f3f1c79c Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Wed, 30 Mar 2022 13:01:54 -0600 Subject: [PATCH 25/25] Removal of non requried conditional --- modules/images/webp-uploads/load.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/modules/images/webp-uploads/load.php b/modules/images/webp-uploads/load.php index a129637ac3..fe6d0a3354 100644 --- a/modules/images/webp-uploads/load.php +++ b/modules/images/webp-uploads/load.php @@ -719,11 +719,6 @@ function webp_uploads_update_rest_attachment( WP_REST_Response $response, WP_Pos * @return array The updated metadata for the attachment to be stored in the meta table. */ function webp_uploads_update_attachment_metadata( $data, $attachment_id ) { - - if ( ! doing_action( 'wp_ajax_image-editor' ) ) { - return; - } - $trace = debug_backtrace( DEBUG_BACKTRACE_IGNORE_ARGS, 10 ); foreach ( $trace as $element ) {