MDL-35380 SCORM: improve check for imsmanifest file and consolidate i…

…nto a single function.

mod/scorm/lang/en/scorm.php

@@ -59,7 +59,8 @@
 $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['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';

mod/scorm/lib.php

@@ -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;

mod/scorm/locallib.php

@@ -1886,3 +1886,41 @@ function scorm_check_url($url) {
 
     return true;
 }
+
+/**
+ * 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 = $packer->list_files($file);
+
+    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;
+}

mod/scorm/mod_form.php

@@ -346,28 +346,8 @@ function validation($data, $files) {
                 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
-                    }
-                }
+                $errors = array_merge($errors, scorm_validate_package($filename));
+
                 unlink($filename);
             }
 

mod/scorm/tests/validatepackage_test.php

@@ -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();
+    }
+}
+