Skip to:
Content

BuddyPress.org

Changeset 13112


Ignore:
Timestamp:
09/19/2021 03:58:09 PM (3 years ago)
Author:
imath
Message:

Improve SQL performance when updating/deleting a list of notifications

Instead of looping into a list of notifications to run an update/delete query for each notification, update/delete a list of notifications using a single query. Use primarly this change for message thread notifications and bulk notification actions.

Mainly introduces 2 new static methods to BP_Notifications_Notification:

  • BP_Notifications_Notification::update_id_list() let you update a list of notifications using their notification IDs or component item IDs.
  • BP_Notifications_Notification::delete_id_list() let you delete a list of notifications using their notification IDs or component item IDs.

Props oztaser

Fixes #8426

Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/bp-messages/bp-messages-notifications.php

    r13102 r13112  
    248248
    249249    // Mark each notification for each PM message as read.
    250     foreach ( $message_ids as $message_id ) {
    251         bp_notifications_mark_notifications_by_item_id( bp_loggedin_user_id(), (int) $message_id, buddypress()->messages->id, 'new_message' );
    252     }
     250    bp_notifications_mark_notifications_by_item_ids( bp_loggedin_user_id(), $message_ids, 'messages', 'new_message', false );
    253251}
    254252add_action( 'thread_loop_start', 'bp_messages_screen_conversation_mark_notifications', 10 );
    … …  
    262260 *
    263261 * @param int $thread_id ID of the thread being marked as read.
    264  */
    265 function bp_messages_mark_notification_on_mark_thread( $thread_id ) {
     262 * @param int $user_id   ID of the user who read the thread.
     263 * @param int $num_rows  The number of affected rows by the "mark read" update query.
     264 */
     265function bp_messages_mark_notification_on_mark_thread( $thread_id, $user_id = 0, $num_rows = 0 ) {
     266    if ( ! $num_rows ) {
     267        return;
     268    }
     269
    266270    $thread_messages = BP_Messages_Thread::get_messages( $thread_id );
    267 
    268     foreach ( $thread_messages as $thread_message ) {
    269         bp_notifications_mark_notifications_by_item_id( bp_loggedin_user_id(), $thread_message->id, buddypress()->messages->id, 'new_message' );
    270     }
    271 }
    272 add_action( 'messages_thread_mark_as_read', 'bp_messages_mark_notification_on_mark_thread' );
     271    $message_ids     = wp_list_pluck( $thread_messages, 'id' );
     272
     273    bp_notifications_mark_notifications_by_item_ids( $user_id, $message_ids, 'messages', 'new_message', false );
     274}
     275add_action( 'messages_thread_mark_as_read', 'bp_messages_mark_notification_on_mark_thread', 10, 3 );
    273276
    274277/**
    … …  
    278281 *
    279282 * @param int   $thread_id   ID of the thread.
    280  * @param array $message_ids IDs of the messages.
     283 * @param int[] $message_ids The list of message IDs to delete.
    281284 */
    282285function bp_messages_message_delete_notifications( $thread_id, $message_ids ) {
    … …  
    284287    $thread = new BP_Messages_Thread( $thread_id );
    285288    foreach ( $thread->get_recipients() as $recipient ) {
    286         foreach ( $message_ids as $message_id ) {
    287             if ( ! empty( $recipient->user_id ) ) {
    288                 bp_notifications_delete_notifications_by_item_id(
    289                     $recipient->user_id,
    290                     (int) $message_id,
    291                     buddypress()->messages->id,
    292                     'new_message'
    293                 );
    294             }
    295         }
     289        if ( ! isset( $recipient->user_id ) || ! $recipient->user_id ) {
     290            continue;
     291        }
     292
     293        bp_notifications_delete_notifications_by_item_ids( $recipient->user_id, $message_ids, buddypress()->messages->id, 'new_message' );
    296294    }
    297295}

  • trunk/src/bp-messages/classes/class-bp-messages-thread.php

    r13108 r13112  
    767767        }
    768768
    769         $bp     = buddypress();
    770         $retval = $wpdb->query( $wpdb->prepare( "UPDATE {$bp->messages->table_name_recipients} SET unread_count = 0 WHERE user_id = %d AND thread_id = %d", $user_id, $thread_id ) );
     769        $bp       = buddypress();
     770        $num_rows = $wpdb->query( $wpdb->prepare( "UPDATE {$bp->messages->table_name_recipients} SET unread_count = 0 WHERE user_id = %d AND thread_id = %d", $user_id, $thread_id ) );
    771771
    772772        wp_cache_delete( 'thread_recipients_' . $thread_id, 'bp_messages' );
    … …  
    778778         * @since 2.8.0
    779779         * @since 9.0.0 Added the `user_id` parameter.
     780         * @since 10.0.0 Added the `$num_rows` parameter.
    780781         *
    781782         * @param int $thread_id The message thread ID.
    782783         * @param int $user_id   The user the thread will be marked as read.
     784         * @param bool|int $num_rows    Number of threads marked as unread or false on error.
    783785         */
    784         do_action( 'messages_thread_mark_as_read', $thread_id, $user_id );
    785 
    786         return $retval;
     786        do_action( 'messages_thread_mark_as_read', $thread_id, $user_id, $num_rows );
     787
     788        return $num_rows;
    787789    }
    788790
    … …  
    811813        }
    812814
    813         $bp     = buddypress();
    814         $retval = $wpdb->query( $wpdb->prepare( "UPDATE {$bp->messages->table_name_recipients} SET unread_count = 1 WHERE user_id = %d AND thread_id = %d", $user_id, $thread_id ) );
     815        $bp       = buddypress();
     816        $num_rows = $wpdb->query( $wpdb->prepare( "UPDATE {$bp->messages->table_name_recipients} SET unread_count = 1 WHERE user_id = %d AND thread_id = %d", $user_id, $thread_id ) );
    815817
    816818        wp_cache_delete( 'thread_recipients_' . $thread_id, 'bp_messages' );
    … …  
    822824         * @since 2.8.0
    823825         * @since 9.0.0  Added the `$user_id` parameter.
    824          * @since 10.0.0 Added the `$retval` parameter.
     826         * @since 10.0.0 Added the `$num_rows` parameter.
    825827         *
    826828         * @param int      $thread_id The message thread ID.
    827829         * @param int      $user_id   The user the thread will be marked as unread.
    828          * @param bool|int $retval     =Number of threads marked as unread or false on error.
     830         * @param bool|int $num_rows  Number of threads marked as unread or false on error.
    829831         */
    830         do_action( 'messages_thread_mark_as_unread', $thread_id, $user_id, $retval );
    831 
    832         return $retval;
     832        do_action( 'messages_thread_mark_as_unread', $thread_id, $user_id, $num_rows );
     833
     834        return $num_rows;
    833835    }
    834836

  • trunk/src/bp-notifications/actions/bulk-manage.php

    r13091 r13112  
    1313 * @since 2.2.0
    1414 *
    15  * @return bool
     15 * @return false|void
    1616 */
    1717function bp_notifications_action_bulk_manage() {
    … …  
    4343    switch ( $action ) {
    4444        case 'delete':
    45             foreach ( $notifications as $notification ) {
    46                 bp_notifications_delete_notification( $notification );
    47             }
     45            bp_notifications_delete_notifications_by_ids( $notifications );
    4846            bp_core_add_message( __( 'Notifications deleted.', 'buddypress' ) );
    4947            break;
    5048
    5149        case 'read':
    52             foreach ( $notifications as $notification ) {
    53                 bp_notifications_mark_notification( $notification, false );
    54             }
     50            bp_notifications_mark_notifications_by_ids( $notifications, false );
    5551            bp_core_add_message( __( 'Notifications marked as read', 'buddypress' ) );
    5652            break;
    5753
    5854        case 'unread':
    59             foreach ( $notifications as $notification ) {
    60                 bp_notifications_mark_notification( $notification, true );
    61             }
     55            bp_notifications_mark_notifications_by_ids( $notifications, true );
    6256            bp_core_add_message( __( 'Notifications marked as unread.', 'buddypress' ) );
    6357            break;

  • trunk/src/bp-notifications/bp-notifications-functions.php

    r13108 r13112  
    386386
    387387/**
     388 * Delete notifications by notification ids.
     389 *
     390 * @since 10.0.0
     391 *
     392 * @param  int[]     $ids IDs of the associated notifications.
     393 * @return int|false      The number of rows updated. False on error.
     394 */
     395function bp_notifications_delete_notifications_by_ids( $ids ) {
     396    return BP_Notifications_Notification::delete_by_id_list( 'id', $ids );
     397}
     398
     399/**
     400 * Delete notifications by item ids and user.
     401 *
     402 * @since 10.0.0
     403 *
     404 * @param  int       $user_id          ID of the user whose notifications are being deleted.
     405 * @param  int[]     $item_ids         IDs of the associated items.
     406 * @param  string    $component_name   Name of the associated component.
     407 * @param  string    $component_action Name of the associated action.
     408 * @return int|false                   The number of rows updated. False on error.
     409 */
     410function bp_notifications_delete_notifications_by_item_ids( $user_id, $item_ids, $component_name, $component_action ) {
     411    return BP_Notifications_Notification::delete_by_id_list(
     412        'item_id',
     413        $item_ids,
     414        array(
     415            'user_id'          => $user_id,
     416            'component_name'   => $component_name,
     417            'component_action' => $component_action
     418        )
     419    );
     420}
     421
     422/**
    388423 * Delete all notifications by type.
    389424 *
    … …  
    581616            'component_name'   => $component_name,
    582617            'component_action' => $component_action,
     618        )
     619    );
     620}
     621
     622/**
     623 * Mark notifications read/unread by item ids and user.
     624 *
     625 * @since 10.0.0
     626 *
     627 * @param  int       $user_id          ID of the user whose notifications are being deleted.
     628 * @param  int[]     $item_ids         IDs of the associated items.
     629 * @param  string    $component_name   Name of the associated component.
     630 * @param  string    $component_action Name of the associated action.
     631 * @param  int|false $is_new           0 for read, 1 for unread.
     632 * @return int|false                   The number of rows updated. False on error.
     633 */
     634function bp_notifications_mark_notifications_by_item_ids( $user_id, $item_ids, $component_name, $component_action, $is_new = false ) {
     635    return BP_Notifications_Notification::update_id_list(
     636        'item_id',
     637        $item_ids,
     638        array(
     639            'is_new' => $is_new,
     640        ),
     641        array(
     642            'user_id'          => $user_id,
     643            'component_name'   => $component_name,
     644            'component_action' => $component_action
     645        )
     646    );
     647}
     648
     649/**
     650 * Mark notifications read/unread by notification ids.
     651 *
     652 * @since 10.0.0
     653 *
     654 * @param  int[]     $ids     IDs of the associated notification items.
     655 * @param  int|false $is_new  0 for read, 1 for unread.
     656 * @return int|false          The number of rows updated. False on error.
     657 */
     658function bp_notifications_mark_notifications_by_ids( $ids, $is_new = false ) {
     659    return BP_Notifications_Notification::update_id_list(
     660        'id',
     661        $ids,
     662        array(
     663            'is_new' => $is_new,
    583664        )
    584665    );

  • trunk/src/bp-notifications/classes/class-bp-notifications-notification.php

    r13108 r13112  
    912912
    913913    /**
     914     * Update notifications using a list of ids/items_ids.
     915     *
     916     * @since 10.0.0
     917     *
     918     * @param string $field The name of the db field of the items to update.
     919     *                      Possible values are `id` or `item_id`.
     920     * @param int[]  $items The list of items to update.
     921     * @param array  $data  Array of notification data to update.
     922     * @param array  $where The WHERE params to use to specify the item IDs to update.
     923     * @return int|false    The number of updated rows. False on error.
     924     */
     925    public static function update_id_list( $field, $items = array(), $data = array(), $where = array() ) {
     926        global $wpdb;
     927        $bp = buddypress();
     928
     929        $supported_fields = array( 'id', 'item_id' );
     930
     931        if ( false === in_array( $field, $supported_fields, true ) ) {
     932            return false;
     933        }
     934
     935        if ( ! is_array( $items ) || ! is_array( $data ) || ! is_array( $where ) ) {
     936            return false;
     937        }
     938
     939        $update_args = self::get_query_clauses( $data );
     940        $where_args  = self::get_query_clauses( $where );
     941
     942        $fields     = array();
     943        $conditions = array();
     944        $values     = array();
     945
     946        $_items       = implode( ',', wp_parse_id_list( $items ) );
     947        $conditions[] = "{$field} IN ({$_items})";
     948
     949        foreach ( $update_args['data'] as $field => $value ) {
     950            $index  = array_search( $field, array_keys( $update_args['data'] ) );
     951            $format = $update_args['format'][ $index ];
     952
     953            $fields[] = "{$field} = {$format}";
     954            $values[] = $value;
     955        }
     956
     957        foreach ( $where_args['data'] as $field => $value ) {
     958            $index  = array_search( $field, array_keys( $where_args['data'] ) );
     959            $format = $where_args['format'][ $index ];
     960
     961            $conditions[] = "{$field} = {$format}";
     962            $values[]     = $value;
     963        }
     964
     965        $fields     = implode( ', ', $fields );
     966        $conditions = implode( ' AND ', $conditions );
     967
     968        /** This action is documented in bp-notifications/classes/class-bp-notifications-notification.php */
     969        do_action( 'bp_notification_before_update', $update_args, $where_args );
     970
     971        return $wpdb->query( $wpdb->prepare( "UPDATE {$bp->notifications->table_name} SET {$fields} WHERE {$conditions}", $values ) );
     972    }
     973
     974    /**
    914975     * Delete notifications.
    915976     *
    … …  
    917978     *
    918979     * @see BP_Notifications_Notification::get() for a description of
    919      *      accepted update/where arguments.
     980     *      accepted where arguments.
    920981     *
    921982     * @param array $args Associative array of columns/values, to determine
    … …  
    9391000
    9401001        return self::_delete( $where['data'], $where['format'] );
     1002    }
     1003
     1004    /**
     1005     * Delete notifications using a list of ids/items_ids.
     1006     *
     1007     * @since 10.0.0
     1008     *
     1009     * @param string $field The name of the db field of the items to delete.
     1010     *                      Possible values are `id` or `item_id`.
     1011     * @param int[]  $items The list of items to delete.
     1012     * @param array  $args  The WHERE arguments to use to specify the item IDs to delete.
     1013     * @return int|false    The number of deleted rows. False on error.
     1014     */
     1015    public static function delete_by_id_list( $field, $items = array(), $args = array() ) {
     1016        global $wpdb;
     1017        $bp = buddypress();
     1018
     1019        $supported_fields = array( 'id', 'item_id' );
     1020
     1021        if ( false === in_array( $field, $supported_fields, true ) ) {
     1022            return false;
     1023        }
     1024
     1025        if ( ! is_array( $items ) || ! is_array( $args ) ) {
     1026            return false;
     1027        }
     1028
     1029        $where = self::get_query_clauses( $args );
     1030
     1031        $conditions = array();
     1032        $values     = array();
     1033
     1034        $_items       = implode( ',', wp_parse_id_list( $items ) );
     1035        $conditions[] = "{$field} IN ({$_items})";
     1036
     1037        foreach ( $where['data'] as $field => $value ) {
     1038            $index  = array_search( $field, array_keys( $where['data'] ) );
     1039            $format = $where['format'][ $index ];
     1040
     1041            $conditions[] = "{$field} = {$format}";
     1042            $values[]     = $value;
     1043        }
     1044
     1045        $conditions = implode( ' AND ', $conditions );
     1046
     1047        /** This action is documented in bp-notifications/classes/class-bp-notifications-notification.php */
     1048        do_action( 'bp_notification_before_delete', $args );
     1049
     1050        if ( ! $values ) {
     1051            return $wpdb->query( "DELETE FROM {$bp->notifications->table_name} WHERE {$conditions}" );
     1052        }
     1053
     1054        return $wpdb->query( $wpdb->prepare( "DELETE FROM {$bp->notifications->table_name} WHERE {$conditions}", $values ) );
    9411055    }
    9421056

  • trunk/tests/phpunit/testcases/messages/notifications.php

    r13035 r13112  
    1212        parent::setUp();
    1313
     14        $this->reset_user_id = get_current_user_id();
     15
    1416        $this->filter_fired = '';
     17    }
     18
     19    public function tearDown() {
     20        parent::tearDown();
     21
     22        $this->set_current_user( $this->reset_user_id );
    1523    }
    1624
    … …  
    4149    /**
    4250     * @group messages_format_notifications
    43      * @group imath
    4451     */
    4552    public function test_friends_format_notifications_bp_messages_single_new_message_notification_nonstring_filter() {
    … …  
    146153    }
    147154
     155    /**
     156     * @ticket BP8426
     157     */
     158    public function test_bp_messages_mark_notification_on_mark_thread() {
     159        $u1 = self::factory()->user->create();
     160        $u2 = self::factory()->user->create();
     161        $m1 = self::factory()->message->create_and_get( array(
     162            'sender_id'  => $u1,
     163            'recipients' => array( $u2 ),
     164            'subject'    => 'Foo',
     165        ) );
     166
     167        self::factory()->message->create_many(
     168            9,
     169            array(
     170                'thread_id' => $m1->thread_id,
     171                'sender_id' => $u2,
     172                'recipients' => array( $u1 ),
     173                'subject' => 'Bar',
     174            )
     175        );
     176
     177        $unreadn = wp_list_pluck(
     178            BP_Notifications_Notification::get(
     179                array(
     180                    'user_id'           => $u1,
     181                    'component_name'    => buddypress()->messages->id,
     182                    'component_action'  => 'new_message',
     183                    'is_new'            => 1,
     184                )
     185            ),
     186            'user_id',
     187            'id'
     188        );
     189
     190        $this->set_current_user( $u1 );
     191
     192        // Mark a thread read.
     193        bp_messages_mark_notification_on_mark_thread( $m1->thread_id, $u1, count( $unreadn ) );
     194
     195        $readn = wp_list_pluck(
     196            BP_Notifications_Notification::get(
     197                array(
     198                    'user_id'           => $u1,
     199                    'component_name'    => buddypress()->messages->id,
     200                    'component_action'  => 'new_message',
     201                    'is_new'            => 0,
     202                )
     203            ),
     204            'user_id',
     205            'id'
     206        );
     207
     208        $this->assertSame( $unreadn, $readn );
     209    }
     210
     211    /**
     212     * @ticket BP8426
     213     * @group message_delete_notifications
     214     */
     215    public function test_bp_messages_message_delete_notifications() {
     216        $u1 = self::factory()->user->create();
     217        $u2 = self::factory()->user->create();
     218        $m1 = self::factory()->message->create_and_get( array(
     219            'sender_id'  => $u1,
     220            'recipients' => array( $u2 ),
     221            'subject'    => 'Foo',
     222        ) );
     223
     224        $message_ids = self::factory()->message->create_many(
     225            9,
     226            array(
     227                'thread_id' => $m1->thread_id,
     228                'sender_id' => $u2,
     229                'recipients' => array( $u1 ),
     230                'subject' => 'Bar',
     231            )
     232        );
     233
     234        $message_ids = wp_list_pluck(
     235            BP_Notifications_Notification::get(
     236                array(
     237                    'user_id'           => $u1,
     238                    'component_name'    => buddypress()->messages->id,
     239                    'component_action'  => 'new_message',
     240                    'is_new'            => 1,
     241                )
     242            ),
     243            'item_id'
     244        );
     245
     246        $test = bp_messages_message_delete_notifications( $m1->thread_id, $message_ids );
     247
     248        $deleted = BP_Notifications_Notification::get(
     249            array(
     250                'user_id'           => $u1,
     251                'component_name'    => buddypress()->messages->id,
     252                'component_action'  => 'new_message',
     253                'is_new'            => 1,
     254            )
     255        );
     256
     257        $this->assertEmpty( $deleted );
     258    }
     259
    148260    public function notification_filter_callback( $value ) {
    149261        $this->filter_fired = current_filter();

  • trunk/tests/phpunit/testcases/notifications/functions.php

    r12605 r13112  
    542542        $this->assertEqualSets( [ $n1 ], wp_list_pluck( $found, 'id' ) );
    543543    }
     544
     545    /**
     546     * @ticket BP8426
     547     */
     548    public function test_bp_notifications_mark_notifications_by_ids() {
     549        $u = self::factory()->user->create();
     550
     551        $n = self::factory()->notification->create(
     552            array(
     553                'component_name'    => 'barfoo',
     554                'component_action'  => 'new_bar',
     555                'item_id'           => 98,
     556                'user_id'           => $u,
     557            )
     558        );
     559
     560        for ( $i = 101; $i < 111; ++$i ) {
     561            self::factory()->notification->create(
     562                array(
     563                    'component_name'    => 'foobar',
     564                    'component_action'  => 'new_foo',
     565                    'item_id'           => $i,
     566                    'user_id'           => $u,
     567                )
     568            );
     569        }
     570
     571        $unread = wp_list_pluck(
     572            BP_Notifications_Notification::get(
     573                array(
     574                    'user_id'           => $u,
     575                    'component_name'    => 'foobar',
     576                    'component_action'  => 'new_foo',
     577                    'is_new'            => 1,
     578                )
     579            ),
     580            'id'
     581        );
     582
     583        bp_notifications_mark_notifications_by_ids( $unread );
     584
     585        $read = wp_list_pluck(
     586            BP_Notifications_Notification::get(
     587                array(
     588                    'user_id'           => $u,
     589                    'component_name'    => 'foobar',
     590                    'component_action'  => 'new_foo',
     591                    'is_new'            => 0,
     592                )
     593            ),
     594            'id'
     595        );
     596
     597        $n_get = BP_Notifications_Notification::get(
     598            array(
     599                'id' => $n,
     600                'component_name'    => 'barfoo',
     601                'component_action'  => 'new_bar',
     602            )
     603        );
     604
     605        $n_obj = reset( $n_get );
     606
     607        $this->assertEquals( $unread, $read );
     608        $this->assertEquals( $n, $n_obj->id );
     609        $this->assertTrue( 1 === (int) $n_obj->is_new );
     610    }
     611
     612    /**
     613     * @ticket BP8426
     614     * @group delete_notifications_by_ids
     615     */
     616    public function test_bp_notifications_delete_notifications_by_ids() {
     617        $u = self::factory()->user->create();
     618
     619        $n = self::factory()->notification->create(
     620            array(
     621                'component_name'    => 'barfoo',
     622                'component_action'  => 'new_bar',
     623                'item_id'           => 98,
     624                'user_id'           => $u,
     625            )
     626        );
     627
     628        for ( $i = 101; $i < 111; ++$i ) {
     629            self::factory()->notification->create(
     630                array(
     631                    'component_name'    => 'foobar',
     632                    'component_action'  => 'new_foo',
     633                    'item_id'           => $i,
     634                    'user_id'           => $u,
     635                )
     636            );
     637        }
     638
     639        $unread = wp_list_pluck(
     640            BP_Notifications_Notification::get(
     641                array(
     642                    'user_id'           => $u,
     643                    'component_name'    => 'foobar',
     644                    'component_action'  => 'new_foo',
     645                    'is_new'            => 1,
     646                )
     647            ),
     648            'id'
     649        );
     650
     651        bp_notifications_delete_notifications_by_ids( $unread );
     652
     653        $deleted = wp_list_pluck(
     654            BP_Notifications_Notification::get(
     655                array(
     656                    'user_id'           => $u,
     657                    'component_name'    => 'foobar',
     658                    'component_action'  => 'new_foo',
     659                    'is_new'            => 1,
     660                )
     661            ),
     662            'id'
     663        );
     664
     665        $n_get = BP_Notifications_Notification::get(
     666            array(
     667                'id' => $n,
     668                'component_name'    => 'barfoo',
     669                'component_action'  => 'new_bar',
     670            )
     671        );
     672
     673        $n_obj = reset( $n_get );
     674
     675        $this->assertEmpty( $deleted );
     676        $this->assertEquals( $n, $n_obj->id );
     677        $this->assertTrue( 1 === (int) $n_obj->is_new );
     678    }
    544679}
Note: See TracChangeset for help on using the changeset viewer.