[BD-13] [BB-5363] refactor: deprecates replace url related properties from ModuleSystem

cms/djangoapps/contentstore/views/preview.py

@@ -28,15 +28,15 @@
 from xmodule.x_module import AUTHOR_VIEW, PREVIEW_VIEWS, STUDENT_VIEW, ModuleSystem, XModule, XModuleDescriptor
 from cms.djangoapps.xblock_config.models import StudioConfig
 from cms.lib.xblock.field_data import CmsFieldData
-from common.djangoapps import static_replace
+from common.djangoapps.static_replace.services import ReplaceURLService
+from common.djangoapps.static_replace.wrapper import replace_urls_wrapper
 from common.djangoapps.edxmako.shortcuts import render_to_string
 from common.djangoapps.edxmako.services import MakoService
 from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService
 from lms.djangoapps.lms_xblock.field_data import LmsFieldData
 from openedx.core.lib.license import wrap_with_license
 from openedx.core.lib.cache_utils import CacheService
 from openedx.core.lib.xblock_utils import (
-    replace_static_urls,
     request_token,
     wrap_fragment,
     wrap_xblock,
@@ -162,6 +162,8 @@ def _preview_module_system(request, descriptor, field_data):
     course_id = descriptor.location.course_key
     display_name_only = (descriptor.category == 'static_tab')
 
+    replace_url_service = ReplaceURLService(course_id=course_id)
+
     wrappers = [
         # This wrapper wraps the module in the template specified above
         partial(
@@ -174,7 +176,7 @@ def _preview_module_system(request, descriptor, field_data):
 
         # This wrapper replaces urls in the output that start with /static
         # with the correct course-specific url for the static content
-        partial(replace_static_urls, None, course_id=course_id),
+        partial(replace_urls_wrapper, replace_url_service=replace_url_service, static_replace_only=True),
         _studio_wrap_xblock,
     ]
 
@@ -199,7 +201,6 @@ def _preview_module_system(request, descriptor, field_data):
         filestore=descriptor.runtime.resources_fs,
         get_module=partial(_load_preview_module, request),
         debug=True,
-        replace_urls=partial(static_replace.replace_static_urls, data_directory=None, course_id=course_id),
         mixins=settings.XBLOCK_MIXINS,
         course_id=course_id,
 
@@ -223,6 +224,7 @@ def _preview_module_system(request, descriptor, field_data):
             "teams_configuration": TeamsConfigurationService(),
             "sandbox": SandboxService(contentstore=contentstore, course_id=course_id),
             "cache": CacheService(cache),
+            'replace_urls': replace_url_service
         },
     )
 

cms/djangoapps/contentstore/views/tests/test_preview.py

@@ -22,6 +22,7 @@
 from xmodule.modulestore.tests.test_asides import AsideTestType
 from cms.djangoapps.contentstore.utils import reverse_usage_url
 from cms.djangoapps.xblock_config.models import StudioConfig
+from common.djangoapps import static_replace
 from common.djangoapps.student.tests.factories import UserFactory
 
 from ..preview import _preview_module_system, get_preview_fragment
@@ -174,6 +175,7 @@ def test_block_branch_not_changed_by_preview_handler(self, default_store):
 @XBlock.needs("field-data")
 @XBlock.needs("i18n")
 @XBlock.needs("mako")
+@XBlock.needs("replace_urls")
 @XBlock.needs("user")
 @XBlock.needs("teams_configuration")
 class PureXBlock(XBlock):
@@ -205,7 +207,7 @@ def setUp(self):
         self.field_data = mock.Mock()
 
     @XBlock.register_temp_plugin(PureXBlock, identifier='pure')
