From e1222c65f467fc88189aa4a79283f47fe75cb7f3 Mon Sep 17 00:00:00 2001 From: goldenapples Date: Thu, 18 Feb 2021 17:08:48 -0500 Subject: [PATCH 1/9] Save filesize with attachment metadata on upload Sets the "filesize" property in the attachment metadata when initially uploading images, so that it doesn't have to be queried for in wp_prepare_attachment_for_js() ajax response. --- inc/class-plugin.php | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/inc/class-plugin.php b/inc/class-plugin.php index 94342b2a..f57a7b0d 100644 --- a/inc/class-plugin.php +++ b/inc/class-plugin.php @@ -110,6 +110,7 @@ public function setup() { add_filter( 'wp_resource_hints', [ $this, 'wp_filter_resource_hints' ], 10, 2 ); add_action( 'wp_handle_sideload_prefilter', [ $this, 'filter_sideload_move_temp_file_to_s3' ] ); + add_filter( 'wp_generate_attachment_metadata', [ $this, 'set_filesize_in_attachment_meta' ] ); add_action( 'wp_get_attachment_url', [ $this, 'add_s3_signed_params_to_attachment_url' ], 10, 2 ); add_action( 'wp_get_attachment_image_src', [ $this, 'add_s3_signed_params_to_attachment_image_src' ], 10, 2 ); @@ -129,6 +130,7 @@ public function tear_down() { remove_filter( 'upload_dir', [ $this, 'filter_upload_dir' ] ); remove_filter( 'wp_image_editors', [ $this, 'filter_editors' ], 9 ); remove_filter( 'wp_handle_sideload_prefilter', [ $this, 'filter_sideload_move_temp_file_to_s3' ] ); + remove_filter( 'wp_generate_attachment_metadata', [ $this, 'set_filesize_in_attachment_meta' ] ); remove_action( 'wp_get_attachment_url', [ $this, 'add_s3_signed_params_to_attachment_url' ] ); remove_action( 'wp_get_attachment_image_src', [ $this, 'add_s3_signed_params_to_attachment_image_src' ] ); @@ -376,7 +378,6 @@ public function filter_editors( array $editors ) : array { return $editors; } - /** * Copy the file from /tmp to an s3 dir so handle_sideload doesn't fail due to * trying to do a rename() on the file cross streams. This is somewhat of a hack @@ -396,6 +397,33 @@ public function filter_sideload_move_temp_file_to_s3( array $file ) { return $file; } + /** + * Store the attachment filesize in the attachment meta array. + * + * Getting the filesize of an image in S3 involves a remote HEAD request, + * which is a bit slower than a local filesystem operation would be. As a + * result, operations like `wp_prepare_attachments_for_js' take substantially + * longer to complete against s3 uploads than if they were performed with a + * local filesystem.i + * + * Saving the filesize in the attachment metadata when the image is + * uploaded allows core to skip this stat when retrieving and formatting it. + * + * @param array $metadata Attachment metadata. + * @return array Attachment metadata array, with "filesize" value added. + */ + function set_filesize_in_attachment_meta( $metadata ) { + $uploads_dir = wp_upload_dir(); + + $file = path_join( $uploads_dir['basedir'], $metadata['file'] ); + + if ( file_exists( $file ) ) { + $metadata['filesize'] = filesize( $file ); + } + + return $metadata; + } + /** * Filters wp_read_image_metadata. exif_read_data() doesn't work on * file streams so we need to make a temporary local copy to extract From fe01c24c1c5ff6f2f48dc1574bf28b7e67baa303 Mon Sep 17 00:00:00 2001 From: goldenapples Date: Fri, 19 Feb 2021 11:46:26 -0500 Subject: [PATCH 2/9] Add type hinting to function signature --- inc/class-plugin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/inc/class-plugin.php b/inc/class-plugin.php index f57a7b0d..87105e26 100644 --- a/inc/class-plugin.php +++ b/inc/class-plugin.php @@ -412,7 +412,7 @@ public function filter_sideload_move_temp_file_to_s3( array $file ) { * @param array $metadata Attachment metadata. * @return array Attachment metadata array, with "filesize" value added. */ - function set_filesize_in_attachment_meta( $metadata ) { + function set_filesize_in_attachment_meta( array $metadata ) : array { $uploads_dir = wp_upload_dir(); $file = path_join( $uploads_dir['basedir'], $metadata['file'] ); From ce82bc4e1667f9dd06237c2343683237955bc170 Mon Sep 17 00:00:00 2001 From: goldenapples Date: Fri, 19 Feb 2021 11:51:16 -0500 Subject: [PATCH 3/9] Get attached file with get_attached_file The attachment metadata array doesn't always have a "file" attribute. This uses the core API for getting the attached file rather than building it from the file path and the upload dir. --- inc/class-plugin.php | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/inc/class-plugin.php b/inc/class-plugin.php index 87105e26..dc39b9e1 100644 --- a/inc/class-plugin.php +++ b/inc/class-plugin.php @@ -110,7 +110,7 @@ public function setup() { add_filter( 'wp_resource_hints', [ $this, 'wp_filter_resource_hints' ], 10, 2 ); add_action( 'wp_handle_sideload_prefilter', [ $this, 'filter_sideload_move_temp_file_to_s3' ] ); - add_filter( 'wp_generate_attachment_metadata', [ $this, 'set_filesize_in_attachment_meta' ] ); + add_filter( 'wp_generate_attachment_metadata', [ $this, 'set_filesize_in_attachment_meta' ], 10, 2 ); add_action( 'wp_get_attachment_url', [ $this, 'add_s3_signed_params_to_attachment_url' ], 10, 2 ); add_action( 'wp_get_attachment_image_src', [ $this, 'add_s3_signed_params_to_attachment_image_src' ], 10, 2 ); @@ -409,13 +409,12 @@ public function filter_sideload_move_temp_file_to_s3( array $file ) { * Saving the filesize in the attachment metadata when the image is * uploaded allows core to skip this stat when retrieving and formatting it. * - * @param array $metadata Attachment metadata. + * @param array $metadata Attachment metadata. + * @param int $attachment_id Attachment ID. * @return array Attachment metadata array, with "filesize" value added. */ - function set_filesize_in_attachment_meta( array $metadata ) : array { - $uploads_dir = wp_upload_dir(); - - $file = path_join( $uploads_dir['basedir'], $metadata['file'] ); + function set_filesize_in_attachment_meta( array $metadata, int $attachment_id ) : array { + $file = get_attached_file( $attachment_id ); if ( file_exists( $file ) ) { $metadata['filesize'] = filesize( $file ); From 4ff03c148127ea23d08dfe70c39973f9b0752935 Mon Sep 17 00:00:00 2001 From: goldenapples Date: Fri, 19 Feb 2021 12:10:57 -0500 Subject: [PATCH 4/9] Add array shape description for psalm --- inc/class-plugin.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/inc/class-plugin.php b/inc/class-plugin.php index dc39b9e1..d42309a5 100644 --- a/inc/class-plugin.php +++ b/inc/class-plugin.php @@ -409,9 +409,9 @@ public function filter_sideload_move_temp_file_to_s3( array $file ) { * Saving the filesize in the attachment metadata when the image is * uploaded allows core to skip this stat when retrieving and formatting it. * - * @param array $metadata Attachment metadata. + * @param array{width: int, height: int, file?: string, sizes: array, image_meta: array} $metadata Attachment metadata. * @param int $attachment_id Attachment ID. - * @return array Attachment metadata array, with "filesize" value added. + * @return array{width: int, height: int, file?: string, sizes: array, image_meta: array, filesize: int} Attachment metadata array, with "filesize" value added. */ function set_filesize_in_attachment_meta( array $metadata, int $attachment_id ) : array { $file = get_attached_file( $attachment_id ); From 018a780c4af7c4af14dcc4c0bfccb8e15ce35199 Mon Sep 17 00:00:00 2001 From: goldenapples Date: Fri, 19 Feb 2021 12:15:28 -0500 Subject: [PATCH 5/9] Remove bonus space --- inc/class-plugin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/inc/class-plugin.php b/inc/class-plugin.php index d42309a5..41796cb6 100644 --- a/inc/class-plugin.php +++ b/inc/class-plugin.php @@ -413,7 +413,7 @@ public function filter_sideload_move_temp_file_to_s3( array $file ) { * @param int $attachment_id Attachment ID. * @return array{width: int, height: int, file?: string, sizes: array, image_meta: array, filesize: int} Attachment metadata array, with "filesize" value added. */ - function set_filesize_in_attachment_meta( array $metadata, int $attachment_id ) : array { + function set_filesize_in_attachment_meta( array $metadata, int $attachment_id ) : array { $file = get_attached_file( $attachment_id ); if ( file_exists( $file ) ) { From 7564fbc3afb30dfe7861efaf4fe581675c812655 Mon Sep 17 00:00:00 2001 From: goldenapples Date: Fri, 19 Feb 2021 12:30:03 -0500 Subject: [PATCH 6/9] Only annotate properties actually used This annotation is too damn long. Cutting out the keys which aren't relevant so that we don't have to define types for all of them. --- inc/class-plugin.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/inc/class-plugin.php b/inc/class-plugin.php index 41796cb6..5096c94a 100644 --- a/inc/class-plugin.php +++ b/inc/class-plugin.php @@ -409,9 +409,9 @@ public function filter_sideload_move_temp_file_to_s3( array $file ) { * Saving the filesize in the attachment metadata when the image is * uploaded allows core to skip this stat when retrieving and formatting it. * - * @param array{width: int, height: int, file?: string, sizes: array, image_meta: array} $metadata Attachment metadata. - * @param int $attachment_id Attachment ID. - * @return array{width: int, height: int, file?: string, sizes: array, image_meta: array, filesize: int} Attachment metadata array, with "filesize" value added. + * @param array{file?: string} $metadata Attachment metadata. + * @param int $attachment_id Attachment ID. + * @return array{file?: string, filesize: int} Attachment metadata array, with "filesize" value added. */ function set_filesize_in_attachment_meta( array $metadata, int $attachment_id ) : array { $file = get_attached_file( $attachment_id ); From 4c50bf1117ef12dbefb9b504d7279820b3ced085 Mon Sep 17 00:00:00 2001 From: goldenapples Date: Fri, 19 Feb 2021 12:37:36 -0500 Subject: [PATCH 7/9] Don't restat file if filesize is already in metadata Forward compatability check in case WP included this property in the future, see https://core.trac.wordpress.org/ticket/49412. --- inc/class-plugin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/inc/class-plugin.php b/inc/class-plugin.php index 5096c94a..b5148530 100644 --- a/inc/class-plugin.php +++ b/inc/class-plugin.php @@ -416,7 +416,7 @@ public function filter_sideload_move_temp_file_to_s3( array $file ) { function set_filesize_in_attachment_meta( array $metadata, int $attachment_id ) : array { $file = get_attached_file( $attachment_id ); - if ( file_exists( $file ) ) { + if ( ! isset( $metadata['filesize'] ) && file_exists( $file ) ) { $metadata['filesize'] = filesize( $file ); } From 069cab852955a1e09a57bc65d31a48cada650d6b Mon Sep 17 00:00:00 2001 From: goldenapples Date: Fri, 19 Feb 2021 12:38:14 -0500 Subject: [PATCH 8/9] Mark filesize property as optional. --- inc/class-plugin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/inc/class-plugin.php b/inc/class-plugin.php index b5148530..6c3c2b81 100644 --- a/inc/class-plugin.php +++ b/inc/class-plugin.php @@ -411,7 +411,7 @@ public function filter_sideload_move_temp_file_to_s3( array $file ) { * * @param array{file?: string} $metadata Attachment metadata. * @param int $attachment_id Attachment ID. - * @return array{file?: string, filesize: int} Attachment metadata array, with "filesize" value added. + * @return array{file?: string, filesize?: int} Attachment metadata array, with "filesize" value added. */ function set_filesize_in_attachment_meta( array $metadata, int $attachment_id ) : array { $file = get_attached_file( $attachment_id ); From 20d54076437f1cb7724acd60caae323b825afd41 Mon Sep 17 00:00:00 2001 From: Joe Hoyle Date: Fri, 19 Feb 2021 20:40:46 +0100 Subject: [PATCH 9/9] Check possible false value from get_attached_file() --- inc/class-plugin.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/inc/class-plugin.php b/inc/class-plugin.php index 6c3c2b81..61671683 100644 --- a/inc/class-plugin.php +++ b/inc/class-plugin.php @@ -415,7 +415,9 @@ public function filter_sideload_move_temp_file_to_s3( array $file ) { */ function set_filesize_in_attachment_meta( array $metadata, int $attachment_id ) : array { $file = get_attached_file( $attachment_id ); - + if ( ! $file ) { + return $metadata; + } if ( ! isset( $metadata['filesize'] ) && file_exists( $file ) ) { $metadata['filesize'] = filesize( $file ); }