Merge "rdbms: avoid strange uses of empty()"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Fri, 2 Mar 2018 19:13:58 +0000 (19:13 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Fri, 2 Mar 2018 19:13:58 +0000 (19:13 +0000)
RELEASE-NOTES-1.31
includes/EditPage.php
includes/installer/DatabaseUpdater.php
includes/libs/rdbms/database/Database.php
includes/libs/rdbms/database/DatabaseMysqlBase.php
includes/parser/Parser.php
includes/resourceloader/ResourceLoader.php
includes/user/User.php
tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php
tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php

index 8113314..0c4bc68 100644 (file)
@@ -256,15 +256,25 @@ changes to languages because of Phabricator reports.
   * Title::isCssJsSubpage – use ::isUserConfigPage
   * Title::isCssSubpage – use ::isUserCssConfigPage
   * Title::isJsSubpage – use ::isUserJsConfigPage
-* The following variables and method in EditPage, deprecated in MediaWiki 1.30, were removed:
+* The following variables and methods in EditPage, deprecated in MediaWiki 1.30, were removed:
   * $isCssJsSubpage — use ::isUserConfigPage()
   * $isCssSubpage — use ::isUserCssConfigPage()
   * $isJsSubpage — use ::isUserJsConfigPage()
   * $isWrongCaseCssJsPage – use ::isWrongCaseUserConfigPage()
+  * ::getSummaryInput() – use ::getSummaryInputWidget()
+  * ::getSummaryInputOOUI() – use ::getSummaryInputWidget()
+  * ::getCheckboxes() – use ::getCheckboxesWidget() or ::getCheckboxesDefinition()
+  * ::getCheckboxesOOUI() – use ::getCheckboxesWidget() or ::getCheckboxesDefinition()
 * The method ResourceLoaderModule::getPosition(), deprecated in 1.29, has been removed.
 * The DeferredStringifier class is deprecated, use Message::listParam() instead.
 * The type string for the parameter $lang of DateFormatter::getInstance is
   deprecated.
+* In User, the cookie-related methods which were wrappers for the functions on the response
+  object, and were deprecated in 1.27, have been removed:
+  * ::setCookie()
+  * ::clearCookie()
+  * ::setExtendedLoginCookie()
+  Note that User::setCookies() remains, and is not deprecated.
 * The global functions wfProfileIn and wfProfileOut, deprecated in 1.25, have been removed.
 
 == Compatibility ==
index 08c4a72..ad5f75d 100644 (file)
@@ -3153,62 +3153,6 @@ ERROR;
                ];
        }
 
