Skip to content

Commit

Permalink
Refactor validation and auto generate parsing tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@smmercuri
smmercuri committed Feb 7, 2025
1 parent 5a9cfc9 commit 23e4d33
Show file tree
Hide file tree
Showing 6 changed files with 3,776 additions and 128 deletions.
2 changes: 1 addition & 1 deletion stack/cas/parsingrules/908_parsons_decode_state_for_display.filter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public function filter(MP_Node $ast, array &$errors, array &$answernotes, stack_
// We validate the node to check that it is a string that represents a Parson's state.
// This is not strictly required as it is prevented by `$node instanceof MP_String`, but it is an additional safety
// measure to ensure we do not dehash other strings.
if ($node instanceof MP_String && stack_parsons_input::validate_parsons_string($node->value)) {
if ($node instanceof MP_String && stack_utils::validate_parsons_string($node->value)) {
$node->value = stack_utils::unhash_parsons_string($node->value);
}

Expand Down
2 changes: 1 addition & 1 deletion stack/cas/parsingrules/909_parsons_get_final_submission.filter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public function filter(MP_Node $ast, array &$errors, array &$answernotes, stack_
// We validate the node to check that it is a string that represents a Parson's state.
// This is not strictly required as it is prevented by `$node instanceof MP_String`, but it is an additional safety
// measure to ensure we do not dehash other strings.
if ($node instanceof MP_String && stack_parsons_input::validate_parsons_string($node->value)) {
if ($node instanceof MP_String && stack_utils::validate_parsons_string($node->value)) {
$decoded = json_decode($node->value);
$node->value = json_encode(reset($decoded));
}
Expand Down
122 changes: 2 additions & 120 deletions stack/input/parsons/parsons.class.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
defined('MOODLE_INTERNAL') || die();

require_once(__DIR__ . '/../string/string.class.php');
require_once(__DIR__ . '/../../utils.class.php');

// phpcs:ignore moodle.Commenting.MissingDocblock.Class
class stack_parsons_input extends stack_string_input {
Expand Down Expand Up @@ -108,7 +109,7 @@ protected function extra_validation($contents) {
$validation = '';
}

if (!self::validate_parsons_string($validation)) {
if (!stack_utils::validate_parsons_string($validation)) {
return stack_string('parsons_got_unrecognised_value');
}
return '';
Expand Down Expand Up @@ -279,123 +280,4 @@ public function get_api_solution_render($tadisplay, $ta) {
public function get_api_solution($value) {
return null;
}

/**
* Takes a PHP array and validates it's structure to check whether it represents a single Parson's state.
* In particular the PHP should be of the following format:
* array(2) {
* [0]=>
* array(2) {
* ["used"]=>
* array(1) {
* [0]=>
* array(1) {
* [0]=>
* array(_) {
* [0]=>
* string(_) <str>
* ...
* [n]=>
* string(_) <str>
* }
* }
* }
* ["available"]=>
* array(_) {
* [0]=>
* string(_) <str>
* ...
* [m]=>
* string(_) <str>
* }
* }
* [1]=>
* int(_)
* }
*
* @param array $input
* @return bool whether $input represents a single Parson's state or not
*/
public static function validate_parsons_state($state) {
// Check if $state is an array.
if (!is_array($state)) {
return false;
}

// Check if it's an array with exactly two elements.
if (count($state) !== 2) {
return false;
}

// Check if the first element is an associative array with keys "used" and "available".
$dict = $state[0];
if (!isset($dict['used']) || !isset($dict['available']) || !is_array($dict['used'])) {
return false;
}

// Validate that "used" is an array of at least two dimensions.
if (!is_array($dict['used'][0]) || !is_array($dict['used'][0][0])) {
return false;
}

// Check if "available" is an array of at least one dimension.
if (!is_array($dict['available'])) {
return false;
}

// Validate that the second element is an integer.
if (!is_int($state[1])) {
return false;
}

// If all checks pass, the string is valid.
return true;
}

/**
* Takes a string and checks whether it is a string containing a list of Parson's states.
* In particular, it checks whether each item in the list is of the following format:
* "[{"used": [[[<str>, ..., <str>]]], "available": [<str>, ..., <str>]}, <int>]"
*
* @param string $input
* @return bool whether $input represents a list of Parson's state or not
*/
public static function validate_parsons_string($input) {
$data = json_decode($input, true);
// When used in the input class $input is a string of a string, so we need to decode twice
// But in later usage (e.g., for filters) $input is just a string
if (is_string($data)) {
$data = json_decode($data, true);
}
//print_r($data);
// Check if the JSON decoding was successful and the resulting structure is an array.
if (json_last_error() !== JSON_ERROR_NONE || !is_array($data)) {
return false;
}

// Check whether each item is a valid PHP array corresponding to a single Parson's state.
foreach ($data as $state) {
if (!self::validate_parsons_state($state)) {
// If one of them fails, then the string is invalid.
return false;
}
}

// If all items pass, then the string is valid.
return true;
}

public static function validate_parsons_contents($contents) {
$strings = function($node) use (&$answernotes, &$errors) {
// We validate the node to check that it is a string that represents a Parson's state.
// This is not strictly required as it is prevented by `$node instanceof MP_String`, but it is an additional safety
// measure to ensure we do not dehash other strings.
if ($node instanceof MP_String && self::validate_parsons_string($node->value)) {
$node->value = stack_utils::unhash_parsons_string($node->value);
}

return true;
};
return $strings($contents);
}
}
119 changes: 119 additions & 0 deletions stack/utils.class.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1119,4 +1119,123 @@ public static function hash_parsons_string_maxima($listofjsons) {
$phplistofjsons = self::maxima_string_to_php_string($listofjsons);
return self::php_string_to_maxima_string(self::hash_parsons_string($phplistofjsons));
}

/**
* Takes a PHP array and validates it's structure to check whether it represents a single Parson's state.
* In particular the PHP should be of the following format:
* array(2) {
* [0]=>
* array(2) {
* ["used"]=>
* array(1) {
* [0]=>
* array(1) {
* [0]=>
* array(_) {
* [0]=>
* string(_) <str>
* ...
* [n]=>
* string(_) <str>
* }
* }
* }
* ["available"]=>
* array(_) {
* [0]=>
* string(_) <str>
* ...
* [m]=>
* string(_) <str>
* }
* }
* [1]=>
* int(_)
* }
*
* @param array $input
* @return bool whether $input represents a single Parson's state or not
*/
public static function validate_parsons_state($state) {
// Check if $state is an array.
if (!is_array($state)) {
return false;
}

// Check if it's an array with exactly two elements.
if (count($state) !== 2) {
return false;
}

// Check if the first element is an associative array with keys "used" and "available".
$dict = $state[0];
if (!isset($dict['used']) || !isset($dict['available']) || !is_array($dict['used'])) {
return false;
}

// Validate that "used" is an array of at least two dimensions.
if (!is_array($dict['used'][0]) || !is_array($dict['used'][0][0])) {
return false;
}

// Check if "available" is an array of at least one dimension.
if (!is_array($dict['available'])) {
return false;
}

// Validate that the second element is an integer.
if (!is_int($state[1])) {
return false;
}

// If all checks pass, the string is valid.
return true;
}

/**
* Takes a string and checks whether it is a string containing a list of Parson's states.
* In particular, it checks whether each item in the list is of the following format:
* "[{"used": [[[<str>, ..., <str>]]], "available": [<str>, ..., <str>]}, <int>]"
*
* @param string $input
* @return bool whether $input represents a list of Parson's state or not
*/
public static function validate_parsons_string($input) {
$data = json_decode($input, true);
// When used in the input class $input is a string of a string, so we need to decode twice
// But in later usage (e.g., for filters) $input is just a string
if (is_string($data)) {
$data = json_decode($data, true);
}
//print_r($data);
// Check if the JSON decoding was successful and the resulting structure is an array.
if (json_last_error() !== JSON_ERROR_NONE || !is_array($data)) {
return false;
}

// Check whether each item is a valid PHP array corresponding to a single Parson's state.
foreach ($data as $state) {
if (!self::validate_parsons_state($state)) {
// If one of them fails, then the string is invalid.
return false;
}
}

// If all items pass, then the string is valid.
return true;
}

public static function validate_parsons_contents($contents) {
$strings = function($node) use (&$answernotes, &$errors) {
// We validate the node to check that it is a string that represents a Parson's state.
// This is not strictly required as it is prevented by `$node instanceof MP_String`, but it is an additional safety
// measure to ensure we do not dehash other strings.
if ($node instanceof MP_String && self::validate_parsons_string($node->value)) {
$node->value = stack_utils::unhash_parsons_string($node->value);
}

return true;
};
return $strings($contents);
}
}
12 changes: 6 additions & 6 deletions ...state_for_display_auto_generated_test.php → ...state_for_display_auto_generated_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,25 +37,25 @@
*
* @group qtype_stack
* @group qtype_stack_ast_filters
* @covers \ast_filter_909_parsons_decode_state_for_display
* @covers \ast_filter_908_parsons_decode_state_for_display
*/
final class ast_filter_909_parsons_decode_state_for_display_auto_generated_test extends qtype_stack_ast_testcase {
final class ast_filter_908_parsons_decode_state_for_display_auto_generated_test extends qtype_stack_ast_testcase {

public function test_affected_no_units(): void {
$this->security = new stack_cas_security(false);
$this->filter = stack_parsing_rule_factory::get_by_common_name('909_parsons_decode_state_for_display');
$this->filter = stack_parsing_rule_factory::get_by_common_name('908_parsons_decode_state_for_display');

}

public function test_affected_units(): void {
$this->security = new stack_cas_security(true);
$this->filter = stack_parsing_rule_factory::get_by_common_name('909_parsons_decode_state_for_display');
$this->filter = stack_parsing_rule_factory::get_by_common_name('908_parsons_decode_state_for_display');

}

public function test_non_affected_units(): void {
$this->security = new stack_cas_security(true);
$this->filter = stack_parsing_rule_factory::get_by_common_name('909_parsons_decode_state_for_display');
$this->filter = stack_parsing_rule_factory::get_by_common_name('908_parsons_decode_state_for_display');

$this->expect('"+"(a,b)',
'"+"(a,b)',
Expand Down Expand Up @@ -1851,7 +1851,7 @@ public function test_non_affected_units(): void {

public function test_non_affected_no_units(): void {
$this->security = new stack_cas_security(false);
$this->filter = stack_parsing_rule_factory::get_by_common_name('909_parsons_decode_state_for_display');
$this->filter = stack_parsing_rule_factory::get_by_common_name('908_parsons_decode_state_for_display');

$this->expect('"+"(a,b)',
'"+"(a,b)',
Expand Down
Loading

0 comments on commit 23e4d33

Please sign in to comment.