diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0544d70bfabff5a53bdd693d5acf21f738e26341..cb648b1d7404a1af4f00ff0aa5b26b4ebfc1e144 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -19,6 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed ###
+* [#73](https://gitlab.com/linkahead/linkahead-pylib/-/issues/73)
+ `Entity.to_xml` now detects potentially infinite recursion and prevents an error
* [#89](https://gitlab.com/linkahead/linkahead-pylib/-/issues/89)
`to_xml` does not add `noscript` or `TransactionBenchmark` tags anymore
* [#103](https://gitlab.com/linkahead/linkahead-pylib/-/issues/103)
diff --git a/src/linkahead/common/models.py b/src/linkahead/common/models.py
index aff85556ce3bf13396b43444e68f81cd26606b9c..74f7fa10fea12ac1cff05aaba26baf9c2a52e589 100644
--- a/src/linkahead/common/models.py
+++ b/src/linkahead/common/models.py
@@ -1232,6 +1232,7 @@ class Entity:
xml: Optional[etree._Element] = None,
add_properties: INHERITANCE = "ALL",
local_serialization: bool = False,
+ visited_entities: Optional[Union[list, None]] = None
) -> etree._Element:
"""Generate an xml representation of this entity. If the parameter xml
is given, all attributes, parents, properties, and messages of this
@@ -1242,9 +1243,10 @@ class Entity:
@param xml: an xml element to which all attributes, parents,
properties, and messages
are to be added.
+ @param visited_entities: recursion check, should never be set manually
FIXME: Add documentation for the add_properties parameter.
- FIXME: Add docuemntation for the local_serialization parameter.
+ FIXME: Add documentation for the local_serialization parameter.
@return: xml representation of this entity.
"""
@@ -1255,9 +1257,17 @@ class Entity:
xml = etree.Element(elem_tag)
assert isinstance(xml, etree._Element)
+ if visited_entities is None:
+ visited_entities = []
+ if self in visited_entities:
+ xml.text = "..."
+ return xml
+ visited_entities.append(self)
+
# unwrap wrapped entity
if self._wrapped_entity is not None:
- xml = self._wrapped_entity.to_xml(xml, add_properties)
+ xml = self._wrapped_entity.to_xml(xml, add_properties,
+ visited_entities=visited_entities.copy())
if self.id is not None:
xml.set("id", str(self.id))
@@ -1272,6 +1282,8 @@ class Entity:
xml.set("description", str(self.description))
if self.version is not None:
+ # Does version.to_xml need visited_entities support?
+ # It does have some recursion with predecessors / successors.
xml.append(self.version.to_xml())
if self.value is not None:
@@ -1281,7 +1293,7 @@ class Entity:
elif self.value.name is not None:
xml.text = str(self.value.name)
else:
- xml.text = str(self.value)
+ self.value.to_xml(xml, visited_entities=visited_entities.copy())
elif isinstance(self.value, list):
for v in self.value:
v_elem = etree.Element("Value")
@@ -1292,7 +1304,7 @@ class Entity:
elif v.name is not None:
v_elem.text = str(v.name)
else:
- v_elem.text = str(v)
+ v.to_xml(v_elem, visited_entities=visited_entities.copy())
elif v == "":
v_elem.append(etree.Element("EmptyString"))
elif v is None:
@@ -1314,7 +1326,10 @@ class Entity:
elif self.datatype.name is not None:
xml.set("datatype", str(self.datatype.name))
else:
- xml.set("datatype", str(self.datatype))
+ dt_str = xml2str(self.datatype.to_xml(visited_entities=visited_entities.copy()))
+ # Todo: Use for pretty-printing with calls from _repr_ only?
+ # dt_str = dt_str.replace('<', 'á¸').replace('>', 'á³').replace(' ', 'â €').replace('"', '\'').replace('\n', '')
+ xml.set("datatype", dt_str)
else:
xml.set("datatype", str(self.datatype))
@@ -1337,10 +1352,11 @@ class Entity:
self.messages.to_xml(xml)
if self.parents is not None:
- self.parents.to_xml(xml)
+ self.parents.to_xml(xml, visited_entities=visited_entities.copy())
if self.properties is not None:
- self.properties.to_xml(xml, add_properties)
+ self.properties.to_xml(xml, add_properties,
+ visited_entities=visited_entities.copy())
if len(self._flags) > 0:
flagattr = ""
@@ -1948,11 +1964,16 @@ class Parent(Entity):
xml: Optional[etree._Element] = None,
add_properties: INHERITANCE = "NONE",
local_serialization: bool = False,
+ visited_entities: Optional[Union[list, None]] = None,
):
if xml is None:
xml = etree.Element("Parent")
- return super().to_xml(xml=xml, add_properties=add_properties)
+ if visited_entities is None:
+ visited_entities = []
+
+ return super().to_xml(xml=xml, add_properties=add_properties,
+ visited_entities=visited_entities)
class _EntityWrapper(object):
@@ -2023,14 +2044,19 @@ class Property(Entity):
xml: Optional[etree._Element] = None,
add_properties: INHERITANCE = "ALL",
local_serialization: bool = False,
+ visited_entities: Optional[Union[list, None]] = None,
):
if xml is None:
xml = etree.Element("Property")
+ if visited_entities is None:
+ visited_entities = []
+
return super(Property, self).to_xml(
xml=xml,
add_properties=add_properties,
local_serialization=local_serialization,
+ visited_entities=visited_entities,
)
def is_reference(self, server_retrieval: bool = False) -> Optional[bool]:
@@ -2188,15 +2214,20 @@ class RecordType(Entity):
xml: Optional[etree._Element] = None,
add_properties: INHERITANCE = "ALL",
local_serialization: bool = False,
+ visited_entities: Optional[Union[list, None]] = None,
) -> etree._Element:
if xml is None:
xml = etree.Element("RecordType")
+ if visited_entities is None:
+ visited_entities = []
+
return Entity.to_xml(
self,
xml=xml,
add_properties=add_properties,
local_serialization=local_serialization,
+ visited_entities=visited_entities,
)
@@ -2227,14 +2258,19 @@ class Record(Entity):
xml: Optional[etree._Element] = None,
add_properties: INHERITANCE = "ALL",
local_serialization: bool = False,
+ visited_entities: Optional[Union[list, None]] = None,
):
if xml is None:
xml = etree.Element("Record")
+ if visited_entities is None:
+ visited_entities = []
+
return super().to_xml(
xml=xml,
add_properties=add_properties,
local_serialization=local_serialization,
+ visited_entities=visited_entities
)
@@ -2304,6 +2340,7 @@ class File(Record):
xml: Optional[etree._Element] = None,
add_properties: INHERITANCE = "ALL",
local_serialization: bool = False,
+ visited_entities: Optional[Union[list, None]] = None,
) -> etree._Element:
"""Convert this file to an xml element.
@@ -2313,8 +2350,12 @@ class File(Record):
if xml is None:
xml = etree.Element("File")
+ if visited_entities is None:
+ visited_entities = []
+
return Entity.to_xml(self, xml=xml, add_properties=add_properties,
- local_serialization=local_serialization)
+ local_serialization=local_serialization,
+ visited_entities=visited_entities)
def download(self, target: Optional[str] = None) -> str:
"""Download this file-entity's actual file from the file server. It
@@ -2476,15 +2517,20 @@ class PropertyList(list):
return self
- def to_xml(self, add_to_element: etree._Element, add_properties: INHERITANCE):
+ def to_xml(self, add_to_element: etree._Element, add_properties: INHERITANCE,
+ visited_entities: Optional[Union[list, None]] = None):
+
+ if visited_entities is None:
+ visited_entities = []
+
p: Property
for p in self:
importance = self._importance.get(p)
if add_properties == FIX and not importance == FIX:
continue
-
- pelem = p.to_xml(xml=etree.Element("Property"), add_properties=FIX)
+ pelem = p.to_xml(xml=etree.Element("Property"), add_properties=FIX,
+ visited_entities=visited_entities.copy())
if p in self._importance:
pelem.set("importance", str(importance))
@@ -2631,7 +2677,12 @@ class ParentList(list):
return self
- def to_xml(self, add_to_element: etree._Element):
+ def to_xml(self, add_to_element: etree._Element,
+ visited_entities: Optional[Union[list, None]] = None):
+
+ if visited_entities is None:
+ visited_entities = []
+
for p in self:
pelem = etree.Element("Parent")
diff --git a/unittests/test_issues.py b/unittests/test_issues.py
index e24afbe8b7be8d9a87d85819eccd3a4bf0d453e8..9d422ff84f8e12cc2589d71055ba11ea4857c874 100644
--- a/unittests/test_issues.py
+++ b/unittests/test_issues.py
@@ -26,6 +26,7 @@ import linkahead as db
from datetime import date, datetime
from pytest import raises
+from linkahead.common.utils import xml2str
def test_issue_100():
@@ -90,3 +91,37 @@ def test_issue_128():
assert prop_list.value == [today, today]
prop_list.value = [now, now]
assert prop_list.value == [now, now]
+
+
+def test_issue_73():
+ """
+ Test to_xml infinite recursion handling with cross- and self-references.
+ https://gitlab.com/linkahead/linkahead-pylib/-/issues/73
+ """
+ # Cross-reference in the property values
+ rt = db.RecordType(name="RT")
+ recA = db.Record().add_parent(rt)
+ recB = db.Record().add_parent(rt)
+ recA.add_property(name="RT", value=recB)
+ recB.add_property(name="RT", value=recA)
+ xml_str = xml2str(recB.to_xml())
+ assert "<Parent name=\"RT" in xml_str
+ assert "<Property name=\"RT" in xml_str
+ assert len(xml_str) < 500
+
+ # Cross-reference in the properties themselves
+ prop1 = db.Property(name="Prop1")
+ prop2 = db.Property(name="Prop2")
+ prop1.add_property(prop2)
+ prop2.add_property(prop1)
+ xml_str = xml2str(prop2.to_xml())
+ assert "<Property name=\"Prop1" in xml_str
+ assert "<Property name=\"Prop2" in xml_str
+ assert len(xml_str) < 500
+
+ # Self-reference in the datatype
+ prop = db.Property()
+ prop.datatype = prop
+ xml_str = xml2str(prop.to_xml())
+ assert "datatype=" in xml_str
+ assert len(xml_str) < 100