From 8c1d750bcc3b592e62764f3e7bcdef6cbb6ab30b Mon Sep 17 00:00:00 2001
From: alaskowski <alaskowski@ethz.ch>
Date: Tue, 7 Feb 2023 14:25:46 +0100
Subject: [PATCH] SSDM-13417: Changed update in transaction mechanism so None
 list attributes are not counted as updated. Created a test that verifies that
 update based on get_samples method is not overriding parents/children

---
 pybis/src/python/pybis/attribute.py    |  13 ++-
 pybis/src/python/pybis/pybis.py        |   1 +
 pybis/src/python/pybis/sample.py       |  12 +-
 pybis/src/python/tests/test_openbis.py | 155 +++++++++++++++++++++++++
 4 files changed, 176 insertions(+), 5 deletions(-)

diff --git a/pybis/src/python/pybis/attribute.py b/pybis/src/python/pybis/attribute.py
index bf767fc3626..a9fa8589e70 100644
--- a/pybis/src/python/pybis/attribute.py
+++ b/pybis/src/python/pybis/attribute.py
@@ -284,8 +284,9 @@ class AttrHolder:
                 # all items in a list
                 if "multi" in self._defs and attr in self._defs["multi"]:
                     items = self.__dict__.get("_" + attr, [])
-                    if items == None:
-                        items = []
+                    if items is None:
+                        # 'None' multivalue attributes should be omitted during update
+                        continue
                     up_obj[attr2ids[attr]] = {
                         "actions": [
                             {
@@ -745,6 +746,8 @@ class AttrHolder:
         """add parent to _parents list"""
         if not isinstance(parents_to_add, list):
             parents_to_add = [parents_to_add]
+        if self.__dict__["_parents"] is None:
+            self.__dict__["_parents"] = []
         for parent in parents_to_add:
             ident = self._ident_for_whatever(parent)
             if ident not in self.__dict__["_parents"]:
@@ -754,6 +757,8 @@ class AttrHolder:
         """remove parent from _parents list"""
         if not isinstance(parents_to_remove, list):
             parents_to_remove = [parents_to_remove]
+        if self.__dict__["_parents"] is None:
+            self.__dict__["_parents"] = []
         for parent in parents_to_remove:
             ident = self._ident_for_whatever(parent)
             for i, item in enumerate(self.__dict__["_parents"]):
@@ -787,6 +792,8 @@ class AttrHolder:
         """add children to _children list"""
         if getattr(self, "_children") is None:
             self.__dict__["_children"] = []
+        if self.__dict__["_children"] is None:
+            self.__dict__["_children"] = []
         if not isinstance(children, list):
             children = [children]
         for child in children:
@@ -798,6 +805,8 @@ class AttrHolder:
             return
         if not isinstance(children, list):
             children = [children]
+        if self.__dict__["_children"] is None:
+            self.__dict__["_children"] = []
         for child in children:
             ident = self._ident_for_whatever(child)
             for i, item in enumerate(self.__dict__["_children"]):
diff --git a/pybis/src/python/pybis/pybis.py b/pybis/src/python/pybis/pybis.py
index 07e3567e2cf..f8e118db98c 100644
--- a/pybis/src/python/pybis/pybis.py
+++ b/pybis/src/python/pybis/pybis.py
@@ -4811,6 +4811,7 @@ class Openbis:
                         openbis_obj=self,
                         type=self.get_sample_type(obj["type"]["code"]),
                         data=obj,
+                        attrs=attrs
                     ),
                     response,
                 )
diff --git a/pybis/src/python/pybis/sample.py b/pybis/src/python/pybis/sample.py
index 790e9681c4b..9311815a333 100644
--- a/pybis/src/python/pybis/sample.py
+++ b/pybis/src/python/pybis/sample.py
@@ -9,7 +9,7 @@ class Sample(OpenBisObject, entity="sample", single_item_method_name="get_sample
     """A Sample (new: Object) is one of the most commonly used entities in openBIS."""
 
     def __init__(
-        self, openbis_obj, type, project=None, data=None, props=None, **kwargs
+        self, openbis_obj, type, project=None, data=None, props=None, attrs=None, **kwargs
     ):
         self.__dict__["openbis"] = openbis_obj
         self.__dict__["type"] = type
@@ -49,13 +49,19 @@ class Sample(OpenBisObject, entity="sample", single_item_method_name="get_sample
             self.a.__dict__["_parents"] = []
         else:
             if not self.is_new:
-                self.a.__dict__["_parents_orig"] = self.a.__dict__["_parents"]
+                if attrs is not None and "parents" not in attrs:
+                    self.a.__dict__["_parents"] = None
+                else:
+                    self.a.__dict__["_parents_orig"] = self.a.__dict__["_parents"]
 
         if getattr(self, "children") is None:
             self.a.__dict__["_children"] = []
         else:
             if not self.is_new:
-                self.a.__dict__["_children_orig"] = self.a.__dict__["_children"]
+                if attrs is not None and "children" not in attrs:
+                    self.a.__dict__["_children"] = None
+                else:
+                    self.a.__dict__["_children_orig"] = self.a.__dict__["_children"]
 
         if getattr(self, "components") is None:
             self.a.__dict__["_components"] = []
diff --git a/pybis/src/python/tests/test_openbis.py b/pybis/src/python/tests/test_openbis.py
index 4215263aef6..8db5be30fbb 100644
--- a/pybis/src/python/tests/test_openbis.py
+++ b/pybis/src/python/tests/test_openbis.py
@@ -41,3 +41,158 @@ def test_create_permId(openbis_instance):
     assert ts is not None
     count = m.group(1)
     assert count is not None
+
+
+def test_get_samples_update_in_transaction(openbis_instance):
+    '''
+        Update samples in transaction without overriding parents/children
+    '''
+    name_suffix = str(time.time())
+    # Create new space
+    space = openbis_instance.new_space(code='space_name' + name_suffix, description='')
+    space.save()
+
+    # Create new project
+    project = space.new_project(code='project_code' + name_suffix)
+    project.save()
+
+    # Create new experiment
+    experiment = openbis_instance.new_experiment(
+        code='MY_NEW_EXPERIMENT',
+        type='DEFAULT_EXPERIMENT',
+        project=project.code
+    )
+    experiment.save()
+
+    # Create parent sample
+    sample1 = openbis_instance.new_sample(
+        type='YEAST',
+        space=space.code,
+        experiment=experiment.identifier,
+        parents=[],
+        children=[],
+        props={"$name": "sample1"}
+    )
+    sample1.save()
+
+    # Create child sample
+    sample2 = openbis_instance.new_sample(
+        type='YEAST',
+        space=space.code,
+        experiment=experiment.identifier,
+        parents=[sample1],
+        children=[],
+        props={"$name": "sample2"}
+    )
+    sample2.save()
+
+    # Verify samples parent/child relationship
+    sample1 = openbis_instance.get_sample(
+        sample_ident=sample1.identifier,
+        space=space.code,
+        props="*"
+    )
+    sample2 = openbis_instance.get_sample(
+        sample_ident=sample2.identifier,
+        space=space.code,
+        props="*"
+    )
+    assert sample1.children == [sample2.identifier]
+    assert sample2.parents == [sample1.identifier]
+
+    trans = openbis_instance.new_transaction()
+    # get samples that have parents and update name
+    samples = openbis_instance.get_samples(space=space.code, props="*", withParents="*")
+    for sample in samples:
+        sample.props["$name"] = 'new name for sample2'
+        trans.add(sample)
+    # get samples that have children and update name
+    samples = openbis_instance.get_samples(space=space.code, props="*", withChildren="*")
+    for sample in samples:
+        sample.props["$name"] = 'new name for sample1'
+        trans.add(sample)
+    trans.commit()
+
+    # Verify that name has been changed and parent/child relationship remains
+    sample1 = openbis_instance.get_sample(
+        sample_ident=sample1.identifier,
+        space=space.code,
+        props="*"
+    )
+    sample2 = openbis_instance.get_sample(
+        sample_ident=sample2.identifier,
+        space=space.code,
+        props="*"
+    )
+    assert sample1.props["$name"] == 'new name for sample1'
+    assert sample1.children == [sample2.identifier]
+    assert sample2.props["$name"] == 'new name for sample2'
+    assert sample2.parents == [sample1.identifier]
+
+    trans = openbis_instance.new_transaction()
+    # get samples with attributes and change name
+    samples = openbis_instance.get_samples(space=space.code, attrs=["parents", "children"])
+    for sample in samples:
+        sample.props["$name"] = "default name"
+        trans.add(sample)
+    trans.commit()
+
+    # Verify that name has been changed and parent/child relationship remains
+    sample1 = openbis_instance.get_sample(
+        sample_ident=sample1.identifier,
+        space=space.code,
+        props="*"
+    )
+    sample2 = openbis_instance.get_sample(
+        sample_ident=sample2.identifier,
+        space=space.code,
+        props="*"
+    )
+    assert sample1.props["$name"] == 'default name'
+    assert sample1.children == [sample2.identifier]
+    assert sample2.props["$name"] == 'default name'
+    assert sample2.parents == [sample1.identifier]
+
+    sample3 = openbis_instance.new_sample(
+        type='YEAST',
+        space=space.code,
+        experiment=experiment.identifier,
+        parents=[],
+        children=[],
+        props={"$name": "sample3"}
+    )
+    sample3.save()
+
+    trans = openbis_instance.new_transaction()
+    # get sample1 without attributes and add sample3 as a parent
+    samples = openbis_instance.get_samples(space=space.code, identifier=sample1.identifier)
+    for sample in samples:
+        sample.add_parents([sample3.identifier])
+        trans.add(sample)
+    # get sample2 without attributes and remove sample1 as a parent
+    samples = openbis_instance.get_samples(space=space.code, identifier=sample2.identifier)
+    for sample in samples:
+        sample.del_parents([sample1.identifier])
+        trans.add(sample)
+    trans.commit()
+
+    # Verify changes
+    sample1 = openbis_instance.get_sample(
+        sample_ident=sample1.identifier,
+        space=space.code,
+        props="*"
+    )
+    sample2 = openbis_instance.get_sample(
+        sample_ident=sample2.identifier,
+        space=space.code,
+        props="*"
+    )
+    sample3 = openbis_instance.get_sample(
+        sample_ident=sample3.identifier,
+        space=space.code,
+        props="*"
+    )
+    assert sample1.children == []
+    assert sample1.parents == [sample3.identifier]
+    assert sample2.parents == []
+    assert sample3.children == [sample1.identifier]
-- 
GitLab