Skip to content
2 changes: 1 addition & 1 deletion geonode/base/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ def get_attribute(self, instance):
download_urls.append({"url": obj.download_url, "ajax_safe": obj.is_ajax_safe, "default": False})

if asset:
download_urls.append({"url": asset_url, "ajax_safe": True, "default": False if download_urls else True})
download_urls.append({"url": asset_url, "ajax_safe": True, "default": False})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The change on this line unconditionally sets "default": False for the asset download URL. Previously, if no other download URLs were present (download_urls was empty), the asset would be marked as "default": True.

Given the changes in geonode/layers/download_handler.py where DatasetDownloadHandler.download_url now returns None for non-link resources, it's likely that download_urls will often be empty before the asset is added. In such cases, the asset might be the only available download option, and it should logically be marked as the default.

This change could lead to a scenario where no download option is marked as default, potentially breaking client-side logic that expects at least one default download URL.

Suggested change
download_urls.append({"url": asset_url, "ajax_safe": True, "default": False})
download_urls.append({"url": asset_url, "ajax_safe": True, "default": False if download_urls else True})


return download_urls
else:
Expand Down
4 changes: 1 addition & 3 deletions geonode/base/api/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -2885,9 +2885,7 @@ def test_base_resources_return_download_links_for_datasets(self):
Ensure we can access the Resource Base list.
"""
_dataset = Dataset.objects.first()
expected_payload = [
{"url": reverse("dataset_download", args=[_dataset.alternate]), "ajax_safe": True, "default": True}
]
expected_payload = []

# From resource base API
json = self._get_for_object(_dataset, "base-resources-detail")
Expand Down
7 changes: 1 addition & 6 deletions geonode/layers/api/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -682,19 +682,14 @@ def test_download_api(self):
response = self.client.get(url)
self.assertTrue(response.status_code == 200)
data = response.json()["dataset"]
download_url_data = data["download_urls"][0]
download_url = reverse("dataset_download", args=[dataset.alternate])
self.assertEqual(download_url_data["default"], True)
self.assertEqual(download_url_data["ajax_safe"], True)
self.assertEqual(download_url_data["url"], download_url)
self.assertEqual(data["download_urls"], [])

link = Link(link_type="original", url="https://myoriginal.org", resource=dataset)
link.save()

response = self.client.get(url)
data = response.json()["dataset"]
download_url_data = data["download_urls"][0]
download_url = reverse("dataset_download", args=[dataset.alternate])
self.assertEqual(download_url_data["default"], True)
self.assertEqual(download_url_data["ajax_safe"], False)
self.assertEqual(download_url_data["url"], "https://myoriginal.org")
85 changes: 4 additions & 81 deletions geonode/layers/download_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,11 @@
#########################################################################

import logging
import xml.etree.ElementTree as ET

from django.http import Http404, HttpResponse, HttpResponseRedirect, JsonResponse
from django.template.loader import get_template
from django.urls import reverse
from django.http import Http404
from django.utils.translation import gettext_lazy as _
from django.conf import settings
from geonode.base.auth import get_or_create_token
from geonode.geoserver.helpers import wps_format_is_supported
from geonode.layers.views import _resolve_dataset
from geonode.proxy.views import fetch_response_headers
from geonode.utils import HttpClient

logger = logging.getLogger("geonode.layers.download_handler")

Expand All @@ -51,11 +44,7 @@ def get_download_response(self):
Basic method. Should return the Response object
that allow the resource download
"""
resource = self.get_resource()
if not resource:
raise Http404("Resource requested is not available")
response = self.process_dowload(resource)
return response
raise Http404("Direct download for the requested resource is not supported")

@property
def is_link_resource(self):
Expand All @@ -82,7 +71,7 @@ def download_url(self):
if self.is_link_resource:
return resource.link_set.filter(resource=resource.get_self_resource(), link_type="original").first().url

return reverse("dataset_download", args=[resource.alternate])
return None

