Mark assetmanager + trashcan as deprecated, replace existing usages [FC-0138]

cms/djangoapps/contentstore/management/commands/tests/test_export_olx.py

@@ -71,6 +71,7 @@ def check_export_file(self, tar_file, course_key, with_tags=False):
         self.assertIn(f"{dirname}/about", names)  # noqa: PT009
         self.assertIn(f"{dirname}/about/overview.html", names)  # noqa: PT009
         self.assertIn(f"{dirname}/assets/assets.xml", names)  # noqa: PT009
+        assert tar_file.extractfile(f"{dirname}/assets/assets.xml").read() == b'<assets/>'
         self.assertIn(f"{dirname}/policies", names)  # noqa: PT009
         if with_tags:
             self.assertIn(f"{dirname}/tags.csv", names)  # noqa: PT009

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

@@ -25,7 +25,6 @@
 from common.djangoapps.static_replace import replace_static_urls
 from common.djangoapps.student.tests.factories import UserFactory
 from openedx.core.djangoapps.authz.tests.mixins import CourseAuthzTestMixin
-from xmodule.assetstore import AssetMetadata  # pylint: disable=wrong-import-order
 from xmodule.contentstore.content import StaticContent  # pylint: disable=wrong-import-order
 from xmodule.contentstore.django import contentstore  # pylint: disable=wrong-import-order
 from xmodule.modulestore import ModuleStoreEnum  # pylint: disable=wrong-import-order
@@ -225,8 +224,7 @@ def test_mocked_filtered_response(self, mock_get_all_content_for_course):
         """
         Test the ajax asset interfaces
         """
-        asset_key = self.course.id.make_asset_key(
-            AssetMetadata.GENERAL_ASSET_TYPE, 'test.jpg')
+        asset_key = self.course.id.make_asset_key('asset', 'test.jpg')
         upload_date = datetime(2015, 1, 12, 10, 30, tzinfo=UTC)
         thumbnail_location = [
             'c4x', 'edX', 'toy', 'thumbnail', 'test_thumb.jpg', None]

common/djangoapps/static_replace/test/test_static_replace.py

@@ -22,7 +22,6 @@
 )
 from common.djangoapps.static_replace.services import ReplaceURLService
 from common.djangoapps.static_replace.wrapper import replace_urls_wrapper
-from xmodule.assetstore.assetmgr import AssetManager  # pylint: disable=wrong-import-order
 from xmodule.contentstore.content import StaticContent  # pylint: disable=wrong-import-order
 from xmodule.contentstore.django import contentstore  # pylint: disable=wrong-import-order
 from xmodule.exceptions import NotFoundError  # pylint: disable=wrong-import-order
@@ -326,7 +325,7 @@ def get_content_digest_for_asset_path(cls, prefix, path):
         asset_key = StaticContent.get_asset_key_from_path(cls.courses[prefix].id, relative_path)
 
         try:
-            content = AssetManager.find(asset_key, as_stream=True)
+            content = contentstore().find(asset_key, as_stream=True)
             return content.content_digest
         except (ItemNotFoundError, NotFoundError):
             return None

openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py

@@ -26,7 +26,6 @@
 from openedx.core.djangoapps.models.course_details import CourseDetails
 from openedx.core.djangolib.testing.utils import CacheIsolationTestCase
 from openedx.core.lib.courses import course_image_url
