Commit f4dd5082 by Calen Pennington

Merge pull request #1055 from cpennington/cleanup-xml-hotfix

Clean up after hotfix to fix xml loading for import
parents 4a9d1928 f500b722
......@@ -91,7 +91,6 @@ class ImportTestCase(BaseCourseTestCase):
self.assertNotEqual(descriptor1.location, descriptor2.location)
@unittest.skip('Temporarily disabled')
def test_reimport(self):
'''Make sure an already-exported error xml tag loads properly'''
......
# disable missing docstring
#pylint: disable=C0111
from xmodule.x_module import XModuleFields
from xblock.fields import Scope, String, Dict, Boolean, Integer, Float, Any, List
from xblock.field_data import DictFieldData
from xmodule.fields import Date, Timedelta
from xmodule.xml_module import XmlDescriptor, serialize_field, deserialize_field
import unittest
from nose.tools import assert_equals # pylint: disable=E0611
from mock import Mock
from xmodule.modulestore.inheritance import InheritanceKeyValueStore, InheritanceMixin
from nose.tools import assert_equals, assert_not_equals, assert_true, assert_false, assert_in, assert_not_in # pylint: disable=E0611
from xblock.field_data import DictFieldData
from xblock.fields import Scope, String, Dict, Boolean, Integer, Float, Any, List
from xblock.runtime import DbModel
from xmodule.fields import Date, Timedelta
from xmodule.modulestore.inheritance import InheritanceKeyValueStore, InheritanceMixin
from xmodule.x_module import XModuleFields
from xmodule.xml_module import XmlDescriptor, serialize_field, deserialize_field
from xmodule.course_module import CourseDescriptor
from xmodule.seq_module import SequenceDescriptor
from xmodule.tests import get_test_descriptor_system
from xmodule.tests.xml import XModuleXmlImportTest
from xmodule.tests.xml.factories import CourseFactory, SequenceFactory, ProblemFactory
class CrazyJsonString(String):
......@@ -379,3 +387,78 @@ class TestDeserializeTimedelta(TestDeserialize):
self.assertDeserializeEqual('1 day 12 hours 59 minutes 59 seconds',
'"1 day 12 hours 59 minutes 59 seconds"')
self.assertDeserializeNonString()
class TestXmlAttributes(XModuleXmlImportTest):
def test_unknown_attribute(self):
assert_false(hasattr(CourseDescriptor, 'unknown_attr'))
course = self.process_xml(CourseFactory.build(unknown_attr='value'))
assert_false(hasattr(course, 'unknown_attr'))
assert_equals('value', course.xml_attributes['unknown_attr'])
def test_known_attribute(self):
assert_true(hasattr(CourseDescriptor, 'show_chat'))
course = self.process_xml(CourseFactory.build(show_chat='true'))
assert_true(course.show_chat)
assert_not_in('show_chat', course.xml_attributes)
def test_rerandomize_in_policy(self):
# Rerandomize isn't a basic attribute of Sequence
assert_false(hasattr(SequenceDescriptor, 'rerandomize'))
root = SequenceFactory.build(policy={'rerandomize': 'never'})
ProblemFactory.build(parent=root)
seq = self.process_xml(root)
# Rerandomize is added to the constructed sequence via the InheritanceMixin
assert_equals('never', seq.rerandomize)
# Rerandomize is a known value coming from policy, and shouldn't appear
# in xml_attributes
assert_not_in('rerandomize', seq.xml_attributes)
def test_attempts_in_policy(self):
# attempts isn't a basic attribute of Sequence
assert_false(hasattr(SequenceDescriptor, 'attempts'))
root = SequenceFactory.build(policy={'attempts': '1'})
ProblemFactory.build(parent=root)
seq = self.process_xml(root)
# attempts isn't added to the constructed sequence, because
# it's not in the InheritanceMixin
assert_false(hasattr(seq, 'attempts'))
# attempts is an unknown attribute, so we should include it
# in xml_attributes so that it gets written out (despite the misleading
# name)
assert_in('attempts', seq.xml_attributes)
def test_inheritable_attribute(self):
# days_early_for_beta isn't a basic attribute of Sequence
assert_false(hasattr(SequenceDescriptor, 'days_early_for_beta'))
# days_early_for_beta is added by InheritanceMixin
assert_true(hasattr(InheritanceMixin, 'days_early_for_beta'))
root = SequenceFactory.build(policy={'days_early_for_beta': '2'})
ProblemFactory.build(parent=root)
# InheritanceMixin will be used when processing the XML
assert_in(InheritanceMixin, root.xblock_mixins)
seq = self.process_xml(root)
assert_equals(seq.unmixed_class, SequenceDescriptor)
assert_not_equals(type(seq), SequenceDescriptor)
# days_early_for_beta is added to the constructed sequence, because
# it's in the InheritanceMixin
assert_equals(2, seq.days_early_for_beta)
# days_early_for_beta is a known attribute, so we shouldn't include it
# in xml_attributes
assert_not_in('days_early_for_beta', seq.xml_attributes)
"""
Xml parsing tests for XModules
"""
import pprint
from mock import Mock
from xmodule.x_module import XMLParsingSystem, XModuleDescriptor
from xmodule.mako_module import MakoDescriptorSystem
class InMemorySystem(XMLParsingSystem, MakoDescriptorSystem): # pylint: disable=abstract-method
"""
The simplest possible XMLParsingSystem
"""
def __init__(self, xml_import_data):
self.org = xml_import_data.org
self.course = xml_import_data.course
self.default_class = xml_import_data.default_class
self._descriptors = {}
super(InMemorySystem, self).__init__(
policy=xml_import_data.policy,
process_xml=self.process_xml,
load_item=self.load_item,
error_tracker=Mock(),
resources_fs=xml_import_data.filesystem,
mixins=xml_import_data.xblock_mixins,
render_template=lambda template, context: pprint.pformat((template, context))
)
def process_xml(self, xml): # pylint: disable=method-hidden
"""Parse `xml` as an XModuleDescriptor, and add it to `self._descriptors`"""
descriptor = XModuleDescriptor.load_from_xml(xml, self, self.org, self.course, self.default_class)
self._descriptors[descriptor.location.url()] = descriptor
return descriptor
def load_item(self, location): # pylint: disable=method-hidden
"""Return the descriptor loaded for `location`"""
return self._descriptors[location]
class XModuleXmlImportTest(object):
"""Base class for tests that use basic `XModuleDescriptor.load_from_xml` xml parsing"""
def process_xml(self, xml_import_data):
"""Use the `xml_import_data` to import an :class:`XModuleDescriptor` from xml"""
system = InMemorySystem(xml_import_data)
return system.process_xml(xml_import_data.xml_string)
"""
Factories for generating edXML for testing XModule import
"""
import inspect
from fs.memoryfs import MemoryFS
from factory import Factory, lazy_attribute, post_generation, Sequence
from lxml import etree
from xmodule.modulestore.inheritance import InheritanceMixin
class XmlImportData(object):
"""
Class to capture all of the data needed to actually run an XML import,
so that the Factories have something to generate
"""
def __init__(self, xml_node, xml=None, org=None, course=None,
default_class=None, policy=None,
filesystem=None, parent=None,
xblock_mixins=()):
self._xml_node = xml_node
self._xml_string = xml
self.org = org
self.course = course
self.default_class = default_class
self.filesystem = filesystem
self.xblock_mixins = xblock_mixins
self.parent = parent
if policy is None:
self.policy = {}
else:
self.policy = policy
@property
def xml_string(self):
"""Return the stringified version of the generated xml"""
if self._xml_string is not None:
return self._xml_string
return etree.tostring(self._xml_node)
def __repr__(self):
return u"XmlImportData{!r}".format((
self._xml_node, self._xml_string, self.org,
self.course, self.default_class, self.policy,
self.filesystem, self.parent, self.xblock_mixins
))
# Extract all argument names used to construct XmlImportData objects,
# so that the factory doesn't treat them as XML attributes
XML_IMPORT_ARGS = inspect.getargspec(XmlImportData.__init__).args
class XmlImportFactory(Factory):
"""
Factory for generating XmlImportData's, which can hold all the data needed
to run an XModule XML import
"""
FACTORY_FOR = XmlImportData
filesystem = MemoryFS()
xblock_mixins = (InheritanceMixin,)
url_name = Sequence(str)
attribs = {}
policy = {}
tag = 'unknown'
@classmethod
def _adjust_kwargs(cls, **kwargs):
"""
Adjust the kwargs to be passed to the generated class.
Any kwargs that match :fun:`XmlImportData.__init__` will be passed
through. Any other unknown `kwargs` will be treated as XML attributes
:param tag: xml tag for the generated :class:`Element` node
:param text: (Optional) specifies the text of the generated :class:`Element`.
:param policy: (Optional) specifies data for the policy json file for this node
:type policy: dict
:param attribs: (Optional) specify attributes for the XML node
:type attribs: dict
"""
tag = kwargs.pop('tag', 'unknown')
kwargs['policy'] = {'{tag}/{url_name}'.format(tag=tag, url_name=kwargs['url_name']): kwargs['policy']}
kwargs['xml_node'].text = kwargs.pop('text', None)
kwargs['xml_node'].attrib.update(kwargs.pop('attribs', {}))
for key in kwargs.keys():
if key not in XML_IMPORT_ARGS:
kwargs['xml_node'].set(key, kwargs.pop(key))
return kwargs
@lazy_attribute
def xml_node(self):
"""An :class:`xml.etree.Element`"""
return etree.Element(self.tag)
@post_generation
def parent(self, _create, extracted, **_):
"""Hook to merge this xml into a parent xml node"""
if extracted is None:
return
extracted._xml_node.append(self._xml_node) # pylint: disable=no-member, protected-access
extracted.policy.update(self.policy)
class CourseFactory(XmlImportFactory):
"""Factory for <course> nodes"""
tag = 'course'
class SequenceFactory(XmlImportFactory):
"""Factory for <sequential> nodes"""
tag = 'sequential'
class ProblemFactory(XmlImportFactory):
"""Factory for <problem> nodes"""
tag = 'problem'
text = '<h1>Empty Problem!</h1>'
"""
Test that inherited fields work correctly when parsing XML
"""
from nose.tools import assert_equals # pylint: disable=no-name-in-module
from xmodule.tests.xml import XModuleXmlImportTest
from xmodule.tests.xml.factories import CourseFactory, SequenceFactory, ProblemFactory
class TestInheritedFieldParsing(XModuleXmlImportTest):
"""
Test that inherited fields work correctly when parsing XML
"""
def test_null_string(self):
# Test that the string inherited fields are passed through 'deserialize_field',
# which converts the string "null" to the python value None
root = CourseFactory.build(days_early_for_beta="null")
sequence = SequenceFactory.build(parent=root)
ProblemFactory.build(parent=sequence)
course = self.process_xml(root)
assert_equals(None, course.days_early_for_beta)
sequence = course.get_children()[0]
assert_equals(None, sequence.days_early_for_beta)
problem = sequence.get_children()[0]
assert_equals(None, problem.days_early_for_beta)
"""
Tests that policy json files import correctly when loading XML
"""
from nose.tools import assert_equals, assert_raises # pylint: disable=no-name-in-module
from xmodule.tests.xml.factories import CourseFactory
from xmodule.tests.xml import XModuleXmlImportTest
class TestPolicy(XModuleXmlImportTest):
"""
Tests that policy json files import correctly when loading xml
"""
def test_no_attribute_mapping(self):
# Policy files are json, and thus the values aren't passed through 'deserialize_field'
# Therefor, the string 'null' is passed unchanged to the Float field, which will trigger
# a ValueError
with assert_raises(ValueError):
course = self.process_xml(CourseFactory.build(policy={'days_early_for_beta': 'null'}))
# Trigger the exception by looking at the imported data
course.days_early_for_beta # pylint: disable=pointless-statement
def test_course_policy(self):
course = self.process_xml(CourseFactory.build(policy={'days_early_for_beta': None}))
assert_equals(None, course.days_early_for_beta)
course = self.process_xml(CourseFactory.build(policy={'days_early_for_beta': 9}))
assert_equals(9, course.days_early_for_beta)
......@@ -24,7 +24,7 @@ from django.conf import settings
from xmodule.x_module import XModule
from xmodule.editing_module import TabsEditingDescriptor
from xmodule.raw_module import EmptyDataRawDescriptor
from xmodule.xml_module import is_pointer_tag, name_to_pathname
from xmodule.xml_module import is_pointer_tag, name_to_pathname, deserialize_field
from xmodule.modulestore import Location
from xblock.fields import Scope, String, Boolean, Float, List, Integer, ScopeIds
......@@ -217,7 +217,7 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
# For backwards compatibility -- if we've got XML data, parse
# it out and set the metadata fields
if self.data:
field_data = VideoDescriptor._parse_video_xml(self.data)
field_data = self._parse_video_xml(self.data)
self._field_data.set_many(self, field_data)
del self.data
......@@ -241,7 +241,7 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
if is_pointer_tag(xml_object):
filepath = cls._format_filepath(xml_object.tag, name_to_pathname(url_name))
xml_data = etree.tostring(cls.load_file(filepath, system.resources_fs, location))
field_data = VideoDescriptor._parse_video_xml(xml_data)
field_data = cls._parse_video_xml(xml_data)
field_data['location'] = location
kvs = InheritanceKeyValueStore(initial_values=field_data)
field_data = DbModel(kvs)
......@@ -292,8 +292,8 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
xml.append(ele)
return xml
@staticmethod
def _parse_youtube(data):
@classmethod
def _parse_youtube(cls, data):
"""
Parses a string of Youtube IDs such as "1.0:AXdE34_U,1.5:VO3SxfeD"
into a dictionary. Necessary for backwards compatibility with
......@@ -310,14 +310,14 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
# Handle the fact that youtube IDs got double-quoted for a period of time.
# Note: we pass in "VideoFields.youtube_id_1_0" so we deserialize as a String--
# it doesn't matter what the actual speed is for the purposes of deserializing.
youtube_id = VideoDescriptor._deserialize(VideoFields.youtube_id_1_0.name, pieces[1])
youtube_id = deserialize_field(cls.youtube_id_1_0, pieces[1])
ret[speed] = youtube_id
except (ValueError, IndexError):
log.warning('Invalid YouTube ID: %s' % video)
return ret
@staticmethod
def _parse_video_xml(xml_data):
@classmethod
def _parse_video_xml(cls, xml_data):
"""
Parse video fields out of xml_data. The fields are set if they are
present in the XML.
......@@ -326,8 +326,8 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
field_data = {}
conversions = {
'start_time': VideoDescriptor._parse_time,
'end_time': VideoDescriptor._parse_time
'start_time': cls._parse_time,
'end_time': cls._parse_time
}
# Convert between key names for certain attributes --
......@@ -349,10 +349,10 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
for attr, value in xml.items():
if attr in compat_keys:
attr = compat_keys[attr]
if attr in VideoDescriptor.metadata_to_strip + ('url_name', 'name'):
if attr in cls.metadata_to_strip + ('url_name', 'name'):
continue
if attr == 'youtube':
speeds = VideoDescriptor._parse_youtube(value)
speeds = cls._parse_youtube(value)
for speed, youtube_id in speeds.items():
# should have made these youtube_id_1_00 for
# cleanliness, but hindsight doesn't need glasses
......@@ -367,20 +367,13 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
else:
# We export values with json.dumps (well, except for Strings, but
# for about a month we did it for Strings also).
value = VideoDescriptor._deserialize(attr, value)
value = deserialize_field(cls.fields[attr], value)
field_data[attr] = value
return field_data
@classmethod
def _deserialize(cls, attr, value):
"""
Handles deserializing values that may have been encoded with json.dumps.
"""
return cls.get_map_for_field(attr).from_xml(value)
@staticmethod
def _parse_time(str_time):
def _parse_time(cls, str_time):
"""Converts s in '12:34:45' format to seconds. If s is
None, returns empty string"""
if not str_time:
......
......@@ -62,23 +62,6 @@ def get_metadata_from_xml(xml_object, remove=True):
xml_object.remove(meta)
return dmdata
_AttrMapBase = namedtuple('_AttrMap', 'from_xml to_xml')
class AttrMap(_AttrMapBase):
"""
A class that specifies two functions:
from_xml: convert value from the xml representation into
an internal python representation
to_xml: convert the internal python representation into
the value to store in the xml.
"""
def __new__(_cls, from_xml=lambda x: x,
to_xml=lambda x: x):
return _AttrMapBase.__new__(_cls, from_xml, to_xml)
def serialize_field(value):
"""
......@@ -167,20 +150,6 @@ class XmlDescriptor(XModuleDescriptor):
metadata_to_export_to_policy = ('discussion_topics', 'checklists')
@classmethod
def get_map_for_field(cls, attr):
"""
Returns a serialize/deserialize AttrMap for the given field of a class.
Searches through fields defined by cls to find one named attr.
"""
if attr in cls.fields:
from_xml = lambda val: deserialize_field(cls.fields[attr], val)
to_xml = lambda val: serialize_field(val)
return AttrMap(from_xml, to_xml)
else:
return AttrMap()
@classmethod
def definition_from_xml(cls, xml_object, system):
"""
Return the definition to be passed to the newly created descriptor
......@@ -274,19 +243,19 @@ class XmlDescriptor(XModuleDescriptor):
Returns a dictionary {key: value}.
"""
metadata = {}
for attr in xml_object.attrib:
val = xml_object.get(attr)
if val is not None:
# VS[compat]. Remove after all key translations done
attr = cls._translate(attr)
if attr in cls.metadata_to_strip:
# don't load these
continue
attr_map = cls.get_map_for_field(attr)
metadata[attr] = attr_map.from_xml(val)
metadata = {'xml_attributes': {}}
for attr, val in xml_object.attrib.iteritems():
# VS[compat]. Remove after all key translations done
attr = cls._translate(attr)
if attr in cls.metadata_to_strip:
# don't load these
continue
if attr not in cls.fields:
metadata['xml_attributes'][attr] = val
else:
metadata[attr] = deserialize_field(cls.fields[attr], val)
return metadata
@classmethod
......@@ -295,9 +264,14 @@ class XmlDescriptor(XModuleDescriptor):
Add the keys in policy to metadata, after processing them
through the attrmap. Updates the metadata dict in place.
"""
for attr in policy:
attr_map = cls.get_map_for_field(attr)
metadata[cls._translate(attr)] = attr_map.from_xml(policy[attr])
for attr, value in policy.iteritems():
attr = cls._translate(attr)
if attr not in cls.fields:
# Store unknown attributes coming from policy.json
# in such a way that they will export to xml unchanged
metadata['xml_attributes'][attr] = value
else:
metadata[attr] = value
@classmethod
def from_xml(cls, xml_data, system, org=None, course=None):
......@@ -357,11 +331,7 @@ class XmlDescriptor(XModuleDescriptor):
field_data.update(definition)
field_data['children'] = children
field_data['xml_attributes'] = {}
field_data['xml_attributes']['filename'] = definition.get('filename', ['', None]) # for git link
for key, value in metadata.items():
if key not in cls.fields:
field_data['xml_attributes'][key] = value
field_data['location'] = location
field_data['category'] = xml_object.tag
kvs = InheritanceKeyValueStore(initial_values=field_data)
......@@ -415,18 +385,11 @@ class XmlDescriptor(XModuleDescriptor):
# Set the tag so we get the file path right
xml_object.tag = self.category
def val_for_xml(attr):
"""Get the value for this attribute that we want to store.
(Possible format conversion through an AttrMap).
"""
attr_map = self.get_map_for_field(attr)
return attr_map.to_xml(self._field_data.get(self, attr))
# Add the non-inherited metadata
for attr in sorted(own_metadata(self)):
# don't want e.g. data_dir
if attr not in self.metadata_to_strip and attr not in self.metadata_to_export_to_policy:
val = val_for_xml(attr)
val = serialize_field(self._field_data.get(self, attr))
try:
xml_object.set(attr, val)
except Exception, e:
......
......@@ -306,7 +306,7 @@ def get_module_for_descriptor_internal(user, descriptor, field_data_cache, cours
# Construct the key for the module
key = KeyValueStore.Key(
scope=Scope.user_state,
student_id=user.id,
user_id=user.id,
block_scope_id=descriptor.location,
field_name='grade'
)
......
"""
Export all xml courses in a diffable format.
This command loads all of the xml courses in the configured DATA_DIR.
For each of the courses, it loops through all of the modules, and dumps
each as a separate output file containing the json representation
of each of its fields (including those fields that are set as default values).
"""
from __future__ import print_function
import json
from path import path
from django.core.management.base import BaseCommand, CommandError
from django.conf import settings
from xmodule.modulestore.xml import XMLModuleStore
class Command(BaseCommand):
"""
Django management command to export diffable representations of all xml courses
"""
help = '''Dump the in-memory representation of all xml courses in a diff-able format'''
args = '<export path>'
def handle(self, *args, **options):
if len(args) != 1:
raise CommandError('Must called with arguments: {}'.format(self.args))
xml_module_store = XMLModuleStore(
data_dir=settings.DATA_DIR,
default_class='xmodule.hidden_module.HiddenDescriptor',
load_error_modules=True,
xblock_mixins=settings.XBLOCK_MIXINS,
)
export_dir = path(args[0])
for course_id, course_modules in xml_module_store.modules.iteritems():
course_path = course_id.replace('/', '_')
for location, descriptor in course_modules.iteritems():
location_path = location.url().replace('/', '_')
data = {}
for field_name, field in descriptor.fields.iteritems():
try:
data[field_name] = field.read_json(descriptor)
except Exception as exc: # pylint: disable=broad-except
data[field_name] = {
'$type': str(type(exc)),
'$value': descriptor._field_data.get(descriptor, field_name) # pylint: disable=protected-access
}
outdir = export_dir / course_path
outdir.makedirs_p()
with open(outdir / location_path + '.json', 'w') as outfile:
json.dump(data, outfile, sort_keys=True, indent=4)
print('', file=outfile)
......@@ -89,6 +89,9 @@ evaluation=10.0 - ((float(5 * error + warning + refactor + convention) / stateme
# evaluation report (RP0004).
comment=no
# Display symbolic names of messages in reports
symbols=yes
[TYPECHECK]
......@@ -120,7 +123,10 @@ generated-members=
content,
status_code,
# For factory_boy factories
create
create,
build,
# For xblocks
fields,
[BASIC]
......
......@@ -47,7 +47,7 @@ Pillow==1.7.8
pip>=1.4
polib==1.0.3
pycrypto>=2.6
pygments==1.5
pygments==1.6
pygraphviz==1.1
pymongo==2.4.1
pyparsing==1.5.6
......
......@@ -14,8 +14,8 @@
-e git+https://github.com/eventbrite/zendesk.git@d53fe0e81b623f084e91776bcf6369f8b7b63879#egg=zendesk
# Our libraries:
-e git+https://github.com/edx/XBlock.git@aa0d60627#egg=XBlock
-e git+https://github.com/edx/XBlock.git@a8de02c0#egg=XBlock
-e git+https://github.com/edx/codejail.git@0a1b468#egg=codejail
-e git+https://github.com/edx/diff-cover.git@v0.2.3#egg=diff_cover
-e git+https://github.com/edx/diff-cover.git@v0.2.4#egg=diff_cover
-e git+https://github.com/edx/js-test-tool.git@v0.0.7#egg=js_test_tool
-e git+https://github.com/edx/django-waffle.git@823a102e48#egg=django-waffle
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment