From ee7e499c0b65c8281395f6cbcf2794acf7ae60ad Mon Sep 17 00:00:00 2001 From: David Reinhart Date: Thu, 24 Aug 2023 14:07:11 -0700 Subject: [PATCH 1/3] Allow `sample.access_role` value to be set via REST API Allow `access_role` value to be set via warehouse/sample POST endpoint. This column is used in the table's row-level security policy to restrict access to specific database roles as needed. --- lib/id3c/api/datastore.py | 5 ++++- lib/id3c/api/schemas.py | 5 ++++- lib/id3c/db/__init__.py | 23 +++++++++++++++++------ 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/lib/id3c/api/datastore.py b/lib/id3c/api/datastore.py index fafcd6aa..53987328 100644 --- a/lib/id3c/api/datastore.py +++ b/lib/id3c/api/datastore.py @@ -468,6 +468,8 @@ def store_sample(session: DatabaseSession, sample: dict) -> Any: collection_barcode = sample.pop("collection_id", None) collection_identifier = find_identifier(session, collection_barcode) if collection_barcode else None + access_role = sample.pop("access_role", None) + result = { "sample_barcode": sample_barcode, "collection_barcode": collection_barcode @@ -517,7 +519,8 @@ def store_sample(session: DatabaseSession, sample: dict) -> Any: collection_identifier = collection_identifier.uuid if collection_identifier else None, collection_date = collected_date, encounter_id = None, - additional_details = sample) + additional_details = sample, + access_role = access_role) result["sample"] = sample result["status"] = status diff --git a/lib/id3c/api/schemas.py b/lib/id3c/api/schemas.py index 5d3a35af..8976ffe8 100644 --- a/lib/id3c/api/schemas.py +++ b/lib/id3c/api/schemas.py @@ -90,7 +90,10 @@ }, "notes": { "type": "string" - } + }, + "access_role": { + "type": "string" + }, }, "anyOf": [ { "required": diff --git a/lib/id3c/db/__init__.py b/lib/id3c/db/__init__.py index c904d895..3bb58dfc 100644 --- a/lib/id3c/db/__init__.py +++ b/lib/id3c/db/__init__.py @@ -186,7 +186,8 @@ def upsert_sample(db: DatabaseSession, collection_identifier: Optional[str], collection_date: Optional[str], encounter_id: Optional[int], - additional_details: dict) -> Tuple[Any, str]: + additional_details: dict, + access_role: Optional[str] = None) -> Tuple[Any, str]: """ Upsert sample by its *identifier* and/or *collection_identifier*. An existing sample has its *identifier*, *collection_identifier*, @@ -200,13 +201,14 @@ def upsert_sample(db: DatabaseSession, "collection_date": collection_date, "encounter_id": encounter_id, "additional_details": Json(additional_details) if additional_details else None, + "access_role": access_role, } # Look for existing sample(s) with db.cursor() as cursor: cursor.execute(""" select - sample_id as id, identifier, collection_identifier, encounter_id, details, + sample_id as id, identifier, collection_identifier, encounter_id, details, access_role, row ( identifier, collection_identifier @@ -224,7 +226,8 @@ def upsert_sample(db: DatabaseSession, coalesce(%(collection_date)s, collected)::timestamp, coalesce(%(encounter_id)s::integer, encounter_id), coalesce(details, '{}'::jsonb) || coalesce(%(additional_details)s, '{}')::jsonb - )::text as metadata_changed + )::text as metadata_changed, + row(access_role)::text != row(coalesce(%(access_role)s, access_role))::text as access_role_changed from warehouse.sample where identifier = %(identifier)s or collection_identifier = %(collection_identifier)s @@ -239,12 +242,13 @@ def upsert_sample(db: DatabaseSession, status = 'created' sample = db.fetch_row(""" - insert into warehouse.sample (identifier, collection_identifier, collected, encounter_id, details) + insert into warehouse.sample (identifier, collection_identifier, collected, encounter_id, details, access_role) values (%(identifier)s, %(collection_identifier)s, date_or_null(%(collection_date)s), %(encounter_id)s, - %(additional_details)s) + %(additional_details)s, + %(access_role)s) returning sample_id as id, identifier, collection_identifier, encounter_id """, data) @@ -256,9 +260,10 @@ def upsert_sample(db: DatabaseSession, LOG.info(f"Updating existing sample {sample.id}") LOG.info(f"Sample.identifiers_changed is «{sample.identifiers_changed}» ") LOG.info(f"Sample.metadata_changed is «{sample.metadata_changed}» ") + LOG.info(f"Sample.access_role_changed is «{sample.access_role_changed}» ") # can safely skip upsert if metadata is unchanged and not updating identifiers or if all data is unchanged - if sample.metadata_changed == False and (not update_identifiers or sample.identifiers_changed == False): + if sample.metadata_changed == False and sample.access_role_changed == False and (not update_identifiers or sample.identifiers_changed == False): LOG.info(f"Skipping upsert for sample {sample.id} «{sample.identifier}» (no change).") return sample, status @@ -286,9 +291,14 @@ def upsert_sample(db: DatabaseSession, if overwrite_collection_date else SQL(""" collected = coalesce(collected, date_or_null(%(collection_date)s)), """) + # Update access_role if value changed + access_role_update_composable = SQL(""" + access_role = %(access_role)s, """) if sample.access_role_changed else SQL("") + sample = db.fetch_row(SQL(""" update warehouse.sample set {} + {} {} encounter_id = coalesce(%(encounter_id)s, encounter_id), details = coalesce(details, {}) || %(additional_details)s @@ -296,6 +306,7 @@ def upsert_sample(db: DatabaseSession, returning sample_id as id, identifier, collection_identifier, encounter_id """).format(identifiers_update_composable, collected_update_composable, + access_role_update_composable, Literal(Json({}))), { **data, "sample_id": sample.id }) From 4e2b711f077255d878d132d021ecd46370cb1f86 Mon Sep 17 00:00:00 2001 From: David Reinhart Date: Thu, 24 Aug 2023 14:25:09 -0700 Subject: [PATCH 2/3] Remove `_provenance` from sample.details for comparison purposes When detecting data changes in sample upsert function, timestamp changes in the `_provenance` were causing otherwise unchanged records to be updated. Removing this value for comparison purposes in a copy of the dict, and retaining the original for insert and update. --- lib/id3c/db/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/id3c/db/__init__.py b/lib/id3c/db/__init__.py index 3bb58dfc..5b4a68ac 100644 --- a/lib/id3c/db/__init__.py +++ b/lib/id3c/db/__init__.py @@ -201,6 +201,7 @@ def upsert_sample(db: DatabaseSession, "collection_date": collection_date, "encounter_id": encounter_id, "additional_details": Json(additional_details) if additional_details else None, + "additional_details_without_prov": Json({k: additional_details[k] for k in additional_details if k != '_provenance'}) if additional_details else None, "access_role": access_role, } @@ -225,7 +226,7 @@ def upsert_sample(db: DatabaseSession, row( coalesce(%(collection_date)s, collected)::timestamp, coalesce(%(encounter_id)s::integer, encounter_id), - coalesce(details, '{}'::jsonb) || coalesce(%(additional_details)s, '{}')::jsonb + coalesce(details, '{}'::jsonb) || coalesce(%(additional_details_without_prov)s, '{}')::jsonb )::text as metadata_changed, row(access_role)::text != row(coalesce(%(access_role)s, access_role))::text as access_role_changed from warehouse.sample From c2126ad1bcc3aa37f6bfead651ad72b5685a8652 Mon Sep 17 00:00:00 2001 From: David Reinhart Date: Fri, 25 Aug 2023 14:55:07 -0700 Subject: [PATCH 3/3] Update API doc page --- lib/id3c/api/static/index.html | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/id3c/api/static/index.html b/lib/id3c/api/static/index.html index f15dfb16..7780a279 100644 --- a/lib/id3c/api/static/index.html +++ b/lib/id3c/api/static/index.html @@ -143,7 +143,14 @@

POST /v1/warehouse/sample

"collection_matrix": "dry" } - +

An "access_role" value is required for samples from specific projects to enable row-level security: + 

+          {
+            ...
+            "access_role": "some_db_role"
+            ...
+          }
+

GET /v1/warehouse/identifier-set-uses

Retrieve metadata about all identifier set uses.