MDL-35380 SCORM: improve check for imsmanifest file and consolidate into a single...
authorDan Marsden <dan@danmarsden.com>
Fri, 5 Jul 2013 04:38:10 +0000 (16:38 +1200)
committerDan Marsden <dan@danmarsden.com>
Mon, 26 Aug 2013 01:06:59 +0000 (13:06 +1200)
mod/scorm/lang/en/scorm.php
mod/scorm/lib.php
mod/scorm/mod_form.php
mod/scorm/tests/packages/badscorm.zip [new file with mode: 0644]
mod/scorm/tests/packages/invalid.zip [new file with mode: 0644]
mod/scorm/tests/packages/validaicc.zip [new file with mode: 0644]
mod/scorm/tests/packages/validscorm.zip [new file with mode: 0644]
mod/scorm/tests/validatepackage_test.php [new file with mode: 0644]

index dafdd26..9d370e5 100644 (file)
@@ -59,7 +59,8 @@ $string['autocontinue_help'] = 'If enabled, subsequent learning objects are laun
 $string['autocontinuedesc'] = 'If enabled, subsequent learning objects are launched automatically, otherwise the Continue button must be used.';
 $string['averageattempt'] = 'Average attempts';
 $string['badmanifest'] = 'Some manifest errors: see errors log';
-$string['badpackage'] = 'The specified package/manifest is not valid. Check it and try again.';
+$string['badimsmanifestlocation'] = 'An imsmanifest.xml file was found but it was not in the root of your zip file, please re-package your SCORM';
+$string['badarchive'] = 'You must provide a valid zip file';
 $string['browse'] = 'Preview';
 $string['browsed'] = 'Browsed';
 $string['browsemode'] = 'Preview mode';
@@ -222,7 +223,7 @@ $string['noattemptsmade'] = 'Number of attempts you have made';
 $string['no_attributes'] = 'Tag {$a->tag} must have attributes';
 $string['no_children'] = 'Tag {$a->tag} must have children';
 $string['nolimit'] = 'Unlimited attempts';
-$string['nomanifest'] = 'Manifest not found';
+$string['nomanifest'] = 'Incorrect file package - missing imsmanifest.xml or AICC structure';
 $string['noprerequisites'] = 'Sorry but you don\'t have the required prerequisites to access this activity.';
 $string['noreports'] = 'No report to display';
 $string['normal'] = 'Normal';
