feat: deprecate filestore

This:

1. Removes the `filestore` property from the `ModuleSystem` in favor of
   the `runtime.resources_fs` property.
   In the original code, `filestore` is equal to
   `DescriptorSystem.runtime.resources_fs`. It's safe to replace it with
   `ModuleSystem.runtime.resources_fs` because both runtimes are combined
   using the `CachingDescriptorSystem`. It provides the `resources_fs` property
   that uses the same file storage.

2. Renames `filestore` argument to `resources_fs` in the `LoncapaSystem`
   constructor.

3. Adds the deprecated `filestore` property to the `ModuleSystemShim`
   and `RuntimeShim`.

cms/djangoapps/contentstore/views/preview.py

@@ -198,7 +198,6 @@ def _preview_module_system(request, descriptor, field_data):
         static_url=settings.STATIC_URL,
         # TODO (cpennington): Do we want to track how instructors are using the preview problems?
         track_function=lambda event_type, event: None,
-        filestore=descriptor.runtime.resources_fs,
         get_module=partial(_load_preview_module, request),
         debug=True,
         mixins=settings.XBLOCK_MIXINS,

common/lib/capa/capa/capa_problem.py

@@ -103,10 +103,10 @@ class LoncapaSystem(object):
         can_execute_unsafe_code,
         get_python_lib_zip,
         DEBUG,
-        filestore,
         i18n,
         node_path,
         render_template,
+        resources_fs,
         seed,      # Why do we do this if we have self.seed?
         STATIC_URL,
         xqueue,
@@ -118,10 +118,10 @@ class LoncapaSystem(object):
         self.can_execute_unsafe_code = can_execute_unsafe_code
         self.get_python_lib_zip = get_python_lib_zip
         self.DEBUG = DEBUG                              # pylint: disable=invalid-name
-        self.filestore = filestore
         self.i18n = i18n
         self.node_path = node_path
         self.render_template = render_template
+        self.resources_fs = resources_fs
         self.seed = seed                     # Why do we do this if we have self.seed?
         self.STATIC_URL = STATIC_URL                    # pylint: disable=invalid-name
         self.xqueue = xqueue
@@ -814,8 +814,8 @@ class LoncapaProblem(object):
             filename = inc.get('file') if six.PY3 else inc.get('file').decode('utf-8')
             if filename is not None:
                 try:
-                    # open using LoncapaSystem OSFS filestore
-                    ifp = self.capa_system.filestore.open(filename)
+                    # open using LoncapaSystem OSFS filesystem
+                    ifp = self.capa_system.resources_fs.open(filename)
                 except Exception as err:  # lint-amnesty, pylint: disable=broad-except
                     log.warning(
                         'Error %s in problem xml include: %s',
@@ -823,7 +823,7 @@ class LoncapaProblem(object):
                         etree.tostring(inc, pretty_print=True)
                     )
                     log.warning(
-                        'Cannot find file %s in %s', filename, self.capa_system.filestore
+                        'Cannot find file %s in %s', filename, self.capa_system.resources_fs
                     )
                     # if debugging, don't fail - just log error
                     # TODO (vshnayder): need real error handling, display to users
@@ -876,9 +876,9 @@ class LoncapaProblem(object):
                 continue
 
             # path is an absolute path or a path relative to the data dir
-            dir = os.path.join(self.capa_system.filestore.root_path, dir)
-            # Check that we are within the filestore tree.
-            reldir = os.path.relpath(dir, self.capa_system.filestore.root_path)
+            dir = os.path.join(self.capa_system.resources_fs.root_path, dir)
+            # Check that we are within the resources_fs tree.
+            reldir = os.path.relpath(dir, self.capa_system.resources_fs.root_path)
             if ".." in reldir:
                 log.warning("Ignoring Python directory outside of course: %r", dir)
                 continue

common/lib/capa/capa/responsetypes.py

@@ -3268,7 +3268,7 @@ class SchematicResponse(LoncapaResponse):
         answer_src = answer.get('src')
         if answer_src is not None:
             # Untested; never used
-            self.code = self.capa_system.filestore.open('src/' + answer_src).read()
+            self.code = self.capa_system.resources_fs.open('src/' + answer_src).read()
         else:
             self.code = answer.text
 

