Improvements to object caching in bp-activity

[r-a-y]

Similar to #5407, the activity component uses object caching in some places, but it's incomplete and in some cases appears to be incorrect.

This is a general ticket for commits that address the issue for the 2.0 cycle.

[r-a-y]

In 7997:

Properly set object cache for a new BP_Activity_Activity object.

This commit also renames the activity cache key and group to be
consistent with the groups component. (See r7956.)

See #5434.

[boonebgorges]

In 8004:

Clear activity item cache when it receives comments

r7997 fixed a bug was causing single activity items not to be cached properly
when a BP_Activity_Activity object was instantiated. This fix uncovered a bug
in activity commenting, where the activity parent was cached as childless, and
the cache was not invalidated when a new comment was posted.

See #5434

[boonebgorges]

In 8005:

Normalize the cache keys and group name used for activity comments

wp_cache_get( $aid, 'bp_activity_comments' )

is more in line with single item caching across other components.

See r7956, r7997

See #5434

[aaclayton]

Hey boonebgorges, thanks for your work on this. I think it might still need a bit of refining though. I'm trying out the trunk version (as of 5pm EST on 3/4) and I'm still having activity comments being cached improperly. When I post a new activity comment BuddyPress is still not deleting the cached object for the activity.

This occurs both on parent activities that are childless and those which already have child comments. I've verified the bug using both Memcached object cache and verious object caching methods in W3TC.

I'll see if I can figure out where the issue is, but you've had more of an eye on this ticket than myself. Just wanted to give you a heads up that I'm still experiencing failure to invalidate activity comment caching.

[boonebgorges]

Thanks, aaclayton. Steps to reproduce (or ideally a unit test demonstrating the problem) would be great. We'll continue to work on this ticket leading up to release.

[aaclayton]

I'll try to keep digging, but I'm 100% sure it's a caching related issue. When I retrieve activity comments from wp_cache_get( $activity_id, 'bp_activity_comments' ) in the get_activity_comments function it is pulling a cached set of comments which does not include new comment submissions. If I replace this line with $comments = false; to bypass caching the issue is resolved.

I verified this on some of the default WordPress themes for my installation, so it's not a theme-specific issue.

The problem seems to be happening in the bp_activity_new_comment() function. I've looked at the source and wp_cache_delete( $parent_id, 'bp_activity_comments' ); should be invalidating that cached entry, but for some reason it is not. If I precede that line with a wp_cache_flush() for some heavyhanded testing it also fixes the problem.

I'm not sure why the wp_cache_delete in line 1244 does not invalidate the cache...but for some reason (at least on my installation) it is not.

[boonebgorges]

In 8075:

Normalize cache priming in BP_Activity_Activity::get_activity_data()

No need to reproduce logic in bp_get_non_cached_ids(). This brings the caching
strategy in line with other components.

See #5434

[boonebgorges]

In 8076:

Improved cache invalidation when posting activity comments

• Because activity comment caches are stored only with the top-level parent, we should be invalidating the bp_activity_comments cache for that item, not for the immediate parent
• When a new comment is created, clear the bp_activity cache for each ancestor

See #5434

[boonebgorges]

aaclayton - I had a closer look, and there were some odd issues in the way that comment invalidation was happening. (It goes back to the fact that the way our comments are stored is itself kinda odd. Oh well.) My tests are passing now, but that only leaves me about 80% confident, because it could be that the tests are incorrect :) Could you please let me know what you find?

[aaclayton]

Excellent, I'll try to make some time to test this later tonight, tomorrow at the latest! Thanks very much for looking into this Boone.

[boonebgorges]

Nearly all of the places where it is convenient and relevant to use persistent caching in bp-activity have been covered at this point. I'm going to mark this ticket fixed. For specific issues beyond those fixed above, let's open new tickets.