Allow users to delete transform requests

BenGalewsky · BenGalewsky · commit dfe2dbb67765 · 2024-11-15T11:46:54.000-06:00
This is accomplished by adding a new boolean to the transform request table, archived.
Setting archived to True removes the transform for reports and requests.

When a transform is archived, the transform results table records are deleted and the
contents of the bucket that held the transformed files are deleted, along with that bucket.
diff --git a/servicex_app/migrations/versions/v1_6_0.py b/servicex_app/migrations/versions/v1_6_0.py
@@ -0,0 +1,26 @@
+"""
+Mark dataset rows as stale
+
+Revision ID: v1_6_0
+Revises: v1_5_5
+Create Date: 2024-11-14 18:19:00.000000
+"""
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects import postgresql
+
+# revision identifiers, used by Alembic.
+revision = 'v1_6_0'
+down_revision = 'v1_5_5'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    op.add_column('requests', sa.Column('archived',
+                                        sa.Boolean(),
+                                        nullable=False,
+                                        server_default='false'))
+
+def downgrade():
+    op.drop_column('requests', 'archived')
diff --git a/servicex_app/servicex_app/models.py b/servicex_app/servicex_app/models.py
@@ -168,6 +168,7 @@ class TransformRequest(db.Model):
     request_id = db.Column(db.String(48), unique=True, nullable=False, index=True)
     title = db.Column(db.String(128), nullable=True)
     submit_time = db.Column(db.DateTime, nullable=False)
+    archived = db.Column(db.Boolean, nullable=False, default=False)
     finish_time = db.Column(db.DateTime, nullable=True)
     did = db.Column(db.String(512), unique=False, nullable=False)
     did_id = db.Column(db.Integer, unique=False, nullable=False)
@@ -321,6 +322,10 @@ def result_count(self) -> int:
     def results(self) -> List['TransformationResult']:
         return TransformationResult.query.filter_by(request_id=self.request_id).all()
 
+    def truncate_results(self):
+        TransformationResult.query.filter_by(request_id=self.request_id).delete()
+        db.session.commit()
+
     @property
     def all_files(self) -> List['DatasetFile']:
         return DatasetFile.query.filter_by(dataset_id=self.did_id).all()
diff --git a/servicex_app/servicex_app/object_store_manager.py b/servicex_app/servicex_app/object_store_manager.py
@@ -39,3 +39,15 @@ def create_bucket(self, bucket_name):
 
     def list_buckets(self):
         return self.minio_client.list_buckets()
+
+    def delete_bucket_and_contents(self,  bucket_name):
+        # List all objects in the bucket
+        objects = self.minio_client.list_objects(bucket_name, recursive=True)
+
+        # Remove each object
+        for obj in objects:
+            self.minio_client.remove_object(bucket_name, obj.object_name)
+
+        # Remove the bucket itself
+        self.minio_client.remove_bucket(bucket_name)
+        print(f"Bucket '{bucket_name}' deleted successfully.")
diff --git a/servicex_app/servicex_app/resources/transformation/archive.py b/servicex_app/servicex_app/resources/transformation/archive.py
@@ -0,0 +1,64 @@
+# Copyright (c) 2024, IRIS-HEP
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are met:
+#
+# * Redistributions of source code must retain the above copyright notice, this
+#   list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright notice,
+#   this list of conditions and the following disclaimer in the documentation
+#   and/or other materials provided with the distribution.
+#
+# * Neither the name of the copyright holder nor the names of its
+#   contributors may be used to endorse or promote products derived from
+#   this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
+# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+from servicex_app import ObjectStoreManager
+from servicex_app.decorators import auth_required
+from servicex_app.models import TransformRequest
+from servicex_app.resources.servicex_resource import ServiceXResource
+from flask import current_app
+
+
+class ArchiveTransform(ServiceXResource):
+    @classmethod
+    def make_api(cls, object_store_manager: ObjectStoreManager):
+        cls.object_store = object_store_manager
+
+    @auth_required
+    def delete(self, request_id: str):
+        transform_req = TransformRequest.lookup(request_id)
+        if not transform_req:
+            msg = f'Transformation request not found with id: {request_id}'
+            current_app.logger.warning(msg, extra={'requestId': request_id})
+            return {'message': msg}, 404
+
+        if not transform_req.status.is_complete:
+            msg = f"Transform request with id {request_id} is still in progress."
+            current_app.logger.warning(msg, extra={'requestId': request_id})
+            return {"message": msg}, 400
+
+        user = self.get_requesting_user()
+        if user and (not user.admin and user.id != transform_req.submitted_by):
+            return {"message": "You are not authorized to delete this request"}, 403
+
+        transform_req.archived = True
+        transform_req.save_to_db()
+        transform_req.truncate_results()
+        if self.object_store:
+            self.object_store.delete_bucket_and_contents(transform_req.request_id)
+        return {
+            "message": f"Transform request with id {request_id} has been archived."
+        }, 200
diff --git a/servicex_app/servicex_app/resources/transformation/get_all.py b/servicex_app/servicex_app/resources/transformation/get_all.py
@@ -50,5 +50,5 @@ def get(self):
             transforms = TransformRequest.query.filter_by(submitted_by=query_id)
         else:
             current_app.logger.debug("Querying for all  transform requests")
