Merge pull request #307 from humanmade/fix-delete-files

Fix delete files on delete attachment

inc/class-s3-uploads.php

@@ -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' );
@@ -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' ) );
 	}
 
 	/**
@@ -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
 	 */
-	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() {