Changeset 13430

Timestamp:

02/23/2023 12:43:14 AM (2 years ago)

Author:

imath

Message:

Improve comparison consistency in BP_Friends_Friendship

• Fix a bug introduced in [13092] making sure to cast IDs retrieved by $wpdb->results() as integers when comparing them to another integer (user ID).
• Use strong comparisons everywhere in the class.
• Add unit tests.

Props boonebgorges

Fixes #8844 (branch 11.0)

Location:

branches/11.0

Files:

• src/bp-friends/classes/class-bp-friends-friendship.php (modified) (11 diffs)
• tests/phpunit/testcases/friends/class-bp-friends-friendship.php (modified) (1 diff)

branches/11.0/src/bp-friends/classes/class-bp-friends-friendship.php

@@ -268 +268 @@
         if ( empty( $user_id ) ) {
             $user_id = bp_loggedin_user_id();
+        }
+
+        $friendships = array();
+        $operator    = strtoupper( $operator );
+
+        if ( ! in_array( $operator, array( 'AND', 'OR', 'NOT' ), true ) ) {
+            return $friendships;
         }
 
@@ -303 +310 @@
         }
 
+        $int_keys  = array( 'id', 'initiator_user_id', 'friend_user_id' );
+        $bool_keys = array( 'is_confirmed', 'is_limited' );
+
         // Assemble filter array.
         $filters = wp_array_slice_assoc( $r, array( 'id', 'initiator_user_id', 'friend_user_id', 'is_confirmed', 'is_limited' ) );
@@ -308 +318 @@
             if ( is_null( $filter_value ) ) {
                 unset( $filters[ $filter_name ] );
+            } elseif ( in_array( $filter_name, $int_keys, true ) ) {
+                $filters[ $filter_name ] = (int) $filter_value;
+            } else {
+                $filters[ $filter_name ] = (bool) $filter_value;
             }
         }
 
         // Populate friendship array from cache, and normalize.
