Skip to content

Commit

Permalink
MDL-78192 external: allow nullable non-scalar params and results
Browse files Browse the repository at this point in the history
  • Loading branch information
@skodak
skodak committed Jun 23, 2023
1 parent 3cd8474 commit 16729b7
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 12 deletions.
6 changes: 6 additions & 0 deletions lib/external/classes/external_api.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
Expand Down Expand Up @@ -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.');
Expand Down
7 changes: 6 additions & 1 deletion lib/external/classes/external_description.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -52,5 +56,6 @@ public function __construct($desc, $required, $default) {
$this->desc = $desc;
$this->required = $required;
$this->default = $default;
$this->allownull = (bool)$allownull;
}
}
2 changes: 1 addition & 1 deletion lib/external/classes/external_function_parameters.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,6 @@ public function __construct(
}
}
}
parent::__construct($keys, $desc, $required, $default);
parent::__construct($keys, $desc, $required, $default, NULL_NOT_ALLOWED);
}
}
6 changes: 4 additions & 2 deletions lib/external/classes/external_multiple_structure.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
6 changes: 4 additions & 2 deletions lib/external/classes/external_single_structure.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
6 changes: 1 addition & 5 deletions lib/external/classes/external_value.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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;
}
}
96 changes: 95 additions & 1 deletion lib/external/tests/external_api_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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);
}

/**
Expand Down

0 comments on commit 16729b7

Please sign in to comment.