Merge pull request #11320 from edx/ammar/boomark-button-requirejs-caching

bookmark button js caching

common/djangoapps/pipeline_mako/__init__.py

@@ -5,6 +5,9 @@ from pipeline.packager import Packager
 from pipeline.utils import guess_type
 from static_replace import try_staticfiles_lookup
 
+from django.conf import settings as django_settings
+from django.contrib.staticfiles.storage import staticfiles_storage
+
 
 def compressed_css(package_name, raw=False):
     package = settings.PIPELINE_CSS.get(package_name, {})
@@ -79,3 +82,63 @@ def render_individual_js(package, paths, templates=None):
     if templates:
         tags.append(render_inline_js(package, templates))
     return '\n'.join(tags)
+
+
+def render_require_js_path_overrides(path_overrides):  # pylint: disable=invalid-name
+    """Render JavaScript to override default RequireJS paths.
+
+    The Django pipeline appends a hash to JavaScript files,
+    so if the JS asset isn't included in the bundle for the page,
+    we need to tell RequireJS where to look.
+
+    For example:
+
+        "js/vendor/jquery.min.js" --> "js/vendor/jquery.min.abcd1234"
+
+    To achive this we will add overrided paths in requirejs config at runtime.
+
+    So that any reference to 'jquery' in a JavaScript module
+    will cause RequireJS to load '/static/js/vendor/jquery.min.abcd1234.js'
+
+    If running in DEBUG mode (as in devstack), the resolved JavaScript URLs
+    won't contain hashes, so the new paths will match the original paths.
+
+    Arguments:
+        path_overrides (dict): Mapping of RequireJS module names to
+            filesystem paths.
+
+    Returns:
+        unicode: The HTML of the <script> tag with the path overrides.
+
+    """
+    # Render the <script> tag that overrides the paths
+    # Note: We don't use a Mako template to render this because Mako apparently
+    # acquires a lock when loading templates, which can lead to a deadlock if
+    # this function is called from within another template.
+    # The rendered <script> tag with overrides should be included *after*
+    # the application's RequireJS config, which defines a `require` object.
+    html = '''<script type="text/javascript">
+        (function (require) {{
+          require.config({{
+              paths: {{
+                {overrides}
+            }}
+          }});
+        }}).call(this, require || RequireJS.require);
+    </script>'''
+
+    new_paths = []
+    for url_path in path_overrides:
+        # Calculate the full URL, including any hashes added to the filename by the pipeline.
+        # This will also include the base static URL (for example, "/static/") and the
+        # ".js" extension.
+        actual_url = staticfiles_storage.url(url_path)
+
+        # RequireJS assumes that every file it tries to load has a ".js" extension, so
+        # we need to remove ".js" from the module path.
+        # RequireJS also already has a base URL set to the base static URL, so we can remove that.
+        path = actual_url.replace('.js', '').replace(django_settings.STATIC_URL, '')
+
+        new_paths.append("'{module}': '{path}'".format(module=url_path.replace('.js', ''), path=path))
+
+    return html.format(overrides=',\n'.join(new_paths))

common/djangoapps/pipeline_mako/tests/test_render.py

@@ -0,0 +1,33 @@
+""" Tests for rendering functions in the mako pipeline. """
+
+from django.test import TestCase
+from pipeline_mako import render_require_js_path_overrides
+
+
+class RequireJSPathOverridesTest(TestCase):
+    """Test RequireJS path overrides. """
+
+    OVERRIDES = [
+        'js/vendor/jquery.min.js',
+        'js/vendor/backbone-min.js',
+        'js/vendor/text.js'
+    ]
+
+    OVERRIDES_JS = [
+        "<script type=\"text/javascript\">",
+        "(function (require) {",
+        "require.config({",
+        "paths: {",
+        "'js/vendor/jquery.min': 'js/vendor/jquery.min',",
+        "'js/vendor/backbone-min': 'js/vendor/backbone-min',",
+        "'js/vendor/text': 'js/vendor/text'",
+        "}",
+        "});",
+        "}).call(this, require || RequireJS.require);",
+        "</script>"
+    ]
+
+    def test_requirejs_path_overrides(self):
+        result = render_require_js_path_overrides(self.OVERRIDES)
+        # To make the string comparision easy remove the whitespaces
+        self.assertEqual(map(str.strip, result.splitlines()), self.OVERRIDES_JS)

lms/envs/common.py

@@ -1612,7 +1612,20 @@ REQUIRE_EXCLUDE = ("build.txt",)
 # and defines some "args" function that returns a list with the command arguments to execute.
 REQUIRE_ENVIRONMENT = "node"
 
-
+# In production, the Django pipeline appends a file hash to JavaScript file names.
+# This makes it difficult for RequireJS to load its requirements, since module names
+# specified in JavaScript code do not include the hash.
+# For this reason, we calculate the actual path including the hash on the server
+# when rendering the page.  We then override the default paths provided to RequireJS
+# so it can resolve the module name to the correct URL.
+#
+# If you want to load JavaScript dependencies using RequireJS
+# but you don't want to include those dependencies in the JS bundle for the page,
+# then you need to add the js urls in this list.
+REQUIRE_JS_PATH_OVERRIDES = [
+    'js/bookmarks/views/bookmark_button.js',
+    'js/views/message_banner.js'
+]
 ################################# CELERY ######################################
 
 # Message configuration

lms/templates/courseware/courseware.html

@@ -5,6 +5,7 @@ from django.utils.translation import ugettext as _
 from django.template.defaultfilters import escapejs
 from django.conf import settings
 from edxnotes.helpers import is_feature_enabled as is_edxnotes_enabled
+from pipeline_mako import render_require_js_path_overrides
 %>
 <%
   include_special_exams = settings.FEATURES.get('ENABLE_SPECIAL_EXAMS', False) and (course.enable_proctored_exams or course.enable_timed_exams)
@@ -14,6 +15,11 @@ from edxnotes.helpers import is_feature_enabled as is_edxnotes_enabled
 </%def>
 
 <%block name="bodyclass">view-in-course view-courseware courseware ${course.css_class or ''}</%block>
+
+<%block name="js_overrides">
+${render_require_js_path_overrides(settings.REQUIRE_JS_PATH_OVERRIDES)}
+</%block>
+
 <%block name="title"><title>
     % if section_title:
 ${static.get_page_title_breadcrumbs(section_title, course_name())}

lms/templates/main.html

@@ -90,6 +90,9 @@ from branding import api as branding_api
   </script>
   <script type="text/javascript" src="${static.url("lms/js/require-config.js")}"></script>
 
+  <%block name="js_overrides">
+  </%block>
+
   % if not disable_courseware_js:
     <%static:js group='module-js'/>
   % endif