Merge branch 'MDL-67296-master' of git://github.com/vmdef/moodle

h5p/classes/core.php

@@ -27,6 +27,7 @@ namespace core_h5p;
 defined('MOODLE_INTERNAL') || die();
 
 require_once("$CFG->libdir/filelib.php");
+
 use H5PCore;
 use H5PFrameworkInterface;
 use stdClass;
@@ -170,6 +171,9 @@ class core extends \H5PCore {
 
         $typesinstalled = [];
 
+        $factory = new factory();
+        $framework = $factory->get_framework();
+
         foreach ($contenttypes->contentTypes as $type) {
             // Don't fetch content types that require a higher H5P core API version.
             if (!$this->is_required_core_api($type->coreApiVersionNeeded)) {
@@ -183,9 +187,6 @@ class core extends \H5PCore {
                 'patchVersion' => $type->version->patch
             ];
 
-            $factory = new \core_h5p\factory();
-            $framework = $factory->get_framework();
-
             $shoulddownload = true;
             if ($framework->getLibraryId($type->id, $type->version->major, $type->version->minor)) {
                 if (!$framework->isPatchedLibrary($library)) {
@@ -216,7 +217,7 @@ class core extends \H5PCore {
      * @return int|null Returns the id of the content type library installed, null otherwise.
      */
     public function fetch_content_type(array $library): ?int {
-        $factory = new \core_h5p\factory();
+        $factory = new factory();
 
         // Get a temp path to download the content type.
         $temppath = make_request_directory();
@@ -247,7 +248,9 @@ class core extends \H5PCore {
         $file->delete();
 
         $librarykey = static::libraryToString($library);
-        return $factory->get_storage()->h5pC->librariesJsonData[$librarykey]["libraryId"];
+        $libraryid = $factory->get_storage()->h5pC->librariesJsonData[$librarykey]["libraryId"];
+
+        return $libraryid;
     }
 
     /**

h5p/classes/framework.php

@@ -100,7 +100,7 @@ class framework implements \H5PFrameworkInterface {
         $response = download_file_content($url, null, $data, true, 300, 20,
                 false, $stream);
 
-        if (empty($response->error)) {
+        if (empty($response->error) && ($response->status != '404')) {
             return $response->results;
         } else {
             $this->setErrorMessage($response->error, 'failed-fetching-external-data');

h5p/tests/framework_test.php

@@ -81,8 +81,9 @@ class framework_testcase extends \advanced_testcase {
 
         $this->resetAfterTest();
 
+        $library = 'H5P.Accordion';
         // Provide a valid URL to an external H5P content.
-        $url = "https://h5p.org/sites/default/files/h5p/exports/arithmetic-quiz-22-57860.h5p";
+        $url = $this->getExternalTestFileUrl('/'.$library.'.h5p');
 
         // Test fetching an external H5P content without defining a path to where the file should be stored.
         $data = $this->framework->fetchExternalData($url, null, true);
@@ -112,8 +113,9 @@ class framework_testcase extends \advanced_testcase {
 
         $this->resetAfterTest();
 
+        $library = 'H5P.Accordion';
         // Provide a valid URL to an external H5P content.
-        $url = "https://h5p.org/sites/default/files/h5p/exports/arithmetic-quiz-22-57860.h5p";
+        $url = $this->getExternalTestFileUrl('/'.$library.'.h5p');
 
         $h5pfolderpath = $CFG->tempdir . uniqid('/h5p-');
 
@@ -145,7 +147,7 @@ class framework_testcase extends \advanced_testcase {
         $this->resetAfterTest();
 
         // Provide an URL to an external file that is not an H5P content file.
-        $url = "https://www.w3.org/WAI/ER/tests/xhtml/testfiles/resources/pdf/dummy.pdf";
+        $url = $this->getExternalTestFileUrl('/h5pcontenttypes.json');
 
         $data = $this->framework->fetchExternalData($url, null, true);
 
@@ -156,7 +158,7 @@ class framework_testcase extends \advanced_testcase {
         // The uploaded file should exist on the filesystem with it's original extension.
         // NOTE: The file would be later validated by the H5P Validator.
         $h5pfolderpath = $this->framework->getUploadedH5pFolderPath();
-        $this->assertTrue(file_exists($h5pfolderpath . '.pdf'));
+        $this->assertTrue(file_exists($h5pfolderpath . '.json'));
     }
 
     /**

h5p/tests/generator/lib.php

@@ -23,7 +23,8 @@
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 
-use core_h5p\factory;
+use core_h5p\autoloader;
+use core_h5p\core;
 
 defined('MOODLE_INTERNAL') || die();
 
@@ -343,30 +344,28 @@ class core_h5p_generator extends \component_generator_base {
     }
 
     /**
-     * Create content type records in the h5p_libraries database table.
+     * Create H5P content type records in the h5p_libraries database table.
      *
-     * @param int $pending Number of content types not installed
+     * @param array $typestonotinstall H5P content types that should not be installed
+     * @param core $core h5p_test_core instance required to use the exttests URL
      * @return array Data of the content types not installed.
      */
-    public function create_content_types(int $pending): array {
+    public function create_content_types(array $typestonotinstall, core $core): array {
         global $DB;
 
-        $factory = new factory();
+        autoloader::register();
-        $core = $factory->get_core();
 
         // Get info of latest content types versions.
         $contenttypes = $core->get_latest_content_types()->contentTypes;
 
-        $size = count($contenttypes) - $pending;
+        $installedtypes = 0;
-
-        // Avoid to install 2 content types.
-        $chunks = array_chunk($contenttypes, $size);
-
-        $contenttypes = $chunks[0];
-        $pendingtypes = $chunks[1];
 
         // Fake installation of all other H5P content types.
         foreach ($contenttypes as $contenttype) {
+            // Don't install pending content types.
+            if (in_array($contenttype->id, $typestonotinstall)) {
+                continue;
+            }
             $library = [
                 'machinename' => $contenttype->id,
                 'majorversion' => $contenttype->version->major,
@@ -377,8 +376,9 @@ class core_h5p_generator extends \component_generator_base {
                 'coreminor' => $contenttype->coreApiVersionNeeded->minor
             ];
             $DB->insert_record('h5p_libraries', (object) $library);
+            $installedtypes++;
         }
 
-        return [$contenttypes, $pendingtypes];
+        return [$installedtypes, count($typestonotinstall)];
     }
 }

h5p/tests/h5p_core_test.php

@@ -23,9 +23,7 @@
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 
-namespace core_h5p\local\tests;
+namespace core_h5p;
-
-use core_h5p\factory;
 
 defined('MOODLE_INTERNAL') || die();
 
@@ -41,10 +39,16 @@ defined('MOODLE_INTERNAL') || die();
 class h5p_core_test extends \advanced_testcase {
 
     protected function setup() {
+        global $CFG;
         parent::setUp();
 
-        $factory = new factory();
+        autoloader::register();
+
+        require_once($CFG->libdir . '/tests/fixtures/testable_core_h5p.php');
+
+        $factory = new h5p_test_factory();
         $this->core = $factory->get_core();
+        $this->core->set_endpoint($this->getExternalTestFileUrl(''));
     }
 
     /**
@@ -54,12 +58,12 @@ class h5p_core_test extends \advanced_testcase {
     public function test_fetch_content_type(): void {
         global $DB;
 
-        $this->resetAfterTest(true);
-
         if (!PHPUNIT_LONGTEST) {
             $this->markTestSkipped('PHPUNIT_LONGTEST is not defined');
         }
 
+        $this->resetAfterTest(true);
+
         // Get info of latest content types versions.
         $contenttypes = $this->core->get_latest_content_types()->contentTypes;
         // We are installing the first content type.
@@ -95,27 +99,27 @@ class h5p_core_test extends \advanced_testcase {
     public function test_fetch_latest_content_types(): void {
         global $DB;
 
-        $this->resetAfterTest(true);
-
         if (!PHPUNIT_LONGTEST) {
             $this->markTestSkipped('PHPUNIT_LONGTEST is not defined');
         }
 
+        $this->resetAfterTest(true);
+
         $contentfiles = $DB->count_records('h5p_libraries');
 
         // Initially there are no h5p records in database.
         $this->assertEquals(0, $contentfiles);
 
+        $contenttypespending = ['H5P.Accordion'];
+
         // Fetch generator.
         $generator = \testing_util::get_data_generator();
         $h5pgenerator = $generator->get_plugin_generator('core_h5p');
 
         // Get info of latest content types versions.
-        [$contenttypes, $contenttoinstall] = $h5pgenerator->create_content_types(1);
+        [$installedtypes, $typesnotinstalled] = $h5pgenerator->create_content_types($contenttypespending, $this->core);
         // Number of H5P content types.
-        $numcontenttypes = count($contenttypes) + count($contenttoinstall);
+        $numcontenttypes = $installedtypes + $typesnotinstalled;
-
-        $contenttoinstall = $contenttoinstall[0];
 
         // Content type libraries has runnable set to 1.
         $conditions = ['runnable' => 1];
@@ -123,7 +127,7 @@ class h5p_core_test extends \advanced_testcase {
 
         // There is a record for each installed content type, except the one that was hold for later.
         $this->assertEquals($numcontenttypes - 1, count($contentfiles));
-        $this->assertArrayNotHasKey($contenttoinstall->id, $contentfiles);
+        $this->assertArrayNotHasKey($contenttypespending[0], $contentfiles);
 
         $result = $this->core->fetch_latest_content_types();
 
@@ -131,9 +135,9 @@ class h5p_core_test extends \advanced_testcase {
 
         // There is a new record for the new installed content type.
         $this->assertCount($numcontenttypes, $contentfiles);
-        $this->assertArrayHasKey($contenttoinstall->id, $contentfiles);
+        $this->assertArrayHasKey($contenttypespending[0], $contentfiles);
         $this->assertCount(1, $result->typesinstalled);
-        $this->assertStringStartsWith($contenttoinstall->id, $result->typesinstalled[0]['name']);
+        $this->assertStringStartsWith($contenttypespending[0], $result->typesinstalled[0]['name']);
 
         // New execution doesn't install any content type.
         $result = $this->core->fetch_latest_content_types();

lib/classes/task/h5p_get_content_types_task.php

@@ -44,13 +44,23 @@ class h5p_get_content_types_task extends scheduled_task {
         return get_string('h5pgetcontenttypestask', 'admin');
     }
 
+    /**
+     * Get an \core_h5p\core instance.
+     *
+     * @return \core_h5p\core
+     */
+    public function get_core() {
+        $factory = new factory();
+        $core = $factory->get_core();
+        return $core;
+    }
+
     /**
      * Execute the task.
      */
     public function execute() {
 
-        $factory = new factory();
+        $core = $this->get_core();
-        $core = $factory->get_core();
 
         $result = $core->fetch_latest_content_types();
 

lib/tests/fixtures/testable_core_h5p.php

@@ -0,0 +1,102 @@
+<?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/>.
+
+/**
+ * Fixture for testing the functionality of core_h5p.
+ *
+ * @package     core
+ * @subpackage  fixtures
+ * @category    test
+ * @copyright   2019 Victor Deniz <victor@moodle.com>
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+namespace core_h5p;
+
+defined('MOODLE_INTERNAL') || die();
+
+/**
+ * H5P factory class stub for testing purposes.
+ *
+ * This class extends the real one to return the H5P core class stub.
+ *
+ * @copyright  2019 Victor Deniz <victor@moodle.com>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class h5p_test_factory extends factory {
+    /**
+     * Returns an instance of the \core_h5p\core stub class.
+     *
+     * @return core_h5p\core
+     */
+    public function get_core(): core {
+        if ($this->core === null ) {
+            $fs = new file_storage();
+            $language = framework::get_language();
+            $context = \context_system::instance();
+
+            $url = \moodle_url::make_pluginfile_url($context->id, 'core_h5p', '', null,
+                '', '')->out();
+
+            $this->core = new h5p_test_core($this->get_framework(), $fs, $url, $language, true);
+        }
+
+        return $this->core;
+    }
+}
+
+/**
+ * H5P core class stub for testing purposes.
+ *
+ * Modifies get_api_endpoint method to use local URLs.
+ *
+ * @copyright   2019 Victor Deniz <victor@moodle.com>
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class h5p_test_core extends core {
+    /** @var $endpoint Endpoint URL for testing H5P API */
+    protected $endpoint;
+
+    /**
+     * Set the endpoint URL
+     *
+     * @param string $url Endpoint URL
+     * @return void
+     */
+    public function set_endpoint($url): void {
+        $this->endpoint = $url;
+    }
+
+    /**
+     * Get the URL of the test endpoints instead of the H5P ones.
+     *
+     * If $library is null, moodle_url is the endpoint of the json test file with the H5P content types definition. If library is
+     * the machine name of a content type, moodle_url is the test URL for downloading the content type file.
+     *
+     * @param string|null $library The filename of the H5P content type file in external.
+     * @return \moodle_url The moodle_url of the file in external.
+     */
+    public function get_api_endpoint(?string $library): \moodle_url {
+
+        if ($library) {
+            $h5purl = $this->endpoint . '/' . $library . '.h5p';
+        } else {
+            $h5purl = $h5purl = $this->endpoint . '/h5pcontenttypes.json';
+        }
+
+        return new \moodle_url($h5purl);
+    }
+}

lib/tests/h5p_get_content_types_task_test.php

@@ -22,6 +22,9 @@
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 
+use core_h5p\autoloader;
+use core_h5p\h5p_test_factory;
+
 defined('MOODLE_INTERNAL') || die();
 
 /**
@@ -35,12 +38,22 @@ defined('MOODLE_INTERNAL') || die();
  */
 class h5p_get_content_types_task_test extends advanced_testcase {
 
+    protected function setup() {
+        global $CFG;
+        parent::setUp();
+
+        autoloader::register();
+
+        require_once($CFG->libdir . '/tests/fixtures/testable_core_h5p.php');
+    }
+
     /**
      * Test task execution
      *
      * return void
      */
     public function test_task_execution(): void {
+
         if (!PHPUNIT_LONGTEST) {
             $this->markTestSkipped('PHPUNIT_LONGTEST is not defined');
         }
@@ -51,10 +64,23 @@ class h5p_get_content_types_task_test extends advanced_testcase {
         $generator = \testing_util::get_data_generator();
         $h5pgenerator = $generator->get_plugin_generator('core_h5p');
 
-        $h5pgenerator->create_content_types(2);
+        $factory = new h5p_test_factory();
+        $core = $factory->get_core();
+        $core->set_endpoint($this->getExternalTestFileUrl(''));
+        $contenttypespending = ['H5P.Accordion'];
 
-        $task = new \core\task\h5p_get_content_types_task();
+        $h5pgenerator->create_content_types( $contenttypespending, $core);
-        $task->execute();
+
-        $this->expectOutputRegex('/2 new content types/');
+        // Mock implementation of \core\task\h5p_get_content_types_task::get_core to avoid external systems.
+        $mocktask = $this->getMockBuilder(\core\task\h5p_get_content_types_task::class)
+            ->setMethods(['get_core'])
+            ->getMock();
+
+        $mocktask->expects($this->any())
+            ->method('get_core')
+            ->willReturn($core);
+
+        $mocktask->execute();
+        $this->expectOutputRegex('/1 new content types/');
     }
 }