-       /**
-        * Standard summary input and label (wgSummary), abstracted so EditPage
-        * subclasses may reorganize the form.
-        * Note that you do not need to worry about the label's for=, it will be
-        * inferred by the id given to the input. You can remove them both by
-        * passing [ 'id' => false ] to $userInputAttrs.
-        *
-        * @deprecated since 1.30 Use getSummaryInputWidget() instead
-        * @param string $summary The value of the summary input
-        * @param string $labelText The html to place inside the label
-        * @param array $inputAttrs Array of attrs to use on the input
-        * @param array $spanLabelAttrs Array of attrs to use on the span inside the label
-        * @return array An array in the format [ $label, $input ]
-        */
-       public function getSummaryInput( $summary = "", $labelText = null,
-               $inputAttrs = null, $spanLabelAttrs = null
-       ) {
-               wfDeprecated( __METHOD__, '1.30' );
-               $inputAttrs = $this->getSummaryInputAttributes( $inputAttrs );
-               $inputAttrs += Linker::tooltipAndAccesskeyAttribs( 'summary' );
-
-               $spanLabelAttrs = ( is_array( $spanLabelAttrs ) ? $spanLabelAttrs : [] ) + [
-                       'class' => $this->missingSummary ? 'mw-summarymissed' : 'mw-summary',
-                       'id' => "wpSummaryLabel"
-               ];
-
-               $label = null;
-               if ( $labelText ) {
-                       $label = Xml::tags(
-                               'label',
-                               $inputAttrs['id'] ? [ 'for' => $inputAttrs['id'] ] : null,
-                               $labelText
-                       );
-                       $label = Xml::tags( 'span', $spanLabelAttrs, $label );
-               }
-
-               $input = Html::input( 'wpSummary', $summary, 'text', $inputAttrs );
-
-               return [ $label, $input ];
-       }
-
-       /**
-        * Builds a standard summary input with a label.
-        *
-        * @deprecated since 1.30 Use getSummaryInputWidget() instead
-        * @param string $summary The value of the summary input
-        * @param string $labelText The html to place inside the label
-        * @param array $inputAttrs Array of attrs to use on the input
-        *
-        * @return OOUI\FieldLayout OOUI FieldLayout with Label and Input
-        */
-       function getSummaryInputOOUI( $summary = "", $labelText = null, $inputAttrs = null ) {
-               wfDeprecated( __METHOD__, '1.30' );
-               return $this->getSummaryInputWidget( $summary, $labelText, $inputAttrs );
-       }
-
        /**
         * Builds a standard summary input with a label.
         *
@@ -4225,76 +4169,6 @@ ERROR;
                return $checkboxes;
        }
 
-       /**
-        * Returns an array of html code of the following checkboxes old style:
-        * minor and watch
-        *
-        * @deprecated since 1.30 Use getCheckboxesWidget() or getCheckboxesDefinition() instead
-        * @param int &$tabindex Current tabindex
-        * @param array $checked See getCheckboxesDefinition()
-        * @return array
-        */
-       public function getCheckboxes( &$tabindex, $checked ) {
-               wfDeprecated( __METHOD__, '1.30' );
-               $checkboxes = [];
-               $checkboxesDef = $this->getCheckboxesDefinition( $checked );
-
-               // Backwards-compatibility for the EditPageBeforeEditChecks hook
-               if ( !$this->isNew ) {
-                       $checkboxes['minor'] = '';
-               }
-               $checkboxes['watch'] = '';
-
-               foreach ( $checkboxesDef as $name => $options ) {
-                       $legacyName = isset( $options['legacy-name'] ) ? $options['legacy-name'] : $name;
-                       $label = $this->context->msg( $options['label-message'] )->parse();
-                       $attribs = [
-                               'tabindex' => ++$tabindex,
-                               'id' => $options['id'],
-                       ];
-                       $labelAttribs = [
-                               'for' => $options['id'],
-                       ];
-                       if ( isset( $options['tooltip'] ) ) {
-                               $attribs['accesskey'] = $this->context->msg( "accesskey-{$options['tooltip']}" )->text();
-                               $labelAttribs['title'] = Linker::titleAttrib( $options['tooltip'], 'withaccess' );
-                       }
-                       if ( isset( $options['title-message'] ) ) {
-                               $labelAttribs['title'] = $this->context->msg( $options['title-message'] )->text();
-                       }
-                       if ( isset( $options['label-id'] ) ) {
-                               $labelAttribs['id'] = $options['label-id'];
-                       }
-                       $checkboxHtml =
-                               Xml::check( $name, $options['default'], $attribs ) .
-                               '&#160;' .
-                               Xml::tags( 'label', $labelAttribs, $label );
-
-                       $checkboxes[ $legacyName ] = $checkboxHtml;
-               }
-
-               // Avoid PHP 7.1 warning of passing $this by reference
-               $editPage = $this;
-               Hooks::run( 'EditPageBeforeEditChecks', [ &$editPage, &$checkboxes, &$tabindex ], '1.29' );
-               return $checkboxes;
-       }
-
-       /**
-        * Returns an array of checkboxes for the edit form, including 'minor' and 'watch' checkboxes and
-        * any other added by extensions.
-        *
-        * @deprecated since 1.30 Use getCheckboxesWidget() or getCheckboxesDefinition() instead
-        * @param int &$tabindex Current tabindex
-        * @param array $checked Array of checkbox => bool, where bool indicates the checked
-        *                 status of the checkbox
-        *
-        * @return array Associative array of string keys to OOUI\FieldLayout instances
-        */
-       public function getCheckboxesOOUI( &$tabindex, $checked ) {
-               wfDeprecated( __METHOD__, '1.30' );
-               return $this->getCheckboxesWidget( $tabindex, $checked );
-       }
-
        /**
         * Returns an array of checkboxes for the edit form, including 'minor' and 'watch' checkboxes and
         * any other added by extensions.
index 79ea4f8..0d55454 100644 (file)
@@ -1227,7 +1227,7 @@ abstract class DatabaseUpdater {
                                "maintenance/migrateComments.php.\n"
                        );
                        $task = $this->maintenance->runChild( MigrateComments::class, 'migrateComments.php' );
-                       $task->execute();
+                       $ok = $task->execute();
                        $this->output( $ok ? "done.\n" : "errors were encountered.\n" );
                }
        }
index f1af074..b8c15be 100644 (file)
@@ -1435,14 +1435,27 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                list( $startOpts, $useIndex, $preLimitTail, $postLimitTail, $ignoreIndex ) =
                        $this->makeSelectOptions( $options );
 
-               if ( !empty( $conds ) ) {
-                       if ( is_array( $conds ) ) {
-                               $conds = $this->makeList( $conds, self::LIST_AND );
-                       }
+               if ( is_array( $conds ) ) {
+                       $conds = $this->makeList( $conds, self::LIST_AND );
+               }
+
+               if ( $conds === null || $conds === false ) {
+                       $this->queryLogger->warning(
+                               __METHOD__
+                               . ' called from '
+                               . $fname
+                               . ' with incorrect parameters: $conds must be a string or an array'
+                       );
+                       $conds = '';
+               }
+
+               if ( $conds === '' ) {
+                       $sql = "SELECT $startOpts $vars $from $useIndex $ignoreIndex $preLimitTail";
+               } elseif ( is_string( $conds ) ) {
                        $sql = "SELECT $startOpts $vars $from $useIndex $ignoreIndex " .
                                "WHERE $conds $preLimitTail";
                } else {
-                       $sql = "SELECT $startOpts $vars $from $useIndex $ignoreIndex $preLimitTail";
+                       throw new DBUnexpectedError( $this, __METHOD__ . ' called with incorrect parameters' );
                }
 
                if ( isset( $options['LIMIT'] ) ) {
index cfbe077..a5220b9 100644 (file)
@@ -527,13 +527,16 @@ abstract class DatabaseMysqlBase extends Database {
                // checking if the target table has an auto-increment column that
                // isn't set in $varMap, that seems unlikely to be worth the extra
                // complexity.
-               return ( (int)$row->innodb_autoinc_lock_mode === 0 );
+               return (
+                       in_array( 'NO_AUTO_COLUMNS', $insertOptions ) ||
+                       (int)$row->innodb_autoinc_lock_mode === 0
+               );
        }
 
        /**
         * @return stdClass Process cached row
         */
