L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-85975 backup: Handle nulls and arrays in restored questiondata

Browse Source 
unset_excluded_fields() was using isset() to determine if the field
targeted for removal is present in the provided data structure. However,
isset() returns false if the field exists, but contains null. This means
the field will not be unset when it should be.

This resolves this by changing the isset() to a property_exists() check
for objects, and array_key_exists() check for arrays. It also expands
the function to properly handle arrays of arrays, or arrays of values,
which was not fully covered before and is used by some third-party
question types.
This commit is contained in:
Mark Johnson
2025-10-20 08:45:46 +07:00
committed by Huong Nguyen
parent edc5590782
commit d74ce2b179
3 changed files with 212 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.upgradenotes/MDL-85975-2025092314370040.yml
+17
View File
@@ -0,0 +1,17 @@
issueNumber: MDL-85975
notes:
core:
- message: >
`restore_qtype_plugin::unset_excluded_fields` now returns the modified
questiondata structure,
in order to support structures that contain arrays.
If your qtype plugin overrides
`restore_qtype_plugin::remove_excluded_question_data` without
calling the parent method, you may need to modify your overridden method
to use the returned
value.
type: fixed
public/backup/moodle2/restore_qtype_plugin.class.php
+48 -14
View File
@@ -568,8 +568,7 @@ abstract class restore_qtype_plugin extends restore_plugin {
foreach ($excludefields as $excludefield) {
$pathparts = explode('/', ltrim($excludefield, '/'));
$data = $questiondata;
self::unset_excluded_fields($data, $pathparts);
$questiondata = self::unset_excluded_fields($questiondata, $pathparts);
}
return $questiondata;
@@ -581,25 +580,60 @@ abstract class restore_qtype_plugin extends restore_plugin {
* If any of the elements in the path is an array, this is called recursively on each element in the array to unset fields
* in each child of the array.
*
* @param stdClass|array $data The questiondata object, or a subsection of it.
* @param stdClass|array $data The questiondata structure, or a subsection of it.
* @param array $pathparts The remaining elements in the path to the excluded field.
* @return void
* @return stdClass|array The $data structure with excluded fields removed.
*/
private static function unset_excluded_fields(stdClass|array $data, array $pathparts): void {
private static function unset_excluded_fields(stdClass|array $data, array $pathparts): stdClass|array {
$element = array_shift($pathparts);
if (!isset($data->{$element})) {
// This element is not present in the data structure, nothing to unset.
return;
$unset = false;
// Get the current element from the data structure.
if (is_object($data)) {
if (!property_exists($data, $element)) {
// This element is not present in the data structure, nothing to unset.
return $data;
}
$dataelement = $data->{$element};
} else { // It's an array.
if (!array_key_exists($element, $data)) {
return $data;
}
$dataelement = $data[$element];
}
if (is_object($data->{$element})) {
self::unset_excluded_fields($data->{$element}, $pathparts);
} else if (is_array($data->{$element})) {
foreach ($data->{$element} as $item) {
self::unset_excluded_fields($item, $pathparts);
// Check if we need to recur, or unset this element.
if (is_object($dataelement)) {
$dataelement = self::unset_excluded_fields($dataelement, $pathparts);
} else if (is_array($dataelement)) {
foreach ($dataelement as $key => $item) {
if (is_object($item) || is_array($item)) {
// This is an array of objects or arrays, recur.
$dataelement[$key] = self::unset_excluded_fields($item, $pathparts);
} else {
// This is an associative array of values, check if they should be removed.
$subelement = reset($pathparts);
if ($key == $subelement) {
unset($dataelement[$key]);
}
}
}
} else if (empty($pathparts)) {
// This is the last element of the path and it's a scalar value, unset it.
unset($data->{$element});
$unset = true;
}
// Write the modified element back to the data structure, or unset it.
if (is_object($data)) {
if ($unset) {
unset($data->{$element});
} else {
$data->{$element} = $dataelement;
}
} else {
if ($unset) {
unset($data[$element]);
} else {
$data[$element] = $dataelement;
}
}
return $data;
}
}
public/backup/moodle2/tests/restore_qtype_plugin_test.php
+147
View File
@@ -0,0 +1,147 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
namespace core;
/**
* Tests for question type restore methods
*
* @package core
* @copyright 2025 onwards Catalyst IT EU {@link https://catalyst-eu.net}
* @author Mark Johnson <[email protected]>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
* @covers \restore_qtype_plugin
*/
final class restore_qtype_plugin_test extends \basic_testcase {
/**
* All default and specified fields should be removed from the provided data structure.
*/
public function test_remove_excluded_question_data(): void {
global $CFG;
require_once($CFG->dirroot . '/backup/moodle2/restore_plugin.class.php');
require_once($CFG->dirroot . '/backup/moodle2/restore_qtype_plugin.class.php');
$data = (object) [
// Default excluded fields should be removed.
'id' => 1,
'createdby' => 2,
'modifiedby' => 3,
// This field is not specified for removal, it should remain.
'questiontext' => 'Some question text',
// Excluded paths that address an array should operate on all items in the array.
'hints' => [
(object) [
'id' => 4,
'questionid' => 1,
// This field is not specified for removal.
'text' => 'Lorem ipsum',
],
(object) [
'id' => 5,
'questionid' => 1,
'text' => 'Lorem ipsum',
],
],
'options' => [ // This is an array of arrays, rather than an array of objects. It should be handled the same.
[
'id' => 6,
'questionid' => 1,
// This field is not specified for removal.
'option' => true,
],
[
'id' => 7,
'questionid' => 1,
'option' => false,
],
[
'id' => 8,
'questionid' => 1,
'option' => false,
],
],
'custom1' => 'Some custom text',
// This field is not specified for removal.
'custom2' => 'Some custom text2',
// Fields specified for removal should be removed even if they contain null values.
'custom3' => null,
'customarray' => [
(object) [
// Null values should also be removed.
'id' => null,
// This field is not specified for removal.
'text' => 'Custom item text',
],
(object) [
'id' => null,
'text' => 'Custom item text2',
],
],
'customstructure' => [ // This array contains scalar values, not a list of objects/arrays.
'id' => null,
'text' => 'Custom structure text',
'number' => 1,
'bool' => true,
],
];
$expecteddata = (object) [
'questiontext' => 'Some question text',
'hints' => [
(object) [
'text' => 'Lorem ipsum',
],
(object) [
'text' => 'Lorem ipsum',
],
],
'options' => [
[
'option' => true,
],
[
'option' => false,
],
[
'option' => false,
],
],
'custom2' => 'Some custom text2',
'customarray' => [
(object) [
'text' => 'Custom item text',
],
(object) [
'text' => 'Custom item text2',
],
],
'customstructure' => [
'text' => 'Custom structure text',
'number' => 1,
],
];
$excludedfields = [
'/custom1',
'/custom3',
'/customarray/id',
'/customstructure/id',
'/customstructure/bool',
// A field that is not in the data structure will be ignored.
'/custom4',
];
$this->assertEquals($expecteddata, \restore_qtype_plugin::remove_excluded_question_data($data, $excludedfields));
}
}