diff --git a/lib/external/classes/external_api.php b/lib/external/classes/external_api.php index 5bd51bd93c7aa..c770a80ab7158 100644 --- a/lib/external/classes/external_api.php +++ b/lib/external/classes/external_api.php @@ -312,6 +312,9 @@ public static function set_timeout($seconds = 360) { * @since Moodle 2.0 */ public static function validate_parameters(external_description $description, $params) { + if ($params === null && $description->allownull == NULL_ALLOWED) { + return null; + } if ($description instanceof external_value) { if (is_array($params) || is_object($params)) { throw new invalid_parameter_exception('Scalar type expected, array or object received.'); @@ -398,6 +401,9 @@ public static function validate_parameters(external_description $description, $p * @since Moodle 2.0 */ public static function clean_returnvalue(external_description $description, $response) { + if ($response === null && $description->allownull == NULL_ALLOWED) { + return null; + } if ($description instanceof external_value) { if (is_array($response) || is_object($response)) { throw new invalid_response_exception('Scalar type expected, array or object received.'); diff --git a/lib/external/classes/external_description.php b/lib/external/classes/external_description.php index ef44a87acf796..6deea883fb549 100644 --- a/lib/external/classes/external_description.php +++ b/lib/external/classes/external_description.php @@ -33,14 +33,18 @@ abstract class external_description { /** @var mixed Default value */ public $default; + /** @var bool Allow null values */ + public $allownull; + /** * Contructor. * * @param string $desc Description of element * @param int $required Whether the element value is required. Valid values are VALUE_DEFAULT, VALUE_REQUIRED, VALUE_OPTIONAL. * @param mixed $default The default value + * @param bool $allownull Allow null value */ - public function __construct($desc, $required, $default) { + public function __construct($desc, $required, $default, $allownull = NULL_NOTALLOWED) { if (!in_array($required, [VALUE_DEFAULT, VALUE_REQUIRED, VALUE_OPTIONAL], true)) { $requiredstr = $required; if (is_array($required)) { @@ -52,5 +56,6 @@ public function __construct($desc, $required, $default) { $this->desc = $desc; $this->required = $required; $this->default = $default; + $this->allownull = (bool)$allownull; } } diff --git a/lib/external/classes/external_function_parameters.php b/lib/external/classes/external_function_parameters.php index 73ec95d969f3a..085e91d3a8d04 100644 --- a/lib/external/classes/external_function_parameters.php +++ b/lib/external/classes/external_function_parameters.php @@ -50,6 +50,6 @@ public function __construct( } } } - parent::__construct($keys, $desc, $required, $default); + parent::__construct($keys, $desc, $required, $default, NULL_NOT_ALLOWED); } } diff --git a/lib/external/classes/external_multiple_structure.php b/lib/external/classes/external_multiple_structure.php index 3a16c8a32b090..8362b818e0a95 100644 --- a/lib/external/classes/external_multiple_structure.php +++ b/lib/external/classes/external_multiple_structure.php @@ -35,14 +35,16 @@ class external_multiple_structure extends external_description { * @param string $desc * @param int $required * @param array $default + * @param bool $allownull */ public function __construct( external_description $content, $desc = '', $required = VALUE_REQUIRED, - $default = null + $default = null, + $allownull = NULL_NOT_ALLOWED ) { - parent::__construct($desc, $required, $default); + parent::__construct($desc, $required, $default, $allownull); $this->content = $content; } } diff --git a/lib/external/classes/external_single_structure.php b/lib/external/classes/external_single_structure.php index 52231aaff4d6a..7aa7bad0a89de 100644 --- a/lib/external/classes/external_single_structure.php +++ b/lib/external/classes/external_single_structure.php @@ -35,14 +35,16 @@ class external_single_structure extends external_description { * @param string $desc * @param int $required * @param array $default + * @param bool $allownull */ public function __construct( array $keys, $desc = '', $required = VALUE_REQUIRED, - $default = null + $default = null, + $allownull = NULL_NOT_ALLOWED ) { - parent::__construct($desc, $required, $default); + parent::__construct($desc, $required, $default, $allownull); $this->keys = $keys; } } diff --git a/lib/external/classes/external_value.php b/lib/external/classes/external_value.php index 45aef862b035b..037441e29dd98 100644 --- a/lib/external/classes/external_value.php +++ b/lib/external/classes/external_value.php @@ -28,9 +28,6 @@ class external_value extends external_description { /** @var mixed Value type PARAM_XX */ public $type; - /** @var bool Allow null values */ - public $allownull; - /** * Constructor for the external_value class. * @@ -47,8 +44,7 @@ public function __construct( $default = null, $allownull = NULL_ALLOWED ) { - parent::__construct($desc, $required, $default); + parent::__construct($desc, $required, $default, $allownull); $this->type = $type; - $this->allownull = $allownull; } } diff --git a/lib/external/tests/external_api_test.php b/lib/external/tests/external_api_test.php index 915831892ef9c..cc359cae2294a 100644 --- a/lib/external/tests/external_api_test.php +++ b/lib/external/tests/external_api_test.php @@ -82,6 +82,52 @@ public function test_validate_params(): void { $this->assertSame('someid', key($result)); $this->assertSame(6, $result['someid']); $this->assertSame('aaa', $result['text']); + + // Missing required value (an exception is thrown). + $testdata = []; + try { + external_api::clean_returnvalue($description, $testdata); + $this->fail('Exception expected'); + } catch (\moodle_exception $ex) { + $this->assertInstanceOf(\invalid_response_exception::class, $ex); + $this->assertSame('Invalid response value detected (Error in response - ' + . 'Missing following required key in a single structure: text)', $ex->getMessage()); + } + + // Test nullable external_value may optionally return data. + $description = new external_function_parameters([ + 'value' => new external_value(PARAM_INT, '', VALUE_REQUIRED, null, NULL_ALLOWED) + ]); + $testdata = ['value' => null]; + $cleanedvalue = external_api::clean_returnvalue($description, $testdata); + $this->assertSame($testdata, $cleanedvalue); + $testdata = ['value' => 1]; + $cleanedvalue = external_api::clean_returnvalue($description, $testdata); + $this->assertSame($testdata, $cleanedvalue); + + // Test nullable external_single_structure may optionally return data. + $description = new external_function_parameters([ + 'value' => new external_single_structure(['value2' => new external_value(PARAM_INT)], + '', VALUE_REQUIRED, null, NULL_ALLOWED) + ]); + $testdata = ['value' => null]; + $cleanedvalue = external_api::clean_returnvalue($description, $testdata); + $this->assertSame($testdata, $cleanedvalue); + $testdata = ['value' => ['value2' => 1]]; + $cleanedvalue = external_api::clean_returnvalue($description, $testdata); + $this->assertSame($testdata, $cleanedvalue); + + // Test nullable external_multiple_structure may optionally return data. + $description = new external_function_parameters([ + 'value' => new external_multiple_structure( + new external_value(PARAM_INT), '', VALUE_REQUIRED, null, NULL_ALLOWED) + ]); + $testdata = ['value' => null]; + $cleanedvalue = external_api::clean_returnvalue($description, $testdata); + $this->assertSame($testdata, $cleanedvalue); + $testdata = ['value' => [1]]; + $cleanedvalue = external_api::clean_returnvalue($description, $testdata); + $this->assertSame($testdata, $cleanedvalue); } /** @@ -162,8 +208,56 @@ public function test_clean_returnvalue(): void { $singlestructure['object'] = $object; $singlestructure['value2'] = 'Some text'; $testdata = [$singlestructure]; - $this->expectException('invalid_response_exception'); + try { + external_api::clean_returnvalue($returndesc, $testdata); + $this->fail('Exception expected'); + } catch (\moodle_exception $ex) { + $this->assertInstanceOf(\invalid_response_exception::class, $ex); + $this->assertSame('Invalid response value detected (object => Invalid response value detected ' + . '(Error in response - Missing following required key in a single structure: value1): Error in response - ' + . 'Missing following required key in a single structure: value1)', $ex->getMessage()); + } + + // Fail if no data provided when value required. + $testdata = null; + try { + external_api::clean_returnvalue($returndesc, $testdata); + $this->fail('Exception expected'); + } catch (\moodle_exception $ex) { + $this->assertInstanceOf(\invalid_response_exception::class, $ex); + $this->assertSame('Invalid response value detected (Only arrays accepted. The bad value is: \'\')', + $ex->getMessage()); + } + + // Test nullable external_multiple_structure may optionally return data. + $returndesc = new external_multiple_structure( + new external_value(PARAM_INT), + '', VALUE_REQUIRED, null, NULL_ALLOWED); + $testdata = null; + $cleanedvalue = external_api::clean_returnvalue($returndesc, $testdata); + $this->assertSame($testdata, $cleanedvalue); + $testdata = [1]; + $cleanedvalue = external_api::clean_returnvalue($returndesc, $testdata); + $this->assertSame($testdata, $cleanedvalue); + + // Test nullable external_single_structure may optionally return data. + $returndesc = new external_single_structure(['value' => new external_value(PARAM_INT)], + '', VALUE_REQUIRED, null, NULL_ALLOWED); + $testdata = null; + $cleanedvalue = external_api::clean_returnvalue($returndesc, $testdata); + $this->assertSame($testdata, $cleanedvalue); + $testdata = ['value' => 1]; + $cleanedvalue = external_api::clean_returnvalue($returndesc, $testdata); + $this->assertSame($testdata, $cleanedvalue); + + // Test nullable external_value may optionally return data. + $returndesc = new external_value(PARAM_INT, '', VALUE_REQUIRED, null, NULL_ALLOWED); + $testdata = null; + $cleanedvalue = external_api::clean_returnvalue($returndesc, $testdata); + $this->assertSame($testdata, $cleanedvalue); + $testdata = 1; $cleanedvalue = external_api::clean_returnvalue($returndesc, $testdata); + $this->assertSame($testdata, $cleanedvalue); } /**