Ticket #7237: 7237.2.diff

src/bp-activity/bp-activity-cache.php

@@ -63 +63 @@
         }
 }
 add_action( 'bp_activity_deleted_activities', 'bp_activity_clear_cache_for_deleted_activity' );
+
+/**
+ * Reset 'last_changed' cache incrementor.
+ *
+ * @since 2.7.0
+ */
+function bp_activity_reset_cache_incrementor() {
+        wp_cache_delete( 'last_changed', 'bp_activity' );
+}
+add_action( 'bp_activity_delete', 'bp_activity_reset_cache_incrementor' );
+add_action( 'bp_activity_add', 'bp_activity_reset_cache_incrementor' );

src/bp-activity/classes/class-bp-activity-activity.php

@@ -645 +645 @@
                          */
                         $activity_ids_sql = apply_filters( 'bp_activity_paged_activities_sql', $activity_ids_sql, $r );
 
-                        $activity_ids = $wpdb->get_col( $activity_ids_sql );
+                        $cached = bp_core_get_incremented_cache( $activity_ids_sql, 'bp_activity' );
+                        if ( false === $cached ) {
+                                $activity_ids = $wpdb->get_col( $activity_ids_sql );
+                                bp_core_set_incremented_cache( $activity_ids_sql, 'bp_activity', $activity_ids );
+                        } else {
+                                $activity_ids = $cached;
+                        }
 
                         $retval['has_more_items'] = ! empty( $per_page ) && count( $activity_ids ) > $per_page;
 
@@ -702 +708 @@
                          * @param string $sort      Sort direction for query.
                          */
                         $total_activities_sql = apply_filters( 'bp_activity_total_activities_sql', "SELECT count(DISTINCT a.id) FROM {$bp->activity->table_name} a {$join_sql} {$where_sql}", $where_sql, $sort );
-                        $total_activities     = $wpdb->get_var( $total_activities_sql );
+                        $cached = bp_core_get_incremented_cache( $total_activities_sql, 'bp_activity' );
+                        if ( false === $cached ) {
+                                $total_activities = $wpdb->get_var( $total_activities_sql );
+                                bp_core_set_incremented_cache( $total_activities_sql, 'bp_activity', $total_activities );
+                        } else {
+                                $total_activities = $cached;
+                        }
 
                         if ( !empty( $r['max'] ) ) {
                                 if ( (int) $total_activities > (int) $r['max'] ) {

src/bp-core/bp-core-cache.php

@@ -262 +262 @@
 
         return $cache;
 }
+
+/**
+ * Gets a cached list of IDs matching a query key.
+ *
+ * A utility function for use by query methods like BP_Activity_Activity::get().
+ *
+ * @since 2.7.0
+ *
+ * @param string $key   Unique key for the query. Usually a SQL string.
+ * @param string $group Cache group. Eg 'bp_activity'.
+ * @return array|bool False if no cached values are found, otherwise an array of IDs.
+ */
+function bp_core_get_incremented_cache( $key, $group ) {
+        $cache_key = bp_core_get_incremented_cache_key( $key, $group );
+        return wp_cache_get( $cache_key, $group );
+}
+
+/**
+ * Caches a list of IDs matched by an object query.
+ *
+ * A utility function for use by query methods like BP_Activity_Activity::get().
+ *
+ * @since 2.7.0
+ *
+ * @param string $key   Unique key for the query. Usually a SQL string.
+ * @param string $group Cache group. Eg 'bp_activity'.
+ * @param array  $ids   Array of IDs.
+ * @return bool
+ */
+function bp_core_set_incremented_cache( $key, $group, $ids ) {
+        $cache_key = bp_core_get_incremented_cache_key( $key, $group );
+        return wp_cache_set( $cache_key, $ids, $group );
+}
+
+/**
+ * Gets the key to be used when caching an ID query using WP's wp_cache_*() functions.
+ *
+ * The $key is hashed with a component-specific incrementor, which is used to
+ * invalidate multiple caches at once.
+ *
+ * @since 2.7.0
+ *
+ * @param string $key   Unique key for the query. Usually a SQL string.
+ * @param string $group Cache group. Eg 'bp_activity'.
+ * @return string
+ */
+function bp_core_get_incremented_cache_key( $key, $group ) {
+        $incrementor = bp_core_get_incrementor( $group );
+        $cache_key = md5( $key . $incrementor );
+        return $cache_key;
+}
+
+/**
+ * Gets a group-specific cache incrementor.
+ *
+ * The incrementor is paired with query identifiers (like SQL strings) to
+ * create cache keys that can be invalidated en masse.
+ *
+ * If an incrementor does not yet exist for the given `$group`, one will
+ * be created.
+ *
+ * @since 2.7.0
+ *
+ * @param string $group Cache group. Eg 'bp_activity'.
+ * @return string
+ */
+function bp_core_get_incrementor( $group ) {
+        $incrementor = wp_cache_get( 'last_changed', $group );
+        if ( ! $incrementor ) {
+                $incrementor = microtime();
+                wp_cache_set( 'last_changed', $incrementor, $group );
+        }
+
+        return $incrementor;
+}

tests/phpunit/testcases/activity/cache.php

@@ -83 +83 @@
 
                 $this->assertSame( 'bar foo', $a_fp['activities'][0]->content );
         }
