Merge pull request #919 from alleyinteractive/fix/APPLE-121/bundle-re…

…mote

APPLE-121 Fix issue with bundling images with same name

apple-news.php

@@ -14,7 +14,7 @@
  * Plugin Name: Publish to Apple News
  * Plugin URI:  http://github.com/alleyinteractive/apple-news
  * Description: Export and sync posts to Apple format.
- * Version:     2.3.0
+ * Version:     2.3.1
  * Author:      Alley
  * Author URI:  https://alley.co
  * Text Domain: apple-news

includes/class-apple-news.php

@@ -17,6 +17,13 @@
  */
 class Apple_News {
 
+	/**
+	 * An array of bundle hashes that match an asset URL to a bundle filename.
+	 *
+	 * @var array
+	 */
+	private static $bundle_hashes = [];
+
 	/**
 	 * Link to support for the plugin on github.
 	 *
@@ -39,7 +46,7 @@ class Apple_News {
 	 * @var string
 	 * @access public
 	 */
-	public static $version = '2.3.0';
+	public static $version = '2.3.1';
 
 	/**
 	 * Link to support for the plugin on WordPress.org.
@@ -193,14 +200,51 @@ public static function get_capability_for_post_type( $capability, $post_type ) {
 	 */
 	public static function get_filename( $path ) {
 
+		// If we already have a hash for this path, return it.
+		if ( isset( self::$bundle_hashes[ $path ] ) ) {
+			return self::$bundle_hashes[ $path ];
+		}
+
 		// Remove any URL parameters.
 		// This is important for sites using WordPress VIP or Jetpack Photon.
 		$url_parts = wp_parse_url( $path );
 		if ( empty( $url_parts['path'] ) ) {
 			return '';
 		}
 
-		return str_replace( ' ', '', basename( $url_parts['path'] ) );
+		// Compute base filename.
+		$filename = str_replace( ' ', '', basename( $url_parts['path'] ) );
+
+		// Ensure there are no filename collisions with existing bundles.
+		$bundle_filenames = array_values( self::$bundle_hashes );
+		sort( $bundle_filenames );
+		if ( in_array( $filename, $bundle_filenames, true ) ) {
+			$file_number    = 1;
+			$filename_parts = pathinfo( $filename );
+			$pattern        = sprintf(
+				'/^%s-([0-9]+)\.%s$/',
+				preg_quote( $filename_parts['filename'], '/' ),
+				preg_quote( $filename_parts['extension'], '/' )
+			);
+			foreach ( self::$bundle_hashes as $bundle_filename ) {
+				if ( preg_match( $pattern, $bundle_filename, $matches ) ) {
+					$file_number = max( $file_number, (int) $matches[1] + 1 );
+				}
+			}
+
+			// Apply the new filename to avoid collisions.
+			$filename = sprintf(
+				'%s-%d.%s',
+				$filename_parts['filename'],
+				$file_number,
+				$filename_parts['extension']
+			);
+		}
+
+		// Store this path/filename pair in the bundle hashes property for future use.
+		self::$bundle_hashes[ $path ] = $filename;
+
+		return $filename;
 	}
 
 	/**

package.json

@@ -1,6 +1,6 @@
 {
   "name": "publish-to-apple-news",
-  "version": "2.3.0",
+  "version": "2.3.1",
   "license": "GPLv3",
   "main": "index.php",
   "engines": {

readme.txt

@@ -5,7 +5,7 @@ Tags: publish, apple, news, iOS
 Requires at least: 4.0
 Tested up to: 5.8
 Requires PHP: 5.6
-Stable tag: 2.3.0
+Stable tag: 2.3.1
 License: GPLv3 or later
 License URI: https://www.gnu.org/licenses/gpl.html
 
@@ -46,6 +46,9 @@ Please visit our [wiki](https://github.com/alleyinteractive/apple-news/wiki) for
 
 == Changelog ==
 
+= 2.3.1 =
+* Bugfix: Fixes an issue where images with different URLs but the same filename are bundled with the same name when not using remote images, which can lead to images appearing out of order.
+
 = 2.3.0 =
 * Bugfix: Fixes an issue with some of the example themes where pullquotes would create invalid JSON due to the default-pullquote textStyle not being set. Props to @soulseekah for the fix.
 * Bugfix: Fixes an issue where a custom filter is used to make all image URLs root-relative when using featured images to populate the Cover component, which was leading to an INVALID_DOCUMENT error from the News API due to the root-relative URL (e.g., /path/to/my/image.jpg instead of https://example.org/path/to/my/image.jpg).

tests/admin/test-class-admin-apple-themes.php

@@ -430,7 +430,7 @@ public function testJSONSaveValidTokens() {
 		$json = <<<JSON
 {
     "role": "audio",
-    "URL": "http://someurl.com",
+    "URL": "https://www.example.org",
     "style": {
         "backgroundColor": "#body_background_color#"
     }

tests/apple-exporter/builders/test-class-components.php

@@ -24,32 +24,32 @@ public function dataImageFullSizeUrl() {
 		return [
 			// An image without crops should return itself.
 			[
-				'https://example.org/wp-content/uploads/2020/07/image.jpg',
-				'https://example.org/wp-content/uploads/2020/07/image.jpg',
+				'https://www.example.org/wp-content/uploads/2020/07/sample-image.jpg',
+				'https://www.example.org/wp-content/uploads/2020/07/sample-image.jpg',
 			],
 
 			// An image with a crop should return the original image without the crop.
 			[
-				'https://example.org/wp-content/uploads/2020/07/image-150x150.jpg',
-				'https://example.org/wp-content/uploads/2020/07/image.jpg',
+				'https://www.example.org/wp-content/uploads/2020/07/sample-image-150x150.jpg',
+				'https://www.example.org/wp-content/uploads/2020/07/sample-image.jpg',
 			],
 
 			// Scaled images should return the un-scaled version.
 			[
-				'https://example.org/wp-content/uploads/2020/07/image-scaled.jpg',
-				'https://example.org/wp-content/uploads/2020/07/image.jpg',
+				'https://www.example.org/wp-content/uploads/2020/07/sample-image-scaled.jpg',
+				'https://www.example.org/wp-content/uploads/2020/07/sample-image.jpg',
 			],
 
 			// Rotated images should return the un-rotated version.
 			[
-				'https://example.org/wp-content/uploads/2020/07/image-rotated.jpg',
-				'https://example.org/wp-content/uploads/2020/07/image.jpg',
+				'https://www.example.org/wp-content/uploads/2020/07/sample-image-rotated.jpg',
+				'https://www.example.org/wp-content/uploads/2020/07/sample-image.jpg',
 			],
 
 			// Photon images should return the original.
 			[
-				'https://i1.wp.com/example.org/wp-content/uploads/2020/07/image.jpg?w=234&crop=0%2C5px%2C100%2C134px&ssl=1',
-				'https://example.org/wp-content/uploads/2020/07/image.jpg',
+				'https://i1.wp.com/www.example.org/wp-content/uploads/2020/07/sample-image.jpg?w=234&crop=0%2C5px%2C100%2C134px&ssl=1',
+				'https://www.example.org/wp-content/uploads/2020/07/sample-image.jpg',
 			],
 		];
 	}
@@ -272,6 +272,42 @@ public function testGetImageFullSizeUrl( $original, $expected ) {
 		$this->assertEquals( $expected, $method->invokeArgs( $builder, [ $original ] ) );
 	}
 
+	/**
+	 * Tests the functionality of the maybe_bundle_source function.
+	 */
+	public function testImageBundling() {
+		// Ensure remote images are turned off for this test.
+		$use_remote_images                 = $this->settings->use_remote_images;
+		$this->settings->use_remote_images = 'no';
+
+		// Make a post with multiple images with the same filename.
+		$post_content = <<<HTML
+<!-- wp:image {"sizeSlug":"large"} -->
+<figure class="wp-block-image size-large"><img src="https://www.example.org/wp-content/2021/12/filename.jpg" alt="Sample Image 1"/></figure>
+<!-- /wp:image -->
+
+<!-- wp:image {"sizeSlug":"large"} -->
+<figure class="wp-block-image size-large"><img src="https://www.example.org/wp-content/2021/11/filename.jpg" alt="Sample Image 2"/></figure>
+<!-- /wp:image -->
+
+<!-- wp:image {"sizeSlug":"large"} -->
+<figure class="wp-block-image size-large"><img src="https://www.example.org/wp-content/2021/10/filename.jpg" alt="Sample Image 3"/></figure>
+<!-- /wp:image -->
+HTML;
+		$post_id      = self::factory()->post->create( [ 'post_content' => $post_content ] );
+		$image        = $this->get_new_attachment( $post_id );
+		set_post_thumbnail( $post_id, $image );
+		$json = $this->get_json_for_post( $post_id );
+
+		// Reset the use remote images setting.
+		$this->settings->use_remote_images = $use_remote_images;
+
+		// Ensure the images are saved with different bundle filenames.
+		$this->assertEquals( 'bundle://filename.jpg', $json['components'][1]['components'][3]['URL'] );
+		$this->assertEquals( 'bundle://filename-1.jpg', $json['components'][1]['components'][4]['URL'] );
+		$this->assertEquals( 'bundle://filename-2.jpg', $json['components'][1]['components'][5]['URL'] );
+	}
+
 	/**
 	 * Ensures that the specified component order is respected.
 	 *

tests/apple-exporter/builders/test-class-metadata.php

@@ -22,12 +22,12 @@ class Metadata_Test extends Apple_News_Testcase {
 	public function data_video() {
 		return [
 			[
-				'https://example.com/wp-content/uploads/2017/02/example-poster.jpg',
-				'https://example.com/wp-content/uploads/2017/02/example-video.mp4',
+				'https://www.example.org/wp-content/uploads/2017/02/example-poster.jpg',
+				'https://www.example.org/wp-content/uploads/2017/02/example-video.mp4',
 			],
 			[
-				'https://example.com/wp-content/uploads/2017/02/example-poster.jpg',
-				'https://example.com/wp-content/uploads/2017/02/example-video.m3u8',
+				'https://www.example.org/wp-content/uploads/2017/02/example-poster.jpg',
+				'https://www.example.org/wp-content/uploads/2017/02/example-video.m3u8',
 			],
 		];
 	}

tests/apple-exporter/components/test-class-audio.php

@@ -22,7 +22,7 @@ class Audio_Test extends Component_TestCase {
 	 */
 	public function testGeneratedJSON() {
 		$component = new Audio(
-			'<audio><source src="http://someurl.com/audio-file.mp3?some_query=string"></audio>',
+			'<audio><source src="https://www.example.org/audio-file.mp3?some_query=string"></audio>',
 			$this->workspace,
 			$this->settings,
 			$this->styles,
@@ -31,15 +31,15 @@ public function testGeneratedJSON() {
 
 		$json = $component->to_array();
 		$this->assertEquals( 'audio', $json['role'] );
-		$this->assertEquals( 'http://someurl.com/audio-file.mp3?some_query=string', $json['URL'] );
+		$this->assertEquals( 'https://www.example.org/audio-file.mp3?some_query=string', $json['URL'] );
 	}
 
 	/**
 	 * Tests HTML formatting with captions.
 	 */
 	public function testCaption() {
 		$component = new Audio(
-			'<figure class="wp-block-audio"><audio controls="" src="https://www.someurl.com/Song-1.mp3"/><figcaption>caption</figcaption></figure>',
+			'<figure class="wp-block-audio"><audio controls="" src="https://www.example.org/Song-1.mp3"/><figcaption>caption</figcaption></figure>',
 			$this->workspace,
 			$this->settings,
 			$this->styles,
@@ -53,7 +53,7 @@ public function testCaption() {
 				'components' => array(
 					array(
 						'role' => 'audio',
-						'URL' => 'https://www.someurl.com/Song-1.mp3',
+						'URL' => 'https://www.example.org/Song-1.mp3',
 					),
 					array(
 						'role' => 'caption',
@@ -71,7 +71,7 @@ public function testCaption() {
 	 */
 	public function testFilter() {
 		$component = new Audio(
-			'<audio><source src="http://someurl.com/audio-file.mp3?some_query=string"></audio>',
+			'<audio><source src="https://www.example.org/audio-file.mp3?some_query=string"></audio>',
 			$this->workspace,
 			$this->settings,
 			$this->styles,
@@ -81,13 +81,13 @@ public function testFilter() {
 		add_filter(
 			'apple_news_audio_json',
 			function( $json ) {
-				$json['URL'] = 'http://someurl.com/audio-file.mp3?some_query=string';
+				$json['URL'] = 'https://www.example.org/audio-file.mp3?some_query=string';
 				return $json;
 			}
 		);
 
 		$json = $component->to_array();
 		$this->assertEquals( 'audio', $json['role'] );
-		$this->assertEquals( 'http://someurl.com/audio-file.mp3?some_query=string', $json['URL'] );
+		$this->assertEquals( 'https://www.example.org/audio-file.mp3?some_query=string', $json['URL'] );
 	}
 }

tests/apple-exporter/components/test-class-body.php

@@ -197,10 +197,10 @@ public function data_empty_html() {
 	public function data_link_types() {
 		return [
 			// Standard link, non-https.
-			[ 'http://example.com', true ],
+			[ 'http://www.example.org', true ],
 
 			// Standard link, https.
-			[ 'https://example.com', true ],
+			[ 'https://www.example.org', true ],
 
 			// Root-relative URL. Should be permitted, but auto-converted to a fully qualified URL.
 			[ '/test', true ],
@@ -224,7 +224,7 @@ public function data_link_types() {
 			[ 'musics://abc123', true ],
 
 			// A mailto link.
-			[ 'mailto:example@example.com', true ],
+			[ 'mailto:example@example.org', true ],
 
 			// A hosted calendar.
 			[ 'webcal://abc123', true ],
@@ -384,7 +384,7 @@ public function test_link_types( $link, $should_work ) {
 
 		// Negotiate expected value and test.
 		if ( 0 === strpos( $link, '/' ) ) {
-			$link = 'http://example.org' . $link;
+			$link = 'https://www.example.org' . $link;
 		} elseif ( 0 === strpos( $link, '#' ) ) {
 			$link = get_permalink( $post_id ) . $link;
 		}

tests/apple-exporter/components/test-class-heading.php

@@ -137,7 +137,7 @@ public function test_image_splitting_with_link( $meta_order ) {
 
 		// Validate image split in generated JSON.
 		$this->assertEquals( 'photo', $json['components'][2]['role'] );
-		$this->assertEquals( 'http://example.org/example-image.jpg', $json['components'][2]['URL'] );
+		$this->assertEquals( 'https://www.example.org/example-image.jpg', $json['components'][2]['URL'] );
 	}
 
 	/**