Merge "Do not run ipblocks cleanup randomly, just do it all the time"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Wed, 28 Feb 2018 03:53:47 +0000 (03:53 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Wed, 28 Feb 2018 03:53:48 +0000 (03:53 +0000)
1  2 
includes/Block.php

diff --combined includes/Block.php
@@@ -206,25 -206,12 +206,25 @@@ class Block 
         * @return array
         */
        public static function selectFields() {
 +              global $wgActorTableSchemaMigrationStage;
 +
 +              if ( $wgActorTableSchemaMigrationStage > MIGRATION_WRITE_BOTH ) {
 +                      // If code is using this instead of self::getQueryInfo(), there's a
 +                      // decent chance it's going to try to directly access
 +                      // $row->ipb_by or $row->ipb_by_text and we can't give it
 +                      // useful values here once those aren't being written anymore.
 +                      throw new BadMethodCallException(
 +                              'Cannot use ' . __METHOD__ . ' when $wgActorTableSchemaMigrationStage > MIGRATION_WRITE_BOTH'
 +                      );
 +              }
 +
                wfDeprecated( __METHOD__, '1.31' );
                return [
                        'ipb_id',
                        'ipb_address',
                        'ipb_by',
                        'ipb_by_text',
 +                      'ipb_by_actor' => $wgActorTableSchemaMigrationStage > MIGRATION_OLD ? 'ipb_by_actor' : null,
                        'ipb_timestamp',
                        'ipb_auto',
                        'ipb_anon_only',
         */
        public static function getQueryInfo() {
                $commentQuery = CommentStore::getStore()->getJoin( 'ipb_reason' );
 +              $actorQuery = ActorMigration::newMigration()->getJoin( 'ipb_by' );
                return [
 -                      'tables' => [ 'ipblocks' ] + $commentQuery['tables'],
 +                      'tables' => [ 'ipblocks' ] + $commentQuery['tables'] + $actorQuery['tables'],
                        'fields' => [
                                'ipb_id',
                                'ipb_address',
 -                              'ipb_by',
 -                              'ipb_by_text',
                                'ipb_timestamp',
                                'ipb_auto',
                                'ipb_anon_only',
                                'ipb_block_email',
                                'ipb_allow_usertalk',
                                'ipb_parent_block_id',
 -                      ] + $commentQuery['fields'],
 -                      'joins' => $commentQuery['joins'],
 +                      ] + $commentQuery['fields'] + $actorQuery['fields'],
 +                      'joins' => $commentQuery['joins'] + $actorQuery['joins'],
                ];
        }
  
         */
        protected function initFromRow( $row ) {
                $this->setTarget( $row->ipb_address );
 -              if ( $row->ipb_by ) { // local user
 -                      $this->setBlocker( User::newFromId( $row->ipb_by ) );
 -              } else { // foreign user
 -                      $this->setBlocker( $row->ipb_by_text );
 -              }
 +              $this->setBlocker( User::newFromAnyId(
 +                      $row->ipb_by, $row->ipb_by_text, isset( $row->ipb_by_actor ) ? $row->ipb_by_actor : null
 +              ) );
  
                $this->mTimestamp = wfTimestamp( TS_MW, $row->ipb_timestamp );
                $this->mAuto = $row->ipb_auto;
                if ( $this->getSystemBlockType() !== null ) {
                        throw new MWException( 'Cannot insert a system block into the database' );
                }
 +              if ( !$this->getBlocker() || $this->getBlocker()->getName() === '' ) {
 +                      throw new MWException( 'Cannot insert a block without a blocker set' );
 +              }
  
                wfDebug( "Block::insert; timestamp {$this->mTimestamp}\n" );
  
                        $dbw = wfGetDB( DB_MASTER );
                }
  
-               # Periodic purge via commit hooks
-               if ( mt_rand( 0, 9 ) == 0 ) {
-                       self::purgeExpired();
-               }
+               self::purgeExpired();
  
                $row = $this->getDatabaseArray( $dbw );
  
                $a = [
                        'ipb_address'          => (string)$this->target,
                        'ipb_user'             => $uid,
 -                      'ipb_by'               => $this->getBy(),
 -                      'ipb_by_text'          => $this->getByName(),
                        'ipb_timestamp'        => $dbw->timestamp( $this->mTimestamp ),
                        'ipb_auto'             => $this->mAuto,
                        'ipb_anon_only'        => !$this->isHardblock(),
                        'ipb_block_email'      => $this->prevents( 'sendemail' ),
                        'ipb_allow_usertalk'   => !$this->prevents( 'editownusertalk' ),
                        'ipb_parent_block_id'  => $this->mParentBlockId
 -              ] + CommentStore::getStore()->insert( $dbw, 'ipb_reason', $this->mReason );
 +              ] + CommentStore::getStore()->insert( $dbw, 'ipb_reason', $this->mReason )
 +                      + ActorMigration::newMigration()->getInsertValues( $dbw, 'ipb_by', $this->getBlocker() );
  
                return $a;
        }
         */
        protected function getAutoblockUpdateArray( IDatabase $dbw ) {
                return [
 -                      'ipb_by'               => $this->getBy(),
 -                      'ipb_by_text'          => $this->getByName(),
                        'ipb_create_account'   => $this->prevents( 'createaccount' ),
                        'ipb_deleted'          => (int)$this->mHideName, // typecast required for SQLite
                        'ipb_allow_usertalk'   => !$this->prevents( 'editownusertalk' ),
 -              ] + CommentStore::getStore()->insert( $dbw, 'ipb_reason', $this->mReason );
 +              ] + CommentStore::getStore()->insert( $dbw, 'ipb_reason', $this->mReason )
 +                      + ActorMigration::newMigration()->getInsertValues( $dbw, 'ipb_by', $this->getBlocker() );
        }
  
        /**
                        return;
                }
  
 +              $target = $block->getTarget();
 +              if ( is_string( $target ) ) {
 +                      $target = User::newFromName( $target, false );
 +              }
 +
                $dbr = wfGetDB( DB_REPLICA );
 +              $rcQuery = ActorMigration::newMigration()->getWhere( $dbr, 'rc_user', $target, false );
  
                $options = [ 'ORDER BY' => 'rc_timestamp DESC' ];
 -              $conds = [ 'rc_user_text' => (string)$block->getTarget() ];
  
                // Just the last IP used.
                $options['LIMIT'] = 1;
  
 -              $res = $dbr->select( 'recentchanges', [ 'rc_ip' ], $conds,
 -                      __METHOD__, $options );
 +              $res = $dbr->select(
 +                      [ 'recentchanges' ] + $rcQuery['tables'],
 +                      [ 'rc_ip' ],
 +                      $rcQuery['conds'],
 +                      __METHOD__,
 +                      $options,
 +                      $rcQuery['joins']
 +              );
  
                if ( !$res->numRows() ) {
                        # No results, don't autoblock anything
                        wfGetDB( DB_MASTER ),
                        __METHOD__,
                        function ( IDatabase $dbw, $fname ) {
-                               $dbw->delete(
-                                       'ipblocks',
+                               $ids = $dbw->selectFieldValues( 'ipblocks',
+                                       'ipb_id',
                                        [ 'ipb_expiry < ' . $dbw->addQuotes( $dbw->timestamp() ) ],
                                        $fname
                                );
+                               if ( $ids ) {
+                                       $dbw->delete( 'ipblocks', [ 'ipb_id' => $ids ], $fname );
+                               }
                        }
                ) );
        }
  
        /**
         * Get the user who implemented this block
 -       * @return User|string Local User object or string for a foreign user
 +       * @return User User object. May name a foreign user.
         */
        public function getBlocker() {
                return $this->blocker;