-    @ddt.data("user", "i18n", "field-data", "teams_configuration")
+    @ddt.data("user", "i18n", "field-data", "teams_configuration", "replace_urls")
     def test_expected_services_exist(self, expected_service):
         """
         Tests that the 'user' and 'i18n' services are provided by the Studio runtime.
@@ -287,3 +289,8 @@ def test_no_get_python_lib_zip(self):
     def test_cache(self):
         assert hasattr(self.runtime.cache, 'get')
         assert hasattr(self.runtime.cache, 'set')
+
+    def test_replace_urls(self):
+        html = '<a href="/static/id">'
+        assert self.runtime.replace_urls(html) == \
+            static_replace.replace_static_urls(html, course_id=self.runtime.course_id)

common/djangoapps/static_replace/__init__.py

@@ -147,12 +147,20 @@ def replace(__, prefix, quote, rest):
     )
 
 
-def replace_static_urls(text, data_directory=None, course_id=None, static_asset_path='', static_paths_out=None):
+def replace_static_urls(
+    text,
+    data_directory=None,
+    course_id=None,
+    static_asset_path='',
+    static_paths_out=None,
+    xblock=None,
+    lookup_asset_url=None
+):
     """
     Replace /static/$stuff urls either with their correct url as generated by collectstatic,
     (/static/$md5_hashed_stuff) or by the course-specific content static url
     /static/$course_data_dir/$stuff, or, if course_namespace is not None, by the
-    correct url in the contentstore (/c4x/.. or /asset-loc:..)
+    correct url in the contentstore (/c4x/.. or /asset-loc:..) or by lookup_asset_url
 
     text: The source text to do the substitution in
     data_directory: The directory in which course data is stored
