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

Featured image performance #1400

Merged
merged 3 commits into from
Jan 24, 2017
Merged

Featured image performance #1400

merged 3 commits into from
Jan 24, 2017

Conversation

rclations
Copy link

The Problem

We're running a filter on post body content that checks if the post starts with an image, and if that image matches the post's featured image, removes the content from the post display (to prevent duplicate displays of that featured image).

The issue is the method we're running that check. Currently, we're getting the source for the image in the post body, then running a very slow query to lookup that image's ID in the database. This type of query/logic cannot continue to exist for performance reasons.

The solution

Every image inserted by the media editor should have a wp-image-## class on it, where ## is the id of the media file. By pulling this ID out with regex, we can achieve the same effect, but avoid expensive queries.

Potential Downsides

If the html for the image has been manipulated or inserted by hand so that it does not include the wp-image-## class, then this obviously won't work.

@rclations rclations added the priority: high Either blocks work on a priority-normal task or a solution here informs other work. label Jan 23, 2017
@rclations rclations added this to the hotfix milestone Jan 23, 2017
@benlk
Copy link
Collaborator

benlk commented Jan 24, 2017

Tests are failing on this, for a test that seems relevant:

1) PostTemplatesTestFunctions::test_insert_image_no_thumb
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'<h2>Headings</h2>
+'<p><img src="http://example.org/wp-content/uploads/2017/01/cat-771x510.jpg" alt="1559758083_cef4ef63d2_o" width="771" height="475" class="alignnone size-large" /></p>
+<h2>Headings</h2>
 <p>Integer posuere erat a ante venenatis dapibus posuere velit aliquet. Sed posuere consectetur est at lobortis. Nulla vitae elit libero, a pharetra augue. Fusce dapibus, tellus ac cursus commodo, tortor mauris condimentum nibh, ut fermentum massa justo sit amet risus. Donec sed odio dui.</p>'
/tmp/wordpress/src/wp-content/themes/Largo/tests/inc/test-post-templates.php:79

@rclations
Copy link
Author

okay so the test we had wasn't utilizing the wp-image-[id] class that's now required for this to work. Last commit updates the test to include this

@aschweigert aschweigert merged commit 3ffc26a into master Jan 24, 2017
@aschweigert aschweigert deleted the featured-image-performance branch January 24, 2017 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: high Either blocks work on a priority-normal task or a solution here informs other work.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants