MDL-76723 core_external: Handle format properties properly

- If a text format property defines the 'optional' attribute as true,
then the $required parameter that
is passed to \core\external\exporter::get_format_structure() should be
VALUE_OPTIONAL.
- If a default value is provided for a format property,
\core\external\exporter::get_format_structure() should pass the default
value to the external_format_value constructor.
- Added validation for external_format_value for the default parameter.
  Debugging will be shown in case an invalid default value is passed.
- Amended unit tests for better coverage.
This commit is contained in:
Jun Pataleta
2023-12-19 15:50:21 +08:00
parent def6645cdc
commit bbfbc8abd1
3 changed files with 105 additions and 17 deletions
+9 -2
View File
@@ -425,10 +425,12 @@ abstract class exporter {
* @return external_format_value
*/
final protected static function get_format_structure($property, $definition, $required = VALUE_REQUIRED) {
$default = null;
if (array_key_exists('default', $definition)) {
$required = VALUE_DEFAULT;
$default = $definition['default'];
}
return new external_format_value($property, $required);
return new external_format_value($property, $required, $default);
}
/**
@@ -547,7 +549,12 @@ abstract class exporter {
if (isset($returns[$formatproperty])) {
throw new coding_exception('The format for \'' . $property . '\' is already defined.');
}
$returns[$formatproperty] = self::get_format_structure($property, $properties[$formatproperty]);
$formatpropertydef = $properties[$formatproperty];
$formatpropertyrequired = VALUE_REQUIRED;
if (!empty($formatpropertydef['optional'])) {
$formatpropertyrequired = VALUE_OPTIONAL;
}
$returns[$formatproperty] = self::get_format_structure($property, $formatpropertydef, $formatpropertyrequired);
}
}
}
+8 -1
View File
@@ -38,12 +38,19 @@ class external_format_value extends external_value {
* @param int $default Default value.
*/
public function __construct($textfieldname, $required = VALUE_REQUIRED, $default = null) {
// Make sure the default format's value is correct.
if ($default !== null && !in_array($default, [FORMAT_MOODLE, FORMAT_HTML, FORMAT_PLAIN, FORMAT_MARKDOWN])) {
debugging("Invalid default format for $textfieldname: $default. " .
"It must be either FORMAT_MOODLE, FORMAT_HTML, FORMAT_PLAIN, or FORMAT_MARKDOWN.", DEBUG_DEVELOPER);
$default = null;
}
if ($default == null && $required == VALUE_DEFAULT) {
$default = FORMAT_HTML;
}
$desc = sprintf(
"%s format (%s = HTML, %s = MOODLE, %s = PLAIN, or %s = MARKDOWN",
"%s format (%s = HTML, %s = MOODLE, %s = PLAIN, or %s = MARKDOWN)",
$textfieldname,
FORMAT_HTML,
FORMAT_MOODLE,
+88 -14
View File
@@ -203,24 +203,98 @@ class exporter_test extends \advanced_testcase {
$this->assertEquals('id', $properties['nestedarray']['type']['id']['description']);
}
public function test_format_properties_with_optional() {
/**
* Test the processing of format properties.
*
* @covers \core\external\exporter::get_read_structure
* @return void
*/
public function test_format_properties_with_optional(): void {
$testable = new class([]) extends \core\external\exporter {
public static function define_properties() {
return [
'content' => [
'type' => PARAM_RAW,
'optional' => true,
],
'contentformat' => [
'type' => PARAM_INT,
'optional' => true,
]
];
}
/**
* Properties definition.
*
* @return array[]
*/
public static function define_properties(): array {
return [
'content' => [
'type' => PARAM_RAW,
],
'contentformat' => [
'type' => PARAM_INT,
'optional' => true,
],
'description' => [
'type' => PARAM_RAW,
'optional' => true,
],
'descriptionformat' => [
'type' => PARAM_INT,
'default' => FORMAT_MARKDOWN,
],
'summary' => [
'type' => PARAM_RAW,
],
'summaryformat' => [
'type' => PARAM_INT,
'default' => null,
],
];
}
};
$definition = $testable::get_read_structure();
$this->assertEquals(VALUE_OPTIONAL, $definition->keys['content']->required);
// Check content and its format.
$this->assertEquals(VALUE_REQUIRED, $definition->keys['content']->required);
$this->assertEquals(VALUE_OPTIONAL, $definition->keys['contentformat']->required);
$this->assertEquals(null, $definition->keys['contentformat']->default);
// Check description and its format.
$this->assertEquals(VALUE_OPTIONAL, $definition->keys['description']->required);
$this->assertEquals(VALUE_DEFAULT, $definition->keys['descriptionformat']->required);
$this->assertEquals(FORMAT_MARKDOWN, $definition->keys['descriptionformat']->default);
// Check summary and its format.
$this->assertEquals(VALUE_REQUIRED, $definition->keys['summary']->required);
$this->assertEquals(null, $definition->keys['summary']->default);
$this->assertEquals(VALUE_DEFAULT, $definition->keys['summaryformat']->required);
$this->assertEquals(FORMAT_HTML, $definition->keys['summaryformat']->default);
}
/**
* Test the processing of format properties when an invalid default format is passed.
*
* @covers \core\external\exporter::get_read_structure
* @return void
*/
public function test_optional_format_property_with_invalid_default(): void {
$testable = new class([]) extends \core\external\exporter {
/**
* Properties definition.
*
* @return array[]
*/
public static function define_properties(): array {
return [
'description' => [
'type' => PARAM_RAW,
],
'descriptionformat' => [
'type' => PARAM_INT,
'default' => 999,
],
];
}
};
$definition = $testable::get_read_structure();
$this->assertDebuggingCalled(null, DEBUG_DEVELOPER);
// Check description and its format.
$this->assertEquals(VALUE_REQUIRED, $definition->keys['description']->required);
$this->assertEquals(VALUE_DEFAULT, $definition->keys['descriptionformat']->required);
$this->assertEquals(FORMAT_HTML, $definition->keys['descriptionformat']->default);
}
}