From d085a686e3ea82ded7b54c568c54860c52a061d0 Mon Sep 17 00:00:00 2001 From: Charles Perniciaro III Date: Mon, 18 Apr 2016 16:59:04 -0500 Subject: [PATCH 1/2] 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. From 9a24ffd0d7455337268fe7ed83c71044663dd883 Mon Sep 17 00:00:00 2001 From: Charles Perniciaro III Date: Mon, 18 Apr 2016 17:23:58 -0500 Subject: [PATCH 2/2] removing the skipUnless code broke the linter validation, so I readded it --- rest_framework_xml/parsers.py | 2 +- tests/test_parsers.py | 5 +++++ tests/test_renderers.py | 2 ++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/rest_framework_xml/parsers.py b/rest_framework_xml/parsers.py index b6364c4..7e1bd4a 100644 --- a/rest_framework_xml/parsers.py +++ b/rest_framework_xml/parsers.py @@ -39,7 +39,7 @@ def parse(self, stream, media_type=None, parser_context=None): def _check_xml_list(self, element): """ - Checks that an element has multiple tags and that they are all the same, + 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 diff --git a/tests/test_parsers.py b/tests/test_parsers.py index 27258a8..3e5597a 100644 --- a/tests/test_parsers.py +++ b/tests/test_parsers.py @@ -4,6 +4,7 @@ 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 @@ -95,21 +96,25 @@ def setUp(self): ] } + @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) + @unittest.skipUnless(etree, 'defusedxml not installed') def test_invalid_list_parse(self): parser = XMLParser() data = parser.parse(self._invalid_list_input) self.assertEqual(data, self._invalid_list_output) + @unittest.skipUnless(etree, 'defusedxml not installed') def test_valid_list_parse(self): parser = XMLParser() data = parser.parse(self._valid_list_input) diff --git a/tests/test_renderers.py b/tests/test_renderers.py index 76be755..99c5f22 100644 --- a/tests/test_renderers.py +++ b/tests/test_renderers.py @@ -5,6 +5,7 @@ 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 @@ -96,6 +97,7 @@ 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.