-       private function getReplicationSafetyInfo() {
+       protected function getReplicationSafetyInfo() {
                if ( $this->replicationInfoRow === null ) {
                        $this->replicationInfoRow = $this->selectRow(
                                false,
index 2adfd0a..f6526ac 100644 (file)
@@ -2215,8 +2215,14 @@ class Parser {
                                $link = $origLink;
                        }
 
-                       $unstrip = $this->mStripState->unstripNoWiki( $link );
-                       $nt = is_string( $unstrip ) ? Title::newFromText( $unstrip ) : null;
+                       // \x7f isn't a default legal title char, so most likely strip
+                       // markers will force us into the "invalid form" path above.  But,
+                       // just in case, let's assert that xmlish tags aren't valid in
+                       // the title position.
+                       $unstrip = $this->mStripState->killMarkers( $link );
+                       $noMarkers = ( $unstrip === $link );
+
+                       $nt = $noMarkers ? Title::newFromText( $link ) : null;
                        if ( $nt === null ) {
                                $s .= $prefix . '[[' . $line;
                                continue;
index 830dbb4..f9b03c7 100644 (file)
@@ -1486,10 +1486,8 @@ MESSAGE;
        }
 
        /**
-        * Returns JS code which runs given JS code if the client-side framework is
-        * present.
+        * Wraps JavaScript code to run after startup and base modules.
         *
-        * @deprecated since 1.25; use makeInlineScript instead
         * @param string $script JavaScript code
         * @return string JavaScript code
         */
@@ -1499,10 +1497,10 @@ MESSAGE;
        }
 
        /**
-        * Construct an inline script tag with given JS code.
+        * Returns an HTML script tag that runs given JS code after startup and base modules.
         *
-        * The code will be wrapped in a closure, and it will be executed by ResourceLoader
-        * only if the client has adequate support for MediaWiki JavaScript code.
+        * The code will be wrapped in a closure, and it will be executed by ResourceLoader's
+        * startup module if the client has adequate support for MediaWiki JavaScript code.
         *
         * @param string $script JavaScript code
         * @return WrappedString HTML
index 3102cfc..ab791b4 100644 (file)
@@ -4052,76 +4052,6 @@ class User implements IDBAccessObject, UserIdentity {
                }
        }
 
-       /**
-        * Set a cookie on the user's client. Wrapper for
-        * WebResponse::setCookie
-        * @deprecated since 1.27
-        * @param string $name Name of the cookie to set
-        * @param string $value Value to set
-        * @param int $exp Expiration time, as a UNIX time value;
-        *                   if 0 or not specified, use the default $wgCookieExpiration
-        * @param bool $secure
-        *  true: Force setting the secure attribute when setting the cookie
-        *  false: Force NOT setting the secure attribute when setting the cookie
-        *  null (default): Use the default ($wgCookieSecure) to set the secure attribute
-        * @param array $params Array of options sent passed to WebResponse::setcookie()
-        * @param WebRequest|null $request WebRequest object to use; $wgRequest will be used if null
-        *        is passed.
-        */
-       protected function setCookie(
-               $name, $value, $exp = 0, $secure = null, $params = [], $request = null
-       ) {
-               wfDeprecated( __METHOD__, '1.27' );
-               if ( $request === null ) {
-                       $request = $this->getRequest();
-               }
-               $params['secure'] = $secure;
-               $request->response()->setCookie( $name, $value, $exp, $params );
-       }
-
-       /**
-        * Clear a cookie on the user's client
-        * @deprecated since 1.27
-        * @param string $name Name of the cookie to clear
-        * @param bool $secure
-        *  true: Force setting the secure attribute when setting the cookie
-        *  false: Force NOT setting the secure attribute when setting the cookie
-        *  null (default): Use the default ($wgCookieSecure) to set the secure attribute
-        * @param array $params Array of options sent passed to WebResponse::setcookie()
-        */
-       protected function clearCookie( $name, $secure = null, $params = [] ) {
-               wfDeprecated( __METHOD__, '1.27' );
-               $this->setCookie( $name, '', time() - 86400, $secure, $params );
-       }
-
-       /**
-        * Set an extended login cookie on the user's client. The expiry of the cookie
-        * is controlled by the $wgExtendedLoginCookieExpiration configuration
-        * variable.
-        *
-        * @see User::setCookie
-        *
-        * @deprecated since 1.27
-        * @param string $name Name of the cookie to set
-        * @param string $value Value to set
-        * @param bool $secure
-        *  true: Force setting the secure attribute when setting the cookie
-        *  false: Force NOT setting the secure attribute when setting the cookie
-        *  null (default): Use the default ($wgCookieSecure) to set the secure attribute
-        */
-       protected function setExtendedLoginCookie( $name, $value, $secure ) {
-               global $wgExtendedLoginCookieExpiration, $wgCookieExpiration;
-
-               wfDeprecated( __METHOD__, '1.27' );
-
-               $exp = time();
-               $exp += $wgExtendedLoginCookieExpiration !== null
-                       ? $wgExtendedLoginCookieExpiration
-                       : $wgCookieExpiration;
-
-               $this->setCookie( $name, $value, $exp, $secure );
-       }
-
        /**
         * Persist this user's session (e.g. set cookies)
         *
index 5fcca1a..14c7057 100644 (file)
@@ -29,6 +29,7 @@ use Wikimedia\Rdbms\MySQLMasterPos;
 use Wikimedia\Rdbms\DatabaseMysqlBase;
 use Wikimedia\Rdbms\DatabaseMysqli;
 use Wikimedia\Rdbms\Database;
+use Wikimedia\TestingAccessWrapper;
 
 /**
  * Fake class around abstract class so we can call concrete methods.
@@ -510,4 +511,97 @@ class DatabaseMysqlBaseTest extends PHPUnit\Framework\TestCase {
 
                $this->assertEquals( $pos, $roundtripPos );
        }
+
+       /**
+        * @covers Wikimedia\Rdbms\DatabaseMysqlBase::isInsertSelectSafe
+        * @dataProvider provideInsertSelectCases
+        */
+       public function testInsertSelectIsSafe( $insertOpts, $selectOpts, $row, $safe ) {
+               $db = $this->getMockBuilder( DatabaseMysqli::class )
+                       ->disableOriginalConstructor()
+                       ->setMethods( [ 'getReplicationSafetyInfo' ] )
+                       ->getMock();
+               $db->method( 'getReplicationSafetyInfo' )->willReturn( (object)$row );
+               $dbw = TestingAccessWrapper::newFromObject( $db );
+
+               $this->assertEquals( $safe, $dbw->isInsertSelectSafe( $insertOpts, $selectOpts ) );
+       }
+
+       public function provideInsertSelectCases() {
+               return [
+                       [
+                               [],
+                               [],
+                               [
+                                       'innodb_autoinc_lock_mode' => '2',
+                                       'binlog_format' => 'ROW',
+                               ],
+                               true
+                       ],
+                       [
+                               [],
+                               [ 'LIMIT' => 100 ],
+                               [
+                                       'innodb_autoinc_lock_mode' => '2',
+                                       'binlog_format' => 'ROW',
+                               ],
+                               true
+                       ],
+                       [
+                               [],
+                               [ 'LIMIT' => 100 ],
+                               [
+                                       'innodb_autoinc_lock_mode' => '0',
+                                       'binlog_format' => 'STATEMENT',
+                               ],
+                               false
+                       ],
+                       [
+                               [],
+                               [],
+                               [
+                                       'innodb_autoinc_lock_mode' => '2',
+                                       'binlog_format' => 'STATEMENT',
+                               ],
+                               false
+                       ],
+                       [
+                               [ 'NO_AUTO_COLUMNS' ],
+                               [ 'LIMIT' => 100 ],
+                               [
+                                       'innodb_autoinc_lock_mode' => '0',
+                                       'binlog_format' => 'STATEMENT',
+                               ],
+                               false
+                       ],
+                       [
+                               [],
+                               [],
+                               [
+                                       'innodb_autoinc_lock_mode' => 0,
+                                       'binlog_format' => 'STATEMENT',
+                               ],
+                               true
+                       ],
+                       [
+                               [ 'NO_AUTO_COLUMNS' ],
+                               [],
+                               [
+                                       'innodb_autoinc_lock_mode' => 2,
+                                       'binlog_format' => 'STATEMENT',
+                               ],
+                               true
+                       ],
+                       [
+                               [ 'NO_AUTO_COLUMNS' ],
+                               [],
+                               [
+                                       'innodb_autoinc_lock_mode' => 0,
+                                       'binlog_format' => 'STATEMENT',
+                               ],
+                               true
+                       ],
+
+               ];
+       }
 }
index 3d1fe1a..5c1943b 100644 (file)
@@ -64,6 +64,44 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
                                        "FROM table " .
                                        "WHERE alias = 'text'"
                        ],
+                       [
+                               [
+                                       'tables' => 'table',
+                                       'fields' => [ 'field', 'alias' => 'field2' ],
+                                       'conds' => 'alias = \'text\'',
+                               ],
+                               "SELECT field,field2 AS alias " .
+                               "FROM table " .
+                               "WHERE alias = 'text'"
+                       ],
+                       [
+                               [
+                                       'tables' => 'table',
+                                       'fields' => [ 'field', 'alias' => 'field2' ],
+                                       'conds' => [],
+                               ],
+                               "SELECT field,field2 AS alias " .
+                               "FROM table"
+                       ],
+                       [
+                               [
+                                       'tables' => 'table',
+                                       'fields' => [ 'field', 'alias' => 'field2' ],
+                                       'conds' => '',
+                               ],
+                               "SELECT field,field2 AS alias " .
+                               "FROM table"
+                       ],
+                       [
+                               [
+                                       'tables' => 'table',
+                                       'fields' => [ 'field', 'alias' => 'field2' ],
+                                       'conds' => '0', // T188314
+                               ],
+                               "SELECT field,field2 AS alias " .
+                               "FROM table " .
+                               "WHERE 0"
+                       ],
                        [
                                [
                                        // 'tables' with space prepended indicates pre-escaped table name