Changeset 8332

Timestamp:

04/30/2014 05:22:22 PM (11 years ago)

Author:

boonebgorges

Message:

Overhaul caching for user last_activity values

The last_activity methods in the BP_Core_User class were originally designed
without persistent caching in mind. In r8047, some minimal caching support was
introduced, but it was tacked on and incomplete. In particular, it depended
on an inconsistent structure of return values from the last_activity methods.
See #5128 for more background.

This changeset introduces a number of changes that make last_activity methods
perform more consistently, as well as full support for object caching.

• BP_Core_User::get_last_activity() always returns a multidimensional array of last_activity arrays, keyed by user_id. This remains true even when last_activity data is being fetched only for a single user.
• last_activity data is stored in the bp_last_activity cache bucket without the user-id-keyed wrapper.
• BP_Core_User::get_last_activity() now uses the same caching technique as other BuddyPress and WordPress queries: detect which requested items are not yet in the cache, prime the cache for those items, and then fetch all requested values directly from the cache to build a return value.

Fixes #5590

Props imath, johnjamesjacoby for initial patches

Location:

trunk

Files:

• bp-core/bp-core-classes.php (modified) (5 diffs)
• tests/testcases/core/class-bp-core-user.php (modified) (3 diffs)

trunk/bp-core/bp-core-classes.php

@@ -1322 +1322 @@
         global $wpdb;
 
-        if ( is_array( $user_id ) ) {
-            $user_ids = wp_parse_id_list( $user_id );
-        } else {
-            $user_ids = array( absint( $user_id ) );
-        }
+        // Sanitize and remove empty values
+        $user_ids = array_filter( wp_parse_id_list( $user_id ) );
 
         if ( empty( $user_ids ) ) {
@@ -1332 +1329 @@
         }
 
-        // get cache for single user only
-        // @todo Why only single user?
-        if ( ! is_array( $user_id ) ) {
-            $cache = wp_cache_get( $user_id, 'bp_last_activity' );
-
-            if ( false !== $cache ) {
-                return $cache;
-            }
-        }
-
-        $bp = buddypress();
-
-        $user_ids_sql = implode( ',', $user_ids );
-        $user_count   = count( $user_ids );
-
-        $last_activities = $wpdb->get_results( $wpdb->prepare( "SELECT id, user_id, date_recorded FROM {$bp->members->table_name_last_activity} WHERE component = %s AND type = 'last_activity' AND user_id IN ({$user_ids_sql}) LIMIT {$user_count}", $bp->members->id ) );
-
-        // Re-key
+        $uncached_user_ids = bp_get_non_cached_ids( $user_ids, 'bp_last_activity' );
+        if ( ! empty( $uncached_user_ids ) ) {
+            $bp = buddypress();
+
+            $user_ids_sql = implode( ',', $uncached_user_ids );
+            $user_count   = count( $uncached_user_ids );
+
+            $last_activities = $wpdb->get_results( $wpdb->prepare( "SELECT id, user_id, date_recorded FROM {$bp->members->table_name_last_activity} WHERE component = %s AND type = 'last_activity' AND user_id IN ({$user_ids_sql}) LIMIT {$user_count}", $bp->members->id ) );
+
+            foreach ( $last_activities as $last_activity ) {
+                wp_cache_set( $last_activity->user_id, array(
+                    'user_id'       => $last_activity->user_id,
+                    'date_recorded' => $last_activity->date_recorded,
+                    'activity_id'   => $last_activity->id,
+                ), 'bp_last_activity' );
+            }
+        }
+
+        // Fetch all user data from the cache
         $retval = array();
