Changeset 7956

Timestamp:

02/21/2014 05:51:10 PM (11 years ago)

Author:

boonebgorges

Message:

Overhaul the way that individual group objects are cached

The existing implementation was broken in a number of ways. Most critically,
user-specific data (like is_member) was being stored in the persistent object
cache without respect to user.

This refactor excludes the user-specific data from being cached along with the
group object. It also reworks the way that items are keyed in the cache, for
greater consistency with WordPress and the other BP components.

See #5407

Location:

trunk

Files:

• bp-groups/bp-groups-cache.php (modified) (2 diffs)
• bp-groups/bp-groups-classes.php (modified) (4 diffs)
• bp-groups/bp-groups-functions.php (modified) (1 diff)
• tests/testcases/groups/cache.php (modified) (2 diffs)
• tests/testcases/groups/class-bp-groups-group.php (modified) (1 diff)
• tests/testcases/groups/functions.php (modified) (1 diff)

trunk/bp-groups/bp-groups-cache.php

@@ -65 +65 @@
  */
 function bp_groups_delete_group_cache( $group_id = 0 ) {
-    wp_cache_delete( 'bp_groups_group_' . $group_id . '_load_users', 'bp' );
-    wp_cache_delete( 'bp_groups_group_' . $group_id . '_noload_users', 'bp' );
+    wp_cache_delete( $group_id, 'bp_groups' );
 }
 add_action( 'groups_delete_group',     'bp_groups_delete_group_cache' );
@@ -79 +78 @@
  */
 function bp_groups_delete_group_cache_on_metadata_change( $meta_id, $group_id ) {
-    wp_cache_delete( 'bp_groups_group_' . $group_id . '_load_users', 'bp' );
-    wp_cache_delete( 'bp_groups_group_' . $group_id . '_noload_users', 'bp' );
+    wp_cache_delete( $group_id, 'bp_groups' );
 }
 add_action( 'updated_group_meta', 'bp_groups_delete_group_cache_on_metadata_change', 10, 2 );

trunk/bp-groups/bp-groups-classes.php

@@ -177 +177 @@
         global $wpdb, $bp;
 
-        if ( $group = $wpdb->get_row( $wpdb->prepare( "SELECT g.* FROM {$bp->groups->table_name} g WHERE g.id = %d", $this->id ) ) ) {
-            $this->id                 = $group->id;
-            $this->creator_id         = $group->creator_id;
-            $this->name               = stripslashes($group->name);
-            $this->slug               = $group->slug;
-            $this->description        = stripslashes($group->description);
-            $this->status             = $group->status;
-            $this->enable_forum       = $group->enable_forum;
-            $this->date_created       = $group->date_created;
+        $group = wp_cache_get( $this->id, 'bp_groups' );
+        if ( false === $group ) {
+            $group = $wpdb->get_row( $wpdb->prepare( "SELECT g.* FROM {$bp->groups->table_name} g WHERE g.id = %d", $this->id ) );
+
+            if ( empty( $group ) ) {
+                $this->id = 0;
+                return;
+            }
+
+            $this->id           = $group->id;
+            $this->creator_id   = $group->creator_id;
+            $this->name         = stripslashes( $group->name );
+            $this->slug         = $group->slug;
+            $this->description  = stripslashes( $group->description );
+            $this->status       = $group->status;
+            $this->enable_forum = $group->enable_forum;
+            $this->date_created = $group->date_created;
 
             if ( ! empty( $this->args['populate_extras'] ) ) {
                 $this->last_activity      = groups_get_groupmeta( $this->id, 'last_activity' );
                 $this->total_member_count = groups_get_groupmeta( $this->id, 'total_member_count' );
-                $this->is_member          = BP_Groups_Member::check_is_member( bp_loggedin_user_id(), $this->id );
-                $this->is_invited         = BP_Groups_Member::check_has_invite( bp_loggedin_user_id(), $this->id );
-                $this->is_pending         = BP_Groups_Member::check_for_membership_request( bp_loggedin_user_id(), $this->id );
+
+                // Get group admins and mods
+                $admin_mods = $wpdb->get_results( apply_filters( 'bp_group_admin_mods_user_join_filter', $wpdb->prepare( "SELECT u.ID as user_id, u.user_login, u.user_email, u.user_nicename, m.is_admin, m.is_mod FROM {$wpdb->users} u, {$bp->groups->table_name_members} m WHERE u.ID = m.user_id AND m.group_id = %d AND ( m.is_admin = 1 OR m.is_mod = 1 )", $this->id ) ) );
+
+                foreach ( (array) $admin_mods as $user ) {
+                    if ( (int) $user->is_admin ) {
+                        $this->admins[] = $user;
+                    } else {
+                        $this->mods[] = $user;
+                    }
+                }
+
+                // Cache general (non-user-specific)
+                // information about the group
+                wp_cache_set( $this->id, $this, 'bp_groups' );
+            }
+
+            // Set user-specific data
+            if ( ! empty( $this->args['populate_extras'] ) ) {
+                $this->is_member  = BP_Groups_Member::check_is_member( bp_loggedin_user_id(), $this->id );
+                $this->is_invited = BP_Groups_Member::check_has_invite( bp_loggedin_user_id(), $this->id );
+                $this->is_pending = BP_Groups_Member::check_for_membership_request( bp_loggedin_user_id(), $this->id );
 
                 // If this is a private or hidden group, does the current user have access?
@@ -203 +230 @@
                     $this->user_has_access = true;
                 }
-
-                // Get group admins and mods
-                $admin_mods = $wpdb->get_results( apply_filters( 'bp_group_admin_mods_user_join_filter', $wpdb->prepare( "SELECT u.ID as user_id, u.user_login, u.user_email, u.user_nicename, m.is_admin, m.is_mod FROM {$wpdb->users} u, {$bp->groups->table_name_members} m WHERE u.ID = m.user_id AND m.group_id = %d AND ( m.is_admin = 1 OR m.is_mod = 1 )", $this->id ) ) );
-                foreach( (array) $admin_mods as $user ) {
-                    if ( (int) $user->is_admin )
-                        $this->admins[] = $user;
-                    else
-                        $this->mods[] = $user;
-                }
             }
-        } else {
-            $this->id = 0;
         }
     }
