Move updateNotificationTimestamp callback to DeferredUpdates

* This puts the complex logic here after the commit step for
  all DBs, making the main multi-DB transaction more likely
  to be atomic.
* Also fixed the reuseConnection() call by getting a new
  handle in the callback.

Change-Id: I449a521423ff13bfbf49bdaa6e7e6df2145c8751

diff --git a/includes/WatchedItemStore.php b/includes/WatchedItemStore.php
index 515fbfc..89ca50c 100644 (file)
--- a/includes/WatchedItemStore.php
+++ b/includes/WatchedItemStore.php
@@ -719,28 +719,29 @@ class WatchedItemStore implements StatsdAwareInterface {
         */
        public function updateNotificationTimestamp( User $editor, LinkTarget $target, $timestamp ) {
                $dbw = $this->getConnection( DB_MASTER );
         */
        public function updateNotificationTimestamp( User $editor, LinkTarget $target, $timestamp ) {
                $dbw = $this->getConnection( DB_MASTER );

-               $res = $dbw->select( [ 'watchlist' ],
-                       [ 'wl_user' ],
+               $uids = $dbw->selectFieldValues(
+                       'watchlist',
+                       'wl_user',

                        [
                                'wl_user != ' . intval( $editor->getId() ),
                                'wl_namespace' => $target->getNamespace(),
                                'wl_title' => $target->getDBkey(),
                                'wl_notificationtimestamp IS NULL',
                        [
                                'wl_user != ' . intval( $editor->getId() ),
                                'wl_namespace' => $target->getNamespace(),
                                'wl_title' => $target->getDBkey(),
                                'wl_notificationtimestamp IS NULL',

-                       ], __METHOD__
+                       ],
+                       __METHOD__

                );
                );

+               $this->reuseConnection( $dbw );

 
 

-               $watchers = [];
-               foreach ( $res as $row ) {
-                       $watchers[] = intval( $row->wl_user );
-               }
-
+               $watchers = array_map( 'intval', $uids );

                if ( $watchers ) {
                        // Update wl_notificationtimestamp for all watching users except the editor
                        $fname = __METHOD__;
                if ( $watchers ) {
                        // Update wl_notificationtimestamp for all watching users except the editor
                        $fname = __METHOD__;

-                       $dbw->onTransactionIdle(
-                               function () use ( $dbw, $timestamp, $watchers, $target, $fname ) {
+                       DeferredUpdates::addCallableUpdate(
+                               function () use ( $timestamp, $watchers, $target, $fname ) {

                                        global $wgUpdateRowsPerQuery;
 
                                        global $wgUpdateRowsPerQuery;
 

+                                       $dbw = $this->getConnection( DB_MASTER );
+

                                        $watchersChunks = array_chunk( $watchers, $wgUpdateRowsPerQuery );
                                        foreach ( $watchersChunks as $watchersChunk ) {
                                                $dbw->update( 'watchlist',
                                        $watchersChunks = array_chunk( $watchers, $wgUpdateRowsPerQuery );
                                        foreach ( $watchersChunks as $watchersChunk ) {
                                                $dbw->update( 'watchlist',


@@ -758,12 +759,12 @@ class WatchedItemStore implements StatsdAwareInterface {
                                                }
                                        }
                                        $this->uncacheLinkTarget( $target );
                                                }
                                        }
                                        $this->uncacheLinkTarget( $target );

+
+                                       $this->reuseConnection( $dbw );

                                }
                        );
                }
 
                                }
                        );
                }
 

-               $this->reuseConnection( $dbw );
-

                return $watchers;
        }
 
                return $watchers;
        }
 

diff --git a/tests/phpunit/includes/WatchedItemStoreUnitTest.php b/tests/phpunit/includes/WatchedItemStoreUnitTest.php
index 2d2e726..030d9d5 100644 (file)
--- a/tests/phpunit/includes/WatchedItemStoreUnitTest.php
+++ b/tests/phpunit/includes/WatchedItemStoreUnitTest.php
@@ -2444,10 +2444,10 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase {
        public function testUpdateNotificationTimestamp_watchersExist() {
                $mockDb = $this->getMockDb();
                $mockDb->expects( $this->once() )
        public function testUpdateNotificationTimestamp_watchersExist() {
                $mockDb = $this->getMockDb();
                $mockDb->expects( $this->once() )

-                       ->method( 'select' )
+                       ->method( 'selectFieldValues' )

                        ->with(
                        ->with(

-                               [ 'watchlist' ],
-                               [ 'wl_user' ],
+                               'watchlist',
+                               'wl_user',

                                [
                                        'wl_user != 1',
                                        'wl_namespace' => 0,
                                [
                                        'wl_user != 1',
                                        'wl_namespace' => 0,


@@ -2455,18 +2455,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase {
                                        'wl_notificationtimestamp IS NULL'
                                ]
                        )
                                        'wl_notificationtimestamp IS NULL'
                                ]
                        )

-                       ->will(
-                               $this->returnValue( [
-                                       $this->getFakeRow( [ 'wl_user' => '2' ] ),
-                                       $this->getFakeRow( [ 'wl_user' => '3' ] )
-                               ] )
-                       );
-               $mockDb->expects( $this->once() )
-                       ->method( 'onTransactionIdle' )
-                       ->with( $this->isType( 'callable' ) )
-                       ->will( $this->returnCallback( function( $callable ) {
-                               $callable();
-                       } ) );
+                       ->will( $this->returnValue( [ '2', '3' ] ) );

                $mockDb->expects( $this->once() )
                        ->method( 'update' )
                        ->with(
                $mockDb->expects( $this->once() )
                        ->method( 'update' )
                        ->with(


@@ -2502,10 +2491,10 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase {
        public function testUpdateNotificationTimestamp_noWatchers() {
                $mockDb = $this->getMockDb();
                $mockDb->expects( $this->once() )
        public function testUpdateNotificationTimestamp_noWatchers() {
                $mockDb = $this->getMockDb();
                $mockDb->expects( $this->once() )

-                       ->method( 'select' )
+                       ->method( 'selectFieldValues' )

                        ->with(
                        ->with(

-                               [ 'watchlist' ],
-                               [ 'wl_user' ],
+                               'watchlist',
+                               'wl_user',

                                [
                                        'wl_user != 1',
                                        'wl_namespace' => 0,
                                [
                                        'wl_user != 1',
                                        'wl_namespace' => 0,


@@ -2516,8 +2505,6 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase {
                        ->will(
                                $this->returnValue( [] )
                        );
                        ->will(
                                $this->returnValue( [] )
                        );

-               $mockDb->expects( $this->never() )
-                       ->method( 'onTransactionIdle' );

                $mockDb->expects( $this->never() )
                        ->method( 'update' );
 
                $mockDb->expects( $this->never() )
                        ->method( 'update' );
 


@@ -2551,19 +2538,10 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase {
                                $this->getFakeRow( [ 'wl_notificationtimestamp' => '20151212010101' ] )
                        ) );
                $mockDb->expects( $this->once() )
                                $this->getFakeRow( [ 'wl_notificationtimestamp' => '20151212010101' ] )
                        ) );
                $mockDb->expects( $this->once() )

-                       ->method( 'select' )
+                       ->method( 'selectFieldValues' )

                        ->will(
                        ->will(

-                               $this->returnValue( [
-                                       $this->getFakeRow( [ 'wl_user' => '2' ] ),
-                                       $this->getFakeRow( [ 'wl_user' => '3' ] )
-                               ] )
+                               $this->returnValue( [ '2', '3' ] )

                        );
                        );

-               $mockDb->expects( $this->once() )
-                       ->method( 'onTransactionIdle' )
-                       ->with( $this->isType( 'callable' ) )
-                       ->will( $this->returnCallback( function( $callable ) {
-                               $callable();
-                       } ) );

                $mockDb->expects( $this->once() )
                        ->method( 'update' );
 
                $mockDb->expects( $this->once() )
                        ->method( 'update' );