-            transforms = TransformRequest.query.all()
+            transforms = TransformRequest.query.filter_by(archived=False)
         return TransformRequest.return_json(transforms)
diff --git a/servicex_app/servicex_app/resources/transformation/get_one.py b/servicex_app/servicex_app/resources/transformation/get_one.py
@@ -44,6 +44,12 @@ def get(self, request_id):
             msg = f'Transformation request not found with id: {request_id}'
             current_app.logger.error(msg, extra={'requestId': request_id})
             return {'message': msg}, 404
+
+        if transform.archived:
+            msg = f'Transformation request with id: {request_id} is archived'
+            current_app.logger.error(msg, extra={'requestId': request_id})
+            return {'message': msg}, 404
+
         transform_json = transform.to_json()
         if current_app.config['OBJECT_STORE_ENABLED'] and \
                 transform_json['result-destination'] == TransformRequest.OBJECT_STORE_DEST:
diff --git a/servicex_app/servicex_app/resources/transformation/status.py b/servicex_app/servicex_app/resources/transformation/status.py
@@ -47,6 +47,11 @@ def get(self, request_id):
             current_app.logger.error(msg, extra={'requestId': request_id})
             return {'message': msg}, 404
 
+        if transform.archived:
+            msg = f'Transformation request with id: {request_id} is archived'
+            current_app.logger.error(msg, extra={'requestId': request_id})
+            return {'message': msg}, 404
+
         status_request = status_request_parser.parse_args()
 
         # Format timestamps with military timezone, given that they are in UTC.
