Revert "MDL-35380 SCORM: improve check for imsmanifest file and consolidate into a single function."

This reverts commit 492407e9f7.

mod/scorm/lang/en/scorm.php

@@ -59,8 +59,7 @@ $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['badimsmanifestlocation'] = 'An imsmanifest.xml file was found but it was not in the root of your zip file, please re-package your SCORM';
+$string['badpackage'] = 'The specified package/manifest is not valid. Check it and try again.';
-$string['badarchive'] = 'You must provide a valid zip file';
 $string['browse'] = 'Preview';
 $string['browsed'] = 'Browsed';
 $string['browsemode'] = 'Preview mode';
@@ -223,7 +222,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'] = 'Incorrect file package - missing imsmanifest.xml or AICC structure';
+$string['nomanifest'] = 'Manifest not found';
 $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,10 +1271,32 @@ function scorm_dndupload_handle($uploadinfo) {
     $file = reset($files);
 
     // Validate the file, make sure it's a valid SCORM package!
-    $errors = scorm_validate_package($file);
+    $packer = get_file_packer('application/zip');
-    if (!empty($errors)) {
+    $filelist = $file->list_files($packer);
+
+    if (!is_array($filelist)) {
         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,41 +1886,3 @@ 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,8 +346,28 @@ class mod_scorm_mod_form extends moodleform_mod {
                 make_temp_directory('scormimport');
                 $file->copy_content_to($filename);
 
-                $errors = array_merge($errors, scorm_validate_package($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);
             }
 

mod/scorm/tests/validatepackage_test.php

@@ -1,77 +0,0 @@
-<?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();
-    }
-}
-