common/lib/capa/capa/tests/helpers.py

@@ -72,10 +72,10 @@ def test_capa_system(render_template=None):
         can_execute_unsafe_code=lambda: False,
         get_python_lib_zip=lambda: None,
         DEBUG=True,
-        filestore=fs.osfs.OSFS(os.path.join(TEST_DIR, "test_files")),
         i18n=gettext.NullTranslations(),
         node_path=os.environ.get("NODE_PATH", "/usr/local/lib/node_modules"),
         render_template=render_template or tst_render_template,
+        resources_fs=fs.osfs.OSFS(os.path.join(TEST_DIR, "test_files")),
         seed=0,
         STATIC_URL='/dummy-static/',
         STATUS_CLASS=Status,

common/lib/capa/capa/tests/test_html_render.py

@@ -300,7 +300,7 @@ class CapaHtmlRenderTest(unittest.TestCase):
         self.assertRegex(the_html, r"<div>\s*</div>")
 
     def _create_test_file(self, path, content_str):  # lint-amnesty, pylint: disable=missing-function-docstring
-        test_fp = self.capa_system.filestore.open(path, "w")
+        test_fp = self.capa_system.resources_fs.open(path, "w")
         test_fp.write(content_str)
         test_fp.close()
 

common/lib/xmodule/xmodule/capa_module.py

