Merge pull request #7920 from rtibbles/zip_content_cleanup

learningequality · Mar 25, 2021 · 6e47d67 · 6e47d67
2 parents 893bb57 + c045ca8
commit 6e47d67
Show file tree

Hide file tree

Showing 21 changed files with 190 additions and 240 deletions.
diff --git a/...atic/assets/h5p-standalone-dist/README.md → .../core/content/static/assets/h5p/README.md b/...atic/assets/h5p-standalone-dist/README.md → .../core/content/static/assets/h5p/README.md
diff --git a/...h5p-standalone-dist/fonts/h5p-core-23.eot → ...t/static/assets/h5p/fonts/h5p-core-23.eot b/...h5p-standalone-dist/fonts/h5p-core-23.eot → ...t/static/assets/h5p/fonts/h5p-core-23.eot
diff --git a/...h5p-standalone-dist/fonts/h5p-core-23.svg → ...t/static/assets/h5p/fonts/h5p-core-23.svg b/...h5p-standalone-dist/fonts/h5p-core-23.svg → ...t/static/assets/h5p/fonts/h5p-core-23.svg
diff --git a/...5p-standalone-dist/fonts/h5p-core-23.woff → .../static/assets/h5p/fonts/h5p-core-23.woff b/...5p-standalone-dist/fonts/h5p-core-23.woff → .../static/assets/h5p/fonts/h5p-core-23.woff
diff --git a/kolibri/core/content/static/assets/h5p/images/throbber.gif b/kolibri/core/content/static/assets/h5p/images/throbber.gif
diff --git a/...ts/h5p-standalone-dist/js/frame.bundle.js → ...tent/static/assets/h5p/js/frame.bundle.js b/...ts/h5p-standalone-dist/js/frame.bundle.js → ...tent/static/assets/h5p/js/frame.bundle.js
diff --git a/.../h5p-standalone-dist/styles/h5p-admin.css → ...nt/static/assets/h5p/styles/h5p-admin.css b/.../h5p-standalone-dist/styles/h5p-admin.css → ...nt/static/assets/h5p/styles/h5p-admin.css
diff --git a/...e-dist/styles/h5p-confirmation-dialog.css → ...ts/h5p/styles/h5p-confirmation-dialog.css b/...e-dist/styles/h5p-confirmation-dialog.css → ...ts/h5p/styles/h5p-confirmation-dialog.css
diff --git a/...tandalone-dist/styles/h5p-core-button.css → ...tic/assets/h5p/styles/h5p-core-button.css b/...tandalone-dist/styles/h5p-core-button.css → ...tic/assets/h5p/styles/h5p-core-button.css
diff --git a/...assets/h5p-standalone-dist/styles/h5p.css → .../content/static/assets/h5p/styles/h5p.css b/...assets/h5p-standalone-dist/styles/h5p.css → .../content/static/assets/h5p/styles/h5p.css
diff --git a/kolibri/core/content/templates/content/h5p.html b/kolibri/core/content/templates/content/h5p.html
@@ -1,3 +1,4 @@
+{% load staticfiles %}
 <!DOCTYPE html>
 
 <html>
@@ -13,9 +14,9 @@
             }
         </style>
 
-        <link type="text/css" rel="stylesheet" media="all" href="dist/styles/h5p.css" />
-        <link type="text/css" rel="stylesheet" media="all" href="dist/styles/h5p-confirmation-dialog.css" />
-        <link type="text/css" rel="stylesheet" media="all" href="dist/styles/h5p-core-button.css" />
+        <link type="text/css" rel="stylesheet" media="all" href="{% static 'assets/h5p/styles/h5p.css' %}" />
+        <link type="text/css" rel="stylesheet" media="all" href="{% static 'assets/h5p/styles/h5p-confirmation-dialog.css' %}" />
+        <link type="text/css" rel="stylesheet" media="all" href="{% static 'assets/h5p/styles/h5p-core-button.css' %}" />
 
         {% for path in cssfiles %}
         <link type="text/css" rel="stylesheet" media="all" href="{{ path }}" />
