♻️ Fill in acl w legacy values, only if empty

kf_update_dbgap_consent/sample_status.py

@@ -64,11 +64,45 @@
 from kf_utils.dbgap.release import get_latest_sample_status
 
 
+legacy_acl = {
+    "open_acl": ["*"],
+    "empty_acl": [],
+}
+
+
 def is_localhost(url):
     hosts = {"localhost", "127.0.0.1"}
     return any(hostname in url for hostname in hosts)
 
 
+def filter_out_acl_overwrites(storage, patches):
+    """
+    Remove `acl` patches if the GF already has a value for the acl
+    We want to preserve current `acl` values in Dataservice
+    """
+
+    def is_valid(kf_id, k, v):
+        if k != "acl":
+            return True
+
+        gf = storage["genomic-files"].get(kf_id, {})
+        if not gf:
+            return False
+
+        if gf.get("acl"):
+            return False
+        else:
+            return True
+
+    return {
+        endpoint: {
+            kfid: {k: v for k, v in patch.items() if is_valid(kfid, k, v)}
+            for kfid, patch in ep_patches.items()
+        }
+        for endpoint, ep_patches in patches.items()
+    }
+
+
 class ConsentProcessor:
     def __init__(self, api_url, db_url=None):
         self.api_url = api_url
@@ -277,7 +311,10 @@ def entities_dict(endpoint, filt):
                     `{open_acl}`.
                     """
                     patches["genomic-files"][gfid].update(
-                        {"authz": sorted(open_acl)}
+                        {
+                            "authz": sorted(open_acl),
+                            "acl": legacy_acl["open_acl"],
+                        }
                     )
                 elif controlled_access == True:
                     """
@@ -304,7 +341,8 @@ def entities_dict(endpoint, filt):
                                     f"/programs/{code}"
                                     for code in biospecimen_codes
                                 ]
-                            )
+                            ),
+                            "acl": sorted([code for code in biospecimen_codes]),
                         }
                     )
             # GenomicFile visible = False OR one of contributing Biospecimen
@@ -315,10 +353,12 @@ def entities_dict(endpoint, filt):
                 `{empty_acl}` indicating no access.
                 """
                 patches["genomic-files"][gfid].update(
-                    {"authz": sorted(empty_acl)}
+                    {"authz": sorted(empty_acl), "acl": legacy_acl["empty_acl"]}
                 )
 
         # remove known unneeded patches
+        auth_fields = set(["authz", "acl"])
+
         def cmp(a, b, field_name):
             # Values get filtered out if they are equal to what
             # is already in dataservice.
@@ -328,7 +368,7 @@ def cmp(a, b, field_name):
             # authz = [], this will get filtered out and
             # tests will fail
             # So when testing with localhost we force a patch with authz
-            if field_name == "authz" and is_localhost(self.api_url):
+            if (field_name in auth_fields) and is_localhost(self.api_url):
                 return False
 
             if isinstance(a, list) and isinstance(b, list):
@@ -358,6 +398,9 @@ def cmp(a, b, field_name):
         }
         patches = {k: v for k, v in patches.items() if v}
 
+        # Filter out any patches that would overwrite current GF `acl` values
+        patches = filter_out_acl_overwrites(storage, patches)
+
         # from pprint import pprint
         # breakpoint()
 

tests/data/phs999999_patches.json

@@ -36,24 +36,29 @@
   "genomic-files": {
     "GF_00000000": {
       "authz": [],
+      "acl": [],
       "visible": false,
       "visibility_reason": "Consent Hold",
       "visibility_comment": "Sample is not registered in dbGaP"
     },
     "GF_11111111": {
       "authz": [],
+      "acl": [],
       "visible": false,
       "visibility_reason": "Consent Hold",
       "visibility_comment": "Sample is not registered in dbGaP"
     },
     "GF_22222222": {
-      "authz": ["/programs/phs999999.c1"]
+      "authz": ["/programs/phs999999.c1"],
+      "acl": ["phs999999.c1"]
     },
     "GF_33333333": {
-      "authz": ["/programs/phs999999.c2"]
+      "authz": ["/programs/phs999999.c2"],
+      "acl": ["phs999999.c2"]
     },
     "GF_44444444": {
-      "authz": ["/open"]
+      "authz": ["/open"],
+      "acl": ["*"]
     }
   },
   "sequencing-experiment-genomic-files": {

tests/test_sample_status.py

@@ -4,7 +4,10 @@
 import pytest
 from d3b_utils.requests_retry import Session
 
-from kf_update_dbgap_consent.sample_status import ConsentProcessor
+from kf_update_dbgap_consent.sample_status import (
+    ConsentProcessor,
+    filter_out_acl_overwrites
+)
 
 host = "http://localhost:5000"
 
@@ -126,3 +129,39 @@ def test_sample_status(requests_mock):
         "dbgap_consent_code": None,
     }
     compare(patches, new_expected_patches)
+
+
+def test_filter_out_acl_overwrites():
+    """
+    Test filter_out_acl_overwrites method
+    """
+    storage = {}
+    patches = {}
+    storage["genomic-files"] = {}
+    patches["genomic-files"] = {}
+
+    # Overwrite an ACL that has not been set
+    storage["genomic-files"]["GF_1"] = {
+        "acl": []
+    }
+    patches["genomic-files"]["GF_1"] = {
+        "acl": ["foo"],
+        "visible": True
+    }
+    patches = filter_out_acl_overwrites(storage, patches)
+    assert patches["genomic-files"]["GF_1"] == {
+        "acl": ["foo"],
+        "visible": True
+    }
+    # Overwrite an ACL that is set - should fail
+    storage["genomic-files"]["GF_1"] = {
+        "acl": ["foo"]
+    }
+    patches["genomic-files"]["GF_1"] = {
+        "acl": ["bar"],
+        "visible": True
+    }
+    patches = filter_out_acl_overwrites(storage, patches)
+    assert patches["genomic-files"]["GF_1"] == {
+        "visible": True
+    }