From df1567cc0efbff1c3085b190eb3613cb4dbe6eba Mon Sep 17 00:00:00 2001 From: Mohammad Jangda Date: Fri, 3 Nov 2017 19:10:28 -0400 Subject: [PATCH 1/6] Support extracting dimensions for single URLs Back-compat for sites using the class directly and relying on it for extracting images for just a single URL. --- .../class-amp-image-dimension-extractor.php | 12 ++++++++++ tests/test-amp-image-dimension-extractor.php | 24 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/includes/utils/class-amp-image-dimension-extractor.php b/includes/utils/class-amp-image-dimension-extractor.php index 47a0b923c25..4720a006910 100644 --- a/includes/utils/class-amp-image-dimension-extractor.php +++ b/includes/utils/class-amp-image-dimension-extractor.php @@ -12,6 +12,12 @@ static public function extract( $urls ) { $return_dimensions = array(); + // Back-compat for users calling this method directly + $is_single = is_string( $urls ); + if ( $is_single ) { + $urls = array( $urls ); + } + // Normalize URLs and also track a map of normalized-to-original as we'll need it to reformat things when returning the data. $url_map = array(); $normalized_urls = array(); @@ -22,6 +28,7 @@ static public function extract( $urls ) { $normalized_urls[] = $normalized_url; } else { // This is not a URL we can extract dimensions from, so default to false. + $url_map[ $original_url ] = $original_url; $return_dimensions[ $original_url ] = false; } } @@ -35,6 +42,11 @@ static public function extract( $urls ) { $return_dimensions[ $original_url ] = $dimension; } + // Back-compat: just return the dimensions, not the full mapped array + if ( $is_single ) { + return current( $return_dimensions ); + } + return $return_dimensions; } diff --git a/tests/test-amp-image-dimension-extractor.php b/tests/test-amp-image-dimension-extractor.php index f1010ba618d..75c9af7ae57 100644 --- a/tests/test-amp-image-dimension-extractor.php +++ b/tests/test-amp-image-dimension-extractor.php @@ -20,6 +20,30 @@ public function disable_downloads() { remove_all_filters( 'amp_extract_image_dimensions_batch' ); } + public function single_url__add_mock_dimension_callback() { + add_filter( 'amp_extract_image_dimensions_batch', array( $this, 'single_url__mock_dimension_callback' ) ); + } + + public function single_url__mock_dimension_callback() { + return array( + 'https://example.com/image.png' => array( + 100, + 101, + ), + ); + } + + public function test__single_url() { + add_action( 'amp_extract_image_dimensions_batch_callbacks_registered', array( $this, 'single_url__add_mock_dimension_callback' ), 9999 ); // Run after the `disable_downloads` + + $source_url = 'https://example.com/image.png'; + $expected = array( 100, 101 ); + + $actual = AMP_Image_Dimension_Extractor::extract( $source_url ); + + $this->assertEquals( $expected, $actual ); + } + public function test__should_return_original_urls() { $source_urls = array( 'https://example.com', From a3d3204fb36ba1ef2d282e2dd3276ba5fac6b07e Mon Sep 17 00:00:00 2001 From: Matthew Denton Date: Thu, 31 May 2018 10:41:58 -0400 Subject: [PATCH 2/6] Update comments to include . to align with PHPCS standards. --- includes/utils/class-amp-image-dimension-extractor.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/utils/class-amp-image-dimension-extractor.php b/includes/utils/class-amp-image-dimension-extractor.php index 15160fe5678..900f6b4c571 100644 --- a/includes/utils/class-amp-image-dimension-extractor.php +++ b/includes/utils/class-amp-image-dimension-extractor.php @@ -12,7 +12,7 @@ static public function extract( $urls ) { $return_dimensions = array(); - // Back-compat for users calling this method directly + // Back-compat for users calling this method directly. $is_single = is_string( $urls ); if ( $is_single ) { $urls = array( $urls ); @@ -42,7 +42,7 @@ static public function extract( $urls ) { $return_dimensions[ $original_url ] = $dimension; } - // Back-compat: just return the dimensions, not the full mapped array + // Back-compat: just return the dimensions, not the full mapped array. if ( $is_single ) { return current( $return_dimensions ); } From 92bc88f381f54ca6a58b4f17f79e4820903e9053 Mon Sep 17 00:00:00 2001 From: Matthew Denton Date: Thu, 31 May 2018 10:51:55 -0400 Subject: [PATCH 3/6] Resolve Mutliple Statement Alignment PHPCS warning. --- includes/utils/class-amp-image-dimension-extractor.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/utils/class-amp-image-dimension-extractor.php b/includes/utils/class-amp-image-dimension-extractor.php index 900f6b4c571..d53d3b69820 100644 --- a/includes/utils/class-amp-image-dimension-extractor.php +++ b/includes/utils/class-amp-image-dimension-extractor.php @@ -28,7 +28,7 @@ static public function extract( $urls ) { $normalized_urls[] = $normalized_url; } else { // This is not a URL we can extract dimensions from, so default to false. - $url_map[ $original_url ] = $original_url; + $url_map[ $original_url ] = $original_url; $return_dimensions[ $original_url ] = false; } } From 32c80ef8c5cffb733f6467f267a23839f679ac43 Mon Sep 17 00:00:00 2001 From: Matthew Denton Date: Thu, 2 Aug 2018 09:39:53 -0400 Subject: [PATCH 4/6] Resolving PHPCS formatting issues. ## PHP_CodeSniffer Time: 1.08 secs; Memory: 16Mb tests/test-amp-image-dimension-extractor.php:39:12: error - Missing doc comment for function single_url__add_mock_dimension_callback() (Squiz.Commenting.FunctionComment.Missing) tests/test-amp-image-dimension-extractor.php:43:12: error - Missing doc comment for function single_url__mock_dimension_callback() (Squiz.Commenting.FunctionComment.Missing) tests/test-amp-image-dimension-extractor.php:52:12: error - Missing doc comment for function test__single_url() (Squiz.Commenting.FunctionComment.Missing) tests/test-amp-image-dimension-extractor.php:53:147: error - Inline comments must end in full-stops, exclamation marks, or question marks (Squiz.Commenting.InlineComment.InvalidEndChar) tests/test-amp-image-dimension-extractor.php:56:19: warning - Equals sign not aligned with surrounding assignments; expected 3 spaces but found 1 space (Generic.Formatting.MultipleStatementAlignment.NotSameWarning) --- tests/test-amp-image-dimension-extractor.php | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/test-amp-image-dimension-extractor.php b/tests/test-amp-image-dimension-extractor.php index a9ffffc46fc..22badb10ef1 100644 --- a/tests/test-amp-image-dimension-extractor.php +++ b/tests/test-amp-image-dimension-extractor.php @@ -36,10 +36,16 @@ public function disable_downloads() { remove_all_filters( 'amp_extract_image_dimensions_batch' ); } + /** + * Add Mock dimension callback to amp_extract_image_dimensions_batch filter. + */ public function single_url__add_mock_dimension_callback() { add_filter( 'amp_extract_image_dimensions_batch', array( $this, 'single_url__mock_dimension_callback' ) ); } + /** + * callback to return dimension array. + */ public function single_url__mock_dimension_callback() { return array( 'https://example.com/image.png' => array( @@ -49,11 +55,14 @@ public function single_url__mock_dimension_callback() { ); } + /** + * Test single url returns expected dimensions. + */ public function test__single_url() { - add_action( 'amp_extract_image_dimensions_batch_callbacks_registered', array( $this, 'single_url__add_mock_dimension_callback' ), 9999 ); // Run after the `disable_downloads` + add_action( 'amp_extract_image_dimensions_batch_callbacks_registered', array( $this, 'single_url__add_mock_dimension_callback' ), 9999 ); // Run after the `disable_downloads`. $source_url = 'https://example.com/image.png'; - $expected = array( 100, 101 ); + $expected = array( 100, 101 ); $actual = AMP_Image_Dimension_Extractor::extract( $source_url ); From b27690ea37540237afa29dc4f16685c5cca9b67d Mon Sep 17 00:00:00 2001 From: Matthew Denton Date: Thu, 2 Aug 2018 09:47:22 -0400 Subject: [PATCH 5/6] PHPCS Formatting tests/test-amp-image-dimension-extractor.php:47:8: error - Doc comment short description must start with a capital letter (Generic.Commenting.DocComment.ShortNotCapital) --- tests/test-amp-image-dimension-extractor.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test-amp-image-dimension-extractor.php b/tests/test-amp-image-dimension-extractor.php index 22badb10ef1..4cc4c4a0c28 100644 --- a/tests/test-amp-image-dimension-extractor.php +++ b/tests/test-amp-image-dimension-extractor.php @@ -44,7 +44,7 @@ public function single_url__add_mock_dimension_callback() { } /** - * callback to return dimension array. + * Callback to rdeturn dimension array. */ public function single_url__mock_dimension_callback() { return array( From cb30fca7fecbad11560c4a411e42b03473ccedc6 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 3 Aug 2018 08:36:58 -0700 Subject: [PATCH 6/6] Add phpdoc and use closures to consolidate test code --- .../class-amp-image-dimension-extractor.php | 10 +++++- tests/test-amp-image-dimension-extractor.php | 36 +++++++++---------- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/includes/utils/class-amp-image-dimension-extractor.php b/includes/utils/class-amp-image-dimension-extractor.php index 761188f6b5f..fc9fbe0cb2e 100644 --- a/includes/utils/class-amp-image-dimension-extractor.php +++ b/includes/utils/class-amp-image-dimension-extractor.php @@ -5,7 +5,15 @@ class AMP_Image_Dimension_Extractor { const STATUS_FAILED_LAST_ATTEMPT = 'failed'; const STATUS_IMAGE_EXTRACTION_FAILED = 'failed'; - static public function extract( $urls ) { + /** + * Extract dimensions from image URLs. + * + * @since 0.2 + * + * @param array|string $urls Array of URLs to extract dimensions from, or a single URL string. + * @return array|string Extracted dimensions keyed by original URL, or else the single set of dimensions if one URL string is passed. + */ + public static function extract( $urls ) { if ( ! self::$callbacks_registered ) { self::register_callbacks(); } diff --git a/tests/test-amp-image-dimension-extractor.php b/tests/test-amp-image-dimension-extractor.php index 4cc4c4a0c28..b8126a3cda5 100644 --- a/tests/test-amp-image-dimension-extractor.php +++ b/tests/test-amp-image-dimension-extractor.php @@ -36,30 +36,26 @@ public function disable_downloads() { remove_all_filters( 'amp_extract_image_dimensions_batch' ); } - /** - * Add Mock dimension callback to amp_extract_image_dimensions_batch filter. - */ - public function single_url__add_mock_dimension_callback() { - add_filter( 'amp_extract_image_dimensions_batch', array( $this, 'single_url__mock_dimension_callback' ) ); - } - - /** - * Callback to rdeturn dimension array. - */ - public function single_url__mock_dimension_callback() { - return array( - 'https://example.com/image.png' => array( - 100, - 101, - ), - ); - } - /** * Test single url returns expected dimensions. + * + * @covers \AMP_Image_Dimension_Extractor::extract() */ public function test__single_url() { - add_action( 'amp_extract_image_dimensions_batch_callbacks_registered', array( $this, 'single_url__add_mock_dimension_callback' ), 9999 ); // Run after the `disable_downloads`. + add_action( + 'amp_extract_image_dimensions_batch_callbacks_registered', + function() { + add_filter( 'amp_extract_image_dimensions_batch', function() { + return array( + 'https://example.com/image.png' => array( + 100, + 101, + ), + ); + } ); + }, + 9999 // Run after the `disable_downloads`. + ); $source_url = 'https://example.com/image.png'; $expected = array( 100, 101 );