From 4f5496f02cbee559df0239d83c51e7a235fdbcf8 Mon Sep 17 00:00:00 2001 From: Ahtisham Shahid Date: Thu, 14 May 2020 13:16:28 +0500 Subject: [PATCH] Fixed Import error due to pre tag Fixed regex for nested pre tags Updated import export test fixed quality test issue fixed No exception type(s) specified updated tests fixed linter issue fixed linter issue fixed linter issue --- .../views/tests/test_import_export.py | 60 +++++++++++++------ common/lib/xmodule/xmodule/raw_module.py | 25 +++++--- 2 files changed, 57 insertions(+), 28 deletions(-) diff --git a/cms/djangoapps/contentstore/views/tests/test_import_export.py b/cms/djangoapps/contentstore/views/tests/test_import_export.py index 831b9a1e31..17ca9ede21 100644 --- a/cms/djangoapps/contentstore/views/tests/test_import_export.py +++ b/cms/djangoapps/contentstore/views/tests/test_import_export.py @@ -2,7 +2,6 @@ Unit tests for course import and export """ - import copy import json import logging @@ -55,6 +54,7 @@ class ImportEntranceExamTestCase(CourseTestCase, MilestonesTestCaseMixin): """ Unit tests for importing a course with entrance exam """ + def setUp(self): super(ImportEntranceExamTestCase, self).setUp() self.url = reverse_course_url('import_handler', self.course.id) @@ -533,6 +533,7 @@ class ExportTestCase(CourseTestCase): """ Tests for export_handler. """ + def setUp(self): """ Sets up the test course. @@ -788,9 +789,9 @@ class ExportTestCase(CourseTestCase): @patch('contentstore.views.import_export._latest_task_status') @patch('user_tasks.models.UserTaskArtifact.objects.get') def test_export_status_handler_other( - self, - mock_get_user_task_artifact, - mock_latest_task_status, + self, + mock_get_user_task_artifact, + mock_latest_task_status, ): """ Verify that the export status handler generates the correct export path @@ -808,9 +809,9 @@ class ExportTestCase(CourseTestCase): @patch('contentstore.views.import_export._latest_task_status') @patch('user_tasks.models.UserTaskArtifact.objects.get') def test_export_status_handler_s3( - self, - mock_get_user_task_artifact, - mock_latest_task_status, + self, + mock_get_user_task_artifact, + mock_latest_task_status, ): """ Verify that the export status handler generates the correct export path @@ -828,9 +829,9 @@ class ExportTestCase(CourseTestCase): @patch('contentstore.views.import_export._latest_task_status') @patch('user_tasks.models.UserTaskArtifact.objects.get') def test_export_status_handler_filesystem( - self, - mock_get_user_task_artifact, - mock_latest_task_status, + self, + mock_get_user_task_artifact, + mock_latest_task_status, ): """ Verify that the export status handler generates the correct export path @@ -849,6 +850,7 @@ class TestLibraryImportExport(CourseTestCase): """ Tests for importing content libraries from XML and exporting them to XML. """ + def setUp(self): super(TestLibraryImportExport, self).setUp() self.export_dir = tempfile.mkdtemp() @@ -906,6 +908,7 @@ class TestCourseExportImport(LibraryTestCase): """ Tests for importing after exporting the course containing content libraries from XML. """ + def setUp(self): super(TestCourseExportImport, self).setUp() self.export_dir = tempfile.mkdtemp() @@ -1022,18 +1025,20 @@ class TestCourseExportImport(LibraryTestCase): ) +@ddt.ddt @override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE) class TestCourseExportImportProblem(CourseTestCase): """ Tests for importing after exporting the course containing problem with pre tags from XML. """ + def setUp(self): super(TestCourseExportImportProblem, self).setUp() self.export_dir = tempfile.mkdtemp() self.source_course = CourseFactory.create(default_store=ModuleStoreEnum.Type.split) self.addCleanup(shutil.rmtree, self.export_dir, ignore_errors=True) - def _setup_source_course_with_problem_content(self, publish_item=False): + def _setup_source_course_with_problem_content(self, data, publish_item=False): """ Sets up course with problem content. """ @@ -1058,8 +1063,7 @@ class TestCourseExportImportProblem(CourseTestCase): category='problem', display_name='Test Problem', publish_item=publish_item, - data='
x=10 print("hello \n")
' - '', + data=data, ) def get_problem_content(self, block_location): @@ -1071,21 +1075,39 @@ class TestCourseExportImportProblem(CourseTestCase): return self.get_problem_content(self.store.get_item(block_location).children[0]) - def assert_problem_definition(self, course_location): + def assert_problem_definition(self, course_location, expected_problem_content): """ Asserts that problems' data is as expected with pre-tag content maintained. """ - expected_problem_content = '\n 
\n    x=10 print("hello \n")\n
\n ' \ - '\n\n' problem_content = self.get_problem_content(course_location) self.assertEqual(expected_problem_content, problem_content) - def test_problem_content_on_course_export_import(self): + @ddt.data( + [ + '
x=10 print("hello \n")
' + '
x=10 print("hello \n")
' + '', + + '\n 
\n    x=10 print("hello \n")\n
\n 
\n''
\n      
\n '
+            '  x=10 print("hello \n")\n''      
\n    
\n
\n ' + '\n\n ' + ], + [ + '
x=10 print("hello \n")
' + '', + + '\n 
\n    x=10 print("hello \n")\n
\n ' + '\n\n' + ] + ) + @ddt.unpack + def test_problem_content_on_course_export_import(self, problem_data, expected_problem_content): """ Verify that problem content in destination matches expected problem content, specifically concerned with pre tag data with problem. """ - self._setup_source_course_with_problem_content() + # import pdb;pdb.set_trace() + self._setup_source_course_with_problem_content(problem_data) dest_course = CourseFactory.create(default_store=ModuleStoreEnum.Type.split) @@ -1109,4 +1131,4 @@ class TestCourseExportImportProblem(CourseTestCase): create_if_not_present=True, ) - self.assert_problem_definition(dest_course.location) + self.assert_problem_definition(dest_course.location, expected_problem_content) diff --git a/common/lib/xmodule/xmodule/raw_module.py b/common/lib/xmodule/xmodule/raw_module.py index 4d80245357..9f5b3c9b56 100644 --- a/common/lib/xmodule/xmodule/raw_module.py +++ b/common/lib/xmodule/xmodule/raw_module.py @@ -1,5 +1,3 @@ - - import logging import re @@ -12,7 +10,7 @@ from .exceptions import SerializationError log = logging.getLogger(__name__) -PRE_TAG_REGEX = re.compile(r'
[\s\S]*?
') +PRE_TAG_REGEX = re.compile(r']*>(?:(?=([^<]+))\1|<(?!pre\b[^>]*>))*?') class RawMixin(object): @@ -25,12 +23,21 @@ class RawMixin(object): @classmethod def definition_from_xml(cls, xml_object, system): - pre_tag_data = [etree.tostring(pre_tag_info) for pre_tag_info in xml_object.findall('.//pre')] - data = etree.tostring(xml_object, pretty_print=True, encoding='unicode') - if pre_tag_data: - for index, pre_tag in enumerate(re.findall(PRE_TAG_REGEX, data)): - data = re.sub(pre_tag, pre_tag_data[index].decode(), data) - return {'data': data}, [] + try: + data = etree.tostring(xml_object, pretty_print=True, encoding='unicode') + pre_tag_data = [] + for pre_tag_info in xml_object.findall('.//pre'): + if len(pre_tag_info.findall('.//pre')) == 0: + pre_tag_data.append(etree.tostring(pre_tag_info)) + + if pre_tag_data: + matches = re.finditer(PRE_TAG_REGEX, data) + for match_num, match in enumerate(matches): + data = re.sub(match.group(), pre_tag_data[match_num].decode(), data) + etree.XML(data) # it just checks if generated string is valid xml + return {'data': data}, [] + except etree.XMLSyntaxError: + return {'data': etree.tostring(xml_object, pretty_print=True, encoding='unicode')}, [] def definition_to_xml(self, resource_fs): """