Add caching to count_user_posts.

[spacedmonkey]

Simple caching based on last updated values changing.

Trac ticket: https://core.trac.wordpress.org/ticket/39242

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @jonnynews.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

Core Committers: Use this line as a base for the props when committing in SVN:

Props spacedmonkey, peterwilsoncc, mamaduka, flixos90, johnjamesjacoby, swissspidy, logicrays, dilip2615.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[github-actions]

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

• The Plugin and Theme Directories cannot be accessed within Playground.
• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

[Mamaduka]

Thank you, @spacedmonkey!

PR looks good and matches similar optimizations you worked on a couple of years ago - dfcfe0b.

I trust your judgment regarding unit test coverage.

[peterwilsoncc]

A couple of suggestions inline.

I added the following to tests/phpunit/tests/user/countUserPosts.php while reviewing and testing the code.

/**
 * User count should work for users that don't exist but have posts assigned.
 *
 * @ticket 39242
 */
public function test_count_user_posts_for_non_existent_user() {
	$next_user_id = self::$user_id + 1;

	// Assign post to next user.
	self::factory()->post->create(
		array(
			'post_author' => $next_user_id,
			'post_type'   => 'post',
		)
	);

	$next_user_post_count = count_user_posts( $next_user_id );
	$this->assertSame( '1', $next_user_post_count, 'Non-existent user is expected to have count of one post.' );
}

/**
 * Cached user count value should be accurate after user is created.
 *
 * @ticket 39242
 */
public function test_count_user_posts_for_user_created_after_being_assigned_posts() {
	$next_user_id = self::$user_id + 1;

	// Assign post to next user.
	self::factory()->post->create(
		array(
			'post_author' => $next_user_id,
			'post_type'   => 'post',
		)
	);

	// Cache the user count.
	count_user_posts( $next_user_id );

	// Create user.
	$real_next_user_id = self::factory()->user->create(
		array(
			'role' => 'author',
		)
	);

	$this->assertSame( $next_user_id, $real_next_user_id, 'User ID should match calculated value' );
	$this->assertSame( '1', count_user_posts( $next_user_id ), 'User is expected to have count of one post.' );
}

/**
 * User count cache should be hit regardless of post type order.
 *
 * @ticket 39242
 */
public function test_cache_should_be_hit_regardless_of_post_type_order() {
	// Prime Cache
	count_user_posts( self::$user_id, array( 'wptests_pt', 'post' ) );

	$query_num_start = get_num_queries();
	count_user_posts( self::$user_id, array( 'post', 'wptests_pt' ) );
	$total_queries = get_num_queries() - $query_num_start;

	$this->assertSame( 0, $total_queries );
}

/**
 * User count cache should be hit for string and array of post types.
 *
 * @ticket 39242
 */
public function test_cache_should_be_hit_for_string_and_array_equivalent_queries() {
	// Prime Cache
	count_user_posts( self::$user_id, 'post' );

	$query_num_start = get_num_queries();
	count_user_posts( self::$user_id, array( 'post' ) );
	$total_queries = get_num_queries() - $query_num_start;

	$this->assertSame( 0, $total_queries );
}

[jonnynews]

@peterwilsoncc Thanks for the tests. Complete legend. Added your feedback.

[peterwilsoncc]

Thanks @spacedmonkey, this looks good to me.

I missed a coupe of things while writing up the tests so have pushed a minor change and taken the opportunity to merge in trunk while doing so.

[spacedmonkey]

Suggested change

	$post_type = (array) $post_type;
	$post_type = array_filter( array_unique( (array) $post_type ), 'post_type_exists' );

@peterwilsoncc What do you think of this? Overkill?

[peterwilsoncc]

array_unique() would help, get_posts_by_author_sql() already ensures the post type exists so no need to do it here.

[jonnynews]

Well that's the point, this code would result in hitting the cache, as the cache key would match.

So

count_user_posts( 1, array( 'post', 'invalid' ) );

Or

count_user_posts( 1, array( 'post', '' ) );

Would be the same as

count_user_posts( 1, array( 'post') );

[peterwilsoncc]

That's what I am saying, they should already hit the cache as the it's generated based on the SQL without taking in to account the arguments passed to the function.

[spacedmonkey]

@JJJ @swissspidy Are you interested in reviewing PR?

[felixarntz]

@spacedmonkey This looks great, thanks!

[swissspidy]

@spacedmonkey Had a quick glance earlier today and I think it's a neat way to add this enhancement without having to make too many changes like in the other PRs. You have plenty of approvals already though :-)

[spacedmonkey]

@spacedmonkey Had a quick glance earlier today and I think it's a neat way to add this enhancement without having to make too many changes like in the other PRs. You have plenty of approvals already though :-)

