From 364e534cf82f2a7bcda56ed4e6760fa575262ddc Mon Sep 17 00:00:00 2001 From: asadazam93 Date: Mon, 24 Feb 2020 16:49:03 +0500 Subject: [PATCH] Fixed duplicate asset on creating rerun --- .../lib/xmodule/xmodule/contentstore/mongo.py | 38 +++++++++++++------ .../modulestore/tests/test_contentstore.py | 12 ++++++ 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/common/lib/xmodule/xmodule/contentstore/mongo.py b/common/lib/xmodule/xmodule/contentstore/mongo.py index 4bdbf163fd..d25ff3596e 100644 --- a/common/lib/xmodule/xmodule/contentstore/mongo.py +++ b/common/lib/xmodule/xmodule/contentstore/mongo.py @@ -11,7 +11,7 @@ import pymongo import six from bson.son import SON from fs.osfs import OSFS -from gridfs.errors import NoFile +from gridfs.errors import NoFile, FileExists from mongodb_proxy import autoretry_read from opaque_keys.edx.keys import AssetKey @@ -431,18 +431,32 @@ class MongoContentStore(ContentStore): asset_id = six.text_type( dest_course_key.make_asset_key(asset_key['category'], asset_key['name']).for_branch(None) ) + try: + self.create_asset(source_content, asset_id, asset, asset_key) + except FileExists: + self.fs.delete(file_id=asset_id) + self.create_asset(source_content, asset_id, asset, asset_key) - self.fs.put( - source_content.read(), - _id=asset_id, filename=asset['filename'], content_type=asset['contentType'], - displayname=asset['displayname'], content_son=asset_key, - # thumbnail is not technically correct but will be functionally correct as the code - # only looks at the name which is not course relative. - thumbnail_location=asset['thumbnail_location'], - import_path=asset['import_path'], - # getattr b/c caching may mean some pickled instances don't have attr - locked=asset.get('locked', False) - ) + def create_asset(self, source_content, asset_id, asset, asset_key): + """ + Creates a new asset + :param source_content: + :param asset_id: + :param asset: + :param asset_key: + :return: + """ + self.fs.put( + source_content.read(), + _id=asset_id, filename=asset['filename'], content_type=asset['contentType'], + displayname=asset['displayname'], content_son=asset_key, + # thumbnail is not technically correct but will be functionally correct as the code + # only looks at the name which is not course relative. + thumbnail_location=asset['thumbnail_location'], + import_path=asset['import_path'], + # getattr b/c caching may mean some pickled instances don't have attr + locked=asset.get('locked', False) + ) def delete_all_course_assets(self, course_key): """ diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_contentstore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_contentstore.py index bf7ae8bb0e..faf67c977c 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_contentstore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_contentstore.py @@ -199,6 +199,18 @@ class TestContentstore(unittest.TestCase): __, count = self.contentstore.get_all_content_for_course(dest_course) self.assertEqual(count, len(self.course1_files)) + @ddt.data(True, False) + def test_copy_assets_with_duplicates(self, deprecated): + """ + Copy all assets even if the destination has some duplicate assets + """ + self.set_up_assets(deprecated) + dest_course = self.course2_key + self.contentstore.copy_all_course_assets(self.course1_key, dest_course) + + __, count = self.contentstore.get_all_content_for_course(dest_course) + self.assertEqual(count, 5) + @ddt.data(True, False) def test_delete_assets(self, deprecated): """