REST API: Move sideload metadata writing to the finalize endpoint

Backport of Gutenberg PR #75888. Eliminate the read-modify-write race
between concurrent sideloads for the same attachment by no longer
writing attachment metadata in the sideload endpoint. Instead, sideload
returns lightweight sub-size data (dimensions, filename, filesize) which
the client accumulates and passes to the finalize endpoint, which writes
all collected sub-sizes in a single metadata update.

This matches how core generates sub-sizes (one metadata write after all
sizes exist) and replaces the earlier per-attachment locking approach
that the merged Gutenberg PR ultimately abandoned.

Author: adamsilverstein

src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php

@@ -113,10 +113,46 @@ public function register_routes() {
 						'callback'            => array( $this, 'finalize_item' ),
 						'permission_callback' => array( $this, 'edit_media_item_permissions_check' ),
 						'args'                => array(
-							'id' => array(
+							'id'        => array(
 								'description' => __( 'Unique identifier for the attachment.' ),
 								'type'        => 'integer',
 							),
+							'sub_sizes' => array(
+								'description' => __( 'Array of sub-size metadata collected from sideload responses.' ),
+								'type'        => 'array',
+								'default'     => array(),
+								'items'       => array(
+									'type'       => 'object',
+									'properties' => array(
+										'image_size'     => array(
+											'type'     => 'string',
+											'required' => true,
+										),
+										'width'          => array(
+											'type'    => 'integer',
+											'minimum' => 1,
+										),
+										'height'         => array(
+											'type'    => 'integer',
+											'minimum' => 1,
+										),
+										'file'           => array(
+											'type' => 'string',
+										),
+										'mime_type'      => array(
+											'type'    => 'string',
+											'pattern' => '^image/.*',
+										),
+										'filesize'       => array(
+											'type'    => 'integer',
+											'minimum' => 1,
+										),
+										'original_image' => array(
+											'type' => 'string',
+										),
+									),
+								),
+							),
 						),
 					),
 					'allow_batch' => $this->allow_batch,
@@ -2082,16 +2118,19 @@ public function sideload_item( WP_REST_Request $request ) {
 
 		$image_size = $request['image_size'];
 
-		$metadata = wp_get_attachment_metadata( $attachment_id, true );
-
-		if ( ! $metadata ) {
-			$metadata = array();
-		}
+		// Build sub-size data to return to the client.
+		// The client accumulates these and sends them all to the finalize
+		// endpoint, which writes the metadata in a single operation. This
+		// avoids the read-modify-write race that concurrent sideloads for the
+		// same attachment would otherwise hit.
+		$sub_size_data = array(
+			'image_size' => $image_size,
+		);
 
 		if ( 'original' === $image_size ) {
-			$metadata['original_image'] = wp_basename( $path );
+			$sub_size_data['file'] = wp_basename( $path );
 		} elseif ( 'scaled' === $image_size ) {
-			// The current attached file is the original; record it as original_image.
+			// Record the current attached file as the original.
 			$current_file = get_attached_file( $attachment_id, true );
 
 			if ( ! $current_file ) {
@@ -2102,7 +2141,7 @@ public function sideload_item( WP_REST_Request $request ) {
 				);
 			}
 
-			$metadata['original_image'] = wp_basename( $current_file );
+			$sub_size_data['original_image'] = wp_basename( $current_file );
 
 			// Validate the scaled image before updating the attached file.
 			$size     = wp_getimagesize( $path );
@@ -2117,6 +2156,7 @@ public function sideload_item( WP_REST_Request $request ) {
 			}
 
 			// Update the attached file to point to the scaled version.
+			// This writes to _wp_attached_file meta, not _wp_attachment_metadata.
 			if (
 				get_attached_file( $attachment_id, true ) !== $path &&
 				! update_attached_file( $attachment_id, $path )
@@ -2128,42 +2168,21 @@ public function sideload_item( WP_REST_Request $request ) {
 				);
 			}
 
-			$metadata['width']    = $size[0];
-			$metadata['height']   = $size[1];
-			$metadata['filesize'] = $filesize;
-			$metadata['file']     = _wp_relative_upload_path( $path );
+			$sub_size_data['width']    = $size[0];
+			$sub_size_data['height']   = $size[1];
+			$sub_size_data['filesize'] = $filesize;
+			$sub_size_data['file']     = _wp_relative_upload_path( $path );
 		} else {
-			$metadata['sizes'] = $metadata['sizes'] ?? array();
-
 			$size = wp_getimagesize( $path );
 
-			$metadata['sizes'][ $image_size ] = array(
-				'width'     => $size ? $size[0] : 0,
-				'height'    => $size ? $size[1] : 0,
-				'file'      => wp_basename( $path ),
-				'mime-type' => $type,
-				'filesize'  => wp_filesize( $path ),
-			);
-		}
-
-		wp_update_attachment_metadata( $attachment_id, $metadata );
-
-		$response_request = new WP_REST_Request(
-			WP_REST_Server::READABLE,
-			rest_get_route_for_post( $attachment_id )
-		);
-
-		$response_request['context'] = 'edit';
-
-		if ( isset( $request['_fields'] ) ) {
-			$response_request['_fields'] = $request['_fields'];
+			$sub_size_data['width']     = $size ? $size[0] : 0;
+			$sub_size_data['height']    = $size ? $size[1] : 0;
+			$sub_size_data['file']      = wp_basename( $path );
+			$sub_size_data['mime_type'] = $type;
+			$sub_size_data['filesize']  = wp_filesize( $path );
 		}
 
-		$response = $this->prepare_item_for_response( get_post( $attachment_id ), $response_request );
-
-		$response->header( 'Location', rest_url( rest_get_route_for_post( $attachment_id ) ) );
-
-		return $response;
+		return rest_ensure_response( $sub_size_data );
 	}
 
 	/**
@@ -2237,6 +2256,35 @@ public function finalize_item( WP_REST_Request $request ) {
 			$metadata = array();
 		}
 
+		// Apply all sub-size metadata collected from sideload responses.
+		$sub_sizes = $request['sub_sizes'] ?? array();
+
+		foreach ( $sub_sizes as $sub_size ) {
+			$image_size = $sub_size['image_size'];
+
+			if ( 'original' === $image_size ) {
+				$metadata['original_image'] = $sub_size['file'];
+			} elseif ( 'scaled' === $image_size ) {
+				if ( ! empty( $sub_size['original_image'] ) ) {
+					$metadata['original_image'] = $sub_size['original_image'];
+				}
+				$metadata['width']    = $sub_size['width'] ?? 0;
+				$metadata['height']   = $sub_size['height'] ?? 0;
+				$metadata['filesize'] = $sub_size['filesize'] ?? 0;
+				$metadata['file']     = $sub_size['file'] ?? '';
+			} else {
+				$metadata['sizes'] = $metadata['sizes'] ?? array();
+
+				$metadata['sizes'][ $image_size ] = array(
+					'width'     => $sub_size['width'] ?? 0,
+					'height'    => $sub_size['height'] ?? 0,
+					'file'      => $sub_size['file'] ?? '',
+					'mime-type' => $sub_size['mime_type'] ?? '',
+					'filesize'  => $sub_size['filesize'] ?? 0,
+				);
+			}
+		}
+
 		/** This filter is documented in wp-admin/includes/image.php */
 		$metadata = apply_filters( 'wp_generate_attachment_metadata', $metadata, $attachment_id, 'update' );
 

tests/phpunit/tests/rest-api/rest-attachments-controller.php

@@ -3275,16 +3275,32 @@ public function test_sideload_scaled_image() {
 
 		$this->assertSame( 200, $response->get_status(), 'Sideloading scaled image should succeed.' );
 
+		// The sideload endpoint returns lightweight sub-size data; the metadata
+		// is written later by the finalize endpoint.
+		$sub_size = $response->get_data();
+		$this->assertSame( 'scaled', $sub_size['image_size'], 'Response should echo the image_size.' );
+		$this->assertSame( wp_basename( $original_file ), $sub_size['original_image'], 'Response original_image should be the basename of the original attached file.' );
+		$this->assertGreaterThan( 0, $sub_size['width'], 'Response width should be positive.' );
+		$this->assertGreaterThan( 0, $sub_size['height'], 'Response height should be positive.' );
+		$this->assertGreaterThan( 0, $sub_size['filesize'], 'Response filesize should be positive.' );
+		$this->assertStringContainsString( 'scaled', $sub_size['file'], 'Response file should reference the scaled version.' );
+
+		// The attached file is still repointed to the scaled version during sideload.
+		$new_file = get_attached_file( $attachment_id, true );
+		$this->assertStringContainsString( 'scaled', wp_basename( $new_file ), 'Attached file should now be the scaled version.' );
+
+		// Finalize with the collected sub-size, which writes the metadata.
+		$request = new WP_REST_Request( 'POST', "/wp/v2/media/{$attachment_id}/finalize" );
+		$request->set_param( 'sub_sizes', array( $sub_size ) );
+		$response = rest_get_server()->dispatch( $request );
+		$this->assertSame( 200, $response->get_status(), 'Finalize should succeed.' );
+
 		$metadata = wp_get_attachment_metadata( $attachment_id );
 
 		// The original file should now be recorded as original_image.
 		$this->assertArrayHasKey( 'original_image', $metadata, 'Metadata should contain original_image.' );
 		$this->assertSame( wp_basename( $original_file ), $metadata['original_image'], 'original_image should be the basename of the original attached file.' );
 
-		// The attached file should now point to the scaled version.
-		$new_file = get_attached_file( $attachment_id, true );
-		$this->assertStringContainsString( 'scaled', wp_basename( $new_file ), 'Attached file should now be the scaled version.' );
-
 		// Metadata should have width, height, filesize, and file updated.
 		$this->assertArrayHasKey( 'width', $metadata, 'Metadata should contain width.' );
 		$this->assertArrayHasKey( 'height', $metadata, 'Metadata should contain height.' );
@@ -3541,4 +3557,55 @@ public function test_finalize_item_invalid_id(): void {
 
 		$this->assertErrorResponse( 'rest_post_invalid_id', $response, 404 );
 	}
+
+	/**
+	 * Tests that the finalize endpoint writes regular sub-size metadata
+	 * collected from sideload responses.
+	 *
+	 * @ticket 62243
+	 * @covers WP_REST_Attachments_Controller::finalize_item
+	 * @requires function imagejpeg
+	 */
+	public function test_finalize_writes_regular_sub_sizes(): void {
+		$this->enable_client_side_media_processing();
+
+		wp_set_current_user( self::$author_id );
+
+		// Create an attachment without generating sub-sizes server-side.
+		$request = new WP_REST_Request( 'POST', '/wp/v2/media' );
+		$request->set_header( 'Content-Type', 'image/jpeg' );
+		$request->set_header( 'Content-Disposition', 'attachment; filename=canola.jpg' );
+		$request->set_param( 'generate_sub_sizes', false );
+		$request->set_body( (string) file_get_contents( self::$test_file ) );
+		$response      = rest_get_server()->dispatch( $request );
+		$attachment_id = $response->get_data()['id'];
+
+		$this->assertSame( 201, $response->get_status() );
+
+		// Sideload a thumbnail sub-size; the response carries its metadata.
+		$request = new WP_REST_Request( 'POST', "/wp/v2/media/{$attachment_id}/sideload" );
+		$request->set_header( 'Content-Type', 'image/jpeg' );
+		$request->set_header( 'Content-Disposition', 'attachment; filename=canola-thumb.jpg' );
+		$request->set_param( 'image_size', 'thumbnail' );
+		$request->set_body( (string) file_get_contents( self::$test_file ) );
+		$response = rest_get_server()->dispatch( $request );
+
+		$this->assertSame( 200, $response->get_status(), 'Sideloading a thumbnail should succeed.' );
+
+		$sub_size = $response->get_data();
+		$this->assertSame( 'thumbnail', $sub_size['image_size'], 'Response should echo the image_size.' );
+
+		// Finalize with the collected sub-size, which writes it into metadata.
+		$request = new WP_REST_Request( 'POST', "/wp/v2/media/{$attachment_id}/finalize" );
+		$request->set_param( 'sub_sizes', array( $sub_size ) );
+		$response = rest_get_server()->dispatch( $request );
+
+		$this->assertSame( 200, $response->get_status(), 'Finalize should succeed.' );
+
+		$metadata = wp_get_attachment_metadata( $attachment_id );
+		$this->assertArrayHasKey( 'sizes', $metadata, 'Metadata should contain sizes.' );
+		$this->assertArrayHasKey( 'thumbnail', $metadata['sizes'], 'Metadata sizes should contain the sideloaded thumbnail.' );
+		$this->assertSame( 'image/jpeg', $metadata['sizes']['thumbnail']['mime-type'], 'Thumbnail mime-type should be recorded.' );
+		$this->assertGreaterThan( 0, $metadata['sizes']['thumbnail']['filesize'], 'Thumbnail filesize should be positive.' );
+	}
 }