@@ -161,6 +169,8 @@ def replace_static_urls(text, data_directory=None, course_id=None, static_asset_
     static_paths_out: (optional) pass an array to collect tuples for each static URI found:
       * the original unmodified static URI
       * the updated static URI (will match the original if unchanged)
+    xblock: xblock where the static assets are stored
+    lookup_url_func: Lookup function which returns the correct path of the asset
     """
 
     if static_paths_out is None:
@@ -176,6 +186,10 @@ def replace_static_url(original, prefix, quote, rest):
             static_paths_out.append((original_uri, original_uri))
             return original
 
+        if lookup_asset_url:
+            new_url = lookup_asset_url(xblock, rest) or original_uri
+            return "".join([quote, new_url, quote])
+
         # In debug mode, if we can find the url as is,
         if settings.DEBUG and finders.find(rest, True):
             static_paths_out.append((original_uri, original_uri))

common/djangoapps/static_replace/services.py

@@ -0,0 +1,69 @@
+"""
+Supports replacement of static/course/jump-to-id URLs to absolute URLs in XBlocks.
+"""
+
+from xblock.reference.plugins import Service
+
+from common.djangoapps.static_replace import (
+    replace_course_urls,
+    replace_jump_to_id_urls,
+    replace_static_urls
+)
+
+
+class ReplaceURLService(Service):
+    """
+    A service for replacing static/course/jump-to-id URLs with absolute URLs in XBlocks.
+
+    Args:
+        course_id: Course identifier to be used in the absolute URL
+        data_directory: (optional) Directory in which course data is stored
+        static_asset_path: (optional) Path for static assets, which overrides data_directory and course_id, if nonempty
+        static_paths_out: (optional) Array to collect tuples for each static URI found:
+            * the original unmodified static URI
+            * the updated static URI (will match the original if unchanged)
+        jump_to_id_base_url: (optional) Absolute path to the base of the handler that will perform the redirect
+        lookup_url_func: Lookup function which returns the correct path of the asset
+    """
+    def __init__(
+        self,
+        data_directory=None,
+        course_id=None,
+        static_asset_path='',
+        static_paths_out=None,
+        jump_to_id_base_url=None,
+        lookup_asset_url=None,
+        **kwargs
+    ):
+        super().__init__(**kwargs)
+        self.data_directory = data_directory
+        self.course_id = course_id
+        self.static_asset_path = static_asset_path
+        self.static_paths_out = static_paths_out
+        self.jump_to_id_base_url = jump_to_id_base_url
+        self.lookup_asset_url = lookup_asset_url
+
+    def replace_urls(self, text, static_replace_only=False):
+        """
+        Replaces all static/course/jump-to-id URLs in provided text/html.
+
+        Args:
+            text: String containing the URL to be replaced
+            static_replace_only: If True, only static urls will be replaced
+        """
+        if self.lookup_asset_url:
+            text = replace_static_urls(text, xblock=self.xblock(), lookup_asset_url=self.lookup_asset_url)
+        else:
+            text = replace_static_urls(
+                text,
+                data_directory=self.data_directory,
+                course_id=self.course_id,
+                static_asset_path=self.static_asset_path,
+                static_paths_out=self.static_paths_out
+            )
+            if not static_replace_only:
+                text = replace_course_urls(text, self.course_id)
+                if self.jump_to_id_base_url:
+                    text = replace_jump_to_id_urls(text, self.course_id, self.jump_to_id_base_url)
+
+        return text

common/djangoapps/static_replace/test/test_static_replace.py

@@ -3,6 +3,7 @@
 
 import re
 from io import BytesIO
+from unittest import TestCase
 from unittest.mock import Mock, patch
 from urllib.parse import parse_qsl, quote, urlparse, urlunparse, urlencode
 
@@ -11,6 +12,7 @@
 from django.test import override_settings
 from opaque_keys.edx.keys import CourseKey
 from PIL import Image
+from web_fragments.fragment import Fragment
 
 from common.djangoapps.static_replace import (
     _url_replace_regex,
@@ -19,6 +21,8 @@
     replace_course_urls,
     replace_static_urls
 )
+from common.djangoapps.static_replace.services import ReplaceURLService
+from common.djangoapps.static_replace.wrapper import replace_urls_wrapper
 from xmodule.assetstore.assetmgr import AssetManager  # lint-amnesty, pylint: disable=wrong-import-order
 from xmodule.contentstore.content import StaticContent  # lint-amnesty, pylint: disable=wrong-import-order
 from xmodule.contentstore.django import contentstore  # lint-amnesty, pylint: disable=wrong-import-order
@@ -783,3 +787,143 @@ def test_canonical_asset_path_with_c4x_style_assets(self, base_url, start, expec
         with check_mongo_calls(mongo_calls):
             asset_path = StaticContent.get_canonicalized_asset_path(self.courses[prefix].id, start, base_url, exts)
             assert re.match(expected, asset_path) is not None
+
+
+class ReplaceURLServiceTest(TestCase):
+    """
+    Test ReplaceURLService methods
+    """
+    def setUp(self):
+        super().setUp()
+        self.mock_replace_static_urls = self.create_patch(
+            'common.djangoapps.static_replace.services.replace_static_urls'
+        )
+        self.mock_replace_course_urls = self.create_patch(
+            'common.djangoapps.static_replace.services.replace_course_urls'
+        )
+        self.mock_replace_jump_to_id_urls = self.create_patch(
+            'common.djangoapps.static_replace.services.replace_jump_to_id_urls'
+        )
+
+    def create_patch(self, name):
+        patcher = patch(name)
+        mock_method = patcher.start()
+        self.addCleanup(patcher.stop)
+        return mock_method
+
+    def test_replace_static_url_only(self):
+        """
+        Test only replace_static_urls method called when static_replace_only is passed as True.
+        """
+        replace_url_service = ReplaceURLService(course_id=COURSE_KEY)
+        return_text = replace_url_service.replace_urls("text", static_replace_only=True)
+        assert self.mock_replace_static_urls.called
+        assert not self.mock_replace_course_urls.called
+        assert not self.mock_replace_jump_to_id_urls.called
+
+    def test_replace_course_urls_called(self):
+        """
+        Test replace_course_urls method called static_replace_only is passed as False.
+        """
+        replace_url_service = ReplaceURLService(course_id=COURSE_KEY)
+        return_text = replace_url_service.replace_urls("text")
+        assert self.mock_replace_course_urls.called
+
+    def test_replace_jump_to_id_urls_called(self):
+        """
+        Test replace_jump_to_id_urls method called jump_to_id_base_url is provided.
+        """
+        replace_url_service = ReplaceURLService(course_id=COURSE_KEY, jump_to_id_base_url="/course/course_id")
+        return_text = replace_url_service.replace_urls("text")
+        assert self.mock_replace_jump_to_id_urls.called
+
+    def test_replace_jump_to_id_urls_not_called(self):
+        """
+        Test replace_jump_to_id_urls method called jump_to_id_base_url is not provided.
+        """
+        replace_url_service = ReplaceURLService(course_id=COURSE_KEY)
+        return_text = replace_url_service.replace_urls("text")
+        assert not self.mock_replace_jump_to_id_urls.called
+
+
+@ddt.ddt
+class TestReplaceURLWrapper(SharedModuleStoreTestCase):
+    """
+    Tests for replace_url_wrapper utility function.
+    """
+
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.course_mongo = CourseFactory.create(
+            default_store=ModuleStoreEnum.Type.mongo,
+            org='TestX',
+            number='TS01',
+            run='2015'
+        )
+        cls.course_split = CourseFactory.create(
+            default_store=ModuleStoreEnum.Type.split,
+            org='TestX',
+            number='TS02',
+            run='2015'
+        )
+
+    @ddt.data('course_mongo', 'course_split')
+    def test_replace_jump_to_id_urls(self, course_id):
+        """
+        Verify that the jump-to URL has been replaced.
+        """
+        course = getattr(self, course_id)
+        replace_url_service = ReplaceURLService(course_id=course.id, jump_to_id_base_url='/base_url/')
+        test_replace = replace_urls_wrapper(
+            block=course,
+            view='baseview',
+            frag=Fragment('<a href="/jump_to_id/id">'),
+            context=None,
+            replace_url_service=replace_url_service
+        )
+        assert isinstance(test_replace, Fragment)
+        assert test_replace.content == '<a href="/base_url/id">'
+
+    @ddt.data(
+        ('course_mongo', '<a href="/courses/TestX/TS01/2015/id">'),
+        ('course_split', '<a href="/courses/course-v1:TestX+TS02+2015/id">')
+    )
+    @ddt.unpack
+    def test_replace_course_urls(self, course_id, anchor_tag):
+        """
+        Verify that the course URL has been replaced.
+        """
+        course = getattr(self, course_id)
+        replace_url_service = ReplaceURLService(course_id=course.id)
+        test_replace = replace_urls_wrapper(
+            block=course,
+            view='baseview',
+            frag=Fragment('<a href="/course/id">'),
+            context=None,
+            replace_url_service=replace_url_service
+        )
+        assert isinstance(test_replace, Fragment)
+        assert test_replace.content == anchor_tag
+
+    @ddt.data(
+        ('course_mongo', '<a href="/c4x/TestX/TS01/asset/id">'),
+        ('course_split', '<a href="/asset-v1:TestX+TS02+2015+type@asset+block/id">')
+    )
+    @ddt.unpack
+    def test_replace_static_urls(self, course_id, anchor_tag):
+        """
+        Verify that the static URL has been replaced.
+        """
+        course = getattr(self, course_id)
+        replace_url_service = ReplaceURLService(course_id=course.id)
+        test_replace = replace_urls_wrapper(
+            block=course,
+            view='baseview',
+            frag=Fragment('<a href="/static/id">'),
+            context=None,
+            replace_url_service=replace_url_service,
+            static_replace_only=True
+        )
+        assert isinstance(test_replace, Fragment)
+        assert test_replace.content == anchor_tag

common/djangoapps/static_replace/wrapper.py

@@ -0,0 +1,12 @@
+"""
+Wrapper function to replace static/course/jump-to-id URLs in XBlock to absolute URLs
+"""
+
+from openedx.core.lib.xblock_utils import wrap_fragment
+
+
+def replace_urls_wrapper(block, view, frag, context, replace_url_service, static_replace_only=False):  # pylint: disable=unused-argument
+    """
+    Replace any static/course/jump-to-id URLs in XBlock to absolute URLs
+    """
+    return wrap_fragment(frag, replace_url_service.replace_urls(frag.content, static_replace_only))