task/DES-abc: Move project asset directory deletion to celery task (#118

)

* Move project assset directory deletion to celery task

* Add mock

* Add remove_project_assets to __init__.py

geoapi/services/projects.py

@@ -1,4 +1,3 @@
-import shutil
 from pathlib import Path
 from typing import List, Optional
 
@@ -9,9 +8,9 @@
 from sqlalchemy.exc import IntegrityError
 from geoapi.services.users import UserService
 from geoapi.utils.agave import AgaveUtils, get_system_users
-from geoapi.utils.assets import get_project_asset_dir
 from geoapi.utils.users import is_anonymous
 from geoapi.tasks.external_data import import_from_agave
+from geoapi.tasks.projects import remove_project_assets
 from geoapi.log import logger
 from geoapi.exceptions import ApiException, ObservableProjectAlreadyExists
 
@@ -274,11 +273,8 @@ def delete(user: User, projectId: int) -> dict:
         db_session.query(Project).filter(Project.id == projectId).delete()
         db_session.commit()
 
-        assets_folder = get_project_asset_dir(projectId)
-        try:
-            shutil.rmtree(assets_folder)
-        except FileNotFoundError:
-            pass
+        remove_project_assets.apply_async(args=[projectId])
+
         return {"status": "ok"}
 
     @staticmethod

geoapi/tasks/__init__.py

@@ -1,3 +1,4 @@
 from geoapi.tasks.lidar import convert_to_potree
 from geoapi.tasks.external_data import import_file_from_agave, import_from_agave, refresh_observable_projects
 from geoapi.tasks.streetview import publish, from_tapis_to_streetview, process_streetview_sequences
+from geoapi.tasks.projects import remove_project_assets

geoapi/tasks/projects.py

@@ -0,0 +1,22 @@
+from geoapi.celery_app import app
+from geoapi.utils.assets import get_project_asset_dir
+from geoapi.log import logger
+import shutil
+
+
+@app.task()
+def remove_project_assets(project_id):
+    """
+    Remove all assets associated with a project.
+
+    The directory containing that project's assets will be deleted.
+
+    """
+    logger.info(f"Deleting project:{project_id} started")
+    assets_folder = get_project_asset_dir(project_id)
+    try:
+        shutil.rmtree(assets_folder)
+        logger.info(f"Deleting project:{project_id} finished")
+    except FileNotFoundError:
+        logger.info(f"Deleting project:{project_id} completed but caught FileNotFoundError")
+        pass

geoapi/tests/api_tests/test_projects_routes.py

@@ -1,8 +1,10 @@
 import datetime
 import uuid
+import os
 from geoapi.db import db_session
 from geoapi.models.users import User
 from geoapi.models.project import Project, ProjectUser
+from geoapi.utils.assets import get_project_asset_dir
 
 
 def test_get_projects(test_client, projects_fixture, user1):
@@ -119,7 +121,7 @@ def test_project_data_allow_public_access(test_client, public_projects_fixture,
     assert resp.status_code == 200
 
 
-def test_delete_empty_project(test_client, projects_fixture, user1):
+def test_delete_empty_project(test_client, projects_fixture, remove_project_assets_mock, user1):
     resp = test_client.delete(f'/projects/{projects_fixture.id}/', headers={'x-jwt-assertion-test': user1.jwt})
     assert resp.status_code == 200
     projects = db_session.query(Project).all()
@@ -128,6 +130,17 @@ def test_delete_empty_project(test_client, projects_fixture, user1):
     assert projectUsers == []
 
 
+def test_delete_project(test_client, projects_fixture, remove_project_assets_mock, image_feature_fixture, user1):
+    assert os.path.isdir(get_project_asset_dir(projects_fixture.id))
+    resp = test_client.delete(f'/projects/{projects_fixture.id}/', headers={'x-jwt-assertion-test': user1.jwt})
+    assert resp.status_code == 200
+    projects = db_session.query(Project).all()
+    projectUsers = db_session.query(ProjectUser).all()
+    assert projects == []
+    assert projectUsers == []
+    assert not os.path.isdir(get_project_asset_dir(projects_fixture.id))
+
+
 def test_delete_unauthorized(test_client, projects_fixture, user2):
     resp = test_client.delete(f'/projects/{projects_fixture.id}/', headers={'x-jwt-assertion-test': user2.jwt})
     assert resp.status_code == 403

geoapi/tests/api_tests/test_projects_service.py

@@ -23,7 +23,7 @@ def test_create_project():
     assert proj.description == "test description"
 
 
-def test_delete_project(projects_fixture, user1):
+def test_delete_project(projects_fixture, remove_project_assets_mock, user1):
     ProjectsService.delete(user1, projects_fixture.id)
     projects = db_session.query(Project).all()
     assert projects == []

geoapi/tests/conftest.py

@@ -456,6 +456,20 @@ def get_system_users_mock(userdata):
         yield get_system_users
 
 
+@pytest.fixture(scope="function")
+def remove_project_assets_mock():
+    # we mock method so that we execute it synchronously (and not as a celery task on worker)
+    # when testing some routes
+    with patch('geoapi.services.projects.remove_project_assets') as mock_remove_project:
+        from geoapi.tasks.projects import remove_project_assets
+
+        def remove(args):
+            remove_project_assets(project_id=args[0])
+
+        mock_remove_project.apply_async.side_effect = remove
+        yield mock_remove_project
+
+
 @pytest.fixture(scope="function")
 def tile_server_ini_file_fixture():
     home = os.path.dirname(__file__)