diff --git a/servicex_app/servicex_app/resources/transformation/submit.py b/servicex_app/servicex_app/resources/transformation/submit.py
@@ -176,6 +176,7 @@ def post(self):
             request_rec = TransformRequest(
                 request_id=str(request_id),
                 title=args.get("title"),
+                archived=False,
                 did=dataset_manager.name,
                 did_id=dataset_manager.id,
                 submit_time=datetime.now(tz=timezone.utc),
diff --git a/servicex_app/servicex_app/routes.py b/servicex_app/servicex_app/routes.py
@@ -30,6 +30,7 @@
 from servicex_app.resources.datasets.delete_dataset import DeleteDataset
 from servicex_app.resources.datasets.get_all import AllDatasets
 from servicex_app.resources.datasets.get_one import OneDataset
+from servicex_app.resources.transformation.archive import ArchiveTransform
 
 
 def add_routes(api, transformer_manager, rabbit_mq_adaptor,
@@ -137,6 +138,10 @@ def add_routes(api, transformer_manager, rabbit_mq_adaptor,
     api.add_resource(AllTransformationRequests, prefix)
     prefix += "/<string:request_id>"
     api.add_resource(TransformationRequest, prefix)
+
+    ArchiveTransform.make_api(object_store)
+    api.add_resource(ArchiveTransform, prefix)
+
     api.add_resource(TransformationStatus, prefix + "/status")
 
     DeploymentStatus.make_api(transformer_manager)
diff --git a/servicex_app/servicex_app/web/dashboard.py b/servicex_app/servicex_app/web/dashboard.py
@@ -32,7 +32,7 @@
 def dashboard(template_name: str, user_specific=False):
     args = parser.parse_args()
     sort, order = args["sort"], args["order"]
-    query = TransformRequest.query
+    query = TransformRequest.query.filter_by(archived=False)
 
     if user_specific:
         query = query.filter_by(submitted_by=session["user_id"])
diff --git a/servicex_app/servicex_app_test/resource_test_base.py b/servicex_app/servicex_app_test/resource_test_base.py
@@ -163,6 +163,7 @@ def _generate_transform_request():
         transform_request.transformer_language = "scala"
         transform_request.transformer_command = "echo"
         transform_request.selection = "(cool (is LISP))"
+        transform_request.archived = False
         return transform_request
 
     @staticmethod
diff --git a/servicex_app/servicex_app_test/resources/transformation/test_archive.py b/servicex_app/servicex_app_test/resources/transformation/test_archive.py
@@ -0,0 +1,87 @@
+from unittest.mock import MagicMock
+
+import pytest
+
+from servicex_app.models import TransformRequest, TransformStatus
+from servicex_app_test.resource_test_base import ResourceTestBase
+
+
+class TestTransformArchive(ResourceTestBase):
+    module = "servicex_app.resources.transformation.archive"
+
+    @pytest.fixture
+    def mock_object_store_manager(self, mocker) -> MagicMock:
+        return mocker.MagicMock()
+
+    @pytest.fixture
+    def fake_transform(self, mocker) -> TransformRequest:
+        mock_transform_request_cls = mocker.patch(f"{self.module}.TransformRequest")
+        fake_transform = self._generate_transform_request()
+        fake_transform.save_to_db = mocker.Mock()
+        mock_transform_request_cls.lookup.return_value = fake_transform
+        return fake_transform
+
+    def test_archive(self, fake_transform, mock_object_store_manager):
+        fake_transform.status = TransformStatus.complete
+        fake_transform.save_to_db = MagicMock()
+        fake_transform.truncate_results = MagicMock()
+
+        local_config = {
+            'OBJECT_STORE_ENABLED': True,
+            'MINIO_URL': 'localhost:9000',
+            'MINIO_ACCESS_KEY': 'miniouser',
+            'MINIO_SECRET_KEY': 'leftfoot1'
+        }
+
+        client = self._test_client(extra_config=local_config,
+                                   object_store=mock_object_store_manager)
+
+        resp = client.delete("/servicex/transformation/BR549")
+        assert resp.status_code == 200
+        assert fake_transform.archived
+        assert fake_transform.save_to_db.called
+        assert fake_transform.truncate_results.called
+        mock_object_store_manager.delete_bucket_and_contents.assert_called_once_with("BR549")
+
+    def test_running(self, fake_transform):
+        fake_transform.status = TransformStatus.running
+        fake_transform.save_to_db = MagicMock()
+        fake_transform.truncate_results = MagicMock()
+
+        client = self._test_client()
+
+        resp = client.delete("/servicex/transformation/BR549")
+        assert resp.status_code == 400
+        assert resp.json["message"] == "Transform request with id BR549 is still in progress."
+        assert not fake_transform.archived
+        assert not fake_transform.save_to_db.called
+        assert not fake_transform.truncate_results.called
+
+    def test_not_found(self):
+        client = self._test_client()
+
+        resp = client.delete("/servicex/transformation/BR549")
+        assert resp.status_code == 404
+
+    @pytest.mark.parametrize("user_id, submitter_id, is_admin, expected_status", [
+        (42, 42, False, 200),  # Submitting user wants to delete their own request
+        (42, 42, True, 200),  # Admin wants to delete their own request
+        (42, 43, True, 200),  # Admin wants to delete someone else's request
+        (42, 43, False, 403),  # User tries to delete someone else's request
+    ])
+    def test_submit_transformation_auth_enabled(self,
+                                                user_id, submitter_id, is_admin,
+                                                expected_status,
+                                                fake_transform,
+                                                mock_jwt_extended, mock_requesting_user):
+        fake_transform.status = TransformStatus.complete
+        client = self._test_client(extra_config={'ENABLE_AUTH': True})
+        with client.application.app_context():
+            mock_requesting_user.id = user_id
+            mock_requesting_user.admin = is_admin
+            fake_transform.submitted_by = submitter_id
+
+            resp = client.delete("/servicex/transformation/BR549",
+                                 headers=self.fake_header())
+
+            assert resp.status_code == expected_status
diff --git a/servicex_app/servicex_app_test/resources/transformation/test_get_one.py b/servicex_app/servicex_app_test/resources/transformation/test_get_one.py
@@ -50,3 +50,12 @@ def test_get_single_request_404(self, mocker, client):
             response = client.get('/servicex/transformation/1234')
         assert response.status_code == 404
         mock_lookup.assert_called_with('1234')
+
+    def test_get_single_request_archived(self, mocker, client):
+        with patch('servicex_app.models.TransformRequest.lookup') as mock_lookup:
+            fake_transform_request = self._generate_transform_request()
+            fake_transform_request.archived = True
+            mock_lookup.return_value = fake_transform_request
+            response = client.get('/servicex/transformation/1234')
+        assert response.status_code == 404
+        mock_lookup.assert_called_with('1234')
diff --git a/servicex_app/servicex_app_test/resources/transformation/test_status.py b/servicex_app/servicex_app_test/resources/transformation/test_status.py
@@ -54,3 +54,16 @@ def test_get_status_404(self, mocker, client):
         response = client.get('/servicex/transformation/1234/status')
         assert response.status_code == 404
         mock_transform_request_read.assert_called_with("1234")
+
+    def test_get_status_archived(self, mocker, client):
+        import servicex_app
+        fake_transform_request = self._generate_transform_request()
+        fake_transform_request.archived = True
+        mock_transform_request_read = mocker.patch.object(
+            servicex_app.models.TransformRequest,
+            'lookup',
+            return_value=fake_transform_request)
+
+        response = client.get('/servicex/transformation/1234/status')
+        assert response.status_code == 404
+        mock_transform_request_read.assert_called_with("1234")
diff --git a/servicex_app/servicex_app_test/test_decorators.py b/servicex_app/servicex_app_test/test_decorators.py
@@ -52,6 +52,7 @@ def test_auth_decorator_integration_auth_disabled(self, mocker, client):
         mock = mocker.patch('servicex_app.resources.transformation.get_one'
                             '.TransformRequest.lookup').return_value
         mock.to_json.return_value = data
+        mock.archived = False
         with client.application.app_context():
             response: Response = client.get(f'servicex/transformation/{fake_transform_id}')
             print(response.data)
@@ -90,6 +91,7 @@ def test_auth_decorator_integration_authorized(self, mocker, user):
         mock = mocker.patch('servicex_app.resources.transformation.get_one'
                             '.TransformRequest.lookup').return_value
         mock.submitted_by = user.id
+        mock.archived = False
         mock.to_json.return_value = data
         with client.application.app_context():
             response: Response = client.get(f'servicex/transformation/{fake_transform_id}',
@@ -105,6 +107,7 @@ def test_auth_decorator_integration_oauth(self, mocker, user):
         mock = mocker.patch('servicex_app.resources.transformation.get_one'
                             '.TransformRequest.lookup').return_value
         mock.submitted_by = user.id
+        mock.archived = False
         mock.to_json.return_value = data
         with client.session_transaction() as sess:
             sess['is_authenticated'] = True
diff --git a/servicex_app/servicex_app_test/test_object_store_manager.py b/servicex_app/servicex_app_test/test_object_store_manager.py
@@ -55,3 +55,18 @@ def test_list_buckets(self, mocker):
         bucket_list = result.list_buckets()
         mock_minio.list_buckets.assert_called()
         assert bucket_list == ['a', 'b']
+
+    def test_delete_bucket_and_contents(self, mocker):
+        import minio
+        mock_minio = mocker.MagicMock(minio.api.Minio)
+        mock_object = mocker.MagicMock()
+        mock_object.object_name = "a"
+        mock_minio.list_objects = mocker.Mock(return_value=[mock_object])
+        mocker.patch('minio.Minio', return_value=mock_minio)
+
+        object_store = ObjectStoreManager('localhost:9999', 'foo', 'bar')
+        object_store.delete_bucket_and_contents("123-455")
+
+        mock_minio.list_objects.assert_called_with("123-455", recursive=True)
+        mock_minio.remove_object.assert_called_with("123-455", "a")
+        mock_minio.remove_bucket.assert_called_with("123-455")
diff --git a/servicex_app/servicex_app_test/web/test_global_dashboard.py b/servicex_app/servicex_app_test/web/test_global_dashboard.py
@@ -9,7 +9,7 @@ class TestGlobalDashboard(WebTestBase):
     @fixture
     def mock_query(self, mocker):
         mock_tr = mocker.patch("servicex_app.web.dashboard.TransformRequest")
-        return mock_tr.query.order_by.return_value
+        return mock_tr.query.filter_by().order_by.return_value
 
     def test_get_empty_state(self, client, user, mock_query, captured_templates):
         pagination = mock_query.paginate(page=1, per_page=15, total=0, items=[])
diff --git a/servicex_app/servicex_app_test/web/test_user_dashboard.py b/servicex_app/servicex_app_test/web/test_user_dashboard.py