It more to check to see if you are strongly against it. I know you were worried about this cache being invalidated a lot, but honestly I dont see a way around it.

[peterwilsoncc]

Indentation

[peterwilsoncc]

Suggested change

	$this->assertSame( 0, $total_queries );
	$this->assertSame( 0, $total_queries, 'Cache is expected to be hit for equivalent queries with duplicate post types' );

[JJJ]

@JJJ @swissspidy Are you interested in reviewing PR?

Happy to!

Incrementally, I like it. Specifically:

• Using last-changed is right
• Tests look right

Tangential thoughts while reviewing...

• Do we validate $post_type in any other similar queries?
• Does it matter if the post types are: not registered, private, internal, etc...
• Would adding a filter on $post_type be useful, or expected?

---

Long term, I also like that @swissspidy included count_many_users_posts() in #7993, because it has potential - I think – to speed up the Users list table.

Perhaps that gets its own issue/ticket?

[JJJ]

👍

[peterwilsoncc]

• Do we validate $post_type in any other similar queries?

Sorry, I'm unclear what you are asking here but I possibly answer your question next...

• Does it matter if the post types are: not registered, private, internal, etc...

It doesn't within count_user_posts() as the data is used by get_posts_by_author_sql() which validates the post type exists.

• Would adding a filter on $post_type be useful, or expected?

Possibly useful but as a follow up ticket.

[devsupportlogicrays]

we can get the data using the bellow code and implement on it.

$user_id = get_current_user_id(); 
$post_count = count_user_posts($user_id);

[dilipom13]

<?php

/**
 * Tests for count_user_posts function.
 *
 * @group user
 *
 * @ticket 39242
 */
class Tests_User_CountUserPosts extends WP_UnitTestCase {

	/**
	 * User count should work for users that don't exist but have posts assigned.
	 */
	public function test_count_user_posts_for_non_existent_user() {
		$next_user_id = self::$user_id + 1;

		// Create a post assigned to a non-existent user.
		self::factory()->post->create(
			array(
				'post_author' => $next_user_id,
				'post_type'   => 'post',
			)
		);

		$this->assertEquals( 1, count_user_posts( $next_user_id ) );
	}

	/**
	 * Cached user count value should be accurate after user is created.
	 */
	public function test_count_user_posts_for_user_created_after_being_assigned_posts() {
		$next_user_id = self::$user_id + 1;

		// Assign a post to a non-existent user.
		self::factory()->post->create(
			array(
				'post_author' => $next_user_id,
				'post_type'   => 'post',
			)
		);

		// Cache the user count before creating the user.
		$cached_count = count_user_posts( $next_user_id );

		// Now create the user.
		$real_next_user_id = self::factory()->user->create(
			array(
				'role' => 'author',
			)
		);

		$this->assertEquals( $next_user_id, $real_next_user_id );
		$this->assertEquals( $cached_count, count_user_posts( $next_user_id ) );
	}

	/**
	 * User count cache should be hit regardless of post type order.
	 */
	public function test_cache_should_be_hit_regardless_of_post_type_order() {
		// Prime the cache by running the function once.
		count_user_posts( self::$user_id, array( 'wptests_pt', 'post' ) );

		// Check database query count before running again.
		global $wpdb;
		$start_queries = $wpdb->num_queries;

		// Query with different order of post types.
		count_user_posts( self::$user_id, array( 'post', 'wptests_pt' ) );

		$end_queries = $wpdb->num_queries;

		// Expecting no new queries, since cache should be hit.
		$this->assertEquals( $start_queries, $end_queries );
	}

	/**
	 * User count cache should be hit for string and array of post types.
	 */
	public function test_cache_should_be_hit_for_string_and_array_equivalent_queries() {
		// Prime the cache.
		count_user_posts( self::$user_id, 'post' );

		global $wpdb;
		$start_queries = $wpdb->num_queries;

		// Query using an array instead of a string.
		count_user_posts( self::$user_id, array( 'post' ) );

		$end_queries = $wpdb->num_queries;

		// Expecting no new queries due to cache.
		$this->assertEquals( $start_queries, $end_queries );
	}
}``

We can also use in tests/phpunit/tests/user/countUserPosts.php

I refactored the test methods for better clarity while keeping the functionality intact. I also used assertEquals() where type coercion is acceptable and replaced unnecessary string comparisons to make the tests more robust. Additionally, I added $wpdb->num_queries to track database queries before and after calls, ensuring that the cache is being utilized properly. Let me know if you need any further refinements!

[peterwilsoncc]

@dilipom13 Each of those test cases are currently covered by the test suite.

WordPress aims to maintain the return type of functions, so the tests need to use assertSame(). ::test_count_user_posts_for_user_created_after_being_assigned_posts() needs to use the auto_increment value from the database as the value can change as users are created and subsequently deleted in the test suite.