ApiBase: Use prefixed parameter name for 'missingparam' error
authorBrad Jorsch <bjorsch@wikimedia.org>
Mon, 23 Jul 2018 13:22:23 +0000 (09:22 -0400)
committerBrad Jorsch <bjorsch@wikimedia.org>
Mon, 23 Jul 2018 15:44:09 +0000 (11:44 -0400)
Similar errors use the prefixed parameter name, this one should too.

Bug: T200155
Change-Id: Ia14d6a9c457af06e72428c1eae14bd3849b4595a

RELEASE-NOTES-1.32
includes/api/ApiBase.php
tests/phpunit/includes/api/ApiBaseTest.php

index 5435213..9589c19 100644 (file)
@@ -95,6 +95,9 @@ production.
 * (T198935) User list preferences such as `email-blacklist` and similar
   extension preferences are no longer represented as arrays when returned by
   action=query&meta=userinfo&uiprop=options.
+* 'missingparam' errors will now use the prefixed parameter name in the code
+  and error text, e.g. "noxxfoo" and "The 'xxfoo' parameter must be set" rather
+  than "nofoo" and "The 'foo' parameter must be set".
 
 === Action API internal changes in 1.32 ===
 * Added 'ApiParseMakeOutputPage' hook.
index be4eeef..e546c4a 100644 (file)
@@ -1289,7 +1289,7 @@ abstract class ApiBase extends ContextSource {
                                        case 'text':
                                        case 'password':
                                                if ( $required && $value === '' ) {
-                                                       $this->dieWithError( [ 'apierror-missingparam', $paramName ] );
+                                                       $this->dieWithError( [ 'apierror-missingparam', $encParamName ] );
                                                }
                                                break;
                                        case 'integer': // Force everything using intval() and optionally validate limits
@@ -1443,7 +1443,7 @@ abstract class ApiBase extends ContextSource {
                                }
                        }
                } elseif ( $required ) {
-                       $this->dieWithError( [ 'apierror-missingparam', $paramName ] );
+                       $this->dieWithError( [ 'apierror-missingparam', $encParamName ] );
                }
 
                return $value;
index 4f65ae9..866848b 100644 (file)
@@ -256,7 +256,6 @@ class ApiBaseTest extends ApiTestCase {
        }
 
        /**
-        * @dataProvider provideGetParameterFromSettings
         * @param string|null $input
         * @param array $paramSettings
         * @param mixed $expected
@@ -264,13 +263,20 @@ class ApiBaseTest extends ApiTestCase {
         *   'parseLimits': true|false
         *   'apihighlimits': true|false
         *   'internalmode': true|false
+        *   'prefix': true|false
         * @param string[] $warnings
         */
-       public function testGetParameterFromSettings(
+       private function doGetParameterFromSettings(
                $input, $paramSettings, $expected, $warnings, $options = []
        ) {
                $mock = new MockApi();
                $wrapper = TestingAccessWrapper::newFromObject( $mock );
+               if ( $options['prefix'] ) {
+                       $wrapper->mModulePrefix = 'my';
+                       $paramName = 'Param';
+               } else {
+                       $paramName = 'myParam';
+               }
 
                $context = new DerivativeContext( $mock );
                $context->setRequest( new FauxRequest(
@@ -298,14 +304,14 @@ class ApiBaseTest extends ApiTestCase {
 
                if ( $expected instanceof Exception ) {
                        try {
-                               $wrapper->getParameterFromSettings( 'myParam', $paramSettings,
+                               $wrapper->getParameterFromSettings( $paramName, $paramSettings,
                                        $parseLimits );
                                $this->fail( 'No exception thrown' );
                        } catch ( Exception $ex ) {
                                $this->assertEquals( $expected, $ex );
                        }
                } else {
-                       $result = $wrapper->getParameterFromSettings( 'myParam',
+                       $result = $wrapper->getParameterFromSettings( $paramName,
                                $paramSettings, $parseLimits );
                        if ( isset( $paramSettings[ApiBase::PARAM_TYPE] ) &&
                                $paramSettings[ApiBase::PARAM_TYPE] === 'timestamp' &&
@@ -339,6 +345,28 @@ class ApiBaseTest extends ApiTestCase {
                }
        }
 
+       /**
+        * @dataProvider provideGetParameterFromSettings
+        * @see self::doGetParameterFromSettings()
+        */
+       public function testGetParameterFromSettings_noprefix(
+               $input, $paramSettings, $expected, $warnings, $options = []
+       ) {
+               $options['prefix'] = false;
+               $this->doGetParameterFromSettings( $input, $paramSettings, $expected, $warnings, $options );
+       }
+
+       /**
+        * @dataProvider provideGetParameterFromSettings
+        * @see self::doGetParameterFromSettings()
+        */
+       public function testGetParameterFromSettings_prefix(
+               $input, $paramSettings, $expected, $warnings, $options = []
+       ) {
+               $options['prefix'] = true;
+               $this->doGetParameterFromSettings( $input, $paramSettings, $expected, $warnings, $options );
+       }
+
        public static function provideGetParameterFromSettings() {
                $warnings = [
                        [ 'apiwarn-badutf8', 'myParam' ],