From b57f222eb6fbdd609219876c915cc86d25019b2d Mon Sep 17 00:00:00 2001 From: Mattia Date: Wed, 31 Jul 2024 11:55:54 +0200 Subject: [PATCH 01/12] [Fixes #12456] Implement the ResourceHandler concept --- geonode/base/api/serializers.py | 43 ++-------------------- geonode/documents/handlers.py | 24 +++++++++++++ geonode/geoapps/handlers.py | 15 ++++++++ geonode/layers/handlers.py | 49 +++++++++++++++++++++++++ geonode/maps/handlers.py | 15 ++++++++ geonode/resource/apps.py | 5 +++ geonode/resource/handler.py | 63 +++++++++++++++++++++++++++++++++ geonode/resource/manager.py | 5 +++ geonode/settings.py | 9 +++++ 9 files changed, 188 insertions(+), 40 deletions(-) create mode 100644 geonode/documents/handlers.py create mode 100644 geonode/geoapps/handlers.py create mode 100644 geonode/layers/handlers.py create mode 100644 geonode/maps/handlers.py create mode 100644 geonode/resource/handler.py diff --git a/geonode/base/api/serializers.py b/geonode/base/api/serializers.py index 9a689e4839e..0a784eef406 100644 --- a/geonode/base/api/serializers.py +++ b/geonode/base/api/serializers.py @@ -28,7 +28,6 @@ from django.forms.models import model_to_dict from django.contrib.auth import get_user_model from django.db.models.query import QuerySet -from geonode.assets.utils import get_default_asset from geonode.people import Roles from django.http import QueryDict from deprecated import deprecated @@ -63,10 +62,9 @@ from geonode.geoapps.models import GeoApp from geonode.groups.models import GroupCategory, GroupProfile from geonode.base.api.fields import ComplexDynamicRelationField -from geonode.layers.utils import get_download_handlers, get_default_dataset_download_handler -from geonode.assets.handlers import asset_handler_registry +from geonode.resource.manager import resource_manager from geonode.utils import build_absolute_uri -from geonode.security.utils import get_resources_with_perms, get_geoapp_subtypes +from geonode.security.utils import get_resources_with_perms from geonode.resource.models import ExecutionRequest from django.contrib.gis.geos import Polygon @@ -301,42 +299,7 @@ def get_attribute(self, instance): logger.exception(e) raise e - asset = get_default_asset(_instance) - if asset is not None: - asset_url = asset_handler_registry.get_handler(asset).create_download_url(asset) - - if _instance.resource_type in ["map"] + get_geoapp_subtypes(): - return [] - elif _instance.resource_type in ["document"]: - payload = [ - { - "url": _instance.download_url, - "ajax_safe": _instance.download_is_ajax_safe, - }, - ] - if asset: - payload.append({"url": asset_url, "ajax_safe": False, "default": False}) - return payload - - elif _instance.resource_type in ["dataset"]: - download_urls = [] - # lets get only the default one first to set it - default_handler = get_default_dataset_download_handler() - obj = default_handler(self.context.get("request"), _instance.alternate) - if obj.download_url: - download_urls.append({"url": obj.download_url, "ajax_safe": obj.is_ajax_safe, "default": True}) - # then let's prepare the payload with everything - for handler in get_download_handlers(): - obj = handler(self.context.get("request"), _instance.alternate) - if obj.download_url: - 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}) - - return download_urls - else: - return [] + return resource_manager.get_manager(_instance).download_urls(**self.context) class FavoriteField(DynamicComputedField): diff --git a/geonode/documents/handlers.py b/geonode/documents/handlers.py new file mode 100644 index 00000000000..eb98c7f8f68 --- /dev/null +++ b/geonode/documents/handlers.py @@ -0,0 +1,24 @@ +from geonode.documents.models import Document +from geonode.resource.handler import BaseResourceHandler +import logging + +logger = logging.getLogger() + + +class DocumentHandler(BaseResourceHandler): + + @staticmethod + def can_handle(instance): + return isinstance(instance, Document) + + def download_urls(self): + """ + Specific method that return the download URL of the document + """ + super().download_urls() + return [ + { + "url": self.instance.download_url, + "ajax_safe": self.instance.download_is_ajax_safe, + }, + ] diff --git a/geonode/geoapps/handlers.py b/geonode/geoapps/handlers.py new file mode 100644 index 00000000000..40b433a1354 --- /dev/null +++ b/geonode/geoapps/handlers.py @@ -0,0 +1,15 @@ +from geonode.geoapps.models import GeoApp +from geonode.resource.handler import BaseResourceHandler +import logging + +logger = logging.getLogger() + + +class GeoAppHandler(BaseResourceHandler): + @staticmethod + def can_handle(instance): + return isinstance(instance, GeoApp) + + def download_urls(self): + logger.debug("Download is not available for maps") + return [] diff --git a/geonode/layers/handlers.py b/geonode/layers/handlers.py new file mode 100644 index 00000000000..adafe06506c --- /dev/null +++ b/geonode/layers/handlers.py @@ -0,0 +1,49 @@ +from geonode.assets.utils import get_default_asset +from geonode.base.models import ResourceBase +from geonode.layers.models import Dataset +from geonode.resource.handler import BaseResourceHandler +import logging +from geonode.assets.handlers import asset_handler_registry +from geonode.layers.utils import get_download_handlers, get_default_dataset_download_handler + +logger = logging.getLogger() + + +class Tiles3DHandler(BaseResourceHandler): + @staticmethod + def can_handle(instance): + return isinstance(instance, ResourceBase) and instance.subtype == "3dtiles" + + def download_urls(self, **kwargs): + """ + Specific method that return the download URL of the document + """ + super().download_urls() + asset = get_default_asset(self.instance) + if asset is not None: + asset_url = asset_handler_registry.get_handler(asset).create_download_url(asset) + return asset_url + + +class DatasetHandler(BaseResourceHandler): + @staticmethod + def can_handle(instance): + return isinstance(instance, Dataset) + + def download_urls(self, **kwargs): + super().download_urls() + """ + Specific method that return the download URL of the document + """ + download_urls = [] + # lets get only the default one first to set it + default_handler = get_default_dataset_download_handler() + obj = default_handler(kwargs.get("request"), self.instance.alternate) + if obj.download_url: + download_urls.append({"url": obj.download_url, "ajax_safe": obj.is_ajax_safe, "default": True}) + # then let's prepare the payload with everything + for handler in get_download_handlers(): + obj = handler(kwargs.get("request"), self.instance.alternate) + if obj.download_url: + download_urls.append({"url": obj.download_url, "ajax_safe": obj.is_ajax_safe, "default": False}) + return download_urls diff --git a/geonode/maps/handlers.py b/geonode/maps/handlers.py new file mode 100644 index 00000000000..1b0334ec768 --- /dev/null +++ b/geonode/maps/handlers.py @@ -0,0 +1,15 @@ +from geonode.maps.models import Map +from geonode.resource.handler import BaseResourceHandler +import logging + +logger = logging.getLogger() + + +class MapHandler(BaseResourceHandler): + @staticmethod + def can_handle(instance): + return isinstance(instance, Map) + + def download_urls(self, **kwargs): + logger.debug("Download is not available for maps") + return [] diff --git a/geonode/resource/apps.py b/geonode/resource/apps.py index 791f51317a7..94eeb231229 100644 --- a/geonode/resource/apps.py +++ b/geonode/resource/apps.py @@ -18,6 +18,8 @@ ######################################################################### from django.apps import AppConfig from django.urls import include, re_path +from django.conf import settings +from django.utils.module_loading import import_string class GeoNodeResourceConfig(AppConfig): @@ -28,3 +30,6 @@ def ready(self): from geonode.urls import urlpatterns urlpatterns += [re_path(r"^api/v2/", include("geonode.resource.api.urls"))] + + for el in settings.RESOURCE_HANDLERS: + import_string(el).register() diff --git a/geonode/resource/handler.py b/geonode/resource/handler.py new file mode 100644 index 00000000000..814e1e243f9 --- /dev/null +++ b/geonode/resource/handler.py @@ -0,0 +1,63 @@ +from abc import ABC +import logging + +logger = logging.getLogger(__file__) + + +class BaseResourceHandler(ABC): + """ + Base abstract resource handler object + define the required method needed to define a resource handler + As first implementation it will take care of the download url + and the download response for a resource + """ + + REGISTRY = [] + + def __init__(self) -> None: + self.resource = None + + def __str__(self): + return f"{self.__module__}.{self.__class__.__name__}" + + def __repr__(self): + return self.__str__() + + @classmethod + def init_class(cls, instance): + cls.instance = instance + + @classmethod + def register(cls): + BaseResourceHandler.REGISTRY.append(cls) + + @classmethod + def get_registry(cls): + return BaseResourceHandler.REGISTRY + + def get_handler_by_instance(self, instance): + """ + Given a resource, should return it's handler + """ + for handler in self.get_registry(): + if handler.can_handle(instance): + self.init_class(instance) + return handler() + logger.error("No handlers found for the given resource") + return None + + def download_urls(self, **kwargs): + """ + return the download url for each resource + """ + if not self.instance: + logger.warning("No instance declared, so is not possible to return the download url") + return None + + def download_response(self, **kwargs): + """ + Return the download response for the resource + """ + + +resource_hander = BaseResourceHandler() diff --git a/geonode/resource/manager.py b/geonode/resource/manager.py index eefab451b4a..bae12a532db 100644 --- a/geonode/resource/manager.py +++ b/geonode/resource/manager.py @@ -976,5 +976,10 @@ def set_thumbnail( logger.exception(e) return False + def get_manager(self, instance): + from geonode.resource.handler import resource_hander + + return resource_hander.get_handler_by_instance(instance=instance) + resource_manager = ResourceManager() diff --git a/geonode/settings.py b/geonode/settings.py index f1ea768baae..ef902195391 100644 --- a/geonode/settings.py +++ b/geonode/settings.py @@ -2379,3 +2379,12 @@ def get_geonode_catalogue_service(): ] INSTALLED_APPS += ("geonode.assets",) GEONODE_APPS += ("geonode.assets",) + + +RESOURCE_HANDLERS = [ + "geonode.layers.handlers.Tiles3DHandler", + "geonode.layers.handlers.DatasetHandler", + "geonode.maps.handlers.MapHandler", + "geonode.documents.handlers.DocumentHandler", + "geonode.geoapps.handlers.GeoAppHandler", +] From 4547fb60644b3cd2f2c1875dde2c550aad6cb25a Mon Sep 17 00:00:00 2001 From: Mattia Date: Wed, 31 Jul 2024 11:56:38 +0200 Subject: [PATCH 02/12] [Fixes #12456] Implement the ResourceHandler concept --- geonode/documents/handlers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/geonode/documents/handlers.py b/geonode/documents/handlers.py index eb98c7f8f68..4cffe871c73 100644 --- a/geonode/documents/handlers.py +++ b/geonode/documents/handlers.py @@ -11,7 +11,7 @@ class DocumentHandler(BaseResourceHandler): def can_handle(instance): return isinstance(instance, Document) - def download_urls(self): + def download_urls(self, **kwargs): """ Specific method that return the download URL of the document """ From 6e563b3c7c04c5458229ea20cd4df279b8afcb41 Mon Sep 17 00:00:00 2001 From: Mattia Date: Wed, 31 Jul 2024 12:38:45 +0200 Subject: [PATCH 03/12] [Fixes #12456] Implement the ResourceHandler concept --- geonode/geoapps/handlers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/geonode/geoapps/handlers.py b/geonode/geoapps/handlers.py index 40b433a1354..5ada9dc4140 100644 --- a/geonode/geoapps/handlers.py +++ b/geonode/geoapps/handlers.py @@ -10,6 +10,6 @@ class GeoAppHandler(BaseResourceHandler): def can_handle(instance): return isinstance(instance, GeoApp) - def download_urls(self): + def download_urls(self, **kwargs): logger.debug("Download is not available for maps") return [] From 20fde48599d13756c4abea7a30ad7deec0cb1e8e Mon Sep 17 00:00:00 2001 From: Mattia Date: Wed, 31 Jul 2024 12:50:13 +0200 Subject: [PATCH 04/12] [Fixes #12456] Implement the ResourceHandler concept --- geonode/base/api/serializers.py | 2 +- geonode/layers/handlers.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/geonode/base/api/serializers.py b/geonode/base/api/serializers.py index 23e9ff83aae..42a276cc07a 100644 --- a/geonode/base/api/serializers.py +++ b/geonode/base/api/serializers.py @@ -42,7 +42,7 @@ from avatar.templatetags.avatar_tags import avatar_url from geonode.utils import bbox_swap from geonode.base.api.exceptions import InvalidResourceException - +from geonode.assets.handlers import asset_handler_registry from geonode.favorite.models import Favorite from geonode.base.models import ( Link, diff --git a/geonode/layers/handlers.py b/geonode/layers/handlers.py index adafe06506c..fffc3a8f4c6 100644 --- a/geonode/layers/handlers.py +++ b/geonode/layers/handlers.py @@ -22,7 +22,7 @@ def download_urls(self, **kwargs): asset = get_default_asset(self.instance) if asset is not None: asset_url = asset_handler_registry.get_handler(asset).create_download_url(asset) - return asset_url + return [{"url": asset_url, "ajax_safe": True, "default": True}] class DatasetHandler(BaseResourceHandler): From dd7db07c118ad74f8328587e7c851b3bb98d6193 Mon Sep 17 00:00:00 2001 From: Mattia Date: Wed, 31 Jul 2024 15:13:17 +0200 Subject: [PATCH 05/12] [Fixes #12456] Implement the ResourceHandler concept --- geonode/base/api/tests.py | 3 +-- geonode/resource/handler.py | 5 +++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/geonode/base/api/tests.py b/geonode/base/api/tests.py index 4abd3e52e1c..37a59d8c29f 100644 --- a/geonode/base/api/tests.py +++ b/geonode/base/api/tests.py @@ -2490,8 +2490,7 @@ def test_base_resources_return_download_links_for_documents(self): asset = get_default_asset(doc) handler = asset_handler_registry.get_handler(asset) expected_payload = [ - {"url": build_absolute_uri(doc.download_url), "ajax_safe": doc.download_is_ajax_safe}, - {"ajax_safe": False, "default": False, "url": handler.create_download_url(asset)}, + {"url": build_absolute_uri(doc.download_url), "ajax_safe": doc.download_is_ajax_safe} ] # From resource base API json = self._get_for_object(doc, "base-resources-detail") diff --git a/geonode/resource/handler.py b/geonode/resource/handler.py index 814e1e243f9..3ab8979c626 100644 --- a/geonode/resource/handler.py +++ b/geonode/resource/handler.py @@ -15,7 +15,7 @@ class BaseResourceHandler(ABC): REGISTRY = [] def __init__(self) -> None: - self.resource = None + self.instance = None def __str__(self): return f"{self.__module__}.{self.__class__.__name__}" @@ -44,7 +44,7 @@ def get_handler_by_instance(self, instance): self.init_class(instance) return handler() logger.error("No handlers found for the given resource") - return None + return self def download_urls(self, **kwargs): """ @@ -53,6 +53,7 @@ def download_urls(self, **kwargs): if not self.instance: logger.warning("No instance declared, so is not possible to return the download url") return None + return [] def download_response(self, **kwargs): """ From 74bdc5bbb86332be792b1024b12a50918e92ad1e Mon Sep 17 00:00:00 2001 From: Mattia Date: Wed, 31 Jul 2024 15:13:51 +0200 Subject: [PATCH 06/12] [Fixes #12456] Implement the ResourceHandler concept --- geonode/base/api/tests.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/geonode/base/api/tests.py b/geonode/base/api/tests.py index 37a59d8c29f..9db34249832 100644 --- a/geonode/base/api/tests.py +++ b/geonode/base/api/tests.py @@ -57,8 +57,6 @@ from geonode.assets.utils import create_asset_and_link from geonode.maps.models import Map, MapLayer from geonode.tests.base import GeoNodeBaseTestSupport -from geonode.assets.utils import get_default_asset -from geonode.assets.handlers import asset_handler_registry from geonode.base import enumerations from geonode.base.api.serializers import ResourceBaseSerializer @@ -2487,11 +2485,7 @@ def test_base_resources_return_download_links_for_documents(self): Ensure we can access the Resource Base list. """ doc = Document.objects.first() - asset = get_default_asset(doc) - handler = asset_handler_registry.get_handler(asset) - expected_payload = [ - {"url": build_absolute_uri(doc.download_url), "ajax_safe": doc.download_is_ajax_safe} - ] + expected_payload = [{"url": build_absolute_uri(doc.download_url), "ajax_safe": doc.download_is_ajax_safe}] # From resource base API json = self._get_for_object(doc, "base-resources-detail") download_url = json.get("resource").get("download_urls") From 31547346847b193f467895576e31d64306e6cf9c Mon Sep 17 00:00:00 2001 From: Mattia Date: Thu, 1 Aug 2024 11:38:58 +0200 Subject: [PATCH 07/12] [Fixes #12456] Fix test and instance initialization --- geonode/documents/handlers.py | 2 +- geonode/resource/handler.py | 11 +++-------- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/geonode/documents/handlers.py b/geonode/documents/handlers.py index 4cffe871c73..2f54d8ad1dc 100644 --- a/geonode/documents/handlers.py +++ b/geonode/documents/handlers.py @@ -18,7 +18,7 @@ def download_urls(self, **kwargs): super().download_urls() return [ { - "url": self.instance.download_url, + "url": self.instance.download_url if not self.instance.doc_url else self.instance.doc_url, "ajax_safe": self.instance.download_is_ajax_safe, }, ] diff --git a/geonode/resource/handler.py b/geonode/resource/handler.py index 3ab8979c626..1ae43ee8595 100644 --- a/geonode/resource/handler.py +++ b/geonode/resource/handler.py @@ -14,8 +14,8 @@ class BaseResourceHandler(ABC): REGISTRY = [] - def __init__(self) -> None: - self.instance = None + def __init__(self, instance=None) -> None: + self.instance = instance def __str__(self): return f"{self.__module__}.{self.__class__.__name__}" @@ -23,10 +23,6 @@ def __str__(self): def __repr__(self): return self.__str__() - @classmethod - def init_class(cls, instance): - cls.instance = instance - @classmethod def register(cls): BaseResourceHandler.REGISTRY.append(cls) @@ -41,8 +37,7 @@ def get_handler_by_instance(self, instance): """ for handler in self.get_registry(): if handler.can_handle(instance): - self.init_class(instance) - return handler() + return handler(instance) logger.error("No handlers found for the given resource") return self From 8b96b36866377c3b477b869b0e863b7625f93f65 Mon Sep 17 00:00:00 2001 From: Mattia Date: Thu, 1 Aug 2024 13:22:38 +0200 Subject: [PATCH 08/12] [Fixes #12456] add test coverage --- geonode/base/populate_test_data.py | 4 +- geonode/layers/download_handler.py | 2 +- geonode/resource/test_handler.py | 116 +++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 2 deletions(-) create mode 100644 geonode/resource/test_handler.py diff --git a/geonode/base/populate_test_data.py b/geonode/base/populate_test_data.py index d8cc78afd18..80b031ff565 100644 --- a/geonode/base/populate_test_data.py +++ b/geonode/base/populate_test_data.py @@ -380,7 +380,7 @@ def dump_models(path=None): f.write(result) -def create_single_dataset(name, keywords=None, owner=None, group=None, **kwargs): +def create_single_dataset(name, keywords=None, owner=None, group=None, with_asset=False, **kwargs): admin, created = get_user_model().objects.get_or_create(username="admin") if created: admin.is_superuser = True @@ -415,6 +415,8 @@ def create_single_dataset(name, keywords=None, owner=None, group=None, **kwargs) if isinstance(keywords, list): dataset = add_keywords_to_resource(dataset, keywords) + if with_asset: + _, _ = create_asset_and_link(dataset, dataset.owner, dfile) dataset.set_default_permissions(owner=owner or admin) dataset.clear_dirty_state() diff --git a/geonode/layers/download_handler.py b/geonode/layers/download_handler.py index 7f154a6f30d..db0d5132559 100644 --- a/geonode/layers/download_handler.py +++ b/geonode/layers/download_handler.py @@ -94,7 +94,7 @@ def get_resource(self): self.request, self.resource_name, "base.download_resourcebase", - _("You do not have download permissions for this dataset."), + _("You do not have download permissions for this dataset.") ) except Exception as e: logger.debug(e) diff --git a/geonode/resource/test_handler.py b/geonode/resource/test_handler.py new file mode 100644 index 00000000000..ad621d29801 --- /dev/null +++ b/geonode/resource/test_handler.py @@ -0,0 +1,116 @@ +######################################################################### +# +# Copyright (C) 2021 OSGeo +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# +######################################################################### + + +from collections import namedtuple + +from django.urls import reverse +from geonode.assets.utils import get_default_asset +from geonode.base.populate_test_data import ( + create_single_dataset, + create_single_doc, + create_single_geoapp, + create_single_map, +) +from geonode.documents.handlers import DocumentHandler +from geonode.geoapps.handlers import GeoAppHandler +from geonode.layers.handlers import DatasetHandler, Tiles3DHandler +from geonode.maps.handlers import MapHandler +from geonode.tests.base import GeoNodeBaseTestSupport +from geonode.resource.manager import resource_manager +from geonode.utils import build_absolute_uri + + +class TestResourceManager(GeoNodeBaseTestSupport): + @classmethod + def setUpClass(cls) -> None: + super().setUpClass() + cls.dataset = create_single_dataset("dataset_for_handlers") + cls.map = create_single_map("map_for_handlers") + cls.document = create_single_doc("document_for_handlers") + cls.geoapp = create_single_geoapp("geoapp_for_handlers") + cls.tiles3d = create_single_dataset("tiles3d_for_handlers", with_asset=True) + cls.tiles3d.subtype = "3dtiles" + cls.tiles3d.save() + + def test_correct_resource_manager_is_called_for_dataset(self): + handler = resource_manager.get_manager(self.dataset) + self.assertIsNotNone(handler) + self.assertTrue(isinstance(handler, DatasetHandler)) + + def test_correct_resource_manager_is_called_for_doc(self): + handler = resource_manager.get_manager(self.document) + self.assertIsNotNone(handler) + self.assertTrue(isinstance(handler, DocumentHandler)) + + def test_correct_resource_manager_is_called_for_map(self): + handler = resource_manager.get_manager(self.map) + self.assertIsNotNone(handler) + self.assertTrue(isinstance(handler, MapHandler)) + + def test_correct_resource_manager_is_called_for_geoapp(self): + handler = resource_manager.get_manager(self.geoapp) + self.assertIsNotNone(handler) + self.assertTrue(isinstance(handler, GeoAppHandler)) + + def test_correct_resource_manager_is_called_for_3dtiles(self): + handler = resource_manager.get_manager(self.tiles3d) + self.assertIsNotNone(handler) + self.assertTrue(isinstance(handler, Tiles3DHandler)) + + def test_correct_download_url_for_dataset(self): + handler = resource_manager.get_manager(self.dataset) + self.assertIsNotNone(handler) + + expected_payload = [{"url": self.dataset.download_url, "ajax_safe": True, "default": True}] + + self.assertListEqual(handler.download_urls(), expected_payload) + + def test_correct_download_url_for_doc(self): + handler = resource_manager.get_manager(self.document) + self.assertIsNotNone(handler) + expected_payload = [{"url": self.document.download_url, "ajax_safe": True}] + + self.assertListEqual(handler.download_urls(), expected_payload) + + def test_correct_download_url_for_map(self): + handler = resource_manager.get_manager(self.map) + expected_payload = [] + + self.assertListEqual(handler.download_urls(), expected_payload) + + def test_correct_download_url_for_geoapp(self): + handler = resource_manager.get_manager(self.geoapp) + self.assertIsNotNone(handler) + expected_payload = [] + self.assertListEqual(handler.download_urls(), expected_payload) + + def test_correct_download_url_for_3dtiles(self): + handler = resource_manager.get_manager(self.tiles3d) + asset = get_default_asset(self.tiles3d) + self.assertIsNotNone(handler) + expected_payload = [ + { + "url": build_absolute_uri(reverse("assets-download", kwargs={"pk": asset.pk})), + "ajax_safe": True, + "default": True, + } + ] + + self.assertListEqual(handler.download_urls(), expected_payload) From b2c03bc7a735767f1569b8040e6ed301eed4abf8 Mon Sep 17 00:00:00 2001 From: Mattia Date: Fri, 20 Sep 2024 15:04:01 +0200 Subject: [PATCH 09/12] [Fixes #12456] merge with master --- geonode/layers/download_handler.py | 2 +- geonode/resource/test_handler.py | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/geonode/layers/download_handler.py b/geonode/layers/download_handler.py index db0d5132559..7f154a6f30d 100644 --- a/geonode/layers/download_handler.py +++ b/geonode/layers/download_handler.py @@ -94,7 +94,7 @@ def get_resource(self): self.request, self.resource_name, "base.download_resourcebase", - _("You do not have download permissions for this dataset.") + _("You do not have download permissions for this dataset."), ) except Exception as e: logger.debug(e) diff --git a/geonode/resource/test_handler.py b/geonode/resource/test_handler.py index ad621d29801..b36608e8d46 100644 --- a/geonode/resource/test_handler.py +++ b/geonode/resource/test_handler.py @@ -18,8 +18,6 @@ ######################################################################### -from collections import namedtuple - from django.urls import reverse from geonode.assets.utils import get_default_asset from geonode.base.populate_test_data import ( From 5c109383fe92e54165953f7393d3ad3239d2ce9c Mon Sep 17 00:00:00 2001 From: Mattia Date: Fri, 20 Sep 2024 15:56:02 +0200 Subject: [PATCH 10/12] [Fixes #12456] Fix PR comments --- geonode/resource/apps.py | 6 ++---- geonode/resource/handler.py | 35 ++++++++++++++++++++++------------- geonode/resource/manager.py | 2 +- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/geonode/resource/apps.py b/geonode/resource/apps.py index 94eeb231229..97a5e716216 100644 --- a/geonode/resource/apps.py +++ b/geonode/resource/apps.py @@ -18,8 +18,7 @@ ######################################################################### from django.apps import AppConfig from django.urls import include, re_path -from django.conf import settings -from django.utils.module_loading import import_string +from geonode.resource.handler import resource_registry class GeoNodeResourceConfig(AppConfig): @@ -31,5 +30,4 @@ def ready(self): urlpatterns += [re_path(r"^api/v2/", include("geonode.resource.api.urls"))] - for el in settings.RESOURCE_HANDLERS: - import_string(el).register() + resource_registry.init_registry() diff --git a/geonode/resource/handler.py b/geonode/resource/handler.py index 1ae43ee8595..512ee9dca3d 100644 --- a/geonode/resource/handler.py +++ b/geonode/resource/handler.py @@ -1,9 +1,28 @@ from abc import ABC import logging +from django.conf import settings +from django.utils.module_loading import import_string + logger = logging.getLogger(__file__) +class ResourceHandlerRegistry: + + REGISTRY = [] + + def init_registry(self): + self.register() + + def register(self): + for module_path in settings.RESOURCE_HANDLERS: + self.REGISTRY.append(import_string(module_path)) + + @classmethod + def get_registry(cls): + return ResourceHandlerRegistry.REGISTRY + + class BaseResourceHandler(ABC): """ Base abstract resource handler object @@ -11,9 +30,6 @@ class BaseResourceHandler(ABC): As first implementation it will take care of the download url and the download response for a resource """ - - REGISTRY = [] - def __init__(self, instance=None) -> None: self.instance = instance @@ -23,19 +39,11 @@ def __str__(self): def __repr__(self): return self.__str__() - @classmethod - def register(cls): - BaseResourceHandler.REGISTRY.append(cls) - - @classmethod - def get_registry(cls): - return BaseResourceHandler.REGISTRY - - def get_handler_by_instance(self, instance): + def get_handler(self, instance): """ Given a resource, should return it's handler """ - for handler in self.get_registry(): + for handler in resource_registry.get_registry(): if handler.can_handle(instance): return handler(instance) logger.error("No handlers found for the given resource") @@ -56,4 +64,5 @@ def download_response(self, **kwargs): """ +resource_registry = ResourceHandlerRegistry() resource_hander = BaseResourceHandler() diff --git a/geonode/resource/manager.py b/geonode/resource/manager.py index 67e741c9925..e088af71cd7 100644 --- a/geonode/resource/manager.py +++ b/geonode/resource/manager.py @@ -981,7 +981,7 @@ def set_thumbnail( def get_manager(self, instance): from geonode.resource.handler import resource_hander - return resource_hander.get_handler_by_instance(instance=instance) + return resource_hander.get_handler(instance=instance) resource_manager = ResourceManager() From b84f85bd1bc9280cca2d35a220330f6f5f43b2fa Mon Sep 17 00:00:00 2001 From: Mattia Date: Fri, 20 Sep 2024 16:09:07 +0200 Subject: [PATCH 11/12] [Fixes #12456] Fix PR comments --- geonode/base/api/serializers.py | 2 +- geonode/documents/handlers.py | 3 +-- geonode/resource/handler.py | 22 ++++++++++++---------- geonode/resource/manager.py | 6 +++--- geonode/resource/test_handler.py | 20 ++++++++++---------- 5 files changed, 27 insertions(+), 26 deletions(-) diff --git a/geonode/base/api/serializers.py b/geonode/base/api/serializers.py index 510eb1a5327..e4e013440f8 100644 --- a/geonode/base/api/serializers.py +++ b/geonode/base/api/serializers.py @@ -299,7 +299,7 @@ def get_attribute(self, instance): logger.exception(e) raise e - return resource_manager.get_manager(_instance).download_urls(**self.context) + return resource_manager.get_handler(_instance).download_urls(**self.context) class FavoriteField(DynamicComputedField): diff --git a/geonode/documents/handlers.py b/geonode/documents/handlers.py index 2f54d8ad1dc..3df83cdac11 100644 --- a/geonode/documents/handlers.py +++ b/geonode/documents/handlers.py @@ -15,10 +15,9 @@ def download_urls(self, **kwargs): """ Specific method that return the download URL of the document """ - super().download_urls() return [ { "url": self.instance.download_url if not self.instance.doc_url else self.instance.doc_url, "ajax_safe": self.instance.download_is_ajax_safe, }, - ] + ] or super().download_urls() diff --git a/geonode/resource/handler.py b/geonode/resource/handler.py index 512ee9dca3d..c62bdcd1bd2 100644 --- a/geonode/resource/handler.py +++ b/geonode/resource/handler.py @@ -22,6 +22,16 @@ def register(self): def get_registry(cls): return ResourceHandlerRegistry.REGISTRY + def get_handler(self, instance): + """ + Given a resource, should return it's handler + """ + for handler in self.get_registry(): + if handler.can_handle(instance): + return handler(instance) + logger.error("No handlers found for the given resource") + return None + class BaseResourceHandler(ABC): """ @@ -30,6 +40,7 @@ class BaseResourceHandler(ABC): As first implementation it will take care of the download url and the download response for a resource """ + def __init__(self, instance=None) -> None: self.instance = instance @@ -39,16 +50,6 @@ def __str__(self): def __repr__(self): return self.__str__() - def get_handler(self, instance): - """ - Given a resource, should return it's handler - """ - for handler in resource_registry.get_registry(): - if handler.can_handle(instance): - return handler(instance) - logger.error("No handlers found for the given resource") - return self - def download_urls(self, **kwargs): """ return the download url for each resource @@ -62,6 +63,7 @@ def download_response(self, **kwargs): """ Return the download response for the resource """ + raise NotImplementedError() resource_registry = ResourceHandlerRegistry() diff --git a/geonode/resource/manager.py b/geonode/resource/manager.py index e088af71cd7..9f2d5c94e37 100644 --- a/geonode/resource/manager.py +++ b/geonode/resource/manager.py @@ -978,10 +978,10 @@ def set_thumbnail( logger.exception(e) return False - def get_manager(self, instance): - from geonode.resource.handler import resource_hander + def get_handler(self, instance): + from geonode.resource.handler import resource_registry - return resource_hander.get_handler(instance=instance) + return resource_registry.get_handler(instance=instance) resource_manager = ResourceManager() diff --git a/geonode/resource/test_handler.py b/geonode/resource/test_handler.py index b36608e8d46..680662d8dd7 100644 --- a/geonode/resource/test_handler.py +++ b/geonode/resource/test_handler.py @@ -48,32 +48,32 @@ def setUpClass(cls) -> None: cls.tiles3d.save() def test_correct_resource_manager_is_called_for_dataset(self): - handler = resource_manager.get_manager(self.dataset) + handler = resource_manager.get_handler(self.dataset) self.assertIsNotNone(handler) self.assertTrue(isinstance(handler, DatasetHandler)) def test_correct_resource_manager_is_called_for_doc(self): - handler = resource_manager.get_manager(self.document) + handler = resource_manager.get_handler(self.document) self.assertIsNotNone(handler) self.assertTrue(isinstance(handler, DocumentHandler)) def test_correct_resource_manager_is_called_for_map(self): - handler = resource_manager.get_manager(self.map) + handler = resource_manager.get_handler(self.map) self.assertIsNotNone(handler) self.assertTrue(isinstance(handler, MapHandler)) def test_correct_resource_manager_is_called_for_geoapp(self): - handler = resource_manager.get_manager(self.geoapp) + handler = resource_manager.get_handler(self.geoapp) self.assertIsNotNone(handler) self.assertTrue(isinstance(handler, GeoAppHandler)) def test_correct_resource_manager_is_called_for_3dtiles(self): - handler = resource_manager.get_manager(self.tiles3d) + handler = resource_manager.get_handler(self.tiles3d) self.assertIsNotNone(handler) self.assertTrue(isinstance(handler, Tiles3DHandler)) def test_correct_download_url_for_dataset(self): - handler = resource_manager.get_manager(self.dataset) + handler = resource_manager.get_handler(self.dataset) self.assertIsNotNone(handler) expected_payload = [{"url": self.dataset.download_url, "ajax_safe": True, "default": True}] @@ -81,26 +81,26 @@ def test_correct_download_url_for_dataset(self): self.assertListEqual(handler.download_urls(), expected_payload) def test_correct_download_url_for_doc(self): - handler = resource_manager.get_manager(self.document) + handler = resource_manager.get_handler(self.document) self.assertIsNotNone(handler) expected_payload = [{"url": self.document.download_url, "ajax_safe": True}] self.assertListEqual(handler.download_urls(), expected_payload) def test_correct_download_url_for_map(self): - handler = resource_manager.get_manager(self.map) + handler = resource_manager.get_handler(self.map) expected_payload = [] self.assertListEqual(handler.download_urls(), expected_payload) def test_correct_download_url_for_geoapp(self): - handler = resource_manager.get_manager(self.geoapp) + handler = resource_manager.get_handler(self.geoapp) self.assertIsNotNone(handler) expected_payload = [] self.assertListEqual(handler.download_urls(), expected_payload) def test_correct_download_url_for_3dtiles(self): - handler = resource_manager.get_manager(self.tiles3d) + handler = resource_manager.get_handler(self.tiles3d) asset = get_default_asset(self.tiles3d) self.assertIsNotNone(handler) expected_payload = [ From 880d9746e718b6685c094531bb56821abad8fd9a Mon Sep 17 00:00:00 2001 From: Mattia Date: Fri, 20 Sep 2024 16:41:00 +0200 Subject: [PATCH 12/12] [Fixes #12456] Fix tests --- geonode/resource/handler.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/geonode/resource/handler.py b/geonode/resource/handler.py index c62bdcd1bd2..f771bba7e03 100644 --- a/geonode/resource/handler.py +++ b/geonode/resource/handler.py @@ -30,7 +30,7 @@ def get_handler(self, instance): if handler.can_handle(instance): return handler(instance) logger.error("No handlers found for the given resource") - return None + return BaseResourceHandler() class BaseResourceHandler(ABC): @@ -67,4 +67,3 @@ def download_response(self, **kwargs): resource_registry = ResourceHandlerRegistry() -resource_hander = BaseResourceHandler()