diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 463b1d56507cb970e5ec15e6fa90ce6adf3e123b..db600343569930a436a593a8ab5d511a35bc7aca 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -61,16 +61,6 @@ mypy: - make mypy # run unit tests -unittest_py3.8: - tags: [ docker ] - stage: test - needs: [ ] - image: python:3.8 - script: &python_test_script - # Python docker has problems with tox and pip so use plain pytest here - - touch ~/.pylinkahead.ini - - pip install .[test] - - python -m pytest unittests # This needs to be changed once Python 3.9 isn't the standard Python in Debian # anymore. @@ -90,7 +80,11 @@ unittest_py3.10: stage: test needs: [ ] image: python:3.10 - script: *python_test_script + script: &python_test_script + # Python docker has problems with tox and pip so use plain pytest here + - touch ~/.pylinkahead.ini + - pip install .[test] + - python -m pytest unittests unittest_py3.11: tags: [ docker ] @@ -158,7 +152,7 @@ build-testenv: pages_prepare: &pages_prepare tags: [ cached-dind ] stage: deploy - needs: [ code_style, pylint, unittest_py3.8, unittest_py3.9, unittest_py3.10 ] + needs: [ code_style, pylint, unittest_py3.9, unittest_py3.10 ] only: refs: - /^release-.*$/i diff --git a/CHANGELOG.md b/CHANGELOG.md index fc480143e7b08f88761456c28f39fa4f53c4c50d..a32e16deba8103a3e037c7bedfeb8fc0e4c55a6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added ### * New setup extra `test` which installs the dependencies for testing. +* The Container class has a new member function `filter` which is based o `_filter_entity_list`. +* The `Entity` properties `_cuid` and `_flags` are now available for read-only access + as `cuid` and `flags`, respectively. ### Changed ### @@ -17,14 +20,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed ### +* Support for Python 3.8 + ### 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) `authentication/interface/on_response()` does not overwrite `auth_token` if new value is `None` * [#119](https://gitlab.com/linkahead/linkahead-pylib/-/issues/119) + The diff returned by compare_entities now uses id instead of name as + key if either property does not have a name +* [#87](https://gitlab.com/linkahead/linkahead-pylib/-/issues/87) + `XMLSyntaxError` messages when parsing (incomplete) responses in + case of certain connection timeouts. The diff returned by compare_entities now uses id instead of name as key if either property does not have a name * [#127](https://gitlab.com/linkahead/linkahead-pylib/-/issues/127) pylinkahead.ini now supports None and tuples as values for the `timeout` keyword diff --git a/DEPENDENCIES.md b/DEPENDENCIES.md index e2326b831a71751265c6c2d5a333ccc37145bfa5..e9bd54a1459df22afa307e256625d05e74bdc6a8 100644 --- a/DEPENDENCIES.md +++ b/DEPENDENCIES.md @@ -1,5 +1,5 @@ * caosdb-server >= 0.12.0 -* Python >= 3.8 +* Python >= 3.9 * pip >= 20.0.2 Any other dependencies are defined in the setup.py and are being installed via pip diff --git a/README_SETUP.md b/README_SETUP.md index 8fc93474f31697112fcc237ca890e0c0c4603ffa..f4c921382edb26776391590298faed06a5391396 100644 --- a/README_SETUP.md +++ b/README_SETUP.md @@ -4,7 +4,7 @@ ### How to install ### -First ensure that python with at least version 3.8 is installed. Should this not be +First ensure that python with at least version 3.9 is installed. Should this not be the case, you can use the [Installing python](#installing-python-) guide for your OS. #### Generic installation #### @@ -39,7 +39,7 @@ entries `install_requires` and `extras_require`. #### Linux #### -Make sure that Python (at least version 3.8) and pip is installed, using your system tools and +Make sure that Python (at least version 3.9) and pip is installed, using your system tools and documentation. Then open a terminal and continue in the [Generic installation](#generic-installation) section. diff --git a/setup.py b/setup.py index daf36764ef043916bea44694bf0620505e6fcd9c..7a7bf9bdf4c01bd87681f232df44a3982381ac60 100755 --- a/setup.py +++ b/setup.py @@ -179,7 +179,7 @@ def setup_package(): "Topic :: Scientific/Engineering :: Information Analysis", ], packages=find_packages('src'), - python_requires='>=3.8', + python_requires='>=3.9', package_dir={'': 'src'}, install_requires=['lxml>=4.6.3', "requests[socks]>=2.26", diff --git a/src/doc/tutorials/complex_data_models.rst b/src/doc/tutorials/complex_data_models.rst index 168cf3b9f0d6839ed8f78beb01ae24fb9d489e88..6c9520c94105e4106343ed2ab1a4c807d669d977 100644 --- a/src/doc/tutorials/complex_data_models.rst +++ b/src/doc/tutorials/complex_data_models.rst @@ -78,7 +78,7 @@ Examples Finding parents and properties --------- +------------------------------ To find a specific parent or property of an Entity, its ParentList or PropertyList can be filtered using names, ids, or entities. A short example: @@ -126,3 +126,19 @@ entities. A short example: # Result: [p2_1] The filter function of ParentList works analogously. + +Finding entities in a Container +------------------------------- +In the same way as described above, Container can be filtered. +A short example: + +.. code-block:: python3 + + import linkahead as db + + # Setup a record with six properties + p1 = db.Property(id=101, name="Property 1") + p2 = db.Property(name="Property 2") + c = db.Container().extend([p1,p2]) + c.filter(name="Property 1") + # Result: [p1] diff --git a/src/linkahead/common/models.py b/src/linkahead/common/models.py index aff85556ce3bf13396b43444e68f81cd26606b9c..8bd8eacda1035761bbc7b9aff4134fc45f682da0 100644 --- a/src/linkahead/common/models.py +++ b/src/linkahead/common/models.py @@ -349,6 +349,15 @@ class Entity: def pickup(self, new_pickup): self.__pickup = new_pickup + @property # getter for _cuid + def cuid(self): + # Set if None? + return self._cuid + + @property # getter for _flags + def flags(self): + return self._flags.copy() # for dict[str, str] shallow copy is enough + def grant( self, realm: Optional[str] = None, @@ -1232,6 +1241,7 @@ class Entity: xml: Optional[etree._Element] = None, add_properties: INHERITANCE = "ALL", local_serialization: bool = False, + visited_entities: Optional[list] = None ) -> etree._Element: """Generate an xml representation of this entity. If the parameter xml is given, all attributes, parents, properties, and messages of this @@ -1239,14 +1249,25 @@ class Entity: Raise an error if xml is not a lxml.etree.Element - @param xml: an xml element to which all attributes, parents, - properties, and messages - are to be added. - - FIXME: Add documentation for the add_properties parameter. - FIXME: Add docuemntation for the local_serialization parameter. + Parameters + ---------- + xml : etree._Element, optional + an xml element to which all attributes, parents, + properties, and messages are to be added. Default is None. + visited_entities : list, optional + list of enties that are being printed for recursion check, + should never be set manually. Default is None. + add_properties : INHERITANCE, optional + FIXME: Add documentation for the add_properties + parameter. Default is "ALL". + local_serialization : bool, optional + FIXME: Add documentation for the local_serialization + parameter. Default is False. - @return: xml representation of this entity. + Returns + ------- + xml : etree._Element + xml representation of this entity. """ if xml is None: @@ -1255,9 +1276,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 = xml2str(etree.Comment("Recursive reference")) + 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 +1301,10 @@ class Entity: xml.set("description", str(self.description)) if self.version is not None: + # If this ever causes problems, we might add + # visited_entities support here since it does have some + # recursion with predecessors / successors. But should be + # fine for now, since it is always set by the server. xml.append(self.version.to_xml()) if self.value is not None: @@ -1281,7 +1314,8 @@ class Entity: elif self.value.name is not None: xml.text = str(self.value.name) else: - xml.text = str(self.value) + dt_str = xml2str(self.value.to_xml(visited_entities=visited_entities.copy())) + xml.text = dt_str elif isinstance(self.value, list): for v in self.value: v_elem = etree.Element("Value") @@ -1292,7 +1326,8 @@ class Entity: elif v.name is not None: v_elem.text = str(v.name) else: - v_elem.text = str(v) + dt_str = xml2str(v.to_xml(visited_entities=visited_entities.copy())) + v_elem.text = dt_str elif v == "": v_elem.append(etree.Element("EmptyString")) elif v is None: @@ -1314,7 +1349,11 @@ 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 +1376,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 +1988,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 +2068,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 +2238,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 +2282,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 +2364,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 +2374,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 +2541,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)) @@ -2515,8 +2585,6 @@ class PropertyList(list): Params ------ - listobject : Iterable(Property) - List to be filtered prop : Property Property to match name and ID with. Cannot be set simultaneously with ID or name. @@ -2631,7 +2699,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") @@ -3028,12 +3101,12 @@ def _basic_sync(e_local, e_remote): if e_local.role is None: e_local.role = e_remote.role elif e_remote.role is not None and not e_local.role.lower() == e_remote.role.lower(): - raise ValueError("The resulting entity had a different role ({0}) " - "than the local one ({1}). This probably means, that " + raise ValueError(f"The resulting entity had a different role ({e_remote.role}) " + f"than the local one ({e_local.role}). This probably means, that " "the entity was intialized with a wrong class " "by this client or it has changed in the past and " - "this client did't know about it yet.".format( - e_remote.role, e_local.role)) + "this client did't know about it yet.\nThis is the local version of the" + f" Entity:\n{e_local}\nThis is the remote one:\n{e_remote}") e_local.id = e_remote.id e_local.name = e_remote.name @@ -3665,6 +3738,37 @@ class Container(list): return sync_dict + def filter(self, entity: Optional[Entity] = None, + pid: Union[None, str, int] = None, + name: Optional[str] = None, + conjunction: bool = False) -> list: + """ + Return all Entities from this Container that match the selection criteria. + + Please refer to the documentation of _filter_entity_list for a detailed + description of behaviour. + + Params + ------ + entity : Entity + Entity to match name and ID with + pid : str, int + Parent ID to match + name : str + Parent name to match + simultaneously with ID or name. + conjunction : bool, defaults to False + Set to return only entities that match both id and name + if both are given. + + Returns + ------- + matches : list + List containing all matching Entities + """ + return _filter_entity_list(self, pid=pid, name=name, entity=entity, + conjunction=conjunction) + @staticmethod def _find_dependencies_in_container(container: Container): """Find elements in a container that are a dependency of another element of the same. diff --git a/src/linkahead/exceptions.py b/src/linkahead/exceptions.py index 609d3654ac670a993185ba1faa33db921c44409c..7d4dc0850b811c0d696cc66252aa62541c6d3029 100644 --- a/src/linkahead/exceptions.py +++ b/src/linkahead/exceptions.py @@ -94,12 +94,26 @@ class HTTPServerError(LinkAheadException): """HTTPServerError represents 5xx HTTP server errors.""" def __init__(self, body): - xml = etree.fromstring(body) - error = xml.xpath('/Response/Error')[0] - msg = error.get("description") - - if error.text is not None: - msg = msg + "\n\n" + error.text + try: + # This only works if the server sends a valid XML + # response. Then it can be parsed for more information. + xml = etree.fromstring(body) + if xml.xpath('/Response/Error'): + error = xml.xpath('/Response/Error')[0] + msg = error.get("description") if error.get("description") is not None else "" + + if error.text is not None: + if msg: + msg = msg + "\n\n" + error.text + else: + msg = error.text + else: + # Valid XML, but no error information + msg = body + except etree.XMLSyntaxError: + # Handling of incomplete responses, e.g., due to timeouts, + # c.f. https://gitlab.com/linkahead/linkahead-pylib/-/issues/87. + msg = body LinkAheadException.__init__(self, msg) diff --git a/unittests/test_container.py b/unittests/test_container.py index c3a60140d43383c81f03c38c9dd5cc7779bc77ba..715a6eb8f409c489413120ca72857808029d323a 100644 --- a/unittests/test_container.py +++ b/unittests/test_container.py @@ -70,7 +70,8 @@ def test_get_property_values(): ) assert len(table) == 2 house_row = table[0] - assert house_row == (house.name, 40.2, "ft", window.id, None, None, None, 20.5, 20.5, "m", owner.name) + assert house_row == (house.name, 40.2, "ft", window.id, None, None, None, 20.5, 20.5, "m", + owner.name) owner_row = table[1] assert owner_row == (owner.name, None, None, None, None, None, None, None, None, None, None) @@ -199,3 +200,13 @@ def test_container_slicing(): with pytest.raises(TypeError): cont[[0, 2, 3]] + + +def test_container_filter(): + # this is a very rudimentary test since filter is based on _filter_entity_list which is tested + # separately + cont = db.Container() + cont.extend([db.Record(name=f"TestRec{ii+1}") for ii in range(5)]) + recs = cont.filter(name="TestRec2") + assert len(recs) == 1 + recs[0].name == "TestRec2" diff --git a/unittests/test_error_handling.py b/unittests/test_error_handling.py index 3f5241466e9a8f810b581cbb587e17ccf8f123ee..64f743c85e9df554e7428cf7d8477e8c823a9758 100644 --- a/unittests/test_error_handling.py +++ b/unittests/test_error_handling.py @@ -30,7 +30,7 @@ import linkahead as db from linkahead.common.models import raise_errors from linkahead.exceptions import (AuthorizationError, EntityDoesNotExistError, EntityError, - EntityHasNoDatatypeError, + EntityHasNoDatatypeError, HTTPServerError, TransactionError, UniqueNamesError, UnqualifiedParentsError, UnqualifiedPropertiesError) @@ -315,3 +315,26 @@ def test_container_with_faulty_elements(): # record raises both of them assert (isinstance(err, UnqualifiedParentsError) or isinstance(err, UnqualifiedPropertiesError)) + + +def test_incomplete_server_error_response(): + """The reason behind https://gitlab.com/linkahead/linkahead-pylib/-/issues/87.""" + # Case 1: Response is no XML at all + err = HTTPServerError("Bla") + assert str(err) == "Bla" + + # Case 2: Response is an incomplete XML, e.g. due to very unlucky timeout + err = HTTPServerError("<incomplete>XML</inc") + assert str(err) == "<incomplete>XML</inc" + + # Case 3: Response is complete XML but doesn't have response and or error information + err = HTTPServerError("<complete>XML</complete>") + assert str(err) == "<complete>XML</complete>" + + # Case 4: Response is an XML response but the error is lacking a description + err = HTTPServerError("<Response><Error>complete error</Error></Response>") + assert str(err) == "complete error" + + # Case 5: Healthy error Response + err = HTTPServerError("<Response><Error description='Error'>complete error</Error></Response>") + assert str(err) == "Error\n\ncomplete error" diff --git a/unittests/test_issues.py b/unittests/test_issues.py index e24afbe8b7be8d9a87d85819eccd3a4bf0d453e8..3b0117b28c1300ea1eb0919fce02e3881c2ab025 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,40 @@ 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 "Recursive reference" 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 "Recursive reference" 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 "Recursive reference" in xml_str + assert len(xml_str) < 500