Updated hash_value, started tests

pixl_dcmd/README.md

@@ -1,2 +1,9 @@
 # PIXL DICOM de-identifier
 
+pip install -r requirements.txt
+
+pip install -e ../../pixl_core/
+
+pip install -e .
+
+pytest

pixl_dcmd/src/pixl_config.yml

@@ -0,0 +1,32 @@
+#  Copyright (c) University College London Hospitals NHS Foundation Trust
+#
+#  Licensed under the Apache License, Version 2.0 (the "License");
+#  you may not use this file except in compliance with the License.
+#  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+rabbitmq:
+  host: localhost
+  port: 7008
+  username: rabbitmq_username
+  password: rabbitmq_password
+ehr_api:
+  host: localhost
+  port: 7006
+  default_rate: 1
+pacs_api:
+  host: localhost
+  port: 7007
+  default_rate: 1
+postgres:
+  host: localhost
+  port: 7001
+  username: pixl_db_username
+  password: pixl_db_password
+  database: pixl

pixl_dcmd/src/pixl_dcmd/_config.py

@@ -0,0 +1,32 @@
+#  Copyright (c) University College London Hospitals NHS Foundation Trust
+#
+#  Licensed under the Apache License, Version 2.0 (the "License");
+#  you may not use this file except in compliance with the License.
+#  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+
+"""Configuration of CLI for db from config file."""
+from pathlib import Path
+
+import yaml
+
+
+def _load_config(filename: str = "pixl_config.yml") -> dict:
+    """CLI configuration generated from a .yaml file"""
+    if not Path(filename).exists():
+        msg = f"Failed to find {filename}. It must be present in the current working directory"
+        raise FileNotFoundError(msg)
+
+    with Path(filename).open() as config_file:
+        config_dict = yaml.safe_load(config_file)
+    return dict(config_dict)
+
+
+cli_config = _load_config()

pixl_dcmd/src/pixl_dcmd/_database.py

@@ -15,10 +15,10 @@
 """Interaction with the PIXL database."""
 
 from core.database import Image
-from sqlalchemy import URL, create_engine, update
+from sqlalchemy import URL, create_engine
 from sqlalchemy.orm import sessionmaker
 
-from pixl_cli._config import cli_config
+from pixl_dcmd._config import cli_config
 
 connection_config = cli_config["postgres"]
 
@@ -34,41 +34,36 @@
 engine = create_engine(url)
 
 
-def insert_new_uid_into_db_entity(
-    mrn: str, accession_number: str, new_uid: str
-) -> None:
+def insert_new_uid_into_db_entity(existing_image: Image, hashed_value: str) -> Image:
     PixlSession = sessionmaker(engine)
     with PixlSession() as pixl_session, pixl_session.begin():