+
+        /**
+         * @ticket BP7237
+         * @ticket BP6643
+         */
+        public function test_query_should_be_cached() {
+                global $wpdb;
+
+                $u = $this->factory->user->create();
+                $a = $this->factory->activity->create( array(
+                        'component'     => buddypress()->activity->id,
+                        'type'          => 'activity_update',
+                        'user_id'       => $u,
+                        'content'       => 'foo bar',
+                ) );
+
+                $activity_args = array(
+                        'per_page' => 10,
+                        'fields' => 'ids',
+                        'count_total' => true,
+                );
+
+                $a1 = bp_activity_get( $activity_args );
+
+                $num_queries = $wpdb->num_queries;
+
+                $a2 = bp_activity_get( $activity_args );
+
+                $this->assertEqualSets( $a1, $a2 );
+                $this->assertSame( $num_queries, $wpdb->num_queries );
+        }
+
+        /**
+         * @ticket BP7237
+         * @ticket BP6643
+         */
+        public function test_query_cache_should_be_skipped_for_different_query_params() {
+                global $wpdb;
+
+                $u = $this->factory->user->create();
+                $a = $this->factory->activity->create( array(
+                        'component'     => buddypress()->activity->id,
+                        'type'          => 'activity_update',
+                        'user_id'       => $u,
+                        'content'       => 'foo bar',
+                ) );
+
+                $activity_args = array(
+                        'per_page' => 10,
+                        'fields' => 'ids',
+                        'count_total' => true,
+                        'filter' => array(
+                                'component' => buddypress()->activity->id,
+                        ),
+                );
+
+                $a1 = bp_activity_get( $activity_args );
+
+                $num_queries = $wpdb->num_queries;
+
+                // This is enough to make the ID and COUNT clause miss the cache.
+                $activity_args['filter']['action'] = 'activity_update';
+                $a2 = bp_activity_get( $activity_args );
+
+                $this->assertEqualSets( $a1, $a2 );
+
+                // Two extra queries: one for the IDs, one for the count.
+                $n = $num_queries + 2;
+                $this->assertSame( $num_queries + 2, $wpdb->num_queries );
+        }
+
+        /**
+         * @ticket BP7237
+         * @ticket BP6643
+         */
+        public function test_query_cache_should_be_invalidated_by_activity_add() {
+                global $wpdb;
+
+                $u = $this->factory->user->create();
+                $a1 = $this->factory->activity->create( array(
+                        'component'     => buddypress()->activity->id,
+                        'type'          => 'activity_update',
+                        'user_id'       => $u,
+                        'content'       => 'foo bar',
+                ) );
+
+                $activity_args = array(
+                        'per_page' => 10,
+                        'fields' => 'ids',
+                        'count_total' => true,
+                );
+
+                $q1 = bp_activity_get( $activity_args );
+
+                // Bust the cache.
+                $a2 = $this->factory->activity->create( array(
+                        'component'     => buddypress()->activity->id,
+                        'type'          => 'activity_update',
+                        'user_id'       => $u,
+                        'content'       => 'foo bar',
+                ) );
+
+                $num_queries = $wpdb->num_queries;
+
+                $q2 = bp_activity_get( $activity_args );
+
+                $expected = array( $a1, $a2 );
+
+                $this->assertEqualSets( $expected, $q2['activities'] );
+                $this->assertEquals( 2, $q2['total'] );
+                $this->assertSame( $num_queries + 2, $wpdb->num_queries );
+        }
+
+        /**
+         * @ticket BP7237
+         * @ticket BP6643
+         */
+        public function test_query_cache_should_be_invalidated_by_activity_edit() {
+                global $wpdb;
+
+                $u = $this->factory->user->create();
+                $a = $this->factory->activity->create( array(
+                        'component'     => buddypress()->activity->id,
+                        'type'          => 'activity_update',
+                        'user_id'       => $u,
+                        'content'       => 'foo bar',
+                ) );
+
+                $activity_args = array(
+                        'per_page' => 10,
+                        'fields' => 'ids',
+                        'count_total' => true,
+                );
+
+                $q1 = bp_activity_get( $activity_args );
+
+                // Bust the cache.
+                $this->factory->activity->create( array(
+                        'id'            => $a,
+                        'component'     => buddypress()->activity->id,
+                        'type'          => 'activity_update',
+                        'user_id'       => $u,
+                        'content'       => 'foo bar baz',
+                ) );
+
+                $num_queries = $wpdb->num_queries;
+
+                $q2 = bp_activity_get( $activity_args );
+
+                $this->assertEqualSets( $q1, $q2 );
+                $this->assertSame( $num_queries + 2, $wpdb->num_queries );
+        }
+
+        /**
+         * @ticket BP7237
+         * @ticket BP6643
+         */
+        public function test_query_cache_should_be_invalidated_by_activity_delete() {
+                global $wpdb;
+
+                $u = $this->factory->user->create();
+                $a = $this->factory->activity->create( array(
+                        'component'     => buddypress()->activity->id,
+                        'type'          => 'activity_update',
+                        'user_id'       => $u,
+                        'content'       => 'foo bar',
+                ) );
+
+                $activity_args = array(
+                        'per_page' => 10,
+                        'fields' => 'ids',
+                        'count_total' => true,
+                );
+
+                $q1 = bp_activity_get( $activity_args );
+
+                // Bust the cache.
+                bp_activity_delete( array(
+                        'id' => $a,
+                ) );
+
+                $num_queries = $wpdb->num_queries;
+
+                $q2 = bp_activity_get( $activity_args );
+
+                $this->assertEqualSets( array(), $q2['activities'] );
+                $this->assertEquals( 0, $q2['total'] );
+                $this->assertSame( $num_queries + 2, $wpdb->num_queries );
+        }
 }