Skip to content

Add --skip-copy flag to allow import of media from local filesystem without moving on disk #21

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
danielbachhuber merged 10 commits into from
Jul 3, 2017
Merged

Add --skip-copy flag to allow import of media from local filesystem without moving on disk #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
af02f1c
functional test for --import_only
Jun 21, 2017
995dad7
add --import_only
Jun 21, 2017
a94eaba
fix broken remote file import
Jun 21, 2017
7201e90
update --import_only test
Jun 21, 2017
eeb9836
Revert "WPCS fixes"
Jun 21, 2017
422a202
updated noun in test
Jun 21, 2017
e5ea370
rename to --skip-copy and improve test
Jun 22, 2017
6c763ec
bail early on error
Jun 26, 2017
054115d
Merge branch 'master' into import_only
danielbachhuber Jul 3, 2017
38b6176
Make sure the file isn't copied over
danielbachhuber Jul 3, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions features/media-import.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,25 @@ Feature: Manage WordPress attachments
My fabulous caption
"""

Scenario: Import a file as attachment from a local image and leave it in it's current location
Given download:

@danielbachhuber danielbachhuber Jun 28, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After Given download, you'll also need to run:

And I run `wp option update uploads_use_yearmonth_folders 0`

Otherwise, your file path assertion below will always be invalid, because WordPress will always used date-based folders.

@danielbachhuber danielbachhuber Jul 3, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

38b6176

| path | url |
| {CACHE_DIR}/large-image.jpg | http://wp-cli.org/behat-data/large-image.jpg |
And I run `wp option update uploads_use_yearmonth_folders 0`

When I run `wp media import {CACHE_DIR}/large-image.jpg --skip-copy`
Then STDOUT should contain:
"""
Imported file
"""
And STDOUT should contain:
"""
Success: Imported 1 of 1 items.
"""
And the {CACHE_DIR}/large-image.jpg file should exist

@danielbachhuber danielbachhuber Jun 21, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also assert that there's no file in the new destination

@phlbnks phlbnks Jun 22, 2017

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do. I should get this updated later today.

@phlbnks phlbnks Jun 22, 2017

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danielbachhuber I'm struggling to work out how to write this.
Can you point me ot any docs or examples that would help?

I guess I need to learn what Behat actually does with the image if it imports it? Does it move to {CACHE_DIR}/wp-content/uploads/year/month?
Then I think I'll need to create a new variable in FeatureContext.php for {UPLOADS_DIR} to use in the test. Is that about right?

Thanks!

@gitlost gitlost Jun 22, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should just need And the wp-content/uploads/large-image.jpg file should not exist - see media-regenerate.feature for examples.

Edit - Should add that Daniel is offline till Wednesday slack#cli.

@phlbnks phlbnks Jun 22, 2017

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @gitlost, can't believe I didn't think to check in there :/

And the wp-content/uploads/large-image.jpg file should not exist
And the return code should be 0

Scenario: Import a file and use its filename as the title
Given download:
| path | url |
Expand Down
95 changes: 57 additions & 38 deletions src/Media_Command.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,19 +113,20 @@ function regenerate( $args, $assoc_args = array() ) {
'post_mime_type' => $mime_types,
'post_status' => 'any',
'posts_per_page' => -1,
'fields' => 'ids',
'fields' => 'ids'
);

$images = new WP_Query( $query_args );

$count = $images->post_count;

if ( ! $count ) {
if ( !$count ) {
WP_CLI::warning( 'No images found.' );
return;
}

WP_CLI::log( sprintf( 'Found %1$d %2$s to regenerate.', $count, _n( 'image', 'images', $count ) ) );
WP_CLI::log( sprintf( 'Found %1$d %2$s to regenerate.', $count,
_n( 'image', 'images', $count ) ) );

if ( $image_size ) {
$image_size_filters = $this->add_image_size_filters( $image_size );
Expand Down Expand Up @@ -173,6 +174,9 @@ function regenerate( $args, $assoc_args = array() ) {
* [--desc=<description>]
* : "Description" field (post content) of attachment post.
*
* [--skip-copy]
* : If set, media files (local only) are imported to the library but not moved on disk.
*
* [--featured_image]
* : If set, set the imported image as the Featured Image of the post its attached to.
*
Expand Down Expand Up @@ -222,8 +226,8 @@ function import( $args, $assoc_args = array() ) {
}

if ( isset( $assoc_args['post_id'] ) ) {
if ( ! get_post( $assoc_args['post_id'] ) ) {
WP_CLI::warning( 'Invalid --post_id' );
if ( !get_post( $assoc_args['post_id'] ) ) {
WP_CLI::warning( "Invalid --post_id" );
$assoc_args['post_id'] = false;
}
} else {
Expand All @@ -236,12 +240,16 @@ function import( $args, $assoc_args = array() ) {
$orig_filename = $file;

if ( empty( $is_file_remote ) ) {
if ( ! file_exists( $file ) ) {
if ( !file_exists( $file ) ) {
WP_CLI::warning( "Unable to import file '$file'. Reason: File doesn't exist." );
$errors++;
continue;
}
$tempfile = $this->make_copy( $file );
if ( \WP_CLI\Utils\get_flag_value( $assoc_args, 'skip-copy' ) ) {
$tempfile = $file;
} else {
$tempfile = $this->make_copy( $file );
}
} else {
$tempfile = download_url( $file );
if ( is_wp_error( $tempfile ) ) {
Expand All @@ -256,17 +264,17 @@ function import( $args, $assoc_args = array() ) {

$file_array = array(
'tmp_name' => $tempfile,
'name' => Utils\basename( $file ),
'name' => Utils\basename( $file )
);

$post_array = array(
$post_array= array(
'post_title' => $assoc_args['title'],
'post_excerpt' => $assoc_args['caption'],
'post_content' => $assoc_args['desc'],
'post_content' => $assoc_args['desc']
);
$post_array = wp_slash( $post_array );

// use image exif/iptc data for title and caption defaults if possible.
// use image exif/iptc data for title and caption defaults if possible
if ( empty( $post_array['post_title'] ) || empty( $post_array['post_excerpt'] ) ) {
// @codingStandardsIgnoreStart
$image_meta = @wp_read_image_metadata( $tempfile );
Expand All @@ -286,33 +294,49 @@ function import( $args, $assoc_args = array() ) {
$post_array['post_title'] = preg_replace( '/\.[^.]+$/', '', Utils\basename( $file ) );
}

// Deletes the temporary file.
$success = media_handle_sideload( $file_array, $assoc_args['post_id'], $assoc_args['title'], $post_array );
if ( is_wp_error( $success ) ) {
WP_CLI::warning( sprintf(
"Unable to import file '%s'. Reason: %s",
$orig_filename, implode( ', ', $success->get_error_messages() )
) );
$errors++;
continue;
if ( \WP_CLI\Utils\get_flag_value( $assoc_args, 'skip-copy' ) ) {
$wp_filetype = wp_check_filetype( $file, null );
$post_array['post_mime_type'] = $wp_filetype['type'];
$post_array['post_status'] = 'inherit';

$success = wp_insert_attachment( $post_array, $file, $assoc_args['post_id'] );

@schlessera schlessera Jun 25, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the variable $success is being used several times here, and mostly refers to a "non-success", I would prefer the name to be changed to something like $result instead. Naming it $success seems misleading.

(Yes, I know that name was already in the code... 😄 )

@danielbachhuber danielbachhuber Jul 3, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can handle this in a subsequent PR.

if ( is_wp_error( $success ) ) {
WP_CLI::warning( sprintf(
"Unable to insert file '%s'. Reason: %s",
$orig_filename, implode( ', ', $success->get_error_messages() )
) );
$errors++;
continue;
}
wp_update_attachment_metadata( $success, wp_generate_attachment_metadata( $success, $file ) );
} else {
// Deletes the temporary file.
$success = media_handle_sideload( $file_array, $assoc_args['post_id'], $assoc_args['title'], $post_array );
if ( is_wp_error( $success ) ) {
WP_CLI::warning( sprintf(
"Unable to import file '%s'. Reason: %s",
$orig_filename, implode( ', ', $success->get_error_messages() )
) );
$errors++;
continue;
}
}

// Set alt text.
// Set alt text
if ( $assoc_args['alt'] ) {
update_post_meta( $success, '_wp_attachment_image_alt', wp_slash( $assoc_args['alt'] ) );
}

// Set as featured image, if --post_id and --featured_image are set.
// Set as featured image, if --post_id and --featured_image are set
if ( $assoc_args['post_id'] && \WP_CLI\Utils\get_flag_value( $assoc_args, 'featured_image' ) ) {
update_post_meta( $assoc_args['post_id'], '_thumbnail_id', $success );
}

$attachment_success_text = '';
if ( $assoc_args['post_id'] ) {
$attachment_success_text = " and attached to post {$assoc_args['post_id']}";
if ( \WP_CLI\Utils\get_flag_value( $assoc_args, 'featured_image' ) ) {
if ( \WP_CLI\Utils\get_flag_value( $assoc_args, 'featured_image' ) )
$attachment_success_text .= ' as featured image';
}
}

if ( \WP_CLI\Utils\get_flag_value( $assoc_args, 'porcelain' ) ) {
Expand All @@ -334,18 +358,16 @@ function import( $args, $assoc_args = array() ) {
}
}

// wp_tempnam() inexplicably forces a .tmp extension, which spoils MIME type detection.
// wp_tempnam() inexplicably forces a .tmp extension, which spoils MIME type detection
private function make_copy( $path ) {
$dir = get_temp_dir();
$filename = Utils\basename( $path );
if ( empty( $filename ) ) {
if ( empty( $filename ) )
$filename = time();
}

$filename = $dir . wp_unique_filename( $dir, $filename );
if ( ! copy( $path, $filename ) ) {
if ( !copy( $path, $filename ) )
WP_CLI::error( "Could not create temporary file for $path." );
}

return $filename;
}
Expand All @@ -356,7 +378,7 @@ private function process_regeneration( $id, $skip_delete, $only_missing, $image_
$att_desc = sprintf( '"%1$s" (ID %2$d)', get_the_title( $id ), $id );
$thumbnail_desc = $image_size ? sprintf( '"%s" thumbnail', $image_size ) : 'thumbnail';

if ( false === $fullsizepath || ! file_exists( $fullsizepath ) ) {
if ( false === $fullsizepath || !file_exists( $fullsizepath ) ) {
WP_CLI::warning( "Can't find $att_desc." );
return false;
}
Expand Down Expand Up @@ -418,19 +440,17 @@ private function remove_old_images( $att_id, $fullsizepath, $image_size ) {
foreach ( $metadata['sizes'] as $size_info ) {
$intermediate_path = $dir_path . $size_info['file'];

if ( $intermediate_path === $fullsizepath ) {
if ( $intermediate_path === $fullsizepath )
continue;
}

if ( file_exists( $intermediate_path ) ) {
if ( file_exists( $intermediate_path ) )
unlink( $intermediate_path );
}
}
}

private function needs_regeneration( $att_id, $fullsizepath, $is_pdf, $image_size ) {

$metadata = wp_get_attachment_metadata( $att_id );
$metadata = wp_get_attachment_metadata($att_id);

// Note that an attachment can have no sizes if it's on or below the thumbnail threshold.

Expand All @@ -454,12 +474,11 @@ private function needs_regeneration( $att_id, $fullsizepath, $is_pdf, $image_siz
$dir_path = dirname( $fullsizepath ) . '/';

// Check that the thumbnail files exist.
foreach ( $metadata['sizes'] as $size_info ) {
foreach( $metadata['sizes'] as $size_info ) {
$intermediate_path = $dir_path . $size_info['file'];

if ( $intermediate_path === $fullsizepath ) {
if ( $intermediate_path === $fullsizepath )
continue;
}

if ( ! file_exists( $intermediate_path ) ) {
return true;
Expand Down