Skip to content

Commit

Permalink
Apply suggestion from PR
Browse files Browse the repository at this point in the history
  • Loading branch information
@dhaval-parekh
dhaval-parekh committed Jul 29, 2021
1 parent 4474983 commit cdaa764
Show file tree
Hide file tree
Showing 2 changed files with 120 additions and 57 deletions.
61 changes: 39 additions & 22 deletions includes/utils/class-amp-image-dimension-extractor.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,16 +174,13 @@ public static function extract_by_filename_or_filesystem( $dimensions ) {
continue;
}

// Get image dimension from file name.
$basename = basename( $url );
$filename_without_extension = explode( '.', $basename );
$extension = array_pop( $filename_without_extension );
$filename_without_extension = implode( '.', $filename_without_extension );

$regex = '/-(?<width>\d+)x(?<height>\d+)(?:\.' . $extension . ')$/m';
preg_match( $regex, $basename, $possible_dimension );
$path = wp_parse_url( $url, PHP_URL_PATH );
if ( empty( $path ) ) {
continue;
}

if ( ! empty( $possible_dimension['width'] ) && ! empty( $possible_dimension['height'] ) ) {
// Get image dimension from file name.
if ( preg_match( '/-(?<width>\d+)x(?<height>\d+)\.\w+$/', $path, $possible_dimension ) ) {
$dimensions[ $url ] = [
'width' => (int) $possible_dimension['width'],
'height' => (int) $possible_dimension['height'],
Expand All @@ -192,18 +189,21 @@ public static function extract_by_filename_or_filesystem( $dimensions ) {
}

// Check if it's internal media or not. If it's not then bail out.
if ( 0 !== strpos( trim( $url ), $base_upload_uri ) ) {
// If URL is not valid then bail out.
if ( 0 !== strpos( $url, $base_upload_uri ) || 0 !== validate_file( $url ) ) {
continue;
}

// Get media path.
$attached_path = ltrim( str_replace( $base_upload_uri, '', $url ), '/' );
$attached_path = substr( $url, strlen( $base_upload_uri ) );
$attached_path = wp_parse_url( $attached_path, PHP_URL_PATH );

$imagesize = [];
$url_hash = md5( $url );
$transient_name = sprintf( 'amp_img_%s', $url_hash );
$imagesize = [];
$is_using_object_cache = wp_using_ext_object_cache();

if ( wp_using_ext_object_cache() ) {
list( $transient_name ) = array_values( self::get_cached_dimensions_transient_name( $url ) );

if ( $is_using_object_cache ) {
$imagesize = get_transient( $transient_name );
$imagesize = ( ! empty( $imagesize ) && is_array( $imagesize ) ) ? $imagesize : [];
}
Expand All @@ -214,11 +214,11 @@ public static function extract_by_filename_or_filesystem( $dimensions ) {

if ( function_exists( 'wp_getimagesize' ) ) {
$imagesize = wp_getimagesize( $image_file );
} else {
$imagesize = getimagesize( $image_file );
} elseif ( function_exists( 'getimagesize' ) ) {
$imagesize = @getimagesize( $image_file ); // phpcs:ignore WordPress.PHP.NoSilencedErrors
}

if ( ! empty( $imagesize ) && is_array( $imagesize ) ) {
if ( $is_using_object_cache && ! empty( $imagesize ) && is_array( $imagesize ) ) {
set_transient( $transient_name, $imagesize );
}
}
Expand All @@ -234,6 +234,25 @@ public static function extract_by_filename_or_filesystem( $dimensions ) {
return $dimensions;
}

/**
* To get transient name for URL dimensions.
*
* @param string $url Image URL.
*
* @return array Transient name and lock name.
*/
public static function get_cached_dimensions_transient_name( $url ) {
$response = [];

if ( ! empty( $url ) ) {
$url_hash = md5( $url );
$response['transient_name'] = sprintf( 'amp_img_%s', $url_hash );
$response['transient_lock_name'] = sprintf( 'amp_lock_%s', $url_hash );
}

return $response;
}

/**
* Extract dimensions from downloaded images (or transient/cached dimensions from downloaded images)
*
Expand Down Expand Up @@ -278,8 +297,8 @@ private static function determine_which_images_to_fetch( &$dimensions, &$urls_to
continue;
}

$url_hash = md5( $url );
$transient_name = sprintf( 'amp_img_%s', $url_hash );
list( $transient_name, $transient_lock_name ) = array_values( self::get_cached_dimensions_transient_name( $url ) );

$cached_dimensions = get_transient( $transient_name );

// If we're able to retrieve the dimensions from a transient, set them and move on.
Expand All @@ -297,8 +316,6 @@ private static function determine_which_images_to_fetch( &$dimensions, &$urls_to
continue;
}

$transient_lock_name = sprintf( 'amp_lock_%s', $url_hash );

// If somebody is already trying to extract dimensions for this transient right now, move on.
if ( false !== get_transient( $transient_lock_name ) ) {
$dimensions[ $url ] = false;
Expand Down
116 changes: 81 additions & 35 deletions tests/php/test-amp-image-dimension-extractor.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,60 +185,106 @@ public function test__amp_wp_user_agent() {
$this->assertEquals( $expected, $user_agent );
}

/**
* @covers \AMP_Image_Dimension_Extractor::extract_by_filename_or_filesystem
*/
public function test_extract_by_filename_or_filesystem() {
public function get_data_for_test_extract_by_filename_or_filesystem() {

$attachment_id = $this->factory()->attachment->create_upload_object( __DIR__ . '/data/images/wordpress-logo.png' );

$full_image = wp_get_attachment_image_src( $attachment_id, 'full' );
$thumbnail_image = wp_get_attachment_image_src( $attachment_id, 'thumbnail' );
$external_image = 'https://example.com/wp-content/uploads/2021/04/American_bison_k5680-1-1024x668.jpg';
$external_image_1 = 'https://via.placeholder.com/1500/000.png/FF0';
$external_image_2 = 'https://via.placeholder.com/1000/000.png/FF0';
$full_image = wp_get_attachment_image_src( $attachment_id, 'full' );
$thumbnail_image = wp_get_attachment_image_src( $attachment_id, 'thumbnail' );
$external_image = 'https://example.com/wp-content/uploads/2021/04/American_bison_k5680-1-1024x668.jpg';
$external_image_1 = 'https://via.placeholder.com/1500/000.png/FF0';
$external_image_2 = 'https://via.placeholder.com/1000/000.png/FF0';
$image_with_query_string = 'https://example.com/wp-content/uploads/2021/04/American_bison_k5680-1-512x768.jpg?crop=1';
$internal_image_with_query_string = $full_image[0] . '?crop=1&resize=1';

$expected = [
$full_image[0] => [
'width' => $full_image[1],
'height' => $full_image[2],
return [
$full_image[0] => [
'input' => [],
'expected' => [
'width' => $full_image[1],
'height' => $full_image[2],
],
],
$thumbnail_image[0] => [
'width' => $thumbnail_image[1],
'height' => $thumbnail_image[2],
$thumbnail_image[0] => [
'input' => [],
'expected' => [
'width' => $thumbnail_image[1],
'height' => $thumbnail_image[2],
],
],
$external_image => [
'width' => 1024,
'height' => 668,
$external_image => [
'input' => [],
'expected' => [
'width' => 1024,
'height' => 668,
],
],
$external_image_1 => [],
$external_image_2 => [
'width' => 1000,
'height' => 1000,
$external_image_1 => [
'input' => [],
'expected' => [],
],
];

$input = [
$full_image[0] => [],
$thumbnail_image[0] => [],
$external_image => [],
$external_image_1 => [],
$external_image_2 => [
'width' => 1000,
'height' => 1000,
$external_image_2 => [
'input' => [
'width' => 1000,
'height' => 1000,
],
'expected' => [
'width' => 1000,
'height' => 1000,
],
],
$image_with_query_string => [
'input' => [],
'expected' => [
'width' => 512,
'height' => 768,
],
],
$internal_image_with_query_string => [
'input' => [],
'expected' => [
'width' => $full_image[1],
'height' => $full_image[2],
],
],
];
}

/**
* @covers \AMP_Image_Dimension_Extractor::extract_by_filename_or_filesystem()
*/
public function test_extract_by_filename_or_filesystem() {

$data = $this->get_data_for_test_extract_by_filename_or_filesystem();

$input = wp_list_pluck( $data, 'input' );
$expected = wp_list_pluck( $data, 'expected' );

$this->assertEmpty( AMP_Image_Dimension_Extractor::extract_by_filename_or_filesystem( [] ) );

$output = AMP_Image_Dimension_Extractor::extract_by_filename_or_filesystem( $input );
$this->assertEquals( $expected, $output );

// Check after removing attachment metadata, To check fallback to filesystem.
}

/**
* @covers \AMP_Image_Dimension_Extractor::extract_by_filename_or_filesystem()
*/
public function test_extract_by_filename_or_filesystem_with_object_cache() {

wp_using_ext_object_cache( true );

wp_update_attachment_metadata( $attachment_id, [] );


$data = $this->get_data_for_test_extract_by_filename_or_filesystem();

$input = wp_list_pluck( $data, 'input' );
$expected = wp_list_pluck( $data, 'expected' );

$this->assertEmpty( AMP_Image_Dimension_Extractor::extract_by_filename_or_filesystem( [] ) );

$output = AMP_Image_Dimension_Extractor::extract_by_filename_or_filesystem( $input );
$this->assertEquals( $expected, $output );

}
}

0 comments on commit cdaa764

Please sign in to comment.