diff --git a/servicex_app/migrations/versions/v1_6_0.py b/servicex_app/migrations/versions/v1_6_0.py new file mode 100644 index 000000000..f32aaa656 --- /dev/null +++ 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 index 88cb5f398..cd8f81b37 100644 --- 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 index 98c7d6232..b6d3ad4ec 100644 --- 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 new file mode 100644 index 000000000..48ec58074 --- /dev/null +++ b/servicex_app/servicex_app/resources/transformation/archive.py @@ -0,0 +1,69 @@ +# 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 transform_req.archived: + msg = f'Transformation request with id: {request_id} is already archived.' + 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 index 52db08700..619b990ec 100644 --- 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 index bc3111d8b..917632303 100644 --- 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 index 329480fe4..133228a3c 100644 --- 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 index 72e3585d0..87ddabd22 100644 --- 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 index ace3b0306..9192671e6 100644 --- 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 += "/" 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 index b487628f6..45f6b469c 100644 --- 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 index 005c1251c..b80e00985 100644 --- 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 new file mode 100644 index 000000000..061b24843 --- /dev/null +++ b/servicex_app/servicex_app_test/resources/transformation/test_archive.py @@ -0,0 +1,97 @@ +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") + transform = self._generate_transform_request() + transform.save_to_db = MagicMock() + transform.truncate_results = MagicMock() + mock_transform_request_cls.lookup.return_value = transform + return transform + + def test_archive(self, fake_transform, mock_object_store_manager): + fake_transform.status = TransformStatus.complete + + 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 + + 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_already_archived(self, fake_transform): + fake_transform.status = TransformStatus.running + fake_transform.archived = True + + client = self._test_client() + + resp = client.delete("/servicex/transformation/BR549") + assert resp.status_code == 404 + assert resp.json["message"] == "Transformation request with id: BR549 is already archived." + assert 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 index 3d3e4df52..1df6217a8 100644 --- 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 index 644b3cf56..3d4563fff 100644 --- 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 index 68d5ceee2..6dd80cff2 100644 --- 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 index 9de7fe6a1..73bfc9a50 100644 --- 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 index 84ff90d04..249f1532e 100644 --- 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 index 800ae5c50..ed2bb7765 100644 --- a/servicex_app/servicex_app_test/web/test_user_dashboard.py +++ b/servicex_app/servicex_app_test/web/test_user_dashboard.py @@ -11,25 +11,30 @@ def mock_query(self, mocker): mock_tr = mocker.patch("servicex_app.web.dashboard.TransformRequest") return mock_tr.query.filter_by.return_value.order_by.return_value - def test_get_empty_state(self, client, user, mock_query, captured_templates): + @fixture + def mock_query_two_filters(self, mocker): + mock_tr = mocker.patch("servicex_app.web.dashboard.TransformRequest") + return mock_tr.query.filter_by().filter_by.return_value.order_by.return_value + + def test_get_empty_state(self, client, user, mock_query_two_filters, captured_templates): with client.session_transaction() as sess: sess['user_id'] = user.id print(sess) - pagination = mock_query.paginate(page=1, per_page=15, total=0, items=[]) - mock_query.paginate.return_value = pagination + pagination = mock_query_two_filters.paginate(page=1, per_page=15, total=0, items=[]) + mock_query_two_filters.paginate.return_value = pagination response: Response = client.get(url_for('user-dashboard'), headers=self.fake_header()) assert response.status_code == 200 template, context = captured_templates[0] assert template.name == 'user_dashboard.html' assert context["pagination"] == pagination - def test_get_with_results(self, client, user, mock_query, captured_templates): + def test_get_with_results(self, client, user, mock_query_two_filters, captured_templates): with client.session_transaction() as sess: sess['user_id'] = user.id print(sess) items = [self._test_transformation_req(id=i+1) for i in range(3)] - pagination = mock_query.paginate(page=1, per_page=15, total=100, items=items) - mock_query.paginate.return_value = pagination + pagination = mock_query_two_filters.paginate(page=1, per_page=15, total=100, items=items) + mock_query_two_filters.paginate.return_value = pagination response: Response = client.get(url_for('user-dashboard'), headers=self.fake_header()) assert response.status_code == 200 template, context = captured_templates[0]