Doc count caching redundant?

[gyrus]

We've had repeated issues with the docs count for groups. I found a bug which was fixed (https://github.com/boonebgorges/buddypress-docs/issues/605). I thought that clearing the cached counts (bp-docs-count in groups meta) would then bring things back into line, but there are more issues.

We've added the docs count to the 'Documents' tab in groups, using the bp_get_options_nav_nav-document hook.

I found it necessary to add hooks to bp_docs_after_successful_save and bp_docs_doc_deleted to delete that cached count in the meta (so it got refreshed next time the group was viewed).

However, while those hooks work, we still ended up with wrong counts in the cached meta.

The underlying issue seems to be that docs can have many different access settings, and the cached bp-docs-count is set to the number of docs that can be seen by the next visitor.

Am I missing something? I'm not sure how the docs count is intended to be used, but for our purposes, we're just going to have to do a 'live' calculation for every request. It seems too complex to go into caching the value for different scenarios (logged in / not logged in, member of group / not member of group, etc.).

[boonebgorges]

The underlying issue seems to be that docs can have many different access settings, and the cached bp-docs-count is set to the number of docs that can be seen by the next visitor.

Yes, this sounds right, and it seems like a problem. I think it hasn't been more widely reported because doc count isn't shown anywhere by default - it used to be shown in various places, but we disabled it for reasons that escape me today.

As a workaround for the complexity of deleting caches all the time, you might consider the following:

add_filter( 'bp_docs_get_doc_count', function( $count, $item_id, $item_type ) {
    return bp_docs_update_doc_count( $item_id, $item_type );
}, 10, 3 );

Counts cached in user/group meta will continue to be wrong, but the value returned from bp_docs_get_doc_count() should always be the recalculated value.

Ripping the meta-based cache out altogether is the simplest fix here. If we thought it worthwhile to add a caching layer around it - which it probably is, if you're displaying these items everywhere - you'd probably need to do it on a per-user basis. So, something like:

$last_changed = wp_cache_get_last_changed( 'bp_docs_counts' );
$cache_key = bp_loggedin_user_id() . '_' . $item_type . '_' . $item_id . '_' . $last_changed;
$cached = wp_cache_get( $cache_key, 'bp_docs_counts' );
if ( false === $cached ) {
    // get a fresh count
    wp_cache_set( $cache_key, $count, 'bp_docs_counts' );
}

Then you can just delete the last_changed incrementor in the bp_docs_counts group to invalidate all count caches.

Since this is not currently used in Docs (though correct me if I'm wrong!) I'm not sure I can work on this improvement in the short term. But if you're interested in working on a PR, I think that the strategy described here would work.

[gyrus]

Many thanks, for now I've simply replicated bp_docs_update_doc_count(), omitting the meta stuff. I'll let you know if we come back to it and want to contribute something more comprehensive.