@@ -79,7 +80,7 @@
 
         </script>
 
-        <script type="text/javascript" src="dist/js/frame.bundle.js" crossorigin="anonymous"></script>
+        <script type="text/javascript" src="{% static 'assets/h5p/js/frame.bundle.js' %}" crossorigin="anonymous"></script>
 
         {% for path in jsfiles %}
         <script type="text/javascript" src="{{ path }}"></script>

diff --git a/kolibri/core/content/templates/content/hashi.html b/kolibri/core/content/templates/content/hashi.html
@@ -0,0 +1,13 @@
+{% load staticfiles %}
+<!DOCTYPE html>
+<html>
+    <head>
+        <meta charset="utf-8">
+        <meta http-equiv="X-UA-Compatible" content="IE=Edge,chrome=1">
+        <meta name="viewport" content="width=device-width, initial-scale=1">
+        <meta name="google" content="notranslate">
+    </head>
+    <body style="margin: 0; padding: 0;">
+        <script type="text/javascript" src="{% static hashi_file_path %}"></script>
+    </body>
+</html>
diff --git a/kolibri/core/content/test/test_zipcontent.py b/kolibri/core/content/test/test_zipcontent.py
@@ -7,19 +7,19 @@
 from django.test import override_settings
 from django.test import TestCase
 from django.utils.http import http_date
-from mock import mock_open
 from mock import patch
 
 from kolibri.core.content.models import LocalFile
 from kolibri.core.content.utils.paths import get_content_storage_file_path
 from kolibri.core.content.zip_wsgi import generate_zip_content_response
 from kolibri.core.content.zip_wsgi import get_hashi_view_response
-from kolibri.core.content.zip_wsgi import hashi_template
 from kolibri.core.content.zip_wsgi import INITIALIZE_HASHI_FROM_IFRAME
 from kolibri.utils.tests.helpers import override_option
 
 