@@ -289 +305 @@
         do_action_ref_array( 'groups_group_after_save', array( &$this ) );
 
-        wp_cache_delete( 'bp_groups_group_' . $this->id, 'bp' );
+        wp_cache_delete( $this->id, 'bp_groups' );
 
         return true;
@@ -317 +333 @@
         do_action_ref_array( 'bp_groups_delete_group', array( &$this, $user_ids ) );
 
-        wp_cache_delete( 'bp_groups_group_' . $this->id, 'bp' );
+        wp_cache_delete( $this->id, 'bp_groups' );
 
         // Finally remove the group entry from the DB

trunk/bp-groups/bp-groups-functions.php

@@ -41 +41 @@
  */
 function groups_get_group( $args = '' ) {
-    $defaults = array(
+    $r = wp_parse_args( $args, array(
         'group_id'          => false,
         'load_users'        => false,
         'populate_extras'   => true,
+    ) );
+
+    $group_args = array(
+        'populate_extras' => $r['populate_extras'],
     );
 
-    $args = wp_parse_args( $args, $defaults );
-    extract( $args, EXTR_SKIP );
-
-    $cache_key = 'bp_groups_group_' . $group_id . ( $load_users ? '_load_users' : '_noload_users' );
-
-    if ( !$group = wp_cache_get( $cache_key, 'bp' ) ) {
-        $group_args = array(
-            'populate_extras'   => $populate_extras,
-        );
-
-        $group = new BP_Groups_Group( $group_id, $group_args );
-        wp_cache_set( $cache_key, $group, 'bp' );
-    }
+    $group = new BP_Groups_Group( $r['group_id'], $group_args );
 
     return apply_filters( 'groups_get_group', $group );

trunk/tests/testcases/groups/cache.php

@@ -74 +74 @@
 
         // Prime cache
-        groups_get_group( array( 'group_id' => $g ) );
+        groups_get_group( array( 'group_id' => $g, ) );
 
-        $this->assertNotEmpty( wp_cache_get( 'bp_groups_group_' . $g . '_noload_users', 'bp' ) );
+        $this->assertNotEmpty( wp_cache_get( $g, 'bp_groups' ) );
 
         // Trigger flush
         groups_update_groupmeta( $g, 'foo', 'bar' );
 
-        $this->assertFalse( wp_cache_get( 'bp_groups_group_' . $g . '_noload_users', 'bp' ) );
+        $this->assertFalse( wp_cache_get( $g, 'bp_groups' ) );
     }
 
@@ -95 +95 @@
         groups_get_group( array( 'group_id' => $g ) );
 
-        $this->assertNotEmpty( wp_cache_get( 'bp_groups_group_' . $g . '_noload_users', 'bp' ) );
+        $this->assertNotEmpty( wp_cache_get( $g, 'bp_groups' ) );
 
         // Trigger flush
         groups_update_groupmeta( $g, 'foo', 'baz' );
-        $this->assertFalse( wp_cache_get( 'bp_groups_group_' . $g . '_noload_users', 'bp' ) );
+        $this->assertFalse( wp_cache_get( $g, 'bp_groups' ) );
     }
 

trunk/tests/testcases/groups/class-bp-groups-group.php

@@ -644 +644 @@
 
     /**
+     * @group delete
+     * @group cache
+     */
+    public function test_delete_clear_cache() {
+        $g = $this->factory->group->create();
+
+        // Prime cache
+        groups_get_group( array( 'group_id' => $g, ) );
+
+        $this->assertNotEmpty( wp_cache_get( $g, 'bp_groups' ) );
+
+        $group = new BP_Groups_Group( $g );
+        $group->delete();
+
+        $this->assertFalse( wp_cache_get( $g, 'bp_groups' ) );
+    }
+
+    /**
+     * @group save
+     * @group cache
+     */
+    public function test_save_clear_cache() {
+        $g = $this->factory->group->create();
+
+        // Prime cache
+        groups_get_group( array( 'group_id' => $g, ) );
+
+        $this->assertNotEmpty( wp_cache_get( $g, 'bp_groups' ) );
+
+        $group = new BP_Groups_Group( $g );
+        $group->name = 'Foo';
+        $group->save();
+
+        $this->assertFalse( wp_cache_get( $g, 'bp_groups' ) );
+    }
+    /**
      * @group get_group_extras
      */

trunk/tests/testcases/groups/functions.php

@@ -549 +549 @@
         $this->assertNotEmpty( groups_add_groupmeta( $g, 'foo', 'baz' ) );
     }
+
+    /**
+     * @group groups_get_group
+     * @group cache
+     */
+    public function test_groups_get_group_cache_different_users() {
+        $g = $this->factory->group->create();
+        $u1 = $this->create_user();
+        $u2 = $this->create_user();
+        $this->add_user_to_group( $u1, $g );
+
+        $old_user = get_current_user_id();
+        $this->set_current_user( $u1 );
+
+        $group1 = groups_get_group( array( 'group_id' => $g, ) );
+
+        $this->set_current_user( $u2 );
+
+        $group2 = groups_get_group( array( 'group_id' => $g, ) );
+
+        $this->assertNotEquals( $group1, $group2 );
+
+        $this->set_current_user( $old_user );
+    }
 }