Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fixes #233] [GPKG] must specify native_crs #234

Merged
merged 6 commits into from
May 3, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
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 mkdir -p /usr/src/importer

Expand Down
4 changes: 2 additions & 2 deletions importer/api/serializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class Meta:
"store_spatial_files",
"overwrite_existing_layer",
"skip_existing_layers",
"source"
"source",
)

base_file = serializers.FileField()
Expand All @@ -24,4 +24,4 @@ class Meta:
store_spatial_files = serializers.BooleanField(required=False, default=True)
overwrite_existing_layer = serializers.BooleanField(required=False, default=False)
skip_existing_layers = serializers.BooleanField(required=False, default=False)
source = serializers.CharField(required=False, default='upload')
source = serializers.CharField(required=False, default="upload")
2 changes: 1 addition & 1 deletion importer/api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ def create(self, request, *args, **kwargs):
legacy_upload_name=_file.name,
action=action,
name=_file.name,
source=extracted_params.get('source'),
source=extracted_params.get("source"),
)

sig = import_orchestrator.s(
Expand Down
4 changes: 2 additions & 2 deletions importer/handlers/common/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def extract_params_from_data(_data, action=None):
@staticmethod
def perform_last_step(execution_id):
_exec = orchestrator.get_execution_object(execution_id)

_exec.output_params.update(
**{
"detail_url": [
Expand All @@ -81,7 +81,7 @@ def import_resource(self, files: dict, execution_id: str, **kwargs):
original_handler = orchestrator.load_handler(
dataset.resourcehandlerinfo_set.first().handler_module_path
)()

ResourceHandlerInfo.objects.create(
handler_module_path=dataset.resourcehandlerinfo_set.first().handler_module_path,
resource=dataset,
Expand Down
19 changes: 19 additions & 0 deletions importer/handlers/common/tests_vector.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from importer.handlers.common.vector import BaseVectorFileHandler, import_with_ogr2ogr
from django.contrib.auth import get_user_model
from importer import project_dir
from importer.handlers.gpkg.handler import GPKGFileHandler
from importer.orchestrator import orchestrator
from geonode.base.populate_test_data import create_single_dataset
from geonode.resource.models import ExecutionRequest
Expand All @@ -23,6 +24,7 @@ def setUpClass(cls):
cls.handler = BaseVectorFileHandler()
cls.valid_gpkg = f"{project_dir}/tests/fixture/valid.gpkg"
cls.invalid_gpkg = f"{project_dir}/tests/fixture/invalid.gpkg"
cls.no_crs_gpkg = f"{project_dir}/tests/fixture/noCrsTable.gpkg"
cls.user, _ = get_user_model().objects.get_or_create(username="admin")
cls.invalid_files = {"base_file": cls.invalid_gpkg}
cls.valid_files = {"base_file": cls.valid_gpkg}
Expand Down Expand Up @@ -296,3 +298,20 @@ def test_import_with_ogr2ogr_without_errors_should_call_the_right_command_if_dum
self.assertTrue("-f PGDump /vsistdout/" in _call_as_string)
self.assertTrue("psql -d" in _call_as_string)
self.assertFalse("-f PostgreSQL PG" in _call_as_string)

def test_select_valid_layers(self):
"""
The function should return only the datasets with a geometry
The other one are discarded
"""
all_layers = GPKGFileHandler().get_ogr2ogr_driver().Open(self.no_crs_gpkg)

with self.assertLogs(level="ERROR") as _log:
valid_layer = GPKGFileHandler()._select_valid_layers(all_layers)

self.assertIn(
"The following layer layer_styles does not have a Coordinate Reference System (CRS) and will be skipped.",
[x.message for x in _log.records],
)
self.assertEqual(1, len(valid_layer))
self.assertEqual("mattia_test", valid_layer[0].GetName())
21 changes: 20 additions & 1 deletion importer/handlers/common/vector.py
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,8 @@ def import_resource(self, files: dict, execution_id: str, **kwargs) -> str:
Internally will call the steps required to import the
data inside the geonode_data database
"""
layers = self.get_ogr2ogr_driver().Open(files.get("base_file"))
all_layers = self.get_ogr2ogr_driver().Open(files.get("base_file"))
layers = self._select_valid_layers(all_layers)
# for the moment we skip the dyanamic model creation
layer_count = len(layers)
logger.info(f"Total number of layers available: {layer_count}")
Expand All @@ -317,7 +318,11 @@ def import_resource(self, files: dict, execution_id: str, **kwargs) -> str:
dynamic_model = None
celery_group = None
try:
if len(layers) == 0:
raise Exception("No valid layers found")

# start looping on the layers available

for index, layer in enumerate(layers, start=1):
layer_name = self.fixup_name(layer.GetName())

Expand Down Expand Up @@ -397,6 +402,20 @@ def import_resource(self, files: dict, execution_id: str, **kwargs) -> str:
raise e
return

def _select_valid_layers(self, all_layers):
layers = []
for layer in all_layers:
try:
self.identify_authority(layer)
layers.append(layer)
except Exception as e:
logger.error(e)
logger.error(
f"The following layer {layer.GetName()} does not have a Coordinate Reference System (CRS) and will be skipped."
)
pass
return layers

def find_alternate_by_dataset(self, _exec_obj, layer_name, should_be_overwritten):
workspace = DataPublisher(None).workspace
dataset_available = Dataset.objects.filter(
Expand Down
4 changes: 2 additions & 2 deletions importer/handlers/shapefile/serializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class Meta:
"store_spatial_files",
"overwrite_existing_layer",
"skip_existing_layers",
"source"
"source",
)

base_file = serializers.FileField()
Expand All @@ -30,4 +30,4 @@ class Meta:
store_spatial_files = serializers.BooleanField(required=False, default=True)
overwrite_existing_layer = serializers.BooleanField(required=False, default=False)
skip_existing_layers = serializers.BooleanField(required=False, default=False)
source = serializers.CharField(required=False, default='upload')
source = serializers.CharField(required=False, default="upload")
2 changes: 1 addition & 1 deletion importer/handlers/xml/serializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ class Meta:

base_file = serializers.FileField()
dataset_title = serializers.CharField(required=True)
source = serializers.CharField(required=False, default='resource_file_upload')
source = serializers.CharField(required=False, default="resource_file_upload")
63 changes: 62 additions & 1 deletion importer/tests/end2end/test_end2end.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ def setUpClass(cls) -> None:
super().setUpClass()
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"
file_path = gisdata.VECTOR_DATA
filename = os.path.join(file_path, "san_andres_y_providencia_highway.shp")
cls.valid_shp = {
Expand Down Expand Up @@ -101,8 +102,8 @@ def _assertimport(self, payload, initial_name, overwrite=False, last_update=None
)
self.assertTrue(dataset.exists())

# check if the resource is in geoserver
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 overwrite:
self.assertTrue(dataset.first().last_updated > last_update)
Expand Down Expand Up @@ -154,6 +155,66 @@ def test_import_gpkg_overwrite(self):
self.cat.delete(layer)


class ImporterNoCRSImportTest(BaseImporterEndToEndTest):
@override_settings(ASYNC_SIGNALS=False)
@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_geopackage_with_no_crs_table(self):
layer = self.cat.get_layer("geonode:mattia_test")
if layer:
self.cat.delete(layer)
payload = {
"base_file": open(self.no_crs_gpkg, "rb"),
}
initial_name = "mattia_test"
with self.assertLogs(level="ERROR") as _log:
self._assertimport(payload, initial_name)

self.assertIn(
"The following layer layer_styles does not have a Coordinate Reference System (CRS) and will be skipped.",
[x.message for x in _log.records],
)
layer = self.cat.get_layer("geonode:mattia_test")
if layer:
self.cat.delete(layer)

@override_settings(ASYNC_SIGNALS=False)
@mock.patch.dict(os.environ, {"GEONODE_GEODATABASE": "test_geonode_data"})
@override_settings(
GEODATABASE_URL=f"{geourl.split('/geonode_data')[0]}/test_geonode_data"
)
@mock.patch(
"importer.handlers.common.vector.BaseVectorFileHandler._select_valid_layers"
)
def test_import_geopackage_with_no_crs_table_should_raise_error_if_all_layer_are_invalid(
self, _select_valid_layers
):
_select_valid_layers.return_value = []
layer = self.cat.get_layer("geonode:mattia_test")
if layer:
self.cat.delete(layer)

payload = {
"base_file": open(self.no_crs_gpkg, "rb"),
}

with self.assertLogs(level="ERROR") as _log:
self.client.force_login(self.admin)

response = self.client.post(self.url, data=payload)
self.assertEqual(500, response.status_code)

self.assertIn(
"No valid layers found",
[x.message for x in _log.records],
)
layer = self.cat.get_layer("geonode:mattia_test")
if layer:
self.cat.delete(layer)


class ImporterGeoJsonImportTest(BaseImporterEndToEndTest):
@mock.patch.dict(os.environ, {"GEONODE_GEODATABASE": "test_geonode_data"})
@override_settings(
Expand Down
Binary file added importer/tests/fixture/noCrsTable.gpkg
Binary file not shown.
2 changes: 1 addition & 1 deletion runtest.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 -v2 --noinput