Skip to content
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.

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

Fix delete files on delete attachment #307

Merged
merged 1 commit into from
Apr 16, 2019
Merged
Changes from all commits
Commits
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
46 changes: 19 additions & 27 deletions inc/class-s3-uploads.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,7 @@ public function setup() {

add_filter( 'upload_dir', array( $this, 'filter_upload_dir' ) );
add_filter( 'wp_image_editors', array( $this, 'filter_editors' ), 9 );
add_action( 'delete_attachment', array( $this, 'set_original_file' ) );
add_filter( 'wp_delete_file', array( $this, 'wp_filter_delete_file' ) );
add_action( 'delete_attachment', array( $this, 'delete_attachment_files' ) );
add_filter( 'wp_read_image_metadata', array( $this, 'wp_filter_read_image_metadata' ), 10, 2 );
add_filter( 'wp_resource_hints', array( $this, 'wp_filter_resource_hints' ), 10, 2 );
remove_filter( 'admin_notices', 'wpthumb_errors' );
Expand All @@ -65,7 +64,6 @@ public function tear_down() {
remove_filter( 'upload_dir', array( $this, 'filter_upload_dir' ) );
remove_filter( 'wp_image_editors', array( $this, 'filter_editors' ), 9 );
remove_filter( 'wp_handle_sideload_prefilter', array( $this, 'filter_sideload_move_temp_file_to_s3' ) );
remove_filter( 'wp_delete_file', array( $this, 'wp_filter_delete_file' ) );
}

/**
Expand Down Expand Up @@ -106,35 +104,29 @@ public function filter_upload_dir( $dirs ) {
}

/**
* Capture the full path to the original file being deleted. This
* is used when determining whether an absolute or relative path
* should be used when deleting the file.
* Delete all attachment files from S3 when an attachment is deleted.
*
* @param $post_id
*/
public function set_original_file( $post_id ) {
$this->original_file = get_attached_file( $post_id );
}

/**
* When WordPress removes files, it's expecting to do so on
* absolute file paths, as such it breaks when using uris for
* file paths (such as s3://...). We have to filter the file_path
* to only return the relative section, to play nice with WordPress
* handling.
* WordPress Core's handling of deleting files for attachments via
* wp_delete_attachment_files is not compatible with remote streams, as
* it makes many assumptions about local file paths. The hooks also do
* not exist to be able to modify their behavior. As such, we just clean
* up the s3 files when an attachment is removed, and leave WordPress to try
* a failed attempt at mangling the s3:// urls.
*
* @param string $file_path
* @return string
* @param $post_id
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @param $post_id
* @param int $post_id

*/
public function wp_filter_delete_file( $file_path ) {
$dir = wp_upload_dir();

// When `wp_delete_file()` is called directly, it expects an absolute path.
if ( ! $this->original_file || $file_path === $this->original_file ) {
return $file_path;
public function delete_attachment_files( $post_id ) {
$meta = wp_get_attachment_metadata( $post_id );
$file = get_attached_file( $post_id );

if ( ! empty( $meta['sizes'] ) ) {
foreach ( $meta['sizes'] as $sizeinfo ) {
$intermediate_file = str_replace( basename( $file ), $sizeinfo['file'], $file );
unlink( $intermediate_file );
}
}

return str_replace( trailingslashit( $dir['basedir'] ), '', $file_path );
unlink( $file );
}

public function get_s3_url() {
Expand Down