WordPress.org

Make WordPress Core

Opened 5 weeks ago

Last modified 2 weeks ago

#49490 new enhancement

Missing wrapper function and action hook for new attachments

Reported by: luisrivera Owned by:
Milestone: 5.5 Priority: normal
Severity: normal Version:
Component: Media Keywords: needs-design dev-feedback 2nd-opinion needs-patch
Focuses: rest-api, performance, coding-standards Cc:

Description

Everywhere you look for the code related to inserting a new attachment you find 3 steps are needed:

  1. wp_insert_attachment
  2. wp_generate_attachment_metadata
  3. wp_update_attachment_metadata

I see no reason not to have a wrapper function instead of having these 3 functions being repeated all over core files + 3rd party themes and plugins. Plus, as people commit changes in core mistakes are occurring 49449.

The most important reason to create this new wrapper function is to improve interoperability by adding a real hook to know when an attachment has been totally processed by core. This is needed by many plugins that work with images or caching. Right now the hook most used to know of a new attachment is add_attachment found in wp_insert_post(), but this is run before step 2 and 3, therefore attachment metadata has not been created yet.

Developers are having to create a custom wrapper function to add this hook (my company included) in their projects and that means attachments inserted via WP Dashboard wont be taken into action.

media_handle_upload() is a good example of the wrapper function I am talking about, but sadly it goes out of scope by assuming the file will be located in $_FILES variable instead of receiving the file by a parameter, therefore is not friendly or future proof. When that function was created I guess it wasn't taken into consideration that files can be uploaded to the server in many ways. Plus is lacking the appropriate hooks too.

Even the WP_REST_Attachments_Controller had to create custom methods to fulfill a scenario where attachment data is being sent in the body of the request.

A fast search in core for wp_insert_attachment will illustrate how this function is followed by wp_generate_attachment_metadata() and wp_update_attachment_metadata() in most cases:

  • media_handle_sideload()
  • media_handle_upload()
  • wp_ajax_crop_image()
  • Custom_Background::handle_upload()
  • WP_Site_Icon::insert_attachment()
  • wp_xmlrpc_server::mw_newMediaObject()
  • WP_REST_Attachments_Controller I wont even mention third party implementations I’ve found.

A quick boilerplate...

<?php
/**
 * Creates a single attachment.
 *
 * @param string|array          $args     Required. Arguments for inserting a post of type attachment.
 * @param array                                         $data     Required. An array of data for a single file
 *                                                                                                                                      Like the one supplied by wp_handle_upload() or wp_handle_sideload()
 * @param string                $context  Optional. Provide context for hooks (eg. AJAX, REST)
 * @param bool                  $wp_error Optional. Whether to return a WP_Error on failure. Default false.
 *
 * @return int|WP_Error The attachment ID on success. The value 0 or WP_Error on failure.
 *
 * @author Luis Rivera
 */
function wp_process_attachment ( string|array $args, string $data = false, string $context, bool $wp_error = false ) {
        /**
         * Just let us play
         * Maybe I need to remove a filter base on X condition
         */
        do_action( 'pre_filter_process_attachment', $args, $data, $context );

        /**
         * Allow modifications to the file
         * First filter because post args may depend on file data
         */
        $data = apply_filters( 'wp_process_attachment_file', $data, $args, $context );

        /**
         * Allow modifications to the post args
         */
        $args = apply_filters( 'wp_process_attachment_args', $args, $data, $context );

        /**
         * Filters whether to short-circuit the process
         * At last to evaluate values after filters has been applied
         *
         * If a non-null value is passed to the filter, all the process will be cancel
         *
         * @param mixed                 $process                If null (default) continue process
         *                                                                                                                      If is_wp_error() && $wp_error the error will be returned
         */
        $process = apply_filters( 'wp_process_attachment', null, $args, $data, $context );

        if ( $process !== null ) {
                if ( $wp_error && is_wp_error($process) ) {
                        return $process;
                }
                return false;
        }

        do_action( 'before_process_attachment', $args, $data, $context );

        if ( empty( $args['post_title'] ) ) {
                $args['post_title'] = preg_replace( '/\.[^.]+$/', '', wp_basename( $data['file'] ) );
        }

        if ( empty( $args['post_mime_type'] ) ) {
                $args['post_mime_type'] = $data['type'];
        }

        if ( empty( $args['post_status'] ) ) {
                $args['post_status'] = 'inherit';
        }

        $attachment_id = wp_insert_attachment( $args, $data['file']);

        /** Include image functions to get access to wp_generate_attachment_metadata(). */
        require_once ABSPATH . 'wp-admin/includes/image.php';

        $metadata = wp_generate_attachment_metadata( $attachment_id, $data['file'] );
        wp_update_attachment_metadata( $attachment_id, $metadata );

        do_action( 'after_process_attachment', $attachment_id);

        return $attachment_id;
}

Change History (3)

This ticket was mentioned in Slack in #core-media by pbiron. View the logs.


2 weeks ago

#2 @antpb
2 weeks ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 5.5

This was discussed in the Media component meeting and it was agreed that better consistency across image handling in Core would be a great goal for 5.5. Moving it into the milestone so we can keep an eye on it and discuss further. :)

This ticket was mentioned in Slack in #core-restapi by pbiron. View the logs.


2 weeks ago

Note: See TracTickets for help on using tickets.