From 2bd972168e7b22aaeea2dd13682ced9cf8233452 Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Mon, 11 Jan 2016 18:31:04 +0100 Subject: [PATCH 01/31] Isolate file operations in a separate class --- h5p-default-storage.class.php | 194 ++++++++++++++++++++++++++++ h5p-file-storage.interface.php | 94 ++++++++++++++ h5p.classes.php | 222 +++++++++++---------------------- 3 files changed, 358 insertions(+), 152 deletions(-) create mode 100644 h5p-default-storage.class.php create mode 100644 h5p-file-storage.interface.php diff --git a/h5p-default-storage.class.php b/h5p-default-storage.class.php new file mode 100644 index 0000000..fb7fb3d --- /dev/null +++ b/h5p-default-storage.class.php @@ -0,0 +1,194 @@ +path = $path; + + // TODO: Check if Dirs are ready? Perhaps in each function? + } + + /** + * Store the library folder. + * + * @param array $library + * Library properties + */ + public function saveLibrary($library) { + $dest = $this->path . '/libraries/' . \H5PCore::libraryToString($library, TRUE); + + // Make sure destination dir doesn't exist + \H5PCore::deleteFileTree($dest); + + // Move library folder + self::copyFileTree($library['uploadDirectory'], $dest); + } + + /** + * Store the content folder. + * + * @param string $source + * Path on file system to content directory. + * @param int $id + * What makes this content unique. + */ + public function saveContent($source, $id) { + self::copyFileTree($source, $this->path . '/content/' . $id); + } + + /** + * Remove content folder. + * + * @param int $id + * Content identifier + */ + public function deleteContent($id) { + // TODO + } + + /** + * Creates a stored copy of the content folder. + * + * @param string $id + * Identifier of content to clone. + * @param int $newId + * The cloned content's identifier + */ + public function cloneContent($id, $newId) { + $path = $this->path . '/content/'; + self::copyFileTree($path . $id, $path . $newId); + } + + /** + * Get path to a new unique tmp folder. + * + * @return string + * Path + */ + public function getTmpPath() { + // TODO + } + + /** + * Fetch content folder and save in target directory. + * + * @param int $id + * Content identifier + * @param string $target + * Where the content folder will be saved + */ + public function exportContent($id, $target) { + self::copyFileTree($this->path . '/content/' . $id, $target); + } + + /** + * Fetch library folder and save in target directory. + * + * @param array $library + * Library properties + * @param string $target + * Where the library folder will be saved + */ + public function exportLibrary($library, $target) { + // TODO + } + + /** + * Save export in file system + * + * @param string $source + * Path on file system to temporary export file. + * @param string $filename + * Name of export file. + */ + public function saveExport($source, $filename) { + // TODO + } + + /** + * Removes given export file + * + * @param string $filename + */ + public function removeExport($filename) { + // TODO + } + + /** + * Recursive function for copying directories. + * + * @param string $source + * From path + * @param string $destination + * To path + * @return boolean + * Indicates if the directory existed. + */ + private static function copyFileTree($source, $destination) { + if (!self::dirReady($destination)) { + throw new \Exception('unabletocopy'); + } + + $dir = opendir($source); + if ($dir === FALSE) { + trigger_error('Unable to open directory ' . $source, E_USER_WARNING); + throw new \Exception('unabletocopy'); + } + + while (false !== ($file = readdir($dir))) { + if (($file != '.') && ($file != '..') && $file != '.git' && $file != '.gitignore') { + if (is_dir($source . DIRECTORY_SEPARATOR . $file)) { + self::copyFileTree($source . DIRECTORY_SEPARATOR . $file, $destination . DIRECTORY_SEPARATOR . $file); + } + else { + copy($source . DIRECTORY_SEPARATOR . $file,$destination . DIRECTORY_SEPARATOR . $file); + } + } + } + closedir($dir); + } + + /** + * Recursive function that makes sure the specified directory exists and + * is writable. + * + * @param string $path + * @return bool + */ + private static function dirReady($path) { + if (!file_exists($path)) { + $parent = preg_replace("/\/[^\/]+\/?$/", '', $path); + if (!self::dirReady($parent)) { + return FALSE; + } + + mkdir($path, 0777, true); + } + + if (!is_dir($path)) { + trigger_error('Path is not a directory ' . $path, E_USER_WARNING); + return FALSE; + } + + if (!is_writable($path)) { + trigger_error('Unable to write to ' . $path . ' – check directory permissions –', E_USER_WARNING); + return FALSE; + } + + return TRUE; + } +} diff --git a/h5p-file-storage.interface.php b/h5p-file-storage.interface.php new file mode 100644 index 0000000..959ba8f --- /dev/null +++ b/h5p-file-storage.interface.php @@ -0,0 +1,94 @@ +h5pC->path . DIRECTORY_SEPARATOR . 'content')) { - $this->h5pF->setErrorMessage($this->h5pF->t('Unable to write to the content directory.')); - return FALSE; - } - if (!H5PCore::dirReady($this->h5pC->path . DIRECTORY_SEPARATOR . 'libraries')) { - $this->h5pF->setErrorMessage($this->h5pF->t('Unable to write to the libraries directory.')); - return FALSE; - } - - // Make sure Zip is present. + // Check dependencies, make sure Zip is present if (!class_exists('ZipArchive')) { $this->h5pF->setErrorMessage($this->h5pF->t('Your PHP version does not support ZipArchive.')); return FALSE; @@ -660,13 +650,6 @@ class H5PValidator { $tmpDir = $this->h5pF->getUploadedH5pFolderPath(); $tmpPath = $this->h5pF->getUploadedH5pPath(); - if (!H5PCore::dirReady($tmpDir)) { - $this->h5pF->setErrorMessage($this->h5pF->t('Unable to write to the temporary directory.')); - return FALSE; - } - - $valid = TRUE; - // Extract and then remove the package file. $zip = new ZipArchive; @@ -689,6 +672,7 @@ class H5PValidator { unlink($tmpPath); // Process content and libraries + $valid = TRUE; $libraries = array(); $files = scandir($tmpDir); $mainH5pData; @@ -1304,10 +1288,13 @@ class H5PStorage { $contentId = $this->h5pC->saveContent($content, $contentMainId); $this->contentId = $contentId; - // Move the content folder - $contents_path = $this->h5pC->path . DIRECTORY_SEPARATOR . 'content'; - $destination_path = $contents_path . DIRECTORY_SEPARATOR . $contentId; - $this->h5pC->copyFileTree($current_path, $destination_path); + try { + // Save content folder contents + $this->h5pC->fs->saveContent($current_path, $contentId); + } + catch (Exception $e) { + $this->h5pF->setErrorMessage($this->h5pF->t($e->getMessage())); + } // Remove temp content folder H5PCore::deleteFileTree($basePath); @@ -1355,13 +1342,10 @@ class H5PStorage { // Save library meta data $this->h5pF->saveLibraryData($library, $new); - // Make sure destination dir is free - $libraries_path = $this->h5pC->path . DIRECTORY_SEPARATOR . 'libraries'; - $destination_path = $libraries_path . DIRECTORY_SEPARATOR . H5PCore::libraryToString($library, TRUE); - H5PCore::deleteFileTree($destination_path); + // Save library folder + $this->h5pC->fs->saveLibrary($library); - // Move library folder - $this->h5pC->copyFileTree($library['uploadDirectory'], $destination_path); + // Remove tmp folder H5PCore::deleteFileTree($library['uploadDirectory']); if ($new) { @@ -1420,26 +1404,10 @@ class H5PStorage { * @param int $contentId * The content id */ - public function deletePackage($contentId) { - H5PCore::deleteFileTree($this->h5pC->path . DIRECTORY_SEPARATOR . 'content' . DIRECTORY_SEPARATOR . $contentId); - $this->h5pF->deleteContentData($contentId); - // TODO: Delete export? - } - - /** - * Update an H5P package - * - * @param int $contentId - * The content id - * @param int $contentMainId - * The content main id (used by frameworks supporting revisioning) - * @return boolean - * TRUE if one or more libraries were updated - * FALSE otherwise - */ - public function updatePackage($contentId, $contentMainId = NULL, $options = array()) { - $this->deletePackage($contentId); - return $this->savePackage($contentId, $contentMainId, FALSE, $options); + public function deletePackage($content) { + $this->h5pC->fs->deleteContent($content['id']); + $this->h5pC->fs->removeExport(($content['slug'] ? $content['slug'] . '-' : '') . $content['id'] . '.h5p'); + $this->h5pF->deleteContentData($content['id']); } /** @@ -1456,10 +1424,7 @@ class H5PStorage { * The main id of the new content (used in frameworks that support revisioning) */ public function copyPackage($contentId, $copyFromId, $contentMainId = NULL) { - $source_path = $this->h5pC->path . DIRECTORY_SEPARATOR . 'content' . DIRECTORY_SEPARATOR . $copyFromId; - $destination_path = $this->h5pC->path . DIRECTORY_SEPARATOR . 'content' . DIRECTORY_SEPARATOR . $contentId; - $this->h5pC->copyFileTree($source_path, $destination_path); - + $this->h5pC->fs->cloneContent($contentId, $newContentId); $this->h5pF->copyLibraryUsage($contentId, $copyFromId, $contentMainId); } } @@ -1493,25 +1458,27 @@ Class H5PExport { * @return string */ public function createExportFile($content) { - $h5pDir = $this->h5pC->path . DIRECTORY_SEPARATOR; - $tempPath = $h5pDir . 'temp' . DIRECTORY_SEPARATOR . $content['id']; - $zipPath = $h5pDir . 'exports' . DIRECTORY_SEPARATOR . $content['slug'] . '-' . $content['id'] . '.h5p'; - // Make sure the exports dir is ready - if (!H5PCore::dirReady($h5pDir . 'exports')) { - $this->h5pF->setErrorMessage($this->h5pF->t('Unable to write to the exports directory.')); + // Get path to temporary folder, where export will be contained + $tmpPath = $this->h5pC->fs->getTmpPath(); + mkdir($tmpPath, 0777, true); + + try { + // Create content folder and populate with files + $this->h5pC->fs->exportContent($content['id'], "{$tmpPath}/content"); + } + catch (Exception $e) { + var_dump('Fail 1'); + $this->h5pF->setErrorMessage($this->h5pF->t($e->getMessage())); + H5PCore::deleteFileTree($tmpPath); return FALSE; } - // Create content folder - if ($this->h5pC->copyFileTree($h5pDir . 'content' . DIRECTORY_SEPARATOR . $content['id'], $tempPath . DIRECTORY_SEPARATOR . 'content') === FALSE) { - return FALSE; - } - file_put_contents($tempPath . DIRECTORY_SEPARATOR . 'content' . DIRECTORY_SEPARATOR . 'content.json', $content['params']); + // Update content.json with content from database + file_put_contents("{$tmpPath}/content/content.json", $content['params']); - - // Make embedTypes into an array - $embedTypes = explode(', ', $content['embedType']); // Won't content always be embedded in one way? + // Make embedType into an array + $embedTypes = explode(', ', $content['embedType']); // Build h5p.json $h5pJson = array ( @@ -1525,16 +1492,23 @@ Class H5PExport { foreach ($content['dependencies'] as $dependency) { $library = $dependency['library']; - // Copy library to h5p - $source = $h5pDir . (isset($library['path']) ? $library['path'] : 'libraries' . DIRECTORY_SEPARATOR . H5PCore::libraryToString($library, TRUE)); - $destination = $tempPath . DIRECTORY_SEPARATOR . $library['machineName']; - $this->h5pC->copyFileTree($source, $destination); + try { + // Export required libraries + $this->h5pC->fs->exportLibrary($library, $tmpPath); + } + catch (Exception $e) { + var_dump('Fail 2'); + $this->h5pF->setErrorMessage($this->h5pF->t($e->getMessage())); + H5PCore::deleteFileTree($tmpPath); + return FALSE; + } // Do not add editor dependencies to h5p json. if ($dependency['type'] === 'editor') { continue; } + // Add to h5p.json dependencies $h5pJson[$dependency['type'] . 'Dependencies'][] = array( 'machineName' => $library['machineName'], 'majorVersion' => $library['majorVersion'], @@ -1544,15 +1518,18 @@ Class H5PExport { // Save h5p.json $results = print_r(json_encode($h5pJson), true); - file_put_contents($tempPath . DIRECTORY_SEPARATOR . 'h5p.json', $results); + file_put_contents("{$tmpPath}/h5p.json", $results); // Get a complete file list from our tmp dir $files = array(); - self::populateFileList($tempPath, $files); + self::populateFileList($tmpPath, $files); + + // Get path to temporary export target file + $tmpFile = $this->h5pC->fs->getTmpPath(); // Create new zip instance. $zip = new ZipArchive(); - $zip->open($zipPath, ZIPARCHIVE::CREATE | ZIPARCHIVE::OVERWRITE); + $zip->open($tmpFile, ZIPARCHIVE::CREATE | ZIPARCHIVE::OVERWRITE); // Add all the files from the tmp dir. foreach ($files as $file) { @@ -1561,9 +1538,19 @@ Class H5PExport { $zip->addFile($file->absolutePath, $file->relativePath); } - // Close zip and remove temp dir + // Close zip and remove tmp dir $zip->close(); - H5PCore::deleteFileTree($tempPath); + H5PCore::deleteFileTree($tmpPath); + + try { + // Save export + $this->h5pC->fs->saveExport($tmpFile, $content['slug'] . '-' . $content['id'] . '.h5p'); + } + catch (Exception $e) { + $this->h5pF->setErrorMessage($this->h5pF->t($e->getMessage())); + } + + unlink($tmpFile); } /** @@ -1601,11 +1588,7 @@ Class H5PExport { * @param array $content object */ public function deleteExport($content) { - $h5pDir = $this->h5pC->path . DIRECTORY_SEPARATOR; - $zipPath = $h5pDir . 'exports' . DIRECTORY_SEPARATOR . ($content['slug'] ? $content['slug'] . '-' : '') . $content['id'] . '.h5p'; - if (file_exists($zipPath)) { - unlink($zipPath); - } + $this->h5pC->fs->removeExport(($content['slug'] ? $content['slug'] . '-' : '') . $content['id'] . '.h5p'); } /** @@ -1656,7 +1639,7 @@ class H5PCore { public static $defaultContentWhitelist = 'json png jpg jpeg gif bmp tif tiff svg eot ttf woff otf webm mp4 ogg mp3 txt pdf rtf doc docx xls xlsx ppt pptx odt ods odp xml csv diff patch swf md textile'; public static $defaultLibraryWhitelistExtras = 'js css'; - public $librariesJsonData, $contentJsonData, $mainJsonData, $h5pF, $path, $development_mode, $h5pD, $disableFileCheck; + public $librariesJsonData, $contentJsonData, $mainJsonData, $h5pF, $fs, $development_mode, $h5pD, $disableFileCheck; const SECONDS_IN_WEEK = 604800; private $exportEnabled; @@ -1682,7 +1665,8 @@ class H5PCore { * * @param object $H5PFramework * The frameworks implementation of the H5PFrameworkInterface - * @param string $path H5P file storage directory. + * @param string|\H5P\FileStorage $path H5P file storage directory or class. + * @param string $url To file storage directory. * @param string $language code. Defaults to english. * @param boolean $export enabled? * @param int $development_mode mode. @@ -1690,8 +1674,8 @@ class H5PCore { public function __construct($H5PFramework, $path, $url, $language = 'en', $export = FALSE, $development_mode = H5PDevelopment::MODE_NONE) { $this->h5pF = $H5PFramework; - $this->h5pF = $H5PFramework; - $this->path = $path; + $this->fs = ($path instanceof \H5P\FileStorage ? $path : new \H5P\DefaultStorage($path)); + $this->url = $url; $this->exportEnabled = $export; $this->development_mode = $development_mode; @@ -1766,7 +1750,7 @@ class H5PCore { */ public function filterParameters(&$content) { if (isset($content['filtered']) && $content['filtered'] !== '') { - return $content['filtered']; + //return $content['filtered']; } // Validate and filter against main library semantics. @@ -1794,10 +1778,7 @@ class H5PCore { $content['slug'] = $this->generateContentSlug($content); // Remove old export file - $oldExport = $this->path . '/exports/' . $content['id'] . '.h5p'; - if (file_exists($oldExport)) { - unlink($oldExport); - } + $this->fs->removeExport($content['id'] . '.h5p'); } if ($this->exportEnabled) { @@ -2097,39 +2078,6 @@ class H5PCore { return rmdir($dir); } - /** - * Recursive function for copying directories. - * - * @param string $source - * Path to the directory we'll be copying - * @return boolean - * Indicates if the directory existed. - */ - public function copyFileTree($source, $destination) { - if (!H5PCore::dirReady($destination)) { - $this->h5pF->setErrorMessage($this->h5pF->t('Unable to copy file tree.')); - return FALSE; - } - - $dir = opendir($source); - if ($dir === FALSE) { - $this->h5pF->setErrorMessage($this->h5pF->t('Unable to copy file tree.')); - return FALSE; - } - - while (false !== ($file = readdir($dir))) { - if (($file != '.') && ($file != '..') && $file != '.git' && $file != '.gitignore') { - if (is_dir($source . DIRECTORY_SEPARATOR . $file)) { - $this->copyFileTree($source . DIRECTORY_SEPARATOR . $file, $destination . DIRECTORY_SEPARATOR . $file); - } - else { - copy($source . DIRECTORY_SEPARATOR . $file,$destination . DIRECTORY_SEPARATOR . $file); - } - } - } - closedir($dir); - } - /** * Writes library data as string on the form {machineName} {majorVersion}.{minorVersion} * @@ -2500,36 +2448,6 @@ class H5PCore { return $input; } - - /** - * Recursive function that makes sure the specified directory exists and - * is writable. - * - * @param string $path - * @return bool - */ - public static function dirReady($path) { - if (!file_exists($path)) { - $parent = preg_replace("/\/[^\/]+\/?$/", '', $path); - if (!H5PCore::dirReady($parent)) { - return FALSE; - } - - mkdir($path, 0777, true); - } - - if (!is_dir($path)) { - trigger_error('Path is not a directory ' . $path, E_USER_WARNING); - return FALSE; - } - - if (!is_writable($path)) { - trigger_error('Unable to write to ' . $path . ' – check directory permissions –', E_USER_WARNING); - return FALSE; - } - - return TRUE; - } } /** From 0125afb440fb3423f9ad641546eb1c0bf14de1a9 Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Tue, 12 Jan 2016 10:31:36 +0100 Subject: [PATCH 02/31] Refactor for consistency --- h5p-default-storage.class.php | 2 +- h5p-file-storage.interface.php | 2 +- h5p.classes.php | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/h5p-default-storage.class.php b/h5p-default-storage.class.php index fb7fb3d..08d3810 100644 --- a/h5p-default-storage.class.php +++ b/h5p-default-storage.class.php @@ -124,7 +124,7 @@ class DefaultStorage implements \H5P\FileStorage { * * @param string $filename */ - public function removeExport($filename) { + public function deleteExport($filename) { // TODO } diff --git a/h5p-file-storage.interface.php b/h5p-file-storage.interface.php index 959ba8f..7056c6e 100644 --- a/h5p-file-storage.interface.php +++ b/h5p-file-storage.interface.php @@ -90,5 +90,5 @@ interface FileStorage { * * @param string $filename */ - public function removeExport($filename); + public function deleteExport($filename); } diff --git a/h5p.classes.php b/h5p.classes.php index e767f48..7654779 100644 --- a/h5p.classes.php +++ b/h5p.classes.php @@ -1406,7 +1406,7 @@ class H5PStorage { */ public function deletePackage($content) { $this->h5pC->fs->deleteContent($content['id']); - $this->h5pC->fs->removeExport(($content['slug'] ? $content['slug'] . '-' : '') . $content['id'] . '.h5p'); + $this->h5pC->fs->deleteExport(($content['slug'] ? $content['slug'] . '-' : '') . $content['id'] . '.h5p'); $this->h5pF->deleteContentData($content['id']); } @@ -1588,7 +1588,7 @@ Class H5PExport { * @param array $content object */ public function deleteExport($content) { - $this->h5pC->fs->removeExport(($content['slug'] ? $content['slug'] . '-' : '') . $content['id'] . '.h5p'); + $this->h5pC->fs->deleteExport(($content['slug'] ? $content['slug'] . '-' : '') . $content['id'] . '.h5p'); } /** @@ -1778,7 +1778,7 @@ class H5PCore { $content['slug'] = $this->generateContentSlug($content); // Remove old export file - $this->fs->removeExport($content['id'] . '.h5p'); + $this->fs->deleteExport($content['id'] . '.h5p'); } if ($this->exportEnabled) { From 808e43bc667d4ba709e5573eda2143a924992db8 Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Tue, 12 Jan 2016 10:31:47 +0100 Subject: [PATCH 03/31] Disable generating export on each request --- h5p.classes.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/h5p.classes.php b/h5p.classes.php index 7654779..aa4b9b5 100644 --- a/h5p.classes.php +++ b/h5p.classes.php @@ -1750,7 +1750,7 @@ class H5PCore { */ public function filterParameters(&$content) { if (isset($content['filtered']) && $content['filtered'] !== '') { - //return $content['filtered']; + return $content['filtered']; } // Validate and filter against main library semantics. From f4ba0af1efd4c73e2dd41f6e463bcffc2244138d Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Tue, 12 Jan 2016 12:55:03 +0100 Subject: [PATCH 04/31] Fixed default file storage implementation --- h5p-default-storage.class.php | 46 +++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/h5p-default-storage.class.php b/h5p-default-storage.class.php index 08d3810..517caf9 100644 --- a/h5p-default-storage.class.php +++ b/h5p-default-storage.class.php @@ -1,25 +1,34 @@ path = $path; - - // TODO: Check if Dirs are ready? Perhaps in each function? } /** @@ -47,7 +56,12 @@ class DefaultStorage implements \H5P\FileStorage { * What makes this content unique. */ public function saveContent($source, $id) { - self::copyFileTree($source, $this->path . '/content/' . $id); + $dest = "{$this->path}/content/{$id}"; + + // Remove any old content + \H5PCore::deleteFileTree($dest); + + self::copyFileTree($source, $dest); } /** @@ -57,7 +71,7 @@ class DefaultStorage implements \H5P\FileStorage { * Content identifier */ public function deleteContent($id) { - // TODO + \H5PCore::deleteFileTree("{$this->path}/content/{$id}"); } /** @@ -80,7 +94,7 @@ class DefaultStorage implements \H5P\FileStorage { * Path */ public function getTmpPath() { - // TODO + return $this->path . '/temp/' . uniqid('h5p-'); } /** @@ -92,7 +106,7 @@ class DefaultStorage implements \H5P\FileStorage { * Where the content folder will be saved */ public function exportContent($id, $target) { - self::copyFileTree($this->path . '/content/' . $id, $target); + self::copyFileTree("{$this->path}/content/{$id}", $target); } /** @@ -104,7 +118,8 @@ class DefaultStorage implements \H5P\FileStorage { * Where the library folder will be saved */ public function exportLibrary($library, $target) { - // TODO + $folder = \H5PCore::libraryToString($library, TRUE); + self::copyFileTree("{$this->path}/libraries/{$folder}", $target); } /** @@ -116,7 +131,9 @@ class DefaultStorage implements \H5P\FileStorage { * Name of export file. */ public function saveExport($source, $filename) { - // TODO + $this->deleteExport($filename); + self::dirReady("{$this->path}/exports"); + copy($source, "{$this->path}/exports/{$filename}"); } /** @@ -125,7 +142,10 @@ class DefaultStorage implements \H5P\FileStorage { * @param string $filename */ public function deleteExport($filename) { - // TODO + $target = "{$this->path}/exports/{$filename}"; + if (file_exists($target)) { + unlink($target); + } } /** @@ -151,11 +171,11 @@ class DefaultStorage implements \H5P\FileStorage { while (false !== ($file = readdir($dir))) { if (($file != '.') && ($file != '..') && $file != '.git' && $file != '.gitignore') { - if (is_dir($source . DIRECTORY_SEPARATOR . $file)) { - self::copyFileTree($source . DIRECTORY_SEPARATOR . $file, $destination . DIRECTORY_SEPARATOR . $file); + if (is_dir("{$source}/{$file}")) { + self::copyFileTree("{$source}/{$file}", "{$destination}/{$file}"); } else { - copy($source . DIRECTORY_SEPARATOR . $file,$destination . DIRECTORY_SEPARATOR . $file); + copy("{$source}/{$file}", "{$destination}/{$file}"); } } } From b4fd0e098cfda78f0de2a6d14a6158d4f42528a6 Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Tue, 12 Jan 2016 18:09:31 +0100 Subject: [PATCH 05/31] Aggregate and cache content assets to improve performance --- h5p.classes.php | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/h5p.classes.php b/h5p.classes.php index aa4b9b5..1b5c52d 100644 --- a/h5p.classes.php +++ b/h5p.classes.php @@ -1909,10 +1909,24 @@ class H5PCore { * @return array files. */ public function getDependenciesFiles($dependencies, $prefix = '') { + $aggregateAssets = TRUE; + + if ($aggregateAssets) { + // Get aggregated files for assets + $key = self::getDependenciesHash($dependencies); + + $files = $this->fs->getCachedAssets($key); + if ($files) { + return $files; // Using cached assets + } + } + + // Build files list for assets $files = array( 'scripts' => array(), 'styles' => array() ); + // Using content dependencies foreach ($dependencies as $dependency) { if (isset($dependency['path']) === FALSE) { $dependency['path'] = 'libraries/' . H5PCore::libraryToString($dependency, TRUE); @@ -1923,9 +1937,34 @@ class H5PCore { $this->getDependencyAssets($dependency, 'preloadedJs', $files['scripts'], $prefix); $this->getDependencyAssets($dependency, 'preloadedCss', $files['styles'], $prefix); } + + if ($aggregateAssets) { + // Aggregate and store assets + $this->fs->cacheAssets($files, $key); + + // TODO: Update cache table + } + return $files; } + private static function getDependenciesHash(&$dependencies) { + // Build hash of dependencies + $toHash = array(); + + // Use unique identifier for each library version + for ($i = 0, $s = count($dependencies); $i < $s; $i++) { + $dep = $dependencies[$i]; + $toHash[] = "{$dep['machineName']}-{$dep['majorVersion']}.{$dep['minorVersion']}.{$dep['patchVersion']}"; + } + + // Sort in case the same dependencies comes in a different order + sort($toHash); + + // Calculate hash sum + return hash('sha1', implode('', $toHash)); + } + /** * Load library semantics. * From 1bf393a9afe7cfc257ebb7e569196ce9955c03a2 Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Wed, 13 Jan 2016 08:45:00 +0100 Subject: [PATCH 06/31] Allow custom URL for libraries --- js/h5p.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/h5p.js b/js/h5p.js index 24d9846..16af37f 100644 --- a/js/h5p.js +++ b/js/h5p.js @@ -1482,7 +1482,7 @@ H5P.libraryFromString = function (library) { * The full path to the library. */ H5P.getLibraryPath = function (library) { - return H5PIntegration.url + '/libraries/' + library; + return (H5PIntegration.libraryUrl !== undefined ? H5PIntegration.libraryUrl + '/' : H5PIntegration.url + '/libraries/') + library; }; /** From 00686b733d9194e4aa2469c4ddce36b016943655 Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Wed, 13 Jan 2016 14:23:34 +0100 Subject: [PATCH 07/31] Added support for aggregating and caching library assets --- h5p-default-storage.class.php | 98 ++++++++++++++++++++++++++++++++++ h5p-file-storage.interface.php | 28 ++++++++++ h5p.classes.php | 40 ++++++++++++-- 3 files changed, 161 insertions(+), 5 deletions(-) diff --git a/h5p-default-storage.class.php b/h5p-default-storage.class.php index 517caf9..d445122 100644 --- a/h5p-default-storage.class.php +++ b/h5p-default-storage.class.php @@ -148,6 +148,104 @@ class DefaultStorage implements \H5P\FileStorage { } } + /** + * Will concatenate all JavaScrips and Stylesheets into two files in order + * to improve page performance. + * + * @param array $files + * A set of all the assets required for content to display + * @param string $key + * Hashed key for cached asset + */ + public function cacheAssets(&$files, $key) { + foreach ($files as $type => $assets) { + $content = ''; + + foreach ($assets as $asset) { + // Get content from asset file + $assetContent = file_get_contents($this->path . $asset->path); + $cssRelPath = preg_replace('/[^\/]+$/', '', $asset->path); + + // Get file content and concatenate + if ($type === 'scripts') { + $content .= $assetContent . ";\n"; + } + else { + // Rewrite relative URLs used inside stylesheets + $content .= preg_replace_callback( + '/url\([\'"]?([^"\')]+)[\'"]?\)/i', + function ($matches) use ($cssRelPath) { + return substr($matches[1], 0, 3) !== '../' ? $matches[0] : 'url("../' . $cssRelPath . $matches[1] . '")'; + }, + $assetContent + ) . "\n"; + } + } + + self::dirReady("{$this->path}/cachedassets"); + $ext = ($type === 'scripts' ? 'js' : 'css'); + file_put_contents("{$this->path}/cachedassets/{$key}.{$ext}", $content); + } + + // Use the newly created cache + $files = self::formatCachedAssets($key); + } + + /** + * Will check if there are cache assets available for content. + * + * @param string $key + * Hashed key for cached asset + * @return array + */ + public function getCachedAssets($key) { + if (!file_exists("{$this->path}/cachedassets/{$key}.js") || + !file_exists("{$this->path}/cachedassets/{$key}.css") { + return NULL; + } + return self::formatCachedAssets($key); + } + + /** + * Remove the aggregated cache files. + * + * @param array $keys + * The hash keys of removed files + */ + public function deleteCachedAssets($keys) { + $context = \context_system::instance(); + $fs = get_file_storage(); + + foreach ($keys as $hash) { + foreach (array('js', 'css') as $ext) { + $path = "{$this->path}/cachedassets/{$key}.{$ext}"; + if (file_exists($path)) { + unlink($path); + } + } + } + } + + /** + * Format the cached assets data the way it's supposed to be. + * + * @param string $key + * Hashed key for cached asset + * @return array + */ + private static function formatCachedAssets($key) { + return array( + 'scripts' => array((object) array( + 'path' => "/cachedassets/{$key}.js", + 'version' => '' + )), + 'styles' => array((object) array( + 'path' => "/cachedassets/{$key}.css", + 'version' => '' + )) + ); + } + /** * Recursive function for copying directories. * diff --git a/h5p-file-storage.interface.php b/h5p-file-storage.interface.php index 7056c6e..b0d4b4b 100644 --- a/h5p-file-storage.interface.php +++ b/h5p-file-storage.interface.php @@ -91,4 +91,32 @@ interface FileStorage { * @param string $filename */ public function deleteExport($filename); + + /** + * Will concatenate all JavaScrips and Stylesheets into two files in order + * to improve page performance. + * + * @param array $files + * A set of all the assets required for content to display + * @param string $key + * Hashed key for cached asset + */ + public function cacheAssets(&$files, $key); + + /** + * Will check if there are cache assets available for content. + * + * @param string $key + * Hashed key for cached asset + * @return array + */ + public function getCachedAssets($key); + + /** + * Remove the aggregated cache files. + * + * @param array $keys + * The hash keys of removed files + */ + public function deleteCachedAssets($keys); } diff --git a/h5p.classes.php b/h5p.classes.php index 1b5c52d..816f116 100644 --- a/h5p.classes.php +++ b/h5p.classes.php @@ -536,6 +536,29 @@ interface H5PFrameworkInterface { * @return boolean */ public function isContentSlugAvailable($slug); + + /** + * Stores hash keys for cached assets, aggregated JavaScripts and + * stylesheets, and connects it to libraries so that we know which cache file + * to delete when a library is updated. + * + * @param string $key + * Hash key for the given libraries + * @param array $libraries + * List of dependencies(libraries) used to create the key + */ + public function saveCachedAssets($key, $libraries); + + /** + * Locate hash keys for given library and delete them. + * Used when cache file are deleted. + * + * @param int $library_id + * Library identifier + * @return array + * List of hash keys removed + */ + public function deleteCachedAssets($library_id); } /** @@ -1345,6 +1368,12 @@ class H5PStorage { // Save library folder $this->h5pC->fs->saveLibrary($library); + // Remove cachedassets that uses this library + if ($this->h5pC->aggregateAssets && isset($library['libraryId'])) { + $removedKeys = $this->h5pF->deleteCachedAssets($library['libraryId']); + $this->h5pC->fs->deleteCachedAssets($removedKeys); + } + // Remove tmp folder H5PCore::deleteFileTree($library['uploadDirectory']); @@ -1680,6 +1709,8 @@ class H5PCore { $this->exportEnabled = $export; $this->development_mode = $development_mode; + $this->aggregateAssets = FALSE; // Off by default.. for now + if ($development_mode & H5PDevelopment::MODE_LIBRARY) { $this->h5pD = new H5PDevelopment($this->h5pF, $path . '/', $language); } @@ -1909,9 +1940,7 @@ class H5PCore { * @return array files. */ public function getDependenciesFiles($dependencies, $prefix = '') { - $aggregateAssets = TRUE; - - if ($aggregateAssets) { + if ($this->aggregateAssets) { // Get aggregated files for assets $key = self::getDependenciesHash($dependencies); @@ -1938,11 +1967,12 @@ class H5PCore { $this->getDependencyAssets($dependency, 'preloadedCss', $files['styles'], $prefix); } - if ($aggregateAssets) { + if ($this->aggregateAssets) { // Aggregate and store assets $this->fs->cacheAssets($files, $key); - // TODO: Update cache table + // Keep track of which libraries have been cached in case they are updated + $this->h5pF->saveCachedAssets($key, $dependencies); } return $files; From 12032461bcff69ac64681c8b536b5f16dcb3634c Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Thu, 14 Jan 2016 09:46:00 +0100 Subject: [PATCH 08/31] Follow existing naming conv. removed namespace --- h5p-default-storage.class.php | 4 +--- h5p-file-storage.interface.php | 4 +--- h5p.classes.php | 4 ++-- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/h5p-default-storage.class.php b/h5p-default-storage.class.php index d445122..945aaac 100644 --- a/h5p-default-storage.class.php +++ b/h5p-default-storage.class.php @@ -4,8 +4,6 @@ * File info? */ -namespace H5P; - /** * The default file storage class for H5P. Will carry out the requested file * operations using PHP's standard file operation functions. @@ -17,7 +15,7 @@ namespace H5P; * @copyright 2016 Joubel AS * @license MIT */ -class DefaultStorage implements \H5P\FileStorage { +class DefaultStorage implements \H5PFileStorage { private $path; /** diff --git a/h5p-file-storage.interface.php b/h5p-file-storage.interface.php index b0d4b4b..dfaa67a 100644 --- a/h5p-file-storage.interface.php +++ b/h5p-file-storage.interface.php @@ -4,12 +4,10 @@ * File info? */ -namespace H5P; - /** * Interface needed to handle storage and export of H5P Content. */ -interface FileStorage { +interface H5PFileStorage { /** * Store the library folder. diff --git a/h5p.classes.php b/h5p.classes.php index d74e0b8..9466e3c 100644 --- a/h5p.classes.php +++ b/h5p.classes.php @@ -1694,7 +1694,7 @@ class H5PCore { * * @param object $H5PFramework * The frameworks implementation of the H5PFrameworkInterface - * @param string|\H5P\FileStorage $path H5P file storage directory or class. + * @param string|\H5PFileStorage $path H5P file storage directory or class. * @param string $url To file storage directory. * @param string $language code. Defaults to english. * @param boolean $export enabled? @@ -1703,7 +1703,7 @@ class H5PCore { public function __construct($H5PFramework, $path, $url, $language = 'en', $export = FALSE, $development_mode = H5PDevelopment::MODE_NONE) { $this->h5pF = $H5PFramework; - $this->fs = ($path instanceof \H5P\FileStorage ? $path : new \H5P\DefaultStorage($path)); + $this->fs = ($path instanceof \H5PFileStorage ? $path : new \H5PDefaultStorage($path)); $this->url = $url; $this->exportEnabled = $export; From dc69181025cdfc056056056292db43da495404e9 Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Thu, 14 Jan 2016 15:39:01 +0100 Subject: [PATCH 09/31] Removed debug --- h5p.classes.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/h5p.classes.php b/h5p.classes.php index 9466e3c..d9cb03b 100644 --- a/h5p.classes.php +++ b/h5p.classes.php @@ -1497,7 +1497,6 @@ Class H5PExport { $this->h5pC->fs->exportContent($content['id'], "{$tmpPath}/content"); } catch (Exception $e) { - var_dump('Fail 1'); $this->h5pF->setErrorMessage($this->h5pF->t($e->getMessage())); H5PCore::deleteFileTree($tmpPath); return FALSE; @@ -1526,7 +1525,6 @@ Class H5PExport { $this->h5pC->fs->exportLibrary($library, $tmpPath); } catch (Exception $e) { - var_dump('Fail 2'); $this->h5pF->setErrorMessage($this->h5pF->t($e->getMessage())); H5PCore::deleteFileTree($tmpPath); return FALSE; From 7c146ce919b35ac1172e9c657488054b2f88f352 Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Fri, 15 Jan 2016 14:11:57 +0100 Subject: [PATCH 10/31] Library list: use all available width --- styles/h5p-admin.css | 1 + 1 file changed, 1 insertion(+) diff --git a/styles/h5p-admin.css b/styles/h5p-admin.css index 0122206..f8b2ab1 100644 --- a/styles/h5p-admin.css +++ b/styles/h5p-admin.css @@ -9,6 +9,7 @@ .h5p-admin-table, .h5p-admin-table > tbody { border: none; + width: 100%; } .h5p-admin-table tr:nth-child(odd), From 604218f2e314f58eb3d5f26dc95d200e17774b57 Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Fri, 15 Jan 2016 14:12:24 +0100 Subject: [PATCH 11/31] Hide buttons if there are no URLs --- js/h5p-library-list.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/js/h5p-library-list.js b/js/h5p-library-list.js index dcb0cc8..1ee1bc8 100644 --- a/js/h5p-library-list.js +++ b/js/h5p-library-list.js @@ -50,11 +50,11 @@ var H5PLibraryList = H5PLibraryList || {}; text: library.numLibraryDependencies, class: 'h5p-admin-center' }, - '
\ - \ - \ - \ -
' + '
' + + '' + + (library.detailsUrl ? '' : '') + + (library.deleteUrl ? '' : '') + + '
' ]); H5PLibraryList.addRestricted($('.h5p-admin-restricted', $libraryRow), library.restrictedUrl, library.restricted); @@ -78,7 +78,12 @@ var H5PLibraryList = H5PLibraryList || {}; }); var $deleteButton = $('.h5p-admin-delete-library', $libraryRow); - if (libraries.notCached !== undefined || hasContent || (library.numContentDependencies !== '' && library.numContentDependencies !== 0) || (library.numLibraryDependencies !== '' && library.numLibraryDependencies !== 0)) { + if (libraries.notCached !== undefined || + hasContent || + (library.numContentDependencies !== '' && + library.numContentDependencies !== 0) || + (library.numLibraryDependencies !== '' && + library.numLibraryDependencies !== 0)) { // Disabled delete if content. $deleteButton.attr('disabled', true); } From 15381d64a42bd436f825449adbb5cc1c11edbbeb Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Thu, 21 Jan 2016 09:56:49 +0100 Subject: [PATCH 12/31] Added a general override rule --- styles/h5p.css | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/styles/h5p.css b/styles/h5p.css index 4586952..923af7c 100644 --- a/styles/h5p.css +++ b/styles/h5p.css @@ -421,3 +421,15 @@ div.h5p-fullscreen { .h5p-dialog-ok-button:active { background: #eeffee; } + +/* Override some theme rules that might be a bit extreme */ +.h5p-content span { + display: initial; + float: initial; + width: initial; + min-height: initial; + margin-left: initial; + -webkit-box-sizing: initial; + -moz-box-sizing: initial; + box-sizing: initial; +} From 8597baa253684a91c945faf82372378902f352ef Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Thu, 21 Jan 2016 10:24:36 +0100 Subject: [PATCH 13/31] Revert "Added a general override rule" This reverts commit 15381d64a42bd436f825449adbb5cc1c11edbbeb. Will rename the class instead. --- styles/h5p.css | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/styles/h5p.css b/styles/h5p.css index 923af7c..4586952 100644 --- a/styles/h5p.css +++ b/styles/h5p.css @@ -421,15 +421,3 @@ div.h5p-fullscreen { .h5p-dialog-ok-button:active { background: #eeffee; } - -/* Override some theme rules that might be a bit extreme */ -.h5p-content span { - display: initial; - float: initial; - width: initial; - min-height: initial; - margin-left: initial; - -webkit-box-sizing: initial; - -moz-box-sizing: initial; - box-sizing: initial; -} From 5eb0e0e14c728a924d93cd420626b6126da97bc9 Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Mon, 25 Jan 2016 14:44:10 +0100 Subject: [PATCH 14/31] Avoid caching dependencies without assets --- h5p.classes.php | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/h5p.classes.php b/h5p.classes.php index cd7a64d..e0e0ff4 100644 --- a/h5p.classes.php +++ b/h5p.classes.php @@ -1939,6 +1939,17 @@ class H5PCore { * @return array files. */ public function getDependenciesFiles($dependencies, $prefix = '') { + // Build files list for assets + $files = array( + 'scripts' => array(), + 'styles' => array() + ); + + // Avoid caching empty files + if (empty($dependencies)) { + return $files; + } + if ($this->aggregateAssets) { // Get aggregated files for assets $key = self::getDependenciesHash($dependencies); @@ -1949,11 +1960,6 @@ class H5PCore { } } - // Build files list for assets - $files = array( - 'scripts' => array(), - 'styles' => array() - ); // Using content dependencies foreach ($dependencies as $dependency) { if (isset($dependency['path']) === FALSE) { From fb9a84e665955d42f5ef682c69828454639aecf6 Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Mon, 25 Jan 2016 14:44:40 +0100 Subject: [PATCH 15/31] Dependencies may be indexed differently --- h5p.classes.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/h5p.classes.php b/h5p.classes.php index e0e0ff4..9fcca19 100644 --- a/h5p.classes.php +++ b/h5p.classes.php @@ -1988,8 +1988,7 @@ class H5PCore { $toHash = array(); // Use unique identifier for each library version - for ($i = 0, $s = count($dependencies); $i < $s; $i++) { - $dep = $dependencies[$i]; + foreach ($dependencies as $dep) { $toHash[] = "{$dep['machineName']}-{$dep['majorVersion']}.{$dep['minorVersion']}.{$dep['patchVersion']}"; } From 0f736ae1c5a49f04e619b60c73f3e863968c7d62 Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Mon, 25 Jan 2016 14:46:06 +0100 Subject: [PATCH 16/31] Fixed issue forgotten during refactoring --- h5p-default-storage.class.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/h5p-default-storage.class.php b/h5p-default-storage.class.php index 945aaac..1d8b0a8 100644 --- a/h5p-default-storage.class.php +++ b/h5p-default-storage.class.php @@ -15,7 +15,7 @@ * @copyright 2016 Joubel AS * @license MIT */ -class DefaultStorage implements \H5PFileStorage { +class H5PDefaultStorage implements \H5PFileStorage { private $path; /** From 875a7ccef32421db993072acecbdb21c0f3ba630 Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Mon, 25 Jan 2016 14:46:55 +0100 Subject: [PATCH 17/31] Target correct folder when exporting libs --- h5p-default-storage.class.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/h5p-default-storage.class.php b/h5p-default-storage.class.php index 1d8b0a8..849d7b7 100644 --- a/h5p-default-storage.class.php +++ b/h5p-default-storage.class.php @@ -117,7 +117,7 @@ class H5PDefaultStorage implements \H5PFileStorage { */ public function exportLibrary($library, $target) { $folder = \H5PCore::libraryToString($library, TRUE); - self::copyFileTree("{$this->path}/libraries/{$folder}", $target); + self::copyFileTree("{$this->path}/libraries/{$folder}", "{$target}/{$folder}"); } /** From 64e20011568c0d31207428467d439c19b4fabcd2 Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Mon, 25 Jan 2016 14:47:48 +0100 Subject: [PATCH 18/31] Do not cache assets without files --- h5p-default-storage.class.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/h5p-default-storage.class.php b/h5p-default-storage.class.php index 849d7b7..8f0e9d5 100644 --- a/h5p-default-storage.class.php +++ b/h5p-default-storage.class.php @@ -157,8 +157,11 @@ class H5PDefaultStorage implements \H5PFileStorage { */ public function cacheAssets(&$files, $key) { foreach ($files as $type => $assets) { - $content = ''; + if (empty($assets)) { + continue; // Skip no assets + } + $content = ''; foreach ($assets as $asset) { // Get content from asset file $assetContent = file_get_contents($this->path . $asset->path); From 73fd170a3a54e7a2ac9f40bf3f184dadf7b52387 Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Mon, 25 Jan 2016 14:48:14 +0100 Subject: [PATCH 19/31] Editor may use dirReady for the time being --- h5p-default-storage.class.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/h5p-default-storage.class.php b/h5p-default-storage.class.php index 8f0e9d5..2821b40 100644 --- a/h5p-default-storage.class.php +++ b/h5p-default-storage.class.php @@ -285,10 +285,13 @@ class H5PDefaultStorage implements \H5PFileStorage { * Recursive function that makes sure the specified directory exists and * is writable. * + * TODO: Will be made private when the editor file handling is done by this + * class! + * * @param string $path * @return bool */ - private static function dirReady($path) { + public static function dirReady($path) { if (!file_exists($path)) { $parent = preg_replace("/\/[^\/]+\/?$/", '', $path); if (!self::dirReady($parent)) { From 3defc7abde24120d344b52485375cb650096daca Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Mon, 25 Jan 2016 14:49:29 +0100 Subject: [PATCH 20/31] Fixed CSS url caching bug --- h5p-default-storage.class.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/h5p-default-storage.class.php b/h5p-default-storage.class.php index 2821b40..bc6f83e 100644 --- a/h5p-default-storage.class.php +++ b/h5p-default-storage.class.php @@ -176,10 +176,12 @@ class H5PDefaultStorage implements \H5PFileStorage { $content .= preg_replace_callback( '/url\([\'"]?([^"\')]+)[\'"]?\)/i', function ($matches) use ($cssRelPath) { - return substr($matches[1], 0, 3) !== '../' ? $matches[0] : 'url("../' . $cssRelPath . $matches[1] . '")'; + if (preg_match("/^([a-z0-9]+:)?\//i", $matches[1]) === 1) { + return $matches[0]; // Not relative, skip + } + return 'url("../' . $cssRelPath . $matches[1] . '")'; }, - $assetContent - ) . "\n"; + $assetContent) . "\n"; } } From 36ca38705f039cfa14c7cb3e35626cdc6f56b5ed Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Mon, 25 Jan 2016 14:49:54 +0100 Subject: [PATCH 21/31] Allow assets of only one type --- h5p-default-storage.class.php | 61 ++++++++++++++++------------------- 1 file changed, 28 insertions(+), 33 deletions(-) diff --git a/h5p-default-storage.class.php b/h5p-default-storage.class.php index bc6f83e..bcd7b1a 100644 --- a/h5p-default-storage.class.php +++ b/h5p-default-storage.class.php @@ -92,7 +92,9 @@ class H5PDefaultStorage implements \H5PFileStorage { * Path */ public function getTmpPath() { - return $this->path . '/temp/' . uniqid('h5p-'); + $temp = "{$this->path}/temp"; + self::dirReady($temp); + return "{$temp}/" . uniqid('h5p-'); } /** @@ -187,11 +189,13 @@ class H5PDefaultStorage implements \H5PFileStorage { self::dirReady("{$this->path}/cachedassets"); $ext = ($type === 'scripts' ? 'js' : 'css'); - file_put_contents("{$this->path}/cachedassets/{$key}.{$ext}", $content); + $outputfile = "/cachedassets/{$key}.{$ext}"; + file_put_contents($this->path . $outputfile, $content); + $files[$type] = array((object) array( + 'path' => $outputfile, + 'version' => '' + )); } - - // Use the newly created cache - $files = self::formatCachedAssets($key); } /** @@ -202,11 +206,25 @@ class H5PDefaultStorage implements \H5PFileStorage { * @return array */ public function getCachedAssets($key) { - if (!file_exists("{$this->path}/cachedassets/{$key}.js") || - !file_exists("{$this->path}/cachedassets/{$key}.css") { - return NULL; + $files = array(); + + $js = "/cachedassets/{$key}.js"; + if (file_exists($this->path . $js)) { + $files['scripts'] = array((object) array( + 'path' => $js, + 'version' => '' + )); } - return self::formatCachedAssets($key); + + $css = "/cachedassets/{$key}.css"; + if (file_exists($this->path . $css)) { + $files['styles'] = array((object) array( + 'path' => $css, + 'version' => '' + )); + } + + return empty($files) ? NULL : $files; } /** @@ -216,12 +234,9 @@ class H5PDefaultStorage implements \H5PFileStorage { * The hash keys of removed files */ public function deleteCachedAssets($keys) { - $context = \context_system::instance(); - $fs = get_file_storage(); - foreach ($keys as $hash) { foreach (array('js', 'css') as $ext) { - $path = "{$this->path}/cachedassets/{$key}.{$ext}"; + $path = "{$this->path}/cachedassets/{$hash}.{$ext}"; if (file_exists($path)) { unlink($path); } @@ -229,26 +244,6 @@ class H5PDefaultStorage implements \H5PFileStorage { } } - /** - * Format the cached assets data the way it's supposed to be. - * - * @param string $key - * Hashed key for cached asset - * @return array - */ - private static function formatCachedAssets($key) { - return array( - 'scripts' => array((object) array( - 'path' => "/cachedassets/{$key}.js", - 'version' => '' - )), - 'styles' => array((object) array( - 'path' => "/cachedassets/{$key}.css", - 'version' => '' - )) - ); - } - /** * Recursive function for copying directories. * From 633186c0d5f065d915a59f12dbd7fdd6d6473dac Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Tue, 26 Jan 2016 15:57:10 +0100 Subject: [PATCH 22/31] Support for data urls in css aggr. --- h5p-default-storage.class.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/h5p-default-storage.class.php b/h5p-default-storage.class.php index bcd7b1a..27b250b 100644 --- a/h5p-default-storage.class.php +++ b/h5p-default-storage.class.php @@ -178,7 +178,7 @@ class H5PDefaultStorage implements \H5PFileStorage { $content .= preg_replace_callback( '/url\([\'"]?([^"\')]+)[\'"]?\)/i', function ($matches) use ($cssRelPath) { - if (preg_match("/^([a-z0-9]+:)?\//i", $matches[1]) === 1) { + if (preg_match("/^(data:|([a-z0-9]+:)?\/)/i", $matches[1]) === 1) { return $matches[0]; // Not relative, skip } return 'url("../' . $cssRelPath . $matches[1] . '")'; From 3991875439a0ca049c5361968fbf644d769a223a Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Wed, 27 Jan 2016 12:42:55 +0100 Subject: [PATCH 23/31] Avoid navigating inside iframe HFJ-1479 --- js/h5p.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/js/h5p.js b/js/h5p.js index fe45aaf..b51842f 100644 --- a/js/h5p.js +++ b/js/h5p.js @@ -375,7 +375,8 @@ H5P.getHeadTags = function (contentId) { return tags; }; - return createStyleTags(H5PIntegration.core.styles) + + return '' + + createStyleTags(H5PIntegration.core.styles) + createStyleTags(H5PIntegration.contents['cid-' + contentId].styles) + createScriptTags(H5PIntegration.core.scripts) + createScriptTags(H5PIntegration.contents['cid-' + contentId].scripts) + From 3849f3a0544e83b3da8b5f232d119179951e15be Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Fri, 19 Feb 2016 12:57:53 +0100 Subject: [PATCH 24/31] Improved results and contentuserdata saving by queueing requests --- js/h5p.js | 161 +++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 129 insertions(+), 32 deletions(-) diff --git a/js/h5p.js b/js/h5p.js index 934d8f6..0c17a8e 100644 --- a/js/h5p.js +++ b/js/h5p.js @@ -1580,46 +1580,97 @@ H5P.shuffleArray = function (array) { return array; }; -/** - * Post finished results for user. - * - * @deprecated - * Do not use this function directly, trigger the finish event instead. - * Will be removed march 2016 - * @param {number} contentId - * Identifies the content - * @param {number} score - * Achieved score/points - * @param {number} maxScore - * The maximum score/points that can be achieved - * @param {number} [time] - * Reported time consumption/usage - */ -H5P.setFinished = function (contentId, score, maxScore, time) { - if (H5PIntegration.postUserStatistics === true) { - /** - * Return unix timestamp for the given JS Date. - * - * @private - * @param {Date} date - * @returns {Number} - */ - var toUnix = function (date) { - return Math.round(date.getTime() / 1000); - }; +(function ($) { + var token; + var queue = []; + + /** + * Return unix timestamp for the given JS Date. + * + * @private + * @param {Date} date + * @returns {Number} + */ + var toUnix = function (date) { + return Math.round(date.getTime() / 1000); + }; + + /** + * Post results to server + * + * @param Object result + */ + var post = function (result) { + // Add token to post + result.token = token; + token = null; // Post the results // TODO: Should we use a variable with the complete path? - H5P.jQuery.post(H5PIntegration.ajaxPath + 'setFinished', { + H5P.jQuery.post(H5PIntegration.ajaxPath + 'setFinished', result).fail(function() { + // Reuse token + token = result.token; + }).done(function(data) { + token = data.token; + }).always(function() { + // Check for more requests to run + if (queue[0] !== undefined) { + post(queue.splice(0, 1)); + } + }); + }; + + /** + * Creates the finished result object for the user and schedules it for + * posting to the server. + * + * @deprecated + * Do not use this function directly, trigger the finish event instead. + * Will be removed march 2016 + * @param {number} contentId + * Identifies the content + * @param {number} score + * Achieved score/points + * @param {number} maxScore + * The maximum score/points that can be achieved + * @param {number} [time] + * Reported time consumption/usage + */ + H5P.setFinished = function (contentId, score, maxScore, time) { + if (H5PIntegration.postUserStatistics !== true) { + return; + } + + var result = { contentId: contentId, score: score, maxScore: maxScore, opened: toUnix(H5P.opened[contentId]), finished: toUnix(new Date()), time: time - }); - } -}; + }; + + if (token === undefined) { + if (H5PIntegration.tokens === undefined || + H5PIntegration.tokens.result === undefined) { + token = 'canHasDummyToken'; + } + else { + token = H5PIntegration.tokens.result; + } + } + + if (token === null) { + // Already in progress, add to queue + queue.push(result); + } + else { + post(result); + } + }; + +})(H5P.jQuery); + // Add indexOf to browsers that lack them. (IEs) if (!Array.prototype.indexOf) { @@ -1728,6 +1779,8 @@ H5P.createTitle = function (rawTitle, maxLength) { // Wrap in privates (function ($) { + var token; + var queue = []; /** * Creates ajax requests for inserting, updateing and deleteing @@ -1749,6 +1802,16 @@ H5P.createTitle = function (rawTitle, maxLength) { done('Not signed in.'); return; } + if (token === undefined) { + // Load initial token + if (H5PIntegration.tokens === undefined || + H5PIntegration.tokens.contentUserData === undefined) { + token = 'canHasDummyToken'; + } + else { + token = H5PIntegration.tokens.contentUserData; + } + } var options = { url: H5PIntegration.ajax.contentUserData.replace(':contentId', contentId).replace(':dataType', dataType).replace(':subContentId', subContentId ? subContentId : 0), @@ -1784,10 +1847,44 @@ H5P.createTitle = function (rawTitle, maxLength) { done(undefined, response.data); }; } + if (options.type === 'POST') { + if (token === null) { + // We must queue and wait for a new token + queue.push(options); + } + else { + // Use token + options.data.token = token; + token = null; + } + } - $.ajax(options); + runAjaxRequest(options); } + /** + * + * + * @param Object options Details for request + */ + var runAjaxRequest = function (options) { + var $req = $.ajax(options); + if (options.type === 'POST') { + $req.fail(function() { + // Reuse token + token = options.data.token; + }).done(function(data) { + // Set new token + token = data.token; + }).always(function() { + // Check for more requests to run + if (queue[0] !== undefined) { + runAjaxRequest(queue.splice(0, 1)); + } + }); + } + }; + /** * Get user data for given content. * From 256bf1b6d29a0886e30d6304002d8e6f33bb334c Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Mon, 22 Feb 2016 11:56:52 +0100 Subject: [PATCH 25/31] Revert "Improved results and contentuserdata saving by queueing requests" This reverts commit 3849f3a0544e83b3da8b5f232d119179951e15be. --- js/h5p.js | 161 +++++++++++------------------------------------------- 1 file changed, 32 insertions(+), 129 deletions(-) diff --git a/js/h5p.js b/js/h5p.js index 0c17a8e..934d8f6 100644 --- a/js/h5p.js +++ b/js/h5p.js @@ -1580,97 +1580,46 @@ H5P.shuffleArray = function (array) { return array; }; -(function ($) { - var token; - var queue = []; - - /** - * Return unix timestamp for the given JS Date. - * - * @private - * @param {Date} date - * @returns {Number} - */ - var toUnix = function (date) { - return Math.round(date.getTime() / 1000); - }; - - /** - * Post results to server - * - * @param Object result - */ - var post = function (result) { - // Add token to post - result.token = token; - token = null; +/** + * Post finished results for user. + * + * @deprecated + * Do not use this function directly, trigger the finish event instead. + * Will be removed march 2016 + * @param {number} contentId + * Identifies the content + * @param {number} score + * Achieved score/points + * @param {number} maxScore + * The maximum score/points that can be achieved + * @param {number} [time] + * Reported time consumption/usage + */ +H5P.setFinished = function (contentId, score, maxScore, time) { + if (H5PIntegration.postUserStatistics === true) { + /** + * Return unix timestamp for the given JS Date. + * + * @private + * @param {Date} date + * @returns {Number} + */ + var toUnix = function (date) { + return Math.round(date.getTime() / 1000); + }; // Post the results // TODO: Should we use a variable with the complete path? - H5P.jQuery.post(H5PIntegration.ajaxPath + 'setFinished', result).fail(function() { - // Reuse token - token = result.token; - }).done(function(data) { - token = data.token; - }).always(function() { - // Check for more requests to run - if (queue[0] !== undefined) { - post(queue.splice(0, 1)); - } - }); - }; - - /** - * Creates the finished result object for the user and schedules it for - * posting to the server. - * - * @deprecated - * Do not use this function directly, trigger the finish event instead. - * Will be removed march 2016 - * @param {number} contentId - * Identifies the content - * @param {number} score - * Achieved score/points - * @param {number} maxScore - * The maximum score/points that can be achieved - * @param {number} [time] - * Reported time consumption/usage - */ - H5P.setFinished = function (contentId, score, maxScore, time) { - if (H5PIntegration.postUserStatistics !== true) { - return; - } - - var result = { + H5P.jQuery.post(H5PIntegration.ajaxPath + 'setFinished', { contentId: contentId, score: score, maxScore: maxScore, opened: toUnix(H5P.opened[contentId]), finished: toUnix(new Date()), time: time - }; - - if (token === undefined) { - if (H5PIntegration.tokens === undefined || - H5PIntegration.tokens.result === undefined) { - token = 'canHasDummyToken'; - } - else { - token = H5PIntegration.tokens.result; - } - } - - if (token === null) { - // Already in progress, add to queue - queue.push(result); - } - else { - post(result); - } - }; - -})(H5P.jQuery); - + }); + } +}; // Add indexOf to browsers that lack them. (IEs) if (!Array.prototype.indexOf) { @@ -1779,8 +1728,6 @@ H5P.createTitle = function (rawTitle, maxLength) { // Wrap in privates (function ($) { - var token; - var queue = []; /** * Creates ajax requests for inserting, updateing and deleteing @@ -1802,16 +1749,6 @@ H5P.createTitle = function (rawTitle, maxLength) { done('Not signed in.'); return; } - if (token === undefined) { - // Load initial token - if (H5PIntegration.tokens === undefined || - H5PIntegration.tokens.contentUserData === undefined) { - token = 'canHasDummyToken'; - } - else { - token = H5PIntegration.tokens.contentUserData; - } - } var options = { url: H5PIntegration.ajax.contentUserData.replace(':contentId', contentId).replace(':dataType', dataType).replace(':subContentId', subContentId ? subContentId : 0), @@ -1847,44 +1784,10 @@ H5P.createTitle = function (rawTitle, maxLength) { done(undefined, response.data); }; } - if (options.type === 'POST') { - if (token === null) { - // We must queue and wait for a new token - queue.push(options); - } - else { - // Use token - options.data.token = token; - token = null; - } - } - runAjaxRequest(options); + $.ajax(options); } - /** - * - * - * @param Object options Details for request - */ - var runAjaxRequest = function (options) { - var $req = $.ajax(options); - if (options.type === 'POST') { - $req.fail(function() { - // Reuse token - token = options.data.token; - }).done(function(data) { - // Set new token - token = data.token; - }).always(function() { - // Check for more requests to run - if (queue[0] !== undefined) { - runAjaxRequest(queue.splice(0, 1)); - } - }); - } - }; - /** * Get user data for given content. * From 0aebdb345bde830bb10851ddda5f13b3d47194fe Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Mon, 22 Feb 2016 11:58:25 +0100 Subject: [PATCH 26/31] Lets use a simpler solution --- js/h5p.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/js/h5p.js b/js/h5p.js index 934d8f6..2b0a862 100644 --- a/js/h5p.js +++ b/js/h5p.js @@ -1616,7 +1616,8 @@ H5P.setFinished = function (contentId, score, maxScore, time) { maxScore: maxScore, opened: toUnix(H5P.opened[contentId]), finished: toUnix(new Date()), - time: time + time: time, + token: H5PIntegration.tokens.result }); } }; @@ -1760,7 +1761,8 @@ H5P.createTitle = function (rawTitle, maxLength) { options.data = { data: (data === null ? 0 : data), preload: (preload ? 1 : 0), - invalidate: (invalidate ? 1 : 0) + invalidate: (invalidate ? 1 : 0), + token: H5PIntegration.tokens.contentUserData }; } else { From 4e06733ffbd0c31b5b861e00f9d79431299c2960 Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Mon, 22 Feb 2016 12:01:18 +0100 Subject: [PATCH 27/31] Use correct message --- js/h5p.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/h5p.js b/js/h5p.js index 2b0a862..dd36a7a 100644 --- a/js/h5p.js +++ b/js/h5p.js @@ -1774,7 +1774,7 @@ H5P.createTitle = function (rawTitle, maxLength) { }; options.success = function (response) { if (!response.success) { - done(response.error); + done(response.message); return; } From 0430e6ba28a33365f465afd6ee5849bb0f930703 Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Thu, 25 Feb 2016 13:46:05 +0100 Subject: [PATCH 28/31] Put AJAX response functions in core --- h5p.classes.php | 85 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/h5p.classes.php b/h5p.classes.php index 3ccfd5f..af78c0e 100644 --- a/h5p.classes.php +++ b/h5p.classes.php @@ -2535,6 +2535,91 @@ class H5PCore { return TRUE; } + + /** + * Makes it easier to print response when AJAX request succeeds. + * + * @param mixed $data + * @since 1.6.0 + */ + public static function ajaxSuccess($data = NULL) { + $response = array( + 'success' => TRUE + ); + if ($message !== NULL) { + $response['data'] = $data; + } + self::printJson($response); + } + + /** + * Makes it easier to print response when AJAX request fails. + * Will exit after printing error. + * + * @param string $message + * @since 1.6.0 + */ + public static function ajaxError($message = NULL) { + $response = array( + 'success' => FALSE + ); + if ($message !== NULL) { + $response['message'] = $message; + } + self::printJson($response); + } + + /** + * Print JSON headers with UTF-8 charset and json encode response data. + * Makes it easier to respond using JSON. + * + * @param mixed $data + */ + private static function printJson($data) { + header('Cache-Control: no-cache'); + header('Content-type: application/json; charset=utf-8'); + print json_encode($data); + } + + /** + * Get a new H5P security token for the given action. + * + * @param string $action + * @return string token + */ + public static function createToken($action) { + if (!isset($_SESSION['h5p_token'])) { + // Create an unique key which is used to create action tokens for this session. + $_SESSION['h5p_token'] = uniqid(); + } + + // Timefactor + $time_factor = self::getTimeFactor(); + + // Create and return token + return substr(hash('md5', $action . $time_factor . $_SESSION['h5p_token']), -16, 13); + } + + /** + * Create a time based number which is unique for each 12 hour. + * @return int + */ + private static function getTimeFactor() { + return ceil(time() / (86400 / 2)); + } + + /** + * Verify if the given token is valid for the given action. + * + * @param string $action + * @param string $token + * @return boolean valid token + */ + public static function validToken($action, $token) { + $time_factor = self::getTimeFactor(); + return $token === substr(hash('md5', $action . $time_factor . $_SESSION['h5p_token']), -16, 13) || // Under 12 hours + $token === substr(hash('md5', $action . ($time_factor - 1) . $_SESSION['h5p_token']), -16, 13); // Between 12-24 hours + } } /** From 437598aa8e481a7f14cc1536d5fb4faa4324cc7f Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Mon, 7 Mar 2016 10:58:39 +0100 Subject: [PATCH 29/31] Use key to avoid bugs --- h5p.classes.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/h5p.classes.php b/h5p.classes.php index fc8980c..00f5700 100644 --- a/h5p.classes.php +++ b/h5p.classes.php @@ -2437,10 +2437,9 @@ class H5PCore { $libs = $this->h5pF->loadLibraries(); - foreach($libs as $library) { + foreach($libs as $libName => $library) { foreach($library as $libVersion) { - - $librariesInstalled[] = $libVersion->name.' '.$libVersion->major_version.'.'.$libVersion->minor_version.'.'.$libVersion->patch_version; + $librariesInstalled[] = $libName.' '.$libVersion->major_version.'.'.$libVersion->minor_version.'.'.$libVersion->patch_version; } } From de175737ea53a142b8952f1182f26ba33d056cdb Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Thu, 10 Mar 2016 16:33:53 +0100 Subject: [PATCH 30/31] Use correct variable --- h5p.classes.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/h5p.classes.php b/h5p.classes.php index af78c0e..4b2b3a4 100644 --- a/h5p.classes.php +++ b/h5p.classes.php @@ -2546,7 +2546,7 @@ class H5PCore { $response = array( 'success' => TRUE ); - if ($message !== NULL) { + if ($data !== NULL) { $response['data'] = $data; } self::printJson($response); From 14605dc7900f71801720972608b3a3d431ffedce Mon Sep 17 00:00:00 2001 From: Frode Petterson Date: Thu, 17 Mar 2016 14:22:23 +0100 Subject: [PATCH 31/31] A more natural tabindex HFJ-1549 --- js/h5p.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/js/h5p.js b/js/h5p.js index 4fa0e19..df0d9e6 100644 --- a/js/h5p.js +++ b/js/h5p.js @@ -139,7 +139,7 @@ H5P.init = function (target) { // Check if we should add and display a fullscreen button for this H5P. if (contentData.fullScreen == 1 && H5P.canHasFullScreen) { - H5P.jQuery('
').prependTo($container).children().click(function () { + H5P.jQuery('
').prependTo($container).children().click(function () { H5P.fullScreen($container, instance); }); } @@ -148,7 +148,7 @@ H5P.init = function (target) { var $actions = H5P.jQuery('
    '); if (!(contentData.disable & H5P.DISABLE_DOWNLOAD)) { // Add export button - H5P.jQuery('
  • ' + H5P.t('download') + '
  • ').appendTo($actions).click(function () { + H5P.jQuery('
  • ' + H5P.t('download') + '
  • ').appendTo($actions).click(function () { window.location.href = contentData.exportUrl; }); } @@ -157,7 +157,7 @@ H5P.init = function (target) { if (copyright) { // Add copyright dialog button - H5P.jQuery('
  • ' + H5P.t('copyrights') + '
  • ').appendTo($actions).click(function () { + H5P.jQuery('
  • ' + H5P.t('copyrights') + '
  • ').appendTo($actions).click(function () { // Open dialog with copyright information var dialog = new H5P.Dialog('copyrights', H5P.t('copyrightInformation'), copyright, $container); dialog.open(); @@ -166,7 +166,7 @@ H5P.init = function (target) { } if (!(contentData.disable & H5P.DISABLE_EMBED)) { // Add embed button - H5P.jQuery('
  • ' + H5P.t('embed') + '
  • ').appendTo($actions).click(function () { + H5P.jQuery('
  • ' + H5P.t('embed') + '
  • ').appendTo($actions).click(function () { H5P.openEmbedDialog($actions, contentData.embedCode, contentData.resizeCode, { width: $element.width(), height: $element.height() @@ -545,7 +545,7 @@ H5P.fullScreen = function ($element, instance, exitCallback, body) { } before('h5p-semi-fullscreen'); - var $disable = H5P.jQuery('
    ').appendTo($container.find('.h5p-content-controls')); + var $disable = H5P.jQuery('
    ').appendTo($container.find('.h5p-content-controls')); var keyup, disableSemiFullscreen = H5P.exitFullScreen = function () { if (prevViewportContent) { // Use content from the previous viewport tag @@ -887,7 +887,7 @@ H5P.Dialog = function (name, title, content, $element) {
    \

    ' + title + '

    \
    ' + content + '
    \ -
    \ +
    \
    \
    ') .insertAfter($element)