Skip to content

Commit e9ee595

Browse files
adamsilversteinadamsilverstein
committed
Media: Sideload and clean up animated GIF video companion files
Add server-side support for storing a converted animated GIF's companion files. When client-side media processing is enabled, the editor transcodes an opaque animated GIF to a web-safe video and sideloads both the video and a static first-frame poster as companion files of the GIF attachment, which itself remains the attachment. This change enables those files to be sideloaded correctly. See WordPress/gutenberg#78410. Props khokansardar, westonruter, andrewserong. Fixes #65549. git-svn-id: https://develop.svn.wordpress.org/trunk@62623 602fd350-edb4-49c9-b593-d223f7449a82
1 parent d73908f commit e9ee595

4 files changed

Lines changed: 247 additions & 0 deletions

File tree

‎src/wp-includes/post.php‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6869,6 +6869,33 @@ function wp_delete_attachment_files( $post_id, $meta, $backup_sizes, $file ) {
68696869
}
68706870
}
68716871

6872+
/*
6873+
* Delete the animated-GIF video companions. When the client-side media flow
6874+
* converts an opaque animated GIF to a web-safe video, the converted MP4/WebM
6875+
* and a static first-frame JPEG poster are sideloaded alongside the GIF and
6876+
* recorded under the 'animated_video' and 'animated_video_poster' keys. These
6877+
* are kept separate from 'original_image', which continues to point at the GIF.
6878+
*/
6879+
foreach ( array( 'animated_video', 'animated_video_poster' ) as $companion_key ) {
6880+
if ( empty( $meta[ $companion_key ] ) || ! is_string( $meta[ $companion_key ] ) ) {
6881+
continue;
6882+
}
6883+
6884+
if ( empty( $intermediate_dir ) ) {
6885+
$intermediate_dir = path_join( $uploadpath['basedir'], dirname( $file ) );
6886+
}
6887+
6888+
$companion_file = str_replace( wp_basename( $file ), $meta[ $companion_key ], $file );
6889+
6890+
if ( ! empty( $companion_file ) ) {
6891+
$companion_file = path_join( $uploadpath['basedir'], $companion_file );
6892+
6893+
if ( ! wp_delete_file_from_directory( $companion_file, $intermediate_dir ) ) {
6894+
$deleted = false;
6895+
}
6896+
}
6897+
}
6898+
68726899
if ( is_array( $backup_sizes ) ) {
68736900
$del_dir = path_join( $uploadpath['basedir'], dirname( $meta['file'] ) );
68746901

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,9 @@ public function register_routes() {
123123
$valid_sizes[] = 'full';
124124
// Source-format original (e.g. the HEIC kept alongside its JPEG derivative).
125125
$valid_sizes[] = self::IMAGE_SIZE_SOURCE_ORIGINAL;
126+
// Converted-video companions for an animated GIF (the MP4/WebM and its poster).
127+
$valid_sizes[] = 'animated_video';
128+
$valid_sizes[] = 'animated_video_poster';
126129

127130
$items = is_string( $value ) ? array( $value ) : ( is_array( $value ) ? $value : null );
128131
if ( null === $items ) {
@@ -2432,6 +2435,13 @@ public function sideload_item( WP_REST_Request $request ) {
24322435
* under the dedicated source-image meta key.
24332436
*/
24342437
$sub_size_data['file'] = wp_basename( $path );
2438+
} elseif ( 'animated_video' === $image_size || 'animated_video_poster' === $image_size ) {
2439+
/*
2440+
* Converted-video companion of an animated GIF (the MP4/WebM or
2441+
* its static first-frame poster). Record the filename so
2442+
* finalize_item can store it under its dedicated meta key.
2443+
*/
2444+
$sub_size_data['file'] = wp_basename( $path );
24352445
} elseif ( 'scaled' === $image_size ) {
24362446
// Record the current attached file as the original.
24372447
$current_file = get_attached_file( $attachment_id, true );
@@ -2596,6 +2606,16 @@ public function finalize_item( WP_REST_Request $request ) {
25962606
* is handled by wp_delete_attachment_files().
25972607
*/
25982608
$metadata[ self::META_KEY_SOURCE_IMAGE ] = $sub_size['file'];
2609+
} elseif ( 'animated_video' === $image_size ) {
2610+
/*
2611+
* Converted-video companion of an animated GIF. Stored under its
2612+
* own meta key; 'original_image' keeps pointing at the GIF. Cleanup
2613+
* on attachment delete is handled by wp_delete_attachment_files().
2614+
*/
2615+
$metadata['animated_video'] = $sub_size['file'];
2616+
} elseif ( 'animated_video_poster' === $image_size ) {
2617+
// Static first-frame poster for the converted video.
2618+
$metadata['animated_video_poster'] = $sub_size['file'];
25992619
} elseif ( 'scaled' === $image_size ) {
26002620
if ( ! empty( $sub_size['original_image'] ) ) {
26012621
$metadata['original_image'] = $sub_size['original_image'];

‎tests/phpunit/tests/media/wpDeleteAttachmentAnimatedGifVideo.php‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
<?php
2+
3+
/**
4+
* Tests that wp_delete_attachment_files() removes the animated-GIF video companions.
5+
*
6+
* @group media
7+
* @covers ::wp_delete_attachment_files
8+
*/
9+
class Tests_Media_wpDeleteAttachmentAnimatedGifVideo extends WP_UnitTestCase {
10+
11+
public function tear_down(): void {
12+
$this->remove_added_uploads();
13+
14+
parent::tear_down();
15+
}
16+
17+
/**
18+
* @ticket 65549
19+
*/
20+
public function test_deletes_video_and_poster_companions(): void {
21+
$attachment_id = self::factory()->attachment->create_upload_object( DIR_TESTDATA . '/images/canola.jpg' );
22+
$this->assertIsInt( $attachment_id );
23+
24+
$attached_file = get_attached_file( $attachment_id, true );
25+
$this->assertIsString( $attached_file );
26+
$dir = dirname( $attached_file );
27+
$video_name = 'companion-' . wp_generate_password( 6, false ) . '.mp4';
28+
$poster_name = 'companion-' . wp_generate_password( 6, false ) . '.jpg';
29+
$video_path = $dir . '/' . $video_name;
30+
$poster_path = $dir . '/' . $poster_name;
31+
32+
// Create dummy companion files on disk.
33+
file_put_contents( $video_path, 'test' );
34+
file_put_contents( $poster_path, 'test' );
35+
$this->assertFileExists( $video_path, 'Video fixture should be on disk.' );
36+
$this->assertFileExists( $poster_path, 'Poster fixture should be on disk.' );
37+
38+
// Record the companions as the sideload route does.
39+
$metadata = wp_get_attachment_metadata( $attachment_id, true );
40+
$this->assertIsArray( $metadata );
41+
$metadata['animated_video'] = $video_name;
42+
$metadata['animated_video_poster'] = $poster_name;
43+
wp_update_attachment_metadata( $attachment_id, $metadata );
44+
45+
wp_delete_attachment( $attachment_id, true );
46+
47+
$this->assertNull( get_post( $attachment_id ) );
48+
$this->assertFileDoesNotExist( $video_path, 'Video companion should be deleted alongside the attachment.' );
49+
$this->assertFileDoesNotExist( $poster_path, 'Poster companion should be deleted alongside the attachment.' );
50+
}
51+
52+
/**
53+
* @ticket 65549
54+
*/
55+
public function test_deletes_only_video_when_no_poster_recorded(): void {
56+
$attachment_id = self::factory()->attachment->create_upload_object( DIR_TESTDATA . '/images/canola.jpg' );
57+
$this->assertIsInt( $attachment_id );
58+
59+
$attached_file = get_attached_file( $attachment_id, true );
60+
$this->assertIsString( $attached_file );
61+
$dir = dirname( $attached_file );
62+
$video_name = 'companion-' . wp_generate_password( 6, false ) . '.mp4';
63+
$video_path = $dir . '/' . $video_name;
64+
65+
file_put_contents( $video_path, 'test' );
66+
$this->assertFileExists( $video_path, 'Video fixture should be on disk.' );
67+
68+
$metadata = wp_get_attachment_metadata( $attachment_id, true );
69+
$this->assertIsArray( $metadata );
70+
$metadata['animated_video'] = $video_name;
71+
wp_update_attachment_metadata( $attachment_id, $metadata );
72+
73+
wp_delete_attachment( $attachment_id, true );
74+
75+
$this->assertNull( get_post( $attachment_id ) );
76+
$this->assertFileDoesNotExist( $video_path, 'Video companion should be deleted alongside the attachment.' );
77+
}
78+
79+
/**
80+
* @ticket 65549
81+
*/
82+
public function test_noop_when_no_companion_metadata(): void {
83+
$attachment_id = self::factory()->attachment->create_upload_object( DIR_TESTDATA . '/images/canola.jpg' );
84+
$this->assertIsInt( $attachment_id );
85+
86+
// Sanity: no companion keys on freshly-created metadata.
87+
$metadata = wp_get_attachment_metadata( $attachment_id, true );
88+
$this->assertIsArray( $metadata );
89+
$this->assertArrayNotHasKey( 'animated_video', $metadata );
90+
$this->assertArrayNotHasKey( 'animated_video_poster', $metadata );
91+
92+
// Deletion should complete cleanly even though no companion file is recorded.
93+
wp_delete_attachment( $attachment_id, true );
94+
95+
$this->assertNull( get_post( $attachment_id ) );
96+
}
97+
98+
/**
99+
* Guards against a companion key holding a non-string value (e.g. the array
100+
* form some metadata flows write).
101+
*
102+
* @ticket 65549
103+
*/
104+
public function test_noop_when_companion_metadata_is_not_a_string(): void {
105+
$attachment_id = self::factory()->attachment->create_upload_object( DIR_TESTDATA . '/images/canola.jpg' );
106+
$this->assertIsInt( $attachment_id );
107+
$attached_file = get_attached_file( $attachment_id, true );
108+
$this->assertIsString( $attached_file );
109+
110+
/*
111+
* Place a real file that a buggy, guard-less implementation could try to
112+
* delete after running wp_basename() over the array value below.
113+
*/
114+
$bystander_path = dirname( $attached_file ) . '/should-not-delete.mp4';
115+
file_put_contents( $bystander_path, 'test' );
116+
$this->assertFileExists( $bystander_path, 'Test fixture should be on disk.' );
117+
118+
$metadata = wp_get_attachment_metadata( $attachment_id, true );
119+
$this->assertIsArray( $metadata );
120+
$metadata['animated_video'] = array( 'file' => 'should-not-delete.mp4' );
121+
wp_update_attachment_metadata( $attachment_id, $metadata );
122+
123+
// Deletion should not raise (no str_replace() / file deletion on an array).
124+
wp_delete_attachment( $attachment_id, true );
125+
126+
$this->assertNull( get_post( $attachment_id ) );
127+
$this->assertFileExists( $bystander_path, 'The non-string guard must prevent any file deletion.' );
128+
}
129+
}

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

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3734,6 +3734,77 @@ public function test_sideload_source_original_writes_metadata_source_image(): vo
37343734
$this->assertArrayNotHasKey( 'original_image', $metadata, "Metadata 'original_image' should be untouched by the HEIC sideload." );
37353735
}
37363736

3737+
/**
3738+
* Tests sideloading the animated-GIF video companions ('animated_video' and
3739+
* 'animated_video_poster'). Each filename is recorded under its own metadata
3740+
* key by the finalize endpoint and does not collide with 'original_image',
3741+
* which keeps pointing at the GIF.
3742+
*
3743+
* The uploaded bytes are a JPEG stand-in: the sideload branch only records
3744+
* wp_basename() of the stored file, so the metadata plumbing can be exercised
3745+
* without depending on video upload support in the test environment.
3746+
*
3747+
* @ticket 65549
3748+
* @requires function imagejpeg
3749+
*/
3750+
public function test_sideload_animated_video_companions_write_metadata(): void {
3751+
$this->enable_client_side_media_processing();
3752+
3753+
wp_set_current_user( self::$author_id );
3754+
3755+
// Create the (GIF) attachment the companions belong to.
3756+
$request = new WP_REST_Request( 'POST', '/wp/v2/media' );
3757+
$request->set_header( 'Content-Type', 'image/jpeg' );
3758+
$request->set_header( 'Content-Disposition', 'attachment; filename=canola.jpg' );
3759+
$request->set_body( (string) file_get_contents( self::$test_file ) );
3760+
$response = rest_get_server()->dispatch( $request );
3761+
$attachment_id = $response->get_data()['id'];
3762+
$this->assertSame( 201, $response->get_status() );
3763+
3764+
// Sideload the converted-video companion.
3765+
$request = new WP_REST_Request( 'POST', "/wp/v2/media/{$attachment_id}/sideload" );
3766+
$request->set_header( 'Content-Type', 'image/jpeg' );
3767+
$request->set_header( 'Content-Disposition', 'attachment; filename=canola-video.jpg' );
3768+
$request->set_param( 'image_size', 'animated_video' );
3769+
$request->set_body( (string) file_get_contents( self::$test_file ) );
3770+
$video_response = rest_get_server()->dispatch( $request );
3771+
$this->assertSame( 200, $video_response->get_status(), 'Sideloading animated_video should succeed.' );
3772+
$video_sub_size = $video_response->get_data();
3773+
$this->assertIsArray( $video_sub_size );
3774+
$this->assertSame( 'animated_video', $video_sub_size['image_size'], 'Response should echo the image_size.' );
3775+
3776+
// Sideload the poster companion.
3777+
$request = new WP_REST_Request( 'POST', "/wp/v2/media/{$attachment_id}/sideload" );
3778+
$request->set_header( 'Content-Type', 'image/jpeg' );
3779+
$request->set_header( 'Content-Disposition', 'attachment; filename=canola-poster.jpg' );
3780+
$request->set_param( 'image_size', 'animated_video_poster' );
3781+
$request->set_body( (string) file_get_contents( self::$test_file ) );
3782+
$poster_response = rest_get_server()->dispatch( $request );
3783+
$this->assertSame( 200, $poster_response->get_status(), 'Sideloading animated_video_poster should succeed.' );
3784+
$poster_sub_size = $poster_response->get_data();
3785+
$this->assertIsArray( $poster_sub_size );
3786+
$this->assertSame( 'animated_video_poster', $poster_sub_size['image_size'], 'Response should echo the image_size.' );
3787+
3788+
// Sideload must not write metadata; that happens in finalize.
3789+
$metadata = wp_get_attachment_metadata( $attachment_id, true );
3790+
$this->assertArrayNotHasKey( 'animated_video', $metadata, 'Sideload should not write animated_video metadata.' );
3791+
$this->assertArrayNotHasKey( 'animated_video_poster', $metadata, 'Sideload should not write animated_video_poster metadata.' );
3792+
3793+
// Finalize with both collected sub-sizes, which writes the metadata.
3794+
$request = new WP_REST_Request( 'POST', "/wp/v2/media/{$attachment_id}/finalize" );
3795+
$request->set_param( 'sub_sizes', array( $video_sub_size, $poster_sub_size ) );
3796+
$response = rest_get_server()->dispatch( $request );
3797+
$this->assertSame( 200, $response->get_status(), 'Finalize should succeed.' );
3798+
3799+
$metadata = wp_get_attachment_metadata( $attachment_id );
3800+
$this->assertIsArray( $metadata );
3801+
$this->assertArrayHasKey( 'animated_video', $metadata, "Metadata should contain 'animated_video'." );
3802+
$this->assertMatchesRegularExpression( '/canola-video.*\.jpg$/', $metadata['animated_video'], "Metadata 'animated_video' should reference the video companion filename." );
3803+
$this->assertArrayHasKey( 'animated_video_poster', $metadata, "Metadata should contain 'animated_video_poster'." );
3804+
$this->assertMatchesRegularExpression( '/canola-poster.*\.jpg$/', $metadata['animated_video_poster'], "Metadata 'animated_video_poster' should reference the poster filename." );
3805+
$this->assertArrayNotHasKey( 'original_image', $metadata, "Metadata 'original_image' should be untouched by the companion sideloads." );
3806+
}
3807+
37373808
/**
37383809
* Tests the filter_wp_unique_filename method handles the -scaled suffix.
37393810
*

0 commit comments

Comments
 (0)