Merge pull request #140 from ral-facilities/DSEGOG-321-delete-records

DSEGOG-321 Fully implement delete records

operationsgateway_api/src/records/echo_interface.py

@@ -137,3 +137,24 @@ def delete_file_object(self, object_path: str) -> None:
                 raise EchoS3Error(
                     f"Deletion of {object_path} was unsuccessful",
                 ) from exc
+
+    def delete_directory(self, dir_path: str) -> None:
+        """
+        Given a path, delete an entire 'directory' from Echo. This is used to delete
+        waveforms and images when deleting a record by its ID
+        """
+
+        log.info("Deleting directory from %s", dir_path)
+
+        try:
+            self.bucket.objects.filter(Prefix=dir_path).delete()
+        except ClientError as exc:
+            log.error(
+                "%s: %s",
+                exc.response["Error"]["Code"],
+                exc.response["Error"].get("Message"),
+            )
+            raise EchoS3Error(
+                f"{exc.response['Error']['Code']} when deleting file at"
+                f" '{dir_path}'",
+            ) from exc

operationsgateway_api/src/records/ingestion/partial_import_checks.py

@@ -89,10 +89,13 @@ def metadata_checks(self):
 
     def channel_checks(self):
         """
-        Checks if any of the incoming channels exist in the stored record
+        Checks if any of the incoming channels exist in the stored record. If the
+        channels exist, they're rejected, otherwise they become an accepted channel.
 
-        if they do they are rejected and a channel response similar to the main channel
-        checks is returned
+        For image and waveform channels, a check is conducted to determine whether the
+        associated image/waveform is stored in Echo; there could be a situation where
+        there's an image channel in the record, but the image isn't stored in Echo
+        (perhaps due to a failure in ingestion or someone's manually deleted it)
         """
         ingested_channels = self.ingested_record.channels
         stored_channels = self.stored_record.channels

operationsgateway_api/src/records/record.py

@@ -40,7 +40,7 @@
 
 
 class Record:
