Make WordPress Core

Opened 4 months ago

Closed 2 weeks ago

Last modified 2 weeks ago

#61383 closed enhancement (fixed)

Add new filter to attachment_url_to_postid() for short circuit

Reported by: apermo's profile apermo Owned by: peterwilsoncc's profile peterwilsoncc
Milestone: 6.7 Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch
Focuses: Cc:

Description

The function attachment_url_to_postid() will receive a relative url and then use the query following query to get the corresponding post_id for the attachment.

SELECT post_id, meta_value FROM wp_postmeta WHERE meta_key = '_wp_attached_file' AND meta_value = '2024/06/my-image.jpg'

On large installations, this will be slow, resulting in long delays upon saving. In my example we are talking about 4s per query, since the installation has 500k images in the database. And if a post has a gallery, this can result in save times above 30s. So even for less extreme sizes this query will result in long wait times for editors, if they save posts with lots of images.

Adding a new column to wp_post or a new table to core is unlikely, since the default behavior works fine for standard installations.

So in my case I want to add a custom table, to speed up my installation. And while it currently already is possible to filter the query before it is executed using the filter query this requires a to check all incoming queries, if it possibly is one those that I want to replace.

This is why I propose to add a filter at the beginning of the function, which allows to short circuit the function if anyone wants to use a custom method.

As already mentioned, I've checked the following alternatives for my particular site and I think adding the filter will add value here.

  • Using query deeper inside the function. Will work, but is not ideal since you need to check all queries, and on top, it will only allow you to use the database as source, so if you'd have a different method of getting the result, it would require you to fake a result, or use the filter at the end of the function, after you already did the query.
  • Adding an index to the meta_value column. This speeded up the query from 4s to 20ms in my case, but I do not dare to do so on a production system, since I have no idea what side effects this will have.
  • Overriding the function will not work, since the function does not come wrapped in a if ( function_exists() ) and the function is used twice in core, as well as in Yoast SEO and ACF.

Which comes to the conclusion, by adding the attached filter, it will make it easier to allow custom methods of retrieving the post_id faster for special setups.

Attachments (1)

attachment_url_to_postid.patch (916 bytes) - added by apermo 4 months ago.
Patch for the ticket.

Download all attachments as: .zip

Change History (10)

@apermo
4 months ago

Patch for the ticket.

#1 @audrasjb
4 months ago

  • Milestone changed from Awaiting Review to 6.7

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


3 months ago

#3 @antpb
3 months ago

  • Owner set to antpb
  • Status changed from new to assigned

This was reviewed in the recent media component meeting and it was agreed this would be a good inclusion that could have big impact on larger media sites. The patch looks good on initial review but will need a functional manual test of the filter before I feel comfortable committing. Since its a filter I anticipate little to no issues. :)

#4 follow-up: @peterwilsoncc
4 weeks ago

I think the filter in the proposed patch is good but the docblock improvements:

Filters the attachment ID to allow short-circuit the function.

Allows plugins to short-circuit attachment ID lookups. Plugins making use of this function should return:

  • 0 (integer) to indicate the attachment is not found
  • attachment ID (integer) to indicate the attachment ID found
  • null to indicate WordPress should proceed with the lookup.

Warning: The post ID may be null or zero, both of which cast to a boolean false. For information about casting to booleans see the {@link https://www.php.net/manual/en/language.types.boolean.php PHP documentation}. Use the === operator for testing the post ID when developing filters using this hook.

@param int|null $post_id The result of the post ID lookup. Null to indicate no lookup has been attempted.
@param string $url The URL being looked up.

#5 in reply to: ↑ 4 @apermo
4 weeks ago

Replying to peterwilsoncc:

I think the filter in the proposed patch is good but the docblock improvements:

Filters the attachment ID to allow short-circuit the function.

Allows plugins to short-circuit attachment ID lookups. Plugins making use of this function should return:

  • 0 (integer) to indicate the attachment is not found
  • attachment ID (integer) to indicate the attachment ID found
  • null to indicate WordPress should proceed with the lookup.

Warning: The post ID may be null or zero, both of which cast to a boolean false. For information about casting to booleans see the {@link https://www.php.net/manual/en/language.types.boolean.php PHP documentation}. Use the === operator for testing the post ID when developing filters using this hook.

@param int|null $post_id The result of the post ID lookup. Null to indicate no lookup has been attempted.
@param string $url The URL being looked up.

I like the proposed changes, since it adds a fool proofness to the filter.

This ticket was mentioned in PR #7456 on WordPress/wordpress-develop by @peterwilsoncc.


2 weeks ago
#6

  • Keywords has-unit-tests added

Core-61383

Existing patch with updated docblock and some tests.

#7 @peterwilsoncc
2 weeks ago

  • Keywords has-unit-tests removed
  • Owner changed from antpb to peterwilsoncc

Assinging to self to update the docblock in the patch

#8 @peterwilsoncc
2 weeks ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 59118:

Media: Add short-circuit filter to attachment_url_to_postid().

Introduces the filter pre_attachment_url_to_postid to allow developers to short-circuit the function attachment_url_to_postid().

The return values are expected to be an attachment ID, zero (0) to indicate no attachment was found or null to indicate the function should proceed as usual.

The function performs an expensive database query so developers making use of the function frequently may wish to use a custom table with appropriate indexes to reduce the load on their database server.

Props antpb, apermo, audrasjb, joedolson.
Fixes #61383.

Note: See TracTickets for help on using tickets.