Get parents recursively

Summary

Entity.get_parents_recursively() did not work for unretrieved parents. Does work now. See #159 (closed).

Focus

• Does this solve the issue?
• Is the wrapped-entity mangling the correct way? (If not, how could tests against this look like?)

Test Environment

See pyinttests: pytest-3 -k recurs or pytest-3 tests/test_recursive_parents.py

Check List for the Author

Please, prepare your MR for a review. Be sure to write a summary and a focus and create gitlab comments for the reviewer. They should guide the reviewer through the changes, explain your changes and also point out open questions. For further good practices have a look at our review guidelines

• All automated tests pass
• Reference related issues
• Up-to-date CHANGELOG.md (or not necessary)
• Appropriate user and developer documentation (or not necessary)
  • How do I use the software? Assume "stupid" users.
  • How do I develop or debug the software? Assume novice developers.
• Annotations in code (Gitlab comments)
  • Intent of new code
  • Problems with old code
  • Why this implementation?

Check List for the Reviewer

• I understand the intent of this MR
• All automated tests pass
• Up-to-date CHANGELOG.md (or not necessary)
• Appropriate user and developer documentation (or not necessary)
• The test environment setup works and the intended behavior is reproducible in the test environment
• In-code documentation and comments are up-to-date.
• Check: Are there specifications? Are they satisfied?

For further good practices have a look at our review guidelines.

src/caosdb/common/models.py

@@ -34,6 +34,7 @@ All additional classes are either important for the entities or the
 transactions.
 """
 from __future__ import print_function, unicode_literals
+from __future__ import annotations  # Can be removed with 3.10.
 
 import re
 import sys
@@ -82,7 +83,7 @@ SPECIAL_ATTRIBUTES = ["name", "role", "datatype", "description",
                       "id", "path", "checksum", "size"]
 
 
-class Entity(object):
+class Entity:
 
     """Entity is a generic CaosDB object.
 
@@ -621,26 +622,46 @@ class Entity(object):
 
         return self
 
-    def has_parent(self, parent, recursive=True,
-                   check_name=True, check_id=False):
-        """Checks if this entity has a given parent.
+    def has_parent(self, parent: Entity, recursive: bool = True, retrieve: bool = True,
+                   check_name: bool = True, check_id: bool = False):
+        """Check if this entity has a given parent.
 
         If 'check_name' and 'check_id' are both False, test for identity
         on the Python level. Otherwise use the name and/or ID for the
         check. Note that, if checked, name or ID should not be None,
         lest the check fail.
 
-        @param parent: Check for this parent.
-        @param recursive: Whether to check recursively.
-        @param check_name: Whether to use the name for ancestry check.
-        @param check_id: Whether to use the ID for ancestry check.
-        @return: True if 'parent' is a true parent, False otherwise.
-        """
+Parameters
+----------
+
+parent: Entity
+  Check for this parent.
+
+recursive: bool, optional
+  Whether to check recursively.
+
+check_name: bool, optional
+  Whether to use the name for ancestry check.
+
+check_id: bool, optional
+  Whether to use the ID for ancestry check.
+
+retrieve: bool, optional
+  If False, do not retrieve parents from the server.
+
+Returns
+-------
+out: bool
+  True if ``parent`` is a true parent, False otherwise.
+"""
 
         if recursive:
-            parents = self.get_parents_recursively()
+            parents = self.get_parents_recursively(retrieve=retrieve)
         else:
-            parents = [pp._wrapped_entity for pp in self.parents]
+            if retrieve:
+                parents = [pp.retrieve()._wrapped_entity for pp in self.parents]
+            else:
+                parents = [pp._wrapped_entity for pp in self.parents]
 
         if not (check_name or check_id):
             return parent in parents
@@ -664,18 +685,27 @@ class Entity(object):
 
         return self.parents
 
-    def get_parents_recursively(self):
+    def get_parents_recursively(self, retrieve: bool = True):
         """Get all ancestors of this entity.
 
-        @return: list of Entities
-        """
+Parameters
+----------
+
+retrieve: bool, optional
+  If False, do not retrieve parents from the server.
+
+Returns
+-------
+out: List[Entity]
+  The parents of this Entity
+"""
 
         all_parents = _Parents()
-        self._get_parent_recursively(all_parents)
+        self._get_parent_recursively(all_parents, retrieve=retrieve)
 
         return all_parents
 
-    def _get_parent_recursively(self, all_parents):
+    def _get_parent_recursively(self, all_parents, retrieve: bool = True):
         """Get all ancestors with a little helper.
 
         As a side effect of this method, the ancestors are added to
@@ -688,10 +718,15 @@ class Entity(object):
 
         for parent in self.parents:
             w_parent = parent._wrapped_entity
+            if retrieve:
+                parent.retrieve()
+                for next_parent in parent.parents:
+                    w_parent.add_parent(next_parent)
 
-            if w_parent not in all_parents:
+            if (w_parent.id, w_parent.name) not in [
+                    (all_p.id, all_p.name) for all_p in all_parents]:
                 all_parents.append(w_parent)
-                w_parent._get_parent_recursively(all_parents)
+                w_parent._get_parent_recursively(all_parents, retrieve=retrieve)
 
     def get_parent(self, key):
         """Return the first parent matching the key or None if no match exists.