From bf6ebcc3d86fc17f176e042b1de66fb962683f04 Mon Sep 17 00:00:00 2001
From: cahrens <christina@edx.org>
Date: Wed, 14 Aug 2013 16:13:33 -0400
Subject: [PATCH 1/2] Add deserialize logic for metadata fields. STUD-640

Handle the case of the double-quotes appearing within the string.

We don't need to worry about the whole concatenated ID being
double-quoted, since the concatenation was done by overriding xml_module's
export method.

Use the method already fined in XmlDescriptor.
---
 .../lib/xmodule/xmodule/tests/test_video.py   | 56 +++++++++++++++++++
 common/lib/xmodule/xmodule/video_module.py    | 17 +++++-
 common/lib/xmodule/xmodule/xml_module.py      | 13 +++--
 3 files changed, 80 insertions(+), 6 deletions(-)

diff --git a/common/lib/xmodule/xmodule/tests/test_video.py b/common/lib/xmodule/xmodule/tests/test_video.py
index 4a13d565cca4..a8810a1238d1 100644
--- a/common/lib/xmodule/xmodule/tests/test_video.py
+++ b/common/lib/xmodule/xmodule/tests/test_video.py
@@ -263,6 +263,62 @@ def test_from_xml_no_attributes(self):
             'data': ''
         })
 
+    def test_from_xml_double_quotes(self):
+        """
+        Make sure we can handle the double-quoted string format (which was used for exporting for
+        a few weeks).
+        """
+        module_system = DummySystem(load_error_modules=True)
+        xml_data ='''
+            <video display_name="&quot;display_name&quot;"
+                html5_sources="[&quot;source_1&quot;, &quot;source_2&quot;]"
+                show_captions="false"
+                source="&quot;http://download_video&quot;"
+                sub="&quot;html5_subtitles&quot;"
+                track="&quot;http://download_track&quot;"
+                youtube_id_0_75="&quot;OEoXaMPEzf65&quot;"
+                youtube_id_1_25="&quot;OEoXaMPEzf125&quot;"
+                youtube_id_1_5="&quot;OEoXaMPEzf15&quot;"
+                youtube_id_1_0="&quot;OEoXaMPEzf10&quot;"
+                />
+        '''
+        output = VideoDescriptor.from_xml(xml_data, module_system)
+        self.assert_attributes_equal(output, {
+            'youtube_id_0_75': 'OEoXaMPEzf65',
+            'youtube_id_1_0': 'OEoXaMPEzf10',
+            'youtube_id_1_25': 'OEoXaMPEzf125',
+            'youtube_id_1_5': 'OEoXaMPEzf15',
+            'show_captions': False,
+            'start_time': 0.0,
+            'end_time': 0.0,
+            'track': 'http://download_track',
+            'source': 'http://download_video',
+            'html5_sources': ["source_1", "source_2"],
+            'data': ''
+        })
+
+    def test_from_xml_double_quote_concatenated_youtube(self):
+        module_system = DummySystem(load_error_modules=True)
+        xml_data = '''
+            <video display_name="Test Video"
+                   youtube="1.0:&quot;p2Q6BrNhdh8&quot;,1.25:&quot;1EeWXzPdhSA&quot;">
+            </video>
+        '''
+        output = VideoDescriptor.from_xml(xml_data, module_system)
+        self.assert_attributes_equal(output, {
+            'youtube_id_0_75': '',
+            'youtube_id_1_0': 'p2Q6BrNhdh8',
+            'youtube_id_1_25': '1EeWXzPdhSA',
+            'youtube_id_1_5': '',
+            'show_captions': True,
+            'start_time': 0.0,
+            'end_time': 0.0,
+            'track': '',
+            'source': '',
+            'html5_sources': [],
+            'data': ''
+        })
+
     def test_old_video_format(self):
         """
         Test backwards compatibility with VideoModule's XML format.
diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py
index dd408a6f745a..82dbc8e02cb9 100644
--- a/common/lib/xmodule/xmodule/video_module.py
+++ b/common/lib/xmodule/xmodule/video_module.py
@@ -293,7 +293,10 @@ def _parse_youtube(data):
             pieces = video.split(':')
             try:
                 speed = '%.2f' % float(pieces[0])  # normalize speed
-                youtube_id = pieces[1]
+                # 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])
                 ret[speed] = youtube_id
             except (ValueError, IndexError):
                 log.warning('Invalid YouTube ID: %s' % video)
@@ -310,7 +313,6 @@ def _parse_video_xml(xml_data):
         model_data = {}
 
         conversions = {
-            'show_captions': json.loads,
             'start_time': VideoDescriptor._parse_time,
             'end_time': VideoDescriptor._parse_time
         }
@@ -349,6 +351,11 @@ def _parse_video_xml(xml_data):
                 #  Convert XML attrs into Python values.
                 if attr in conversions:
                     value = conversions[attr](value)
+                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)
+
                 model_data[attr] = value
 
         return model_data
@@ -367,6 +374,12 @@ def _parse_time(str_time):
                 seconds=obj_time.tm_sec
             ).total_seconds()
 
+    @staticmethod
+    def _deserialize(attr, value):
+        """
+        Handles deserializing values that may have been encoded with json.dumps.
+        """
+        return VideoDescriptor.get_map_for_field(VideoDescriptor, attr).from_xml(value)
 
 def _create_youtube_string(module):
     """
diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py
index 5b8d2c8aee0c..f4914a096f78 100644
--- a/common/lib/xmodule/xmodule/xml_module.py
+++ b/common/lib/xmodule/xmodule/xml_module.py
@@ -165,8 +165,13 @@ class XmlDescriptor(XModuleDescriptor):
 
     metadata_to_export_to_policy = ('discussion_topics')
 
-    @classmethod
+    @staticmethod
     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.
+        """
         for field in set(cls.fields + cls.lms.fields):
             if field.name == attr:
                 from_xml = lambda val: deserialize_field(field, val)
@@ -280,7 +285,7 @@ def load_metadata(cls, xml_object):
                     # don't load these
                     continue
 
-                attr_map = cls.get_map_for_field(attr)
+                attr_map = XmlDescriptor.get_map_for_field(cls, attr)
                 metadata[attr] = attr_map.from_xml(val)
         return metadata
 
@@ -291,7 +296,7 @@ def apply_policy(cls, metadata, policy):
         through the attrmap.  Updates the metadata dict in place.
         """
         for attr in policy:
-            attr_map = cls.get_map_for_field(attr)
+            attr_map = XmlDescriptor.get_map_for_field(cls, attr)
             metadata[cls._translate(attr)] = attr_map.from_xml(policy[attr])
 
     @classmethod
@@ -407,7 +412,7 @@ 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)
+            attr_map = XmlDescriptor.get_map_for_field(self, attr)
             return attr_map.to_xml(self._model_data[attr])
 
         # Add the non-inherited metadata

From b4c0a3a546720f4a6db01f3686d1a56e5d820102 Mon Sep 17 00:00:00 2001
From: cahrens <christina@edx.org>
Date: Thu, 15 Aug 2013 10:45:31 -0400
Subject: [PATCH 2/2] Put method back to classmethod.

---
 common/lib/xmodule/xmodule/video_module.py | 6 +++---
 common/lib/xmodule/xmodule/xml_module.py   | 8 ++++----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py
index 82dbc8e02cb9..a87d71246aa1 100644
--- a/common/lib/xmodule/xmodule/video_module.py
+++ b/common/lib/xmodule/xmodule/video_module.py
@@ -374,12 +374,12 @@ def _parse_time(str_time):
                 seconds=obj_time.tm_sec
             ).total_seconds()
 
-    @staticmethod
-    def _deserialize(attr, value):
+    @classmethod
+    def _deserialize(cls, attr, value):
         """
         Handles deserializing values that may have been encoded with json.dumps.
         """
-        return VideoDescriptor.get_map_for_field(VideoDescriptor, attr).from_xml(value)
+        return cls.get_map_for_field(attr).from_xml(value)
 
 def _create_youtube_string(module):
     """
diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py
index f4914a096f78..b0b7f300ed79 100644
--- a/common/lib/xmodule/xmodule/xml_module.py
+++ b/common/lib/xmodule/xmodule/xml_module.py
@@ -165,7 +165,7 @@ class XmlDescriptor(XModuleDescriptor):
 
     metadata_to_export_to_policy = ('discussion_topics')
 
-    @staticmethod
+    @classmethod
     def get_map_for_field(cls, attr):
         """
         Returns a serialize/deserialize AttrMap for the given field of a class.
@@ -285,7 +285,7 @@ def load_metadata(cls, xml_object):
                     # don't load these
                     continue
 
-                attr_map = XmlDescriptor.get_map_for_field(cls, attr)
+                attr_map = cls.get_map_for_field(attr)
                 metadata[attr] = attr_map.from_xml(val)
         return metadata
 
@@ -296,7 +296,7 @@ def apply_policy(cls, metadata, policy):
         through the attrmap.  Updates the metadata dict in place.
         """
         for attr in policy:
-            attr_map = XmlDescriptor.get_map_for_field(cls, attr)
+            attr_map = cls.get_map_for_field(attr)
             metadata[cls._translate(attr)] = attr_map.from_xml(policy[attr])
 
     @classmethod
@@ -412,7 +412,7 @@ def val_for_xml(attr):
             """Get the value for this attribute that we want to store.
             (Possible format conversion through an AttrMap).
              """
-            attr_map = XmlDescriptor.get_map_for_field(self, attr)
+            attr_map = self.get_map_for_field(attr)
             return attr_map.to_xml(self._model_data[attr])
 
         # Add the non-inherited metadata