bp_friend_get_total_requests_count() could use already cached data

[wpdennis]

​bp_friend_get_total_requests_count() calls ​BP_Friends_Friendship::get_friend_user_ids( $user_id, true ):

return apply_filters( 'bp_friend_get_total_requests_count', count( BP_Friends_Friendship::get_friend_user_ids( $user_id, true ) ) );

But get_friend_user_ids() never caches the result. If it would use ​get_friendship_request_user_ids() instead, it would access already cached data:

return apply_filters( 'bp_friend_get_total_requests_count', count( BP_Friends_Friendship::get_friendship_request_user_ids( $user_id ) ) );

[DJPaul]

I *think* you're right. There's some logic in the suggested function that I think covers the use case of the first. If so, we probably should see if it's appropriate to deprecate one of the functions and convert it into a wrapper.

This would be another nice cache win for 2.3, so I'm moving it, as it doesn't seem too complicated to do.

[Mamaduka]

Caching in get_friendship_request_user_ids() was introduced in r8105, as a part of object cache improvement for friends component.

If we're going to deprecated one of those function, I think get_friendship_request_user_ids() should be our choice. Since BP_Friends_Friendship::get_friend_user_ids() can return same values and more. But this gonna make caching in later function bit complicated, where we should change cache group based on args.

I would love to here hear 2nd option on this as well.

P.S. What wpdennis suggests is pretty straightforward and happy to provide patch for it.

[boonebgorges]

If we're going to deprecated one of those function, I think get_friendship_request_user_ids() should be our choice. Since BP_Friends_Friendship::get_friend_user_ids() can return same values and more. But this gonna make caching in later function bit complicated, where we should change cache group based on args.

Agreed. The caching will be more complicated but will have benefits beyond this use case. If we're going to add caching here, we'll need a decent set of unit tests to demonstrate invalidation on new requests, defriending, user spam/delete, etc.

[johnjamesjacoby]

mamaduka: if you don't mind putting a patch together (with some tests) that would be very awesome of you.

I think the direction Boone is suggesting makes sense, though if it's easier to keep it broken up into multiple methods while this is being enhanced, I'm okay with that approach too, and we can always polish it up to be beautiful later, once we've confirmed the individual pieces are working correctly. Whatever is easier, really.

[Mamaduka]

Thanks @r-a-y for moving this to 2.4. Didn't get much time to work on this patches. Plus change require additional unit tests. Will make sure this one gets in core for 2.4 cycle.

[DJPaul]

We're closing this ticket because it has not received any contribution or comments for at least two years. We have decided that it is better to close tickets that are good ideas, which have not gotten (or are unlikely to get) contributions, rather than keep things open indefinitely. This will help us share a more realistic roadmap for BuddyPress with you.

Everyone very much appreciates the time and effort that you spent sharing your idea with us. On behalf of the entire BuddyPress team, thank you.

If you feel strongly that this enhancement should still be added to BuddyPress, and you are able to contribute effort towards it, we encourage you to re-open the ticket, or start a discussion about it in our Slack channel. Please consider that time has proven that good ideas without contributions do not get built.

For more information, see ​https://bpdevel.wordpress.com/2018/01/21/our-awaiting-contributions-milestone-contains/
or find us on Slack, in the #buddypress channel: ​https://make.wordpress.org/chat/