refactor: move filename inside the file_metadata

This causes fewer problems with PHP auto-casting filenames which just happen to be numerical to ints.

Author: MHajoha

classes/local/api/wysiwyg_editor_data.php

@@ -18,7 +18,6 @@
 
 use qtype_questionpy\local\array_converter\attributes\array_element_class;
 use qtype_questionpy\local\array_converter\attributes\array_key;
-use qtype_questionpy\local\array_converter\attributes\array_to_object;
 use qtype_questionpy\local\files\file_metadata;
 
 /**
@@ -45,10 +44,9 @@ public function __construct(
         /** @var string $markupformat */
         #[array_key('markup_format')]
         public string $markupformat,
-        /** @var array<string, file_metadata> $files */
+        /** @var file_metadata[] $files */
         #[array_key('files')]
         #[array_element_class(file_metadata::class)]
-        #[array_to_object]
         public array $files = [],
         /** @var string|null $html */
         #[array_key('html')]

classes/local/files/file_metadata.php

@@ -31,11 +31,14 @@ class file_metadata {
     /**
      * Trivial constructor.
      *
+     * @param string $filename
      * @param string $fileref
      * @param DateTimeImmutable $uploadedat
      * @param string $mimetype
      */
     public function __construct(
+        /** @var string $filename */
+        public string $filename,
         /** @var string $fileref */
         #[array_key('file_ref')]
         public string $fileref,

classes/local/files/options_file_service.php

@@ -69,7 +69,7 @@ public function save_draft_area_files(int $contextid, int $questionid, int $user
         $draftfiles = $fs->get_area_files(context_user::instance($userid)->id, 'user', 'draft', $draftitemid, includedirs: false);
         foreach ($draftfiles as $draftfile) {
             $fileref = qpy_file_ref::from_stored_file($draftfile);
-            // If the user didn't modify/remove the file, it would already be saved and the file ref not changed.
+            // If the file ref already exists, the user just didn't modify/remove the file.
             if (!in_array(strval($fileref), $existingfilerefs)) {
                 $fs->create_file_from_storedfile([
                     'component' => 'qtype_questionpy',
@@ -84,34 +84,36 @@ public function save_draft_area_files(int $contextid, int $questionid, int $user
     }
 
     /**
-     * Populates the given draft area with files listed in `$metadata` and stored in the permanent question file area.
+     * Populates the given draft area with files listed in `$filemetas` and stored in the permanent question file area.
      *
      * (The inverse of {@see save_draft_area_files}.)
      *
      * @param int $contextid Context id of the question (NOT the draft area).
      * @param int $questionid
-     * @param array $metadata array of {@see file_metadata} by filename
+     * @param file_metadata[] $filemetas
      * @param int $userid
      * @param int $draftitemid
      * @throws file_exception
      * @throws stored_file_creation_exception
      * @throws coding_exception
      */
-    public function prepare_draft_area(int $contextid, int $questionid, array $metadata, int $userid, int $draftitemid): void {
+    public function prepare_draft_area(int $contextid, int $questionid, array $filemetas, int $userid, int $draftitemid): void {
         $fs = get_file_storage();
         $files = $fs->get_area_files($contextid, 'qtype_questionpy', self::FILEAREA_UPLOADS, $questionid, includedirs: false);
 
-        foreach ($metadata as $mfilename => $filemetadata) {
+        foreach ($filemetas as $filemetadata) {
             $matchingfiles = array_filter($files, fn($file) => $file->get_filename() === $filemetadata->fileref);
             if (!$matchingfiles) {
-                debugging("Options file '$mfilename' with file_ref '$filemetadata->fileref' could be found in storage.");
+                debugging("Options file '$filemetadata->filename' with file_ref '$filemetadata->fileref' could not be found in "
+                    . 'storage.');
                 continue;
             }
 
             $count = count($matchingfiles);
             if ($count > 1) {
                 // This would mean that the file area contains two files with the same filename, which I'm not sure is possible.
-                debugging("Options file '$mfilename' has ambiguous file_ref '$filemetadata->fileref' which matches $count files.");
+                debugging("Options file '$filemetadata->filename' has ambiguous file_ref '$filemetadata->fileref' which matches "
+                    . "$count files.");
             }
 
             $file = reset($matchingfiles);
@@ -122,7 +124,7 @@ public function prepare_draft_area(int $contextid, int $questionid, array $metad
                 'itemid' => $draftitemid,
                 'contextid' => context_user::instance($userid)->id,
                 'filepath' => '/',
-                'filename' => $mfilename,
+                'filename' => $filemetadata->filename,
             ], $file);
         }
     }
@@ -132,7 +134,7 @@ public function prepare_draft_area(int $contextid, int $questionid, array $metad
      *
      * @param int $userid
      * @param int $draftitemid
-     * @return array<string, file_metadata>
+     * @return file_metadata[]
      * @throws moodle_exception
      * @throws coding_exception
      */
@@ -143,7 +145,8 @@ public function get_qpy_files_metadata_from_draftitem(int $userid, int $draftite
         $metadata = [];
         foreach ($files as $file) {
             $fileref = qpy_file_ref::from_stored_file($file);
-            $metadata[$file->get_filename()] = new file_metadata(
+            $metadata[] = new file_metadata(
+                filename: $file->get_filename(),
                 fileref: $fileref,
                 uploadedat: DateTimeImmutable::createFromFormat('U', $file->get_timemodified()),
                 mimetype: $file->get_mimetype()

classes/local/form/elements/file_upload_element.php

@@ -92,8 +92,8 @@ public function render_to(render_context $context): void {
             }
 
             global $USER;
-            /** @var array<string, file_metadata> $metadatabyname */
-            $metadatabyname = di::get(options_file_service::class)->get_qpy_files_metadata_from_draftitem($USER->id, $draftitemid);
+            /** @var file_metadata[] $filemetas */
+            $filemetas = di::get(options_file_service::class)->get_qpy_files_metadata_from_draftitem($USER->id, $draftitemid);
 
             // At this time, we don't know whether the question will be saved or the draft validated etc., and we don't know the
             // question id, so we don't save the draft files ourselves. But we do need to let question_service know which draft
@@ -103,7 +103,7 @@ public function render_to(render_context $context): void {
             utils::array_set_nested(
                 $alldata,
                 $element->getName(),
-                $metadatabyname ? array_converter::to_array($metadatabyname) : new stdClass()
+                array_converter::to_array($filemetas)
             );
         });
 

classes/local/form/elements/wysiwyg_editor_element.php

@@ -128,13 +128,13 @@ public function render_to(render_context $context): void {
             file_remove_editor_orphaned_files($mydata);
 
             global $USER;
-            /** @var array<string, file_metadata> $metadatabyname */
-            $metadatabyname = di::get(options_file_service::class)->get_qpy_files_metadata_from_draftitem($USER->id, $draftitemid);
+            /** @var file_metadata[] $filemetas */
+            $filemetas = di::get(options_file_service::class)->get_qpy_files_metadata_from_draftitem($USER->id, $draftitemid);
 
             // Since we had to format_text before URL replacement, we need to do it to both $markup and $html :(.
-            $markup = self::replace_draftfile_urls_with_qpy_urls($markup, $metadatabyname, $draftitemid);
+            $markup = self::replace_draftfile_urls_with_qpy_urls($markup, $filemetas, $draftitemid);
             if ($html) {
-                $html = self::replace_draftfile_urls_with_qpy_urls($html, $metadatabyname, $draftitemid);
+                $html = self::replace_draftfile_urls_with_qpy_urls($html, $filemetas, $draftitemid);
             }
 
             $mappedformat = self::FORMAT_MAP[$format] ?? null;
@@ -145,7 +145,7 @@ public function render_to(render_context $context): void {
             $resultdata = new wysiwyg_editor_data(
                 markup: $markup,
                 markupformat: $mappedformat,
-                files: $metadatabyname,
+                files: $filemetas,
                 html: $html
             );
 
@@ -177,13 +177,13 @@ public function render_to(render_context $context): void {
                 $ofs->prepare_draft_area($context->question->contextid, $questionid, $mydata->files, $USER->id, $draftitemid);
             }
 
-            $filenamebyfileref = array_flip(array_map(fn($fmeta) => $fmeta->fileref, $mydata->files));
+            $filenamebyfileref = array_column($mydata->files, 'filename', 'fileref');
 
             $replacedtext = preg_replace_callback(
                 constants::QPY_OPTIONS_URL_PATTERN,
                 function (array $match) use ($draftitemid, $filenamebyfileref) {
                     $filename = $filenamebyfileref[strtolower($match['fileref'])] ?? null;
-                    if (!$filename) {
+                    if ($filename === null) {
                         debugging("Editor text contains QPy URL for nonexistent options file: '$match[0]'");
                         return $match[0];
                     }
@@ -212,13 +212,15 @@ function (array $match) use ($draftitemid, $filenamebyfileref) {
      * In a similar manner to {@see file_rewrite_urls_to_pluginfile()}, this method rewrites draftfile URLs to `qpy://options/...`.
      *
      * @param string $text
-     * @param array $metadatabyname As returned by {@see options_file_service::get_qpy_files_metadata_from_draftitem()}.
+     * @param array $files As returned by {@see options_file_service::get_qpy_files_metadata_from_draftitem()}.
      * @param int $draftitemid
      * @return string
      */
-    private static function replace_draftfile_urls_with_qpy_urls(string $text, array $metadatabyname, int $draftitemid): string {
+    private static function replace_draftfile_urls_with_qpy_urls(string $text, array $files, int $draftitemid): string {
         global $USER;
 
+        $filesbyname = array_column($files, null, 'filename');
+
         $draftfileurls = extract_draft_file_urls_from_text($text);
         $expectedparts = [
             'contextid' => context_user::instance($USER->id)->id,
@@ -236,7 +238,7 @@ private static function replace_draftfile_urls_with_qpy_urls(string $text, array
                 }
             }
 
-            $fileref = $metadatabyname[$draftfileurl['filename']]->fileref ?? null;
+            $fileref = $filesbyname[$draftfileurl['filename']]->fileref ?? null;
             if (!$fileref) {
                 debugging("Editor text contains draftfile link to nonexistent file {$draftfileurl['filename']}.", DEBUG_DEVELOPER);
                 continue;