Proposal: New wrapper function and hook for creating attachments

[luisrivera]

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;
}

[antpb]

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. :)

[luisrivera]

Sorry for the spam but...
Wow! Just noticed this was added to Milestone
Feeling so proud about being able to get involved in such a big project and giving back to this amazing community!

As I mentioned in Slack, I'm pretty sure that attachments are the most used post-type in 99% of WP sites. It has always seemed very strange to me that the absence of proper hooks in the attachment creation process could have been ignored all this time.

My boilerplate should work as is but maybe someone more involved in core-media should double check that I'm not missing anything important. I tried to cover most scenarios I've come across while developing projects where I use WP as a headless rest-api provider.

I think this little improvement will open a world of opportunities for developers, specially the ones building media plugins or the ones trying to integrate WP REST Api. I believe having a fully hookable core is the key for removing any compromises that may exist in the use of our REST api. This will keep WP ahead for the years to come.

[TimothyBlynJacobs]

We scrubbed this in #core-restapi today. Beta 1 is this Tuesday so I'm going to punt this to 5.6. I think this is too large of a change to get in at this point.

[TimothyBlynJacobs]

Hey @luisrivera,

Do you want to take a crack at turning this into a Pull Request? That'd make it easier to review and see the impacts.

I'm removing the needs-design keyword. For reference, we use that for when a ticket needs visual design, not code design :)

[TimothyBlynJacobs]

This hasn't seen any movement recently. Going to move this back to Future Release. If another contributor wants to own it for 5.6, feel free to re-milestone.