From d085a686e3ea82ded7b54c568c54860c52a061d0 Mon Sep 17 00:00:00 2001 From: Charles Perniciaro III Date: Mon, 18 Apr 2016 16:59:04 -0500 Subject: [PATCH] added functionality to correctly parse xml lists --- requirements-test.txt | 2 +- rest_framework_xml/parsers.py | 9 +++++- tests/test_parsers.py | 56 +++++++++++++++++++++++++++++++++-- tests/test_renderers.py | 2 -- 4 files changed, 62 insertions(+), 7 deletions(-) diff --git a/requirements-test.txt b/requirements-test.txt index a0f5aea..b98770a 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -1,6 +1,6 @@ Django>=1.6 djangorestframework>=2.4.3 -pytest-django==2.6 +pytest-django==2.9.1 pytest==2.5.2 pytest-cov==1.6 flake8==2.2.2 diff --git a/rest_framework_xml/parsers.py b/rest_framework_xml/parsers.py index 5454356..b6364c4 100644 --- a/rest_framework_xml/parsers.py +++ b/rest_framework_xml/parsers.py @@ -37,6 +37,13 @@ def parse(self, stream, media_type=None, parser_context=None): return data + def _check_xml_list(self, element): + """ + Checks that an element has multiple tags and that they are all the same, + to validate that the element is a properly formatted list + """ + return len(element) > 1 and len(set([child.tag for child in element])) <= 1 + def _xml_convert(self, element): """ convert the xml `element` into the corresponding python object @@ -48,7 +55,7 @@ def _xml_convert(self, element): return self._type_convert(element.text) else: # if the fist child tag is list-item means all children are list-item - if children[0].tag == "list-item": + if self._check_xml_list(element): data = [] for child in children: data.append(self._xml_convert(child)) diff --git a/tests/test_parsers.py b/tests/test_parsers.py index b04af4a..27258a8 100644 --- a/tests/test_parsers.py +++ b/tests/test_parsers.py @@ -4,7 +4,6 @@ from django.test import TestCase -from django.utils import unittest from django.utils.six.moves import StringIO from rest_framework_xml.parsers import XMLParser from rest_framework_xml.compat import etree @@ -52,15 +51,66 @@ def setUp(self): } ] } + self._invalid_list_input = StringIO( + '' + '' + '' + '1first' + '2second' + '3third' + '' + '' + ) + self._invalid_list_output = { + "list": { + "list-item": { + "sub_id": 1, + "sub_name": "first" + }, + "list-item2": { + "sub_id": 3, + "sub_name": "third" + } + } + } + self._valid_list_input = StringIO( + '' + '' + '' + '1first' + '2second' + '' + '' + ) + self._valid_list_output = { + "list": [ + { + "sub_id": 1, + "sub_name": "first" + }, + { + "sub_id": 2, + "sub_name": "second" + } + ] + } - @unittest.skipUnless(etree, 'defusedxml not installed') def test_parse(self): parser = XMLParser() data = parser.parse(self._input) self.assertEqual(data, self._data) - @unittest.skipUnless(etree, 'defusedxml not installed') def test_complex_data_parse(self): parser = XMLParser() data = parser.parse(self._complex_data_input) self.assertEqual(data, self._complex_data) + + def test_invalid_list_parse(self): + parser = XMLParser() + data = parser.parse(self._invalid_list_input) + self.assertEqual(data, self._invalid_list_output) + + def test_valid_list_parse(self): + parser = XMLParser() + data = parser.parse(self._valid_list_input) + self.assertEqual(data, self._valid_list_output) diff --git a/tests/test_renderers.py b/tests/test_renderers.py index 99c5f22..76be755 100644 --- a/tests/test_renderers.py +++ b/tests/test_renderers.py @@ -5,7 +5,6 @@ from decimal import Decimal from django.test import TestCase -from django.utils import unittest from django.utils.six.moves import StringIO from rest_framework_xml.renderers import XMLRenderer from rest_framework_xml.parsers import XMLParser @@ -97,7 +96,6 @@ def test_render_list(self): self.assertXMLContains(content, '') self.assertXMLContains(content, '') - @unittest.skipUnless(etree, 'defusedxml not installed') def test_render_and_parse_complex_data(self): """ Test XML rendering.