-        foreach ( $last_activities as $last_activity ) {
-            $retval[ $last_activity->user_id ] = array(
-                'user_id'       => $last_activity->user_id,
-                'date_recorded' => $last_activity->date_recorded,
-                'activity_id'   => $last_activity->id,
-            );
-        }
-
-        // If this was a single user query, update the cache
-        if ( ! is_array( $user_id ) && isset( $retval[ $user_id ] ) ) {
-            wp_cache_set( $user_id, $retval[ $user_id ], 'bp_last_activity' );
+        foreach ( $user_ids as $user_id ) {
+            $retval[ $user_id ] = wp_cache_get( $user_id, 'bp_last_activity' );
         }
 
@@ -1386 +1375 @@
         $activity = self::get_last_activity( $user_id );
 
-        if ( ! empty( $activity ) ) {
+        if ( ! empty( $activity[ $user_id ] ) ) {
             $updated = $wpdb->update(
                 $table_name,
@@ -1443 +1432 @@
             );
 
-            // setup activity array for caching
+            // set up activity array for caching
             // view the foreach loop in the get_last_activity() method for format
             $activity = array();
@@ -1454 +1443 @@
 
         // set cache
-        wp_cache_set( $user_id, $activity, 'bp_last_activity' );
+        wp_cache_set( $user_id, $activity[ $user_id ], 'bp_last_activity' );
 
         return $updated;

trunk/tests/testcases/core/class-bp-core-user.php

@@ -159 +159 @@
     /**
      * @group last_activity
+     * @group cache
      */
     public function test_get_last_activity_store_in_cache() {
@@ -176 +177 @@
     /**
      * @group last_activity
-     */
-    public function test_get_last_activity_in_cache_single_user() {
+     * @group cache
+     */
+    public function test_get_last_activity_store_in_cache_multiple_users() {
+        $u1 = $this->create_user();
+        $u2 = $this->create_user();
+        $time = bp_core_current_time();
+
+        // Cache is set during user creation. Clear to reflect actual
+        // pageload
+        wp_cache_delete( $u1, 'bp_last_activity' );
+        wp_cache_delete( $u2, 'bp_last_activity' );
+
+        // prime cache
+        $a = BP_Core_User::get_last_activity( array( $u1, $u2 ) );
+
+        $this->assertSame( $a[ $u1 ], wp_cache_get( $u1, 'bp_last_activity' ) );
+        $this->assertSame( $a[ $u2 ], wp_cache_get( $u2, 'bp_last_activity' ) );
+    }
+
+    /**
+     * @group last_activity
+     * @group cache
+     */
+    public function test_get_last_activity_from_cache_single_user() {
         $u    = $this->create_user();
         $time = bp_core_current_time();
@@ -183 +206 @@
         BP_Core_User::update_last_activity( $u, $time );
 
+        // Cache is set during user creation. Clear to reflect actual
+        // pageload
+        wp_cache_delete( $u, 'bp_last_activity' );
+
+        // Prime cache
+        $uncached = BP_Core_User::get_last_activity( $u );
+
+        // Fetch again to get from the cache
+        $cached = BP_Core_User::get_last_activity( $u );
+
+        $this->assertSame( $uncached, $cached );
+    }
+
+    /**
+     * @group last_activity
+     * @group cache
+     */
+    public function test_get_last_activity_in_cache_multiple_users() {
+        $u1 = $this->create_user();
+        $u2 = $this->create_user();
+        $time = bp_core_current_time();
+
+        BP_Core_User::update_last_activity( $u1, $time );
+        BP_Core_User::update_last_activity( $u2, $time );
+
         // Cache is set during user creation. Clear to reflect actual pageload
-        wp_cache_delete( $u, 'bp_last_activity' );
-
-        // prime cache
-        $a = BP_Core_User::get_last_activity( $u );
-        $b = BP_Core_User::get_last_activity( $u );
-
-        $this->assertSame( $a, $b );
+        wp_cache_delete( $u1, 'bp_last_activity' );
+        wp_cache_delete( $u2, 'bp_last_activity' );
+
+        // Prime cache
+        $uncached = BP_Core_User::get_last_activity( array( $u1, $u2 ) );
+
+        // Second grab will be from the cache
+        $cached = BP_Core_User::get_last_activity( array( $u1, $u2 ) );
+        $cached_u1 = wp_cache_get( $u1, 'bp_last_activity' );
+
+        $this->assertSame( $cached, $uncached );
     }