-        existing_image = (
+        existing_image.hashed_identifier = hashed_value
+        pixl_session.add(existing_image)
+
+        updated_image = (
             pixl_session.query(Image)
             .filter(
-                Image.accession_number == accession_number,
-                Image.mrn == mrn,
+                Image.accession_number == existing_image.accession_number,
+                Image.mrn == existing_image.mrn,
+                Image.hashed_identifier == hashed_value,
             )
             .one_or_none()
         )
 
-        if existing_image:
-            stmt = (
-                update(Image)
-                .where(Image.extract_id == existing_image.extract_id)
-                .values(hashed_identifier=new_uid)
-            )
-            pixl_session.execute(stmt)
+        return updated_image
 
 
-def query_db(mrn: str, accession_number: str) -> bool:
+def query_db(mrn: str, accession_number: str) -> Image:
     PixlSession = sessionmaker(engine)
     with PixlSession() as pixl_session, pixl_session.begin():
         existing_image = (
             pixl_session.query(Image)
             .filter(
                 Image.accession_number == accession_number,
                 Image.mrn == mrn,
+                Image.exported_at is None,
             )
-            .one_or_none()
+            .one()
         )
 
-        if existing_image.exported_at is not None:
-            return True
-        return False
+        return existing_image

pixl_dcmd/src/pixl_dcmd/main.py

@@ -25,7 +25,7 @@
 import requests
 from decouple import config
 from pydicom import Dataset, dcmwrite
-from _database import insert_new_uid_into_db_entity, query_db
+from pixl_dcmd._database import insert_new_uid_into_db_entity, query_db
 
 DicomDataSetType = Union[Union[str, bytes, PathLike[Any]], BinaryIO]
 
@@ -215,8 +215,6 @@ def apply_tag_scheme(dataset: dict, tags: dict) -> dict:
     mrn = dataset[0x0010, 0x0020].value  # Patient ID
     accession_number = dataset[0x0008, 0x0050].value  # Accession Number
 
-    # Query PIXL database
-    query_db(mrn, accession_number)
     salt_plaintext = mrn + accession_number
 
     HASHER_API_AZ_NAME = config("HASHER_API_AZ_NAME")
@@ -279,9 +277,6 @@ def apply_tag_scheme(dataset: dict, tags: dict) -> dict:
                 logging.info(f"\t\tCurrent UID:\t{dataset[grp,el].value}")
                 new_uid = get_encrypted_uid(dataset[grp, el].value, salt)
                 dataset[grp, el].value = new_uid
-
-                # Insert the new_uid into the PIXL database
-                insert_new_uid_into_db_entity(accession_number, mrn, new_uid)
                 logging.info(f"\t\tEncrypted UID:\t{new_uid}")
 
             else:
@@ -422,19 +417,23 @@ def apply_tag_scheme(dataset: dict, tags: dict) -> dict:
         # Change value into hash from hasher API.
         elif op == "secure-hash":
             if [grp, el] in dataset:
-                pat_value = str(dataset[grp, el].value)
-                ep_path = hash_endpoint_path_for_tag(group=grp, element=el)
-                payload = ep_path + "?message=" + pat_value
-                request_url = hasher_host_url + payload
-                response = requests.get(request_url)
-                logging.info(b"RESPONSE = %a}" % response.content)
+                if grp == 0x0010 and el == 0x0020:  # Patient ID
+                    pat_value = mrn + accession_number
+
+                    hashed_value = _hash_values(grp, el, pat_value, hasher_host_url)
+                    # Query PIXL database
+                    existing_image = query_db(mrn, accession_number)
+                    # Insert the hashed_value into the PIXL database
+                    insert_new_uid_into_db_entity(existing_image, hashed_value)
+                else:
+                    pat_value = str(dataset[grp, el].value)
 
-                new_value = response.content
+                    hashed_value = _hash_values(grp, el, pat_value, hasher_host_url)
 
                 if dataset[grp, el].VR == "SH":
-                    new_value = new_value[:16]
+                    hashed_value = hashed_value[:16]
 
-                dataset[grp, el].value = new_value
+                dataset[grp, el].value = hashed_value
 
                 message = f"Changing: {name} (0x{grp:04x},0x{el:04x})"
                 logging.info(f"\t{message}")
@@ -454,3 +453,13 @@ def hash_endpoint_path_for_tag(group: bytes, element: bytes) -> str:
         return "/hash-accession-number"
 
     return "/hash"
+
+
+def _hash_values(grp: bytes, el: bytes, pat_value: str, hasher_host_url: str) -> bytes:
+    ep_path = hash_endpoint_path_for_tag(group=grp, element=el)
+    payload = ep_path + "?message=" + pat_value
+    request_url = hasher_host_url + payload
+    response = requests.get(request_url)
+    logging.info(b"RESPONSE = %a}" % response.content)
+
+    return response.content

pixl_dcmd/src/pixl_dcmd/tests/conftest.py

@@ -0,0 +1,101 @@
+#  Copyright (c) University College London Hospitals NHS Foundation Trust
+#
+#  Licensed under the Apache License, Version 2.0 (the "License");
+#  you may not use this file except in compliance with the License.
+#  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+"""CLI testing fixtures."""
+from __future__ import annotations
+import datetime
+
+import pytest
+from core.database import Base, Extract, Image
+from dateutil.tz import UTC
+from sqlalchemy import Engine, create_engine
+from sqlalchemy.orm import Session, sessionmaker
+
+STUDY_DATE = datetime.date.fromisoformat("2023-01-01")
+
+
+@pytest.fixture()
+def rows_in_session(db_session) -> Session:
+    """Insert a test row for each table, returning the session for use in tests."""
+    extract = Extract(slug="i-am-a-project")
+
+    image_exported = Image(
+        accession_number="123",
+        study_date=STUDY_DATE,
+        mrn="mrn",
+        extract=extract,
+        exported_at=datetime.datetime.now(tz=UTC),
+    )
+    image_not_exported = Image(
+        accession_number="234",
+        study_date=STUDY_DATE,
+        mrn="mrn",
+        extract=extract,
+    )
+    with db_session:
+        db_session.add_all([extract, image_exported, image_not_exported])
+        db_session.commit()
+
+    return db_session
+
+
+@pytest.fixture(scope="module")
+def monkeymodule():
+    """Module level monkey patch."""
+    from _pytest.monkeypatch import MonkeyPatch
+
+    monkeypatch = MonkeyPatch()
+    yield monkeypatch
+    monkeypatch.undo()
+
+
+@pytest.fixture(autouse=True, scope="module")
+def db_engine(monkeymodule) -> Engine:
+    """
+    Patches the database engine with an in memory database
+
+    :returns Engine: Engine for use in other setup fixtures
+    """
+    # SQLite doesnt support schemas, so remove pixl schema from engine options
+    execution_options = {"schema_translate_map": {"pixl": None}}
+    engine = create_engine(
+        "sqlite:///:memory:",
+        execution_options=execution_options,
+        echo=True,
+        echo_pool="debug",
+        future=True,
+    )
+    monkeymodule.setattr("pixl_cli._database.engine", engine)
+
+    Base.metadata.create_all(engine)
+    yield engine
+    Base.metadata.drop_all(engine)
+
+
+@pytest.fixture()
+def db_session(db_engine) -> Session:
+    """
+    Creates a session for interacting with an in memory database.
+
+    Will remove any data from database in setup
+
+    :returns Session: Session for use in other setup fixtures.
+
+    """
+    InMemorySession = sessionmaker(db_engine)
+    with InMemorySession() as session:
+        # sqlite with sqlalchemy doesn't rollback, so manually deleting all database entities
+        session.query(Image).delete()
+        session.query(Extract).delete()
+        yield session
+    session.close()

pixl_dcmd/src/pixl_dcmd/tests/test_pixl_dcmd.py

@@ -16,6 +16,7 @@
 import pydicom
 import pytest
 from pydicom.data import get_testdata_files
+from pathlib import Path
 
 from pixl_dcmd.main import (
     combine_date_time,
@@ -109,3 +110,4 @@ def test_remove_overlay_plane() -> None:
 
 # TODO: def test_anonymisation
 # https://github.com/UCLH-Foundry/PIXL/issues/132
+fpath = Path(__file__).parents[4] / "test/resources/Dicom1.dcm"

pixl_dcmd/src/setup.py

@@ -31,5 +31,5 @@
             "*.tests.*",
         ],
     ),
-    python_requires="==3.9.2",
+    python_requires=">=3.9.2",
 )