Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce filter webp_uploads_pre_generate_additional_image_source to short-circuit generating additional image sources on upload #318

Merged
merged 20 commits into from
May 12, 2022
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
c58ffee
introduce webp_uploads_pre_generate_additional_image_source filter to…
mehulkaklotar Apr 27, 2022
dbce8ce
filter webp_uploads_pre_generate_additional_image_source description …
mehulkaklotar Apr 27, 2022
b6235ed
Merge branch 'trunk' into enhancement/additional-image-souce-filter
mehulkaklotar Apr 29, 2022
6639804
filter default image as null - feedback changes
mehulkaklotar Apr 29, 2022
2160d42
test case for filter webp_uploads_pre_generate_additional_image_sourc…
mehulkaklotar Apr 29, 2022
2ab4be9
Update modules/images/webp-uploads/helper.php
mehulkaklotar May 3, 2022
389d7cb
Update modules/images/webp-uploads/helper.php
mehulkaklotar May 3, 2022
2839ad3
Update modules/images/webp-uploads/helper.php
mehulkaklotar May 3, 2022
2efa53a
php unit test case - remove all filters before test
mehulkaklotar May 3, 2022
071ebd9
Update modules/images/webp-uploads/helper.php
mehulkaklotar May 10, 2022
2583b34
check for wp_error and validation added for filter returned image data
mehulkaklotar May 11, 2022
2b73597
test cases for wp_error and invalid data added for filter webp_upload…
mehulkaklotar May 11, 2022
11f24b1
code reformat changes
mehulkaklotar May 11, 2022
eaa4a41
Merge branch 'trunk' into enhancement/additional-image-souce-filter
mehulkaklotar May 11, 2022
ea6bfd0
revert spacing issues and since return jumbles
mehulkaklotar May 11, 2022
f23fc57
revert spacing issues and since return jumbles
mehulkaklotar May 11, 2022
2b629fc
removed conditions for filter returned invalid data - pr feedback
mehulkaklotar May 11, 2022
39d44c0
image size parameter in the filter added
mehulkaklotar May 11, 2022
37eabf6
full image size as default for image generation fixed
mehulkaklotar May 12, 2022
2419892
Update modules/images/webp-uploads/helper.php
mehulkaklotar May 12, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 39 additions & 0 deletions modules/images/webp-uploads/helper.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,45 @@ function webp_uploads_get_upload_image_mime_transforms() {
* @return array|WP_Error An array with the file and filesize if the image was created correctly otherwise a WP_Error
*/
function webp_uploads_generate_additional_image_source( $attachment_id, array $size_data, $mime, $destination_file_name = null ) {

/**
* Filter to allow the generation of additional image sources, in which a defined mime type
* can be transformed and create additional mime types for the file.
*
mehulkaklotar marked this conversation as resolved.
Show resolved Hide resolved
* Returning an image data array or WP_Error here effectively short-circuits the default logic to generate the image source.
*
* @since n.e.x.t
*
* @param array|null|WP_Error $image Image data {'path'=>string, 'file'=>string, 'width'=>int, 'height'=>int, 'mime-type'=>string} or null or WP_Error.
* @param int $attachment_id The ID of the attachment from where this image would be created.
* @param string $image_size The size name that would be used to create this image, out of the registered subsizes.
* @param array $size_data An array with the dimensions of the image: height, width and crop {'height'=>int, 'width'=>int, 'crop'}.
* @param string $mime The target mime in which the image should be created.
*
mehulkaklotar marked this conversation as resolved.
Show resolved Hide resolved
* @return array|null|WP_Error An array with the file and filesize if the image was created correctly otherwise a WP_Error
*/
$image = apply_filters( 'webp_uploads_pre_generate_additional_image_source', null, $attachment_id, 'full', $size_data, $mime );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mehulkaklotar

so are we going to make bydefault full for all image size in filter? because it doesn't really make sense as I tested it and it turns out all image size data is being worked on in that.

Ahh, sorry I had totally missed this. Yes, this absolutely shouldn't show full for any image size. We will need to provide the name of the image size where the $size_data comes from. We don't have access to that currently from this function, so let's add a new function parameter $size_name right before $size_data.

We then need to pass this new parameter accordingly everywhere that webp_uploads_generate_additional_image_source() is called. Can you please make that update here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added changes for this in the latest commit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mehulkaklotar. Please review my new feedback in #318 (review) - part of your new implementation isn't needed, as the one function by definition is only used for the full image size, so we don't need to try to find the matching size, it's always full in that scenario.


// Try to check for not empty returned filtered image.
if ( ! empty( $image ) ) {
felixarntz marked this conversation as resolved.
Show resolved Hide resolved
if ( is_wp_error( $image ) ) {
return $image;
}

if ( empty( $image['file'] ) ) {
return new WP_Error( 'image_file_not_present', __( 'The file key is not present on the image data', 'performance-lab' ) );
}

if ( empty( $image['path'] ) || ! file_exists( $image['path'] ) ) {
return new WP_Error( 'image_path_not_present', __( 'The file path is not present on the image data', 'performance-lab' ) );
}
felixarntz marked this conversation as resolved.
Show resolved Hide resolved

return array(
'file' => $image['file'],
'filesize' => filesize( $image['path'] ),
);
}

$allowed_mimes = array_flip( wp_get_mime_types() );
if ( ! isset( $allowed_mimes[ $mime ] ) || ! is_string( $allowed_mimes[ $mime ] ) ) {
return new WP_Error( 'image_mime_type_invalid', __( 'The provided mime type is not allowed.', 'performance-lab' ) );
Expand Down
125 changes: 125 additions & 0 deletions tests/modules/images/webp-uploads/helper-tests.php
Original file line number Diff line number Diff line change
Expand Up @@ -231,4 +231,129 @@ function () {
$this->assertWPError( $result );
$this->assertSame( 'image_mime_type_not_supported', $result->get_error_code() );
}

/**
* Create an image with the filter webp_uploads_pre_generate_additional_image_source added.
*
* @test
*/
public function it_should_create_an_image_with_filter_webp_uploads_pre_generate_additional_image_source() {
remove_all_filters( 'webp_uploads_pre_generate_additional_image_source' );

$attachment_id = $this->factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/car.jpeg' );

add_filter(
eugene-manuilov marked this conversation as resolved.
Show resolved Hide resolved
'webp_uploads_pre_generate_additional_image_source',
function () {
return array(
'file' => 'image.webp',
'path' => '/tmp/image.webp',
);
}
);

$size_data = array(
'width' => 300,
'height' => 300,
'crop' => true,
);

$result = webp_uploads_generate_additional_image_source( $attachment_id, $size_data, 'image/webp', '/tmp/image.jpg' );

$this->assertIsArray( $result );
$this->assertArrayHasKey( 'filesize', $result );
$this->assertArrayHasKey( 'file', $result );
$this->assertStringEndsWith( 'image.webp', $result['file'] );
$this->assertFileExists( '/tmp/image.webp' );
}

/**
* Return an error when filter webp_uploads_pre_generate_additional_image_source returns invalid data.
*
* @test
*/
public function it_should_return_an_error_when_filter_webp_uploads_pre_generate_additional_image_source_returns_invalid_data() {
remove_all_filters( 'webp_uploads_pre_generate_additional_image_source' );

$attachment_id = $this->factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/car.jpeg' );

add_filter(
'webp_uploads_pre_generate_additional_image_source',
function () {
return array(
'file' => '',
'path' => '',
);
}
);

$size_data = array(
'width' => 300,
'height' => 300,
'crop' => true,
);

$result = webp_uploads_generate_additional_image_source( $attachment_id, $size_data, 'image/webp', '/tmp/image.jpg' );
$this->assertWPError( $result );
$this->assertSame( 'image_file_not_present', $result->get_error_code() );
}

/**
* Return an error when filter webp_uploads_pre_generate_additional_image_source returns invalid file path.
*
* @test
*/
public function it_should_return_an_error_when_filter_webp_uploads_pre_generate_additional_image_source_returns_invalid_path() {
remove_all_filters( 'webp_uploads_pre_generate_additional_image_source' );

$attachment_id = $this->factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/car.jpeg' );

add_filter(
'webp_uploads_pre_generate_additional_image_source',
function () {
return array(
'file' => 'image.webp',
'path' => '',
);
}
);

$size_data = array(
'width' => 300,
'height' => 300,
'crop' => true,
);

$result = webp_uploads_generate_additional_image_source( $attachment_id, $size_data, 'image/webp', '/tmp/image.jpg' );
$this->assertWPError( $result );
$this->assertSame( 'image_path_not_present', $result->get_error_code() );
}

/**
* Return an error when filter webp_uploads_pre_generate_additional_image_source returns WP_Error.
*
* @test
*/
public function it_should_return_an_error_when_filter_webp_uploads_pre_generate_additional_image_source_returns_wp_error() {
remove_all_filters( 'webp_uploads_pre_generate_additional_image_source' );

$attachment_id = $this->factory->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/car.jpeg' );

add_filter(
'webp_uploads_pre_generate_additional_image_source',
function () {
return new WP_Error( 'image_additional_generated_error', __( 'Additional image was not generated.', 'performance-lab' ) );
}
);

$size_data = array(
'width' => 300,
'height' => 300,
'crop' => true,
);

$result = webp_uploads_generate_additional_image_source( $attachment_id, $size_data, 'image/webp', '/tmp/image.jpg' );
$this->assertWPError( $result );
$this->assertSame( 'image_additional_generated_error', $result->get_error_code() );
}
}