-from xmodule.assetstore.assetmgr import AssetManager  # pylint: disable=wrong-import-order
 from xmodule.contentstore.content import StaticContent  # pylint: disable=wrong-import-order
 from xmodule.contentstore.django import contentstore  # pylint: disable=wrong-import-order
 from xmodule.course_block import (  # pylint: disable=wrong-import-order
@@ -1071,7 +1070,7 @@ def test_happy_path(self, create_after_overview):
         # Now make sure our thumbnails are of the sizes we expect...
         for image_url, expected_size in [(image_urls['small'], config.small), (image_urls['large'], config.large)]:
             image_key = StaticContent.get_location_from_path(image_url)
-            image_content = AssetManager.find(image_key)
+            image_content = contentstore().find(image_key)
             image = Image.open(BytesIO(image_content.data))
             assert image.size == expected_size
 
@@ -1123,7 +1122,7 @@ def test_different_resolutions(self, src_dimensions):
 
         for image_url, target in [(image_urls['small'], config.small), (image_urls['large'], config.large)]:
             image_key = StaticContent.get_location_from_path(image_url)
-            image_content = AssetManager.find(image_key)
+            image_content = contentstore().find(image_key)
             image = Image.open(BytesIO(image_content.data))
 
             # Naming convention for thumbnail

openedx/core/djangoapps/contentserver/test/test_contentserver.py

@@ -22,7 +22,6 @@
 from common.djangoapps.student.models import CourseEnrollment
 from common.djangoapps.student.tests.factories import AdminFactory, UserFactory
 from openedx.core.djangoapps.waffle_utils.models import WaffleFlagCourseOverrideModel
-from xmodule.assetstore.assetmgr import AssetManager
 from xmodule.contentstore.content import VERSIONED_ASSETS_PREFIX, StaticContent
 from xmodule.contentstore.django import contentstore
 from xmodule.modulestore.django import modulestore
@@ -47,7 +46,7 @@ def get_versioned_asset_url(asset_path):
     """
     try:
         locator = StaticContent.get_location_from_path(asset_path)
-        content = AssetManager.find(locator, as_stream=True)
+        content = contentstore().find(locator, as_stream=True)
         return StaticContent.add_version_to_asset_path(asset_path, content.content_digest)
     except (InvalidKeyError, ItemNotFoundError):
         pass
@@ -61,7 +60,7 @@ def get_old_style_versioned_asset_url(asset_path):
     """
     try:
         locator = StaticContent.get_location_from_path(asset_path)
-        content = AssetManager.find(locator, as_stream=True)
+        content = contentstore().find(locator, as_stream=True)
         return f'{VERSIONED_ASSETS_PREFIX}/{content.content_digest}{asset_path}'
     except (InvalidKeyError, ItemNotFoundError):
         pass

openedx/core/djangoapps/contentserver/views.py

@@ -25,8 +25,8 @@
 from common.djangoapps.student.models import CourseEnrollment
 from openedx.core.djangoapps.header_control import force_header_for_response
 from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag
-from xmodule.assetstore.assetmgr import AssetManager
 from xmodule.contentstore.content import XASSET_LOCATION_TAG, StaticContent
+from xmodule.contentstore.django import contentstore
 from xmodule.exceptions import NotFoundError
 from xmodule.modulestore import InvalidLocationError
 from xmodule.modulestore.exceptions import ItemNotFoundError
@@ -141,7 +141,7 @@ def process_request(request):
         if request.META.get('HTTP_RANGE'):
             # If we have a StaticContent, get a StaticContentStream.  Can't manipulate the bytes otherwise.
             if isinstance(content, StaticContent):
-                content = AssetManager.find(loc, as_stream=True)
+                content = contentstore().find(loc, as_stream=True)
 
             header_value = request.META['HTTP_RANGE']
             try:
@@ -367,7 +367,7 @@ def load_asset_from_location(location):
     content = get_cached_content(location)
     if content is None:
         # Not in cache, so just try and load it from the asset manager.
-        content = AssetManager.find(location, as_stream=True)
+        content = contentstore().find(location, as_stream=True)
 
         # Now that we fetched it, let's go ahead and try to cache it. We cap this at 1MB
         # because it's the default for memcached and also we don't want to do too much

openedx/core/lib/courses.py

@@ -9,7 +9,6 @@
 from opaque_keys.edx.locator import CourseKey
 from organizations.models import Organization
 
-from xmodule.assetstore.assetmgr import AssetManager
 from xmodule.contentstore.content import StaticContent
 from xmodule.contentstore.django import contentstore
 from xmodule.modulestore.django import modulestore
@@ -51,7 +50,7 @@ def create_course_image_thumbnail(course, dimensions):
     - dimensions is a tuple of (width, height)
     """
     course_image_asset_key = StaticContent.compute_location(course.id, course.course_image)
-    course_image = AssetManager.find(course_image_asset_key)  # a StaticContent obj
+    course_image = contentstore().find(course_image_asset_key)  # a StaticContent obj
 
     _content, thumb_loc = contentstore().generate_thumbnail(course_image, dimensions=dimensions)
 

openedx/core/lib/xblock_serializer/utils.py

@@ -13,8 +13,8 @@
 from opaque_keys.edx.keys import AssetKey, CourseKey
 
 from common.djangoapps.static_replace import replace_static_urls
-from xmodule.assetstore.assetmgr import AssetManager
 from xmodule.contentstore.content import StaticContent
+from xmodule.contentstore.django import contentstore
 from xmodule.exceptions import NotFoundError
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from xmodule.util.sandboxing import course_code_library_asset_name
@@ -32,7 +32,7 @@ def get_asset_content_from_path(course_key, asset_path):
     """
     try:
         asset_key = StaticContent.get_asset_key_from_path(course_key, asset_path)
-        return AssetManager.find(asset_key)
+        return contentstore().find(asset_key)
     except (ItemNotFoundError, NotFoundError):
         return None
 
@@ -108,7 +108,7 @@ def get_python_lib_zip_if_using(olx: str, course_id: CourseKey) -> StaticFile |
         python_lib_filename = course_code_library_asset_name()
         asset_key = StaticContent.get_asset_key_from_path(course_id, python_lib_filename)
         # Now, it seems like this capa problem uses python_lib.zip - but does it exist in the course?
-        if AssetManager.find(asset_key, throw_on_not_found=False):
+        if contentstore().find(asset_key, throw_on_not_found=False):
             url = '/' + str(StaticContent.compute_location(course_id, python_lib_filename))
             return StaticFile(name=python_lib_filename, url=url, data=None)
     return None
@@ -141,7 +141,7 @@ def get_js_input_files_if_using(olx: str, course_id: CourseKey) -> [StaticFile]:
     if html_file_fullpath:
         html_filename = html_file_fullpath.split('/')[-1]
         asset_key = StaticContent.get_asset_key_from_path(course_id, html_filename)
-        html_file_content = AssetManager.find(asset_key, throw_on_not_found=False)
+        html_file_content = contentstore().find(asset_key, throw_on_not_found=False)
         if html_file_content:
             static_assets = _extract_static_assets(str(html_file_content.data))
             for static_asset in static_assets:

xmodule/assetstore/__init__.py

@@ -1,5 +1,8 @@
 """
 Classes representing asset metadata.
+
+Deprecated: asset metadata in the modulestore is never populated by Studio, and this whole module will be removed.
+Use ``xmodule.contentstore.django.contentstore()`` to access course assets instead.
 """
 
 
@@ -11,7 +14,15 @@
 from lxml import etree
 from opaque_keys.edx.keys import AssetKey, CourseKey  # noqa: F401
 
+from xmodule.util.deprecation import deprecated
+
+DEPRECATION_MESSAGE = (
+    "xmodule.assetstore is deprecated and will be removed. "
+    "Use xmodule.contentstore.django.contentstore() to access course assets instead."
+)
+
 
+@deprecated(DEPRECATION_MESSAGE)
 class AssetMetadata:
     """
     Stores the metadata associated with a particular course asset. The asset metadata gets stored
@@ -229,6 +240,7 @@ def add_all_assets_as_xml(node, assets):
             asset.to_xml(asset_node)
 
 
+@deprecated(DEPRECATION_MESSAGE)
 class CourseAssetsFromStorage:
     """
     Wrapper class for asset metadata lists returned from modulestore storage.

xmodule/assetstore/assetmgr.py

@@ -1,26 +1,27 @@
 """
-Asset Manager
+Very old Asset Manager API.
 
-Interface allowing course asset saving/retrieving.
-Handles:
-  - saving asset in the BlobStore -and- saving asset metadata in course modulestore.
-  - retrieving asset metadata from course modulestore -and- returning URL to asset -or- asset bytes.
-
-Phase 1: Checks to see if an asset's metadata can be found in the course's modulestore.
-    If not found, fails over to access the asset from the contentstore.
-    At first, the asset metadata will never be found, since saving isn't implemented yet.
-Note: Hotfix (PLAT-734) No asset calls find_asset_metadata, and directly accesses from contentstore.
+Has not done anything other than wrap ``contentstore().find(...)`` since 2015.
 
+Deprecated: use ``xmodule.contentstore.django.contentstore().find()`` instead.
 """
 
 from xmodule.contentstore.django import contentstore
+from xmodule.util.deprecation import deprecated
+
+DEPRECATION_MESSAGE = (
+    "AssetManager is deprecated and will be removed. "
+    "Use xmodule.contentstore.django.contentstore().find() instead."
+)
 
 
+@deprecated(DEPRECATION_MESSAGE)
 class AssetManager:
     """
     Manager for saving/loading course assets.
     """
     @staticmethod
+    @deprecated(DEPRECATION_MESSAGE)  # The class decorator only warns on instantiation, so also warn here.
     def find(asset_key, throw_on_not_found=True, as_stream=False):
         """
         Finds course asset in the deprecated contentstore.

xmodule/assetstore/tests/test_asset_xml.py

@@ -5,13 +5,17 @@
 
 import unittest
 
+import pytest
 from lxml import etree
 from opaque_keys.edx.locator import CourseLocator
 from path import Path as path
 
 from xmodule.assetstore import AssetMetadata
 from xmodule.modulestore.tests.test_assetstore import AssetStoreTestData
 
+# These tests exercise the deprecated AssetMetadata class directly.
+pytestmark = pytest.mark.filterwarnings("ignore:xmodule.assetstore is deprecated:DeprecationWarning")
+
 
 class TestAssetXml(unittest.TestCase):
     """

xmodule/contentstore/content.py

@@ -12,7 +12,7 @@
 from opaque_keys.edx.locator import AssetLocator, LibraryLocatorV2
 from PIL import Image
 
-from xmodule.assetstore.assetmgr import AssetManager
+from xmodule.contentstore.django import contentstore
 from xmodule.exceptions import NotFoundError
 from xmodule.modulestore.exceptions import ItemNotFoundError
 
@@ -247,7 +247,7 @@ def get_canonicalized_asset_path(course_key, path, base_url, excluded_exts, enco
         serve_from_cdn = False
         content_digest = None
         try:
-            content = AssetManager.find(asset_key, as_stream=True)
+            content = contentstore().find(asset_key, as_stream=True)
             serve_from_cdn = not getattr(content, "locked", True)
             content_digest = getattr(content, "content_digest", None)
         except (ItemNotFoundError, NotFoundError):

xmodule/contentstore/utils.py

@@ -1,10 +1,12 @@
 # pylint: disable=missing-module-docstring
 
 from xmodule.contentstore.content import StaticContent
+from xmodule.util.deprecation import deprecated
 
 from .django import contentstore
 
 
+@deprecated("trashcan functionality is deprecated.")  # https://github.com/openedx/openedx-platform/issues/39260
 def empty_asset_trashcan(course_locs):
     '''
     This method will hard delete all assets (optionally within a course_id) from the trashcan
@@ -25,6 +27,7 @@ def empty_asset_trashcan(course_locs):
             store.delete(asset['asset_key'])
 
 
+@deprecated("trashcan functionality is deprecated.")  # https://github.com/openedx/openedx-platform/issues/39260
 def restore_asset_from_trashcan(location):
     '''
     This method will restore an asset which got soft deleted and put back in the original course

xmodule/modulestore/__init__.py

@@ -27,6 +27,7 @@
 from openedx.core.lib.json_utils import EdxJSONEncoder  # noqa: F401
 from xmodule.assetstore import AssetMetadata
 from xmodule.errortracker import make_error_tracker
+from xmodule.util.deprecation import deprecated
 from xmodule.util.misc import get_library_or_course_attribute
 
 from .exceptions import InsufficientSpecificationError, InvalidLocationError  # noqa: F401
@@ -585,9 +586,17 @@ def insert_or_update(self, asset_md):
         self.add(metadata_to_insert)
 
 
+ASSET_METADATA_DEPRECATION_MESSAGE = (
+    "Asset metadata in the modulestore is deprecated and will be removed. It is never populated by Studio; "
+    "use xmodule.contentstore.django.contentstore() to access course assets instead."
+)
+
+
 class ModuleStoreAssetBase:
     """
     The methods for accessing assets and their metadata
+
+    The asset metadata methods are deprecated; use ``contentstore()`` to access course assets instead.
     """
     def _find_course_asset(self, asset_key):
         """
@@ -608,6 +617,7 @@ def _find_course_asset(self, asset_key):
 
         return course_assets, idx
 
+    @deprecated(ASSET_METADATA_DEPRECATION_MESSAGE)
     def find_asset_metadata(self, asset_key, **kwargs):
         """
         Find the metadata for a particular course asset.
@@ -627,6 +637,7 @@ def find_asset_metadata(self, asset_key, **kwargs):
         mdata.from_storable(all_assets[asset_idx])
         return mdata
 
+    @deprecated(ASSET_METADATA_DEPRECATION_MESSAGE)
     def get_all_asset_metadata(self, course_key, asset_type, start=0, maxresults=-1, sort=None, **kwargs):  # pylint: disable=unused-argument
         """
         Returns a list of asset metadata for all assets of the given asset_type in the course.
@@ -725,6 +736,7 @@ def _save_assets_by_type(self, course_key, asset_metadata_list, course_assets, u
             all_assets.insert_or_update(asset_md)
         return assets_by_type
 
+    @deprecated(ASSET_METADATA_DEPRECATION_MESSAGE)
     def save_asset_metadata(self, asset_metadata, user_id, import_only):
         """
         Saves the asset metadata for a particular course's asset.
@@ -739,6 +751,7 @@ def save_asset_metadata(self, asset_metadata, user_id, import_only):
         """
         raise NotImplementedError()
 
+    @deprecated(ASSET_METADATA_DEPRECATION_MESSAGE)
     def save_asset_metadata_list(self, asset_metadata_list, user_id, import_only):
         """
         Saves a list of asset metadata for a particular course's asset.
@@ -753,18 +766,21 @@ def save_asset_metadata_list(self, asset_metadata_list, user_id, import_only):
         """
         raise NotImplementedError()
 
+    @deprecated(ASSET_METADATA_DEPRECATION_MESSAGE)
     def set_asset_metadata_attrs(self, asset_key, attrs, user_id):
         """
         Base method to over-ride in modulestore.
         """
         raise NotImplementedError()
 
+    @deprecated(ASSET_METADATA_DEPRECATION_MESSAGE)
     def delete_asset_metadata(self, asset_key, user_id):
         """
         Base method to over-ride in modulestore.
         """
         raise NotImplementedError()
 
+    @deprecated(ASSET_METADATA_DEPRECATION_MESSAGE)
     def set_asset_metadata_attr(self, asset_key, attr, value, user_id):
         """
         Add/set the given attr on the asset at the given location. Value can be any type which pymongo accepts.
@@ -781,6 +797,7 @@ def set_asset_metadata_attr(self, asset_key, attr, value, user_id):
         """
         return self.set_asset_metadata_attrs(asset_key, {attr: value}, user_id)
 
+    @deprecated(ASSET_METADATA_DEPRECATION_MESSAGE)
     def copy_all_asset_metadata(self, source_course_key, dest_course_key, user_id):
         """
         Copy all the course assets from source_course_key to dest_course_key.