-DUMMY_FILENAME = "hashi123.js"
+DUMMY_HTML_FILENAME = "hashi123.html"
+
+DUMMY_JS_FILENAME = "hashi123.js"
 
 hashi_injection = '<script type="text/javascript">{}</script>'.format(
     INITIALIZE_HASHI_FROM_IFRAME
@@ -304,19 +304,23 @@ def test_delete_not_allowed(self):
 class HashiViewTestCase(TestCase):
     def setUp(self):
         self.environ = {}
-        self.hashi_js = "test"
         setup_testing_defaults(self.environ)
 
     def _get_hashi(self, **kwargs):
-        self.environ["PATH_INFO"] = "/" + DUMMY_FILENAME
+        self.environ["PATH_INFO"] = "/" + DUMMY_HTML_FILENAME
         self.environ.update(kwargs)
-        m = mock_open(read_data=self.hashi_js)
-        with patch("kolibri.core.content.zip_wsgi.codecs.open", m), patch(
-            "kolibri.core.content.zip_wsgi.get_hashi_filename",
-            return_value=DUMMY_FILENAME,
+        with patch(
+            "kolibri.core.content.zip_wsgi.get_hashi_html_filename",
+            return_value=DUMMY_HTML_FILENAME,
+        ), patch(
+            "kolibri.core.content.utils.paths.get_hashi_html_filename",
+            return_value=DUMMY_HTML_FILENAME,
         ), patch(
-            "kolibri.core.content.utils.paths.get_hashi_filename",
-            return_value=DUMMY_FILENAME,
+            "kolibri.core.content.zip_wsgi.get_hashi_js_filename",
+            return_value=DUMMY_JS_FILENAME,
+        ), patch(
+            "kolibri.core.content.utils.paths.get_hashi_js_filename",
+            return_value=DUMMY_JS_FILENAME,
         ):
             return get_hashi_view_response(self.environ)
 
@@ -332,15 +336,8 @@ def test_expires_set_on_response(self):
         response = self._get_hashi()
         self.assertIsNotNone(response.get("Expires"))
 
-    def test_response_content(self):
-        response = self._get_hashi()
-        self.assertEqual(response.status_code, 200)
-        self.assertEqual(
-            response.content.decode(), hashi_template.format(self.hashi_js)
-        )
-
     def test_redirect_if_wrong_filename(self):
-        response = self._get_hashi(PATH_INFO="test")
+        response = self._get_hashi(PATH_INFO="test.html")
         self.assertEqual(response.status_code, 301)
 
     def test_options_returns_empty(self):

diff --git a/kolibri/core/content/utils/paths.py b/kolibri/core/content/utils/paths.py
@@ -260,7 +260,7 @@ def get_zip_content_base_path():
 HASHI_FILENAME = None
 
 
-def get_hashi_filename():
+def get_hashi_js_filename():
     global HASHI_FILENAME
     if HASHI_FILENAME is None or getattr(settings, "DEVELOPER_MODE", None):
         with io.open(
@@ -272,13 +272,17 @@ def get_hashi_filename():
     return HASHI_FILENAME
 
 
+def get_hashi_html_filename():
+    return "{}.html".format(".".join(get_hashi_js_filename().split(".")[:-1]))
+
+
 def get_hashi_base_path():
     return "{}{}".format(conf.OPTIONS["Deployment"]["URL_PATH_PREFIX"], HASHI)
 
 
 def get_hashi_path():
     return "{}{}{}".format(
-        conf.OPTIONS["Deployment"]["URL_PATH_PREFIX"], HASHI, get_hashi_filename()
+        conf.OPTIONS["Deployment"]["URL_PATH_PREFIX"], HASHI, get_hashi_html_filename()
     )
 
 

diff --git a/kolibri/core/content/zip_wsgi.py b/kolibri/core/content/zip_wsgi.py
@@ -2,7 +2,6 @@
 from __future__ import print_function
 from __future__ import unicode_literals
 
-import codecs
 import json
 import logging
 import mimetypes
@@ -15,8 +14,6 @@
 import html5lib
 from cheroot import wsgi
 from django.conf import settings
-from django.contrib.staticfiles.finders import find
-from django.contrib.staticfiles.storage import staticfiles_storage
 from django.core.cache import cache
 from django.core.handlers.wsgi import WSGIRequest
 from django.http import HttpResponse
@@ -31,12 +28,12 @@
 from django.utils.cache import patch_response_headers
 from django.utils.encoding import force_str
 from django.utils.http import http_date
-from django.utils.safestring import mark_safe
 
 from kolibri.core.content.errors import InvalidStorageFilenameError
 from kolibri.core.content.utils.paths import get_content_storage_file_path
 from kolibri.core.content.utils.paths import get_hashi_base_path
-from kolibri.core.content.utils.paths import get_hashi_filename
+from kolibri.core.content.utils.paths import get_hashi_html_filename
+from kolibri.core.content.utils.paths import get_hashi_js_filename
 from kolibri.core.content.utils.paths import get_hashi_path
 from kolibri.core.content.utils.paths import get_zip_content_base_path
 
@@ -72,20 +69,12 @@ def django_response_to_wsgi(response, environ, start_response):
     return response
 
 
-hashi_template = """
-<!DOCTYPE html>
-<html>
-    <head>
-        <meta charset="utf-8">
-        <meta http-equiv="X-UA-Compatible" content="IE=Edge,chrome=1">
-        <meta name="viewport" content="width=device-width, initial-scale=1">
-        <meta name="google" content="notranslate">
-    </head>
-    <body style="margin: 0; padding: 0;">
-        <script type="text/javascript">{}</script>
-    </body>
-</html>
-"""
+template_engine = Engine(
+    dirs=[os.path.join(os.path.dirname(__file__), "./templates/content")],
+    libraries={"staticfiles": "django.contrib.staticfiles.templatetags.staticfiles"},
+)
+h5p_template = template_engine.get_template("h5p.html")
+hashi_template = template_engine.get_template("hashi.html")
 
 allowed_methods = set(["GET", "OPTIONS"])
 
@@ -94,39 +83,39 @@ def _hashi_response_from_request(request):
     if request.method not in allowed_methods:
         return HttpResponseNotAllowed(allowed_methods)
 
-    if request.path_info.lstrip("/") != get_hashi_filename():
+    filename = request.path_info.lstrip("/")
+
+    if filename.split(".")[-1] != "html":
+        return HttpResponseNotFound()
+
+    if filename != get_hashi_html_filename():
         return HttpResponsePermanentRedirect(get_hashi_path())
 
     if request.method == "OPTIONS":
         return HttpResponse()
 
+    developer_mode = getattr(settings, "DEVELOPER_MODE", False)
+
     # if client has a cached version, use that we can safely assume nothing has changed
     # as we provide a unique path per compiled hashi JS file.
-    if request.META.get("HTTP_IF_MODIFIED_SINCE"):
+    if request.META.get("HTTP_IF_MODIFIED_SINCE") and not developer_mode:
         return HttpResponseNotModified()
-    CACHE_KEY = "HASHI_VIEW_RESPONSE_{}".format(get_hashi_filename())
+    CACHE_KEY = "HASHI_VIEW_RESPONSE_{}".format(get_hashi_html_filename())
     cached_response = cache.get(CACHE_KEY)
-    if cached_response is not None:
+    if cached_response is not None and not developer_mode:
         return cached_response
-    # Removes Byte Order Mark
-    charset = "utf-8-sig"
-    basename = "content/{filename}".format(filename=get_hashi_filename())
-
-    filename = None
-    # First try finding the file using the storage class.
-    # This is skipped in DEVELOPER_MODE mode as files might be outdated
-    # Or may not even be on disk.
-    if not getattr(settings, "DEVELOPER_MODE", False):
-        filename = staticfiles_storage.path(basename)
-    else:
-        filename = find(basename)
-
-    with codecs.open(filename, "r", charset) as fd:
-        content = fd.read()
-    content_type = "text/html"
-    response = HttpResponse(
-        mark_safe(hashi_template.format(content)), content_type=content_type
+
+    content = hashi_template.render(
+        Context(
+            {
+                "hashi_file_path": "content/{filename}".format(
+                    filename=get_hashi_js_filename()
+                )
+            }
+        )
     )
+
+    response = HttpResponse(content, content_type="text/html")
     response["Content-Length"] = len(response.content)
     response["Last-Modified"] = http_date(time.time())
     patch_response_headers(response, cache_timeout=YEAR_IN_SECONDS)
@@ -240,73 +229,45 @@ def parse_html(content):
         return content
 
 
-h5p_engine = Engine(
-    dirs=[os.path.join(os.path.dirname(__file__), "./templates/content")]
-)
-h5p_template = h5p_engine.get_template("h5p.html")
-
-
-def get_h5p(zf, embedded_filepath):
+def get_h5p(zf):
     file_size = 0
-    if not embedded_filepath:
-        # Get the h5p bootloader, and then run it through our hashi templating code.
-        # return the H5P bootloader code
-        try:
-            h5pdata = load_json_from_zipfile(zf, "h5p.json")
-            contentdata = load_json_from_zipfile(zf, "content/content.json")
-        except KeyError:
-            return HttpResponseNotFound("No valid h5p file was found at this location")
-        jsfiles, cssfiles = recursive_h5p_dependencies(zf, h5pdata)
-        jsfiles = jsfiles.keys()
-        cssfiles = cssfiles.keys()
-        path_includes_version = (
-            "true"
-            if "-" in [name for name in zf.namelist() if "/" in name][0]
-            else "false"
-        )
-        main_library_data = [
-            lib
-            for lib in h5pdata["preloadedDependencies"]
-            if lib["machineName"] == h5pdata["mainLibrary"]
-        ][0]
-        bootstrap_content = h5p_template.render(
-            Context(
-                {
-                    "jsfiles": jsfiles,
-                    "cssfiles": cssfiles,
-                    "content": json.dumps(
-                        json.dumps(
-                            contentdata, separators=(",", ":"), ensure_ascii=False
-                        )
-                    ),
-                    "library": "{machineName} {majorVersion}.{minorVersion}".format(
-                        **main_library_data
-                    ),
-                    "path_includes_version": path_includes_version,
-                }
-            ),
-        )
-        content = parse_html(bootstrap_content)
-        content_type = "text/html"
-        response = HttpResponse(content, content_type=content_type)
-        file_size = len(response.content)
-    elif embedded_filepath.startswith("dist/"):
-        # return static H5P dist resources
-        # First try finding the file using the storage class.
-        # This is skipped in DEVELOPER_MODE mode as files might be outdated
-        basename = "assets/h5p-standalone-" + embedded_filepath
-        if not getattr(settings, "DEVELOPER_MODE", False):
-            path = staticfiles_storage.path(basename)
-        else:
-            path = find(basename)
-        if path is None:
-            return HttpResponseNotFound("{} not found".format(embedded_filepath))
-        # try to guess the MIME type of the embedded file being referenced
-        content_type = (
-            mimetypes.guess_type(embedded_filepath)[0] or "application/octet-stream"
-        )
-        response = FileResponse(open(path, "rb"), content_type=content_type)
-        file_size = os.stat(path).st_size
+    # Get the h5p bootloader, and then run it through our hashi templating code.
+    # return the H5P bootloader code
+    try:
+        h5pdata = load_json_from_zipfile(zf, "h5p.json")
+        contentdata = load_json_from_zipfile(zf, "content/content.json")
+    except KeyError:
+        return HttpResponseNotFound("No valid h5p file was found at this location")
+    jsfiles, cssfiles = recursive_h5p_dependencies(zf, h5pdata)
+    jsfiles = jsfiles.keys()
+    cssfiles = cssfiles.keys()
+    path_includes_version = (
+        "true" if "-" in [name for name in zf.namelist() if "/" in name][0] else "false"
+    )
+    main_library_data = [
+        lib
+        for lib in h5pdata["preloadedDependencies"]
+        if lib["machineName"] == h5pdata["mainLibrary"]
+    ][0]
+    bootstrap_content = h5p_template.render(
+        Context(
+            {
+                "jsfiles": jsfiles,
+                "cssfiles": cssfiles,
+                "content": json.dumps(
+                    json.dumps(contentdata, separators=(",", ":"), ensure_ascii=False)
+                ),
+                "library": "{machineName} {majorVersion}.{minorVersion}".format(
+                    **main_library_data
+                ),
+                "path_includes_version": path_includes_version,
+            }
+        ),
+    )
+    content = parse_html(bootstrap_content)
+    content_type = "text/html"
+    response = HttpResponse(content, content_type=content_type)
+    file_size = len(response.content)
     if file_size:
         response["Content-Length"] = file_size
     return response
@@ -316,10 +277,8 @@ def get_embedded_file(zipped_path, zipped_filename, embedded_filepath):
     with zipfile.ZipFile(zipped_path) as zf:
 
         # handle H5P files
-        if zipped_path.endswith("h5p") and (
-            not embedded_filepath or embedded_filepath.startswith("dist/")
-        ):
-            return get_h5p(zf, embedded_filepath)
+        if zipped_path.endswith("h5p") and not embedded_filepath:
+            return get_h5p(zf)
         # if no path, or a directory, is being referenced, look for an index.html file
         if not embedded_filepath or embedded_filepath.endswith("/"):
             embedded_filepath += "index.html"