From 87fbf39aaeb581c96bb7c5ee42e20564b9e99b58 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Sat, 25 Mar 2023 13:43:55 +0100 Subject: [PATCH] MDL-77333 mod_resource: fixes generator uploading files + tests MDL-76499 revealed a few problems with resource generators: 1. We were not covering with unit tests the upload of files from disk (and here it's where the problem was). 2. There was a little of confusion between disk paths (only needed to upload files) and file_area paths (the generator only creates or uploads files to the root directory of the file area. 3. It was possible to request the upload of a file to the generator without that file effectively existing. This commit fixes those points and covers 99% of the generator code. --- mod/resource/tests/generator/lib.php | 19 ++++++++++++---- mod/resource/tests/generator_test.php | 43 +++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/mod/resource/tests/generator/lib.php b/mod/resource/tests/generator/lib.php index 7d8234cb7d6..22d6caa483c 100644 --- a/mod/resource/tests/generator/lib.php +++ b/mod/resource/tests/generator/lib.php @@ -77,8 +77,6 @@ class mod_resource_generator extends testing_module_generator { } $usercontext = context_user::instance($USER->id); $filename = $record->defaultfilename ?? 'resource' . ($this->instancecount + 1) . '.txt'; - // Set filepath depending on filename. - $filepath = (isset($record->defaultfilename)) ? $CFG->dirroot . '/' : '/'; // Pick a random context id for specified user. $record->files = file_get_unused_draft_itemid(); @@ -86,11 +84,22 @@ class mod_resource_generator extends testing_module_generator { // Add actual file there. $filerecord = ['component' => 'user', 'filearea' => 'draft', 'contextid' => $usercontext->id, 'itemid' => $record->files, - 'filename' => basename($filename), 'filepath' => $filepath]; + 'filename' => basename($filename), 'filepath' => '/']; $fs = get_file_storage(); if ($record->uploaded == 1) { + // For uploading a file, it's required to specify a file, how not! + if (!isset($record->defaultfilename)) { + throw new coding_exception( + 'The $record->defaultfilename option is required in order to upload a file'); + } + // We require the full file path to exist when uploading a real file (fixture or whatever). + $fullfilepath = $CFG->dirroot . '/' . $record->defaultfilename; + if (!is_readable($fullfilepath)) { + throw new coding_exception( + 'The $record->defaultfilename option must point to an existing file within dirroot'); + } // Create file using pathname (defaultfilename) set. - $fs->create_file_from_pathname($filerecord, $filepath . $filename); + $fs->create_file_from_pathname($filerecord, $fullfilepath); } else { // If defaultfilename is not set, create file from string "resource 1.txt". $fs->create_file_from_string($filerecord, 'Test resource ' . $filename . ' file'); @@ -98,6 +107,6 @@ class mod_resource_generator extends testing_module_generator { } // Do work to actually add the instance. - return parent::create_instance($record, (array)$options); + return parent::create_instance($record, $options); } } diff --git a/mod/resource/tests/generator_test.php b/mod/resource/tests/generator_test.php index 291b735ac22..af6016a958d 100644 --- a/mod/resource/tests/generator_test.php +++ b/mod/resource/tests/generator_test.php @@ -21,10 +21,12 @@ namespace mod_resource; * * @package mod_resource * @category phpunit + * @covers \mod_resource_generator * @copyright 2013 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class generator_test extends \advanced_testcase { + public function test_generator() { global $DB, $SITE; @@ -76,5 +78,46 @@ class generator_test extends \advanced_testcase { $this->assertCount(1, $files); $this->assertEquals('myfile.pdf', $file->get_filename()); $this->assertEquals('Test resource myfile.pdf file', $file->get_content()); + + // Create a new resource uploading a file. + $resource = $generator->create_instance([ + 'course' => $SITE->id, + 'uploaded' => true, + 'defaultfilename' => 'mod/resource/tests/fixtures/samplefile.txt', + ]); + + // Check that generated resource module contains the uploaded samplefile.txt. + $cm = get_coursemodule_from_instance('resource', $resource->id); + $context = \context_module::instance($cm->id); + $files = $fs->get_area_files($context->id, 'mod_resource', 'content', false, '', false); + $file = array_values($files)[0]; + $this->assertCount(1, $files); + $this->assertEquals('samplefile.txt', $file->get_filename()); + $this->assertEquals('Hello!', $file->get_content()); + + // Try to generate a resource with uploaded file without specifying the file. + try { + $resource = $generator->create_instance([ + 'course' => $SITE->id, + 'uploaded' => true, + ]); + $this->assertTrue(false, 'coding_exception expected, defaultfilename is required'); + } catch (\Exception $e) { + $this->assertInstanceOf(\coding_exception::class, $e); + $this->assertStringContainsString('defaultfilename option is required', $e->getMessage()); + } + + // Try to generate a resource with uploaded file pointing to non-existing file. + try { + $resource = $generator->create_instance([ + 'course' => $SITE->id, + 'uploaded' => true, + 'defaultfilename' => 'mod/resource/tests/fixtures/doesnotexist.txt', + ]); + $this->assertTrue(false, 'coding_exception expected, defaultfilename must point to an existing file'); + } catch (\Exception $e) { + $this->assertInstanceOf(\coding_exception::class, $e); + $this->assertStringContainsString('defaultfilename option must point to an existing file', $e->getMessage()); + } } } -- 2.11.4.GIT