def get_resource(self):
"""
Expand All @@ -99,70 +88,4 @@ def get_resource(self):
except Exception as e:
logger.debug(e)

return self._resource

def process_dowload(self, resource=None):
"""
Generate the response object
"""
if not resource:
resource = self.get_resource()
if not settings.USE_GEOSERVER:
# if GeoServer is not used, we redirect to the proxy download
return HttpResponseRedirect(reverse("download", args=[resource.id]))

download_format = self.request.GET.get("export_format")

if download_format and not wps_format_is_supported(download_format, resource.subtype):
logger.error("The format provided is not valid for the selected resource")
return JsonResponse({"error": "The format provided is not valid for the selected resource"}, status=500)

_format = "application/zip" if resource.is_vector() else "image/tiff"
# getting default payload
tpl = get_template("geoserver/dataset_download.xml")
ctx = {"alternate": resource.alternate, "download_format": download_format or _format}
# applying context for the payload
payload = tpl.render(ctx)

# init of Client
client = HttpClient()

headers = {"Content-type": "application/xml", "Accept": "application/xml"}

# defining the URL needed fr the download
url = f"{settings.OGC_SERVER['default']['LOCATION']}ows?service=WPS&version=1.0.0&REQUEST=Execute"
if not self.request.user.is_anonymous:
# define access token for the user
access_token = get_or_create_token(self.request.user)
url += f"&access_token={access_token}"

# request to geoserver
response, content = client.request(url=url, data=payload, method="post", headers=headers)

if not response or response.status_code != 200:
logger.error(f"Download dataset exception: error during call with GeoServer: {content}")
return JsonResponse(
{"error": "Download dataset exception: error during call with GeoServer"},
status=500,
)

# error handling
namespaces = {"ows": "http://www.opengis.net/ows/1.1", "wps": "http://www.opengis.net/wps/1.0.0"}
response_type = response.headers.get("Content-Type")
if response_type == "text/xml":
# parsing XML for get exception
content = ET.fromstring(response.text)
exc = content.find("*//ows:Exception", namespaces=namespaces) or content.find(
"ows:Exception", namespaces=namespaces
)
if exc:
exc_text = exc.find("ows:ExceptionText", namespaces=namespaces)
logger.error(f"{exc.attrib.get('exceptionCode')} {exc_text.text}")
return JsonResponse({"error": f"{exc.attrib.get('exceptionCode')}: {exc_text.text}"}, status=500)

return_response = fetch_response_headers(
HttpResponse(content=response.content, status=response.status_code, content_type=download_format),
response.headers,
)
return_response.headers["Content-Type"] = download_format or _format
return return_response
return self._resource
75 changes: 24 additions & 51 deletions geonode/layers/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -644,35 +644,26 @@ def test_dataset_download_not_found_for_non_existing_dataset(self):
self.assertEqual(404, response.status_code)

@override_settings(USE_GEOSERVER=False)
def test_dataset_download_redirect_to_proxy_url(self):
# if settings.USE_GEOSERVER is false, the URL must be redirected
def test_dataset_download_returns_404(self):
self.client.login(username="admin", password="admin")
dataset = Dataset.objects.first()
url = reverse("dataset_download", args=[dataset.alternate])
response = self.client.get(url)
self.assertEqual(302, response.status_code)
self.assertEqual(f"/download/{dataset.id}", response.url)
self.assertEqual(404, response.status_code)

def test_dataset_download_invalid_wps_format(self):
# if settings.USE_GEOSERVER is false, the URL must be redirected
def test_dataset_download_invalid_format(self):
self.client.login(username="admin", password="admin")
dataset = Dataset.objects.first()
url = reverse("dataset_download", args=[dataset.alternate])
response = self.client.get(f"{url}?export_format=foo")
self.assertEqual(500, response.status_code)
self.assertDictEqual({"error": "The format provided is not valid for the selected resource"}, response.json())
self.assertEqual(404, response.status_code)

@patch("geonode.layers.download_handler.HttpClient.request")
def test_dataset_download_call_the_catalog_raise_error_for_no_200(self, mocked_catalog):
_response = MagicMock(status_code=500, content="foo-bar")
mocked_catalog.return_value = _response, "foo-bar"
# if settings.USE_GEOSERVER is false, the URL must be redirected
def test_dataset_download_no_geoserver_call(self):
self.client.login(username="admin", password="admin")
dataset = Dataset.objects.first()
url = reverse("dataset_download", args=[dataset.alternate])
response = self.client.get(url)
self.assertEqual(500, response.status_code)
self.assertDictEqual({"error": "Download dataset exception: error during call with GeoServer"}, response.json())
self.assertEqual(404, response.status_code)

def test_dataset_download_call_the_catalog_raise_error_for_error_content(self):
content = """<?xml version="1.0" encoding="UTF-8"?>
Expand All @@ -686,24 +677,23 @@ def test_dataset_download_call_the_catalog_raise_error_for_error_content(self):
# if settings.USE_GEOSERVER is false, the URL must be redirected
self.client.login(username="admin", password="admin")
dataset = Dataset.objects.first()
with patch("geonode.layers.download_handler.HttpClient.request") as mocked_catalog:
with patch("geonode.utils.HttpClient.request") as mocked_catalog:
mocked_catalog.return_value = _response, content
url = reverse("dataset_download", args=[dataset.alternate])
response = self.client.get(url)
self.assertEqual(500, response.status_code)
self.assertDictEqual({"error": "InvalidParameterValue: Foo Bar Exception"}, response.json())
self.assertEqual(404, response.status_code)

def test_dataset_download_call_the_catalog_works(self):
def test_dataset_download_call_the_catalog(self):
# if settings.USE_GEOSERVER is false, the URL must be redirected
_response = MagicMock(status_code=200, text="", headers={"Content-Type": ""}) # noqa
self.client.login(username="admin", password="admin")
dataset = Dataset.objects.first()
layer = create_dataset(dataset.title, dataset.title, dataset.owner, "Point")
with patch("geonode.layers.download_handler.HttpClient.request") as mocked_catalog:
with patch("geonode.utils.HttpClient.request") as mocked_catalog:
mocked_catalog.return_value = _response, ""
url = reverse("dataset_download", args=[layer.alternate])
response = self.client.get(url)
self.assertTrue(response.status_code == 200)
self.assertTrue(response.status_code == 404)

