From 32564640e7f3b89c6cc946d3e4fe97b261ac88ca Mon Sep 17 00:00:00 2001 From: etj Date: Mon, 22 Apr 2024 17:21:50 +0200 Subject: [PATCH 01/40] Changes to support Assets --- Dockerfile | 1 + importer/handlers/common/raster.py | 4 ++++ importer/handlers/common/vector.py | 4 ++++ 3 files changed, 9 insertions(+) diff --git a/Dockerfile b/Dockerfile index fabb7a98..789aa669 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,6 +1,7 @@ FROM geonode/geonode-base:latest-ubuntu-22.04 RUN git clone https://github.com/GeoNode/geonode.git /usr/src/geonode +RUN cd /usr/src/geonode && git checkout 12124_assets && cd - RUN mkdir -p /usr/src/importer RUN cd .. diff --git a/importer/handlers/common/raster.py b/importer/handlers/common/raster.py index 7f2377f5..8e51e1e5 100644 --- a/importer/handlers/common/raster.py +++ b/importer/handlers/common/raster.py @@ -362,6 +362,10 @@ def create_geonode_resource( dirty_state=True, title=layer_name, owner=_exec.user, + extension=self.supported_file_extension_config["id"], + data_title="Original", + data_type=self.supported_file_extension_config["label"], + link_type="uploaded", # should be in geonode.base.enumerations.LINK_TYPES files=list( set( list(_exec.input_params.get("files", {}).values()) diff --git a/importer/handlers/common/vector.py b/importer/handlers/common/vector.py index fb198491..eff007d7 100644 --- a/importer/handlers/common/vector.py +++ b/importer/handlers/common/vector.py @@ -594,6 +594,10 @@ def create_geonode_resource( dirty_state=True, title=layer_name, owner=_exec.user, + data_title="Original", + data_type=self.supported_file_extension_config["label"], + extension=self.supported_file_extension_config["id"], + link_type="uploaded", # should be in geonode.base.enumerations.LINK_TYPES files=list( set( list(_exec.input_params.get("files", {}).values()) From e8713cbbbeb6af02a1357a70f46dc0098685304e Mon Sep 17 00:00:00 2001 From: mattiagiupponi Date: Wed, 8 May 2024 11:00:54 +0200 Subject: [PATCH 02/40] Fix asset build --- importer/handlers/common/vector.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/importer/handlers/common/vector.py b/importer/handlers/common/vector.py index 95343922..17af9004 100644 --- a/importer/handlers/common/vector.py +++ b/importer/handlers/common/vector.py @@ -31,6 +31,7 @@ from importer.api.exception import ImportException from importer.celery_app import importer_app from geonode.storage.manager import storage_manager +from geonode.assets.utils import copy_assets_and_links from importer.handlers.utils import create_alternate, should_be_imported from importer.models import ResourceHandlerInfo @@ -760,14 +761,15 @@ def copy_geonode_resource( new_alternate: str, **kwargs, ): - resource = self.create_geonode_resource( + new_resource = self.create_geonode_resource( layer_name=data_to_update.get("title"), alternate=new_alternate, execution_id=str(_exec.exec_id), - files=resource.files, + files=[], ) - resource.refresh_from_db() - return resource + copy_assets_and_links(resource, target=new_resource) + new_resource.refresh_from_db() + return new_resource def get_ogr2ogr_task_group( self, From e9d8ced5209874f45f0127758f7e78e315a103ef Mon Sep 17 00:00:00 2001 From: mattiagiupponi Date: Wed, 8 May 2024 12:08:52 +0200 Subject: [PATCH 03/40] Fix asset build --- importer/handlers/common/vector.py | 12 +++++++----- importer/tests/unit/test_task.py | 17 ++++++++++------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/importer/handlers/common/vector.py b/importer/handlers/common/vector.py index 17af9004..345893b6 100644 --- a/importer/handlers/common/vector.py +++ b/importer/handlers/common/vector.py @@ -31,7 +31,7 @@ from importer.api.exception import ImportException from importer.celery_app import importer_app from geonode.storage.manager import storage_manager -from geonode.assets.utils import copy_assets_and_links +from geonode.assets.utils import copy_assets_and_links, get_default_asset from importer.handlers.utils import create_alternate, should_be_imported from importer.models import ResourceHandlerInfo @@ -245,10 +245,12 @@ def perform_last_step(execution_id): _exec.save() if _exec and not _exec.input_params.get("store_spatial_file", False): resources = ResourceHandlerInfo.objects.filter(execution_request=_exec) - # getting all files list - resources_files = list(set(chain(*[x.resource.files for x in resources]))) - # better to delete each single file since it can be a remove storage service - list(map(storage_manager.delete, resources_files)) + # getting all assets list + assets = [get_default_asset(x.resource) for x in resources] + # we need to loop and cancel one by one to activate the signal + # which delete the file from the filesystem + for asset in assets: + asset.delete() def extract_resource_to_publish( self, files, action, layer_name, alternate, **kwargs diff --git a/importer/tests/unit/test_task.py b/importer/tests/unit/test_task.py index c9b599f9..8ea4c955 100644 --- a/importer/tests/unit/test_task.py +++ b/importer/tests/unit/test_task.py @@ -27,6 +27,7 @@ from dynamic_models.models import ModelSchema, FieldSchema from dynamic_models.exceptions import DynamicModelError, InvalidFieldNameError from importer.models import ResourceHandlerInfo +from importer import project_dir from importer.tests.utils import ( ImporterBaseTestSupport, @@ -39,13 +40,14 @@ class TestCeleryTasks(ImporterBaseTestSupport): def setUp(self): self.user = get_user_model().objects.first() + self.existing_file = f"{project_dir}/tests/fixture/valid.gpkg" self.exec_id = orchestrator.create_execution_request( 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": "/filepath"}, + "files": {"base_file": self.existing_file}, # "overwrite_existing_layer": True, "store_spatial_files": True, }, @@ -82,7 +84,7 @@ def test_import_resource_should_rase_exp_if_is_invalid( func_name="dummy_func", step="dummy_step", legacy_upload_name="dummy", - input_params={"files": "/filepath", "store_spatial_files": True}, + input_params={"files": self.existing_file, "store_spatial_files": True}, ) is_valid.side_effect = Exception("Invalid format type") @@ -116,7 +118,7 @@ def test_import_resource_should_work( func_name="dummy_func", step="dummy_step", legacy_upload_name="dummy", - input_params={"files": "/filepath", "store_spatial_files": True}, + input_params={"files": self.existing_file, "store_spatial_files": True}, ) import_resource( @@ -189,7 +191,7 @@ def test_publish_resource_if_overwrite_should_call_the_publishing( step="dummy_step", legacy_upload_name="dummy", input_params={ - "files": {"base_file": "/filepath"}, + "files": {"base_file": self.existing_file}, "overwrite_existing_layer": True, "store_spatial_files": True, }, @@ -244,7 +246,7 @@ def test_publish_resource_if_overwrite_should_not_call_the_publishing( step="dummy_step", legacy_upload_name="dummy", input_params={ - "files": {"base_file": "/filepath"}, + "files": {"base_file": self.existing_file}, "overwrite_existing_layer": True, "store_spatial_files": True, }, @@ -388,7 +390,7 @@ def test_rollback_works_as_expected_vector_step( step=conf[0], # step name action="import", input_params={ - "files": {"base_file": "/filepath"}, + "files": {"base_file": self.existing_file}, "overwrite_existing_layer": True, "store_spatial_files": True, "handler_module_path": "importer.handlers.gpkg.handler.GPKGFileHandler", @@ -509,13 +511,14 @@ class TestDynamicModelSchema(TransactionImporterBaseTestSupport): def setUp(self): self.user = get_user_model().objects.first() + self.existing_file = f"{project_dir}/tests/fixture/valid.gpkg" self.exec_id = orchestrator.create_execution_request( 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": "/filepath"}, + "files": {"base_file": self.existing_file}, # "overwrite_existing_layer": True, "store_spatial_files": True, }, From 98833fa4e2014829836ff1f0aff5f380c482edc2 Mon Sep 17 00:00:00 2001 From: mattiagiupponi Date: Fri, 10 May 2024 15:51:36 +0200 Subject: [PATCH 04/40] Remove original file at upload end --- importer/handlers/common/metadata.py | 6 ++++++ importer/handlers/common/raster.py | 5 +++++ importer/handlers/common/vector.py | 7 +++++++ 3 files changed, 18 insertions(+) diff --git a/importer/handlers/common/metadata.py b/importer/handlers/common/metadata.py index 5b303577..1ef78fb8 100644 --- a/importer/handlers/common/metadata.py +++ b/importer/handlers/common/metadata.py @@ -7,6 +7,7 @@ from importer.orchestrator import orchestrator from django.shortcuts import get_object_or_404 from geonode.layers.models import Dataset +from geonode.storage.manager import storage_manager logger = logging.getLogger(__name__) @@ -66,6 +67,11 @@ def perform_last_step(execution_id): } ) _exec.save() + # since the original file is now available as asset, we can delete the input files + # TODO must be improved. The asset should be created in the beginning + for _file in _exec.input_params.get("files", {}).values(): + if storage_manager.exists(_file): + storage_manager.delete(_file) def import_resource(self, files: dict, execution_id: str, **kwargs): _exec = orchestrator.get_execution_object(execution_id) diff --git a/importer/handlers/common/raster.py b/importer/handlers/common/raster.py index 02999693..ce18f156 100644 --- a/importer/handlers/common/raster.py +++ b/importer/handlers/common/raster.py @@ -214,6 +214,11 @@ def perform_last_step(execution_id): } ) _exec.save() + # since the original file is now available as asset, we can delete the input files + # TODO must be improved. The asset should be created in the beginning + for _file in _exec.input_params.get("files", {}).values(): + if storage_manager.exists(_file): + storage_manager.delete(_file) def extract_resource_to_publish( self, files, action, layer_name, alternate, **kwargs diff --git a/importer/handlers/common/vector.py b/importer/handlers/common/vector.py index 345893b6..213ff216 100644 --- a/importer/handlers/common/vector.py +++ b/importer/handlers/common/vector.py @@ -252,6 +252,13 @@ def perform_last_step(execution_id): for asset in assets: asset.delete() + # since the original file is now available as asset, we can delete the input files + # TODO must be improved. The asset should be created in the beginning + for _file in _exec.input_params.get("files", {}).values(): + if storage_manager.exists(_file): + storage_manager.delete(_file) + + def extract_resource_to_publish( self, files, action, layer_name, alternate, **kwargs ): From caa720047b6d49080483bbd382679a8fbe16261b Mon Sep 17 00:00:00 2001 From: Mattia Date: Thu, 23 May 2024 15:23:00 +0200 Subject: [PATCH 05/40] Let importer create the asset --- importer/api/views.py | 39 ++++++++++++++++++++++-- importer/celery_tasks.py | 10 ++++-- importer/handlers/base.py | 8 ++--- importer/handlers/common/raster.py | 2 +- importer/handlers/common/tests_vector.py | 2 +- importer/handlers/common/vector.py | 17 +++-------- importer/handlers/csv/handler.py | 4 +-- importer/handlers/sld/tests.py | 5 ++- importer/handlers/xml/tests.py | 5 ++- importer/tests/unit/test_task.py | 2 +- 10 files changed, 64 insertions(+), 30 deletions(-) diff --git a/importer/api/views.py b/importer/api/views.py index 0d71b5eb..a5bd7a10 100644 --- a/importer/api/views.py +++ b/importer/api/views.py @@ -17,6 +17,8 @@ # ######################################################################### import logging +import os +from pathlib import Path from urllib.parse import urljoin from django.conf import settings from django.urls import reverse @@ -46,6 +48,9 @@ from rest_framework.parsers import FileUploadParser, MultiPartParser from rest_framework.permissions import IsAuthenticatedOrReadOnly from rest_framework.response import Response +from geonode.assets.handlers import asset_handler_registry +from geonode.utils import get_allowed_extensions +from geonode.assets.local import LocalAssetHandler logger = logging.getLogger(__name__) @@ -91,6 +96,8 @@ def create(self, request, *args, **kwargs): """ _file = request.FILES.get("base_file") or request.data.get("base_file") execution_id = None + asset_handler = LocalAssetHandler() + asset_dir = asset_handler._create_asset_dir() serializer = self.get_serializer_class() data = serializer(data=request.data) @@ -111,7 +118,9 @@ def create(self, request, *args, **kwargs): remote_files={"base_file": _data.get("zip_file", _data.get("kmz_file"))} ) # cloning and unzip the base_file - storage_manager.clone_remote_files() + storage_manager.clone_remote_files( + cloning_directory=asset_dir, create_tempdir=False + ) # update the payload with the unziped paths _data.update(storage_manager.get_retrieved_paths()) @@ -125,9 +134,13 @@ def create(self, request, *args, **kwargs): # means that the storage manager is not initialized yet, so # the file is not a zip storage_manager = StorageManager(remote_files=_data) - storage_manager.clone_remote_files() + storage_manager.clone_remote_files( + cloning_directory=asset_dir, create_tempdir=False + ) # get filepath - files = storage_manager.get_retrieved_paths() + asset, files = self.generate_asset_and_retrieve_paths( + request, storage_manager, handler + ) upload_validator = UploadLimitValidator(request.user) upload_validator.validate_parallelism_limit_per_user() @@ -144,6 +157,10 @@ def create(self, request, *args, **kwargs): input_params={ **{"files": files, "handler_module_path": str(handler)}, **extracted_params, + **{ + "asset_id": asset.id, + "asset_module_path": f"{asset.__module__}.{asset.__class__.__name__}", + }, }, legacy_upload_name=_file.name, action=action, @@ -161,6 +178,8 @@ def create(self, request, *args, **kwargs): # cloned files to keep the storage under control if storage_manager is not None: storage_manager.delete_retrieved_paths(force=True) + if asset: + asset.objects.delete() if execution_id: orchestrator.set_as_failed(execution_id=str(execution_id), reason=e) logger.exception(e) @@ -168,6 +187,20 @@ def create(self, request, *args, **kwargs): raise ImportException(detail="No handlers found for this dataset type") + def generate_asset_and_retrieve_paths(self, request, storage_manager, handler): + asset_handler = asset_handler_registry.get_default_handler() + _files = storage_manager.get_retrieved_paths() + asset = asset_handler.create( + title="Original", + owner=request.user, + description=None, + type=str(handler), + files=list(set(_files.values())), + clone_files=False, + ) + + return asset, _files + class ResourceImporter(DynamicModelViewSet): authentication_classes = [ diff --git a/importer/celery_tasks.py b/importer/celery_tasks.py index c7cfb2f9..a86d3da4 100644 --- a/importer/celery_tasks.py +++ b/importer/celery_tasks.py @@ -329,6 +329,12 @@ def create_geonode_resource( _files = _exec.input_params.get("files") + _asset = ( + import_string(_exec.input_params.get("asset_module_path")) + .objects.filter(id=_exec.input_params.get("asset_id")) + .first() + ) + handler = import_string(handler_module_path)() _overwrite = _exec.input_params.get("overwrite_existing_layer") @@ -337,14 +343,14 @@ def create_geonode_resource( layer_name=layer_name, alternate=alternate, execution_id=execution_id, - files=_files, + asset=_asset, ) else: resource = handler.create_geonode_resource( layer_name=layer_name, alternate=alternate, execution_id=execution_id, - files=_files, + asset=_asset, ) if _overwrite: diff --git a/importer/handlers/base.py b/importer/handlers/base.py index 6e29446e..43c1041f 100644 --- a/importer/handlers/base.py +++ b/importer/handlers/base.py @@ -150,12 +150,12 @@ def perform_last_step(execution_id): ] _exec.output_params.update({"resources": resource_output_params}) _exec.save() - + # since the original file is now available as asset, we can delete the input files # TODO must be improved. The asset should be created in the beginning - for _file in _exec.input_params.get("files", {}).values(): - if storage_manager.exists(_file): - storage_manager.delete(_file) + # for _file in _exec.input_params.get("files", {}).values(): + # if storage_manager.exists(_file): + # storage_manager.delete(_file) return _exec diff --git a/importer/handlers/common/raster.py b/importer/handlers/common/raster.py index ae9cd968..4f261020 100644 --- a/importer/handlers/common/raster.py +++ b/importer/handlers/common/raster.py @@ -312,7 +312,7 @@ def create_geonode_resource( alternate: str, execution_id: str, resource_type: Dataset = Dataset, - files=None, + asset=None, ): """ Base function to create the resource into geonode. Each handler can specify diff --git a/importer/handlers/common/tests_vector.py b/importer/handlers/common/tests_vector.py index a2fe8efb..300b09e9 100644 --- a/importer/handlers/common/tests_vector.py +++ b/importer/handlers/common/tests_vector.py @@ -328,7 +328,7 @@ def test_select_valid_layers(self): self.assertEqual(1, len(valid_layer)) self.assertEqual("mattia_test", valid_layer[0].GetName()) - @override_settings(MEDIA_ROOT='/tmp') + @override_settings(MEDIA_ROOT="/tmp") def test_perform_last_step(self): """ Output params in perform_last_step should return the detail_url and the ID diff --git a/importer/handlers/common/vector.py b/importer/handlers/common/vector.py index d6b6d3a0..a6e9d526 100644 --- a/importer/handlers/common/vector.py +++ b/importer/handlers/common/vector.py @@ -228,8 +228,7 @@ def perform_last_step(execution_id): # which delete the file from the filesystem for asset in assets: asset.delete() - - + def extract_resource_to_publish( self, files, action, layer_name, alternate, **kwargs ): @@ -568,7 +567,7 @@ def create_geonode_resource( alternate: str, execution_id: str, resource_type: Dataset = Dataset, - files=None, + asset=None, ): """ Base function to create the resource into geonode. Each handler can specify @@ -591,6 +590,7 @@ def create_geonode_resource( logger.warning( f"The dataset required {alternate} does not exists, but an overwrite is required, the resource will be created" ) + saved_dataset = resource_manager.create( None, resource_type=resource_type, @@ -603,16 +603,7 @@ def create_geonode_resource( dirty_state=True, title=layer_name, owner=_exec.user, - data_title="Original", - data_type=self.supported_file_extension_config["label"], - extension=self.supported_file_extension_config["id"], - link_type="uploaded", # should be in geonode.base.enumerations.LINK_TYPES - files=list( - set( - list(_exec.input_params.get("files", {}).values()) - or list(files) - ) - ), + asset=asset, ), ) diff --git a/importer/handlers/csv/handler.py b/importer/handlers/csv/handler.py index 67b338f8..f1433ed2 100644 --- a/importer/handlers/csv/handler.py +++ b/importer/handlers/csv/handler.py @@ -243,9 +243,7 @@ def extract_resource_to_publish( return [ { "name": alternate or layer_name, - "crs": ( - self.identify_authority(_l) - ), + "crs": (self.identify_authority(_l)), } for _l in layers if self.fixup_name(_l.GetName()) == layer_name diff --git a/importer/handlers/sld/tests.py b/importer/handlers/sld/tests.py index ace68be0..26f0eb99 100644 --- a/importer/handlers/sld/tests.py +++ b/importer/handlers/sld/tests.py @@ -24,7 +24,10 @@ def setUpClass(cls): cls.user, _ = get_user_model().objects.get_or_create(username="admin") cls.invalid_files = {"base_file": cls.invalid_sld, "sld_file": cls.invalid_sld} - cls.valid_files = {"base_file": "/tmp/test_sld.sld", "sld_file": "/tmp/test_sld.sld"} + cls.valid_files = { + "base_file": "/tmp/test_sld.sld", + "sld_file": "/tmp/test_sld.sld", + } cls.owner = get_user_model().objects.first() cls.layer = create_single_dataset(name="sld_dataset", owner=cls.owner) diff --git a/importer/handlers/xml/tests.py b/importer/handlers/xml/tests.py index 9262f7c8..8f51e3cc 100644 --- a/importer/handlers/xml/tests.py +++ b/importer/handlers/xml/tests.py @@ -23,7 +23,10 @@ def setUpClass(cls): shutil.copy(cls.valid_xml, "/tmp") cls.user, _ = get_user_model().objects.get_or_create(username="admin") cls.invalid_files = {"base_file": cls.invalid_xml, "xml_file": cls.invalid_xml} - cls.valid_files = {"base_file": "/tmp/test_xml.xml", "xml_file": "/tmp/test_xml.xml"} + cls.valid_files = { + "base_file": "/tmp/test_xml.xml", + "xml_file": "/tmp/test_xml.xml", + } cls.owner = get_user_model().objects.first() cls.layer = create_single_dataset(name="extruded_polygon", owner=cls.owner) diff --git a/importer/tests/unit/test_task.py b/importer/tests/unit/test_task.py index 8d37b219..9b49b483 100644 --- a/importer/tests/unit/test_task.py +++ b/importer/tests/unit/test_task.py @@ -506,7 +506,7 @@ def test_import_metadata_should_work_as_expected(self): layer.refresh_from_db() self.assertEqual(layer.title, "test_dataset") - #verify that the original has been deleted + # verify that the original has been deleted self.assertFalse(os.path.exists(xml_in_tmp)) From 437346a76d51a3f7706c132f37d148c67d45bb84 Mon Sep 17 00:00:00 2001 From: Mattia Date: Thu, 23 May 2024 15:27:21 +0200 Subject: [PATCH 06/40] Let importer create the asset --- importer/handlers/common/raster.py | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/importer/handlers/common/raster.py b/importer/handlers/common/raster.py index 4f261020..522a2830 100644 --- a/importer/handlers/common/raster.py +++ b/importer/handlers/common/raster.py @@ -335,6 +335,7 @@ def create_geonode_resource( logger.warning( f"The dataset required {alternate} does not exists, but an overwrite is required, the resource will be created" ) + saved_dataset = resource_manager.create( None, resource_type=resource_type, @@ -346,16 +347,7 @@ def create_geonode_resource( dirty_state=True, title=layer_name, owner=_exec.user, - extension=self.supported_file_extension_config["id"], - data_title="Original", - data_type=self.supported_file_extension_config["label"], - link_type="uploaded", # should be in geonode.base.enumerations.LINK_TYPES - files=list( - set( - list(_exec.input_params.get("files", {}).values()) - or list(files) - ) - ), + asset=asset ), ) From 50d2a08b8874c2d2a277f48b49e17a60bdb75f8f Mon Sep 17 00:00:00 2001 From: etj Date: Fri, 24 May 2024 09:41:44 +0200 Subject: [PATCH 07/40] Fixes --- importer/api/views.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/importer/api/views.py b/importer/api/views.py index a5bd7a10..b5d145ca 100644 --- a/importer/api/views.py +++ b/importer/api/views.py @@ -127,6 +127,7 @@ def create(self, request, *args, **kwargs): handler = orchestrator.get_handler(_data) if _file and handler: + asset = None try: # cloning data into a local folder extracted_params, _data = handler.extract_params_from_data(_data) @@ -179,7 +180,7 @@ def create(self, request, *args, **kwargs): if storage_manager is not None: storage_manager.delete_retrieved_paths(force=True) if asset: - asset.objects.delete() + asset.delete() if execution_id: orchestrator.set_as_failed(execution_id=str(execution_id), reason=e) logger.exception(e) From c2b4f2c2a9c896157cb64810ac6b614ab500bfc1 Mon Sep 17 00:00:00 2001 From: Mattia Date: Fri, 24 May 2024 15:48:57 +0200 Subject: [PATCH 08/40] Add test coverage for asset-importer --- importer/api/tests.py | 48 +++++++++++++++++++++++++++++++++++++++++++ importer/api/views.py | 2 +- 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/importer/api/tests.py b/importer/api/tests.py index bcf8b357..a0f98730 100644 --- a/importer/api/tests.py +++ b/importer/api/tests.py @@ -11,6 +11,9 @@ from importer.models import ResourceHandlerInfo from importer.tests.utils import ImporterBaseTestSupport +from importer.orchestrator import orchestrator +from django.utils.module_loading import import_string +from geonode.assets.models import LocalAsset class TestImporterViewSet(ImporterBaseTestSupport): @@ -153,3 +156,48 @@ def test_copy_ther_resource_if_file_handler_is_set(self, _orc): self.assertEqual(200, response.status_code) _orc.s.assert_called_once() + + @patch("importer.api.views.import_orchestrator") + def test_asset_is_created_before_the_import_start(self, patch_upload): + patch_upload.apply_async.side_effect = MagicMock() + + self.client.force_login(get_user_model().objects.get(username="admin")) + payload = { + "base_file": SimpleUploadedFile( + name="test.geojson", content=b"some-content" + ), + "store_spatial_files": True, + } + + response = self.client.post(self.url, data=payload) + + self.assertEqual(201, response.status_code) + + self.assertTrue(201, response.status_code) + + _exec = orchestrator.get_execution_object(response.json()['execution_id']) + + asset_handler = import_string(_exec.input_params['asset_module_path']) + self.assertTrue(asset_handler.objects.filter(id=_exec.input_params['asset_id'])) + + asset_handler.objects.filter(id=_exec.input_params['asset_id']).delete() + + + @patch("importer.api.views.import_orchestrator") + @patch("importer.api.views.UploadLimitValidator.validate_parallelism_limit_per_user") + def test_asset_should_be_deleted_if_created_during_with_exception(self, validate_parallelism_limit_per_user, patch_upload): + patch_upload.apply_async.s.side_effect = MagicMock() + validate_parallelism_limit_per_user.side_effect = Exception("random exception") + + self.client.force_login(get_user_model().objects.get(username="admin")) + payload = { + "base_file": SimpleUploadedFile( + name="test.geojson", content=b"some-content" + ), + "store_spatial_files": True, + } + + response = self.client.post(self.url, data=payload) + + self.assertEqual(500, response.status_code) + self.assertFalse(LocalAsset.objects.exists()) diff --git a/importer/api/views.py b/importer/api/views.py index a5bd7a10..a3ecef67 100644 --- a/importer/api/views.py +++ b/importer/api/views.py @@ -179,7 +179,7 @@ def create(self, request, *args, **kwargs): if storage_manager is not None: storage_manager.delete_retrieved_paths(force=True) if asset: - asset.objects.delete() + asset.delete() if execution_id: orchestrator.set_as_failed(execution_id=str(execution_id), reason=e) logger.exception(e) From 749f085c26cb246d56c7b9d0d9287d52975c71fc Mon Sep 17 00:00:00 2001 From: Mattia Date: Fri, 24 May 2024 15:51:03 +0200 Subject: [PATCH 09/40] Add test coverage for asset-importer --- importer/api/tests.py | 17 ++++++++++------- importer/api/views.py | 3 --- importer/handlers/base.py | 7 ------- importer/handlers/common/metadata.py | 1 - importer/handlers/common/raster.py | 2 +- importer/handlers/common/vector.py | 1 - 6 files changed, 11 insertions(+), 20 deletions(-) diff --git a/importer/api/tests.py b/importer/api/tests.py index a0f98730..7c54b2ba 100644 --- a/importer/api/tests.py +++ b/importer/api/tests.py @@ -175,17 +175,20 @@ def test_asset_is_created_before_the_import_start(self, patch_upload): self.assertTrue(201, response.status_code) - _exec = orchestrator.get_execution_object(response.json()['execution_id']) + _exec = orchestrator.get_execution_object(response.json()["execution_id"]) - asset_handler = import_string(_exec.input_params['asset_module_path']) - self.assertTrue(asset_handler.objects.filter(id=_exec.input_params['asset_id'])) - - asset_handler.objects.filter(id=_exec.input_params['asset_id']).delete() + asset_handler = import_string(_exec.input_params["asset_module_path"]) + self.assertTrue(asset_handler.objects.filter(id=_exec.input_params["asset_id"])) + asset_handler.objects.filter(id=_exec.input_params["asset_id"]).delete() @patch("importer.api.views.import_orchestrator") - @patch("importer.api.views.UploadLimitValidator.validate_parallelism_limit_per_user") - def test_asset_should_be_deleted_if_created_during_with_exception(self, validate_parallelism_limit_per_user, patch_upload): + @patch( + "importer.api.views.UploadLimitValidator.validate_parallelism_limit_per_user" + ) + def test_asset_should_be_deleted_if_created_during_with_exception( + self, validate_parallelism_limit_per_user, patch_upload + ): patch_upload.apply_async.s.side_effect = MagicMock() validate_parallelism_limit_per_user.side_effect = Exception("random exception") diff --git a/importer/api/views.py b/importer/api/views.py index b5d145ca..d3f34968 100644 --- a/importer/api/views.py +++ b/importer/api/views.py @@ -17,8 +17,6 @@ # ######################################################################### import logging -import os -from pathlib import Path from urllib.parse import urljoin from django.conf import settings from django.urls import reverse @@ -49,7 +47,6 @@ from rest_framework.permissions import IsAuthenticatedOrReadOnly from rest_framework.response import Response from geonode.assets.handlers import asset_handler_registry -from geonode.utils import get_allowed_extensions from geonode.assets.local import LocalAssetHandler logger = logging.getLogger(__name__) diff --git a/importer/handlers/base.py b/importer/handlers/base.py index 43c1041f..e005fcd4 100644 --- a/importer/handlers/base.py +++ b/importer/handlers/base.py @@ -7,7 +7,6 @@ from importer.utils import ImporterRequestAction as ira from django_celery_results.models import TaskResult from django.db.models import Q -from geonode.storage.manager import storage_manager logger = logging.getLogger(__name__) @@ -151,12 +150,6 @@ def perform_last_step(execution_id): _exec.output_params.update({"resources": resource_output_params}) _exec.save() - # since the original file is now available as asset, we can delete the input files - # TODO must be improved. The asset should be created in the beginning - # for _file in _exec.input_params.get("files", {}).values(): - # if storage_manager.exists(_file): - # storage_manager.delete(_file) - return _exec def fixup_name(self, name): diff --git a/importer/handlers/common/metadata.py b/importer/handlers/common/metadata.py index 8b86db1c..14a80454 100644 --- a/importer/handlers/common/metadata.py +++ b/importer/handlers/common/metadata.py @@ -7,7 +7,6 @@ from importer.orchestrator import orchestrator from django.shortcuts import get_object_or_404 from geonode.layers.models import Dataset -from geonode.storage.manager import storage_manager logger = logging.getLogger(__name__) diff --git a/importer/handlers/common/raster.py b/importer/handlers/common/raster.py index 522a2830..419c86d6 100644 --- a/importer/handlers/common/raster.py +++ b/importer/handlers/common/raster.py @@ -347,7 +347,7 @@ def create_geonode_resource( dirty_state=True, title=layer_name, owner=_exec.user, - asset=asset + asset=asset, ), ) diff --git a/importer/handlers/common/vector.py b/importer/handlers/common/vector.py index a6e9d526..7d4b34bb 100644 --- a/importer/handlers/common/vector.py +++ b/importer/handlers/common/vector.py @@ -2,7 +2,6 @@ from django.db import connections from importer.publisher import DataPublisher from importer.utils import call_rollback_function, find_key_recursively -from itertools import chain import json import logging import os From 4e37f9de1548754a9b2de5375f99569374543775 Mon Sep 17 00:00:00 2001 From: Mattia Date: Fri, 24 May 2024 15:53:36 +0200 Subject: [PATCH 10/40] Add test coverage for asset-importer --- importer/handlers/common/vector.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/importer/handlers/common/vector.py b/importer/handlers/common/vector.py index 7d4b34bb..d311b07c 100644 --- a/importer/handlers/common/vector.py +++ b/importer/handlers/common/vector.py @@ -224,7 +224,7 @@ def perform_last_step(execution_id): # getting all assets list assets = [get_default_asset(x.resource) for x in resources] # we need to loop and cancel one by one to activate the signal - # which delete the file from the filesystem + # that delete the file from the filesystem for asset in assets: asset.delete() From e1344a9f5e0122c90cfb6a2f85f025f3c725b648 Mon Sep 17 00:00:00 2001 From: Mattia Date: Fri, 24 May 2024 15:59:53 +0200 Subject: [PATCH 11/40] Add test coverage for asset-importer --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index a6fc2c4a..48d40a98 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,7 @@ FROM geonode/geonode-base:latest-ubuntu-22.04 RUN rm -rf /usr/src/geonode RUN git clone https://github.com/GeoNode/geonode.git /usr/src/geonode -RUN cd /usr/src/geonode && git checkout 12124_assets && cd - +RUN cd /usr/src/geonode && git checkout 12124_assets_20240523 && cd - RUN mkdir -p /usr/src/importer RUN cd .. From c9c5251ed78b5ceec0ee4191a618f1a7158fa0e9 Mon Sep 17 00:00:00 2001 From: Mattia Date: Mon, 27 May 2024 14:59:20 +0200 Subject: [PATCH 12/40] Fix test coverage --- .env_test | 5 +++-- Dockerfile | 2 +- importer/handlers/README.md | 2 +- importer/handlers/common/raster.py | 8 ++++---- importer/handlers/common/vector.py | 9 +++++---- importer/tests/end2end/test_end2end.py | 3 ++- importer/tests/unit/test_task.py | 18 +++++++++++++++--- runtest.sh | 2 +- 8 files changed, 32 insertions(+), 17 deletions(-) diff --git a/.env_test b/.env_test index dcfa53ec..f751770a 100644 --- a/.env_test +++ b/.env_test @@ -178,8 +178,9 @@ DEBUG=False SECRET_KEY='myv-y4#7j-d*p-__@j#*3z@!y24fz8%^z2v6atuy4bo9vqr1_a' -STATIC_ROOT=/mnt/volumes/statics/static/ -MEDIA_ROOT=/mnt/volumes/statics/uploaded/ +STATIC_ROOT=/tmp/statics/static/ +MEDIA_ROOT=/tmp/statics/uploaded/ +ASSET_ROOT=/tmp/statics/assets/ GEOIP_PATH=/mnt/volumes/statics/geoip.db CACHE_BUSTING_STATIC_ENABLED=False diff --git a/Dockerfile b/Dockerfile index 48d40a98..083ebab5 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,7 @@ FROM geonode/geonode-base:latest-ubuntu-22.04 RUN rm -rf /usr/src/geonode RUN git clone https://github.com/GeoNode/geonode.git /usr/src/geonode -RUN cd /usr/src/geonode && git checkout 12124_assets_20240523 && cd - +RUN cd /usr/src/geonode && git fetch --all && git checkout 12124_assets_20240523 && cd - RUN mkdir -p /usr/src/importer RUN cd .. diff --git a/importer/handlers/README.md b/importer/handlers/README.md index 8916e951..37d982a7 100644 --- a/importer/handlers/README.md +++ b/importer/handlers/README.md @@ -158,7 +158,7 @@ class BaseVectorFileHandler(BaseHandler): return def overwrite_geonode_resource( - self, layer_name: str, alternate: str, execution_id: str, resource_type: Dataset = Dataset, files=None + self, layer_name: str, alternate: str, execution_id: str, resource_type: Dataset = Dataset, asset=None ): """ Base function to override the resource into geonode. Each handler can specify diff --git a/importer/handlers/common/raster.py b/importer/handlers/common/raster.py index 419c86d6..2990c4c5 100644 --- a/importer/handlers/common/raster.py +++ b/importer/handlers/common/raster.py @@ -369,7 +369,7 @@ def overwrite_geonode_resource( alternate: str, execution_id: str, resource_type: Dataset = Dataset, - files=None, + asset=None, ): dataset = resource_type.objects.filter(alternate__icontains=alternate) @@ -397,7 +397,7 @@ def overwrite_geonode_resource( f"The dataset required {alternate} does not exists, but an overwrite is required, the resource will be created" ) return self.create_geonode_resource( - layer_name, alternate, execution_id, resource_type, files + layer_name, alternate, execution_id, resource_type, asset ) elif not dataset.exists() and not _overwrite: logger.warning( @@ -479,9 +479,9 @@ def copy_geonode_resource( layer_name=data_to_update.get("title"), alternate=new_alternate, execution_id=str(_exec.exec_id), - files=kwargs.get("kwargs", {}) + asset=kwargs.get("kwargs", {}) .get("new_file_location", {}) - .get("files", []), + .get("asset", []), ) resource.refresh_from_db() return resource diff --git a/importer/handlers/common/vector.py b/importer/handlers/common/vector.py index d311b07c..e53a9610 100644 --- a/importer/handlers/common/vector.py +++ b/importer/handlers/common/vector.py @@ -624,7 +624,7 @@ def overwrite_geonode_resource( alternate: str, execution_id: str, resource_type: Dataset = Dataset, - files=None, + asset=None, ): dataset = resource_type.objects.filter(alternate__icontains=alternate) @@ -637,7 +637,7 @@ def overwrite_geonode_resource( dataset = dataset.first() dataset = resource_manager.update( - dataset.uuid, instance=dataset, files=files + dataset.uuid, instance=dataset, files=asset.location ) self.handle_xml_file(dataset, _exec) @@ -653,7 +653,7 @@ def overwrite_geonode_resource( f"The dataset required {alternate} does not exists, but an overwrite is required, the resource will be created" ) return self.create_geonode_resource( - layer_name, alternate, execution_id, resource_type, files + layer_name, alternate, execution_id, resource_type, asset ) elif not dataset.exists() and not _overwrite: logger.warning( @@ -731,11 +731,12 @@ def copy_geonode_resource( new_alternate: str, **kwargs, ): + new_resource = self.create_geonode_resource( layer_name=data_to_update.get("title"), alternate=new_alternate, execution_id=str(_exec.exec_id), - files=[], + asset=get_default_asset(resource), ) copy_assets_and_links(resource, target=new_resource) new_resource.refresh_from_db() diff --git a/importer/tests/end2end/test_end2end.py b/importer/tests/end2end/test_end2end.py index a042e270..f9ba8759 100644 --- a/importer/tests/end2end/test_end2end.py +++ b/importer/tests/end2end/test_end2end.py @@ -190,6 +190,7 @@ def test_import_geopackage_with_no_crs_table(self): @mock.patch( "importer.handlers.common.vector.BaseVectorFileHandler._select_valid_layers" ) + @override_settings(MEDIA_ROOT="/tmp/", ASSET_ROOT="/tmp/") def test_import_geopackage_with_no_crs_table_should_raise_error_if_all_layer_are_invalid( self, _select_valid_layers ): @@ -206,7 +207,7 @@ def test_import_geopackage_with_no_crs_table_should_raise_error_if_all_layer_are self.client.force_login(self.admin) response = self.client.post(self.url, data=payload) - self.assertEqual(500, response.status_code) + self.assertEqual(400, response.status_code) self.assertIn( "No valid layers found", diff --git a/importer/tests/unit/test_task.py b/importer/tests/unit/test_task.py index 9b49b483..2bacc12c 100644 --- a/importer/tests/unit/test_task.py +++ b/importer/tests/unit/test_task.py @@ -24,6 +24,7 @@ from geonode.resource.enumerator import ExecutionRequestAction from geonode.base.models import ResourceBase from geonode.base.populate_test_data import create_single_dataset +from geonode.assets.handlers import asset_handler_registry from dynamic_models.models import ModelSchema, FieldSchema from dynamic_models.exceptions import DynamicModelError, InvalidFieldNameError from importer.models import ResourceHandlerInfo @@ -40,7 +41,19 @@ class TestCeleryTasks(ImporterBaseTestSupport): def setUp(self): self.user = get_user_model().objects.first() + self.existing_file = f"{project_dir}/tests/fixture/valid.gpkg" + self.asset_handler = asset_handler_registry.get_default_handler() + + self.asset = self.asset_handler.create( + title="Original", + owner=self.user, + description=None, + type="importer.handlers.gpkg.handler.GPKGFileHandler", + files=[self.existing_file], + clone_files=False, + ) + self.exec_id = orchestrator.create_execution_request( user=get_user_model().objects.get(username=self.user), func_name="dummy_func", @@ -50,6 +63,8 @@ def setUp(self): "files": {"base_file": self.existing_file}, # "overwrite_existing_layer": True, "store_spatial_files": True, + "asset_id": self.asset.id, + "asset_module_path": f"{self.asset.__module__}.{self.asset.__class__.__name__}", }, ) @@ -506,9 +521,6 @@ def test_import_metadata_should_work_as_expected(self): layer.refresh_from_db() self.assertEqual(layer.title, "test_dataset") - # verify that the original has been deleted - self.assertFalse(os.path.exists(xml_in_tmp)) - class TestDynamicModelSchema(TransactionImporterBaseTestSupport): databases = ("default", "datastore") diff --git a/runtest.sh b/runtest.sh index 0150efab..8bbfbf35 100755 --- a/runtest.sh +++ b/runtest.sh @@ -2,4 +2,4 @@ set -a . ./.env_test set +a -coverage run --append --source='.' /usr/src/geonode/manage.py test importer -v2 --noinput +coverage run --append --source='.' /usr/src/geonode/manage.py test importer.tests.end2end.test_end2end.ImporterNoCRSImportTest.test_import_geopackage_with_no_crs_table_should_raise_error_if_all_layer_are_invalid -v2 --noinput From b2de28a9fc51f8db419752d3ed1637f36d6f517c Mon Sep 17 00:00:00 2001 From: Mattia Date: Mon, 27 May 2024 14:59:39 +0200 Subject: [PATCH 13/40] Fix test coverage --- runtest.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtest.sh b/runtest.sh index 8bbfbf35..0150efab 100755 --- a/runtest.sh +++ b/runtest.sh @@ -2,4 +2,4 @@ set -a . ./.env_test set +a -coverage run --append --source='.' /usr/src/geonode/manage.py test importer.tests.end2end.test_end2end.ImporterNoCRSImportTest.test_import_geopackage_with_no_crs_table_should_raise_error_if_all_layer_are_invalid -v2 --noinput +coverage run --append --source='.' /usr/src/geonode/manage.py test importer -v2 --noinput From 647241bc7a3c41fb841416b8658095a011fcacc8 Mon Sep 17 00:00:00 2001 From: Mattia Date: Mon, 27 May 2024 15:07:29 +0200 Subject: [PATCH 14/40] Fix test coverage --- importer/tests/end2end/test_end2end.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/importer/tests/end2end/test_end2end.py b/importer/tests/end2end/test_end2end.py index f9ba8759..dd8de7ab 100644 --- a/importer/tests/end2end/test_end2end.py +++ b/importer/tests/end2end/test_end2end.py @@ -207,7 +207,7 @@ def test_import_geopackage_with_no_crs_table_should_raise_error_if_all_layer_are self.client.force_login(self.admin) response = self.client.post(self.url, data=payload) - self.assertEqual(400, response.status_code) + self.assertEqual(500, response.status_code) self.assertIn( "No valid layers found", From d91657255d258c51ae84fbb6dc58d91e0b962edf Mon Sep 17 00:00:00 2001 From: Mattia Date: Thu, 30 May 2024 16:30:28 +0200 Subject: [PATCH 15/40] First implementation of 3dtiles handler --- importer/api/views.py | 12 ++++++++---- importer/datastore.py | 2 +- importer/handlers/common/vector.py | 25 ++++++++++++++----------- importer/handlers/geojson/handler.py | 13 ++++++++++++- importer/handlers/utils.py | 2 +- importer/orchestrator.py | 13 +++++-------- 6 files changed, 41 insertions(+), 26 deletions(-) diff --git a/importer/api/views.py b/importer/api/views.py index d3f34968..a4679310 100644 --- a/importer/api/views.py +++ b/importer/api/views.py @@ -20,7 +20,7 @@ from urllib.parse import urljoin from django.conf import settings from django.urls import reverse - +from pathlib import Path from geonode.resource.enumerator import ExecutionRequestAction from django.utils.translation import gettext_lazy as _ from dynamic_rest.filters import DynamicFilterBackend, DynamicSortingFilter @@ -111,6 +111,7 @@ def create(self, request, *args, **kwargs): if "zip_file" in _data or "kmz_file" in _data: # if a zipfile is provided, we need to unzip it before searching for an handler + zipname = Path(_data['base_file'].name).stem storage_manager = StorageManager( remote_files={"base_file": _data.get("zip_file", _data.get("kmz_file"))} ) @@ -119,7 +120,10 @@ def create(self, request, *args, **kwargs): cloning_directory=asset_dir, create_tempdir=False ) # update the payload with the unziped paths - _data.update(storage_manager.get_retrieved_paths()) + _data.update({ + **{"original_zip_name": zipname}, + **storage_manager.get_retrieved_paths() + }) handler = orchestrator.get_handler(_data) @@ -174,10 +178,10 @@ def create(self, request, *args, **kwargs): except Exception as e: # in case of any exception, is better to delete the # cloned files to keep the storage under control - if storage_manager is not None: - storage_manager.delete_retrieved_paths(force=True) if asset: asset.delete() + elif storage_manager is not None: + storage_manager.delete_retrieved_paths(force=True) if execution_id: orchestrator.set_as_failed(execution_id=str(execution_id), reason=e) logger.exception(e) diff --git a/importer/datastore.py b/importer/datastore.py index 7a629a81..4c85c9c8 100644 --- a/importer/datastore.py +++ b/importer/datastore.py @@ -12,7 +12,7 @@ def __init__( self, files: list, handler_module_path: str, - user: get_user_model(), + user: get_user_model(), # type: ignore execution_id: str, ) -> None: self.files = files diff --git a/importer/handlers/common/vector.py b/importer/handlers/common/vector.py index e53a9610..4c175c55 100644 --- a/importer/handlers/common/vector.py +++ b/importer/handlers/common/vector.py @@ -593,17 +593,7 @@ def create_geonode_resource( saved_dataset = resource_manager.create( None, resource_type=resource_type, - defaults=dict( - name=alternate, - workspace=workspace, - store=os.environ.get("GEONODE_GEODATABASE", "geonode_data"), - subtype="vector", - alternate=f"{workspace}:{alternate}", - dirty_state=True, - title=layer_name, - owner=_exec.user, - asset=asset, - ), + defaults=self.generate_resource_payload(layer_name, alternate, asset, _exec, workspace), ) saved_dataset.refresh_from_db() @@ -618,6 +608,19 @@ def create_geonode_resource( saved_dataset.refresh_from_db() return saved_dataset + def generate_resource_payload(self, layer_name, alternate, asset, _exec, workspace): + return dict( + name=alternate, + workspace=workspace, + store=os.environ.get("GEONODE_GEODATABASE", "geonode_data"), + subtype="vector", + alternate=f"{workspace}:{alternate}", + dirty_state=True, + title=layer_name, + owner=_exec.user, + asset=asset, + ) + def overwrite_geonode_resource( self, layer_name: str, diff --git a/importer/handlers/geojson/handler.py b/importer/handlers/geojson/handler.py index 9b7ad951..98570180 100644 --- a/importer/handlers/geojson/handler.py +++ b/importer/handlers/geojson/handler.py @@ -58,7 +58,18 @@ def can_handle(_data) -> bool: if not base: return False ext = base.split(".")[-1] if isinstance(base, str) else base.name.split(".")[-1] - return ext in ["json", "geojson"] + if ext in ["json", "geojson"]: + ''' + Check if is a real geojson based on specification + https://datatracker.ietf.org/doc/html/rfc7946#section-1.4 + ''' + _file = base + if isinstance(base, str): + with open(base, 'r') as f: + _file = json.loads(f.read()) + + return _file.get('type', None) in ['FeatureCollection', 'Feature'] + return False @staticmethod def is_valid(files, user): diff --git a/importer/handlers/utils.py b/importer/handlers/utils.py index d83ee6cd..bb274f6d 100644 --- a/importer/handlers/utils.py +++ b/importer/handlers/utils.py @@ -43,7 +43,7 @@ } -def should_be_imported(layer: str, user: get_user_model(), **kwargs) -> bool: +def should_be_imported(layer: str, user: get_user_model(), **kwargs) -> bool: # type: ignore """ If layer_name + user (without the addition of any execution id) already exists, will apply one of the rule available: diff --git a/importer/orchestrator.py b/importer/orchestrator.py index 9e75bf8d..b93c9378 100644 --- a/importer/orchestrator.py +++ b/importer/orchestrator.py @@ -171,14 +171,11 @@ def set_as_failed(self, execution_id, reason=None, delete_file=True): # delete if delete_file: exec_obj = self.get_execution_object(execution_id) - _files = exec_obj.input_params.get("files") - # better to delete each single file since it can be a remote storage service - if _files: - logger.info( - "Execution failed, removing uploaded file to save disk space" - ) - for _file in _files.values(): - storage_manager.delete(_file) + # cleanup asset in case of fail + asset_handler = import_string(exec_obj.input_params['asset_module_path']) + asset = asset_handler.objects.filter(pk=exec_obj.input_params['asset_id']) + if asset.exists(): + asset.first().delete() def set_as_partially_failed(self, execution_id, reason=None): """ From a2c42e61b38b5e71197057d5be8b3ecea50c97cd Mon Sep 17 00:00:00 2001 From: Mattia Date: Thu, 30 May 2024 16:31:22 +0200 Subject: [PATCH 16/40] First implementation of 3dtiles handler --- importer/handlers/tiles3d/__init__.py | 0 importer/handlers/tiles3d/exceptions.py | 9 ++ importer/handlers/tiles3d/handler.py | 190 ++++++++++++++++++++++++ importer/handlers/tiles3d/tests.py | 142 ++++++++++++++++++ 4 files changed, 341 insertions(+) create mode 100644 importer/handlers/tiles3d/__init__.py create mode 100644 importer/handlers/tiles3d/exceptions.py create mode 100644 importer/handlers/tiles3d/handler.py create mode 100644 importer/handlers/tiles3d/tests.py diff --git a/importer/handlers/tiles3d/__init__.py b/importer/handlers/tiles3d/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/importer/handlers/tiles3d/exceptions.py b/importer/handlers/tiles3d/exceptions.py new file mode 100644 index 00000000..f8108c64 --- /dev/null +++ b/importer/handlers/tiles3d/exceptions.py @@ -0,0 +1,9 @@ +from rest_framework.exceptions import APIException +from rest_framework import status + + +class Invalid3DTilesException(APIException): + status_code = status.HTTP_400_BAD_REQUEST + default_detail = "The 3dtiles provided is invalid" + default_code = "invalid_3dtiles" + category = "importer" diff --git a/importer/handlers/tiles3d/handler.py b/importer/handlers/tiles3d/handler.py new file mode 100644 index 00000000..8e82d982 --- /dev/null +++ b/importer/handlers/tiles3d/handler.py @@ -0,0 +1,190 @@ +import json +import logging +import os +from pathlib import Path +from geonode.layers.models import Dataset +from geonode.resource.enumerator import ExecutionRequestAction as exa +from geonode.upload.utils import UploadLimitValidator +from importer.orchestrator import orchestrator +from importer.celery_tasks import import_orchestrator +from importer.handlers.common.vector import BaseVectorFileHandler +from importer.handlers.utils import create_alternate, should_be_imported +from importer.publisher import DataPublisher +from importer.utils import ImporterRequestAction as ira +from geonode.base.models import ResourceBase +from importer.handlers.tiles3d.exceptions import Invalid3DTilesException + +logger = logging.getLogger(__name__) + + +class Tiles3DFileHandler(BaseVectorFileHandler): + """ + Handler to import 3Dtiles files into GeoNode data db + It must provide the task_lists required to comple the upload + """ + + ACTIONS = { + exa.IMPORT.value: ( + "start_import", + "importer.import_resource", + "importer.create_geonode_resource", + ), + exa.COPY.value: ( + "start_copy", + "importer.copy_geonode_resource", + ), + ira.ROLLBACK.value: ( + "start_rollback", + "importer.rollback", + ), + } + + @property + def supported_file_extension_config(self): + return { + "id": "3dtiles", + "label": "3D Tiles", + "format": "vector", + "ext": ["json"], + "optional": ["xml", "sld"], + } + + @staticmethod + def can_handle(_data) -> bool: + """ + This endpoint will return True or False if with the info provided + the handler is able to handle the file or not + """ + base = _data.get("base_file") + if not base: + return False + ext = base.split(".")[-1] if isinstance(base, str) else base.name.split(".")[-1] + input_filename = os.path.basename(base if isinstance(base, str) else base.name) + if ext in ["json"] and "tileset.json" in input_filename: + return True + return False + + @staticmethod + def is_valid(files, user): + """ + Define basic validation steps: + """ + # calling base validation checks + BaseVectorFileHandler.is_valid(files, user) + # getting the upload limit validation + upload_validator = UploadLimitValidator(user) + upload_validator.validate_parallelism_limit_per_user() + + _file = files.get("base_file") + if not _file: + raise Invalid3DTilesException("base file is not provided") + + filename = os.path.basename(_file) + + if len(filename.split(".")) > 2: + # means that there is a dot other than the one needed for the extension + # if we keep it ogr2ogr raise an error, better to remove it + raise Invalid3DTilesException( + "Please remove the additional dots in the filename" + ) + + try: + with open(_file, "r") as _readed_file: + json.loads(_readed_file.read()) + except Exception: + raise Invalid3DTilesException("The provided GeoJson is not valid") + + return True + + @staticmethod + def extract_params_from_data(_data, action=None): + """ + Remove from the _data the params that needs to save into the executionRequest object + all the other are returned + """ + if action == exa.COPY.value: + title = json.loads(_data.get("defaults")) + return {"title": title.pop("title")}, _data + + return { + "skip_existing_layers": _data.pop("skip_existing_layers", "False"), + "overwrite_existing_layer": _data.pop("overwrite_existing_layer", "False"), + "store_spatial_file": _data.pop("store_spatial_files", "True"), + "source": _data.pop("source", "upload"), + "original_zip_name": _data.pop("original_zip_name", None), + }, _data + + def import_resource(self, files: dict, execution_id: str, **kwargs) -> str: + logger.info("Total number of layers available: 1") + + _exec = self._get_execution_request_object(execution_id) + + _input = {**_exec.input_params, **{"total_layers": 1}} + + orchestrator.update_execution_request_status( + execution_id=str(execution_id), input_params=_input + ) + filename = ( + _exec.input_params.get("original_zip_name") + or Path(files.get("base_file")).ste + ) + # start looping on the layers available + layer_name = self.fixup_name(filename) + should_be_overwritten = _exec.input_params.get("overwrite_existing_layer") + # should_be_imported check if the user+layername already exists or not + if should_be_imported( + layer_name, + _exec.user, + skip_existing_layer=_exec.input_params.get("skip_existing_layer"), + overwrite_existing_layer=should_be_overwritten, + ): + + user_datasets = ResourceBase.objects.filter( + owner=_exec.user, alternate=layer_name + ) + + dataset_exists = user_datasets.exists() + + if dataset_exists and should_be_overwritten: + layer_name, alternate = ( + layer_name, + user_datasets.first().alternate.split(":")[-1], + ) + elif not dataset_exists: + alternate = layer_name + else: + alternate = create_alternate(layer_name, execution_id) + + import_orchestrator.apply_async( + ( + files, + execution_id, + str(self), + "importer.import_resource", + layer_name, + alternate, + exa.IMPORT.value, + ) + ) + return layer_name, alternate, execution_id + + def create_geonode_resource( + self, + layer_name: str, + alternate: str, + execution_id: str, + resource_type: Dataset = ..., + asset=None, + ): + return super().create_geonode_resource( + layer_name, alternate, execution_id, ResourceBase, asset + ) + + def generate_resource_payload(self, layer_name, alternate, asset, _exec, workspace): + return dict( + subtype="3dtiles", + dirty_state=True, + title=layer_name, + owner=_exec.user, + asset=asset, + ) diff --git a/importer/handlers/tiles3d/tests.py b/importer/handlers/tiles3d/tests.py new file mode 100644 index 00000000..cd43c576 --- /dev/null +++ b/importer/handlers/tiles3d/tests.py @@ -0,0 +1,142 @@ +import uuid +import os +from django.test import TestCase +from mock import MagicMock, patch +from importer.handlers.common.vector import import_with_ogr2ogr +from importer.handlers.geojson.exceptions import InvalidGeoJsonException +from importer.handlers.geojson.handler import GeoJsonFileHandler +from django.contrib.auth import get_user_model +from importer import project_dir +from geonode.upload.models import UploadParallelismLimit +from geonode.upload.api.exceptions import UploadParallelismLimitException +from geonode.base.populate_test_data import create_single_dataset +from osgeo import ogr + + +class TestGeoJsonFileHandler(TestCase): + databases = ("default", "datastore") + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.handler = GeoJsonFileHandler() + cls.valid_geojson = f"{project_dir}/tests/fixture/valid.geojson" + cls.invalid_geojson = f"{project_dir}/tests/fixture/invalid.geojson" + cls.user, _ = get_user_model().objects.get_or_create(username="admin") + cls.invalid_files = {"base_file": cls.invalid_geojson} + cls.valid_files = {"base_file": cls.valid_geojson} + cls.owner = get_user_model().objects.first() + cls.layer = create_single_dataset( + name="stazioni_metropolitana", owner=cls.owner + ) + + def test_task_list_is_the_expected_one(self): + expected = ( + "start_import", + "importer.import_resource", + "importer.publish_resource", + "importer.create_geonode_resource", + ) + self.assertEqual(len(self.handler.ACTIONS["import"]), 4) + self.assertTupleEqual(expected, self.handler.ACTIONS["import"]) + + def test_task_list_is_the_expected_one_copy(self): + expected = ( + "start_copy", + "importer.copy_dynamic_model", + "importer.copy_geonode_data_table", + "importer.publish_resource", + "importer.copy_geonode_resource", + ) + self.assertEqual(len(self.handler.ACTIONS["copy"]), 5) + self.assertTupleEqual(expected, self.handler.ACTIONS["copy"]) + + def test_is_valid_should_raise_exception_if_the_parallelism_is_met(self): + parallelism, created = UploadParallelismLimit.objects.get_or_create( + slug="default_max_parallel_uploads" + ) + old_value = parallelism.max_number + try: + UploadParallelismLimit.objects.filter( + slug="default_max_parallel_uploads" + ).update(max_number=0) + + with self.assertRaises(UploadParallelismLimitException): + self.handler.is_valid(files=self.valid_files, user=self.user) + + finally: + parallelism.max_number = old_value + parallelism.save() + + def test_is_valid_should_pass_with_valid_geojson(self): + self.handler.is_valid(files=self.valid_files, user=self.user) + + def test_is_valid_should_raise_exception_if_the_geojson_is_invalid(self): + data = { + "base_file": "/using/double/dot/in/the/name/is/an/error/file.invalid.geojson" + } + with self.assertRaises(InvalidGeoJsonException) as _exc: + self.handler.is_valid(files=data, user=self.user) + + self.assertIsNotNone(_exc) + self.assertTrue( + "Please remove the additional dots in the filename" + in str(_exc.exception.detail) + ) + + def test_is_valid_should_raise_exception_if_the_geojson_is_invalid_format(self): + with self.assertRaises(InvalidGeoJsonException) as _exc: + self.handler.is_valid(files=self.invalid_files, user=self.user) + + self.assertIsNotNone(_exc) + self.assertTrue( + "The provided GeoJson is not valid" in str(_exc.exception.detail) + ) + + def test_get_ogr2ogr_driver_should_return_the_expected_driver(self): + expected = ogr.GetDriverByName("GEOJSON") + actual = self.handler.get_ogr2ogr_driver() + self.assertEqual(type(expected), type(actual)) + + def test_can_handle_should_return_true_for_geojson(self): + actual = self.handler.can_handle(self.valid_files) + self.assertTrue(actual) + + def test_can_handle_should_return_false_for_other_files(self): + actual = self.handler.can_handle({"base_file": "random.gpkg"}) + self.assertFalse(actual) + + @patch("importer.handlers.common.vector.Popen") + def test_import_with_ogr2ogr_without_errors_should_call_the_right_command( + self, _open + ): + _uuid = uuid.uuid4() + + comm = MagicMock() + comm.communicate.return_value = b"", b"" + _open.return_value = comm + + _task, alternate, execution_id = import_with_ogr2ogr( + execution_id=str(_uuid), + files=self.valid_files, + original_name="dataset", + handler_module_path=str(self.handler), + ovverwrite_layer=False, + alternate="alternate", + ) + + self.assertEqual("ogr2ogr", _task) + self.assertEqual(alternate, "alternate") + self.assertEqual(str(_uuid), execution_id) + + _open.assert_called_once() + _open.assert_called_with( + "/usr/bin/ogr2ogr --config PG_USE_COPY YES -f PostgreSQL PG:\" dbname='test_geonode_data' host=" + + os.getenv("DATABASE_HOST", "localhost") + + " port=5432 user='geonode_data' password='geonode_data' \" \"" + + self.valid_files.get("base_file") + + '" -nln alternate "dataset" -lco GEOMETRY_NAME=geometry', + stdout=-1, + stderr=-1, + shell=True, # noqa + ) From a61aac7e37b08252085a641da651c890dc848af9 Mon Sep 17 00:00:00 2001 From: Mattia Date: Thu, 30 May 2024 18:15:22 +0200 Subject: [PATCH 17/40] First implementation of 3dtiles handler --- importer/handlers/tiles3d/handler.py | 8 +++++++- importer/handlers/tiles3d/tests.py | 6 +++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/importer/handlers/tiles3d/handler.py b/importer/handlers/tiles3d/handler.py index 8e82d982..44eb0309 100644 --- a/importer/handlers/tiles3d/handler.py +++ b/importer/handlers/tiles3d/handler.py @@ -2,6 +2,7 @@ import logging import os from pathlib import Path +from geonode.assets.utils import get_default_asset from geonode.layers.models import Dataset from geonode.resource.enumerator import ExecutionRequestAction as exa from geonode.upload.utils import UploadLimitValidator @@ -176,9 +177,14 @@ def create_geonode_resource( resource_type: Dataset = ..., asset=None, ): - return super().create_geonode_resource( + resource = super().create_geonode_resource( layer_name, alternate, execution_id, ResourceBase, asset ) + # we want just the tileset.json as location of the asset + asset = get_default_asset(resource) + asset.location = [path for path in asset.location if "tileset.json" in path] + asset.save() + return resource def generate_resource_payload(self, layer_name, alternate, asset, _exec, workspace): return dict( diff --git a/importer/handlers/tiles3d/tests.py b/importer/handlers/tiles3d/tests.py index cd43c576..52790ddc 100644 --- a/importer/handlers/tiles3d/tests.py +++ b/importer/handlers/tiles3d/tests.py @@ -4,7 +4,7 @@ from mock import MagicMock, patch from importer.handlers.common.vector import import_with_ogr2ogr from importer.handlers.geojson.exceptions import InvalidGeoJsonException -from importer.handlers.geojson.handler import GeoJsonFileHandler +from importer.handlers.tiles3d.handler import Tiles3DFileHandler from django.contrib.auth import get_user_model from importer import project_dir from geonode.upload.models import UploadParallelismLimit @@ -13,13 +13,13 @@ from osgeo import ogr -class TestGeoJsonFileHandler(TestCase): +class TestTiles3DFileHandler(TestCase): databases = ("default", "datastore") @classmethod def setUpClass(cls): super().setUpClass() - cls.handler = GeoJsonFileHandler() + cls.handler = Tiles3DFileHandler() cls.valid_geojson = f"{project_dir}/tests/fixture/valid.geojson" cls.invalid_geojson = f"{project_dir}/tests/fixture/invalid.geojson" cls.user, _ = get_user_model().objects.get_or_create(username="admin") From b6180fc8bdfb296f155a2d2f59b83f8a58ce26aa Mon Sep 17 00:00:00 2001 From: Mattia Date: Thu, 30 May 2024 18:28:10 +0200 Subject: [PATCH 18/40] First implementation of 3dtiles handler --- importer/handlers/tiles3d/tests.py | 68 +++++------------- importer/tests/fixture/3dtilesample/README.md | 2 + .../tests/fixture/3dtilesample/invalid.zip | 0 .../fixture/3dtilesample/valid_3dtiles.zip | Bin 0 -> 1333 bytes 4 files changed, 18 insertions(+), 52 deletions(-) create mode 100644 importer/tests/fixture/3dtilesample/README.md create mode 100644 importer/tests/fixture/3dtilesample/invalid.zip create mode 100644 importer/tests/fixture/3dtilesample/valid_3dtiles.zip diff --git a/importer/handlers/tiles3d/tests.py b/importer/handlers/tiles3d/tests.py index 52790ddc..8e600632 100644 --- a/importer/handlers/tiles3d/tests.py +++ b/importer/handlers/tiles3d/tests.py @@ -3,7 +3,7 @@ from django.test import TestCase from mock import MagicMock, patch from importer.handlers.common.vector import import_with_ogr2ogr -from importer.handlers.geojson.exceptions import InvalidGeoJsonException +from importer.handlers.3dtiles.exceptions import Invalid3dtilesException from importer.handlers.tiles3d.handler import Tiles3DFileHandler from django.contrib.auth import get_user_model from importer import project_dir @@ -20,24 +20,23 @@ class TestTiles3DFileHandler(TestCase): def setUpClass(cls): super().setUpClass() cls.handler = Tiles3DFileHandler() - cls.valid_geojson = f"{project_dir}/tests/fixture/valid.geojson" - cls.invalid_geojson = f"{project_dir}/tests/fixture/invalid.geojson" + cls.valid_3dtile = f"{project_dir}/tests/fixture/3dtilesample/valid_3dtiles.zip" + cls.invalid_3dtile = f"{project_dir}/tests/fixture/3dtilesample/invalid.zip" cls.user, _ = get_user_model().objects.get_or_create(username="admin") - cls.invalid_files = {"base_file": cls.invalid_geojson} - cls.valid_files = {"base_file": cls.valid_geojson} + cls.invalid_files = {"base_file": cls.invalid_3dtile} + cls.valid_files = {"base_file": cls.valid_3dtile} cls.owner = get_user_model().objects.first() cls.layer = create_single_dataset( - name="stazioni_metropolitana", owner=cls.owner + name="urban_forestry_street_tree_benefits_epsg_26985", owner=cls.owner ) def test_task_list_is_the_expected_one(self): expected = ( "start_import", "importer.import_resource", - "importer.publish_resource", "importer.create_geonode_resource", ) - self.assertEqual(len(self.handler.ACTIONS["import"]), 4) + self.assertEqual(len(self.handler.ACTIONS["import"]), 3) self.assertTupleEqual(expected, self.handler.ACTIONS["import"]) def test_task_list_is_the_expected_one_copy(self): @@ -68,14 +67,14 @@ def test_is_valid_should_raise_exception_if_the_parallelism_is_met(self): parallelism.max_number = old_value parallelism.save() - def test_is_valid_should_pass_with_valid_geojson(self): + def test_is_valid_should_pass_with_valid_3dtiles(self): self.handler.is_valid(files=self.valid_files, user=self.user) - def test_is_valid_should_raise_exception_if_the_geojson_is_invalid(self): + def test_is_valid_should_raise_exception_if_the_3dtiles_is_invalid(self): data = { - "base_file": "/using/double/dot/in/the/name/is/an/error/file.invalid.geojson" + "base_file": "/using/double/dot/in/the/name/is/an/error/file.invalid.json" } - with self.assertRaises(InvalidGeoJsonException) as _exc: + with self.assertRaises(Invalid3dtilesException) as _exc: self.handler.is_valid(files=data, user=self.user) self.assertIsNotNone(_exc) @@ -84,59 +83,24 @@ def test_is_valid_should_raise_exception_if_the_geojson_is_invalid(self): in str(_exc.exception.detail) ) - def test_is_valid_should_raise_exception_if_the_geojson_is_invalid_format(self): - with self.assertRaises(InvalidGeoJsonException) as _exc: + def test_is_valid_should_raise_exception_if_the_3dtiles_is_invalid_format(self): + with self.assertRaises(Invalid3dtilesException) as _exc: self.handler.is_valid(files=self.invalid_files, user=self.user) self.assertIsNotNone(_exc) self.assertTrue( - "The provided GeoJson is not valid" in str(_exc.exception.detail) + "The provided 3dtiles is not valid" in str(_exc.exception.detail) ) def test_get_ogr2ogr_driver_should_return_the_expected_driver(self): - expected = ogr.GetDriverByName("GEOJSON") + expected = ogr.GetDriverByName("3dtiles") actual = self.handler.get_ogr2ogr_driver() self.assertEqual(type(expected), type(actual)) - def test_can_handle_should_return_true_for_geojson(self): + def test_can_handle_should_return_true_for_3dtiles(self): actual = self.handler.can_handle(self.valid_files) self.assertTrue(actual) def test_can_handle_should_return_false_for_other_files(self): actual = self.handler.can_handle({"base_file": "random.gpkg"}) self.assertFalse(actual) - - @patch("importer.handlers.common.vector.Popen") - def test_import_with_ogr2ogr_without_errors_should_call_the_right_command( - self, _open - ): - _uuid = uuid.uuid4() - - comm = MagicMock() - comm.communicate.return_value = b"", b"" - _open.return_value = comm - - _task, alternate, execution_id = import_with_ogr2ogr( - execution_id=str(_uuid), - files=self.valid_files, - original_name="dataset", - handler_module_path=str(self.handler), - ovverwrite_layer=False, - alternate="alternate", - ) - - self.assertEqual("ogr2ogr", _task) - self.assertEqual(alternate, "alternate") - self.assertEqual(str(_uuid), execution_id) - - _open.assert_called_once() - _open.assert_called_with( - "/usr/bin/ogr2ogr --config PG_USE_COPY YES -f PostgreSQL PG:\" dbname='test_geonode_data' host=" - + os.getenv("DATABASE_HOST", "localhost") - + " port=5432 user='geonode_data' password='geonode_data' \" \"" - + self.valid_files.get("base_file") - + '" -nln alternate "dataset" -lco GEOMETRY_NAME=geometry', - stdout=-1, - stderr=-1, - shell=True, # noqa - ) diff --git a/importer/tests/fixture/3dtilesample/README.md b/importer/tests/fixture/3dtilesample/README.md new file mode 100644 index 00000000..cd862833 --- /dev/null +++ b/importer/tests/fixture/3dtilesample/README.md @@ -0,0 +1,2 @@ +Example provided by CensiumGS +https://github.com/CesiumGS/3d-tiles-samples \ No newline at end of file diff --git a/importer/tests/fixture/3dtilesample/invalid.zip b/importer/tests/fixture/3dtilesample/invalid.zip new file mode 100644 index 00000000..e69de29b diff --git a/importer/tests/fixture/3dtilesample/valid_3dtiles.zip b/importer/tests/fixture/3dtilesample/valid_3dtiles.zip new file mode 100644 index 0000000000000000000000000000000000000000..93955274f1e22d02cddd40aa8bbc2df9a27403ec GIT binary patch literal 1333 zcmWIWW@Zs#U|`^2SUYK7#EdHzrRhuz3=-@N4Ezi-36|UTN~eh;(R0rr}y?fB3)&wVGi3IzAt@r zkkhX~ROpGsEm5ai_1E@an15xXVZyqym06jK^54w-`7G$risGuv20i8MZHf2HdNyyn zJxAfqZxvtRX^Sph=lta6=x~Eo^?u+Qm2VuW5gj^zo(NL&oKkQktBEgxe|)DO`x;xO{>|M>6Ht9RABe zdg4L%#Q8YS}>O; zPikN6$v&sGtzy>6l)8iG9|y0^>`?xvY~FKo=hBiNOC`!3-)Yn=_)@LF@9<9hPr%E5 zhj|S)VLw)0n%}sGp>o{^@1^^hDw#h;J>2dUZyPQj{&Cm7q~L2Zr*B0^e%8(J-+0z- z`|XbX%KJs{ZJqP=$LnuZe9{%icmCR`elP61SWWEi`m@RNW4=BA!5aWiuZMu?wed>w z;?=;^8^p-KzynOLC7C&?#i=EFS;hHz;KaHHm{36&EwSeI8FC*o5NQ4WudAR~TIk?W z-zBq~Ep~K>L??4@vD~ptY0AIy$y+9B`964XuX@k#ww9gj-`53;-RR>@EI;KPxcp)T zugY|RFNSj;Z9Nngc`4Md<%#j6?wiU-?k^Ln{(NECzJ$x$vocvfzn!WrTr=D6aQM+s z9rtZ&yG(YUtu?-r%J&&|KooZ+t-IL+<5AS{h4_s9o>ii`iO11@nM&y zqMY9&naTVJ4?hEXSoNj4qbtzEUO>zV^l*@?ql>SrUTzB5w;zB$1z|Mb7O(XXyqCx`b9te2b2%<3x4pvCPf#xz~LowqMVkj_b8QwN- y0G3Lqu?sX2OUxphh#6oA6PGkD1eRP#!3Q)BOV9;)v$BEw!wiJqfV3tHhz9^W6%+#i literal 0 HcmV?d00001 From b12ddfaa98d9b7f29d39d998433dde28e71e40a5 Mon Sep 17 00:00:00 2001 From: Mattia Date: Wed, 5 Jun 2024 12:19:28 +0200 Subject: [PATCH 19/40] Add test coverage for 3dtiles --- .env_test | 2 +- Dockerfile | 2 +- importer/api/tests.py | 10 +-- importer/api/views.py | 5 +- importer/handlers/geojson/handler.py | 18 ++-- importer/handlers/gpkg/tests.py | 18 +++- importer/handlers/tiles3d/handler.py | 35 +++++++- importer/handlers/tiles3d/tests.py | 87 ++++++++++++++++--- importer/orchestrator.py | 1 + importer/tests/end2end/test_end2end.py | 3 +- .../fixture/3dtilesample/invalid_tileset.json | 3 + .../tests/fixture/3dtilesample/tileset.json | 16 ++++ importer/tests/unit/test_orchestrator.py | 41 ++++++++- runtest.sh | 1 + 14 files changed, 208 insertions(+), 34 deletions(-) create mode 100755 importer/tests/fixture/3dtilesample/invalid_tileset.json create mode 100755 importer/tests/fixture/3dtilesample/tileset.json diff --git a/.env_test b/.env_test index f751770a..6c572785 100644 --- a/.env_test +++ b/.env_test @@ -180,7 +180,7 @@ SECRET_KEY='myv-y4#7j-d*p-__@j#*3z@!y24fz8%^z2v6atuy4bo9vqr1_a' STATIC_ROOT=/tmp/statics/static/ MEDIA_ROOT=/tmp/statics/uploaded/ -ASSET_ROOT=/tmp/statics/assets/ +ASSETS_ROOT=/tmp/statics/assets/ GEOIP_PATH=/mnt/volumes/statics/geoip.db CACHE_BUSTING_STATIC_ENABLED=False diff --git a/Dockerfile b/Dockerfile index 083ebab5..03e9cb5e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,7 @@ FROM geonode/geonode-base:latest-ubuntu-22.04 RUN rm -rf /usr/src/geonode RUN git clone https://github.com/GeoNode/geonode.git /usr/src/geonode -RUN cd /usr/src/geonode && git fetch --all && git checkout 12124_assets_20240523 && cd - +RUN cd /usr/src/geonode && git fetch --all && git checkout 12226_directory_assets_3 && cd - RUN mkdir -p /usr/src/importer RUN cd .. diff --git a/importer/api/tests.py b/importer/api/tests.py index 7c54b2ba..e4ff7dc9 100644 --- a/importer/api/tests.py +++ b/importer/api/tests.py @@ -53,7 +53,7 @@ def test_raise_exception_if_file_is_not_a_handled(self): def test_gpkg_raise_error_with_invalid_payload(self): self.client.force_login(get_user_model().objects.get(username="admin")) payload = { - "base_file": SimpleUploadedFile(name="test.gpkg", content=b"some-content"), + "base_file": SimpleUploadedFile(name="test.gpkg", content=b'{"type": "FeatureCollection", "content": "some-content"}'), "store_spatial_files": "invalid", } expected = { @@ -73,7 +73,7 @@ def test_gpkg_task_is_called(self, patch_upload): self.client.force_login(get_user_model().objects.get(username="admin")) payload = { - "base_file": SimpleUploadedFile(name="test.gpkg", content=b"some-content"), + "base_file": SimpleUploadedFile(name="test.gpkg", content=b'{"type": "FeatureCollection", "content": "some-content"}'), "store_spatial_files": True, } @@ -88,7 +88,7 @@ def test_geojson_task_is_called(self, patch_upload): self.client.force_login(get_user_model().objects.get(username="admin")) payload = { "base_file": SimpleUploadedFile( - name="test.geojson", content=b"some-content" + name="test.geojson", content=b'{"type": "FeatureCollection", "content": "some-content"}' ), "store_spatial_files": True, } @@ -164,7 +164,7 @@ def test_asset_is_created_before_the_import_start(self, patch_upload): self.client.force_login(get_user_model().objects.get(username="admin")) payload = { "base_file": SimpleUploadedFile( - name="test.geojson", content=b"some-content" + name="test.geojson", content=b'{"type": "FeatureCollection", "content": "some-content"}' ), "store_spatial_files": True, } @@ -195,7 +195,7 @@ def test_asset_should_be_deleted_if_created_during_with_exception( self.client.force_login(get_user_model().objects.get(username="admin")) payload = { "base_file": SimpleUploadedFile( - name="test.geojson", content=b"some-content" + name="test.geojson", content=b'{"type": "FeatureCollection", "content": "some-content"}' ), "store_spatial_files": True, } diff --git a/importer/api/views.py b/importer/api/views.py index a4679310..51ee6409 100644 --- a/importer/api/views.py +++ b/importer/api/views.py @@ -179,7 +179,10 @@ def create(self, request, *args, **kwargs): # in case of any exception, is better to delete the # cloned files to keep the storage under control if asset: - asset.delete() + try: + asset.delete() + except Exception as _exc: + logger.warning(_exc) elif storage_manager is not None: storage_manager.delete_retrieved_paths(force=True) if execution_id: diff --git a/importer/handlers/geojson/handler.py b/importer/handlers/geojson/handler.py index 98570180..f7535a48 100644 --- a/importer/handlers/geojson/handler.py +++ b/importer/handlers/geojson/handler.py @@ -63,12 +63,18 @@ def can_handle(_data) -> bool: Check if is a real geojson based on specification https://datatracker.ietf.org/doc/html/rfc7946#section-1.4 ''' - _file = base - if isinstance(base, str): - with open(base, 'r') as f: - _file = json.loads(f.read()) - - return _file.get('type', None) in ['FeatureCollection', 'Feature'] + try: + _file = base + if isinstance(base, str): + with open(base, 'r') as f: + _file = json.loads(f.read()) + else: + _file = json.loads(base.read()) + + return _file.get('type', None) in ['FeatureCollection', 'Feature'] + + except Exception: + return False return False @staticmethod diff --git a/importer/handlers/gpkg/tests.py b/importer/handlers/gpkg/tests.py index 0eb3b844..bca621f0 100644 --- a/importer/handlers/gpkg/tests.py +++ b/importer/handlers/gpkg/tests.py @@ -11,6 +11,7 @@ from geonode.base.populate_test_data import create_single_dataset from osgeo import ogr from django_celery_results.models import TaskResult +from geonode.assets.handlers import asset_handler_registry from importer.handlers.gpkg.tasks import SingleMessageErrorHandler @@ -117,6 +118,19 @@ def test_single_message_error_handler(self): # lets copy the file to the temporary folder # later will be removed shutil.copy(self.valid_gpkg, "/tmp") + + user = get_user_model().objects.first() + asset_handler = asset_handler_registry.get_default_handler() + + asset = asset_handler.create( + title="Original", + owner=user, + description=None, + type="importer.handlers.gpkg.handler.GPKGFileHandler", + files=["/tmp/valid.gpkg"], + clone_files=False, + ) + exec_id = orchestrator.create_execution_request( user=get_user_model().objects.first(), func_name="funct1", @@ -124,7 +138,10 @@ def test_single_message_error_handler(self): input_params={ "files": {"base_file": "/tmp/valid.gpkg"}, "skip_existing_layer": True, + "store_spatial_file": False, "handler_module_path": str(self.handler), + "asset_id": asset.id, + "asset_module_path": f"{asset.__module__}.{asset.__class__.__name__}", }, ) @@ -145,4 +162,3 @@ def test_single_message_error_handler(self): ) self.assertEqual("FAILURE", TaskResult.objects.get(task_id=str(exec_id)).status) - self.assertFalse(os.path.exists("/tmp/valid.gpkg")) diff --git a/importer/handlers/tiles3d/handler.py b/importer/handlers/tiles3d/handler.py index 44eb0309..6729358c 100644 --- a/importer/handlers/tiles3d/handler.py +++ b/importer/handlers/tiles3d/handler.py @@ -91,9 +91,38 @@ def is_valid(files, user): try: with open(_file, "r") as _readed_file: - json.loads(_readed_file.read()) - except Exception: - raise Invalid3DTilesException("The provided GeoJson is not valid") + _file = json.loads(_readed_file.read()) + # required key described in the specification of 3dtiles + # https://docs.ogc.org/cs/22-025r4/22-025r4.html#toc92 + is_valid = all( + key in _file.keys() for key in ("asset", "geometricError", "root") + ) + + if not is_valid: + raise Invalid3DTilesException( + "The provided 3DTiles is not valid, some of the mandatory keys are missing. Mandatory keys are: 'asset', 'geometricError', 'root'" + ) + + # if the keys are there, let's check if the mandatory child are there too + asset = _file.get("asset", {}).get("version", None) + if not asset: + raise Invalid3DTilesException( + "The mandatory 'version' for the key 'asset' is missing" + ) + volume = _file.get("root", {}).get("boundingVolume", None) + if not volume: + raise Invalid3DTilesException( + "The mandatory 'boundingVolume' for the key 'root' is missing" + ) + + error = _file.get("root", {}).get("geometricError", None) + if error is None: + raise Invalid3DTilesException( + "The mandatory 'geometricError' for the key 'root' is missing" + ) + + except Exception as e: + raise Invalid3DTilesException(e) return True diff --git a/importer/handlers/tiles3d/tests.py b/importer/handlers/tiles3d/tests.py index 8e600632..aad167ad 100644 --- a/importer/handlers/tiles3d/tests.py +++ b/importer/handlers/tiles3d/tests.py @@ -1,9 +1,7 @@ -import uuid +import json import os from django.test import TestCase -from mock import MagicMock, patch -from importer.handlers.common.vector import import_with_ogr2ogr -from importer.handlers.3dtiles.exceptions import Invalid3dtilesException +from importer.handlers.tiles3d.exceptions import Invalid3DTilesException from importer.handlers.tiles3d.handler import Tiles3DFileHandler from django.contrib.auth import get_user_model from importer import project_dir @@ -21,6 +19,10 @@ def setUpClass(cls): super().setUpClass() cls.handler = Tiles3DFileHandler() cls.valid_3dtile = f"{project_dir}/tests/fixture/3dtilesample/valid_3dtiles.zip" + cls.valid_tileset = f"{project_dir}/tests/fixture/3dtilesample/tileset.json" + cls.invalid_tileset = ( + f"{project_dir}/tests/fixture/3dtilesample/invalid_tileset.json" + ) cls.invalid_3dtile = f"{project_dir}/tests/fixture/3dtilesample/invalid.zip" cls.user, _ = get_user_model().objects.get_or_create(username="admin") cls.invalid_files = {"base_file": cls.invalid_3dtile} @@ -42,12 +44,9 @@ def test_task_list_is_the_expected_one(self): def test_task_list_is_the_expected_one_copy(self): expected = ( "start_copy", - "importer.copy_dynamic_model", - "importer.copy_geonode_data_table", - "importer.publish_resource", "importer.copy_geonode_resource", ) - self.assertEqual(len(self.handler.ACTIONS["copy"]), 5) + self.assertEqual(len(self.handler.ACTIONS["copy"]), 2) self.assertTupleEqual(expected, self.handler.ACTIONS["copy"]) def test_is_valid_should_raise_exception_if_the_parallelism_is_met(self): @@ -68,13 +67,13 @@ def test_is_valid_should_raise_exception_if_the_parallelism_is_met(self): parallelism.save() def test_is_valid_should_pass_with_valid_3dtiles(self): - self.handler.is_valid(files=self.valid_files, user=self.user) + self.handler.is_valid(files={"base_file": self.valid_tileset}, user=self.user) def test_is_valid_should_raise_exception_if_the_3dtiles_is_invalid(self): data = { "base_file": "/using/double/dot/in/the/name/is/an/error/file.invalid.json" } - with self.assertRaises(Invalid3dtilesException) as _exc: + with self.assertRaises(Invalid3DTilesException) as _exc: self.handler.is_valid(files=data, user=self.user) self.assertIsNotNone(_exc) @@ -84,13 +83,73 @@ def test_is_valid_should_raise_exception_if_the_3dtiles_is_invalid(self): ) def test_is_valid_should_raise_exception_if_the_3dtiles_is_invalid_format(self): - with self.assertRaises(Invalid3dtilesException) as _exc: - self.handler.is_valid(files=self.invalid_files, user=self.user) + with self.assertRaises(Invalid3DTilesException) as _exc: + self.handler.is_valid( + files={"base_file": self.invalid_tileset}, user=self.user + ) + + self.assertIsNotNone(_exc) + self.assertTrue( + "The provided 3DTiles is not valid, some of the mandatory keys are missing. Mandatory keys are: 'asset', 'geometricError', 'root'" + in str(_exc.exception.detail) + ) + + def test_validate_should_raise_exception_for_invalid_asset_key(self): + _json = { + "asset": {"invalid_key": ""}, + "geometricError": 1.0, + "root": {"boundingVolume": {"box": []}, "geometricError": 0.0}, + } + _path = "/tmp/tileset.json" + with open(_path, "w") as _f: + _f.write(json.dumps(_json)) + with self.assertRaises(Invalid3DTilesException) as _exc: + self.handler.is_valid(files={"base_file": _path}, user=self.user) self.assertIsNotNone(_exc) self.assertTrue( - "The provided 3dtiles is not valid" in str(_exc.exception.detail) + "The mandatory 'version' for the key 'asset' is missing" + in str(_exc.exception.detail) + ) + os.remove(_path) + + def test_validate_should_raise_exception_for_invalid_root_boundingVolume(self): + _json = { + "asset": {"version": "1.1"}, + "geometricError": 1.0, + "root": {"foo": {"box": []}, "geometricError": 0.0}, + } + _path = "/tmp/tileset.json" + with open(_path, "w") as _f: + _f.write(json.dumps(_json)) + with self.assertRaises(Invalid3DTilesException) as _exc: + self.handler.is_valid(files={"base_file": _path}, user=self.user) + + self.assertIsNotNone(_exc) + self.assertTrue( + "The mandatory 'boundingVolume' for the key 'root' is missing" + in str(_exc.exception.detail) + ) + os.remove(_path) + + def test_validate_should_raise_exception_for_invalid_root_geometricError(self): + _json = { + "asset": {"version": "1.1"}, + "geometricError": 1.0, + "root": {"boundingVolume": {"box": []}, "foo": 0.0}, + } + _path = "/tmp/tileset.json" + with open(_path, "w") as _f: + _f.write(json.dumps(_json)) + with self.assertRaises(Invalid3DTilesException) as _exc: + self.handler.is_valid(files={"base_file": _path}, user=self.user) + + self.assertIsNotNone(_exc) + self.assertTrue( + "The mandatory 'geometricError' for the key 'root' is missing" + in str(_exc.exception.detail) ) + os.remove(_path) def test_get_ogr2ogr_driver_should_return_the_expected_driver(self): expected = ogr.GetDriverByName("3dtiles") @@ -98,7 +157,7 @@ def test_get_ogr2ogr_driver_should_return_the_expected_driver(self): self.assertEqual(type(expected), type(actual)) def test_can_handle_should_return_true_for_3dtiles(self): - actual = self.handler.can_handle(self.valid_files) + actual = self.handler.can_handle({"base_file": self.valid_tileset}) self.assertTrue(actual) def test_can_handle_should_return_false_for_other_files(self): diff --git a/importer/orchestrator.py b/importer/orchestrator.py index b93c9378..054bfa99 100644 --- a/importer/orchestrator.py +++ b/importer/orchestrator.py @@ -298,6 +298,7 @@ def create_execution_request( action=None, name=None, source=None, + asset_module_path=None, ) -> UUID: """ Create an execution request for the user. Return the UUID of the request diff --git a/importer/tests/end2end/test_end2end.py b/importer/tests/end2end/test_end2end.py index dd8de7ab..c4deab1f 100644 --- a/importer/tests/end2end/test_end2end.py +++ b/importer/tests/end2end/test_end2end.py @@ -190,7 +190,7 @@ def test_import_geopackage_with_no_crs_table(self): @mock.patch( "importer.handlers.common.vector.BaseVectorFileHandler._select_valid_layers" ) - @override_settings(MEDIA_ROOT="/tmp/", ASSET_ROOT="/tmp/") + @override_settings(MEDIA_ROOT="/tmp/", ASSETS_ROOT="/tmp/") def test_import_geopackage_with_no_crs_table_should_raise_error_if_all_layer_are_invalid( self, _select_valid_layers ): @@ -201,6 +201,7 @@ def test_import_geopackage_with_no_crs_table_should_raise_error_if_all_layer_are payload = { "base_file": open(self.no_crs_gpkg, "rb"), + "store_spatial_file": True } with self.assertLogs(level="ERROR") as _log: diff --git a/importer/tests/fixture/3dtilesample/invalid_tileset.json b/importer/tests/fixture/3dtilesample/invalid_tileset.json new file mode 100755 index 00000000..a76162d5 --- /dev/null +++ b/importer/tests/fixture/3dtilesample/invalid_tileset.json @@ -0,0 +1,3 @@ +{ + "asset": "value" +} diff --git a/importer/tests/fixture/3dtilesample/tileset.json b/importer/tests/fixture/3dtilesample/tileset.json new file mode 100755 index 00000000..e9d6cbf2 --- /dev/null +++ b/importer/tests/fixture/3dtilesample/tileset.json @@ -0,0 +1,16 @@ +{ + "asset" : { + "version" : "1.1" + }, + "geometricError" : 1.0, + "root" : { + "boundingVolume" : { + "box" : [ 0.5, 0.5, 1.0, 0.5, 0.0, 0.0, 0.0, -0.5, 0.0, 0.0, 0.0, 1.0 ] + }, + "geometricError" : 0.0, + "refine" : "REPLACE", + "content" : { + "uri" : "0_0_0-1_1_2.glb" + } + } + } diff --git a/importer/tests/unit/test_orchestrator.py b/importer/tests/unit/test_orchestrator.py index 0a769252..66c6be19 100644 --- a/importer/tests/unit/test_orchestrator.py +++ b/importer/tests/unit/test_orchestrator.py @@ -13,6 +13,7 @@ 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 @@ -183,10 +184,24 @@ def test_perform_with_error_set_invalid_status(self, mock_celery): def test_set_as_failed(self): # creating the temporary file that will be deleted - fake_path = f"{settings.MEDIA_ROOT}/file.txt" + os.makedirs(settings.ASSETS_ROOT, exist_ok=True) + + fake_path = f"{settings.ASSETS_ROOT}file.txt" with open(fake_path, "w"): pass + user = get_user_model().objects.first() + asset_handler = asset_handler_registry.get_default_handler() + + asset = asset_handler.create( + title="Original", + owner=user, + description=None, + type="importer.handlers.gpkg.handler.GPKGFileHandler", + files=[fake_path], + clone_files=False, + ) + self.assertTrue(os.path.exists(fake_path)) # we need to create first the execution _uuid = self.orchestrator.create_execution_request( @@ -196,6 +211,8 @@ def test_set_as_failed(self): input_params={ "files": {"base_file": fake_path}, "store_spatial_files": True, + "asset_id": asset.id, + "asset_module_path": f"{asset.__module__}.{asset.__class__.__name__}", }, ) _uuid = str(_uuid) @@ -215,6 +232,7 @@ def test_set_as_failed(self): req.delete() legacy.delete() + def test_set_as_completed(self): # we need to create first the execution _uuid = self.orchestrator.create_execution_request( @@ -315,6 +333,24 @@ def test_evaluate_execution_progress_should_fail_if_one_task_is_failed(self): Should set it fail if all the execution are done and at least 1 is failed """ # create the celery task result entry + os.makedirs(settings.ASSETS_ROOT, exist_ok=True) + + fake_path = f"{settings.ASSETS_ROOT}/file.txt" + with open(fake_path, "w"): + pass + + user = get_user_model().objects.first() + asset_handler = asset_handler_registry.get_default_handler() + + asset = asset_handler.create( + title="Original", + owner=user, + description=None, + type="importer.handlers.gpkg.handler.GPKGFileHandler", + files=[fake_path], + clone_files=False, + ) + try: exec_id = str( self.orchestrator.create_execution_request( @@ -322,6 +358,9 @@ def test_evaluate_execution_progress_should_fail_if_one_task_is_failed(self): func_name="test", step="test", legacy_upload_name="test", + input_params={ + "asset_id": asset.id, + "asset_module_path": f"{asset.__module__}.{asset.__class__.__name__}",} ) ) diff --git a/runtest.sh b/runtest.sh index 0150efab..7395f363 100755 --- a/runtest.sh +++ b/runtest.sh @@ -2,4 +2,5 @@ set -a . ./.env_test set +a + coverage run --append --source='.' /usr/src/geonode/manage.py test importer -v2 --noinput From d097dfb43110f16793311955e96f2eb2feafbc94 Mon Sep 17 00:00:00 2001 From: Mattia Date: Wed, 5 Jun 2024 12:22:06 +0200 Subject: [PATCH 20/40] test coverage --- .github/workflows/runtests.yml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/.github/workflows/runtests.yml b/.github/workflows/runtests.yml index e965cdab..18c3f039 100644 --- a/.github/workflows/runtests.yml +++ b/.github/workflows/runtests.yml @@ -20,6 +20,24 @@ jobs: run: docker exec django4importer /bin/sh -c "sh /usr/src/importer/entrypoint_test.sh" - name: Run geonode-importer tests run: docker exec django4importer /bin/sh -c "sh /usr/src/importer/runtest.sh" + - name: Code Coverage Report + uses: irongut/CodeCoverageSummary@v1.3.0 + with: + filename: /usr/src/importer/.coverage + badge: true + fail_below_min: true + format: markdown + hide_branch_rate: false + hide_complexity: true + indicators: true + output: both + thresholds: '60 80' + - name: Add Coverage PR Comment + uses: marocchino/sticky-pull-request-comment@v2 + if: github.event_name == 'pull_request' + with: + recreate: true + path: code-coverage-results.md - name: Stop containers if: always() run: docker-compose -f "docker-compose-test.yaml" down \ No newline at end of file From bede3ea47125e3a04286dbd159cadd210610f54b Mon Sep 17 00:00:00 2001 From: Mattia Date: Wed, 5 Jun 2024 12:31:16 +0200 Subject: [PATCH 21/40] test coverage --- .github/workflows/runtests.yml | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/.github/workflows/runtests.yml b/.github/workflows/runtests.yml index 18c3f039..a9dae536 100644 --- a/.github/workflows/runtests.yml +++ b/.github/workflows/runtests.yml @@ -20,24 +20,20 @@ jobs: run: docker exec django4importer /bin/sh -c "sh /usr/src/importer/entrypoint_test.sh" - name: Run geonode-importer tests run: docker exec django4importer /bin/sh -c "sh /usr/src/importer/runtest.sh" - - name: Code Coverage Report - uses: irongut/CodeCoverageSummary@v1.3.0 - with: - filename: /usr/src/importer/.coverage - badge: true - fail_below_min: true - format: markdown - hide_branch_rate: false - hide_complexity: true - indicators: true - output: both - thresholds: '60 80' - - name: Add Coverage PR Comment - uses: marocchino/sticky-pull-request-comment@v2 - if: github.event_name == 'pull_request' - with: - recreate: true - path: code-coverage-results.md + - name: Coverage comment + id: coverage_comment + uses: py-cov-action/python-coverage-comment-action@v3 + with: + GITHUB_TOKEN: ${{ github.token }} + + - name: Store Pull Request comment to be posted + uses: actions/upload-artifact@v4 + if: steps.coverage_comment.outputs.COMMENT_FILE_WRITTEN == 'true' + with: + # If you use a different name, update COMMENT_ARTIFACT_NAME accordingly + name: python-coverage-comment-action + # If you use a different name, update COMMENT_FILENAME accordingly + path: python-coverage-comment-action.txt - name: Stop containers if: always() run: docker-compose -f "docker-compose-test.yaml" down \ No newline at end of file From 1becb43395cf9f21d47c2aa06d5900e1bf05308b Mon Sep 17 00:00:00 2001 From: Mattia Date: Wed, 5 Jun 2024 12:40:53 +0200 Subject: [PATCH 22/40] test coverage --- .coveragerc | 2 ++ .github/workflows/runtests.yml | 20 +++++++++----------- 2 files changed, 11 insertions(+), 11 deletions(-) create mode 100644 .coveragerc diff --git a/.coveragerc b/.coveragerc new file mode 100644 index 00000000..ce2f455f --- /dev/null +++ b/.coveragerc @@ -0,0 +1,2 @@ +[run] +relative_files = True \ No newline at end of file diff --git a/.github/workflows/runtests.yml b/.github/workflows/runtests.yml index a9dae536..7e194f96 100644 --- a/.github/workflows/runtests.yml +++ b/.github/workflows/runtests.yml @@ -21,19 +21,17 @@ jobs: - name: Run geonode-importer tests run: docker exec django4importer /bin/sh -c "sh /usr/src/importer/runtest.sh" - name: Coverage comment - id: coverage_comment - uses: py-cov-action/python-coverage-comment-action@v3 - with: - GITHUB_TOKEN: ${{ github.token }} + id: coverage_comment + uses: py-cov-action/python-coverage-comment-action@v3 + with: + GITHUB_TOKEN: ${{ github.token }} - name: Store Pull Request comment to be posted - uses: actions/upload-artifact@v4 - if: steps.coverage_comment.outputs.COMMENT_FILE_WRITTEN == 'true' - with: - # If you use a different name, update COMMENT_ARTIFACT_NAME accordingly - name: python-coverage-comment-action - # If you use a different name, update COMMENT_FILENAME accordingly - path: python-coverage-comment-action.txt + uses: actions/upload-artifact@v4 + if: steps.coverage_comment.outputs.COMMENT_FILE_WRITTEN == 'true' + with: + name: python-coverage-comment-action + path: python-coverage-comment-action.txt - name: Stop containers if: always() run: docker-compose -f "docker-compose-test.yaml" down \ No newline at end of file From 686690da5debc2cb7bc62a7f0fb12d9c3239428d Mon Sep 17 00:00:00 2001 From: Mattia Date: Wed, 5 Jun 2024 15:43:30 +0200 Subject: [PATCH 23/40] set resource_type as dataset --- importer/handlers/tiles3d/handler.py | 1 + 1 file changed, 1 insertion(+) diff --git a/importer/handlers/tiles3d/handler.py b/importer/handlers/tiles3d/handler.py index 6729358c..6b6d3a52 100644 --- a/importer/handlers/tiles3d/handler.py +++ b/importer/handlers/tiles3d/handler.py @@ -217,6 +217,7 @@ def create_geonode_resource( def generate_resource_payload(self, layer_name, alternate, asset, _exec, workspace): return dict( + resource_type="dataset", subtype="3dtiles", dirty_state=True, title=layer_name, From 950ce937689cd9e105d47f696b2c470945236b85 Mon Sep 17 00:00:00 2001 From: Mattia Date: Wed, 5 Jun 2024 18:27:10 +0200 Subject: [PATCH 24/40] Fix asset location for 3dtiles handler --- importer/handlers/tiles3d/handler.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/importer/handlers/tiles3d/handler.py b/importer/handlers/tiles3d/handler.py index 6b6d3a52..6f620b83 100644 --- a/importer/handlers/tiles3d/handler.py +++ b/importer/handlers/tiles3d/handler.py @@ -6,6 +6,8 @@ from geonode.layers.models import Dataset from geonode.resource.enumerator import ExecutionRequestAction as exa from geonode.upload.utils import UploadLimitValidator +from importer.handlers.base import BaseHandler +from importer.models import ResourceHandlerInfo from importer.orchestrator import orchestrator from importer.celery_tasks import import_orchestrator from importer.handlers.common.vector import BaseVectorFileHandler @@ -206,13 +208,14 @@ def create_geonode_resource( resource_type: Dataset = ..., asset=None, ): - resource = super().create_geonode_resource( - layer_name, alternate, execution_id, ResourceBase, asset - ) # we want just the tileset.json as location of the asset - asset = get_default_asset(resource) asset.location = [path for path in asset.location if "tileset.json" in path] asset.save() + + resource = super().create_geonode_resource( + layer_name, alternate, execution_id, ResourceBase, asset + ) + return resource def generate_resource_payload(self, layer_name, alternate, asset, _exec, workspace): @@ -223,4 +226,6 @@ def generate_resource_payload(self, layer_name, alternate, asset, _exec, workspa title=layer_name, owner=_exec.user, asset=asset, + link_type="uploaded", + extension="3dtiles" ) From dc214d22bcba13efa302f61baa852edcf610e3de Mon Sep 17 00:00:00 2001 From: Mattia Date: Thu, 6 Jun 2024 12:21:28 +0200 Subject: [PATCH 25/40] Fix runtest --- runtest.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtest.sh b/runtest.sh index 7395f363..6ba4ac31 100755 --- a/runtest.sh +++ b/runtest.sh @@ -3,4 +3,4 @@ set -a . ./.env_test set +a -coverage run --append --source='.' /usr/src/geonode/manage.py test importer -v2 --noinput +coverage run --source='.' --omit="*/test*" /usr/src/geonode/manage.py test importer -v2 --noinput From c7641f0e518da187710aaebe039141421af1633a Mon Sep 17 00:00:00 2001 From: Mattia Date: Thu, 6 Jun 2024 12:22:02 +0200 Subject: [PATCH 26/40] Fix runtest --- runtest.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtest.sh b/runtest.sh index 6ba4ac31..94a97352 100755 --- a/runtest.sh +++ b/runtest.sh @@ -3,4 +3,4 @@ set -a . ./.env_test set +a -coverage run --source='.' --omit="*/test*" /usr/src/geonode/manage.py test importer -v2 --noinput +coverage run --source='.' --omit="*/test*" /usr/src/geonode/manage.py test importer -v2 --noinput \ No newline at end of file From b611c3d8f63085fc913ce052e4248c486eb65b39 Mon Sep 17 00:00:00 2001 From: Mattia Date: Thu, 6 Jun 2024 13:07:59 +0200 Subject: [PATCH 27/40] use region for cacluate bbox in 3dtiles --- importer/handlers/tiles3d/handler.py | 30 ++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/importer/handlers/tiles3d/handler.py b/importer/handlers/tiles3d/handler.py index 6f620b83..4e9dcf16 100644 --- a/importer/handlers/tiles3d/handler.py +++ b/importer/handlers/tiles3d/handler.py @@ -2,17 +2,14 @@ import logging import os from pathlib import Path -from geonode.assets.utils import get_default_asset +import math from geonode.layers.models import Dataset from geonode.resource.enumerator import ExecutionRequestAction as exa from geonode.upload.utils import UploadLimitValidator -from importer.handlers.base import BaseHandler -from importer.models import ResourceHandlerInfo from importer.orchestrator import orchestrator from importer.celery_tasks import import_orchestrator from importer.handlers.common.vector import BaseVectorFileHandler from importer.handlers.utils import create_alternate, should_be_imported -from importer.publisher import DataPublisher from importer.utils import ImporterRequestAction as ira from geonode.base.models import ResourceBase from importer.handlers.tiles3d.exceptions import Invalid3DTilesException @@ -216,6 +213,31 @@ def create_geonode_resource( layer_name, alternate, execution_id, ResourceBase, asset ) + # fixing-up bbox for the 3dtile object + js_file = None + with open(asset.location[0]) as _file: + js_file = json.loads(_file.read()) + + if not js_file: + return resource + + # checking if the region is inside the json file + region = js_file.get("root", {}).get("boundingVolume", {}).get("region", None) + if not region: + logger.info(f"No region found, the BBOX will not be updated for 3dtiles: {resource.title}") + return resource + west, south, east, nord = region[:4] + # [xmin, ymin, xmax, ymax] + resource.set_bbox_polygon( + bbox=[ + math.degrees(west), + math.degrees(south), + math.degrees(east), + math.degrees(nord) + ], + srid='EPSG:4326' + ) + return resource def generate_resource_payload(self, layer_name, alternate, asset, _exec, workspace): From d4ce079c7a9cdc04c9f2d6148a14a033bbc515f8 Mon Sep 17 00:00:00 2001 From: Mattia Date: Thu, 6 Jun 2024 15:48:58 +0200 Subject: [PATCH 28/40] Add test coverage and refactor with black --- importer/api/tests.py | 19 ++-- importer/api/views.py | 12 +-- importer/datastore.py | 2 +- importer/handlers/common/vector.py | 24 ++--- importer/handlers/geojson/handler.py | 8 +- importer/handlers/gpkg/tests.py | 2 +- importer/handlers/tiles3d/handler.py | 13 +-- importer/handlers/tiles3d/tests.py | 109 ++++++++++++++++++++++- importer/handlers/utils.py | 2 +- importer/orchestrator.py | 4 +- importer/tests/end2end/test_end2end.py | 82 ++++++++++++++--- importer/tests/unit/test_orchestrator.py | 4 +- importer/tests/unit/test_task.py | 2 +- 13 files changed, 232 insertions(+), 51 deletions(-) diff --git a/importer/api/tests.py b/importer/api/tests.py index e4ff7dc9..5144ba3d 100644 --- a/importer/api/tests.py +++ b/importer/api/tests.py @@ -53,7 +53,10 @@ def test_raise_exception_if_file_is_not_a_handled(self): def test_gpkg_raise_error_with_invalid_payload(self): self.client.force_login(get_user_model().objects.get(username="admin")) payload = { - "base_file": SimpleUploadedFile(name="test.gpkg", content=b'{"type": "FeatureCollection", "content": "some-content"}'), + "base_file": SimpleUploadedFile( + name="test.gpkg", + content=b'{"type": "FeatureCollection", "content": "some-content"}', + ), "store_spatial_files": "invalid", } expected = { @@ -73,7 +76,10 @@ def test_gpkg_task_is_called(self, patch_upload): self.client.force_login(get_user_model().objects.get(username="admin")) payload = { - "base_file": SimpleUploadedFile(name="test.gpkg", content=b'{"type": "FeatureCollection", "content": "some-content"}'), + "base_file": SimpleUploadedFile( + name="test.gpkg", + content=b'{"type": "FeatureCollection", "content": "some-content"}', + ), "store_spatial_files": True, } @@ -88,7 +94,8 @@ def test_geojson_task_is_called(self, patch_upload): self.client.force_login(get_user_model().objects.get(username="admin")) payload = { "base_file": SimpleUploadedFile( - name="test.geojson", content=b'{"type": "FeatureCollection", "content": "some-content"}' + name="test.geojson", + content=b'{"type": "FeatureCollection", "content": "some-content"}', ), "store_spatial_files": True, } @@ -164,7 +171,8 @@ def test_asset_is_created_before_the_import_start(self, patch_upload): self.client.force_login(get_user_model().objects.get(username="admin")) payload = { "base_file": SimpleUploadedFile( - name="test.geojson", content=b'{"type": "FeatureCollection", "content": "some-content"}' + name="test.geojson", + content=b'{"type": "FeatureCollection", "content": "some-content"}', ), "store_spatial_files": True, } @@ -195,7 +203,8 @@ def test_asset_should_be_deleted_if_created_during_with_exception( self.client.force_login(get_user_model().objects.get(username="admin")) payload = { "base_file": SimpleUploadedFile( - name="test.geojson", content=b'{"type": "FeatureCollection", "content": "some-content"}' + name="test.geojson", + content=b'{"type": "FeatureCollection", "content": "some-content"}', ), "store_spatial_files": True, } diff --git a/importer/api/views.py b/importer/api/views.py index 51ee6409..8b603a33 100644 --- a/importer/api/views.py +++ b/importer/api/views.py @@ -111,7 +111,7 @@ def create(self, request, *args, **kwargs): if "zip_file" in _data or "kmz_file" in _data: # if a zipfile is provided, we need to unzip it before searching for an handler - zipname = Path(_data['base_file'].name).stem + zipname = Path(_data["base_file"].name).stem storage_manager = StorageManager( remote_files={"base_file": _data.get("zip_file", _data.get("kmz_file"))} ) @@ -120,10 +120,12 @@ def create(self, request, *args, **kwargs): cloning_directory=asset_dir, create_tempdir=False ) # update the payload with the unziped paths - _data.update({ - **{"original_zip_name": zipname}, - **storage_manager.get_retrieved_paths() - }) + _data.update( + { + **{"original_zip_name": zipname}, + **storage_manager.get_retrieved_paths(), + } + ) handler = orchestrator.get_handler(_data) diff --git a/importer/datastore.py b/importer/datastore.py index 4c85c9c8..d472031c 100644 --- a/importer/datastore.py +++ b/importer/datastore.py @@ -12,7 +12,7 @@ def __init__( self, files: list, handler_module_path: str, - user: get_user_model(), # type: ignore + user: get_user_model(), # type: ignore execution_id: str, ) -> None: self.files = files diff --git a/importer/handlers/common/vector.py b/importer/handlers/common/vector.py index 4c175c55..b60ea731 100644 --- a/importer/handlers/common/vector.py +++ b/importer/handlers/common/vector.py @@ -593,7 +593,9 @@ def create_geonode_resource( saved_dataset = resource_manager.create( None, resource_type=resource_type, - defaults=self.generate_resource_payload(layer_name, alternate, asset, _exec, workspace), + defaults=self.generate_resource_payload( + layer_name, alternate, asset, _exec, workspace + ), ) saved_dataset.refresh_from_db() @@ -610,16 +612,16 @@ def create_geonode_resource( def generate_resource_payload(self, layer_name, alternate, asset, _exec, workspace): return dict( - name=alternate, - workspace=workspace, - store=os.environ.get("GEONODE_GEODATABASE", "geonode_data"), - subtype="vector", - alternate=f"{workspace}:{alternate}", - dirty_state=True, - title=layer_name, - owner=_exec.user, - asset=asset, - ) + name=alternate, + workspace=workspace, + store=os.environ.get("GEONODE_GEODATABASE", "geonode_data"), + subtype="vector", + alternate=f"{workspace}:{alternate}", + dirty_state=True, + title=layer_name, + owner=_exec.user, + asset=asset, + ) def overwrite_geonode_resource( self, diff --git a/importer/handlers/geojson/handler.py b/importer/handlers/geojson/handler.py index f7535a48..ea79ea0d 100644 --- a/importer/handlers/geojson/handler.py +++ b/importer/handlers/geojson/handler.py @@ -59,19 +59,19 @@ def can_handle(_data) -> bool: return False ext = base.split(".")[-1] if isinstance(base, str) else base.name.split(".")[-1] if ext in ["json", "geojson"]: - ''' + """ Check if is a real geojson based on specification https://datatracker.ietf.org/doc/html/rfc7946#section-1.4 - ''' + """ try: _file = base if isinstance(base, str): - with open(base, 'r') as f: + with open(base, "r") as f: _file = json.loads(f.read()) else: _file = json.loads(base.read()) - return _file.get('type', None) in ['FeatureCollection', 'Feature'] + return _file.get("type", None) in ["FeatureCollection", "Feature"] except Exception: return False diff --git a/importer/handlers/gpkg/tests.py b/importer/handlers/gpkg/tests.py index bca621f0..91c5cd09 100644 --- a/importer/handlers/gpkg/tests.py +++ b/importer/handlers/gpkg/tests.py @@ -118,7 +118,7 @@ def test_single_message_error_handler(self): # lets copy the file to the temporary folder # later will be removed shutil.copy(self.valid_gpkg, "/tmp") - + user = get_user_model().objects.first() asset_handler = asset_handler_registry.get_default_handler() diff --git a/importer/handlers/tiles3d/handler.py b/importer/handlers/tiles3d/handler.py index 4e9dcf16..c0ba8a08 100644 --- a/importer/handlers/tiles3d/handler.py +++ b/importer/handlers/tiles3d/handler.py @@ -220,11 +220,13 @@ def create_geonode_resource( if not js_file: return resource - + # checking if the region is inside the json file region = js_file.get("root", {}).get("boundingVolume", {}).get("region", None) if not region: - logger.info(f"No region found, the BBOX will not be updated for 3dtiles: {resource.title}") + logger.info( + f"No region found, the BBOX will not be updated for 3dtiles: {resource.title}" + ) return resource west, south, east, nord = region[:4] # [xmin, ymin, xmax, ymax] @@ -233,9 +235,9 @@ def create_geonode_resource( math.degrees(west), math.degrees(south), math.degrees(east), - math.degrees(nord) + math.degrees(nord), ], - srid='EPSG:4326' + srid="EPSG:4326", ) return resource @@ -249,5 +251,6 @@ def generate_resource_payload(self, layer_name, alternate, asset, _exec, workspa owner=_exec.user, asset=asset, link_type="uploaded", - extension="3dtiles" + extension="3dtiles", + alternate=alternate, ) diff --git a/importer/handlers/tiles3d/tests.py b/importer/handlers/tiles3d/tests.py index aad167ad..ba7fbaeb 100644 --- a/importer/handlers/tiles3d/tests.py +++ b/importer/handlers/tiles3d/tests.py @@ -1,14 +1,17 @@ import json import os +import shutil from django.test import TestCase from importer.handlers.tiles3d.exceptions import Invalid3DTilesException from importer.handlers.tiles3d.handler import Tiles3DFileHandler from django.contrib.auth import get_user_model from importer import project_dir +from importer.orchestrator import orchestrator from geonode.upload.models import UploadParallelismLimit from geonode.upload.api.exceptions import UploadParallelismLimitException from geonode.base.populate_test_data import create_single_dataset from osgeo import ogr +from geonode.assets.handlers import asset_handler_registry class TestTiles3DFileHandler(TestCase): @@ -20,6 +23,9 @@ def setUpClass(cls): cls.handler = Tiles3DFileHandler() cls.valid_3dtile = f"{project_dir}/tests/fixture/3dtilesample/valid_3dtiles.zip" cls.valid_tileset = f"{project_dir}/tests/fixture/3dtilesample/tileset.json" + cls.valid_tileset_with_region = ( + f"{project_dir}/tests/fixture/3dtilesample/tileset_with_region.json" + ) cls.invalid_tileset = ( f"{project_dir}/tests/fixture/3dtilesample/invalid_tileset.json" ) @@ -27,10 +33,11 @@ def setUpClass(cls): cls.user, _ = get_user_model().objects.get_or_create(username="admin") cls.invalid_files = {"base_file": cls.invalid_3dtile} cls.valid_files = {"base_file": cls.valid_3dtile} - cls.owner = get_user_model().objects.first() + cls.owner = get_user_model().objects.exclude(username="AnonymousUser").first() cls.layer = create_single_dataset( name="urban_forestry_street_tree_benefits_epsg_26985", owner=cls.owner ) + cls.asset_handler = asset_handler_registry.get_default_handler() def test_task_list_is_the_expected_one(self): expected = ( @@ -69,6 +76,22 @@ def test_is_valid_should_raise_exception_if_the_parallelism_is_met(self): def test_is_valid_should_pass_with_valid_3dtiles(self): self.handler.is_valid(files={"base_file": self.valid_tileset}, user=self.user) + def test_is_valid_should_raise_exception_if_no_basefile_is_supplied(self): + data = {"base_file": ""} + with self.assertRaises(Invalid3DTilesException) as _exc: + self.handler.is_valid(files=data, user=self.user) + + self.assertIsNotNone(_exc) + self.assertTrue("base file is not provided" in str(_exc.exception.detail)) + + def test_extract_params_from_data(self): + actual, _data = self.handler.extract_params_from_data( + _data={"defaults": '{"title":"title_of_the_cloned_resource"}'}, + action="copy", + ) + + self.assertEqual(actual, {"title": "title_of_the_cloned_resource"}) + def test_is_valid_should_raise_exception_if_the_3dtiles_is_invalid(self): data = { "base_file": "/using/double/dot/in/the/name/is/an/error/file.invalid.json" @@ -163,3 +186,87 @@ def test_can_handle_should_return_true_for_3dtiles(self): def test_can_handle_should_return_false_for_other_files(self): actual = self.handler.can_handle({"base_file": "random.gpkg"}) self.assertFalse(actual) + + def test_can_handle_should_return_false_if_no_basefile(self): + actual = self.handler.can_handle({"base_file": ""}) + self.assertFalse(actual) + + def test_supported_file_extension_config(self): + """ + should return the expected value + """ + expected = { + "id": "3dtiles", + "label": "3D Tiles", + "format": "vector", + "ext": ["json"], + "optional": ["xml", "sld"], + } + actual = self.handler.supported_file_extension_config + self.assertDictEqual(actual, expected) + + def test_generate_resource_payload(self): + exec_id = orchestrator.create_execution_request( + user=self.owner, + func_name="funct1", + step="step", + input_params={"files": self.valid_files, "skip_existing_layer": True}, + ) + _exec_obj = orchestrator.get_execution_object(exec_id) + expected = dict( + resource_type="dataset", + subtype="3dtiles", + dirty_state=True, + title="Layer name", + owner=self.owner, + asset="asset", + link_type="uploaded", + extension="3dtiles", + ) + + actual = self.handler.generate_resource_payload( + "Layer name", "alternate", "asset", _exec_obj, None + ) + self.assertDictEqual(actual, expected) + + def test_create_geonode_resource_validate_bbox_with_region(self): + shutil.copy(self.valid_tileset_with_region, "/tmp/tileset.json") + + exec_id = orchestrator.create_execution_request( + user=self.owner, + func_name="funct1", + step="step", + input_params={ + "files": {"base_file": "/tmp/tileset.json"}, + "skip_existing_layer": True, + }, + ) + + asset = self.asset_handler.create( + title="Original", + owner=self.owner, + description=None, + type=str(self.handler), + files=["/tmp/tileset.json"], + clone_files=False, + ) + + resource = self.handler.create_geonode_resource( + "layername", + "layeralternate", + execution_id=exec_id, + resource_type="ResourceBase", + asset=asset, + ) + + # validate bbox + default_bbox = [-180.0, 180.0, -90.0, 90.0, "EPSG:4326"] + self.assertFalse(resource.bbox == default_bbox) + expected = [ + -75.6144410959485, + -75.60974751970046, + 40.040721313841274, + 40.04433990901052, + "EPSG:4326", + ] + self.assertTrue(resource.bbox == expected) diff --git a/importer/handlers/utils.py b/importer/handlers/utils.py index bb274f6d..1ef3680e 100644 --- a/importer/handlers/utils.py +++ b/importer/handlers/utils.py @@ -43,7 +43,7 @@ } -def should_be_imported(layer: str, user: get_user_model(), **kwargs) -> bool: # type: ignore +def should_be_imported(layer: str, user: get_user_model(), **kwargs) -> bool: # type: ignore """ If layer_name + user (without the addition of any execution id) already exists, will apply one of the rule available: diff --git a/importer/orchestrator.py b/importer/orchestrator.py index 054bfa99..81b63ffa 100644 --- a/importer/orchestrator.py +++ b/importer/orchestrator.py @@ -172,8 +172,8 @@ def set_as_failed(self, execution_id, reason=None, delete_file=True): if delete_file: exec_obj = self.get_execution_object(execution_id) # cleanup asset in case of fail - asset_handler = import_string(exec_obj.input_params['asset_module_path']) - asset = asset_handler.objects.filter(pk=exec_obj.input_params['asset_id']) + asset_handler = import_string(exec_obj.input_params["asset_module_path"]) + asset = asset_handler.objects.filter(pk=exec_obj.input_params["asset_id"]) if asset.exists(): asset.first().delete() diff --git a/importer/tests/end2end/test_end2end.py b/importer/tests/end2end/test_end2end.py index c4deab1f..d14794a3 100644 --- a/importer/tests/end2end/test_end2end.py +++ b/importer/tests/end2end/test_end2end.py @@ -1,6 +1,7 @@ import ast import os import time +from uuid import uuid4 import mock from django.conf import settings @@ -16,6 +17,9 @@ from importer.tests.utils import ImporterBaseTestSupport import gisdata from geonode.base.populate_test_data import create_single_dataset +from django.db.models import Q +from geonode.base.models import ResourceBase +from geonode.resource.manager import resource_manager geourl = settings.GEODATABASE_URL @@ -24,6 +28,7 @@ class BaseImporterEndToEndTest(ImporterBaseTestSupport): @classmethod def setUpClass(cls) -> None: super().setUpClass() + cls.user = get_user_model().objects.exclude(username="Anonymous").first() cls.valid_gkpg = f"{project_dir}/tests/fixture/valid.gpkg" cls.valid_geojson = f"{project_dir}/tests/fixture/valid.geojson" cls.no_crs_gpkg = f"{project_dir}/tests/fixture/noCrsTable.gpkg" @@ -38,6 +43,9 @@ def setUpClass(cls) -> None: cls.valid_kml = f"{project_dir}/tests/fixture/valid.kml" cls.valid_tif = f"{project_dir}/tests/fixture/test_grid.tif" cls.valid_csv = f"{project_dir}/tests/fixture/valid.csv" + cls.valid_3dtiles = ( + f"{project_dir}/tests/fixture/3dtilesample/valid_3dtiles.zip" + ) cls.url = reverse("importer_upload") ogc_server_settings = OGC_Servers_Handler(settings.OGC_SERVER)["default"] @@ -58,7 +66,14 @@ def tearDown(self) -> None: for el in Dataset.objects.all(): el.delete() - def _assertimport(self, payload, initial_name, overwrite=False, last_update=None): + def _assertimport( + self, + payload, + initial_name, + overwrite=False, + last_update=None, + skip_geoserver=False, + ): try: self.client.force_login(self.admin) @@ -99,21 +114,24 @@ def _assertimport(self, payload, initial_name, overwrite=False, last_update=None self.assertTrue(entries.as_model().objects.exists()) # check if the geonode resource exists - dataset = Dataset.objects.filter( - alternate__icontains=f"geonode:{initial_name}" + resource = ResourceBase.objects.filter( + Q(alternate__icontains=f"geonode:{initial_name}") + | Q(alternate__icontains=initial_name) ) - self.assertTrue(dataset.exists()) + self.assertTrue(resource.exists()) - resources = self.cat.get_resources() - # check if the resource is in geoserver - self.assertTrue(dataset.first().title in [y.name for y in resources]) + if not skip_geoserver: + resources = self.cat.get_resources() + # check if the resource is in geoserver + self.assertTrue(resource.first().title in [y.name for y in resources]) if overwrite: - self.assertTrue(dataset.first().last_updated > last_update) + self.assertTrue(resource.first().last_updated > last_update) finally: - dataset = Dataset.objects.filter( - alternate__icontains=f"geonode:{initial_name}" + resource = ResourceBase.objects.filter( + Q(alternate__icontains=f"geonode:{initial_name}") + | Q(alternate__icontains=initial_name) ) - dataset.delete() + resource.delete() class ImporterGeoPackageImportTest(BaseImporterEndToEndTest): @@ -201,7 +219,7 @@ def test_import_geopackage_with_no_crs_table_should_raise_error_if_all_layer_are payload = { "base_file": open(self.no_crs_gpkg, "rb"), - "store_spatial_file": True + "store_spatial_file": True, } with self.assertLogs(level="ERROR") as _log: @@ -426,3 +444,43 @@ def test_import_raster_overwrite(self): layer = self.cat.get_layer("test_grid") if layer: self.cat.delete(layer) + + +class Importer3DtilesImportTest(BaseImporterEndToEndTest): + @mock.patch.dict(os.environ, {"GEONODE_GEODATABASE": "test_geonode_data"}) + @override_settings( + GEODATABASE_URL=f"{geourl.split('/geonode_data')[0]}/test_geonode_data" + ) + def test_import_3dtiles(self): + payload = { + "zip_file": open(self.valid_3dtiles, "rb"), + "base_file": open(self.valid_3dtiles, "rb"), + } + initial_name = "valid_3dtiles" + self._assertimport(payload, initial_name, skip_geoserver=True) + + @mock.patch.dict(os.environ, {"GEONODE_GEODATABASE": "test_geonode_data"}) + @override_settings( + GEODATABASE_URL=f"{geourl.split('/geonode_data')[0]}/test_geonode_data" + ) + def test_import_3dtiles_overwrite(self): + payload = { + "zip_file": open(self.valid_3dtiles, "rb"), + "base_file": open(self.valid_3dtiles, "rb"), + } + initial_name = "valid_3dtiles" + # importing the resource + resource = resource_manager.create( + str(uuid4()), + resource_type=ResourceBase, + defaults={"title": "simple resourcebase", "owner": self.user}, + ) + + payload["overwrite_existing_layer"] = True + self._assertimport( + payload, + initial_name, + overwrite=True, + last_update=resource.last_updated, + skip_geoserver=True, + ) diff --git a/importer/tests/unit/test_orchestrator.py b/importer/tests/unit/test_orchestrator.py index 66c6be19..4f6ef315 100644 --- a/importer/tests/unit/test_orchestrator.py +++ b/importer/tests/unit/test_orchestrator.py @@ -232,7 +232,6 @@ def test_set_as_failed(self): req.delete() legacy.delete() - def test_set_as_completed(self): # we need to create first the execution _uuid = self.orchestrator.create_execution_request( @@ -360,7 +359,8 @@ def test_evaluate_execution_progress_should_fail_if_one_task_is_failed(self): legacy_upload_name="test", input_params={ "asset_id": asset.id, - "asset_module_path": f"{asset.__module__}.{asset.__class__.__name__}",} + "asset_module_path": f"{asset.__module__}.{asset.__class__.__name__}", + }, ) ) diff --git a/importer/tests/unit/test_task.py b/importer/tests/unit/test_task.py index 2bacc12c..f076ab34 100644 --- a/importer/tests/unit/test_task.py +++ b/importer/tests/unit/test_task.py @@ -41,7 +41,7 @@ class TestCeleryTasks(ImporterBaseTestSupport): def setUp(self): self.user = get_user_model().objects.first() - + self.existing_file = f"{project_dir}/tests/fixture/valid.gpkg" self.asset_handler = asset_handler_registry.get_default_handler() From 6bb056a7a76c36cdc47de25e5a9b154ffbd150b3 Mon Sep 17 00:00:00 2001 From: Mattia Date: Thu, 6 Jun 2024 16:08:49 +0200 Subject: [PATCH 29/40] Add test coverage and refactor with black --- .../3dtilesample/tileset_with_region.json | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 importer/tests/fixture/3dtilesample/tileset_with_region.json diff --git a/importer/tests/fixture/3dtilesample/tileset_with_region.json b/importer/tests/fixture/3dtilesample/tileset_with_region.json new file mode 100644 index 00000000..bfdb5f53 --- /dev/null +++ b/importer/tests/fixture/3dtilesample/tileset_with_region.json @@ -0,0 +1,21 @@ +{ + "asset": { + "version": "1.0" + }, + "geometricError": 70, + "root": { + "boundingVolume": { + "region": [ + -1.3197209591796106, + 0.6988424218, + -1.3196390408203893, + 0.6989055782, + 0, + 20 + ] + }, + "geometricError": 70, + "refine": "ADD" + } +} + \ No newline at end of file From 2661110b16fe226e3a2872b8da5bda4fd96f0452 Mon Sep 17 00:00:00 2001 From: Mattia Date: Thu, 6 Jun 2024 18:17:57 +0200 Subject: [PATCH 30/40] Add test coverage and refactor with black --- importer/handlers/tiles3d/tests.py | 2 ++ importer/tests/end2end/test_end2end.py | 3 ++- runtest.sh | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/importer/handlers/tiles3d/tests.py b/importer/handlers/tiles3d/tests.py index ba7fbaeb..2fcd2834 100644 --- a/importer/handlers/tiles3d/tests.py +++ b/importer/handlers/tiles3d/tests.py @@ -222,11 +222,13 @@ def test_generate_resource_payload(self): asset="asset", link_type="uploaded", extension="3dtiles", + alternate="alternate" ) actual = self.handler.generate_resource_payload( "Layer name", "alternate", "asset", _exec_obj, None ) + self.assertSetEqual(set(list(actual.keys())), set(list(expected.keys()))) self.assertDictEqual(actual, expected) def test_create_geonode_resource_validate_bbox_with_region(self): diff --git a/importer/tests/end2end/test_end2end.py b/importer/tests/end2end/test_end2end.py index d14794a3..259bfb4a 100644 --- a/importer/tests/end2end/test_end2end.py +++ b/importer/tests/end2end/test_end2end.py @@ -78,7 +78,7 @@ def _assertimport( self.client.force_login(self.admin) response = self.client.post(self.url, data=payload) - self.assertEqual(201, response.status_code) + self.assertEqual(201, response.status_code, response.json()) # if is async, we must wait. It will wait for 1 min before raise exception if ast.literal_eval(os.getenv("ASYNC_SIGNALS", "False")): @@ -452,6 +452,7 @@ class Importer3DtilesImportTest(BaseImporterEndToEndTest): GEODATABASE_URL=f"{geourl.split('/geonode_data')[0]}/test_geonode_data" ) def test_import_3dtiles(self): + ResourceBase.objects.filter(alternate__icontains="valid_3dtiles").delete() payload = { "zip_file": open(self.valid_3dtiles, "rb"), "base_file": open(self.valid_3dtiles, "rb"), diff --git a/runtest.sh b/runtest.sh index 94a97352..6ba4ac31 100755 --- a/runtest.sh +++ b/runtest.sh @@ -3,4 +3,4 @@ set -a . ./.env_test set +a -coverage run --source='.' --omit="*/test*" /usr/src/geonode/manage.py test importer -v2 --noinput \ No newline at end of file +coverage run --source='.' --omit="*/test*" /usr/src/geonode/manage.py test importer -v2 --noinput From 8dbc004c0959c803940f3f01dacc3e08f9c9b67c Mon Sep 17 00:00:00 2001 From: Mattia Date: Fri, 14 Jun 2024 18:17:28 +0200 Subject: [PATCH 31/40] fix path stem --- importer/handlers/tiles3d/handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/importer/handlers/tiles3d/handler.py b/importer/handlers/tiles3d/handler.py index c0ba8a08..06e146d1 100644 --- a/importer/handlers/tiles3d/handler.py +++ b/importer/handlers/tiles3d/handler.py @@ -155,7 +155,7 @@ def import_resource(self, files: dict, execution_id: str, **kwargs) -> str: ) filename = ( _exec.input_params.get("original_zip_name") - or Path(files.get("base_file")).ste + or Path(files.get("base_file")).stem ) # start looping on the layers available layer_name = self.fixup_name(filename) From 83cd097bb05cfcc6295709a7783367f24b0a5b87 Mon Sep 17 00:00:00 2001 From: mattiagiupponi <51856725+mattiagiupponi@users.noreply.github.com> Date: Mon, 17 Jun 2024 10:28:45 +0200 Subject: [PATCH 32/40] Assets data retriever (#244) * [Fixes #242] CRS parsing is not correctly handled for CSV files * Let importer create the asset * Add test coverage for asset-importer --- .env_test | 5 ++- Dockerfile | 2 +- importer/__init__.py | 2 +- importer/api/tests.py | 51 ++++++++++++++++++++++++ importer/api/views.py | 42 +++++++++++++++++-- importer/celery_tasks.py | 10 ++++- importer/handlers/README.md | 2 +- importer/handlers/base.py | 7 ---- importer/handlers/common/metadata.py | 1 - importer/handlers/common/raster.py | 22 ++++------ importer/handlers/common/tests_vector.py | 2 +- importer/handlers/common/vector.py | 29 +++++--------- importer/handlers/csv/handler.py | 11 +++-- importer/handlers/sld/tests.py | 5 ++- importer/handlers/xml/tests.py | 5 ++- importer/tests/end2end/test_end2end.py | 44 ++++++++++++++++++++ importer/tests/unit/test_task.py | 18 +++++++-- 17 files changed, 196 insertions(+), 62 deletions(-) diff --git a/.env_test b/.env_test index dcfa53ec..f751770a 100644 --- a/.env_test +++ b/.env_test @@ -178,8 +178,9 @@ DEBUG=False SECRET_KEY='myv-y4#7j-d*p-__@j#*3z@!y24fz8%^z2v6atuy4bo9vqr1_a' -STATIC_ROOT=/mnt/volumes/statics/static/ -MEDIA_ROOT=/mnt/volumes/statics/uploaded/ +STATIC_ROOT=/tmp/statics/static/ +MEDIA_ROOT=/tmp/statics/uploaded/ +ASSET_ROOT=/tmp/statics/assets/ GEOIP_PATH=/mnt/volumes/statics/geoip.db CACHE_BUSTING_STATIC_ENABLED=False diff --git a/Dockerfile b/Dockerfile index a6fc2c4a..083ebab5 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,7 @@ FROM geonode/geonode-base:latest-ubuntu-22.04 RUN rm -rf /usr/src/geonode RUN git clone https://github.com/GeoNode/geonode.git /usr/src/geonode -RUN cd /usr/src/geonode && git checkout 12124_assets && cd - +RUN cd /usr/src/geonode && git fetch --all && git checkout 12124_assets_20240523 && cd - RUN mkdir -p /usr/src/importer RUN cd .. diff --git a/importer/__init__.py b/importer/__init__.py index e3c65827..4138c65c 100644 --- a/importer/__init__.py +++ b/importer/__init__.py @@ -20,7 +20,7 @@ project_dir = os.path.dirname(os.path.abspath(__file__)) -VERSION = (1, 0, 10) +VERSION = (1, 1, 0) __version__ = ".".join([str(i) for i in VERSION]) __author__ = "geosolutions-it" __email__ = "info@geosolutionsgroup.com" diff --git a/importer/api/tests.py b/importer/api/tests.py index bcf8b357..7c54b2ba 100644 --- a/importer/api/tests.py +++ b/importer/api/tests.py @@ -11,6 +11,9 @@ from importer.models import ResourceHandlerInfo from importer.tests.utils import ImporterBaseTestSupport +from importer.orchestrator import orchestrator +from django.utils.module_loading import import_string +from geonode.assets.models import LocalAsset class TestImporterViewSet(ImporterBaseTestSupport): @@ -153,3 +156,51 @@ def test_copy_ther_resource_if_file_handler_is_set(self, _orc): self.assertEqual(200, response.status_code) _orc.s.assert_called_once() + + @patch("importer.api.views.import_orchestrator") + def test_asset_is_created_before_the_import_start(self, patch_upload): + patch_upload.apply_async.side_effect = MagicMock() + + self.client.force_login(get_user_model().objects.get(username="admin")) + payload = { + "base_file": SimpleUploadedFile( + name="test.geojson", content=b"some-content" + ), + "store_spatial_files": True, + } + + response = self.client.post(self.url, data=payload) + + self.assertEqual(201, response.status_code) + + self.assertTrue(201, response.status_code) + + _exec = orchestrator.get_execution_object(response.json()["execution_id"]) + + asset_handler = import_string(_exec.input_params["asset_module_path"]) + self.assertTrue(asset_handler.objects.filter(id=_exec.input_params["asset_id"])) + + asset_handler.objects.filter(id=_exec.input_params["asset_id"]).delete() + + @patch("importer.api.views.import_orchestrator") + @patch( + "importer.api.views.UploadLimitValidator.validate_parallelism_limit_per_user" + ) + def test_asset_should_be_deleted_if_created_during_with_exception( + self, validate_parallelism_limit_per_user, patch_upload + ): + patch_upload.apply_async.s.side_effect = MagicMock() + validate_parallelism_limit_per_user.side_effect = Exception("random exception") + + self.client.force_login(get_user_model().objects.get(username="admin")) + payload = { + "base_file": SimpleUploadedFile( + name="test.geojson", content=b"some-content" + ), + "store_spatial_files": True, + } + + response = self.client.post(self.url, data=payload) + + self.assertEqual(500, response.status_code) + self.assertFalse(LocalAsset.objects.exists()) diff --git a/importer/api/views.py b/importer/api/views.py index 0d71b5eb..6c13157b 100644 --- a/importer/api/views.py +++ b/importer/api/views.py @@ -46,6 +46,8 @@ from rest_framework.parsers import FileUploadParser, MultiPartParser from rest_framework.permissions import IsAuthenticatedOrReadOnly from rest_framework.response import Response +from geonode.assets.handlers import asset_handler_registry +from geonode.assets.local import LocalAssetHandler logger = logging.getLogger(__name__) @@ -91,6 +93,8 @@ def create(self, request, *args, **kwargs): """ _file = request.FILES.get("base_file") or request.data.get("base_file") execution_id = None + asset_handler = LocalAssetHandler() + asset_dir = asset_handler._create_asset_dir() serializer = self.get_serializer_class() data = serializer(data=request.data) @@ -111,13 +115,16 @@ def create(self, request, *args, **kwargs): remote_files={"base_file": _data.get("zip_file", _data.get("kmz_file"))} ) # cloning and unzip the base_file - storage_manager.clone_remote_files() + storage_manager.clone_remote_files( + cloning_directory=asset_dir, create_tempdir=False + ) # update the payload with the unziped paths _data.update(storage_manager.get_retrieved_paths()) handler = orchestrator.get_handler(_data) if _file and handler: + asset = None try: # cloning data into a local folder extracted_params, _data = handler.extract_params_from_data(_data) @@ -125,9 +132,13 @@ def create(self, request, *args, **kwargs): # means that the storage manager is not initialized yet, so # the file is not a zip storage_manager = StorageManager(remote_files=_data) - storage_manager.clone_remote_files() + storage_manager.clone_remote_files( + cloning_directory=asset_dir, create_tempdir=False + ) # get filepath - files = storage_manager.get_retrieved_paths() + asset, files = self.generate_asset_and_retrieve_paths( + request, storage_manager, handler + ) upload_validator = UploadLimitValidator(request.user) upload_validator.validate_parallelism_limit_per_user() @@ -144,6 +155,10 @@ def create(self, request, *args, **kwargs): input_params={ **{"files": files, "handler_module_path": str(handler)}, **extracted_params, + **{ + "asset_id": asset.id, + "asset_module_path": f"{asset.__module__}.{asset.__class__.__name__}", + }, }, legacy_upload_name=_file.name, action=action, @@ -159,7 +174,12 @@ def create(self, request, *args, **kwargs): except Exception as e: # in case of any exception, is better to delete the # cloned files to keep the storage under control - if storage_manager is not None: + if asset: + try: + asset.delete() + except Exception as _exc: + logger.warning(_exc) + elif storage_manager is not None: storage_manager.delete_retrieved_paths(force=True) if execution_id: orchestrator.set_as_failed(execution_id=str(execution_id), reason=e) @@ -168,6 +188,20 @@ def create(self, request, *args, **kwargs): raise ImportException(detail="No handlers found for this dataset type") + def generate_asset_and_retrieve_paths(self, request, storage_manager, handler): + asset_handler = asset_handler_registry.get_default_handler() + _files = storage_manager.get_retrieved_paths() + asset = asset_handler.create( + title="Original", + owner=request.user, + description=None, + type=str(handler), + files=list(set(_files.values())), + clone_files=False, + ) + + return asset, _files + class ResourceImporter(DynamicModelViewSet): authentication_classes = [ diff --git a/importer/celery_tasks.py b/importer/celery_tasks.py index c7cfb2f9..a86d3da4 100644 --- a/importer/celery_tasks.py +++ b/importer/celery_tasks.py @@ -329,6 +329,12 @@ def create_geonode_resource( _files = _exec.input_params.get("files") + _asset = ( + import_string(_exec.input_params.get("asset_module_path")) + .objects.filter(id=_exec.input_params.get("asset_id")) + .first() + ) + handler = import_string(handler_module_path)() _overwrite = _exec.input_params.get("overwrite_existing_layer") @@ -337,14 +343,14 @@ def create_geonode_resource( layer_name=layer_name, alternate=alternate, execution_id=execution_id, - files=_files, + asset=_asset, ) else: resource = handler.create_geonode_resource( layer_name=layer_name, alternate=alternate, execution_id=execution_id, - files=_files, + asset=_asset, ) if _overwrite: diff --git a/importer/handlers/README.md b/importer/handlers/README.md index 8916e951..37d982a7 100644 --- a/importer/handlers/README.md +++ b/importer/handlers/README.md @@ -158,7 +158,7 @@ class BaseVectorFileHandler(BaseHandler): return def overwrite_geonode_resource( - self, layer_name: str, alternate: str, execution_id: str, resource_type: Dataset = Dataset, files=None + self, layer_name: str, alternate: str, execution_id: str, resource_type: Dataset = Dataset, asset=None ): """ Base function to override the resource into geonode. Each handler can specify diff --git a/importer/handlers/base.py b/importer/handlers/base.py index 6e29446e..e005fcd4 100644 --- a/importer/handlers/base.py +++ b/importer/handlers/base.py @@ -7,7 +7,6 @@ from importer.utils import ImporterRequestAction as ira from django_celery_results.models import TaskResult from django.db.models import Q -from geonode.storage.manager import storage_manager logger = logging.getLogger(__name__) @@ -150,12 +149,6 @@ def perform_last_step(execution_id): ] _exec.output_params.update({"resources": resource_output_params}) _exec.save() - - # since the original file is now available as asset, we can delete the input files - # TODO must be improved. The asset should be created in the beginning - for _file in _exec.input_params.get("files", {}).values(): - if storage_manager.exists(_file): - storage_manager.delete(_file) return _exec diff --git a/importer/handlers/common/metadata.py b/importer/handlers/common/metadata.py index 8b86db1c..14a80454 100644 --- a/importer/handlers/common/metadata.py +++ b/importer/handlers/common/metadata.py @@ -7,7 +7,6 @@ from importer.orchestrator import orchestrator from django.shortcuts import get_object_or_404 from geonode.layers.models import Dataset -from geonode.storage.manager import storage_manager logger = logging.getLogger(__name__) diff --git a/importer/handlers/common/raster.py b/importer/handlers/common/raster.py index ae9cd968..2990c4c5 100644 --- a/importer/handlers/common/raster.py +++ b/importer/handlers/common/raster.py @@ -312,7 +312,7 @@ def create_geonode_resource( alternate: str, execution_id: str, resource_type: Dataset = Dataset, - files=None, + asset=None, ): """ Base function to create the resource into geonode. Each handler can specify @@ -335,6 +335,7 @@ def create_geonode_resource( logger.warning( f"The dataset required {alternate} does not exists, but an overwrite is required, the resource will be created" ) + saved_dataset = resource_manager.create( None, resource_type=resource_type, @@ -346,16 +347,7 @@ def create_geonode_resource( dirty_state=True, title=layer_name, owner=_exec.user, - extension=self.supported_file_extension_config["id"], - data_title="Original", - data_type=self.supported_file_extension_config["label"], - link_type="uploaded", # should be in geonode.base.enumerations.LINK_TYPES - files=list( - set( - list(_exec.input_params.get("files", {}).values()) - or list(files) - ) - ), + asset=asset, ), ) @@ -377,7 +369,7 @@ def overwrite_geonode_resource( alternate: str, execution_id: str, resource_type: Dataset = Dataset, - files=None, + asset=None, ): dataset = resource_type.objects.filter(alternate__icontains=alternate) @@ -405,7 +397,7 @@ def overwrite_geonode_resource( f"The dataset required {alternate} does not exists, but an overwrite is required, the resource will be created" ) return self.create_geonode_resource( - layer_name, alternate, execution_id, resource_type, files + layer_name, alternate, execution_id, resource_type, asset ) elif not dataset.exists() and not _overwrite: logger.warning( @@ -487,9 +479,9 @@ def copy_geonode_resource( layer_name=data_to_update.get("title"), alternate=new_alternate, execution_id=str(_exec.exec_id), - files=kwargs.get("kwargs", {}) + asset=kwargs.get("kwargs", {}) .get("new_file_location", {}) - .get("files", []), + .get("asset", []), ) resource.refresh_from_db() return resource diff --git a/importer/handlers/common/tests_vector.py b/importer/handlers/common/tests_vector.py index a2fe8efb..300b09e9 100644 --- a/importer/handlers/common/tests_vector.py +++ b/importer/handlers/common/tests_vector.py @@ -328,7 +328,7 @@ def test_select_valid_layers(self): self.assertEqual(1, len(valid_layer)) self.assertEqual("mattia_test", valid_layer[0].GetName()) - @override_settings(MEDIA_ROOT='/tmp') + @override_settings(MEDIA_ROOT="/tmp") def test_perform_last_step(self): """ Output params in perform_last_step should return the detail_url and the ID diff --git a/importer/handlers/common/vector.py b/importer/handlers/common/vector.py index d6b6d3a0..e53a9610 100644 --- a/importer/handlers/common/vector.py +++ b/importer/handlers/common/vector.py @@ -2,7 +2,6 @@ from django.db import connections from importer.publisher import DataPublisher from importer.utils import call_rollback_function, find_key_recursively -from itertools import chain import json import logging import os @@ -225,11 +224,10 @@ def perform_last_step(execution_id): # getting all assets list assets = [get_default_asset(x.resource) for x in resources] # we need to loop and cancel one by one to activate the signal - # which delete the file from the filesystem + # that delete the file from the filesystem for asset in assets: asset.delete() - - + def extract_resource_to_publish( self, files, action, layer_name, alternate, **kwargs ): @@ -568,7 +566,7 @@ def create_geonode_resource( alternate: str, execution_id: str, resource_type: Dataset = Dataset, - files=None, + asset=None, ): """ Base function to create the resource into geonode. Each handler can specify @@ -591,6 +589,7 @@ def create_geonode_resource( logger.warning( f"The dataset required {alternate} does not exists, but an overwrite is required, the resource will be created" ) + saved_dataset = resource_manager.create( None, resource_type=resource_type, @@ -603,16 +602,7 @@ def create_geonode_resource( dirty_state=True, title=layer_name, owner=_exec.user, - data_title="Original", - data_type=self.supported_file_extension_config["label"], - extension=self.supported_file_extension_config["id"], - link_type="uploaded", # should be in geonode.base.enumerations.LINK_TYPES - files=list( - set( - list(_exec.input_params.get("files", {}).values()) - or list(files) - ) - ), + asset=asset, ), ) @@ -634,7 +624,7 @@ def overwrite_geonode_resource( alternate: str, execution_id: str, resource_type: Dataset = Dataset, - files=None, + asset=None, ): dataset = resource_type.objects.filter(alternate__icontains=alternate) @@ -647,7 +637,7 @@ def overwrite_geonode_resource( dataset = dataset.first() dataset = resource_manager.update( - dataset.uuid, instance=dataset, files=files + dataset.uuid, instance=dataset, files=asset.location ) self.handle_xml_file(dataset, _exec) @@ -663,7 +653,7 @@ def overwrite_geonode_resource( f"The dataset required {alternate} does not exists, but an overwrite is required, the resource will be created" ) return self.create_geonode_resource( - layer_name, alternate, execution_id, resource_type, files + layer_name, alternate, execution_id, resource_type, asset ) elif not dataset.exists() and not _overwrite: logger.warning( @@ -741,11 +731,12 @@ def copy_geonode_resource( new_alternate: str, **kwargs, ): + new_resource = self.create_geonode_resource( layer_name=data_to_update.get("title"), alternate=new_alternate, execution_id=str(_exec.exec_id), - files=[], + asset=get_default_asset(resource), ) copy_assets_and_links(resource, target=new_resource) new_resource.refresh_from_db() diff --git a/importer/handlers/csv/handler.py b/importer/handlers/csv/handler.py index d805883a..f1433ed2 100644 --- a/importer/handlers/csv/handler.py +++ b/importer/handlers/csv/handler.py @@ -243,10 +243,15 @@ def extract_resource_to_publish( return [ { "name": alternate or layer_name, - "crs": ( - self.identify_authority(_l) if _l.GetSpatialRef() else "EPSG:4326" - ), + "crs": (self.identify_authority(_l)), } for _l in layers if self.fixup_name(_l.GetName()) == layer_name ] + + def identify_authority(self, layer): + try: + authority_code = super().identify_authority(layer=layer) + return authority_code + except Exception: + return "EPSG:4326" diff --git a/importer/handlers/sld/tests.py b/importer/handlers/sld/tests.py index ace68be0..26f0eb99 100644 --- a/importer/handlers/sld/tests.py +++ b/importer/handlers/sld/tests.py @@ -24,7 +24,10 @@ def setUpClass(cls): cls.user, _ = get_user_model().objects.get_or_create(username="admin") cls.invalid_files = {"base_file": cls.invalid_sld, "sld_file": cls.invalid_sld} - cls.valid_files = {"base_file": "/tmp/test_sld.sld", "sld_file": "/tmp/test_sld.sld"} + cls.valid_files = { + "base_file": "/tmp/test_sld.sld", + "sld_file": "/tmp/test_sld.sld", + } cls.owner = get_user_model().objects.first() cls.layer = create_single_dataset(name="sld_dataset", owner=cls.owner) diff --git a/importer/handlers/xml/tests.py b/importer/handlers/xml/tests.py index 9262f7c8..8f51e3cc 100644 --- a/importer/handlers/xml/tests.py +++ b/importer/handlers/xml/tests.py @@ -23,7 +23,10 @@ def setUpClass(cls): shutil.copy(cls.valid_xml, "/tmp") cls.user, _ = get_user_model().objects.get_or_create(username="admin") cls.invalid_files = {"base_file": cls.invalid_xml, "xml_file": cls.invalid_xml} - cls.valid_files = {"base_file": "/tmp/test_xml.xml", "xml_file": "/tmp/test_xml.xml"} + cls.valid_files = { + "base_file": "/tmp/test_xml.xml", + "xml_file": "/tmp/test_xml.xml", + } cls.owner = get_user_model().objects.first() cls.layer = create_single_dataset(name="extruded_polygon", owner=cls.owner) diff --git a/importer/tests/end2end/test_end2end.py b/importer/tests/end2end/test_end2end.py index 0cc3001b..dd8de7ab 100644 --- a/importer/tests/end2end/test_end2end.py +++ b/importer/tests/end2end/test_end2end.py @@ -37,6 +37,7 @@ def setUpClass(cls) -> None: } cls.valid_kml = f"{project_dir}/tests/fixture/valid.kml" cls.valid_tif = f"{project_dir}/tests/fixture/test_grid.tif" + cls.valid_csv = f"{project_dir}/tests/fixture/valid.csv" cls.url = reverse("importer_upload") ogc_server_settings = OGC_Servers_Handler(settings.OGC_SERVER)["default"] @@ -189,6 +190,7 @@ def test_import_geopackage_with_no_crs_table(self): @mock.patch( "importer.handlers.common.vector.BaseVectorFileHandler._select_valid_layers" ) + @override_settings(MEDIA_ROOT="/tmp/", ASSET_ROOT="/tmp/") def test_import_geopackage_with_no_crs_table_should_raise_error_if_all_layer_are_invalid( self, _select_valid_layers ): @@ -258,6 +260,48 @@ def test_import_geojson_overwrite(self): self.cat.delete(layer) +class ImporterGCSVImportTest(BaseImporterEndToEndTest): + @mock.patch.dict(os.environ, {"GEONODE_GEODATABASE": "test_geonode_data"}) + @override_settings( + GEODATABASE_URL=f"{geourl.split('/geonode_data')[0]}/test_geonode_data" + ) + def test_import_geojson(self): + layer = self.cat.get_layer("geonode:valid") + if layer: + self.cat.delete(layer) + + payload = { + "base_file": open(self.valid_csv, "rb"), + } + initial_name = "valid" + self._assertimport(payload, initial_name) + layer = self.cat.get_layer("geonode:valid") + if layer: + self.cat.delete(layer) + + @mock.patch.dict(os.environ, {"GEONODE_GEODATABASE": "test_geonode_data"}) + @override_settings( + GEODATABASE_URL=f"{geourl.split('/geonode_data')[0]}/test_geonode_data" + ) + def test_import_csv_overwrite(self): + prev_dataset = create_single_dataset(name="valid") + + layer = self.cat.get_layer("geonode:valid") + if layer: + self.cat.delete(layer) + payload = { + "base_file": open(self.valid_csv, "rb"), + } + initial_name = "valid" + payload["overwrite_existing_layer"] = True + self._assertimport( + payload, initial_name, overwrite=True, last_update=prev_dataset.last_updated + ) + layer = self.cat.get_layer("geonode:valid") + if layer: + self.cat.delete(layer) + + class ImporterKMLImportTest(BaseImporterEndToEndTest): @mock.patch.dict(os.environ, {"GEONODE_GEODATABASE": "test_geonode_data"}) @override_settings( diff --git a/importer/tests/unit/test_task.py b/importer/tests/unit/test_task.py index 8d37b219..2bacc12c 100644 --- a/importer/tests/unit/test_task.py +++ b/importer/tests/unit/test_task.py @@ -24,6 +24,7 @@ from geonode.resource.enumerator import ExecutionRequestAction from geonode.base.models import ResourceBase from geonode.base.populate_test_data import create_single_dataset +from geonode.assets.handlers import asset_handler_registry from dynamic_models.models import ModelSchema, FieldSchema from dynamic_models.exceptions import DynamicModelError, InvalidFieldNameError from importer.models import ResourceHandlerInfo @@ -40,7 +41,19 @@ class TestCeleryTasks(ImporterBaseTestSupport): def setUp(self): self.user = get_user_model().objects.first() + self.existing_file = f"{project_dir}/tests/fixture/valid.gpkg" + self.asset_handler = asset_handler_registry.get_default_handler() + + self.asset = self.asset_handler.create( + title="Original", + owner=self.user, + description=None, + type="importer.handlers.gpkg.handler.GPKGFileHandler", + files=[self.existing_file], + clone_files=False, + ) + self.exec_id = orchestrator.create_execution_request( user=get_user_model().objects.get(username=self.user), func_name="dummy_func", @@ -50,6 +63,8 @@ def setUp(self): "files": {"base_file": self.existing_file}, # "overwrite_existing_layer": True, "store_spatial_files": True, + "asset_id": self.asset.id, + "asset_module_path": f"{self.asset.__module__}.{self.asset.__class__.__name__}", }, ) @@ -506,9 +521,6 @@ def test_import_metadata_should_work_as_expected(self): layer.refresh_from_db() self.assertEqual(layer.title, "test_dataset") - #verify that the original has been deleted - self.assertFalse(os.path.exists(xml_in_tmp)) - class TestDynamicModelSchema(TransactionImporterBaseTestSupport): databases = ("default", "datastore") From 253581b6cd52d91210e2285bc116c034920b3a89 Mon Sep 17 00:00:00 2001 From: Mattia Date: Mon, 17 Jun 2024 18:01:59 +0200 Subject: [PATCH 33/40] [Fixes #247] Introducing assets in geonode-importer --- importer/handlers/tiles3d/__init__.py | 0 importer/handlers/tiles3d/exceptions.py | 9 - importer/handlers/tiles3d/handler.py | 256 ---------------- importer/handlers/tiles3d/tests.py | 274 ------------------ importer/tests/fixture/3dtilesample/README.md | 2 - .../tests/fixture/3dtilesample/invalid.zip | 0 .../fixture/3dtilesample/invalid_tileset.json | 3 - .../tests/fixture/3dtilesample/tileset.json | 16 - .../3dtilesample/tileset_with_region.json | 21 -- .../fixture/3dtilesample/valid_3dtiles.zip | Bin 1333 -> 0 bytes 10 files changed, 581 deletions(-) delete mode 100644 importer/handlers/tiles3d/__init__.py delete mode 100644 importer/handlers/tiles3d/exceptions.py delete mode 100644 importer/handlers/tiles3d/handler.py delete mode 100644 importer/handlers/tiles3d/tests.py delete mode 100644 importer/tests/fixture/3dtilesample/README.md delete mode 100644 importer/tests/fixture/3dtilesample/invalid.zip delete mode 100755 importer/tests/fixture/3dtilesample/invalid_tileset.json delete mode 100755 importer/tests/fixture/3dtilesample/tileset.json delete mode 100644 importer/tests/fixture/3dtilesample/tileset_with_region.json delete mode 100644 importer/tests/fixture/3dtilesample/valid_3dtiles.zip diff --git a/importer/handlers/tiles3d/__init__.py b/importer/handlers/tiles3d/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/importer/handlers/tiles3d/exceptions.py b/importer/handlers/tiles3d/exceptions.py deleted file mode 100644 index f8108c64..00000000 --- a/importer/handlers/tiles3d/exceptions.py +++ /dev/null @@ -1,9 +0,0 @@ -from rest_framework.exceptions import APIException -from rest_framework import status - - -class Invalid3DTilesException(APIException): - status_code = status.HTTP_400_BAD_REQUEST - default_detail = "The 3dtiles provided is invalid" - default_code = "invalid_3dtiles" - category = "importer" diff --git a/importer/handlers/tiles3d/handler.py b/importer/handlers/tiles3d/handler.py deleted file mode 100644 index 06e146d1..00000000 --- a/importer/handlers/tiles3d/handler.py +++ /dev/null @@ -1,256 +0,0 @@ -import json -import logging -import os -from pathlib import Path -import math -from geonode.layers.models import Dataset -from geonode.resource.enumerator import ExecutionRequestAction as exa -from geonode.upload.utils import UploadLimitValidator -from importer.orchestrator import orchestrator -from importer.celery_tasks import import_orchestrator -from importer.handlers.common.vector import BaseVectorFileHandler -from importer.handlers.utils import create_alternate, should_be_imported -from importer.utils import ImporterRequestAction as ira -from geonode.base.models import ResourceBase -from importer.handlers.tiles3d.exceptions import Invalid3DTilesException - -logger = logging.getLogger(__name__) - - -class Tiles3DFileHandler(BaseVectorFileHandler): - """ - Handler to import 3Dtiles files into GeoNode data db - It must provide the task_lists required to comple the upload - """ - - ACTIONS = { - exa.IMPORT.value: ( - "start_import", - "importer.import_resource", - "importer.create_geonode_resource", - ), - exa.COPY.value: ( - "start_copy", - "importer.copy_geonode_resource", - ), - ira.ROLLBACK.value: ( - "start_rollback", - "importer.rollback", - ), - } - - @property - def supported_file_extension_config(self): - return { - "id": "3dtiles", - "label": "3D Tiles", - "format": "vector", - "ext": ["json"], - "optional": ["xml", "sld"], - } - - @staticmethod - def can_handle(_data) -> bool: - """ - This endpoint will return True or False if with the info provided - the handler is able to handle the file or not - """ - base = _data.get("base_file") - if not base: - return False - ext = base.split(".")[-1] if isinstance(base, str) else base.name.split(".")[-1] - input_filename = os.path.basename(base if isinstance(base, str) else base.name) - if ext in ["json"] and "tileset.json" in input_filename: - return True - return False - - @staticmethod - def is_valid(files, user): - """ - Define basic validation steps: - """ - # calling base validation checks - BaseVectorFileHandler.is_valid(files, user) - # getting the upload limit validation - upload_validator = UploadLimitValidator(user) - upload_validator.validate_parallelism_limit_per_user() - - _file = files.get("base_file") - if not _file: - raise Invalid3DTilesException("base file is not provided") - - filename = os.path.basename(_file) - - if len(filename.split(".")) > 2: - # means that there is a dot other than the one needed for the extension - # if we keep it ogr2ogr raise an error, better to remove it - raise Invalid3DTilesException( - "Please remove the additional dots in the filename" - ) - - try: - with open(_file, "r") as _readed_file: - _file = json.loads(_readed_file.read()) - # required key described in the specification of 3dtiles - # https://docs.ogc.org/cs/22-025r4/22-025r4.html#toc92 - is_valid = all( - key in _file.keys() for key in ("asset", "geometricError", "root") - ) - - if not is_valid: - raise Invalid3DTilesException( - "The provided 3DTiles is not valid, some of the mandatory keys are missing. Mandatory keys are: 'asset', 'geometricError', 'root'" - ) - - # if the keys are there, let's check if the mandatory child are there too - asset = _file.get("asset", {}).get("version", None) - if not asset: - raise Invalid3DTilesException( - "The mandatory 'version' for the key 'asset' is missing" - ) - volume = _file.get("root", {}).get("boundingVolume", None) - if not volume: - raise Invalid3DTilesException( - "The mandatory 'boundingVolume' for the key 'root' is missing" - ) - - error = _file.get("root", {}).get("geometricError", None) - if error is None: - raise Invalid3DTilesException( - "The mandatory 'geometricError' for the key 'root' is missing" - ) - - except Exception as e: - raise Invalid3DTilesException(e) - - return True - - @staticmethod - def extract_params_from_data(_data, action=None): - """ - Remove from the _data the params that needs to save into the executionRequest object - all the other are returned - """ - if action == exa.COPY.value: - title = json.loads(_data.get("defaults")) - return {"title": title.pop("title")}, _data - - return { - "skip_existing_layers": _data.pop("skip_existing_layers", "False"), - "overwrite_existing_layer": _data.pop("overwrite_existing_layer", "False"), - "store_spatial_file": _data.pop("store_spatial_files", "True"), - "source": _data.pop("source", "upload"), - "original_zip_name": _data.pop("original_zip_name", None), - }, _data - - def import_resource(self, files: dict, execution_id: str, **kwargs) -> str: - logger.info("Total number of layers available: 1") - - _exec = self._get_execution_request_object(execution_id) - - _input = {**_exec.input_params, **{"total_layers": 1}} - - orchestrator.update_execution_request_status( - execution_id=str(execution_id), input_params=_input - ) - filename = ( - _exec.input_params.get("original_zip_name") - or Path(files.get("base_file")).stem - ) - # start looping on the layers available - layer_name = self.fixup_name(filename) - should_be_overwritten = _exec.input_params.get("overwrite_existing_layer") - # should_be_imported check if the user+layername already exists or not - if should_be_imported( - layer_name, - _exec.user, - skip_existing_layer=_exec.input_params.get("skip_existing_layer"), - overwrite_existing_layer=should_be_overwritten, - ): - - user_datasets = ResourceBase.objects.filter( - owner=_exec.user, alternate=layer_name - ) - - dataset_exists = user_datasets.exists() - - if dataset_exists and should_be_overwritten: - layer_name, alternate = ( - layer_name, - user_datasets.first().alternate.split(":")[-1], - ) - elif not dataset_exists: - alternate = layer_name - else: - alternate = create_alternate(layer_name, execution_id) - - import_orchestrator.apply_async( - ( - files, - execution_id, - str(self), - "importer.import_resource", - layer_name, - alternate, - exa.IMPORT.value, - ) - ) - return layer_name, alternate, execution_id - - def create_geonode_resource( - self, - layer_name: str, - alternate: str, - execution_id: str, - resource_type: Dataset = ..., - asset=None, - ): - # we want just the tileset.json as location of the asset - asset.location = [path for path in asset.location if "tileset.json" in path] - asset.save() - - resource = super().create_geonode_resource( - layer_name, alternate, execution_id, ResourceBase, asset - ) - - # fixing-up bbox for the 3dtile object - js_file = None - with open(asset.location[0]) as _file: - js_file = json.loads(_file.read()) - - if not js_file: - return resource - - # checking if the region is inside the json file - region = js_file.get("root", {}).get("boundingVolume", {}).get("region", None) - if not region: - logger.info( - f"No region found, the BBOX will not be updated for 3dtiles: {resource.title}" - ) - return resource - west, south, east, nord = region[:4] - # [xmin, ymin, xmax, ymax] - resource.set_bbox_polygon( - bbox=[ - math.degrees(west), - math.degrees(south), - math.degrees(east), - math.degrees(nord), - ], - srid="EPSG:4326", - ) - - return resource - - def generate_resource_payload(self, layer_name, alternate, asset, _exec, workspace): - return dict( - resource_type="dataset", - subtype="3dtiles", - dirty_state=True, - title=layer_name, - owner=_exec.user, - asset=asset, - link_type="uploaded", - extension="3dtiles", - alternate=alternate, - ) diff --git a/importer/handlers/tiles3d/tests.py b/importer/handlers/tiles3d/tests.py deleted file mode 100644 index 2fcd2834..00000000 --- a/importer/handlers/tiles3d/tests.py +++ /dev/null @@ -1,274 +0,0 @@ -import json -import os -import shutil -from django.test import TestCase -from importer.handlers.tiles3d.exceptions import Invalid3DTilesException -from importer.handlers.tiles3d.handler import Tiles3DFileHandler -from django.contrib.auth import get_user_model -from importer import project_dir -from importer.orchestrator import orchestrator -from geonode.upload.models import UploadParallelismLimit -from geonode.upload.api.exceptions import UploadParallelismLimitException -from geonode.base.populate_test_data import create_single_dataset -from osgeo import ogr -from geonode.assets.handlers import asset_handler_registry - - -class TestTiles3DFileHandler(TestCase): - databases = ("default", "datastore") - - @classmethod - def setUpClass(cls): - super().setUpClass() - cls.handler = Tiles3DFileHandler() - cls.valid_3dtile = f"{project_dir}/tests/fixture/3dtilesample/valid_3dtiles.zip" - cls.valid_tileset = f"{project_dir}/tests/fixture/3dtilesample/tileset.json" - cls.valid_tileset_with_region = ( - f"{project_dir}/tests/fixture/3dtilesample/tileset_with_region.json" - ) - cls.invalid_tileset = ( - f"{project_dir}/tests/fixture/3dtilesample/invalid_tileset.json" - ) - cls.invalid_3dtile = f"{project_dir}/tests/fixture/3dtilesample/invalid.zip" - cls.user, _ = get_user_model().objects.get_or_create(username="admin") - cls.invalid_files = {"base_file": cls.invalid_3dtile} - cls.valid_files = {"base_file": cls.valid_3dtile} - cls.owner = get_user_model().objects.exclude(username="AnonymousUser").first() - cls.layer = create_single_dataset( - name="urban_forestry_street_tree_benefits_epsg_26985", owner=cls.owner - ) - cls.asset_handler = asset_handler_registry.get_default_handler() - - def test_task_list_is_the_expected_one(self): - expected = ( - "start_import", - "importer.import_resource", - "importer.create_geonode_resource", - ) - self.assertEqual(len(self.handler.ACTIONS["import"]), 3) - self.assertTupleEqual(expected, self.handler.ACTIONS["import"]) - - def test_task_list_is_the_expected_one_copy(self): - expected = ( - "start_copy", - "importer.copy_geonode_resource", - ) - self.assertEqual(len(self.handler.ACTIONS["copy"]), 2) - self.assertTupleEqual(expected, self.handler.ACTIONS["copy"]) - - def test_is_valid_should_raise_exception_if_the_parallelism_is_met(self): - parallelism, created = UploadParallelismLimit.objects.get_or_create( - slug="default_max_parallel_uploads" - ) - old_value = parallelism.max_number - try: - UploadParallelismLimit.objects.filter( - slug="default_max_parallel_uploads" - ).update(max_number=0) - - with self.assertRaises(UploadParallelismLimitException): - self.handler.is_valid(files=self.valid_files, user=self.user) - - finally: - parallelism.max_number = old_value - parallelism.save() - - def test_is_valid_should_pass_with_valid_3dtiles(self): - self.handler.is_valid(files={"base_file": self.valid_tileset}, user=self.user) - - def test_is_valid_should_raise_exception_if_no_basefile_is_supplied(self): - data = {"base_file": ""} - with self.assertRaises(Invalid3DTilesException) as _exc: - self.handler.is_valid(files=data, user=self.user) - - self.assertIsNotNone(_exc) - self.assertTrue("base file is not provided" in str(_exc.exception.detail)) - - def test_extract_params_from_data(self): - actual, _data = self.handler.extract_params_from_data( - _data={"defaults": '{"title":"title_of_the_cloned_resource"}'}, - action="copy", - ) - - self.assertEqual(actual, {"title": "title_of_the_cloned_resource"}) - - def test_is_valid_should_raise_exception_if_the_3dtiles_is_invalid(self): - data = { - "base_file": "/using/double/dot/in/the/name/is/an/error/file.invalid.json" - } - with self.assertRaises(Invalid3DTilesException) as _exc: - self.handler.is_valid(files=data, user=self.user) - - self.assertIsNotNone(_exc) - self.assertTrue( - "Please remove the additional dots in the filename" - in str(_exc.exception.detail) - ) - - def test_is_valid_should_raise_exception_if_the_3dtiles_is_invalid_format(self): - with self.assertRaises(Invalid3DTilesException) as _exc: - self.handler.is_valid( - files={"base_file": self.invalid_tileset}, user=self.user - ) - - self.assertIsNotNone(_exc) - self.assertTrue( - "The provided 3DTiles is not valid, some of the mandatory keys are missing. Mandatory keys are: 'asset', 'geometricError', 'root'" - in str(_exc.exception.detail) - ) - - def test_validate_should_raise_exception_for_invalid_asset_key(self): - _json = { - "asset": {"invalid_key": ""}, - "geometricError": 1.0, - "root": {"boundingVolume": {"box": []}, "geometricError": 0.0}, - } - _path = "/tmp/tileset.json" - with open(_path, "w") as _f: - _f.write(json.dumps(_json)) - with self.assertRaises(Invalid3DTilesException) as _exc: - self.handler.is_valid(files={"base_file": _path}, user=self.user) - - self.assertIsNotNone(_exc) - self.assertTrue( - "The mandatory 'version' for the key 'asset' is missing" - in str(_exc.exception.detail) - ) - os.remove(_path) - - def test_validate_should_raise_exception_for_invalid_root_boundingVolume(self): - _json = { - "asset": {"version": "1.1"}, - "geometricError": 1.0, - "root": {"foo": {"box": []}, "geometricError": 0.0}, - } - _path = "/tmp/tileset.json" - with open(_path, "w") as _f: - _f.write(json.dumps(_json)) - with self.assertRaises(Invalid3DTilesException) as _exc: - self.handler.is_valid(files={"base_file": _path}, user=self.user) - - self.assertIsNotNone(_exc) - self.assertTrue( - "The mandatory 'boundingVolume' for the key 'root' is missing" - in str(_exc.exception.detail) - ) - os.remove(_path) - - def test_validate_should_raise_exception_for_invalid_root_geometricError(self): - _json = { - "asset": {"version": "1.1"}, - "geometricError": 1.0, - "root": {"boundingVolume": {"box": []}, "foo": 0.0}, - } - _path = "/tmp/tileset.json" - with open(_path, "w") as _f: - _f.write(json.dumps(_json)) - with self.assertRaises(Invalid3DTilesException) as _exc: - self.handler.is_valid(files={"base_file": _path}, user=self.user) - - self.assertIsNotNone(_exc) - self.assertTrue( - "The mandatory 'geometricError' for the key 'root' is missing" - in str(_exc.exception.detail) - ) - os.remove(_path) - - def test_get_ogr2ogr_driver_should_return_the_expected_driver(self): - expected = ogr.GetDriverByName("3dtiles") - actual = self.handler.get_ogr2ogr_driver() - self.assertEqual(type(expected), type(actual)) - - def test_can_handle_should_return_true_for_3dtiles(self): - actual = self.handler.can_handle({"base_file": self.valid_tileset}) - self.assertTrue(actual) - - def test_can_handle_should_return_false_for_other_files(self): - actual = self.handler.can_handle({"base_file": "random.gpkg"}) - self.assertFalse(actual) - - def test_can_handle_should_return_false_if_no_basefile(self): - actual = self.handler.can_handle({"base_file": ""}) - self.assertFalse(actual) - - def test_supported_file_extension_config(self): - """ - should return the expected value - """ - expected = { - "id": "3dtiles", - "label": "3D Tiles", - "format": "vector", - "ext": ["json"], - "optional": ["xml", "sld"], - } - actual = self.handler.supported_file_extension_config - self.assertDictEqual(actual, expected) - - def test_generate_resource_payload(self): - exec_id = orchestrator.create_execution_request( - user=self.owner, - func_name="funct1", - step="step", - input_params={"files": self.valid_files, "skip_existing_layer": True}, - ) - _exec_obj = orchestrator.get_execution_object(exec_id) - expected = dict( - resource_type="dataset", - subtype="3dtiles", - dirty_state=True, - title="Layer name", - owner=self.owner, - asset="asset", - link_type="uploaded", - extension="3dtiles", - alternate="alternate" - ) - - actual = self.handler.generate_resource_payload( - "Layer name", "alternate", "asset", _exec_obj, None - ) - self.assertSetEqual(set(list(actual.keys())), set(list(expected.keys()))) - self.assertDictEqual(actual, expected) - - def test_create_geonode_resource_validate_bbox_with_region(self): - shutil.copy(self.valid_tileset_with_region, "/tmp/tileset.json") - - exec_id = orchestrator.create_execution_request( - user=self.owner, - func_name="funct1", - step="step", - input_params={ - "files": {"base_file": "/tmp/tileset.json"}, - "skip_existing_layer": True, - }, - ) - - asset = self.asset_handler.create( - title="Original", - owner=self.owner, - description=None, - type=str(self.handler), - files=["/tmp/tileset.json"], - clone_files=False, - ) - - resource = self.handler.create_geonode_resource( - "layername", - "layeralternate", - execution_id=exec_id, - resource_type="ResourceBase", - asset=asset, - ) - - # validate bbox - default_bbox = [-180.0, 180.0, -90.0, 90.0, "EPSG:4326"] - self.assertFalse(resource.bbox == default_bbox) - expected = [ - -75.6144410959485, - -75.60974751970046, - 40.040721313841274, - 40.04433990901052, - "EPSG:4326", - ] - self.assertTrue(resource.bbox == expected) diff --git a/importer/tests/fixture/3dtilesample/README.md b/importer/tests/fixture/3dtilesample/README.md deleted file mode 100644 index cd862833..00000000 --- a/importer/tests/fixture/3dtilesample/README.md +++ /dev/null @@ -1,2 +0,0 @@ -Example provided by CensiumGS -https://github.com/CesiumGS/3d-tiles-samples \ No newline at end of file diff --git a/importer/tests/fixture/3dtilesample/invalid.zip b/importer/tests/fixture/3dtilesample/invalid.zip deleted file mode 100644 index e69de29b..00000000 diff --git a/importer/tests/fixture/3dtilesample/invalid_tileset.json b/importer/tests/fixture/3dtilesample/invalid_tileset.json deleted file mode 100755 index a76162d5..00000000 --- a/importer/tests/fixture/3dtilesample/invalid_tileset.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "asset": "value" -} diff --git a/importer/tests/fixture/3dtilesample/tileset.json b/importer/tests/fixture/3dtilesample/tileset.json deleted file mode 100755 index e9d6cbf2..00000000 --- a/importer/tests/fixture/3dtilesample/tileset.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "asset" : { - "version" : "1.1" - }, - "geometricError" : 1.0, - "root" : { - "boundingVolume" : { - "box" : [ 0.5, 0.5, 1.0, 0.5, 0.0, 0.0, 0.0, -0.5, 0.0, 0.0, 0.0, 1.0 ] - }, - "geometricError" : 0.0, - "refine" : "REPLACE", - "content" : { - "uri" : "0_0_0-1_1_2.glb" - } - } - } diff --git a/importer/tests/fixture/3dtilesample/tileset_with_region.json b/importer/tests/fixture/3dtilesample/tileset_with_region.json deleted file mode 100644 index bfdb5f53..00000000 --- a/importer/tests/fixture/3dtilesample/tileset_with_region.json +++ /dev/null @@ -1,21 +0,0 @@ -{ - "asset": { - "version": "1.0" - }, - "geometricError": 70, - "root": { - "boundingVolume": { - "region": [ - -1.3197209591796106, - 0.6988424218, - -1.3196390408203893, - 0.6989055782, - 0, - 20 - ] - }, - "geometricError": 70, - "refine": "ADD" - } -} - \ No newline at end of file diff --git a/importer/tests/fixture/3dtilesample/valid_3dtiles.zip b/importer/tests/fixture/3dtilesample/valid_3dtiles.zip deleted file mode 100644 index 93955274f1e22d02cddd40aa8bbc2df9a27403ec..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 1333 zcmWIWW@Zs#U|`^2SUYK7#EdHzrRhuz3=-@N4Ezi-36|UTN~eh;(R0rr}y?fB3)&wVGi3IzAt@r zkkhX~ROpGsEm5ai_1E@an15xXVZyqym06jK^54w-`7G$risGuv20i8MZHf2HdNyyn zJxAfqZxvtRX^Sph=lta6=x~Eo^?u+Qm2VuW5gj^zo(NL&oKkQktBEgxe|)DO`x;xO{>|M>6Ht9RABe zdg4L%#Q8YS}>O; zPikN6$v&sGtzy>6l)8iG9|y0^>`?xvY~FKo=hBiNOC`!3-)Yn=_)@LF@9<9hPr%E5 zhj|S)VLw)0n%}sGp>o{^@1^^hDw#h;J>2dUZyPQj{&Cm7q~L2Zr*B0^e%8(J-+0z- z`|XbX%KJs{ZJqP=$LnuZe9{%icmCR`elP61SWWEi`m@RNW4=BA!5aWiuZMu?wed>w z;?=;^8^p-KzynOLC7C&?#i=EFS;hHz;KaHHm{36&EwSeI8FC*o5NQ4WudAR~TIk?W z-zBq~Ep~K>L??4@vD~ptY0AIy$y+9B`964XuX@k#ww9gj-`53;-RR>@EI;KPxcp)T zugY|RFNSj;Z9Nngc`4Md<%#j6?wiU-?k^Ln{(NECzJ$x$vocvfzn!WrTr=D6aQM+s z9rtZ&yG(YUtu?-r%J&&|KooZ+t-IL+<5AS{h4_s9o>ii`iO11@nM&y zqMY9&naTVJ4?hEXSoNj4qbtzEUO>zV^l*@?ql>SrUTzB5w;zB$1z|Mb7O(XXyqCx`b9te2b2%<3x4pvCPf#xz~LowqMVkj_b8QwN- y0G3Lqu?sX2OUxphh#6oA6PGkD1eRP#!3Q)BOV9;)v$BEw!wiJqfV3tHhz9^W6%+#i From 095fc8977d63088d71ad748fd3aa785f87a71476 Mon Sep 17 00:00:00 2001 From: Mattia Date: Mon, 17 Jun 2024 18:13:02 +0200 Subject: [PATCH 34/40] [Fixes #246] Introducing 3dtiles in geonode-importer --- importer/handlers/tiles3d/__init__.py | 0 importer/handlers/tiles3d/exceptions.py | 9 + importer/handlers/tiles3d/handler.py | 256 ++++++++++++++++ importer/handlers/tiles3d/tests.py | 274 ++++++++++++++++++ importer/tests/fixture/3dtilesample/README.md | 2 + .../tests/fixture/3dtilesample/invalid.zip | 0 .../fixture/3dtilesample/invalid_tileset.json | 3 + .../tests/fixture/3dtilesample/tileset.json | 16 + .../3dtilesample/tileset_with_region.json | 21 ++ .../fixture/3dtilesample/valid_3dtiles.zip | Bin 0 -> 1333 bytes 10 files changed, 581 insertions(+) create mode 100755 importer/handlers/tiles3d/__init__.py create mode 100755 importer/handlers/tiles3d/exceptions.py create mode 100755 importer/handlers/tiles3d/handler.py create mode 100755 importer/handlers/tiles3d/tests.py create mode 100755 importer/tests/fixture/3dtilesample/README.md create mode 100755 importer/tests/fixture/3dtilesample/invalid.zip create mode 100755 importer/tests/fixture/3dtilesample/invalid_tileset.json create mode 100755 importer/tests/fixture/3dtilesample/tileset.json create mode 100755 importer/tests/fixture/3dtilesample/tileset_with_region.json create mode 100755 importer/tests/fixture/3dtilesample/valid_3dtiles.zip diff --git a/importer/handlers/tiles3d/__init__.py b/importer/handlers/tiles3d/__init__.py new file mode 100755 index 00000000..e69de29b diff --git a/importer/handlers/tiles3d/exceptions.py b/importer/handlers/tiles3d/exceptions.py new file mode 100755 index 00000000..f8108c64 --- /dev/null +++ b/importer/handlers/tiles3d/exceptions.py @@ -0,0 +1,9 @@ +from rest_framework.exceptions import APIException +from rest_framework import status + + +class Invalid3DTilesException(APIException): + status_code = status.HTTP_400_BAD_REQUEST + default_detail = "The 3dtiles provided is invalid" + default_code = "invalid_3dtiles" + category = "importer" diff --git a/importer/handlers/tiles3d/handler.py b/importer/handlers/tiles3d/handler.py new file mode 100755 index 00000000..06e146d1 --- /dev/null +++ b/importer/handlers/tiles3d/handler.py @@ -0,0 +1,256 @@ +import json +import logging +import os +from pathlib import Path +import math +from geonode.layers.models import Dataset +from geonode.resource.enumerator import ExecutionRequestAction as exa +from geonode.upload.utils import UploadLimitValidator +from importer.orchestrator import orchestrator +from importer.celery_tasks import import_orchestrator +from importer.handlers.common.vector import BaseVectorFileHandler +from importer.handlers.utils import create_alternate, should_be_imported +from importer.utils import ImporterRequestAction as ira +from geonode.base.models import ResourceBase +from importer.handlers.tiles3d.exceptions import Invalid3DTilesException + +logger = logging.getLogger(__name__) + + +class Tiles3DFileHandler(BaseVectorFileHandler): + """ + Handler to import 3Dtiles files into GeoNode data db + It must provide the task_lists required to comple the upload + """ + + ACTIONS = { + exa.IMPORT.value: ( + "start_import", + "importer.import_resource", + "importer.create_geonode_resource", + ), + exa.COPY.value: ( + "start_copy", + "importer.copy_geonode_resource", + ), + ira.ROLLBACK.value: ( + "start_rollback", + "importer.rollback", + ), + } + + @property + def supported_file_extension_config(self): + return { + "id": "3dtiles", + "label": "3D Tiles", + "format": "vector", + "ext": ["json"], + "optional": ["xml", "sld"], + } + + @staticmethod + def can_handle(_data) -> bool: + """ + This endpoint will return True or False if with the info provided + the handler is able to handle the file or not + """ + base = _data.get("base_file") + if not base: + return False + ext = base.split(".")[-1] if isinstance(base, str) else base.name.split(".")[-1] + input_filename = os.path.basename(base if isinstance(base, str) else base.name) + if ext in ["json"] and "tileset.json" in input_filename: + return True + return False + + @staticmethod + def is_valid(files, user): + """ + Define basic validation steps: + """ + # calling base validation checks + BaseVectorFileHandler.is_valid(files, user) + # getting the upload limit validation + upload_validator = UploadLimitValidator(user) + upload_validator.validate_parallelism_limit_per_user() + + _file = files.get("base_file") + if not _file: + raise Invalid3DTilesException("base file is not provided") + + filename = os.path.basename(_file) + + if len(filename.split(".")) > 2: + # means that there is a dot other than the one needed for the extension + # if we keep it ogr2ogr raise an error, better to remove it + raise Invalid3DTilesException( + "Please remove the additional dots in the filename" + ) + + try: + with open(_file, "r") as _readed_file: + _file = json.loads(_readed_file.read()) + # required key described in the specification of 3dtiles + # https://docs.ogc.org/cs/22-025r4/22-025r4.html#toc92 + is_valid = all( + key in _file.keys() for key in ("asset", "geometricError", "root") + ) + + if not is_valid: + raise Invalid3DTilesException( + "The provided 3DTiles is not valid, some of the mandatory keys are missing. Mandatory keys are: 'asset', 'geometricError', 'root'" + ) + + # if the keys are there, let's check if the mandatory child are there too + asset = _file.get("asset", {}).get("version", None) + if not asset: + raise Invalid3DTilesException( + "The mandatory 'version' for the key 'asset' is missing" + ) + volume = _file.get("root", {}).get("boundingVolume", None) + if not volume: + raise Invalid3DTilesException( + "The mandatory 'boundingVolume' for the key 'root' is missing" + ) + + error = _file.get("root", {}).get("geometricError", None) + if error is None: + raise Invalid3DTilesException( + "The mandatory 'geometricError' for the key 'root' is missing" + ) + + except Exception as e: + raise Invalid3DTilesException(e) + + return True + + @staticmethod + def extract_params_from_data(_data, action=None): + """ + Remove from the _data the params that needs to save into the executionRequest object + all the other are returned + """ + if action == exa.COPY.value: + title = json.loads(_data.get("defaults")) + return {"title": title.pop("title")}, _data + + return { + "skip_existing_layers": _data.pop("skip_existing_layers", "False"), + "overwrite_existing_layer": _data.pop("overwrite_existing_layer", "False"), + "store_spatial_file": _data.pop("store_spatial_files", "True"), + "source": _data.pop("source", "upload"), + "original_zip_name": _data.pop("original_zip_name", None), + }, _data + + def import_resource(self, files: dict, execution_id: str, **kwargs) -> str: + logger.info("Total number of layers available: 1") + + _exec = self._get_execution_request_object(execution_id) + + _input = {**_exec.input_params, **{"total_layers": 1}} + + orchestrator.update_execution_request_status( + execution_id=str(execution_id), input_params=_input + ) + filename = ( + _exec.input_params.get("original_zip_name") + or Path(files.get("base_file")).stem + ) + # start looping on the layers available + layer_name = self.fixup_name(filename) + should_be_overwritten = _exec.input_params.get("overwrite_existing_layer") + # should_be_imported check if the user+layername already exists or not + if should_be_imported( + layer_name, + _exec.user, + skip_existing_layer=_exec.input_params.get("skip_existing_layer"), + overwrite_existing_layer=should_be_overwritten, + ): + + user_datasets = ResourceBase.objects.filter( + owner=_exec.user, alternate=layer_name + ) + + dataset_exists = user_datasets.exists() + + if dataset_exists and should_be_overwritten: + layer_name, alternate = ( + layer_name, + user_datasets.first().alternate.split(":")[-1], + ) + elif not dataset_exists: + alternate = layer_name + else: + alternate = create_alternate(layer_name, execution_id) + + import_orchestrator.apply_async( + ( + files, + execution_id, + str(self), + "importer.import_resource", + layer_name, + alternate, + exa.IMPORT.value, + ) + ) + return layer_name, alternate, execution_id + + def create_geonode_resource( + self, + layer_name: str, + alternate: str, + execution_id: str, + resource_type: Dataset = ..., + asset=None, + ): + # we want just the tileset.json as location of the asset + asset.location = [path for path in asset.location if "tileset.json" in path] + asset.save() + + resource = super().create_geonode_resource( + layer_name, alternate, execution_id, ResourceBase, asset + ) + + # fixing-up bbox for the 3dtile object + js_file = None + with open(asset.location[0]) as _file: + js_file = json.loads(_file.read()) + + if not js_file: + return resource + + # checking if the region is inside the json file + region = js_file.get("root", {}).get("boundingVolume", {}).get("region", None) + if not region: + logger.info( + f"No region found, the BBOX will not be updated for 3dtiles: {resource.title}" + ) + return resource + west, south, east, nord = region[:4] + # [xmin, ymin, xmax, ymax] + resource.set_bbox_polygon( + bbox=[ + math.degrees(west), + math.degrees(south), + math.degrees(east), + math.degrees(nord), + ], + srid="EPSG:4326", + ) + + return resource + + def generate_resource_payload(self, layer_name, alternate, asset, _exec, workspace): + return dict( + resource_type="dataset", + subtype="3dtiles", + dirty_state=True, + title=layer_name, + owner=_exec.user, + asset=asset, + link_type="uploaded", + extension="3dtiles", + alternate=alternate, + ) diff --git a/importer/handlers/tiles3d/tests.py b/importer/handlers/tiles3d/tests.py new file mode 100755 index 00000000..2fcd2834 --- /dev/null +++ b/importer/handlers/tiles3d/tests.py @@ -0,0 +1,274 @@ +import json +import os +import shutil +from django.test import TestCase +from importer.handlers.tiles3d.exceptions import Invalid3DTilesException +from importer.handlers.tiles3d.handler import Tiles3DFileHandler +from django.contrib.auth import get_user_model +from importer import project_dir +from importer.orchestrator import orchestrator +from geonode.upload.models import UploadParallelismLimit +from geonode.upload.api.exceptions import UploadParallelismLimitException +from geonode.base.populate_test_data import create_single_dataset +from osgeo import ogr +from geonode.assets.handlers import asset_handler_registry + + +class TestTiles3DFileHandler(TestCase): + databases = ("default", "datastore") + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.handler = Tiles3DFileHandler() + cls.valid_3dtile = f"{project_dir}/tests/fixture/3dtilesample/valid_3dtiles.zip" + cls.valid_tileset = f"{project_dir}/tests/fixture/3dtilesample/tileset.json" + cls.valid_tileset_with_region = ( + f"{project_dir}/tests/fixture/3dtilesample/tileset_with_region.json" + ) + cls.invalid_tileset = ( + f"{project_dir}/tests/fixture/3dtilesample/invalid_tileset.json" + ) + cls.invalid_3dtile = f"{project_dir}/tests/fixture/3dtilesample/invalid.zip" + cls.user, _ = get_user_model().objects.get_or_create(username="admin") + cls.invalid_files = {"base_file": cls.invalid_3dtile} + cls.valid_files = {"base_file": cls.valid_3dtile} + cls.owner = get_user_model().objects.exclude(username="AnonymousUser").first() + cls.layer = create_single_dataset( + name="urban_forestry_street_tree_benefits_epsg_26985", owner=cls.owner + ) + cls.asset_handler = asset_handler_registry.get_default_handler() + + def test_task_list_is_the_expected_one(self): + expected = ( + "start_import", + "importer.import_resource", + "importer.create_geonode_resource", + ) + self.assertEqual(len(self.handler.ACTIONS["import"]), 3) + self.assertTupleEqual(expected, self.handler.ACTIONS["import"]) + + def test_task_list_is_the_expected_one_copy(self): + expected = ( + "start_copy", + "importer.copy_geonode_resource", + ) + self.assertEqual(len(self.handler.ACTIONS["copy"]), 2) + self.assertTupleEqual(expected, self.handler.ACTIONS["copy"]) + + def test_is_valid_should_raise_exception_if_the_parallelism_is_met(self): + parallelism, created = UploadParallelismLimit.objects.get_or_create( + slug="default_max_parallel_uploads" + ) + old_value = parallelism.max_number + try: + UploadParallelismLimit.objects.filter( + slug="default_max_parallel_uploads" + ).update(max_number=0) + + with self.assertRaises(UploadParallelismLimitException): + self.handler.is_valid(files=self.valid_files, user=self.user) + + finally: + parallelism.max_number = old_value + parallelism.save() + + def test_is_valid_should_pass_with_valid_3dtiles(self): + self.handler.is_valid(files={"base_file": self.valid_tileset}, user=self.user) + + def test_is_valid_should_raise_exception_if_no_basefile_is_supplied(self): + data = {"base_file": ""} + with self.assertRaises(Invalid3DTilesException) as _exc: + self.handler.is_valid(files=data, user=self.user) + + self.assertIsNotNone(_exc) + self.assertTrue("base file is not provided" in str(_exc.exception.detail)) + + def test_extract_params_from_data(self): + actual, _data = self.handler.extract_params_from_data( + _data={"defaults": '{"title":"title_of_the_cloned_resource"}'}, + action="copy", + ) + + self.assertEqual(actual, {"title": "title_of_the_cloned_resource"}) + + def test_is_valid_should_raise_exception_if_the_3dtiles_is_invalid(self): + data = { + "base_file": "/using/double/dot/in/the/name/is/an/error/file.invalid.json" + } + with self.assertRaises(Invalid3DTilesException) as _exc: + self.handler.is_valid(files=data, user=self.user) + + self.assertIsNotNone(_exc) + self.assertTrue( + "Please remove the additional dots in the filename" + in str(_exc.exception.detail) + ) + + def test_is_valid_should_raise_exception_if_the_3dtiles_is_invalid_format(self): + with self.assertRaises(Invalid3DTilesException) as _exc: + self.handler.is_valid( + files={"base_file": self.invalid_tileset}, user=self.user + ) + + self.assertIsNotNone(_exc) + self.assertTrue( + "The provided 3DTiles is not valid, some of the mandatory keys are missing. Mandatory keys are: 'asset', 'geometricError', 'root'" + in str(_exc.exception.detail) + ) + + def test_validate_should_raise_exception_for_invalid_asset_key(self): + _json = { + "asset": {"invalid_key": ""}, + "geometricError": 1.0, + "root": {"boundingVolume": {"box": []}, "geometricError": 0.0}, + } + _path = "/tmp/tileset.json" + with open(_path, "w") as _f: + _f.write(json.dumps(_json)) + with self.assertRaises(Invalid3DTilesException) as _exc: + self.handler.is_valid(files={"base_file": _path}, user=self.user) + + self.assertIsNotNone(_exc) + self.assertTrue( + "The mandatory 'version' for the key 'asset' is missing" + in str(_exc.exception.detail) + ) + os.remove(_path) + + def test_validate_should_raise_exception_for_invalid_root_boundingVolume(self): + _json = { + "asset": {"version": "1.1"}, + "geometricError": 1.0, + "root": {"foo": {"box": []}, "geometricError": 0.0}, + } + _path = "/tmp/tileset.json" + with open(_path, "w") as _f: + _f.write(json.dumps(_json)) + with self.assertRaises(Invalid3DTilesException) as _exc: + self.handler.is_valid(files={"base_file": _path}, user=self.user) + + self.assertIsNotNone(_exc) + self.assertTrue( + "The mandatory 'boundingVolume' for the key 'root' is missing" + in str(_exc.exception.detail) + ) + os.remove(_path) + + def test_validate_should_raise_exception_for_invalid_root_geometricError(self): + _json = { + "asset": {"version": "1.1"}, + "geometricError": 1.0, + "root": {"boundingVolume": {"box": []}, "foo": 0.0}, + } + _path = "/tmp/tileset.json" + with open(_path, "w") as _f: + _f.write(json.dumps(_json)) + with self.assertRaises(Invalid3DTilesException) as _exc: + self.handler.is_valid(files={"base_file": _path}, user=self.user) + + self.assertIsNotNone(_exc) + self.assertTrue( + "The mandatory 'geometricError' for the key 'root' is missing" + in str(_exc.exception.detail) + ) + os.remove(_path) + + def test_get_ogr2ogr_driver_should_return_the_expected_driver(self): + expected = ogr.GetDriverByName("3dtiles") + actual = self.handler.get_ogr2ogr_driver() + self.assertEqual(type(expected), type(actual)) + + def test_can_handle_should_return_true_for_3dtiles(self): + actual = self.handler.can_handle({"base_file": self.valid_tileset}) + self.assertTrue(actual) + + def test_can_handle_should_return_false_for_other_files(self): + actual = self.handler.can_handle({"base_file": "random.gpkg"}) + self.assertFalse(actual) + + def test_can_handle_should_return_false_if_no_basefile(self): + actual = self.handler.can_handle({"base_file": ""}) + self.assertFalse(actual) + + def test_supported_file_extension_config(self): + """ + should return the expected value + """ + expected = { + "id": "3dtiles", + "label": "3D Tiles", + "format": "vector", + "ext": ["json"], + "optional": ["xml", "sld"], + } + actual = self.handler.supported_file_extension_config + self.assertDictEqual(actual, expected) + + def test_generate_resource_payload(self): + exec_id = orchestrator.create_execution_request( + user=self.owner, + func_name="funct1", + step="step", + input_params={"files": self.valid_files, "skip_existing_layer": True}, + ) + _exec_obj = orchestrator.get_execution_object(exec_id) + expected = dict( + resource_type="dataset", + subtype="3dtiles", + dirty_state=True, + title="Layer name", + owner=self.owner, + asset="asset", + link_type="uploaded", + extension="3dtiles", + alternate="alternate" + ) + + actual = self.handler.generate_resource_payload( + "Layer name", "alternate", "asset", _exec_obj, None + ) + self.assertSetEqual(set(list(actual.keys())), set(list(expected.keys()))) + self.assertDictEqual(actual, expected) + + def test_create_geonode_resource_validate_bbox_with_region(self): + shutil.copy(self.valid_tileset_with_region, "/tmp/tileset.json") + + exec_id = orchestrator.create_execution_request( + user=self.owner, + func_name="funct1", + step="step", + input_params={ + "files": {"base_file": "/tmp/tileset.json"}, + "skip_existing_layer": True, + }, + ) + + asset = self.asset_handler.create( + title="Original", + owner=self.owner, + description=None, + type=str(self.handler), + files=["/tmp/tileset.json"], + clone_files=False, + ) + + resource = self.handler.create_geonode_resource( + "layername", + "layeralternate", + execution_id=exec_id, + resource_type="ResourceBase", + asset=asset, + ) + + # validate bbox + default_bbox = [-180.0, 180.0, -90.0, 90.0, "EPSG:4326"] + self.assertFalse(resource.bbox == default_bbox) + expected = [ + -75.6144410959485, + -75.60974751970046, + 40.040721313841274, + 40.04433990901052, + "EPSG:4326", + ] + self.assertTrue(resource.bbox == expected) diff --git a/importer/tests/fixture/3dtilesample/README.md b/importer/tests/fixture/3dtilesample/README.md new file mode 100755 index 00000000..cd862833 --- /dev/null +++ b/importer/tests/fixture/3dtilesample/README.md @@ -0,0 +1,2 @@ +Example provided by CensiumGS +https://github.com/CesiumGS/3d-tiles-samples \ No newline at end of file diff --git a/importer/tests/fixture/3dtilesample/invalid.zip b/importer/tests/fixture/3dtilesample/invalid.zip new file mode 100755 index 00000000..e69de29b diff --git a/importer/tests/fixture/3dtilesample/invalid_tileset.json b/importer/tests/fixture/3dtilesample/invalid_tileset.json new file mode 100755 index 00000000..a76162d5 --- /dev/null +++ b/importer/tests/fixture/3dtilesample/invalid_tileset.json @@ -0,0 +1,3 @@ +{ + "asset": "value" +} diff --git a/importer/tests/fixture/3dtilesample/tileset.json b/importer/tests/fixture/3dtilesample/tileset.json new file mode 100755 index 00000000..e9d6cbf2 --- /dev/null +++ b/importer/tests/fixture/3dtilesample/tileset.json @@ -0,0 +1,16 @@ +{ + "asset" : { + "version" : "1.1" + }, + "geometricError" : 1.0, + "root" : { + "boundingVolume" : { + "box" : [ 0.5, 0.5, 1.0, 0.5, 0.0, 0.0, 0.0, -0.5, 0.0, 0.0, 0.0, 1.0 ] + }, + "geometricError" : 0.0, + "refine" : "REPLACE", + "content" : { + "uri" : "0_0_0-1_1_2.glb" + } + } + } diff --git a/importer/tests/fixture/3dtilesample/tileset_with_region.json b/importer/tests/fixture/3dtilesample/tileset_with_region.json new file mode 100755 index 00000000..bfdb5f53 --- /dev/null +++ b/importer/tests/fixture/3dtilesample/tileset_with_region.json @@ -0,0 +1,21 @@ +{ + "asset": { + "version": "1.0" + }, + "geometricError": 70, + "root": { + "boundingVolume": { + "region": [ + -1.3197209591796106, + 0.6988424218, + -1.3196390408203893, + 0.6989055782, + 0, + 20 + ] + }, + "geometricError": 70, + "refine": "ADD" + } +} + \ No newline at end of file diff --git a/importer/tests/fixture/3dtilesample/valid_3dtiles.zip b/importer/tests/fixture/3dtilesample/valid_3dtiles.zip new file mode 100755 index 0000000000000000000000000000000000000000..93955274f1e22d02cddd40aa8bbc2df9a27403ec GIT binary patch literal 1333 zcmWIWW@Zs#U|`^2SUYK7#EdHzrRhuz3=-@N4Ezi-36|UTN~eh;(R0rr}y?fB3)&wVGi3IzAt@r zkkhX~ROpGsEm5ai_1E@an15xXVZyqym06jK^54w-`7G$risGuv20i8MZHf2HdNyyn zJxAfqZxvtRX^Sph=lta6=x~Eo^?u+Qm2VuW5gj^zo(NL&oKkQktBEgxe|)DO`x;xO{>|M>6Ht9RABe zdg4L%#Q8YS}>O; zPikN6$v&sGtzy>6l)8iG9|y0^>`?xvY~FKo=hBiNOC`!3-)Yn=_)@LF@9<9hPr%E5 zhj|S)VLw)0n%}sGp>o{^@1^^hDw#h;J>2dUZyPQj{&Cm7q~L2Zr*B0^e%8(J-+0z- z`|XbX%KJs{ZJqP=$LnuZe9{%icmCR`elP61SWWEi`m@RNW4=BA!5aWiuZMu?wed>w z;?=;^8^p-KzynOLC7C&?#i=EFS;hHz;KaHHm{36&EwSeI8FC*o5NQ4WudAR~TIk?W z-zBq~Ep~K>L??4@vD~ptY0AIy$y+9B`964XuX@k#ww9gj-`53;-RR>@EI;KPxcp)T zugY|RFNSj;Z9Nngc`4Md<%#j6?wiU-?k^Ln{(NECzJ$x$vocvfzn!WrTr=D6aQM+s z9rtZ&yG(YUtu?-r%J&&|KooZ+t-IL+<5AS{h4_s9o>ii`iO11@nM&y zqMY9&naTVJ4?hEXSoNj4qbtzEUO>zV^l*@?ql>SrUTzB5w;zB$1z|Mb7O(XXyqCx`b9te2b2%<3x4pvCPf#xz~LowqMVkj_b8QwN- y0G3Lqu?sX2OUxphh#6oA6PGkD1eRP#!3Q)BOV9;)v$BEw!wiJqfV3tHhz9^W6%+#i literal 0 HcmV?d00001 From b8536c11feb9c2b12952a9aca70132d93b9f27a1 Mon Sep 17 00:00:00 2001 From: Mattia Date: Mon, 17 Jun 2024 18:14:11 +0200 Subject: [PATCH 35/40] [Fixes #247] Remove typos --- importer/tests/end2end/test_end2end.py | 44 -------------------------- 1 file changed, 44 deletions(-) diff --git a/importer/tests/end2end/test_end2end.py b/importer/tests/end2end/test_end2end.py index 259bfb4a..ee7aee1f 100644 --- a/importer/tests/end2end/test_end2end.py +++ b/importer/tests/end2end/test_end2end.py @@ -43,9 +43,6 @@ def setUpClass(cls) -> None: cls.valid_kml = f"{project_dir}/tests/fixture/valid.kml" cls.valid_tif = f"{project_dir}/tests/fixture/test_grid.tif" cls.valid_csv = f"{project_dir}/tests/fixture/valid.csv" - cls.valid_3dtiles = ( - f"{project_dir}/tests/fixture/3dtilesample/valid_3dtiles.zip" - ) cls.url = reverse("importer_upload") ogc_server_settings = OGC_Servers_Handler(settings.OGC_SERVER)["default"] @@ -444,44 +441,3 @@ def test_import_raster_overwrite(self): layer = self.cat.get_layer("test_grid") if layer: self.cat.delete(layer) - - -class Importer3DtilesImportTest(BaseImporterEndToEndTest): - @mock.patch.dict(os.environ, {"GEONODE_GEODATABASE": "test_geonode_data"}) - @override_settings( - GEODATABASE_URL=f"{geourl.split('/geonode_data')[0]}/test_geonode_data" - ) - def test_import_3dtiles(self): - ResourceBase.objects.filter(alternate__icontains="valid_3dtiles").delete() - payload = { - "zip_file": open(self.valid_3dtiles, "rb"), - "base_file": open(self.valid_3dtiles, "rb"), - } - initial_name = "valid_3dtiles" - self._assertimport(payload, initial_name, skip_geoserver=True) - - @mock.patch.dict(os.environ, {"GEONODE_GEODATABASE": "test_geonode_data"}) - @override_settings( - GEODATABASE_URL=f"{geourl.split('/geonode_data')[0]}/test_geonode_data" - ) - def test_import_3dtiles_overwrite(self): - payload = { - "zip_file": open(self.valid_3dtiles, "rb"), - "base_file": open(self.valid_3dtiles, "rb"), - } - initial_name = "valid_3dtiles" - # importing the resource - resource = resource_manager.create( - str(uuid4()), - resource_type=ResourceBase, - defaults={"title": "simple resourcebase", "owner": self.user}, - ) - - payload["overwrite_existing_layer"] = True - self._assertimport( - payload, - initial_name, - overwrite=True, - last_update=resource.last_updated, - skip_geoserver=True, - ) From cf51cdd67d4c0e76d532ee16f178fa7551e8bea1 Mon Sep 17 00:00:00 2001 From: Mattia Date: Wed, 19 Jun 2024 10:18:03 +0200 Subject: [PATCH 36/40] [Fixes #246] Add function for calcultating bbox from boundingVolume --- importer/handlers/tiles3d/handler.py | 52 ++++++-- importer/handlers/tiles3d/tests.py | 143 ++++++++++++++++++---- importer/handlers/tiles3d/utils.py | 174 +++++++++++++++++++++++++++ 3 files changed, 337 insertions(+), 32 deletions(-) create mode 100644 importer/handlers/tiles3d/utils.py diff --git a/importer/handlers/tiles3d/handler.py b/importer/handlers/tiles3d/handler.py index 06e146d1..8ea6b4b8 100755 --- a/importer/handlers/tiles3d/handler.py +++ b/importer/handlers/tiles3d/handler.py @@ -6,6 +6,7 @@ from geonode.layers.models import Dataset from geonode.resource.enumerator import ExecutionRequestAction as exa from geonode.upload.utils import UploadLimitValidator +from importer.handlers.tiles3d.utils import box_to_wgs84 from importer.orchestrator import orchestrator from importer.celery_tasks import import_orchestrator from importer.handlers.common.vector import BaseVectorFileHandler @@ -221,6 +222,26 @@ def create_geonode_resource( if not js_file: return resource + resource = self.set_bbox_from_region(js_file, resource=resource) + + resource = self.set_bbox_from_boundingVolume(js_file, resource=resource) + + return resource + + def generate_resource_payload(self, layer_name, alternate, asset, _exec, workspace): + return dict( + resource_type="dataset", + subtype="3dtiles", + dirty_state=True, + title=layer_name, + owner=_exec.user, + asset=asset, + link_type="uploaded", + extension="3dtiles", + alternate=alternate, + ) + + def set_bbox_from_region(self, js_file, resource): # checking if the region is inside the json file region = js_file.get("root", {}).get("boundingVolume", {}).get("region", None) if not region: @@ -242,15 +263,24 @@ def create_geonode_resource( return resource - def generate_resource_payload(self, layer_name, alternate, asset, _exec, workspace): - return dict( - resource_type="dataset", - subtype="3dtiles", - dirty_state=True, - title=layer_name, - owner=_exec.user, - asset=asset, - link_type="uploaded", - extension="3dtiles", - alternate=alternate, + def set_bbox_from_boundingVolume(self, js_file, resource): + transform_raw = ( + js_file.get("root", {}).get("boundingVolume", {}).get("transform", None) ) + box_raw = js_file.get("root", {}).get("boundingVolume", {}).get("box", None) + if not transform_raw and not box_raw: + # skipping if values are missing from the json file + return resource + result = box_to_wgs84(box_raw, transform_raw) + # [xmin, ymin, xmax, ymax] + resource.set_bbox_polygon( + bbox=[ + result["minx"], + result["miny"], + result["maxx"], + result["maxy"], + ], + srid="EPSG:4326", + ) + + return resource diff --git a/importer/handlers/tiles3d/tests.py b/importer/handlers/tiles3d/tests.py index 2fcd2834..b1e25d5e 100755 --- a/importer/handlers/tiles3d/tests.py +++ b/importer/handlers/tiles3d/tests.py @@ -38,6 +38,7 @@ def setUpClass(cls): name="urban_forestry_street_tree_benefits_epsg_26985", owner=cls.owner ) cls.asset_handler = asset_handler_registry.get_default_handler() + cls.default_bbox = [-180.0, 180.0, -90.0, 90.0, "EPSG:4326"] def test_task_list_is_the_expected_one(self): expected = ( @@ -222,7 +223,7 @@ def test_generate_resource_payload(self): asset="asset", link_type="uploaded", extension="3dtiles", - alternate="alternate" + alternate="alternate", ) actual = self.handler.generate_resource_payload( @@ -234,24 +235,7 @@ def test_generate_resource_payload(self): def test_create_geonode_resource_validate_bbox_with_region(self): shutil.copy(self.valid_tileset_with_region, "/tmp/tileset.json") - exec_id = orchestrator.create_execution_request( - user=self.owner, - func_name="funct1", - step="step", - input_params={ - "files": {"base_file": "/tmp/tileset.json"}, - "skip_existing_layer": True, - }, - ) - - asset = self.asset_handler.create( - title="Original", - owner=self.owner, - description=None, - type=str(self.handler), - files=["/tmp/tileset.json"], - clone_files=False, - ) + exec_id, asset = self._generate_execid_asset() resource = self.handler.create_geonode_resource( "layername", @@ -262,8 +246,7 @@ def test_create_geonode_resource_validate_bbox_with_region(self): ) # validate bbox - default_bbox = [-180.0, 180.0, -90.0, 90.0, "EPSG:4326"] - self.assertFalse(resource.bbox == default_bbox) + self.assertFalse(resource.bbox == self.default_bbox) expected = [ -75.6144410959485, -75.60974751970046, @@ -272,3 +255,121 @@ def test_create_geonode_resource_validate_bbox_with_region(self): "EPSG:4326", ] self.assertTrue(resource.bbox == expected) + + def test_set_bbox_from_bounding_volume_wit_transform(self): + # https://github.com/geosolutions-it/MapStore2/blob/master/web/client/api/__tests__/ThreeDTiles-test.js#L102-L146 + tilesetjson_file = { + "asset": {"version": "1.1"}, + "geometricError": 1.0, + "root": { + "boundingVolume": { + "transform": [ + 96.86356343768793, + 24.848542777253734, + 0, + 0, + -15.986465724980844, + 62.317780594908875, + 76.5566922962899, + 0, + 19.02322243409411, + -74.15554020821229, + 64.3356267137516, + 0, + 1215107.7612304366, + -4736682.902037748, + 4081926.095098698, + 1, + ], + "box": [0, 0, 0, 7.0955, 0, 0, 0, 3.1405, 0, 0, 0, 5.0375], + } + }, + } + + with open("/tmp/tileset.json", "w+") as js_file: + js_file.write(json.dumps(tilesetjson_file)) + + exec_id, asset = self._generate_execid_asset() + + resource = self.handler.create_geonode_resource( + "layername", + "layeralternate", + execution_id=exec_id, + resource_type="ResourceBase", + asset=asset, + ) + self.assertFalse(resource.bbox == self.default_bbox) + + self.assertEqual(resource.bbox_x0, -75.61852101302848) + self.assertEqual(resource.bbox_x1, -75.60566760262047) + self.assertEqual(resource.bbox_y0, 40.03610390613993) + self.assertEqual(resource.bbox_y1, 40.04895731654794) + + os.remove("/tmp/tileset.json") + + def test_set_bbox_from_bounding_volume_without_transform(self): + # https://github.com/geosolutions-it/MapStore2/blob/master/web/client/api/__tests__/ThreeDTiles-test.js#L147-L180 + tilesetjson_file = { + "asset": {"version": "1.1"}, + "geometricError": 1.0, + "root": { + "boundingVolume": { + "box": [ + 0.2524109, + 9.536743e-7, + 4.5, + 16.257824, + 0.0, + 0.0, + 0.0, + -19.717258, + 0.0, + 0.0, + 0.0, + 4.5, + ] + } + }, + } + + with open("/tmp/tileset.json", "w+") as js_file: + js_file.write(json.dumps(tilesetjson_file)) + + exec_id, asset = self._generate_execid_asset() + + resource = self.handler.create_geonode_resource( + "layername", + "layeralternate", + execution_id=exec_id, + resource_type="ResourceBase", + asset=asset, + ) + self.assertFalse(resource.bbox == self.default_bbox) + + self.assertEqual(resource.bbox_x0, -1.3348442882497923e-05) + self.assertEqual(resource.bbox_x1, 0.0004463052796897286) + self.assertEqual(resource.bbox_y0, 86.81078622278615) + self.assertEqual(resource.bbox_y1, 86.81124587650872) + + os.remove("/tmp/tileset.json") + + def _generate_execid_asset(self): + exec_id = orchestrator.create_execution_request( + user=self.owner, + func_name="funct1", + step="step", + input_params={ + "files": {"base_file": "/tmp/tileset.json"}, + "skip_existing_layer": True, + }, + ) + asset = self.asset_handler.create( + title="Original", + owner=self.owner, + description=None, + type=str(self.handler), + files=["/tmp/tileset.json"], + clone_files=False, + ) + + return exec_id, asset diff --git a/importer/handlers/tiles3d/utils.py b/importer/handlers/tiles3d/utils.py new file mode 100644 index 00000000..5ff4401f --- /dev/null +++ b/importer/handlers/tiles3d/utils.py @@ -0,0 +1,174 @@ +import math +import logging +import numpy as np + +logger = logging.getLogger(__name__) + + +wgs84OneOverRadii = np.array( + [1.0 / 6378137.0, 1.0 / 6378137.0, 1.0 / 6356752.3142451793] +) +wgs84OneOverRadiiSquared = np.array( + [ + 1.0 / (6378137.0 * 6378137.0), + 1.0 / (6378137.0 * 6378137.0), + 1.0 / (6356752.3142451793 * 6356752.3142451793), + ] +) +wgs84CenterToleranceSquared = 0.1 +CesiumMath_EPSILON12 = 0.000000000001 + + +def fromOrientedBoundingBox(center, halfAxes): + u = halfAxes[:, 0] + v = halfAxes[:, 1] + w = halfAxes[:, 2] + + u = np.add(u, v) + w = np.add(u, w) + + return {"center": center, "radius": np.linalg.norm(u)} + + +def scaleToGeodeticSurface( + cartesian, oneOverRadii, oneOverRadiiSquared, centerToleranceSquared +): + # https://github.com/CesiumGS/cesium/blob/main/packages/engine/Source/Core/scaleToGeodeticSurface.js#L25 + positionX = cartesian[0] + positionY = cartesian[1] + positionZ = cartesian[2] + + oneOverRadiiX = oneOverRadii[0] + oneOverRadiiY = oneOverRadii[1] + oneOverRadiiZ = oneOverRadii[2] + + x2 = positionX * positionX * oneOverRadiiX * oneOverRadiiX + y2 = positionY * positionY * oneOverRadiiY * oneOverRadiiY + z2 = positionZ * positionZ * oneOverRadiiZ * oneOverRadiiZ + + squaredNorm = x2 + y2 + z2 + ratio = math.sqrt(1.0 / squaredNorm) + + intersection = cartesian * ratio + + if squaredNorm < centerToleranceSquared: + return intersection[:3] if np.isfinite(ratio) else np.NaN + + oneOverRadiiSquaredX = oneOverRadiiSquared[0] + oneOverRadiiSquaredY = oneOverRadiiSquared[1] + oneOverRadiiSquaredZ = oneOverRadiiSquared[2] + + gradient = np.ones(3) + gradient[0] = intersection[0] * oneOverRadiiSquaredX * 2.0 + gradient[1] = intersection[1] * oneOverRadiiSquaredY * 2.0 + gradient[2] = intersection[2] * oneOverRadiiSquaredZ * 2.0 + + _lambda = ((1.0 - ratio) * np.linalg.norm(cartesian)) / ( + 0.5 * np.linalg.norm(gradient) + ) + correction = 0.0 + + while True: + _lambda -= correction + + xMultiplier = 1.0 / (1.0 + _lambda * oneOverRadiiSquaredX) + yMultiplier = 1.0 / (1.0 + _lambda * oneOverRadiiSquaredY) + zMultiplier = 1.0 / (1.0 + _lambda * oneOverRadiiSquaredZ) + + xMultiplier2 = xMultiplier * xMultiplier + yMultiplier2 = yMultiplier * yMultiplier + zMultiplier2 = zMultiplier * zMultiplier + + xMultiplier3 = xMultiplier2 * xMultiplier + yMultiplier3 = yMultiplier2 * yMultiplier + zMultiplier3 = zMultiplier2 * zMultiplier + + func = x2 * xMultiplier2 + y2 * yMultiplier2 + z2 * zMultiplier2 - 1.0 + + # "denominator" here refers to the use of this expression in the velocity and acceleration + # computations in the sections to follow. + denominator = ( + x2 * xMultiplier3 * oneOverRadiiSquaredX + + y2 * yMultiplier3 * oneOverRadiiSquaredY + + z2 * zMultiplier3 * oneOverRadiiSquaredZ + ) + + derivative = -2.0 * denominator + correction = func / derivative + + if math.fabs(func) > CesiumMath_EPSILON12: + break + + result = np.ones(3) + result[0] = positionX * xMultiplier + result[1] = positionY * yMultiplier + result[2] = positionZ * zMultiplier + + return result + + +def fromCartesian(cartesian): + # https://github.com/CesiumGS/cesium/blob/main/packages/engine/Source/Core/Cartographic.js#L116 + p = scaleToGeodeticSurface( + cartesian, + wgs84OneOverRadii, + wgs84OneOverRadiiSquared, + wgs84CenterToleranceSquared, + ) + + n = p * wgs84OneOverRadiiSquared + n = n / np.linalg.norm(n) + + h = cartesian[:3] - p + longitude = math.atan2(n[1], n[0]) + latitude = math.asin(n[2]) + + height = math.copysign(1, h.dot(cartesian[:3])) * np.linalg.norm(h) + + return {"longitude": longitude, "latitude": latitude, "height": height} + + +def box_to_wgs84(box_raw, transform_raw): + box = box_raw + transform_raw = transform_raw or [1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1] + # transform_raw = transform_raw if transform_raw else range[12] + + transform = np.array( + [ + transform_raw[0:4], + transform_raw[4:8], + transform_raw[8:12], + transform_raw[12:16], + ] + ) + + point = np.array([box[0], box[1], box[2], 1]) + center = point.dot(transform) # Cesium.Matrix4.multiplyByPoint + rotationScale = transform[:3, :3] # Cesium.Matrix4.getMatrix3 + halfAxes = np.multiply( + rotationScale, + np.array( + [ + [box[3], box[4], box[5]], + [box[6], box[7], box[8]], + [box[9], box[10], box[11]], + ] + ), + ) + + sphere = fromOrientedBoundingBox(center, halfAxes) + cartographic = fromCartesian(sphere["center"]) + # print(cartographic) + + lng = math.degrees(cartographic["longitude"]) + lat = math.degrees(cartographic["latitude"]) + + # https://github.com/geosolutions-it/MapStore2/blob/master/web/client/utils/MapUtils.js#L51C16-L51C34 + radiusDegrees = sphere["radius"] / 111194.87428468118 + + return { + "minx": lng - radiusDegrees, + "miny": lat - radiusDegrees, + "maxx": lng + radiusDegrees, + "maxy": lat + radiusDegrees, + } From 5c6596a9d98ef5d6f94b8df4bdc791c17dadf2aa Mon Sep 17 00:00:00 2001 From: Mattia Date: Thu, 20 Jun 2024 11:42:43 +0200 Subject: [PATCH 37/40] Add ID for base handlers --- Dockerfile | 2 +- importer/api/views.py | 2 +- importer/handlers/base.py | 12 +++++++++++- importer/handlers/gpkg/tests.py | 2 +- importer/orchestrator.py | 7 +++++++ importer/tests/unit/test_orchestrator.py | 8 ++++++-- importer/tests/unit/test_task.py | 2 +- 7 files changed, 28 insertions(+), 7 deletions(-) diff --git a/Dockerfile b/Dockerfile index 03e9cb5e..e7c2cc3c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,7 @@ FROM geonode/geonode-base:latest-ubuntu-22.04 RUN rm -rf /usr/src/geonode RUN git clone https://github.com/GeoNode/geonode.git /usr/src/geonode -RUN cd /usr/src/geonode && git fetch --all && git checkout 12226_directory_assets_3 && cd - +RUN cd /usr/src/geonode && git fetch --all && git checkout assets_master && cd - RUN mkdir -p /usr/src/importer RUN cd .. diff --git a/importer/api/views.py b/importer/api/views.py index 8b603a33..dcaf4140 100644 --- a/importer/api/views.py +++ b/importer/api/views.py @@ -201,7 +201,7 @@ def generate_asset_and_retrieve_paths(self, request, storage_manager, handler): title="Original", owner=request.user, description=None, - type=str(handler), + type=handler.id, files=list(set(_files.values())), clone_files=False, ) diff --git a/importer/handlers/base.py b/importer/handlers/base.py index e005fcd4..6c74ae8b 100644 --- a/importer/handlers/base.py +++ b/importer/handlers/base.py @@ -4,6 +4,7 @@ from geonode.resource.enumerator import ExecutionRequestAction as exa from geonode.layers.models import Dataset +from importer.api.exception import ImportException from importer.utils import ImporterRequestAction as ira from django_celery_results.models import TaskResult from django.db.models import Q @@ -56,9 +57,18 @@ def get_task_list(cls, action) -> tuple: def default_geometry_column_name(self): return "geometry" + @property + def id(self): + pk = self.supported_file_extension_config.get("id", None) + if pk is None: + raise ImportException( + "PK must be defined, check that supported_file_extension_config had been correctly defined, it cannot be empty" + ) + return pk + @property def supported_file_extension_config(self): - return NotImplementedError + return {} @property def can_handle_xml_file(self) -> bool: diff --git a/importer/handlers/gpkg/tests.py b/importer/handlers/gpkg/tests.py index 91c5cd09..4374099c 100644 --- a/importer/handlers/gpkg/tests.py +++ b/importer/handlers/gpkg/tests.py @@ -126,7 +126,7 @@ def test_single_message_error_handler(self): title="Original", owner=user, description=None, - type="importer.handlers.gpkg.handler.GPKGFileHandler", + type="gpkg", files=["/tmp/valid.gpkg"], clone_files=False, ) diff --git a/importer/orchestrator.py b/importer/orchestrator.py index 81b63ffa..afd18f8c 100644 --- a/importer/orchestrator.py +++ b/importer/orchestrator.py @@ -63,6 +63,13 @@ def load_handler(self, module_path): except Exception: raise ImportException(detail=f"The handler is not available: {module_path}") + def load_handler_by_id(self, handler_id): + for handler in BaseHandler.get_registry(): + if handler().id == handler_id: + return handler + logger.error("Handler not found") + return None + def get_execution_object(self, exec_id): """ Returns the ExecutionRequest object with the detail about the diff --git a/importer/tests/unit/test_orchestrator.py b/importer/tests/unit/test_orchestrator.py index 4f6ef315..d3ab4e7a 100644 --- a/importer/tests/unit/test_orchestrator.py +++ b/importer/tests/unit/test_orchestrator.py @@ -60,6 +60,10 @@ def test_load_handler(self): ) self.assertIsInstance(actual(), BaseHandler) + def test_load_handler_by_id(self): + actual = self.orchestrator.load_handler_by_id("gpkg") + self.assertIsInstance(actual(), BaseHandler) + def test_get_execution_object_raise_exp_if_not_exists(self): with self.assertRaises(ImportException) as _exc: self.orchestrator.get_execution_object(str(uuid.uuid4())) @@ -197,7 +201,7 @@ def test_set_as_failed(self): title="Original", owner=user, description=None, - type="importer.handlers.gpkg.handler.GPKGFileHandler", + type="gpkg", files=[fake_path], clone_files=False, ) @@ -345,7 +349,7 @@ def test_evaluate_execution_progress_should_fail_if_one_task_is_failed(self): title="Original", owner=user, description=None, - type="importer.handlers.gpkg.handler.GPKGFileHandler", + type="gpkg", files=[fake_path], clone_files=False, ) diff --git a/importer/tests/unit/test_task.py b/importer/tests/unit/test_task.py index d61cf5c7..34dee1b5 100644 --- a/importer/tests/unit/test_task.py +++ b/importer/tests/unit/test_task.py @@ -48,7 +48,7 @@ def setUp(self): title="Original", owner=self.user, description=None, - type="importer.handlers.gpkg.handler.GPKGFileHandler", + type="gpkg", files=[self.existing_file], clone_files=False, ) From 103342ea853862b9267d1a508d9d06117947f5e2 Mon Sep 17 00:00:00 2001 From: Mattia Date: Fri, 21 Jun 2024 16:31:38 +0200 Subject: [PATCH 38/40] [Fixes #246] Fix transform for 3dtiles handling --- importer/handlers/tiles3d/handler.py | 4 +--- importer/handlers/tiles3d/tests.py | 6 +++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/importer/handlers/tiles3d/handler.py b/importer/handlers/tiles3d/handler.py index 8ea6b4b8..be153e1e 100755 --- a/importer/handlers/tiles3d/handler.py +++ b/importer/handlers/tiles3d/handler.py @@ -264,9 +264,7 @@ def set_bbox_from_region(self, js_file, resource): return resource def set_bbox_from_boundingVolume(self, js_file, resource): - transform_raw = ( - js_file.get("root", {}).get("boundingVolume", {}).get("transform", None) - ) + transform_raw = js_file.get("root", {}).get("transform", []) box_raw = js_file.get("root", {}).get("boundingVolume", {}).get("box", None) if not transform_raw and not box_raw: # skipping if values are missing from the json file diff --git a/importer/handlers/tiles3d/tests.py b/importer/handlers/tiles3d/tests.py index b1e25d5e..0d493ce6 100755 --- a/importer/handlers/tiles3d/tests.py +++ b/importer/handlers/tiles3d/tests.py @@ -262,8 +262,7 @@ def test_set_bbox_from_bounding_volume_wit_transform(self): "asset": {"version": "1.1"}, "geometricError": 1.0, "root": { - "boundingVolume": { - "transform": [ + "transform": [ 96.86356343768793, 24.848542777253734, 0, @@ -280,7 +279,8 @@ def test_set_bbox_from_bounding_volume_wit_transform(self): -4736682.902037748, 4081926.095098698, 1, - ], + ], + "boundingVolume": { "box": [0, 0, 0, 7.0955, 0, 0, 0, 3.1405, 0, 0, 0, 5.0375], } }, From 3a8f2137549d7011ae6858ded1d673fc95ed443c Mon Sep 17 00:00:00 2001 From: Mattia Date: Mon, 24 Jun 2024 11:15:48 +0200 Subject: [PATCH 39/40] [Fixes #246] Fix setting of bbox --- importer/handlers/tiles3d/handler.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/importer/handlers/tiles3d/handler.py b/importer/handlers/tiles3d/handler.py index be153e1e..4af71dcb 100755 --- a/importer/handlers/tiles3d/handler.py +++ b/importer/handlers/tiles3d/handler.py @@ -222,9 +222,10 @@ def create_geonode_resource( if not js_file: return resource - resource = self.set_bbox_from_region(js_file, resource=resource) - - resource = self.set_bbox_from_boundingVolume(js_file, resource=resource) + if self._has_region(js_file): + resource = self.set_bbox_from_region(js_file, resource=resource) + else: + resource = self.set_bbox_from_boundingVolume(js_file, resource=resource) return resource @@ -266,7 +267,7 @@ def set_bbox_from_region(self, js_file, resource): def set_bbox_from_boundingVolume(self, js_file, resource): transform_raw = js_file.get("root", {}).get("transform", []) box_raw = js_file.get("root", {}).get("boundingVolume", {}).get("box", None) - if not transform_raw and not box_raw: + if not box_raw or (not transform_raw and not box_raw): # skipping if values are missing from the json file return resource result = box_to_wgs84(box_raw, transform_raw) @@ -282,3 +283,6 @@ def set_bbox_from_boundingVolume(self, js_file, resource): ) return resource + + def _has_region(self, js_file): + return js_file.get("root", {}).get("boundingVolume", {}).get("region", None) \ No newline at end of file From de0efef06e6fe34352b2381dcd950482bfd07fbe Mon Sep 17 00:00:00 2001 From: George Petrakis Date: Wed, 3 Jul 2024 15:14:17 +0300 Subject: [PATCH 40/40] [Fixes #246] Implementation of boundingBox.sphere parsing for 3D Tiles - Issue #79 (#253) * boundingBox.sphere parser for 3D Tiles * cleaning the code in tests for the sphere_to_wgs84 function --- importer/handlers/tiles3d/handler.py | 34 +++++++- importer/handlers/tiles3d/tests.py | 125 +++++++++++++++++++++++++++ importer/handlers/tiles3d/utils.py | 56 +++++++++++- 3 files changed, 212 insertions(+), 3 deletions(-) diff --git a/importer/handlers/tiles3d/handler.py b/importer/handlers/tiles3d/handler.py index 4af71dcb..1a9ff92a 100755 --- a/importer/handlers/tiles3d/handler.py +++ b/importer/handlers/tiles3d/handler.py @@ -6,7 +6,7 @@ from geonode.layers.models import Dataset from geonode.resource.enumerator import ExecutionRequestAction as exa from geonode.upload.utils import UploadLimitValidator -from importer.handlers.tiles3d.utils import box_to_wgs84 +from importer.handlers.tiles3d.utils import box_to_wgs84, sphere_to_wgs84 from importer.orchestrator import orchestrator from importer.celery_tasks import import_orchestrator from importer.handlers.common.vector import BaseVectorFileHandler @@ -224,6 +224,8 @@ def create_geonode_resource( if self._has_region(js_file): resource = self.set_bbox_from_region(js_file, resource=resource) + elif self._has_sphere(js_file): + resource = self.set_bbox_from_boundingVolume_sphere(js_file, resource=resource) else: resource = self.set_bbox_from_boundingVolume(js_file, resource=resource) @@ -267,9 +269,11 @@ def set_bbox_from_region(self, js_file, resource): def set_bbox_from_boundingVolume(self, js_file, resource): transform_raw = js_file.get("root", {}).get("transform", []) box_raw = js_file.get("root", {}).get("boundingVolume", {}).get("box", None) + if not box_raw or (not transform_raw and not box_raw): # skipping if values are missing from the json file return resource + result = box_to_wgs84(box_raw, transform_raw) # [xmin, ymin, xmax, ymax] resource.set_bbox_polygon( @@ -283,6 +287,32 @@ def set_bbox_from_boundingVolume(self, js_file, resource): ) return resource + + def set_bbox_from_boundingVolume_sphere(self, js_file, resource): + transform_raw = js_file.get("root", {}).get("transform", []) + sphere_raw = js_file.get("root", {}).get("boundingVolume", {}).get("sphere", None) + + if not sphere_raw or (not transform_raw and not sphere_raw): + # skipping if values are missing from the json file + return resource + if not transform_raw and (sphere_raw[0], sphere_raw[1], sphere_raw[2]) == (0, 0, 0): + return resource + result = sphere_to_wgs84(sphere_raw, transform_raw) + # [xmin, ymin, xmax, ymax] + resource.set_bbox_polygon( + bbox=[ + result["minx"], + result["miny"], + result["maxx"], + result["maxy"], + ], + srid="EPSG:4326", + ) + + return resource def _has_region(self, js_file): - return js_file.get("root", {}).get("boundingVolume", {}).get("region", None) \ No newline at end of file + return js_file.get("root", {}).get("boundingVolume", {}).get("region", None) + + def _has_sphere(self, js_file): + return js_file.get("root", {}).get("boundingVolume", {}).get("sphere", None) diff --git a/importer/handlers/tiles3d/tests.py b/importer/handlers/tiles3d/tests.py index 0d493ce6..cf5666f8 100755 --- a/importer/handlers/tiles3d/tests.py +++ b/importer/handlers/tiles3d/tests.py @@ -353,6 +353,131 @@ def test_set_bbox_from_bounding_volume_without_transform(self): os.remove("/tmp/tileset.json") + def test_set_bbox_from_bounding_volume_sphere_with_transform(self): + # https://github.com/geosolutions-it/MapStore2/blob/master/web/client/api/__tests__/ThreeDTiles-test.js#L102-L146 + tilesetjson_file = { + "asset": {"version": "1.1"}, + "geometricError": 1.0, + "root": { + "transform": [ + 0.968635634376879, + 0.24848542777253735, + 0, + 0, + -0.15986460794399626, + 0.6231776137472074, + 0.7655670897127491, + 0, + 0.190232265775849, + -0.7415555636019701, + 0.6433560687121489, + 0, + 1215012.8828876738, + -4736313.051199594, + 4081605.22126042, + 1, + ], + "boundingVolume": { + "sphere": [0, 0, 0, 5] + } + }, + } + + with open("/tmp/tileset.json", "w+") as js_file: + js_file.write(json.dumps(tilesetjson_file)) + + exec_id, asset = self._generate_execid_asset() + + resource = self.handler.create_geonode_resource( + "layername", + "layeralternate", + execution_id=exec_id, + resource_type="ResourceBase", + asset=asset, + ) + self.assertFalse(resource.bbox == self.default_bbox) + + self.assertEqual(resource.bbox_x0, -75.61213927392595) + self.assertEqual(resource.bbox_x1, -75.61204934172301) + self.assertEqual(resource.bbox_y0, 40.042485645323616) + self.assertEqual(resource.bbox_y1, 40.042575577526556) + + os.remove("/tmp/tileset.json") + + def test_set_bbox_from_bounding_volume_sphere_without_transform(self): + # https://github.com/geosolutions-it/MapStore2/blob/master/web/client/api/__tests__/ThreeDTiles-test.js#L53C4-L79C8 + tilesetjson_file = { + "asset": {"version": "1.1"}, + "geometricError": 1.0, + "root": { + "boundingVolume": { + "sphere": [ + 0.2524109, + 9.536743E-7, + 4.5, + 5 + ] + } + }, + } + + with open("/tmp/tileset.json", "w+") as js_file: + js_file.write(json.dumps(tilesetjson_file)) + + exec_id, asset = self._generate_execid_asset() + + resource = self.handler.create_geonode_resource( + "layername", + "layeralternate", + execution_id=exec_id, + resource_type="ResourceBase", + asset=asset, + ) + self.assertFalse(resource.bbox == self.default_bbox) + + self.assertEqual(resource.bbox_x0, 0.00017151231693387494) + self.assertEqual(resource.bbox_x1, 0.00026144451987335574) + self.assertEqual(resource.bbox_y0, 86.81097108354597) + self.assertEqual(resource.bbox_y1, 86.8110610157489) + + os.remove("/tmp/tileset.json") + + + def test_set_bbox_from_bounding_volume_sphere_with_center_zero_without_transform(self): + # https://github.com/geosolutions-it/MapStore2/blob/master/web/client/api/__tests__/ThreeDTiles-test.js#L53C4-L79C8 + # This test should not extract bbox from boundingVolume sphere with center 0, 0, 0 + tilesetjson_file = { + "asset": {"version": "1.1"}, + "geometricError": 1.0, + "root": { + "boundingVolume": { + "sphere": [ + 0, + 0, + 0, + 5 + ] + } + }, + } + + with open("/tmp/tileset.json", "w+") as js_file: + js_file.write(json.dumps(tilesetjson_file)) + + exec_id, asset = self._generate_execid_asset() + + resource = self.handler.create_geonode_resource( + "layername", + "layeralternate", + execution_id=exec_id, + resource_type="ResourceBase", + asset=asset, + ) + self.assertTrue(resource.bbox == self.default_bbox) + + os.remove("/tmp/tileset.json") + + def _generate_execid_asset(self): exec_id = orchestrator.create_execution_request( user=self.owner, diff --git a/importer/handlers/tiles3d/utils.py b/importer/handlers/tiles3d/utils.py index 5ff4401f..a544b4b1 100644 --- a/importer/handlers/tiles3d/utils.py +++ b/importer/handlers/tiles3d/utils.py @@ -127,6 +127,24 @@ def fromCartesian(cartesian): return {"longitude": longitude, "latitude": latitude, "height": height} +def getScale(matrix): + ''' + Cesium.Matrix4.getScale() + Extracts the non-uniform scale assuming the matrix is an affine transformation + # https://github.com/CesiumGS/cesium/blob/1.118/packages/engine/Source/Core/Matrix4.js#L1596-L1612 + ''' + + # check the type of the matrix + if isinstance(matrix, np.ndarray)!=True: + print('Please define a NumPy array object') + + x = np.linalg.norm([matrix[0][0], matrix[0][1], matrix[0][2]]) + y = np.linalg.norm([matrix[1][0], matrix[1][1], matrix[1][2]]) + z = np.linalg.norm([matrix[2][0], matrix[2][1], matrix[2][2]]) + + result = np.array([x, y, z]) + + return result def box_to_wgs84(box_raw, transform_raw): box = box_raw @@ -158,7 +176,6 @@ def box_to_wgs84(box_raw, transform_raw): sphere = fromOrientedBoundingBox(center, halfAxes) cartographic = fromCartesian(sphere["center"]) - # print(cartographic) lng = math.degrees(cartographic["longitude"]) lat = math.degrees(cartographic["latitude"]) @@ -172,3 +189,40 @@ def box_to_wgs84(box_raw, transform_raw): "maxx": lng + radiusDegrees, "maxy": lat + radiusDegrees, } + +def sphere_to_wgs84(sphere_raw, transform_raw): + + transform_raw = transform_raw or [1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1] + + transform = np.array([ + transform_raw[0:4], + transform_raw[4:8], + transform_raw[8:12], + transform_raw[12:16], + ]) + + centerPoint = np.array([sphere_raw[0], sphere_raw[1], sphere_raw[2], 1]) + + radius = sphere_raw[3] + + # Sphere center after the transformation + center = centerPoint.dot(transform) # Cesium.Matrix4.multiplyByPoint + + scale = getScale(transform) + uniformScale = np.max(scale) + radiusDegrees = (radius * uniformScale) / 111194.87428468118 # degrees of one meter + + cartographic = fromCartesian(center) + + if not cartographic: + return None + + lng = math.degrees(cartographic['longitude']) + lat = math.degrees(cartographic['latitude']) + + return { + "minx": lng - radiusDegrees, + "miny": lat - radiusDegrees, + "maxx": lng + radiusDegrees, + "maxy": lat + radiusDegrees + }