[Fixes #227] Delete legacy upload code (#239)

* [Fixes #227] Cleanup not-user code
* [Fixes #227] Fix test

Author: mattiagiupponi

importer/orchestrator.py

@@ -1,5 +1,4 @@
 import logging
-import os
 from typing import Optional
 from uuid import UUID
 
@@ -9,9 +8,8 @@
 from django.utils import timezone
 from django.utils.module_loading import import_string
 from django_celery_results.models import TaskResult
-from geonode.base.enumerations import STATE_INVALID, STATE_PROCESSED, STATE_RUNNING
 from geonode.resource.models import ExecutionRequest
-from geonode.upload.models import Upload
+from geonode.storage.manager import storage_manager
 from rest_framework import serializers
 
 from importer.api.exception import ImportException
@@ -30,13 +28,8 @@ class ImportOrchestrator:
     it call the next step of the import pipeline
     Params:
 
-    enable_legacy_upload_status default=True: if true, will save the upload progress
-        also in the legacy upload system
     """
 
-    def __init__(self, enable_legacy_upload_status=True) -> None:
-        self.enable_legacy_upload_status = enable_legacy_upload_status
-
     def get_handler(self, _data) -> Optional[BaseHandler]:
         """
         If is part of the supported format, return the handler which can handle the import
@@ -172,7 +165,6 @@ def set_as_failed(self, execution_id, reason=None, delete_file=True):
             finished=timezone.now(),
             last_updated=timezone.now(),
             log=reason,
-            legacy_status=STATE_INVALID,
         )
         # delete
         if delete_file:
@@ -194,7 +186,6 @@ def set_as_partially_failed(self, execution_id, reason=None):
             finished=timezone.now(),
             last_updated=timezone.now(),
             log=f"The execution is completed, but the following layers are not imported: \n {', '.join(reason)}. Check the logs for additional infos",
-            legacy_status=STATE_INVALID,
         )
 
     def set_as_completed(self, execution_id):
@@ -206,7 +197,6 @@ def set_as_completed(self, execution_id):
             status=ExecutionRequest.STATUS_FINISHED,
             finished=timezone.now(),
             last_updated=timezone.now(),
-            legacy_status=STATE_PROCESSED,
         )
 
     def evaluate_execution_progress(
@@ -301,7 +291,6 @@ def create_execution_request(
         step: str,
         input_params: dict = {},
         resource=None,
-        legacy_upload_name="",
         action=None,
         name=None,
         source=None,
@@ -320,29 +309,12 @@ def create_execution_request(
             name=name,
             source=source,
         )
-        if self.enable_legacy_upload_status:
-            # getting the package name from the base_filename
-            Upload.objects.create(
-                name=legacy_upload_name
-                or os.path.basename(input_params.get("files", {}).get("base_file")),
-                state=STATE_RUNNING,
-                user=user,
-                metadata={
-                    **{
-                        "func_name": func_name,
-                        "step": step,
-                        "exec_id": str(execution.exec_id),
-                    },
-                    **input_params,
-                },
-            )
         return execution.exec_id
 
     def update_execution_request_status(
         self,
         execution_id,
         status=None,
-        legacy_status=STATE_RUNNING,
         celery_task_request=None,
         **kwargs,
     ):
@@ -355,12 +327,6 @@ def update_execution_request_status(
 
         ExecutionRequest.objects.filter(exec_id=execution_id).update(**kwargs)
 
-        if self.enable_legacy_upload_status:
-            Upload.objects.filter(metadata__contains=execution_id).update(
-                state=legacy_status,
-                complete=True,
-                metadata={**kwargs, **{"exec_id": execution_id}},
-            )
         if celery_task_request:
             TaskResult.objects.filter(task_id=celery_task_request.id).update(
                 task_args=celery_task_request.args
@@ -376,4 +342,4 @@ def _last_step(self, execution_id, handler_module_path):
         return self.load_handler(handler_module_path).perform_last_step(execution_id)
 
 
-orchestrator = ImportOrchestrator(enable_legacy_upload_status=False)
+orchestrator = ImportOrchestrator()

importer/tests/unit/test_orchestrator.py

@@ -10,12 +10,10 @@
 from importer.handlers.base import BaseHandler
 from importer.handlers.shapefile.serializer import ShapeFileSerializer
 from importer.orchestrator import ImportOrchestrator
-from geonode.upload.models import Upload
 from django.utils import timezone
 from django_celery_results.models import TaskResult
 from geonode.assets.handlers import asset_handler_registry
 
-from geonode.base import enumerations as enum
 from geonode.resource.models import ExecutionRequest
 
 # Create your tests here.
@@ -103,8 +101,6 @@ def test_create_execution_request(self):
         self.assertEqual(count + 1, ExecutionRequest.objects.count())
         self.assertDictEqual(input_files, exec_obj.input_params)
         self.assertEqual(exec_obj.STATUS_READY, exec_obj.status)
-        # check that also the legacy is created
-        self.assertIsNotNone(Upload.objects.get(metadata__icontains=exec_id))
 
     @patch("importer.orchestrator.importer_app.tasks.get")
     def test_perform_next_step(self, mock_celery):
@@ -184,7 +180,6 @@ def test_perform_with_error_set_invalid_status(self, mock_celery):
         _excec = ExecutionRequest.objects.filter(exec_id=_id).first()
         self.assertIsNotNone(_excec)
         self.assertEqual(ExecutionRequest.STATUS_FAILED, _excec.status)
-        self.assertIsNotNone(Upload.objects.get(metadata__icontains=_id))
 
     def test_set_as_failed(self):
         # creating the temporary file that will be deleted
@@ -227,14 +222,8 @@ def test_set_as_failed(self):
         self.assertTrue(req.status, ExecutionRequest.STATUS_FAILED)
         self.assertTrue(req.log, "automatic test")
         self.assertFalse(os.path.exists(fake_path))
-        # check legacy execution status
-        legacy = Upload.objects.filter(metadata__contains=_uuid)
-        self.assertTrue(legacy.exists())
-        self.assertEqual(legacy.first().state, enum.STATE_INVALID)
-
         # cleanup
         req.delete()
-        legacy.delete()
 
     def test_set_as_completed(self):
         # we need to create first the execution
@@ -254,14 +243,8 @@ def test_set_as_completed(self):
         req = ExecutionRequest.objects.get(exec_id=_uuid)
         self.assertTrue(req.status, ExecutionRequest.STATUS_FINISHED)
 
-        # check legacy execution status
-        legacy = Upload.objects.filter(metadata__contains=_uuid)
-        self.assertTrue(legacy.exists())
-        self.assertEqual(legacy.first().state, enum.STATE_PROCESSED)
-
         # cleanup
         req.delete()
-        legacy.delete()
 
     def test_update_execution_request_status(self):
         # we need to create first the execution
@@ -287,14 +270,8 @@ def test_update_execution_request_status(self):
         self.assertTrue(req.func_name, "function_name")
         self.assertTrue(req.step, "step_here")
 
-        # check legacy execution status
-        legacy = Upload.objects.filter(metadata__contains=_uuid)
-        self.assertTrue(legacy.exists())
-        self.assertEqual(legacy.first().state, enum.STATE_RUNNING)
-
         # cleanup
         req.delete()
-        legacy.delete()
 
     def test_evaluate_execution_progress_should_continue_if_some_task_is_not_finished(
         self,
@@ -306,7 +283,6 @@ def test_evaluate_execution_progress_should_continue_if_some_task_is_not_finishe
                     user=get_user_model().objects.first(),
                     func_name="test",
                     step="test",
-                    legacy_upload_name="test",
                 )
             )
 
@@ -360,7 +336,6 @@ def test_evaluate_execution_progress_should_fail_if_one_task_is_failed(self):
                     user=get_user_model().objects.first(),
                     func_name="test",
                     step="test",
-                    legacy_upload_name="test",
                     input_params={
                         "asset_id": asset.id,
                         "asset_module_path": f"{asset.__module__}.{asset.__class__.__name__}",
@@ -389,7 +364,6 @@ def test_evaluate_execution_progress_should_set_as_completed(self):
                     user=get_user_model().objects.first(),
                     func_name="test",
                     step="test",
-                    legacy_upload_name="test",
                 )
             )
 

importer/tests/unit/test_task.py

@@ -57,7 +57,6 @@ def setUp(self):
             user=get_user_model().objects.get(username=self.user),
             func_name="dummy_func",
             step="dummy_step",
-            legacy_upload_name="dummy",
             input_params={
                 "files": {"base_file": self.existing_file},
                 # "overwrite_existing_layer": True,
@@ -97,7 +96,6 @@ def test_import_resource_should_rase_exp_if_is_invalid(
             user=get_user_model().objects.get(username=user),
             func_name="dummy_func",
             step="dummy_step",
-            legacy_upload_name="dummy",
             input_params={"files": self.existing_file, "store_spatial_files": True},
         )
 
@@ -131,7 +129,6 @@ def test_import_resource_should_work(
             user=get_user_model().objects.get(username=user),
             func_name="dummy_func",
             step="dummy_step",
-            legacy_upload_name="dummy",
             input_params={"files": self.existing_file, "store_spatial_files": True},
         )
 
@@ -203,7 +200,6 @@ def test_publish_resource_if_overwrite_should_call_the_publishing(
                 user=get_user_model().objects.get(username=self.user),
                 func_name="dummy_func",
                 step="dummy_step",
-                legacy_upload_name="dummy",
                 input_params={
                     "files": {"base_file": self.existing_file},
                     "overwrite_existing_layer": True,
@@ -258,7 +254,6 @@ def test_publish_resource_if_overwrite_should_not_call_the_publishing(
                     user=get_user_model().objects.get(username=self.user),
                     func_name="dummy_func",
                     step="dummy_step",
-                    legacy_upload_name="dummy",
                     input_params={
                         "files": {"base_file": self.existing_file},
                         "overwrite_existing_layer": True,
@@ -531,7 +526,6 @@ def setUp(self):
             user=get_user_model().objects.get(username=self.user),
             func_name="dummy_func",
             step="dummy_step",
-            legacy_upload_name="dummy",
             input_params={
                 "files": {"base_file": self.existing_file},
                 # "overwrite_existing_layer": True,