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

Improve cache and performance of get_user_data_from_wp_global_styles. #42152

Closed
wants to merge 2 commits into from

Conversation

spacedmonkey
Copy link
Member

What?

Trac ticket: https://core.trac.wordpress.org/ticket/55392
Orignal PR - WordPress/wordpress-develop#2414

Why?

How?

Testing Instructions

Screenshots or screencast

@spacedmonkey spacedmonkey self-assigned this Jul 5, 2022
$gs_query = new WP_Query();
$recent_posts = $gs_query->query( $args );
if ( count( $recent_posts ) === 1 ) {
$user_cpt = get_post( $recent_posts[0], ARRAY_A );
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes an issue, where a WP_Post object is returned.

),
true
);
if ( ! is_wp_error( $cpt_post_id ) ) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check should always have been here. WP_Insert_post can sometimes return a WP_Error.

Copy link
Contributor

@draganescu draganescu Jul 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if is_wp_error is true? It seems it remains empty array and we count on array with length zero to be evaluated to false. Could we make this clear in code?

);

$cache_key = sprintf( 'wp_global_styles_%s', md5( serialize( $args ) ) );
$post_id = (int) get_transient( $cache_key );
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use transient is better for performance here. Most sites do not run object caches. Transinets save to options table and will have 3 database queries per page load.

Comment on lines +65 to +67
if ( $post_id > 0 && in_array( get_post_status( $post_id ), (array) $post_status_filter, true ) ) {
return get_post( $post_id, ARRAY_A );
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improve handling of post status changed. If this post is deleted or changed to draft.

'post_status' => 'publish',
'post_title' => __( 'Custom Styles', 'default' ),
'post_type' => $post_type_filter,
'post_name' => 'wp-global-styles-' . urlencode( wp_get_theme()->get_stylesheet() ),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spacedmonkey
Copy link
Member Author

Love to get this into WP 6.1 cc @draganescu

@peterwilsoncc
Copy link
Contributor

@spacedmonkey Now that WP_Query has caching, is this still required and/or as urgent?

@spacedmonkey
Copy link
Member Author

Closed in favour of #44290

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants