Skip to content

Commit

Permalink
Fix #256 Avoid parsing tail content twice
Browse files Browse the repository at this point in the history
  • Loading branch information
tefra committed Sep 13, 2020
1 parent 2c0b0fc commit aafe3d7
Show file tree
Hide file tree
Showing 4 changed files with 142 additions and 63 deletions.
96 changes: 64 additions & 32 deletions tests/formats/dataclass/parsers/test_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,23 @@ class FooMixed:
)


def add_attr(x, *args):
x["a"] = 1


def add_text(x, *args):
x["b"] = 2
return True


def add_child(x, *args):
x["c"] = 3


def add_content(x, *args):
x["content"] = 3


class ElementNodeTests(TestCase):
def setUp(self) -> None:
super().setUp()
Expand All @@ -69,20 +86,16 @@ def setUp(self) -> None:
)

@mock.patch.object(ParserUtils, "bind_element_children")
@mock.patch.object(ParserUtils, "bind_wildcard")
@mock.patch.object(ParserUtils, "bind_element")
@mock.patch.object(ParserUtils, "bind_element_attrs")
def test_bind(
self, mock_bind_element_attrs, mock_bind_element, mock_bind_element_children
self,
mock_bind_element_attrs,
mock_bind_element,
mock_bind_wildcard,
mock_bind_element_children,
):
def add_attr(x, *args):
x["a"] = 1

def add_text(x, *args):
x["b"] = 2

def add_child(x, *args):
x["c"] = 3

mock_bind_element_attrs.side_effect = add_attr
mock_bind_element.side_effect = add_text
mock_bind_element_children.side_effect = add_child
Expand All @@ -106,29 +119,27 @@ def add_child(x, *args):
mock.ANY, node.meta, node.attrs, node.ns_map
)
mock_bind_element.assert_called_once_with(
mock.ANY, node.meta, "text", "tail", node.attrs, node.ns_map
mock.ANY, node.meta, "text", node.ns_map
)
mock_bind_element_children.assert_called_once_with(
mock.ANY, node.meta, 0, objects
)
self.assertEqual(0, mock_bind_wildcard.call_count)

@mock.patch.object(ParserUtils, "bind_element_children")
@mock.patch.object(ParserUtils, "bind_wildcard")
@mock.patch.object(ParserUtils, "bind_element")
@mock.patch.object(ParserUtils, "bind_element_attrs")
def test_bind_with_mixed_flag_true(
self, mock_bind_element_attrs, mock_bind_element, mock_bind_element_children
def test_bind_with_wildcard_var(
self,
mock_bind_element_attrs,
mock_bind_element,
mock_bind_wildcard,
mock_bind_element_children,
):
def add_attr(x, *args):
x["a"] = 1

def add_text(x, *args):
x["b"] = 2

def add_child(x, *args):
x["c"] = 3

mock_bind_element_attrs.side_effect = add_attr
mock_bind_element.side_effect = add_text
mock_bind_element.return_value = False
mock_bind_wildcard.side_effect = add_text
mock_bind_element_children.side_effect = add_child

node = ElementNode(
Expand All @@ -140,6 +151,35 @@ def add_child(x, *args):
ns_map={"ns0": "xsdata"},
)

objects = [1, 2, 3]

self.assertTrue(node.bind("foo", "text", "tail", objects))
self.assertEqual("foo", objects[-1][0])
self.assertEqual(Foo(1, 2, 3), objects[-1][1])

mock_bind_element_attrs.assert_called_once_with(
mock.ANY, node.meta, node.attrs, node.ns_map
)
mock_bind_element.assert_called_once_with(
mock.ANY, node.meta, "text", node.ns_map
)
mock_bind_element_children.assert_called_once_with(
mock.ANY, node.meta, 0, objects
)
mock_bind_wildcard.assert_called_once_with(
mock.ANY, node.meta, "text", "tail", node.attrs, node.ns_map
)

@mock.patch.object(ParserUtils, "bind_element_children")
@mock.patch.object(ParserUtils, "bind_element")
@mock.patch.object(ParserUtils, "bind_element_attrs")
def test_bind_with_mixed_flag_true(
self, mock_bind_element_attrs, mock_bind_element, mock_bind_element_children
):
mock_bind_element_attrs.side_effect = add_attr
mock_bind_element.side_effect = add_text
mock_bind_element_children.side_effect = add_child

node = ElementNode(
position=0,
meta=self.context.build(Foo),
Expand Down Expand Up @@ -169,18 +209,10 @@ def test_bind_with_mixed_content_var(
mock_bind_wildcard_element,
mock_bind_mixed_content,
):
def add_attr(x, *args):
x["a"] = 1

def add_text(x, *args):
x["b"] = 2

def add_child(x, *args):
x["content"] = 3

mock_bind_element_attrs.side_effect = add_attr
mock_bind_wildcard_element.side_effect = add_text
mock_bind_mixed_content.side_effect = add_child
mock_bind_mixed_content.side_effect = add_content

node = ElementNode(
position=0,
Expand Down
40 changes: 28 additions & 12 deletions tests/formats/dataclass/parsers/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,42 +217,58 @@ def test_bind_element_attrs_skip_empty_attrs(self, mock_find_var):
self.assertEqual(0, len(params))
self.assertEqual(0, mock_find_var.call_count)

def test_bind_element_with_no_text_var(self):
params = {}
metadata = self.ctx.build(Books)
ParserUtils.bind_element(params, metadata, "foo", None, {}, {})
self.assertEqual({}, params)

@mock.patch.object(ParserUtils, "parse_value", return_value="yes!")
def test_bind_element_with_text_var(self, mock_parse_value):
def test_bind_element(self, mock_parse_value):
metadata = self.ctx.build(SizeType)
var = metadata.find_var(mode=FindMode.TEXT)
params = {}
ns_map = {"a": "b"}

ParserUtils.bind_element(params, metadata, None, None, {}, ns_map)
self.assertFalse(ParserUtils.bind_element(params, metadata, None, ns_map))
self.assertEqual({}, params)

ParserUtils.bind_element(params, metadata, "foo", None, {}, ns_map)
self.assertTrue(ParserUtils.bind_element(params, metadata, "foo", ns_map))
self.assertEqual({"value": "yes!"}, params)
mock_parse_value.assert_called_once_with(
"foo", var.types, var.default, ns_map, var.is_list
)

def test_bind_element_with_wildcard_var(self):
def test_bind_element_with_no_text_var(self):
params = {}
metadata = self.ctx.build(Books)
self.assertFalse(ParserUtils.bind_element(params, metadata, "foo", {}))
self.assertEqual({}, params)

def test_bind_wildcard(self):
metadata = self.ctx.build(Umbrella)
params = {}
attrs = {"a": "b"}
ns_map = {"a": "b"}

ParserUtils.bind_element(params, metadata, None, None, attrs, ns_map)
result = ParserUtils.bind_wildcard(params, metadata, None, None, attrs, ns_map)
self.assertFalse(result)
self.assertEqual({}, params)

ParserUtils.bind_element(params, metadata, "foo", "bar", attrs, ns_map)
result = ParserUtils.bind_wildcard(params, metadata, None, "bar", attrs, ns_map)
expected = AnyElement(text=None, tail="bar", attributes=attrs, ns_map=ns_map)
self.assertTrue(result)
self.assertEqual(expected, params["any_element"])

params.clear()
result = ParserUtils.bind_wildcard(
params, metadata, "foo", "bar", attrs, ns_map
)
expected = AnyElement(text="foo", tail="bar", attributes=attrs, ns_map=ns_map)
self.assertTrue(result)
self.assertEqual(expected, params["any_element"])

def test_bind_wildcard_with_no_wildcard_var(self):
params = {}
metadata = self.ctx.build(Books)
result = ParserUtils.bind_wildcard(params, metadata, "foo", "bar", {}, {})
self.assertFalse(result)
self.assertEqual({}, params)

def test_bind_element_param(self):
var = XmlVar(name="a", qname="a")
params = {}
Expand Down
5 changes: 3 additions & 2 deletions xsdata/formats/dataclass/parsers/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,14 @@ def bind(self, qname: str, text: NoneStr, tail: NoneStr, objects: List) -> bool:
)
else:
ParserUtils.bind_element_children(params, self.meta, self.position, objects)
ParserUtils.bind_element(
text_node = ParserUtils.bind_element(params, self.meta, text, self.ns_map)
wild_node = not text_node and ParserUtils.bind_wildcard(
params, self.meta, text, tail, self.attrs, self.ns_map
)

objects.append((qname, self.meta.clazz(**params)))

if not mixed_var and self.mixed:
if not mixed_var and self.mixed and not wild_node:
tail = ParserUtils.normalize_content(tail)
if tail:
objects.append((None, tail))
Expand Down
64 changes: 47 additions & 17 deletions xsdata/formats/dataclass/parsers/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,10 @@ def bind_wildcard_element(
tail: Optional[str],
attrs: Dict,
ns_map: Dict,
):
) -> bool:
"""
Extract the text and tail content and bind it accordingly in the params
dictionary.
dictionary. Return if any data was bound.
- var is a list prepend the text and append the tail.
- var is present in the params assign the text and tail to the generic object.
Expand All @@ -182,7 +182,7 @@ def bind_wildcard_element(
txt = cls.normalize_content(txt)
tail = cls.normalize_content(tail)
if txt is None and tail is None:
return
return False

if var.is_list:
params.setdefault(var.name, [])
Expand All @@ -203,6 +203,8 @@ def bind_wildcard_element(

params[var.name] = generic

return True

@classmethod
def normalize_content(cls, value: Optional[str]) -> Optional[str]:
"""
Expand Down Expand Up @@ -239,21 +241,23 @@ def bind_element(
params: Dict,
metadata: XmlMeta,
txt: Optional[str],
tail: Optional[str],
attrs: Dict,
ns_map: Dict,
):
"""Add the given element's text content if any to the params dictionary
with the text var name as key."""
var = metadata.find_var(mode=FindMode.TEXT)
wildcard = None if var else metadata.find_var(mode=FindMode.WILDCARD)

if var and var.init and txt is not None:
params[var.name] = cls.parse_value(
txt, var.types, var.default, ns_map, var.tokens
)
elif wildcard:
cls.bind_wildcard_element(params, wildcard, txt, tail, attrs, ns_map)
) -> bool:
"""
Add the given element's text content if any to the params dictionary
with the text var name as key.
Return if any data was bound.
"""

if txt is not None:
var = metadata.find_var(mode=FindMode.TEXT)
if var and var.init and txt is not None:
params[var.name] = cls.parse_value(
txt, var.types, var.default, ns_map, var.tokens
)
return True
return False

@classmethod
def bind_element_attrs(
Expand Down Expand Up @@ -303,3 +307,29 @@ def find_eligible_wildcard(
),
None,
)

@classmethod
def bind_wildcard(
cls,
params: Dict,
metadata: XmlMeta,
txt: Optional[str],
tail: Optional[str],
attrs: Dict,
ns_map: Dict,
) -> bool:
"""
Add the given element's content and attributes to any available
wildcard variable.
Return if any data was bound.
"""

if txt is not None or tail is not None:
wildcard = metadata.find_var(mode=FindMode.WILDCARD)
if wildcard:
return cls.bind_wildcard_element(
params, wildcard, txt, tail, attrs, ns_map
)

return False

0 comments on commit aafe3d7

Please sign in to comment.