-        $friendships = array();
-        $int_keys    = array( 'id', 'initiator_user_id', 'friend_user_id' );
-        $bool_keys   = array( 'is_confirmed', 'is_limited' );
         foreach ( $friendship_ids as $friendship_id ) {
             // Create a limited BP_Friends_Friendship object (don't fetch the user details).
@@ -335 +346 @@
 
             // We need to support the same operators as wp_list_filter().
-            if ( 'OR' == $operator || 'NOT' == $operator ) {
+            if ( 'OR' === $operator || 'NOT' === $operator ) {
                 $matched = 0;
 
                 foreach ( $filters as $filter_name => $filter_value ) {
-                    if ( isset( $friendship->{$filter_name} ) && $filter_value == $friendship->{$filter_name} ) {
+                    if ( isset( $friendship->{$filter_name} ) && $filter_value === $friendship->{$filter_name} ) {
                         $matched++;
                     }
                 }
 
-                if ( ( 'OR' == $operator && $matched > 0 )
-                  || ( 'NOT' == $operator && 0 == $matched ) ) {
+                if ( ( 'OR' === $operator && $matched > 0 )
+                  || ( 'NOT' === $operator && 0 === $matched ) ) {
                     $friendships[ $friendship->id ] = $friendship;
                 }
@@ -355 +366 @@
                  */
                 foreach ( $filters as $filter_name => $filter_value ) {
-                    if ( ! isset( $friendship->{$filter_name} ) || $filter_value != $friendship->{$filter_name} ) {
+                    if ( ! isset( $friendship->{$filter_name} ) || $filter_value !== $friendship->{$filter_name} ) {
                         continue 2;
                     }
@@ -432 +443 @@
 
         $friendships = self::get_friendships( $user_id, $args );
+        $user_id     = (int) $user_id;
 
         $fids = array();
         foreach ( $friendships as $friendship ) {
+            $friend_id = $friendship->friend_user_id;
+            if ( $friendship->friend_user_id === $user_id ) {
+                $friend_id = $friendship->initiator_user_id;
+            }
+
             if ( ! empty( $assoc_arr ) ) {
-                $fids[] = array( 'user_id' => ( $friendship->friend_user_id == $user_id ) ? $friendship->initiator_user_id : $friendship->friend_user_id );
+                $fids[] = array( 'user_id' => $friend_id );
             } else {
-                $fids[] = ( $friendship->friend_user_id == $user_id ) ? $friendship->initiator_user_id : $friendship->friend_user_id;
+                $fids[] = $friend_id;
             }
         }
@@ -637 +654 @@
 
         // Can't friend yourself.
-        if ( $initiator_userid === $possible_friend_userid ) {
+        if ( (int) $initiator_userid === (int) $possible_friend_userid ) {
             return 'not_friends';
         }
@@ -663 +680 @@
 
         $bp                  = buddypress();
+        $user_id             = (int) $user_id;
         $possible_friend_ids = wp_parse_id_list( $possible_friend_ids );
 
@@ -940 +958 @@
         $sql     = $wpdb->prepare( "SELECT friend_user_id, initiator_user_id FROM {$bp->friends->table_name} WHERE (friend_user_id = %d || initiator_user_id = %d) && is_confirmed = 1 ORDER BY rand() LIMIT %d", $user_id, $user_id, $total_friends );
         $results = $wpdb->get_results( $sql );
+        $user_id = (int) $user_id;
 
         for ( $i = 0, $count = count( $results ); $i < $count; ++$i ) {
-            $fids[] = ( $results[ $i ]->friend_user_id === $user_id ) ? $results[ $i ]->initiator_user_id : $results[ $i ]->friend_user_id;
+            $friend_user_id    = (int) $results[ $i ]->friend_user_id;
+            $initiator_user_id = (int) $results[ $i ]->initiator_user_id;
+
+            if ( $friend_user_id === $user_id ) {
+                $fids[] = $initiator_user_id;
+            } else {
+                $fids[] = $friend_user_id;
+            }
         }
 
@@ -1059 +1085 @@
         global $wpdb;
 
-        $bp = buddypress();
+        $bp      = buddypress();
+        $user_id = (int) $user_id;
 
         // Get all friendships, of any status, for the user.
@@ -1068 +1095 @@
             $friendship_ids[] = $friendship->id;
             if ( $friendship->is_confirmed ) {
-                $friend_ids[] = ( $friendship->friend_user_id == $user_id ) ? $friendship->initiator_user_id : $friendship->friend_user_id;
+                if ( $friendship->friend_user_id === $user_id ) {
+                    $friend_ids[] = $friendship->initiator_user_id;
+                } else {
+                    $friend_ids[] = $friendship->friend_user_id;
+                }
             }
         }

branches/11.0/tests/phpunit/testcases/friends/class-bp-friends-friendship.php

@@ -192 +192 @@
         $this->assertEquals( $first_query_count, $wpdb->num_queries );
     }
+
+    /**
+     * @ticket BP8844
+     */
+    public function test_get_random_friends() {
+        $user_id = self::factory()->user->create();
+        $friends = self::factory()->user->create_many( 4 );
+
+        foreach ( $friends as $initiator_id ) {
+            friends_add_friend( $initiator_id, $user_id, true );
+        }
+
+        $random = BP_Friends_Friendship::get_random_friends( $user_id, 4 );
+        $this->assertFalse( in_array( $user_id, $random, true ), 'The requested user id should not be listed into random friends' );
+        $this->assertTrue( 4 === count( $random ) );
+    }
+
+    /**
+     * @ticket BP8844
+     */
+    public function test_get_friend_user_ids() {
+        $user_id = self::factory()->user->create();
+        $friends = self::factory()->user->create_many( 4 );
+
+        foreach ( $friends as $initiator_id ) {
+            friends_add_friend( $initiator_id, $user_id, true );
+        }
+
+        $friend_user_ids = BP_Friends_Friendship::get_friend_user_ids( $user_id );
+        $this->assertFalse( in_array( $user_id, $friend_user_ids, true ), 'The requested user id should not be listed into random friends' );
+        $this->assertTrue( 4 === count( $friend_user_ids ) );
+    }
+
+    /**
+     * @ticket BP8844
+     */
+    public function test_delete_all_for_user() {
+        $user_id = self::factory()->user->create();
+        $friends = self::factory()->user->create_many( 4 );
+
+        foreach ( $friends as $initiator_id ) {
+            friends_add_friend( $initiator_id, $user_id, true );
+        }
+
+        BP_Friends_Friendship::delete_all_for_user( $user_id );
+        $friend_user_ids = BP_Friends_Friendship::get_friendship_ids_for_user( $user_id );
+        $this->assertEmpty( $friend_user_ids );
+    }
+
+    /**
+     * @ticket BP8844
+     */
+    public function test_get_friendships() {
+        $u1 = self::factory()->user->create();
+        $u2 = self::factory()->user->create();
+        $u3 = self::factory()->user->create();
+
+        friends_add_friend( $u2, $u1, false );
+        friends_add_friend( $u3, $u1, true );
+
+        $friendships = BP_Friends_Friendship::get_friendships( $u1, array( 'initiator_user_id' => $u2 ), 'not' );
+        $friendship = reset( $friendships );
+        $this->assertTrue( $u3 === $friendship->initiator_user_id && 1 === count( $friendships ) );
+
+        $friendships = BP_Friends_Friendship::get_friendships( $u1, array( 'initiator_user_id' => $u3, 'is_confirmed' => 0 ), 'or' );
+        $this->assertTrue( 2 === count( $friendships ) );
+    }
 }