diff --git a/set_ode.php b/set_ode.php index d938f30..21fac8b 100644 --- a/set_ode.php +++ b/set_ode.php @@ -143,25 +143,43 @@ echo json_encode($resultmsg); exit(1); } -// Package is valid so delete files from package area and move the new one. -$fs->delete_area_files($context->id, 'mod_exeweb', 'package'); -$exeweb->revision++; -$fileinfo['itemid'] = $exeweb->revision; +// Store the new package on a fresh revision before touching the previous one. +// Old package and content files are deleted only after the new revision is in place, +// so the activity remains consistent if extraction fails midway. +$newrevision = (int)$exeweb->revision + 1; +$fileinfo['itemid'] = $newrevision; $fileinfo['filearea'] = 'package'; $package = $fs->create_file_from_storedfile($fileinfo, $tmpfile); $fs->delete_area_files($context->id, 'mod_exeweb', 'temppackage'); -// Process package contents. + +// Process package contents. expand_package() deletes content files for ALL itemids +// before extracting the new ones into $package->get_itemid(), keeping the file area clean. $contentslist = exeweb_package::expand_package($package); -$mainfile = exeweb_package::get_mainfile($contentslist, $package->get_contextid()); +// Pass $package->get_itemid() so get_mainfile() searches in the new revision. +// Without it the lookup defaulted to itemid 0 and the entry file was never refreshed, +// which left the activity pointing to outdated entrypath/entryname after each online save. +$mainfile = exeweb_package::get_mainfile($contentslist, $package->get_contextid(), $package->get_itemid()); if ($mainfile !== false) { file_set_sortorder($mainfile->get_contextid(), 'mod_exeweb', 'content', - $exeweb->revision, $mainfile->get_filepath(), $mainfile->get_filename(), 1); - $data->entrypath = $mainfile->get_filepath(); - $data->entryname = $mainfile->get_filename(); + $newrevision, $mainfile->get_filepath(), $mainfile->get_filename(), 1); + $exeweb->entrypath = $mainfile->get_filepath(); + $exeweb->entryname = $mainfile->get_filename(); } +// Persist the new revision and entry file metadata together. +$exeweb->revision = $newrevision; +$exeweb->timemodified = time(); +$exeweb->usermodified = $user->id; $DB->update_record('exeweb', $exeweb); +// Delete leftover package files from previous revisions only after success. +$packagefiles = $fs->get_area_files($context->id, 'mod_exeweb', 'package', false, 'itemid', false); +foreach ($packagefiles as $storedfile) { + if ((int)$storedfile->get_itemid() !== $newrevision) { + $storedfile->delete(); + } +} + // Prepare OK response. $resultmsg['status'] = '0'; $resultmsg['description'] = 'OK'; diff --git a/tests/exeweb_package_test.php b/tests/exeweb_package_test.php new file mode 100644 index 0000000..495a24b --- /dev/null +++ b/tests/exeweb_package_test.php @@ -0,0 +1,176 @@ +. + +/** + * Unit tests for mod_exeweb package handling. + * + * Covers the regression behind issue #42 where webzip packages received from + * eXeLearning Online were extracted into the right itemid but + * {@see exeweb_package::get_mainfile()} was queried with the default itemid + * (0), leaving entrypath/entryname stale and breaking image references on the + * served HTML page. + * + * @package mod_exeweb + * @copyright 2026 eXeLearning + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +namespace mod_exeweb; + +/** + * Unit tests for {@see exeweb_package}. + */ +final class exeweb_package_test extends \advanced_testcase { + + /** + * Build an in-memory zip mimicking the layout produced by Html5Exporter + * when eXeLearning Online sends a webzip back to mod_exeweb. The structure + * matches what the editor generates: index.html at the root, an asset + * inside content/resources/, and a content.xml descriptor (the only file + * the validation rules require by default). + * + * @return string Raw zip contents. + */ + private function build_webzip_contents(): string { + $tempzip = tempnam(make_request_directory(), 'exeweb_test_'); + $zip = new \ZipArchive(); + $zip->open($tempzip, \ZipArchive::OVERWRITE); + $zip->addFromString('index.html', ''); + $zip->addFromString('content/resources/photo.png', "PNG\x89\x50\x4e\x47\r\n\x1a\nFAKE"); + $zip->addFromString('content.xml', ''); + $zip->close(); + + $bytes = file_get_contents($tempzip); + @unlink($tempzip); + + return $bytes; + } + + /** + * Store a synthetic webzip in the package filearea at the requested itemid + * and return the resulting stored_file. Mirrors the relevant portion of + * set_ode.php so the test exercises the same Moodle file API calls. + * + * @param int $contextid + * @param int $itemid + * @return \stored_file + */ + private function store_package_file(int $contextid, int $itemid): \stored_file { + $fs = get_file_storage(); + $fileinfo = [ + 'contextid' => $contextid, + 'component' => 'mod_exeweb', + 'filearea' => 'package', + 'itemid' => $itemid, + 'filepath' => '/', + 'filename' => 'package.zip', + ]; + return $fs->create_file_from_string($fileinfo, $this->build_webzip_contents()); + } + + /** + * Regression test: get_mainfile() must locate index.html in the same + * itemid where expand_package() extracted the contents. The default + * itemid 0 used by the previous implementation silently missed the entry + * file when the package was stored on a non-zero revision (the case for + * every online save after the first one). + * + * @covers \mod_exeweb\exeweb_package::expand_package + * @covers \mod_exeweb\exeweb_package::get_mainfile + */ + public function test_get_mainfile_uses_package_itemid(): void { + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course(); + $exeweb = $this->getDataGenerator()->create_module('exeweb', ['course' => $course->id]); + $context = \context_module::instance($exeweb->cmid); + + // Simulate an online save landing on revision 3 (the regression + // scenario reported in issue #42). + $itemid = 3; + $package = $this->store_package_file($context->id, $itemid); + + $contentslist = exeweb_package::expand_package($package); + $this->assertNotEmpty($contentslist, 'expand_package must return the extracted file list'); + + $mainfile = exeweb_package::get_mainfile($contentslist, $context->id, $itemid); + $this->assertNotFalse($mainfile, 'get_mainfile must locate index.html when given the correct itemid'); + $this->assertSame('index.html', $mainfile->get_filename()); + $this->assertSame('/', $mainfile->get_filepath()); + + // Default-argument behaviour exposes the bug: index.html lives at + // itemid 3, but a call without itemid hits the default 0 and returns + // false. + $missingmainfile = exeweb_package::get_mainfile($contentslist, $context->id); + $this->assertFalse($missingmainfile, + 'A call without the itemid argument must miss the entry file (regression for issue #42)'); + } + + /** + * Verify that the package extraction places nested resources in the same + * relative path that the HTML file references. This mirrors the URL the + * Moodle pluginfile handler builds when serving the activity, so a hit + * here implies the served page can resolve its assets. + * + * @covers \mod_exeweb\exeweb_package::expand_package + */ + public function test_expand_package_preserves_resource_paths(): void { + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course(); + $exeweb = $this->getDataGenerator()->create_module('exeweb', ['course' => $course->id]); + $context = \context_module::instance($exeweb->cmid); + + $itemid = 5; + $package = $this->store_package_file($context->id, $itemid); + + exeweb_package::expand_package($package); + + $fs = get_file_storage(); + $resource = $fs->get_file($context->id, 'mod_exeweb', 'content', $itemid, + '/content/resources/', 'photo.png'); + $this->assertNotFalse($resource, + 'Resources referenced by HTML must extract to the same path within the content filearea'); + } + + /** + * Calling expand_package() twice with two different revisions reflects an + * online save flow. After the second call only files for the latest + * revision remain in the content filearea, matching the URL the activity + * view will request. + * + * @covers \mod_exeweb\exeweb_package::expand_package + */ + public function test_expand_package_replaces_old_revisions(): void { + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course(); + $exeweb = $this->getDataGenerator()->create_module('exeweb', ['course' => $course->id]); + $context = \context_module::instance($exeweb->cmid); + + $firstpackage = $this->store_package_file($context->id, 1); + exeweb_package::expand_package($firstpackage); + + $secondpackage = $this->store_package_file($context->id, 2); + exeweb_package::expand_package($secondpackage); + + $fs = get_file_storage(); + $previous = $fs->get_area_files($context->id, 'mod_exeweb', 'content', 1, '', false); + $this->assertEmpty($previous, 'Stale content from previous revisions must be cleaned up'); + + $current = $fs->get_area_files($context->id, 'mod_exeweb', 'content', 2, '', false); + $this->assertNotEmpty($current, 'Latest revision must hold the freshly extracted files'); + } +}