WordPress.org

Make WordPress Core

Opened 14 months ago

Last modified 5 months ago

#38280 new enhancement

Make term count for a specific object type available

Reported by: desrosj Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.7
Component: Taxonomy Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

Term count is the total number of relationships with no context of the object types the relationships belong to.

Currently, the best way to determine the count of a term for a specific object type is to run a WP_Query with a WP_Tax_Query for that term.

$term_count_query = new WP_Query( array(
        'post_type' => 'some-post-type',
        'tax_query' => array(
                array(
                        'taxonomy' => 'some-taxonomy',
                        'field' => 'slug',
                        'terms' => array( 'term-slug' )
                ),
        ),
        'posts_per_page' => 1,
) );
$term_count_for_post_type = $term_count_query->found_posts;

Now that term meta is in core, it would be great if these counts were stored in term meta for easy access.

To start, these counts could be available through a function. To go a step further, an argument could be added to get_terms() and other functions to filter the term counts and reflect the object types terms are being grabbed for.

Attachments (6)

38280.diff (3.6 KB) - added by desrosj 14 months ago.
Stores object type specific counts in term meta when term counts are updated. Adds wp_get_term_count_for_object_type() to retrieve term counts.
38280.2.diff (6.1 KB) - added by desrosj 12 months ago.
Updated ticket adds unit tests, casts get_term_meta() call into an array to prevent PHP notice, improved function Docblock.
38280.3.diff (8.0 KB) - added by desrosj 12 months ago.
38280.4.diff (9.8 KB) - added by desrosj 12 months ago.
38280.5.diff (11.7 KB) - added by desrosj 9 months ago.
Updated patch adds another meta key for tracking the object types that were counted. Also removes all previously counted object types each time to prevent abandoned meta keys.
38280.6.diff (12.1 KB) - added by boonebgorges 5 months ago.

Download all attachments as: .zip

Change History (16)

@desrosj
14 months ago

Stores object type specific counts in term meta when term counts are updated. Adds wp_get_term_count_for_object_type() to retrieve term counts.

@desrosj
12 months ago

Updated ticket adds unit tests, casts get_term_meta() call into an array to prevent PHP notice, improved function Docblock.

@desrosj
12 months ago

#1 @desrosj
12 months ago

Talked this over with @johnbillion at WordCamp US. He suggested single meta entries for each object type. This would unlock the potential for sorting terms by object type counts.

The newest patch reflects this change, and also adds unit tests.

The patch works and I can verify that through testing it and checking the database/displaying counts and function results. The tests all pass when run directly (using phpunit -group 38280 or phpunit -group taxonomy), but for some reason they fail when running all unit tests. A second set of eyes would be appreciated.

#2 @desrosj
12 months ago

Also, the attached patch follows the following methodology:

  • Do not create meta entry if one object type is registered on a taxonomy.
  • If two or more object types are registered, only store a meta entry for object types who's counts are greater than 0.
  • The term count remains the cumulative count for all relationships.

#3 @johnbillion
12 months ago

This is looking good. Some feedback:

  • This function will return 0 for terms that haven't yet had their post count cache populated (ie. all terms that already exist when the site gets upgraded to WP 4.8, for example). Should the term count its posts if the term meta cache doesn't yet exist?
  • The $wpdb->prepare() call added on line 3427 of src/wp-includes/taxonomy.php should properly prepare $type instead of concatenating it into the SQL.
  • There should be a test that asserts that the count is correctly updated when terms are removed from an object with wp_remove_object_terms().

@desrosj
12 months ago

#4 @desrosj
12 months ago

Updated patch to address all three items.

Bullet point one made me think of a couple scenarios that could result in some abandoned meta caches:

  • What if a taxonomy is removed from a post type?
  • What if a post type is removed?

Should we check for these some how?

#5 @desrosj
10 months ago

  • Keywords needs-refresh has-patch has-unit-tests added

#7 @desrosj
10 months ago

  • Keywords needs-refresh removed

Patch actually does not need a refresh. Still having the issue of tests passing when the taxonomy or 38280 group is run, but failing when all tests are run though.

#8 @boonebgorges
9 months ago

@desrosj Nice job on this patch so far.

The comment // When a count for an object type does not exist, if any other meta caches exist we can assume 0. makes me nervous. I guess this is because you're not storing 0s, and the presence a count for another object type means that WP has recently done a relevant count. I worry that this is going to be subject to weird race conditions or data integrity problems. (For example: what if a taxonomy originally assigned to 'post' becomes assigned to array( 'post', 'page' )?) How about some combination of your original array-based approach and the one-entry-per-object-type approach? There'd be a single key containing a list of the object types for which a count exists, but the counts themselves would be stored in their own rows. This way, if $type appears in the array, and if get_term_meta( $term_id, '_wp_object_count_' . $type, true ) returned false for a $type, we can be confident that $type has indeed been counted.

Bullet point one made me think of a couple scenarios that could result in some abandoned meta caches

These situations are rare enough that I wouldn't worry about it too much. As a precaution, _update_post_term_count() could delete_post_meta() for all of a post's existing counts before generating and saving new ones (another argument for having a separate list of the object types that have been counted). This way, the stale data will be removed during the next count.

@desrosj
9 months ago

Updated patch adds another meta key for tracking the object types that were counted. Also removes all previously counted object types each time to prevent abandoned meta keys.

This ticket was mentioned in Slack in #core by desrosj. View the logs.


8 months ago

#10 @boonebgorges
5 months ago

38280.6.diff does some formatting cleanup, including some slight reorganization of return statements in wp_get_term_count_for_object_type() to reduce the amount of if indentation.

This reorganization shows that there are some cases in which wp_get_term_count_for_object_type() doesn't return a value at all. Should we be returning 0 as a default value? The fact that these cases didn't come up makes me think we're missing some cases in the unit tests.

Can you explain this logic to me?

 	* If the object type has been counted, and other counts exist in meta, we can 
 	* assume this term has 0 relationships for this object type. 

Why is it necessary to check for *other* counts? Shouldn't it be enough for the type to be listed in _wp_counted_object_types? If there's a scenario where these values might get out of whack, we should (a) have a test that describes it, and/or (b) defend against that scenario :)

The invalidation part is looking pretty good to me.

Note: See TracTickets for help on using tickets.