def test_dataset_download_call_the_catalog_not_work_without_download_resurcebase_perm(self):
dataset = Dataset.objects.first()
Expand All @@ -713,55 +703,42 @@ def test_dataset_download_call_the_catalog_not_work_without_download_resurcebase
response = self.client.get(url)
self.assertEqual(404, response.status_code)

def test_dataset_download_call_the_catalog_work_anonymous(self):
# if settings.USE_GEOSERVER is false, the URL must be redirected
def test_dataset_download_anonymous(self):
_response = MagicMock(status_code=200, text="", headers={"Content-Type": ""}) # noqa
dataset = Dataset.objects.first()
layer = create_dataset(dataset.title, dataset.title, dataset.owner, "Point")
with patch("geonode.layers.download_handler.HttpClient.request") as mocked_catalog:
with patch("geonode.utils.HttpClient.request") as mocked_catalog:
mocked_catalog.return_value = _response, ""
url = reverse("dataset_download", args=[layer.alternate])
response = self.client.get(url)
self.assertTrue(response.status_code == 200)
self.assertTrue(response.status_code == 404)

@override_settings(USE_GEOSERVER=True)
@patch("geonode.layers.download_handler.get_template")
def test_dataset_download_call_the_catalog_work_for_raster(self, pathed_template):
@patch("django.template.loader.get_template")
def test_dataset_download_call_the_catalog_for_raster(self, pathed_template):
# if settings.USE_GEOSERVER is false, the URL must be redirected
_response = MagicMock(status_code=200, text="", headers={"Content-Type": ""}) # noqa
dataset = Dataset.objects.filter(subtype="raster").first()
layer = create_dataset(dataset.title, dataset.title, dataset.owner, "Point")
Dataset.objects.filter(alternate=layer.alternate).update(subtype="raster")
with patch("geonode.layers.download_handler.HttpClient.request") as mocked_catalog:
with patch("geonode.utils.HttpClient.request") as mocked_catalog:
mocked_catalog.return_value = _response, ""
url = reverse("dataset_download", args=[layer.alternate])
response = self.client.get(url)
self.assertTrue(response.status_code == 200)
"""
Evaluate that the context used by the template contains the right mimetype for the resource
"""
self.assertTupleEqual(
({"alternate": layer.alternate, "download_format": "image/tiff"},), pathed_template.mock_calls[1].args
)
self.assertTrue(response.status_code == 404)

@override_settings(USE_GEOSERVER=True)
@patch("geonode.layers.download_handler.get_template")
def test_dataset_download_call_the_catalog_work_for_vector(self, pathed_template):
@patch("django.template.loader.get_template")
def test_dataset_download_call_the_catalog_not_work_for_vector(self, pathed_template):
# if settings.USE_GEOSERVER is false, the URL must be redirected
_response = MagicMock(status_code=200, text="", headers={"Content-Type": ""}) # noqa
dataset = Dataset.objects.filter(subtype="vector").first()
layer = create_dataset(dataset.title, dataset.title, dataset.owner, "Point")
with patch("geonode.layers.download_handler.HttpClient.request") as mocked_catalog:
with patch("geonode.utils.HttpClient.request") as mocked_catalog:
mocked_catalog.return_value = _response, ""
url = reverse("dataset_download", args=[layer.alternate])
response = self.client.get(url)
self.assertTrue(response.status_code == 200)
"""
Evaluate that the context used by the template contains the right mimetype for the resource
"""
self.assertTupleEqual(
({"alternate": layer.alternate, "download_format": "application/zip"},), pathed_template.mock_calls[1].args
)
self.assertTrue(response.status_code == 404)

@patch.object(Dataset, "get_choices", new_callable=PropertyMock)
def test_supports_time_with_vector_time_subtype(self, mock_get_choices):
Expand Down Expand Up @@ -1327,8 +1304,8 @@ def setUp(self):
self.sut = DatasetDownloadHandler(request, self.dataset.alternate)

def test_download_url_without_original_link(self):
expected_url = reverse("dataset_download", args=[self.dataset.alternate])
self.assertEqual(expected_url, self.sut.download_url)

self.assertIsNone(self.sut.download_url)

def test_download_url_with_original_link(self):
Link.objects.update_or_create(
Expand All @@ -1347,10 +1324,6 @@ def test_download_url_with_original_link(self):
def test_get_resource_exists(self):
self.assertIsNotNone(self.sut.get_resource())

def test_process_dowload(self):
response = self.sut.get_download_response()
self.assertIsNotNone(response)


class DummyDownloadHandler(DatasetDownloadHandler):
def get_download_response(self):
Expand Down
Loading