-    def __init__(self, record: RecordModel) -> None:
+    def __init__(self, record: Union[RecordModel, dict]) -> None:
         """
         Store a record within the object. If the input is a dictionary, it will be
         converted into a `RecordModel` object

operationsgateway_api/src/routes/records.py

@@ -12,8 +12,10 @@
 )
 from operationsgateway_api.src.error_handling import endpoint_error_handling
 from operationsgateway_api.src.exceptions import QueryParameterError
+from operationsgateway_api.src.records.echo_interface import EchoInterface
 from operationsgateway_api.src.records.image import Image
 from operationsgateway_api.src.records.record import Record as Record
+from operationsgateway_api.src.records.waveform import Waveform
 from operationsgateway_api.src.routes.common_parameters import ParameterHandler
 
 
@@ -263,11 +265,13 @@ async def delete_record_by_id(
     ],
     access_token: Annotated[str, Depends(authorise_route)],
 ):
-    # TODO 2 - full implementation will require searching through waveform channels to
-    # remove the documents in the waveforms collection. The images will need to be
-    # removed from disk too
     log.info("Deleting record by ID: %s", id_)
 
     await Record.delete_record(id_)
+    echo = EchoInterface()
+    log.info("Deleting waveforms for record ID '%s'", id_)
+    echo.delete_directory(f"{Waveform.echo_prefix}/{id_}/")
+    log.info("Deleting images for record ID '%s'", id_)
+    echo.delete_directory(f"{Image.echo_prefix}/{id_}/")
 
     return Response(status_code=HTTPStatus.NO_CONTENT.value)

test/endpoints/conftest.py

@@ -4,8 +4,12 @@
 import pymongo
 import pytest_asyncio
 
+from operationsgateway_api.src.models import WaveformModel
 from operationsgateway_api.src.mongo.interface import MongoDBInterface
 from operationsgateway_api.src.records.echo_interface import EchoInterface
+from operationsgateway_api.src.records.image import Image
+from operationsgateway_api.src.records.record import Record
+from operationsgateway_api.src.records.waveform import Waveform
 
 
 async def add_user(auth_type):
@@ -69,27 +73,16 @@ async def remove_record(timestamp_id):
     )
 
 
-async def remove_image(images):
+@pytest_asyncio.fixture(scope="function")
+async def reset_record_storage():
+    yield
+    record_id = "20200407142816"
+    await remove_record(record_id)
     echo = EchoInterface()
-    for image_path in images:
-        echo.delete_file_object(image_path)
 
+    echo.delete_directory(f"{Waveform.echo_prefix}/{record_id}/")
+    echo.delete_directory(f"{Image.echo_prefix}/{record_id}/")
 
-async def remove_waveform():
-    await MongoDBInterface.delete_one(
-        "waveforms",
-        filter_={"_id": "20200407142816_PM-201-HJ-PD"},
-    )
-
-
-@pytest_asyncio.fixture(scope="function")
-async def reset_databases():
-    yield
-    await remove_record("20200407142816")
-    await remove_image(
-        ["20200407142816/PM-201-FE-CAM-1.png", "20200407142816/PM-201-FE-CAM-2.png"],
-    )
-    await remove_waveform()
     if os.path.exists("test.h5"):
         os.remove("test.h5")
 
@@ -116,3 +109,58 @@ async def remove_experiment_fixture():
             "start_date": datetime(1920, 4, 30, 10, 0),
         },
     )
+
+
+@pytest_asyncio.fixture(scope="function")
+async def data_for_delete_records():
+    record_id = "19000000000011"
+    test_record = {
+        "_id": record_id,
+        "metadata": {
+            "epac_ops_data_version": "1.0",
+            "shotnum": 423648000000,
+            "timestamp": "2023-06-05T08:00:00",
+        },
+        "channels": {
+            "test-scalar-channel-id": {
+                "metadata": {"channel_dtype": "scalar", "units": "µm"},
+                "data": 5.126920467610521,
+            },
+            "test-image-channel-id": {
+                "metadata": {"channel_dtype": "image"},
+                "image_path": f"{record_id}/test-image-channel-id.png",
+                "thumbnail": "i5~9=",
+            },
+            "test-waveform-channel-id": {
+                "metadata": {"channel_dtype": "waveform"},
+                "waveform_path": f"{record_id}/test-waveform-channel-id.json",
+                "thumbnail": "i5~9=",
+            },
+        },
+    }
+
+    record_instance = Record(test_record)
+    await record_instance.insert()
+
+    echo = EchoInterface()
+    with open("test/images/original_image.png", "rb") as f:
+        echo.upload_file_object(
+            f,
+            f"{Image.echo_prefix}/{record_id}/test-image-channel-id.png",
+        )
+    waveform = Waveform(WaveformModel(x=[1.0, 2.0, 3.0], y=[1.0, 2.0, 3.0]))
+    waveform_bytes = waveform.to_json()
+    echo.upload_file_object(
+        waveform_bytes,
+        f"{Waveform.echo_prefix}/{record_id}/test-waveform-channel-id.json",
+    )
+
+    yield
+
+    await Record.delete_record(record_id)
+    echo.delete_file_object(
+        f"{Image.echo_prefix}/{record_id}/test-image-channel-id.png",
+    )
+    echo.delete_file_object(
+        f"{Waveform.echo_prefix}/{record_id}/test-waveform-channel-id.json",
+    )

test/endpoints/test_delete_record.py

@@ -1,7 +1,11 @@
 from fastapi.testclient import TestClient
 import pytest
 
+from operationsgateway_api.src.exceptions import MissingDocumentError
+from operationsgateway_api.src.records.echo_interface import EchoInterface
+from operationsgateway_api.src.records.image import Image
 from operationsgateway_api.src.records.record import Record
+from operationsgateway_api.src.records.waveform import Waveform
 
 
 class TestDeleteRecordById:
@@ -10,30 +14,27 @@ async def test_delete_record_success(
         self,
         test_app: TestClient,
         login_and_get_token,
+        data_for_delete_records,
     ):
         record_id = 19000000000011
-        test_record = {
-            "_id": "19000000000011",
-            "metadata": {
-                "epac_ops_data_version": "1.0",
-                "shotnum": 423648000000,
-                "timestamp": "2023-06-05T08:00:00",
-            },
-            "channels": {
-                "test-scalar-channel-id": {
-                    "metadata": {"channel_dtype": "scalar", "units": "µm"},
-                    "data": 5.126920467610521,
-                },
-            },
-        }
-
-        record_instance = Record(test_record)
-        await record_instance.insert()
-
         delete_response = test_app.delete(
             f"/records/{record_id}",
             headers={"Authorization": f"Bearer {login_and_get_token}"},
         )
 
         assert delete_response.status_code == 204
-        assert (await record_instance.find_existing_record()) is None
+        # Checks the record has been deleted from the database
+        with pytest.raises(MissingDocumentError):
+            await Record.find_record_by_id(record_id, {})
+
+        # Check that waveform and image have been removed from Echo
+        echo = EchoInterface()
+        waveform_query = echo.bucket.objects.filter(
+            Prefix=f"{Waveform.echo_prefix}/{record_id}/",
+        )
+        assert list(waveform_query) == []
+
+        image_query = echo.bucket.objects.filter(
+            Prefix=f"{Image.echo_prefix}/{record_id}/",
+        )
+        assert list(image_query) == []

test/endpoints/test_submit_hdf.py

@@ -76,7 +76,7 @@ class TestSubmitHDF:
     @pytest.mark.asyncio
     async def test_ingest_data_success(
         self,
-        reset_databases,
+        reset_record_storage,
         test_app: TestClient,
         login_and_get_token,
     ):
@@ -121,7 +121,7 @@ async def test_ingest_data_success(
     @pytest.mark.asyncio
     async def test_merge_record_success(
         self,
-        reset_databases,
+        reset_record_storage,
         test_app: TestClient,
         login_and_get_token,
     ):
@@ -194,7 +194,7 @@ async def test_hdf_rejects(
         data_version,
         active_area,
         expected_response,
-        reset_databases,
+        reset_record_storage,
         test_app: TestClient,
         login_and_get_token,
     ):
@@ -218,7 +218,7 @@ async def test_hdf_rejects(
     @pytest.mark.asyncio
     async def test_partial_import_record_reject(
         self,
-        reset_databases,
+        reset_record_storage,
         test_app: TestClient,
         login_and_get_token,
     ):
@@ -249,7 +249,7 @@ async def test_partial_import_record_reject(
     @pytest.mark.asyncio
     async def test_channel_all_fail(
         self,
-        reset_databases,
+        reset_record_storage,
         test_app: TestClient,
         login_and_get_token,
     ):
@@ -289,7 +289,7 @@ async def test_channel_all_fail(
     @pytest.mark.asyncio
     async def test_channel_multiple_reject_types(
         self,
-        reset_databases,
+        reset_record_storage,
         test_app: TestClient,
         login_and_get_token,
     ):

test/records/ingestion/test_partial_import.py

@@ -1,4 +1,5 @@
 import copy
+from unittest.mock import patch
 
 import pytest
 
@@ -219,4 +220,16 @@ async def test_import_channel_checks(self, remove_hdf_file, test_type, response)
             stored_record,
         )
 
-        assert partial_import_checker.channel_checks() == response
+        # This test doesn't use any data stored in the database/Echo, it provides
+        # instances of RecordModel as inputs to PartialImportChecks. For image and
+        # waveform channels, a check is conducted to make sure the associated
+        # image/waveform is actually on Echo and because we're not using stored data for
+        # this test, we need to mock that check
+        with patch.object(
+            partial_import_checker,
+            "_is_image_or_waveform_stored",
+        ) as mock_is_stored:
+            mock_is_stored.return_value = True
+            partial_import_channel_checks = partial_import_checker.channel_checks()
+
+        assert partial_import_channel_checks == response

test/records/test_echo_interface.py

@@ -276,3 +276,56 @@ def test_invalid_delete_file_object(self, _):
             test_echo = EchoInterface()
             with pytest.raises(EchoS3Error):
                 test_echo.delete_file_object(self.test_image_path)
+
+    @patch(
+        "operationsgateway_api.src.config.Config.config.echo.url",
+        config_echo_url,
+    )
+    @patch(
+        "operationsgateway_api.src.config.Config.config.echo.access_key",
+        config_echo_access_key,
+    )
+    @patch(
+        "operationsgateway_api.src.config.Config.config.echo.secret_key",
+        config_echo_secret_key,
+    )
+    @patch(
+        "operationsgateway_api.src.config.Config.config.echo.bucket_name",
+        config_image_bucket_name,
+    )
+    @patch("boto3.resource")
+    def test_valid_delete_directory(self, _):
+        test_echo = EchoInterface()
+        test_echo.delete_directory("test/image/19900202143000/")
+        assert test_echo.bucket.objects.filter.return_value.delete.call_count == 1
+
+    @patch(
+        "operationsgateway_api.src.config.Config.config.echo.url",
+        config_echo_url,
+    )
+    @patch(
+        "operationsgateway_api.src.config.Config.config.echo.access_key",
+        config_echo_access_key,
+    )
+    @patch(
+        "operationsgateway_api.src.config.Config.config.echo.secret_key",
+        config_echo_secret_key,
+    )
+    @patch(
+        "operationsgateway_api.src.config.Config.config.echo.bucket_name",
+        config_image_bucket_name,
+    )
+    @patch("boto3.resource")
+    def test_invalid_delete_directory(self, _):
+        with patch("boto3.resource") as mock_resource:
+            mock_bucket = mock_resource.return_value.Bucket.return_value
+
+            mock_bucket.objects.filter.return_value.delete.side_effect = ClientError(
+                {"Error": {"Code": "AccessDenied", "Message": "Access Denied"}},
+                "delete",
+            )
+
+            test_echo = EchoInterface()
+
+            with pytest.raises(EchoS3Error, match="when deleting file at"):
+                test_echo.delete_directory("test/image/19900202143000/")