index 94e8103..02da6c9 100644 (file)
@@ -1271,32 +1271,10 @@ function scorm_dndupload_handle($uploadinfo) {
     $file = reset($files);
 
     // Validate the file, make sure it's a valid SCORM package!
-    $packer = get_file_packer('application/zip');
-    $filelist = $file->list_files($packer);
-
-    if (!is_array($filelist)) {
+    $errors = scorm_validate_package($file);
+    if (!empty($errors)) {
         return false;
-    } else {
-        $manifestpresent = false;
-        $aiccfound = false;
-
-        foreach ($filelist as $info) {
-            if ($info->pathname == 'imsmanifest.xml') {
-                $manifestpresent = true;
-                break;
-            }
-
-            if (preg_match('/\.cst$/', $info->pathname)) {
-                $aiccfound = true;
-                break;
-            }
-        }
-
-        if (!$manifestpresent && !$aiccfound) {
-            return false;
-        }
     }
-
     // Create a default scorm object to pass to scorm_add_instance()!
     $scorm = get_config('scorm');
     $scorm->course = $uploadinfo->course->id;
@@ -1343,3 +1321,41 @@ function scorm_set_completion($scorm, $userid, $completionstate = COMPLETION_COM
         $completion->update_state($cm, $completionstate, $userid);
     }
 }
+
+/**
+ * Check that a Zip file contains a valid SCORM package
+ *
+ * @param $file stored_file a Zip file.
+ * @return array empty if no issue is found. Array of error message otherwise
+ */
+function scorm_validate_package($file) {
+    $packer = get_file_packer('application/zip');
+    $errors = array();
+    $filelist = $file->list_files($packer);
+
+    if (!is_array($filelist)) {
+        $errors['packagefile'] = get_string('badarchive', 'scorm');
+    } else {
+        $aiccfound = false;
+        $badmanifestpresent = false;
+        foreach ($filelist as $info) {
+            if ($info->pathname == 'imsmanifest.xml') {
+                return array();
+            } else if (strpos($info->pathname, 'imsmanifest.xml') !== false) {
+                // This package has an imsmanifest file inside a folder of the package.
+                $badmanifestpresent = true;
+            }
+            if (preg_match('/\.cst$/', $info->pathname)) {
+                return array();
+            }
+        }
+        if (!$aiccfound) {
+            if ($badmanifestpresent) {
+                $errors['packagefile'] = get_string('badimsmanifestlocation', 'scorm');
+            } else {
+                $errors['packagefile'] = get_string('nomanifest', 'scorm');
+            }
+        }
+    }
+    return $errors;
+}
\ No newline at end of file
index 8263a6d..c69ef6f 100644 (file)
@@ -342,33 +342,7 @@ class mod_scorm_mod_form extends moodleform_mod {
                     return $errors;
                 }
                 $file = reset($files);
-                $filename = $CFG->tempdir.'/scormimport/scrom_'.time();
-                make_temp_directory('scormimport');
-                $file->copy_content_to($filename);
-
-                $packer = get_file_packer('application/zip');
-
-                $filelist = $packer->list_files($filename);
-                if (!is_array($filelist)) {
-                    $errors['packagefile'] = 'Incorrect file package - not an archive'; //TODO: localise
-                } else {
-                    $manifestpresent = false;
-                    $aiccfound       = false;
-                    foreach ($filelist as $info) {
-                        if ($info->pathname == 'imsmanifest.xml') {
-                            $manifestpresent = true;
-                            break;
-                        }
-                        if (preg_match('/\.cst$/', $info->pathname)) {
-                            $aiccfound = true;
-                            break;
-                        }
-                    }
-                    if (!$manifestpresent and !$aiccfound) {
-                        $errors['packagefile'] = 'Incorrect file package - missing imsmanifest.xml or AICC structure'; //TODO: localise
-                    }
-                }
-                unlink($filename);
+                $errors = array_merge($errors, scorm_validate_package($file));
             }
 
         } else if ($type === SCORM_TYPE_EXTERNAL) {
diff --git a/mod/scorm/tests/packages/badscorm.zip b/mod/scorm/tests/packages/badscorm.zip
new file mode 100644 (file)
index 0000000..b2c52b6
Binary files /dev/null and b/mod/scorm/tests/packages/badscorm.zip differ
diff --git a/mod/scorm/tests/packages/invalid.zip b/mod/scorm/tests/packages/invalid.zip
new file mode 100644 (file)
index 0000000..f7fc62c
Binary files /dev/null and b/mod/scorm/tests/packages/invalid.zip differ
diff --git a/mod/scorm/tests/packages/validaicc.zip b/mod/scorm/tests/packages/validaicc.zip
new file mode 100644 (file)
index 0000000..9e26697
Binary files /dev/null and b/mod/scorm/tests/packages/validaicc.zip differ
diff --git a/mod/scorm/tests/packages/validscorm.zip b/mod/scorm/tests/packages/validscorm.zip
new file mode 100644 (file)
index 0000000..5d00afb
Binary files /dev/null and b/mod/scorm/tests/packages/validscorm.zip differ
diff --git a/mod/scorm/tests/validatepackage_test.php b/mod/scorm/tests/validatepackage_test.php
new file mode 100644 (file)
index 0000000..2c24981
--- /dev/null
@@ -0,0 +1,77 @@
+<?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/>.
+
+/**
+ * Unit tests for the mod_quiz_display_options class.
+ *
+ * @package    mod_scorm
+ * @category   phpunit
+ * @copyright  2013 Dan Marsden
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+
+defined('MOODLE_INTERNAL') || die();
+
+global $CFG;
+require_once($CFG->dirroot . '/mod/scorm/locallib.php');
+
+
+/**
+ * Unit tests for {@link mod_scorm}.
+ *
+ * @copyright  2013 Dan Marsden
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class mod_scorm_validatepackage_testcase extends basic_testcase {
+    public function test_validate_package() {
+        global $CFG;
+        $filename = "validscorm.zip";
+        $file = new zip_archive();
+        $file->open($CFG->dirroot.'/mod/scorm/tests/packages/'.$filename, file_archive::OPEN);
+        $errors = scorm_validate_package($file);
+        $this->assertEmpty($errors);
+        $file->close();
+
+        $filename = "validaicc.zip";
+        $file = new zip_archive();
+        $file->open($CFG->dirroot.'/mod/scorm/tests/packages/'.$filename, file_archive::OPEN);
+        $errors = scorm_validate_package($file);
+        $this->assertEmpty($errors);
+        $file->close();
+
+        $filename = "invalid.zip";
+        $file = new zip_archive();
+        $file->open($CFG->dirroot.'/mod/scorm/tests/packages/'.$filename, file_archive::OPEN);
+        $errors = scorm_validate_package($file);
+        $this->assertArrayHasKey('packagefile', $errors);
+        if (isset($errors['packagefile'])) {
+            $this->assertEquals(get_string('nomanifest', 'scorm'), $errors['packagefile']);
+        }
+        $file->close();
+
+        $filename = "badscorm.zip";
+        $file = new zip_archive();
+        $file->open($CFG->dirroot.'/mod/scorm/tests/packages/'.$filename, file_archive::OPEN);
+        $errors = scorm_validate_package($file);
+        $this->assertArrayHasKey('packagefile', $errors);
+        if (isset($errors['packagefile'])) {
+            $this->assertEquals(get_string('badimsmanifestlocation', 'scorm'), $errors['packagefile']);
+        }
+        $file->close();
+    }
+}
+