@@ -616,10 +616,10 @@ class ProblemBlock(
             can_execute_unsafe_code=None,
             get_python_lib_zip=None,
             DEBUG=None,
-            filestore=self.runtime.resources_fs,
             i18n=self.runtime.service(self, "i18n"),
             node_path=None,
             render_template=None,
+            resources_fs=self.runtime.resources_fs,
             seed=None,
             STATIC_URL=None,
             xqueue=None,
@@ -678,10 +678,10 @@ class ProblemBlock(
             can_execute_unsafe_code=lambda: None,
             get_python_lib_zip=(lambda: get_python_lib_zip(contentstore, self.runtime.course_id)),
             DEBUG=None,
-            filestore=self.runtime.resources_fs,
             i18n=self.runtime.service(self, "i18n"),
             node_path=None,
             render_template=None,
+            resources_fs=self.runtime.resources_fs,
             seed=1,
             STATIC_URL=None,
             xqueue=None,
@@ -827,10 +827,10 @@ class ProblemBlock(
             can_execute_unsafe_code=sandbox_service.can_execute_unsafe_code,
             get_python_lib_zip=sandbox_service.get_python_lib_zip,
             DEBUG=self.runtime.DEBUG,
-            filestore=self.runtime.filestore,
             i18n=self.runtime.service(self, "i18n"),
             node_path=self.runtime.node_path,
             render_template=self.runtime.service(self, 'mako').render_template,
+            resources_fs=self.runtime.resources_fs,
             seed=seed,  # Why do we do this if we have self.seed?
             STATIC_URL=self.runtime.STATIC_URL,
             xqueue=self.runtime.service(self, 'xqueue'),

common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py

@@ -46,7 +46,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):  # li
         module_data: a dict mapping Location -> json that was cached from the
             underlying modulestore
         """
-        # needed by capa_problem (as runtime.filestore via this.resources_fs)
+        # needed by capa_problem (as runtime.resources_fs via this.resources_fs)
         if course_entry.course_key.course:
             root = modulestore.fs_root / course_entry.course_key.org / course_entry.course_key.course / course_entry.course_key.run  # lint-amnesty, pylint: disable=line-too-long
         else:

common/lib/xmodule/xmodule/tests/__init__.py

@@ -82,6 +82,10 @@ class TestModuleSystem(ModuleSystem):  # pylint: disable=abstract-method
     def get_asides(self, block):
         return []
 
+    @property
+    def resources_fs(self):
+        return Mock(name='TestModuleSystem.resources_fs', root_path='.'),
+
     def __repr__(self):
         """
         Custom hacky repr.
@@ -149,7 +153,6 @@ def get_test_system(
         static_url='/static',
         track_function=Mock(name='get_test_system.track_function'),
         get_module=get_module,
-        filestore=Mock(name='get_test_system.filestore', root_path='.'),
         debug=True,
         hostname="edx.org",
         services={

common/lib/xmodule/xmodule/x_module.py

@@ -2004,6 +2004,19 @@ class ModuleSystemShim:
         if replace_urls_service:
             return partial(replace_urls_service.replace_urls)
 
+    @property
+    def filestore(self):
+        """
+        A filestore ojbect. Defaults to an instance of OSFS based at settings.DATA_DIR.
+
+        Deprecated in favor of runtime.resources_fs property.
+        """
+        warnings.warn(
+            'runtime.filestore is deprecated. Please use the runtime.resources_fs service instead.',
+            DeprecationWarning, stacklevel=3,
+        )
+        return self.resources_fs
+
 
 class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, Runtime):
     """
@@ -2020,9 +2033,8 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim,
 
     def __init__(
             self, static_url, track_function, get_module,
-            descriptor_runtime, filestore=None,
-            debug=False, hostname="", publish=None, node_path="",
-            course_id=None, error_descriptor_class=None,
+            descriptor_runtime, debug=False, hostname="", publish=None,
+            node_path="", course_id=None, error_descriptor_class=None,
             field_data=None, rebind_noauth_module_to_user=None,
             **kwargs):
         """
@@ -2039,9 +2051,6 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim,
                          module instance object.  If the current user does not have
                          access to that location, returns None.
 
-        filestore - A filestore ojbect.  Defaults to an instance of OSFS based
-                         at settings.DATA_DIR.
-
         descriptor_runtime - A `DescriptorSystem` to use for loading xblocks by id
 
         course_id - the course_id containing this module
@@ -2064,7 +2073,6 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim,
 
         self.STATIC_URL = static_url
         self.track_function = track_function
-        self.filestore = filestore
         self.get_module = get_module
         self.DEBUG = self.debug = debug
         self.HOSTNAME = self.hostname = hostname

lms/djangoapps/courseware/module_render.py

@@ -727,8 +727,6 @@ def get_module_system_for_user(
     system = LmsModuleSystem(
         track_function=track_function,
         static_url=settings.STATIC_URL,
-        # TODO (cpennington): Figure out how to share info between systems
-        filestore=descriptor.runtime.resources_fs,
         get_module=inner_get_module,
         user=user,
         debug=settings.DEBUG,

openedx/core/djangoapps/xblock/runtime/shims.py

@@ -137,9 +137,12 @@ class RuntimeShim:
     @property
     def filestore(self):
         """
-        Alternate name for 'resources_fs'. Not sure if either name is deprecated
-        but we should deprecate one :)
+        Alternate name for 'resources_fs'.
         """
+        warnings.warn(
+            'filestore is deprecated. Please use runtime.resources_fs instead.',
+            DeprecationWarning, stacklevel=3,
+        )
         return self.resources_fs
 
     @property
@@ -223,7 +226,7 @@ class RuntimeShim:
         # XBlock repo at xblock.reference.plugins.FSService and is available in
         # the old runtime as the 'fs' service.
         warnings.warn(
-            "Use of legacy runtime.resources_fs or .filestore won't be able to find resources.",
+            "Use of legacy runtime.resources_fs won't be able to find resources.",
             stacklevel=3,
         )
         fake_fs = MemoryFS()

openedx/core/lib/xblock_utils/__init__.py

@@ -294,9 +294,9 @@ def add_staff_markup(user, disable_staff_debug_info, block, view, frag, context)
         histogram = None
         render_histogram = False
 
-    if settings.FEATURES.get('ENABLE_LMS_MIGRATION') and hasattr(block.runtime, 'filestore'):
+    if settings.FEATURES.get('ENABLE_LMS_MIGRATION') and hasattr(block.runtime, 'resources_fs'):
         [filepath, filename] = getattr(block, 'xml_attributes', {}).get('filename', ['', None])
-        osfs = block.runtime.filestore
+        osfs = block.runtime.resources_fs
         if filename is not None and osfs.exists(filename):
             # if original, unmangled filename exists then use it (github
             # doesn't like symlinks)