diff --git a/lms/urls.py b/lms/urls.py index 7a3f6fd78f..38229b07ee 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -221,7 +221,7 @@ if settings.WIKI_ENABLED: # never be returned by a reverse() so they come after the other url patterns url(r'^courses/{}/course_wiki/?$'.format(settings.COURSE_ID_PATTERN), course_wiki_views.course_wiki_redirect, name='course_wiki'), - url(r'^courses/{}/wiki/'.format(settings.COURSE_KEY_REGEX), include(wiki_pattern())), + url(r'^courses/{}/wiki/'.format(settings.COURSE_KEY_REGEX), include(wiki_pattern(app_name='course_wiki_do_not_reverse', namespace='course_wiki_do_not_reverse'))), ] COURSE_URLS = [ diff --git a/openedx/core/djangoapps/theming/storage.py b/openedx/core/djangoapps/theming/storage.py index 2457c38242..86bcec667c 100644 --- a/openedx/core/djangoapps/theming/storage.py +++ b/openedx/core/djangoapps/theming/storage.py @@ -4,12 +4,18 @@ See https://docs.djangoproject.com/en/1.8/ref/contrib/staticfiles/ """ import os.path import posixpath +import re +import django from django.conf import settings from django.contrib.staticfiles.finders import find from django.contrib.staticfiles.storage import CachedFilesMixin, StaticFilesStorage from django.utils._os import safe_join -from django.utils.six.moves.urllib.parse import unquote, urlsplit # pylint: disable=no-name-in-module, import-error +from django.utils.six.moves.urllib.parse import ( # pylint: disable=no-name-in-module, import-error + unquote, + urldefrag, + urlsplit +) from pipeline.storage import PipelineMixin from openedx.core.djangoapps.theming.helpers import ( @@ -109,10 +115,10 @@ class ThemeCachedFilesMixin(CachedFilesMixin): """ Comprehensive theme aware CachedFilesMixin. Main purpose of subclassing CachedFilesMixin is to override the following methods. - 1 - url + 1 - _url 2 - url_converter - url: + _url: This method takes asset name as argument and is responsible for adding hash to the name to support caching. This method is called during both collectstatic command and live server run. @@ -136,13 +142,16 @@ class ThemeCachedFilesMixin(CachedFilesMixin): want it to return absolute url (e.g. url("/static/images/logo.790c9a5340cb.png")) so that it works properly with themes. - The overridden method here simply comments out the two lines that convert absolute url to relative url, + The overridden method here simply comments out the line that convert absolute url to relative url, hence absolute urls are used instead of relative urls. """ - def url(self, name, force=False): + def _processed_asset_name(self, name): """ - Returns themed url for the given asset. + Returns either a themed or unthemed version of the given asset name, + depending on several factors. + + See the class docstring for more info. """ theme = get_current_theme() if theme and theme.theme_dir_name not in name: @@ -160,15 +169,46 @@ class ThemeCachedFilesMixin(CachedFilesMixin): if theme in [theme.theme_dir_name for theme in get_themes()]: asset_name = "/".join(name.split("/")[1:]) - return super(ThemeCachedFilesMixin, self).url(asset_name, force) + return asset_name - def url_converter(self, name, template=None): + # TODO: Remove Django 1.11 upgrade shim + # SHIM: This override method modifies the name argument to contain a theme + # prefix when Django < 1.11. In Django >= 1.11, asset name processing is + # done in the _url function, so this method becomes a no-op passthrough. + # After the 1.11 upgrade, delete this method. + def url(self, name, force=False): + """ + This override method serves a similar function to _url, but this is + needed for Django < 1.11. + """ + if django.VERSION < (1, 11): + processed_asset_name = self._processed_asset_name(name) + return super(ThemeCachedFilesMixin, self).url(processed_asset_name, force) + else: + # Passthrough directly to the function we are overriding. _url in + # Django 1.11+ will take care of processing the asset name. + return super(ThemeCachedFilesMixin, self).url(name, force) + + def _url(self, hashed_name_func, name, force=False, hashed_files=None): + """ + This override method swaps out `name` with a processed version. + + See the class docstring for more info. + """ + processed_asset_name = self._processed_asset_name(name) + return super(ThemeCachedFilesMixin, self)._url(hashed_name_func, processed_asset_name, force, hashed_files) + + # TODO: Remove Django 1.11 upgrade shim + # SHIM: This method implements url_converter for Django < 1.11. + # After the 1.11 upgrade, delete this method. + def _url_converter__lt_111(self, name, template=None): """ This is an override of url_converter from CachedFilesMixin. - It just comments out two lines at the end of the method. - The purpose of this override is to make converter method return absolute urls instead of relative urls. - This behavior is necessary for theme overrides, as we get 404 on assets with relative urls on a themed site. + There are two lines commented out in order to make the converter method + return absolute urls instead of relative urls. This behavior is + necessary for theme overrides, as we get 404 on assets with relative + urls on a themed site. """ if template is None: template = self.default_template @@ -219,6 +259,92 @@ class ThemeCachedFilesMixin(CachedFilesMixin): return converter + # TODO: Remove Django 1.11 upgrade shim + # SHIM: This method implements url_converter for Django >= 1.11. + # After the 1.11 upgrade, rename this method to url_converter. + def _url_converter__gte_111(self, name, hashed_files, template=None): + """ + This is an override of url_converter from CachedFilesMixin. + + It changes one line near the end of the method (see the NOTE) in order + to return absolute urls instead of relative urls. This behavior is + necessary for theme overrides, as we get 404 on assets with relative + urls on a themed site. + """ + if template is None: + template = self.default_template + + def converter(matchobj): + """ + Convert the matched URL to a normalized and hashed URL. + This requires figuring out which files the matched URL resolves + to and calling the url() method of the storage. + """ + matched, url = matchobj.groups() + + # Ignore absolute/protocol-relative and data-uri URLs. + if re.match(r'^[a-z]+:', url): + return matched + + # Ignore absolute URLs that don't point to a static file (dynamic + # CSS / JS?). Note that STATIC_URL cannot be empty. + if url.startswith('/') and not url.startswith(settings.STATIC_URL): + return matched + + # Strip off the fragment so a path-like fragment won't interfere. + url_path, fragment = urldefrag(url) + + if url_path.startswith('/'): + # Otherwise the condition above would have returned prematurely. + assert url_path.startswith(settings.STATIC_URL) + target_name = url_path[len(settings.STATIC_URL):] + else: + # We're using the posixpath module to mix paths and URLs conveniently. + source_name = name if os.sep == '/' else name.replace(os.sep, '/') + target_name = posixpath.join(posixpath.dirname(source_name), url_path) + + # Determine the hashed name of the target file with the storage backend. + hashed_url = self._url( + self._stored_name, unquote(target_name), + force=True, hashed_files=hashed_files, + ) + + # NOTE: + # The line below was commented out so that absolute urls are used instead of relative urls to make themed + # assets work correctly. + # + # The line is commented and not removed to make future django upgrade easier and show exactly what is + # changed in this method override + # + #transformed_url = '/'.join(url_path.split('/')[:-1] + hashed_url.split('/')[-1:]) + transformed_url = hashed_url # This line was added. + + # Restore the fragment that was stripped off earlier. + if fragment: + transformed_url += ('?#' if '?#' in url else '#') + fragment + + # Return the hashed version to the file + return template % unquote(transformed_url) + + return converter + + # TODO: Remove Django 1.11 upgrade shim + # SHIM: This method switches the implementation of url_converter according + # to the Django version. After the 1.11 upgrade, do these things: + # + # 1. delete _url_converter__lt_111. + # 2. delete url_converter (below). + # 3. rename _url_converter__gte_111 to url_converter. + def url_converter(self, *args, **kwargs): + """ + An implementation selector for the url_converter method. This is in + place only for the Django 1.11 upgrade. + """ + if django.VERSION < (1, 11): + return self._url_converter__lt_111(*args, **kwargs) + else: + return self._url_converter__gte_111(*args, **kwargs) + class ThemePipelineMixin(PipelineMixin): """