Skip to content

Commit

Permalink
Add test coverage and refactor with black
Browse files Browse the repository at this point in the history
  • Loading branch information
mattiagiupponi committed Jun 6, 2024
1 parent b611c3d commit d4ce079
Show file tree
Hide file tree
Showing 13 changed files with 232 additions and 51 deletions.
19 changes: 14 additions & 5 deletions importer/api/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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,
}

Expand All @@ -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,
}
Expand Down Expand Up @@ -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,
}
Expand Down Expand Up @@ -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,
}
Expand Down
12 changes: 7 additions & 5 deletions importer/api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"))}
)
Expand All @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion importer/datastore.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 13 additions & 11 deletions importer/handlers/common/vector.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions importer/handlers/geojson/handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion importer/handlers/gpkg/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
13 changes: 8 additions & 5 deletions importer/handlers/tiles3d/handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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
Expand All @@ -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,
)
109 changes: 108 additions & 1 deletion importer/handlers/tiles3d/tests.py
Original file line number Diff line number Diff line change
@@ -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):
Expand All @@ -20,17 +23,21 @@ 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"
)
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.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 = (
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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)
2 changes: 1 addition & 1 deletion importer/handlers/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions importer/orchestrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
Loading

0 comments on commit d4ce079

Please sign in to comment.