diff --git a/common/lib/xmodule/xmodule/abtest_module.py b/common/lib/xmodule/xmodule/abtest_module.py index 8fe9c67554..325d98cc5f 100644 --- a/common/lib/xmodule/xmodule/abtest_module.py +++ b/common/lib/xmodule/xmodule/abtest_module.py @@ -149,7 +149,7 @@ class ABTestDescriptor(ABTestFields, RawDescriptor, XmlDescriptor): for child_loc in group: child = self.system.load_item(child_loc) - group_elem.append(etree.fromstring(child.export_to_xml(resource_fs))) + self.runtime.add_block_as_child_node(child, group_elem) return xml_object diff --git a/common/lib/xmodule/xmodule/conditional_module.py b/common/lib/xmodule/xmodule/conditional_module.py index e296af5aa5..58f7dc5bce 100644 --- a/common/lib/xmodule/xmodule/conditional_module.py +++ b/common/lib/xmodule/xmodule/conditional_module.py @@ -222,9 +222,9 @@ class ConditionalDescriptor(ConditionalFields, SequenceDescriptor): show_tag_list = [] for child in xml_object: if child.tag == 'show': - location = ConditionalDescriptor.parse_sources(child, system) - children.extend(location) - show_tag_list.extend(location.url()) # pylint: disable=no-member + locations = ConditionalDescriptor.parse_sources(child, system) + children.extend(locations) + show_tag_list.extend(location.url() for location in locations) # pylint: disable=no-member else: try: descriptor = system.process_xml(etree.tostring(child)) @@ -244,6 +244,5 @@ class ConditionalDescriptor(ConditionalFields, SequenceDescriptor): tag_name='show', sources=location) xml_object.append(etree.fromstring(show_str)) else: - xml_object.append( - etree.fromstring(child.export_to_xml(resource_fs))) + self.runtime.add_block_as_child_node(child, xml_object) return xml_object diff --git a/common/lib/xmodule/xmodule/crowdsource_hinter.py b/common/lib/xmodule/xmodule/crowdsource_hinter.py index d2c6224112..0ccd7d6613 100644 --- a/common/lib/xmodule/xmodule/crowdsource_hinter.py +++ b/common/lib/xmodule/xmodule/crowdsource_hinter.py @@ -400,6 +400,5 @@ class CrowdsourceHinterDescriptor(CrowdsourceHinterFields, RawDescriptor): def definition_to_xml(self, resource_fs): xml_object = etree.Element('crowdsource_hinter') for child in self.get_children(): - xml_object.append( - etree.fromstring(child.export_to_xml(resource_fs))) + self.runtime.add_block_as_child_node(child, xml_object) return xml_object diff --git a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py index e9360a53de..c78b8d240f 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py @@ -3,6 +3,7 @@ Methods for exporting course data to XML """ import logging +import lxml.etree from xmodule.modulestore import Location from xmodule.modulestore.inheritance import own_metadata from fs.osfs import OSFS @@ -57,11 +58,13 @@ def export_to_xml(modulestore, contentstore, course_location, root_dir, course_d course = modulestore.get_course(course_id) fs = OSFS(root_dir) - export_fs = fs.makeopendir(course_dir) + export_fs = course.runtime.export_fs = fs.makeopendir(course_dir) + + root = lxml.etree.Element('unknown') + course.add_xml_to_node(root) - xml = course.export_to_xml(export_fs) with export_fs.open('course.xml', 'w') as course_xml: - course_xml.write(xml) + lxml.etree.ElementTree(root).write(course_xml) # export the static assets policies_dir = export_fs.makeopendir('policies') @@ -112,7 +115,9 @@ def export_to_xml(modulestore, contentstore, course_location, root_dir, course_d sequential = modulestore.get_item(Location(parent_locs[0])) index = sequential.children.index(draft_vertical.location.url()) draft_vertical.xml_attributes['index_in_children_list'] = str(index) - draft_vertical.export_to_xml(draft_course_dir) + draft_vertical.runtime.export_fs = draft_course_dir + node = lxml.etree.Element('unknown') + draft_vertical.add_xml_to_node(node) def export_extra_content(export_fs, modulestore, course_id, course_location, category_type, dirname, file_suffix=''): diff --git a/common/lib/xmodule/xmodule/randomize_module.py b/common/lib/xmodule/xmodule/randomize_module.py index 56b7c079aa..30febac9b2 100644 --- a/common/lib/xmodule/xmodule/randomize_module.py +++ b/common/lib/xmodule/xmodule/randomize_module.py @@ -100,8 +100,7 @@ class RandomizeDescriptor(RandomizeFields, SequenceDescriptor): xml_object = etree.Element('randomize') for child in self.get_children(): - xml_object.append( - etree.fromstring(child.export_to_xml(resource_fs))) + self.runtime.add_block_as_child_node(child, xml_object) return xml_object def has_dynamic_children(self): diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 4387d6286b..51aa2d4261 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -150,6 +150,5 @@ class SequenceDescriptor(SequenceFields, MakoModuleDescriptor, XmlDescriptor): def definition_to_xml(self, resource_fs): xml_object = etree.Element('sequential') for child in self.get_children(): - xml_object.append( - etree.fromstring(child.export_to_xml(resource_fs))) + self.runtime.add_block_as_child_node(child, xml_object) return xml_object diff --git a/common/lib/xmodule/xmodule/tests/test_export.py b/common/lib/xmodule/xmodule/tests/test_export.py index a1fe66af39..dacc433bee 100644 --- a/common/lib/xmodule/xmodule/tests/test_export.py +++ b/common/lib/xmodule/xmodule/tests/test_export.py @@ -2,19 +2,25 @@ Tests of XML export """ -from datetime import datetime, timedelta, tzinfo -from tempfile import mkdtemp -import unittest -import shutil -from textwrap import dedent +import ddt +import lxml.etree import mock - +import os import pytz +import shutil +import tarfile +import unittest +import uuid + +from datetime import datetime, timedelta, tzinfo from fs.osfs import OSFS from path import path -import uuid -import tarfile -import os +from tempfile import mkdtemp +from textwrap import dedent + +from xblock.core import XBlock +from xblock.fields import String, Scope, Integer +from xblock.test.tools import blocks_are_equivalent from xmodule.modulestore import Location from xmodule.modulestore.xml import XMLModuleStore @@ -23,6 +29,7 @@ from xmodule.modulestore.xml_exporter import ( ) from xmodule.tests import DATA_DIR from xmodule.tests.helpers import directories_equal +from xmodule.x_module import XModuleMixin def strip_filenames(descriptor): @@ -43,6 +50,16 @@ def strip_filenames(descriptor): descriptor.save() +class PureXBlock(XBlock): + + """Class for testing pure XBlocks.""" + + has_children = True + field1 = String(default="something", scope=Scope.user_state) + field2 = Integer(scope=Scope.user_state) + + +@ddt.ddt class RoundTripTestCase(unittest.TestCase): """ Check that our test courses roundtrip properly. @@ -51,8 +68,25 @@ class RoundTripTestCase(unittest.TestCase): Thus we make sure that export and import work properly. """ + + def setUp(self): + self.maxDiff = None + self.temp_dir = mkdtemp() + self.addCleanup(shutil.rmtree, self.temp_dir) + @mock.patch('xmodule.course_module.requests.get') - def check_export_roundtrip(self, data_dir, course_dir, mock_get): + @ddt.data( + "toy", + "simple", + "conditional_and_poll", + "self_assessment", + "graphic_slider_tool", + "test_exam_registration", + "word_cloud", + "pure_xblock", + ) + @XBlock.register_temp_plugin(PureXBlock, 'pure') + def test_export_roundtrip(self, course_dir, mock_get): # Patch network calls to retrieve the textbook TOC mock_get.return_value.text = dedent(""" @@ -64,11 +98,11 @@ class RoundTripTestCase(unittest.TestCase): root_dir = path(self.temp_dir) print("Copying test course to temp dir {0}".format(root_dir)) - data_dir = path(data_dir) + data_dir = path(DATA_DIR) shutil.copytree(data_dir / course_dir, root_dir / course_dir) print("Starting import") - initial_import = XMLModuleStore(root_dir, course_dirs=[course_dir]) + initial_import = XMLModuleStore(root_dir, course_dirs=[course_dir], xblock_mixins=(XModuleMixin,)) courses = initial_import.get_courses() self.assertEquals(len(courses), 1) @@ -78,14 +112,15 @@ class RoundTripTestCase(unittest.TestCase): # will still be there. print("Starting export") fs = OSFS(root_dir) - export_fs = fs.makeopendir(course_dir) + initial_course.runtime.export_fs = fs.makeopendir(course_dir) + root = lxml.etree.Element('root') - xml = initial_course.export_to_xml(export_fs) - with export_fs.open('course.xml', 'w') as course_xml: - course_xml.write(xml) + initial_course.add_xml_to_node(root) + with initial_course.runtime.export_fs.open('course.xml', 'w') as course_xml: + lxml.etree.ElementTree(root).write(course_xml) print("Starting second import") - second_import = XMLModuleStore(root_dir, course_dirs=[course_dir]) + second_import = XMLModuleStore(root_dir, course_dirs=[course_dir], xblock_mixins=(XModuleMixin,)) courses2 = second_import.get_courses() self.assertEquals(len(courses2), 1) @@ -98,48 +133,24 @@ class RoundTripTestCase(unittest.TestCase): strip_filenames(initial_course) strip_filenames(exported_course) - self.assertEquals(initial_course, exported_course) + self.assertTrue(blocks_are_equivalent(initial_course, exported_course)) self.assertEquals(initial_course.id, exported_course.id) course_id = initial_course.id print("Checking key equality") - self.assertEquals(sorted(initial_import.modules[course_id].keys()), - sorted(second_import.modules[course_id].keys())) + self.assertItemsEqual( + initial_import.modules[course_id].keys(), + second_import.modules[course_id].keys() + ) print("Checking module equality") for location in initial_import.modules[course_id].keys(): print("Checking", location) - self.assertEquals(initial_import.modules[course_id][location], - second_import.modules[course_id][location]) + self.assertTrue(blocks_are_equivalent( + initial_import.modules[course_id][location], + second_import.modules[course_id][location] + )) - def setUp(self): - self.maxDiff = None - self.temp_dir = mkdtemp() - self.addCleanup(shutil.rmtree, self.temp_dir) - - def test_toy_roundtrip(self): - self.check_export_roundtrip(DATA_DIR, "toy") - - def test_simple_roundtrip(self): - self.check_export_roundtrip(DATA_DIR, "simple") - - def test_conditional_and_poll_roundtrip(self): - self.check_export_roundtrip(DATA_DIR, "conditional_and_poll") - - def test_selfassessment_roundtrip(self): - #Test selfassessment xmodule to see if it exports correctly - self.check_export_roundtrip(DATA_DIR, "self_assessment") - - def test_graphicslidertool_roundtrip(self): - #Test graphicslidertool xmodule to see if it exports correctly - self.check_export_roundtrip(DATA_DIR, "graphic_slider_tool") - - def test_exam_registration_roundtrip(self): - # Test exam_registration xmodule to see if it exports correctly - self.check_export_roundtrip(DATA_DIR, "test_exam_registration") - - def test_word_cloud_roundtrip(self): - self.check_export_roundtrip(DATA_DIR, "word_cloud") class TestEdxJsonEncoder(unittest.TestCase): diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py index 12f889d897..0f0aa80145 100644 --- a/common/lib/xmodule/xmodule/tests/test_import.py +++ b/common/lib/xmodule/xmodule/tests/test_import.py @@ -158,9 +158,9 @@ class ImportTestCase(BaseCourseTestCase): system = self.get_system() descriptor = system.process_xml(bad_xml) - resource_fs = None - tag_xml = descriptor.export_to_xml(resource_fs) - re_import_descriptor = system.process_xml(tag_xml) + node = etree.Element('unknown') + descriptor.add_xml_to_node(node) + re_import_descriptor = system.process_xml(etree.tostring(node)) self.assertEqual(re_import_descriptor.__class__.__name__, 'ErrorDescriptorWithMixins') @@ -182,12 +182,11 @@ class ImportTestCase(BaseCourseTestCase): descriptor = system.process_xml(xml_str_in) # export it - resource_fs = None - xml_str_out = descriptor.export_to_xml(resource_fs) + node = etree.Element('unknown') + descriptor.add_xml_to_node(node) # Now make sure the exported xml is a sequential - xml_out = etree.fromstring(xml_str_out) - self.assertEqual(xml_out.tag, 'sequential') + self.assertEqual(node.tag, 'sequential') def test_metadata_import_export(self): """Two checks: @@ -221,19 +220,19 @@ class ImportTestCase(BaseCourseTestCase): ) # Now export and check things - resource_fs = MemoryFS() - exported_xml = descriptor.export_to_xml(resource_fs) + descriptor.runtime.export_fs = MemoryFS() + node = etree.Element('unknown') + descriptor.add_xml_to_node(node) # Check that the exported xml is just a pointer - print("Exported xml:", exported_xml) - pointer = etree.fromstring(exported_xml) - self.assertTrue(is_pointer_tag(pointer)) + print("Exported xml:", etree.tostring(node)) + self.assertTrue(is_pointer_tag(node)) # but it's a special case course pointer - self.assertEqual(pointer.attrib['course'], COURSE) - self.assertEqual(pointer.attrib['org'], ORG) + self.assertEqual(node.attrib['course'], COURSE) + self.assertEqual(node.attrib['org'], ORG) # Does the course still have unicorns? - with resource_fs.open('course/{url_name}.xml'.format(url_name=url_name)) as f: + with descriptor.runtime.export_fs.open('course/{url_name}.xml'.format(url_name=url_name)) as f: course_xml = etree.fromstring(f.read()) self.assertEqual(course_xml.attrib['unicorn'], 'purple') @@ -247,7 +246,7 @@ class ImportTestCase(BaseCourseTestCase): # Does the chapter tag now have a due attribute? # hardcoded path to child - with resource_fs.open('chapter/ch.xml') as f: + with descriptor.runtime.export_fs.open('chapter/ch.xml') as f: chapter_xml = etree.fromstring(f.read()) self.assertEqual(chapter_xml.tag, 'chapter') self.assertFalse('due' in chapter_xml.attrib) diff --git a/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py b/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py index a5247b97de..53d14d6890 100644 --- a/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py +++ b/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py @@ -6,16 +6,27 @@ functionality # pylint: disable=protected-access import webob -from nose.tools import assert_equal, assert_is_instance # pylint: disable=E0611 -from unittest.case import SkipTest +import ddt +from factory import ( + BUILD_STRATEGY, + Factory, + lazy_attribute, + LazyAttributeSequence, + post_generation, + SubFactory, + use_strategy, +) +from fs.memoryfs import MemoryFS +from lxml import etree from mock import Mock +from unittest.case import SkipTest, TestCase from xblock.field_data import DictFieldData from xblock.fields import ScopeIds from xmodule.modulestore import Location -from xmodule.x_module import ModuleSystem, XModule, XModuleDescriptor +from xmodule.x_module import ModuleSystem, XModule, XModuleDescriptor, DescriptorSystem from xmodule.mako_module import MakoDescriptorSystem from xmodule.annotatable_module import AnnotatableDescriptor from xmodule.capa_module import CapaDescriptor @@ -37,30 +48,37 @@ from xmodule.vertical_module import VerticalDescriptor from xmodule.wrapper_module import WrapperDescriptor from xmodule.tests import get_test_descriptor_system, get_test_system -LEAF_XMODULES = ( - AnnotatableDescriptor, - CapaDescriptor, - CombinedOpenEndedDescriptor, - DiscussionDescriptor, - GraphicalSliderToolDescriptor, - HtmlDescriptor, - PeerGradingDescriptor, - PollDescriptor, - WordCloudDescriptor, + +# A dictionary that maps specific XModuleDescriptor classes without children +# to a list of sample field values to test with. +# TODO: Add more types of sample data +LEAF_XMODULES = { + AnnotatableDescriptor: [{}], + CapaDescriptor: [{}], + CombinedOpenEndedDescriptor: [{}], + DiscussionDescriptor: [{}], + GraphicalSliderToolDescriptor: [{}], + HtmlDescriptor: [{}], + PeerGradingDescriptor: [{}], + PollDescriptor: [{'display_name': 'Poll Display Name'}], + WordCloudDescriptor: [{}], # This is being excluded because it has dependencies on django #VideoDescriptor, -) +} -CONTAINER_XMODULES = ( - ConditionalDescriptor, - CourseDescriptor, - CrowdsourceHinterDescriptor, - RandomizeDescriptor, - SequenceDescriptor, - VerticalDescriptor, - WrapperDescriptor, -) +# A dictionary that maps specific XModuleDescriptor classes with children +# to a list of sample field values to test with. +# TODO: Add more types of sample data +CONTAINER_XMODULES = { + ConditionalDescriptor: [{}], + CourseDescriptor: [{}], + CrowdsourceHinterDescriptor: [{}], + RandomizeDescriptor: [{}], + SequenceDescriptor: [{}], + VerticalDescriptor: [{}], + WrapperDescriptor: [{}], +} # These modules are editable in studio yet NOT_STUDIO_EDITABLE = ( @@ -69,189 +87,213 @@ NOT_STUDIO_EDITABLE = ( PollDescriptor ) -class TestXBlockWrapper(object): - """Helper methods used in test case classes below.""" - @property - def leaf_module_runtime(self): - return get_test_system() +def flatten(class_dict): + """ + Flatten a dict from cls -> [fields, ...] and yields values of the form (cls, fields) + for each entry in the dictionary value. + """ + for cls, fields_list in class_dict.items(): + for fields in fields_list: + yield (cls, fields) - def leaf_descriptor(self, descriptor_cls): - location = Location('i4x://org/course/category/name') - runtime = get_test_descriptor_system() - return runtime.construct_xblock_from_class( - descriptor_cls, - ScopeIds(None, descriptor_cls.__name__, location, location), - DictFieldData({}), - ) - def leaf_module(self, descriptor_cls): - """Returns a descriptor that is ready to proxy as an xmodule""" - descriptor = self.leaf_descriptor(descriptor_cls) - descriptor.xmodule_runtime = self.leaf_module_runtime - return descriptor +@use_strategy(BUILD_STRATEGY) +class ModuleSystemFactory(Factory): + FACTORY_FOR = ModuleSystem - def container_module_runtime(self, depth): - runtime = self.leaf_module_runtime + @classmethod + def _build(cls, target_class, *args, **kwargs): + return get_test_system(*args, **kwargs) + + +@use_strategy(BUILD_STRATEGY) +class DescriptorSystemFactory(Factory): + FACTORY_FOR = DescriptorSystem + + @classmethod + def _build(cls, target_class, *args, **kwargs): + return get_test_descriptor_system(*args, **kwargs) + + +class LeafModuleRuntimeFactory(ModuleSystemFactory): + pass + + +class ContainerModuleRuntimeFactory(ModuleSystemFactory): + @post_generation + def depth(self, create, depth, **kwargs): if depth == 0: - runtime.get_module.side_effect = lambda x: self.leaf_module(HtmlDescriptor) + self.get_module.side_effect = lambda x: LeafModuleFactory(descriptor_cls=HtmlDescriptor) else: - runtime.get_module.side_effect = lambda x: self.container_module(VerticalDescriptor, depth - 1) - runtime.position = 2 - return runtime + self.get_module.side_effect = lambda x: ContainerModuleFactory(descriptor_cls=VerticalDescriptor, depth=depth-1) - def container_descriptor(self, descriptor_cls, depth): - """Return an instance of `descriptor_cls` with `depth` levels of children""" - location = Location('i4x://org/course/category/name') - runtime = get_test_descriptor_system() + @post_generation + def position(self, create, position=2, **kwargs): + self.position = position + +class ContainerDescriptorRuntimeFactory(DescriptorSystemFactory): + @post_generation + def depth(self, create, depth, **kwargs): if depth == 0: - runtime.load_item.side_effect = lambda x: self.leaf_module(HtmlDescriptor) + self.load_item.side_effect = lambda x: LeafModuleFactory(descriptor_cls=HtmlDescriptor) else: - runtime.load_item.side_effect = lambda x: self.container_module(VerticalDescriptor, depth - 1) + self.load_item.side_effect = lambda x: ContainerModuleFactory(descriptor_cls=VerticalDescriptor, depth=depth-1) - return runtime.construct_xblock_from_class( - descriptor_cls, - ScopeIds(None, descriptor_cls.__name__, location, location), - DictFieldData({ - 'children': range(3) - }), + @post_generation + def position(self, create, position=2, **kwargs): + self.position = position + + +@use_strategy(BUILD_STRATEGY) +class LeafDescriptorFactory(Factory): + FACTORY_FOR = XModuleDescriptor + + runtime = SubFactory(DescriptorSystemFactory) + url_name = LazyAttributeSequence('{.block_type}_{}'.format) + + @lazy_attribute + def location(self): + return Location('i4x://org/course/category/{}'.format(self.url_name)) + + @lazy_attribute + def block_type(self): + return self.descriptor_cls.__name__ + + @lazy_attribute + def definition_id(self): + return self.location + + @lazy_attribute + def usage_id(self): + return self.location + + @classmethod + def _build(cls, target_class, *args, **kwargs): + runtime = kwargs.pop('runtime') + desc_cls = kwargs.pop('descriptor_cls') + block_type = kwargs.pop('block_type') + def_id = kwargs.pop('definition_id') + usage_id = kwargs.pop('usage_id') + + block = runtime.construct_xblock_from_class( + desc_cls, + ScopeIds(None, block_type, def_id, usage_id), + DictFieldData(dict(**kwargs)) ) + block.save() + return block - def container_module(self, descriptor_cls, depth): - """Returns a descriptor that is ready to proxy as an xmodule""" - descriptor = self.container_descriptor(descriptor_cls, depth) - descriptor.xmodule_runtime = self.container_module_runtime(depth) - return descriptor -class TestStudentView(TestXBlockWrapper): +class LeafModuleFactory(LeafDescriptorFactory): + + @post_generation + def xmodule_runtime(self, create, xmodule_runtime, **kwargs): + if xmodule_runtime is None: + xmodule_runtime = LeafModuleRuntimeFactory() + + self.xmodule_runtime = xmodule_runtime + + +class ContainerDescriptorFactory(LeafDescriptorFactory): + runtime = SubFactory(ContainerDescriptorRuntimeFactory) + children = range(3) + + +class ContainerModuleFactory(LeafModuleFactory): + @lazy_attribute + def xmodule_runtime(self): + return ContainerModuleRuntimeFactory(depth=self.depth) + + +@ddt.ddt +class XBlockWrapperTestMixin(object): + """ + This is a mixin for building tests of the implementation of the XBlock + api by wrapping XModule native functions. + + You can creat an actual test case by inheriting from this class and UnitTest, + and implement skip_if_invalid and check_property. + """ + + def skip_if_invalid(self, descriptor_cls): + """ + Raise SkipTest if this descriptor_cls shouldn't be tested. + """ + pass + + def check_property(self, descriptor): + raise SkipTest("check_property not defined") # Test that for all of the leaf XModule Descriptors, - # the student_view wrapper returns the same thing in its content - # as get_html returns - def test_student_view_leaf_node(self): - for descriptor_cls in LEAF_XMODULES: - yield self.check_student_view_leaf_node, descriptor_cls + # the test property holds + @ddt.data(*flatten(LEAF_XMODULES)) + def test_leaf_node(self, cls_and_fields): + descriptor_cls, fields = cls_and_fields + self.skip_if_invalid(descriptor_cls) + descriptor = LeafModuleFactory(descriptor_cls=descriptor_cls, **fields) + self.check_property(descriptor) - # Check that when an xmodule is instantiated from descriptor_cls - # it generates the same thing from student_view that it does from get_html - def check_student_view_leaf_node(self, descriptor_cls): + # Test that when an xmodule is generated from descriptor_cls + # with only xmodule children, the test property holds + @ddt.data(*flatten(CONTAINER_XMODULES)) + def test_container_node_xmodules_only(self, cls_and_fields): + descriptor_cls, fields = cls_and_fields + self.skip_if_invalid(descriptor_cls) + descriptor = ContainerModuleFactory(descriptor_cls=descriptor_cls, depth=2, **fields) + self.check_property(descriptor) - if descriptor_cls.module_class.student_view != XModule.student_view: - raise SkipTest(descriptor_cls.__name__ + " implements student_view") - - descriptor = self.leaf_module(descriptor_cls) - assert_equal( - descriptor._xmodule.get_html(), - descriptor.render('student_view').content - ) - - # Test that for all container XModule Descriptors, - # their corresponding XModule renders the same thing using student_view - # as it does using get_html, under the following conditions: - # a) All of its descendents are xmodules - # b) Some of its descendents are xmodules and some are xblocks - # c) All of its descendents are xblocks - def test_student_view_container_node(self): - for descriptor_cls in CONTAINER_XMODULES: - yield self.check_student_view_container_node_xmodules_only, descriptor_cls - yield self.check_student_view_container_node_mixed, descriptor_cls - yield self.check_student_view_container_node_xblocks_only, descriptor_cls - - # Check that when an xmodule is generated from descriptor_cls - # with only xmodule children, it generates the same html from student_view - # as it does using get_html - def check_student_view_container_node_xmodules_only(self, descriptor_cls): - - if descriptor_cls.module_class.student_view != XModule.student_view: - raise SkipTest(descriptor_cls.__name__ + " implements student_view") - - descriptor = self.container_module(descriptor_cls, 2) - assert_equal( - descriptor._xmodule.get_html(), - descriptor.render('student_view').content - ) - - # Check that when an xmodule is generated from descriptor_cls - # with mixed xmodule and xblock children, it generates the same html from student_view - # as it does using get_html - def check_student_view_container_node_mixed(self, descriptor_cls): + # Test that when an xmodule is generated from descriptor_cls + # with mixed xmodule and xblock children, the test property holds + @ddt.data(*flatten(CONTAINER_XMODULES)) + def test_container_node_mixed(self, cls_and_fields): raise SkipTest("XBlock support in XDescriptor not yet fully implemented") - # Check that when an xmodule is generated from descriptor_cls - # with only xblock children, it generates the same html from student_view - # as it does using get_html - def check_student_view_container_node_xblocks_only(self, descriptor_cls): + # Test that when an xmodule is generated from descriptor_cls + # with only xblock children, the test property holds + @ddt.data(*flatten(CONTAINER_XMODULES)) + def test_container_node_xblocks_only(self, cls_and_fields): raise SkipTest("XBlock support in XModules not yet fully implemented") -class TestStudioView(TestXBlockWrapper): +class TestStudentView(XBlockWrapperTestMixin, TestCase): + """ + This tests that student_view and XModule.get_html produce the same results. + """ + def skip_if_invalid(self, descriptor_cls): + if descriptor_cls.module_class.student_view != XModule.student_view: + raise SkipTest(descriptor_cls.__name__ + " implements student_view") - # Test that for all of the Descriptors listed in LEAF_XMODULES, - # the studio_view wrapper returns the same thing in its content - # as get_html returns - def test_studio_view_leaf_node(self): - for descriptor_cls in LEAF_XMODULES: - yield self.check_studio_view_leaf_node, descriptor_cls + def check_property(self, descriptor): + """ + Assert that both student_view and get_html render the same. + """ + self.assertEqual( + descriptor._xmodule.get_html(), + descriptor.render('student_view').content + ) - # Check that when a descriptor is instantiated from descriptor_cls - # it generates the same thing from studio_view that it does from get_html - def check_studio_view_leaf_node(self, descriptor_cls): + +class TestStudioView(XBlockWrapperTestMixin, TestCase): + """ + This tests that studio_view and XModuleDescriptor.get_html produce the same results + """ + def skip_if_invalid(self, descriptor_cls): if descriptor_cls in NOT_STUDIO_EDITABLE: raise SkipTest(descriptor_cls.__name__ + " is not editable in studio") if descriptor_cls.studio_view != XModuleDescriptor.studio_view: raise SkipTest(descriptor_cls.__name__ + " implements studio_view") - descriptor = self.leaf_descriptor(descriptor_cls) - assert_equal(descriptor.get_html(), descriptor.render('studio_view').content) + def check_property(self, descriptor): + """ + Assert that studio_view and get_html render the same. + """ + self.assertEqual(descriptor.get_html(), descriptor.render('studio_view').content) - # Test that for all of the Descriptors listed in CONTAINER_XMODULES - # render the same thing using studio_view as they do using get_html, under the following conditions: - # a) All of its descendants are xmodules - # b) Some of its descendants are xmodules and some are xblocks - # c) All of its descendants are xblocks - def test_studio_view_container_node(self): - for descriptor_cls in CONTAINER_XMODULES: - yield self.check_studio_view_container_node_xmodules_only, descriptor_cls - yield self.check_studio_view_container_node_mixed, descriptor_cls - yield self.check_studio_view_container_node_xblocks_only, descriptor_cls - - - # Check that when a descriptor is generated from descriptor_cls - # with only xmodule children, it generates the same html from studio_view - # as it does using get_html - def check_studio_view_container_node_xmodules_only(self, descriptor_cls): - if descriptor_cls in NOT_STUDIO_EDITABLE: - raise SkipTest(descriptor_cls.__name__ + "is not editable in studio") - - if descriptor_cls.studio_view != XModuleDescriptor.studio_view: - raise SkipTest(descriptor_cls.__name__ + " implements studio_view") - - descriptor = self.container_descriptor(descriptor_cls, 2) - assert_equal(descriptor.get_html(), descriptor.render('studio_view').content) - - # Check that when a descriptor is generated from descriptor_cls - # with mixed xmodule and xblock children, it generates the same html from studio_view - # as it does using get_html - def check_studio_view_container_node_mixed(self, descriptor_cls): - if descriptor_cls in NOT_STUDIO_EDITABLE: - raise SkipTest(descriptor_cls.__name__ + "is not editable in studio") - - raise SkipTest("XBlock support in XDescriptor not yet fully implemented") - - # Check that when a descriptor is generated from descriptor_cls - # with only xblock children, it generates the same html from studio_view - # as it does using get_html - def check_studio_view_container_node_xblocks_only(self, descriptor_cls): - if descriptor_cls in NOT_STUDIO_EDITABLE: - raise SkipTest(descriptor_cls.__name__ + "is not editable in studio") - - raise SkipTest("XBlock support in XModules not yet fully implemented") - - -class TestXModuleHandler(TestXBlockWrapper): +class TestXModuleHandler(TestCase): """ Tests that the xmodule_handler function correctly wraps handle_ajax """ @@ -271,5 +313,27 @@ class TestXModuleHandler(TestXBlockWrapper): def test_xmodule_handler_return_value(self): response = self.module.xmodule_handler(self.request) - assert_is_instance(response, webob.Response) - assert_equal(response.body, '{}') + self.assertIsInstance(response, webob.Response) + self.assertEqual(response.body, '{}') + + +class TestXmlExport(XBlockWrapperTestMixin, TestCase): + """ + This tests that XModuleDescriptor.export_to_xml and add_xml_to_node produce the same results. + """ + def skip_if_invalid(self, descriptor_cls): + if descriptor_cls.add_xml_to_node != XModuleDescriptor.add_xml_to_node: + raise SkipTest(descriptor_cls.__name__ + " implements add_xml_to_node") + + def check_property(self, descriptor): + xmodule_api_fs = MemoryFS() + xblock_api_fs = MemoryFS() + + descriptor.runtime.export_fs = xblock_api_fs + xblock_node = etree.Element('unknown') + descriptor.add_xml_to_node(xblock_node) + + xmodule_node = etree.fromstring(descriptor.export_to_xml(xmodule_api_fs)) + + self.assertEquals(list(xmodule_api_fs.walk()), list(xblock_api_fs.walk())) + self.assertEquals(etree.tostring(xmodule_node), etree.tostring(xblock_node)) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 9fe208fd04..9acc686208 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -222,6 +222,7 @@ class XModuleMixin(XBlockMixin): for child_loc in self.children: try: child = self.runtime.get_block(child_loc) + child.runtime.export_fs = self.runtime.export_fs except ItemNotFoundError: log.exception(u'Unable to load item {loc}, skipping'.format(loc=child_loc)) continue @@ -685,6 +686,21 @@ class XModuleDescriptor(XModuleMixin, HTMLSnippet, ResourceTemplates, XBlock): """ raise NotImplementedError('Modules must implement from_xml to be parsable from xml') + def add_xml_to_node(self, node): + """ + Export this :class:`XModuleDescriptor` as XML, by setting attributes on the provided + `node`. + """ + xml_string = self.export_to_xml(self.runtime.export_fs) + exported_node = etree.fromstring(xml_string) + node.tag = exported_node.tag + node.text = exported_node.text + node.tail = exported_node.tail + for key, value in exported_node.items(): + node.set(key, value) + + node.extend(list(exported_node)) + def export_to_xml(self, resource_fs): """ Returns an xml string representing this module, and all modules @@ -926,6 +942,9 @@ class DescriptorSystem(ConfigurableFragmentWrapper, Runtime): # pylint: disable """ super(DescriptorSystem, self).__init__(**kwargs) + # This is used by XModules to write out separate files during xml export + self.export_fs = None + self.load_item = load_item self.resources_fs = resources_fs self.error_tracker = error_tracker @@ -996,6 +1015,11 @@ class DescriptorSystem(ConfigurableFragmentWrapper, Runtime): # pylint: disable def publish(self, block, event): raise NotImplementedError("edX Platform doesn't currently implement XBlock publish") + def add_block_as_child_node(self, block, node): + child = etree.SubElement(node, "unknown") + child.set('url_name', block.url_name) + block.add_xml_to_node(child) + class XMLParsingSystem(DescriptorSystem): def __init__(self, process_xml, **kwargs): diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index d4ea3b17ee..565d3597ca 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -363,6 +363,10 @@ class XmlDescriptor(XModuleDescriptor): resource_fs is a pyfilesystem object (from the fs package) """ + # Set up runtime.export_fs so that it's available through future + # uses of the pure xblock add_xml_to_node api + self.runtime.export_fs = resource_fs + # Get the definition xml_object = self.definition_to_xml(resource_fs) self.clean_metadata_from_xml(xml_object) @@ -377,12 +381,11 @@ class XmlDescriptor(XModuleDescriptor): val = serialize_field(self._field_data.get(self, attr)) try: xml_object.set(attr, val) - except Exception, e: + except Exception: logging.exception( - u'Failed to serialize metadata attribute %s with value %s in module %s. This could mean data loss!!! Exception: %s', - attr, val, self.url_name, e + u'Failed to serialize metadata attribute %s with value %s in module %s. This could mean data loss!!!', + attr, val, self.url_name ) - pass for key, value in self.xml_attributes.items(): if key not in self.metadata_to_strip: diff --git a/common/test/data/pure_xblock/course.xml b/common/test/data/pure_xblock/course.xml new file mode 100644 index 0000000000..b3d829ad02 --- /dev/null +++ b/common/test/data/pure_xblock/course.xml @@ -0,0 +1,8 @@ + + + + + + + + \ No newline at end of file diff --git a/lms/djangoapps/courseware/management/commands/clean_xml.py b/lms/djangoapps/courseware/management/commands/clean_xml.py index 45674f66e0..dd61ff36a2 100644 --- a/lms/djangoapps/courseware/management/commands/clean_xml.py +++ b/lms/djangoapps/courseware/management/commands/clean_xml.py @@ -1,3 +1,4 @@ +import lxml.etree import os import sys import traceback @@ -30,9 +31,11 @@ def export(course, export_dir): ' May clobber/confuse things'.format(dir=export_dir)) try: - xml = course.export_to_xml(fs) + course.runtime.export_fs = fs + root = lxml.etree.Element('root') + course.add_xml_to_node(root) with fs.open('course.xml', mode='w') as f: - f.write(xml) + root.write(f) return True except: diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 06358f427e..61ef871263 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -15,7 +15,7 @@ -e git+https://github.com/eventbrite/zendesk.git@d53fe0e81b623f084e91776bcf6369f8b7b63879#egg=zendesk # Our libraries: --e git+https://github.com/edx/XBlock.git@de92d3bf798699a6bbd06b54012ef15934c41ac0#egg=XBlock +-e git+https://github.com/edx/XBlock.git@3830ee50015b460fad63ff3b71f77bf1a2684195#egg=XBlock -e git+https://github.com/edx/codejail.git@e3d98f9455#egg=codejail -e git+https://github.com/edx/diff-cover.git@v0.2.9#egg=diff_cover -e git+https://github.com/edx/js-test-tool.git@v0.1.5#egg=js_test_tool