diff --git a/geonode/api/tests.py b/geonode/api/tests.py index 448c6775bfb..3a8d7a41f0a 100644 --- a/geonode/api/tests.py +++ b/geonode/api/tests.py @@ -27,6 +27,7 @@ from django.urls import reverse from django.contrib.auth.models import Group from django.contrib.auth import get_user_model +from django.test import SimpleTestCase from django.test.utils import override_settings from guardian.shortcuts import get_anonymous_user @@ -1387,3 +1388,46 @@ def test_delete_asset_no_permission(self): response = self.client.delete(url) self.assertEqual(response.status_code, 403) self.assertTrue(Asset.objects.filter(pk=self.asset1.pk).exists()) + + +class RouterUrlpatternsCompletenessTest(SimpleTestCase): + """Guards the shared /api/v2/ router against app-loading-order regressions.""" + + def test_router_is_complete(self): + from geonode.api.urls import router + + names = {getattr(pattern, "name", None) for pattern in router.urls} + + # first 5 were previously only reachable via a ready() hook or a stale router.urls read; + # base-resources-list is a baseline that was never affected + for expected in ( + "executionrequest-list", + "upload-size-limits-list", + "upload-parallelism-limits-list", + "assets-list", + "metadata-list", + "base-resources-list", + ): + with self.subTest(name=expected): + self.assertIn(expected, names) + + def test_resource_and_harvesting_no_longer_inject_urls_from_ready(self): + import inspect + + import geonode.harvesting.apps + import geonode.resource.apps + + for module in (geonode.resource.apps, geonode.harvesting.apps): + with self.subTest(module=module.__name__): + source = inspect.getsource(module) + self.assertNotIn("geonode.urls", source) + self.assertNotIn("urlpatterns", source) + + def test_upload_no_longer_has_the_dead_override_mechanism(self): + import inspect + + import geonode.upload.apps + + source = inspect.getsource(geonode.upload.apps) + self.assertNotIn("run_setup_hooks", source) + self.assertNotIn("url_already_injected", source) diff --git a/geonode/facets/urls.py b/geonode/facets/urls.py index 8d96976e53c..94367ad3c3f 100644 --- a/geonode/facets/urls.py +++ b/geonode/facets/urls.py @@ -20,7 +20,8 @@ from django.urls import path from .views import ListFacetsView, GetFacetView +# "facets" prefix comes from the include() in geonode/urls.py. urlpatterns = [ - path("facets", ListFacetsView.as_view(), name="list_facets"), - path("facets/", GetFacetView.as_view(), name="get_facet"), + path("", ListFacetsView.as_view(), name="list_facets"), + path("/", GetFacetView.as_view(), name="get_facet"), ] diff --git a/geonode/harvesting/apps.py b/geonode/harvesting/apps.py index eee03c89a34..c85761f1355 100644 --- a/geonode/harvesting/apps.py +++ b/geonode/harvesting/apps.py @@ -19,7 +19,6 @@ from django.apps import AppConfig from django.conf import settings -from django.urls import include, re_path from . import config @@ -28,10 +27,8 @@ class HarvestingAppConfig(AppConfig): name = "geonode.harvesting" def ready(self): - from geonode.urls import urlpatterns from . import signals # noqa - urlpatterns += [re_path(r"^api/v2/", include("geonode.harvesting.api.urls"))] settings.CELERY_BEAT_SCHEDULE["harvesting-scheduler"] = { "task": "geonode.harvesting.tasks.harvesting_scheduler", "schedule": config.get_setting("HARVESTER_SCHEDULER_FREQUENCY_MINUTES") * 0.5, diff --git a/geonode/management_commands_http/urls.py b/geonode/management_commands_http/urls.py index d7277e401b3..30a82f5ff3e 100644 --- a/geonode/management_commands_http/urls.py +++ b/geonode/management_commands_http/urls.py @@ -22,9 +22,10 @@ from geonode.management_commands_http.routers import router +# "management/" prefix comes from the include() in geonode/urls.py. urlpatterns = [ - re_path(r"management/commands/$", ManagementCommandView.as_view()), - re_path(r"management/commands/(?P\w+)/$", ManagementCommandView.as_view()), - re_path(r"management/commands/(?P\w+)/", include(router.urls)), - path("management/", include(router.urls)), + re_path(r"commands/$", ManagementCommandView.as_view()), + re_path(r"commands/(?P\w+)/$", ManagementCommandView.as_view()), + re_path(r"commands/(?P\w+)/", include(router.urls)), + path("", include(router.urls)), ] diff --git a/geonode/metadata/api/urls.py b/geonode/metadata/api/urls.py index a5c94b26410..bd5890f8c79 100644 --- a/geonode/metadata/api/urls.py +++ b/geonode/metadata/api/urls.py @@ -31,42 +31,43 @@ router.register(r"metadata", views.MetadataViewSet, "metadata") -urlpatterns = router.urls + [ +# metadata-* routes come from router.urls in geonode/urls.py; only extra paths here. +urlpatterns = [ path( - r"metadata/autocomplete/thesaurus//keywords", + r"autocomplete/thesaurus//keywords", views.tkeywords_autocomplete, name="metadata_autocomplete_tkeywords", ), - path(r"metadata/autocomplete/users", ProfileAutocomplete.as_view(), name="metadata_autocomplete_users"), + path(r"autocomplete/users", ProfileAutocomplete.as_view(), name="metadata_autocomplete_users"), path( - r"metadata/autocomplete/resources", + r"autocomplete/resources", MetadataLinkedResourcesAutocomplete.as_view(), name="metadata_autocomplete_resources", ), path( - r"metadata/autocomplete/regions", + r"autocomplete/regions", MetadataRegionsAutocomplete.as_view(), name="metadata_autocomplete_regions", ), path( - r"metadata/autocomplete/hkeywords", + r"autocomplete/hkeywords", MetadataHKeywordAutocomplete.as_view(), name="metadata_autocomplete_hkeywords", ), path( - r"metadata/autocomplete/groups", + r"autocomplete/groups", MetadataGroupAutocomplete.as_view(), name="metadata_autocomplete_groups", ), path( - r"metadata/autocomplete/categories", + r"autocomplete/categories", views.categories_autocomplete, name="metadata_autocomplete_categories", ), path( - r"metadata/autocomplete/licenses", + r"autocomplete/licenses", views.licenses_autocomplete, name="metadata_autocomplete_licenses", ), - # path(r"metadata/autocomplete/users", login_required(ProfileAutocomplete.as_view()), name="metadata_autocomplete_users"), + # path(r"autocomplete/users", login_required(ProfileAutocomplete.as_view()), name="metadata_autocomplete_users"), ] diff --git a/geonode/resource/apps.py b/geonode/resource/apps.py index 026ebcd838d..d80ca56256d 100644 --- a/geonode/resource/apps.py +++ b/geonode/resource/apps.py @@ -17,7 +17,6 @@ # ######################################################################### from django.apps import AppConfig -from django.urls import include, re_path class GeoNodeResourceConfig(AppConfig): @@ -25,13 +24,10 @@ class GeoNodeResourceConfig(AppConfig): verbose_name = "GeoNode Resource Service and Manager" def ready(self): - from geonode.urls import urlpatterns - - urlpatterns += [re_path(r"^api/v2/", include("geonode.resource.api.urls"))] run_setup_hooks() def run_setup_hooks(*args, **kwargs): from geonode.resource.registry import resource_manager_registry - resource_manager_registry.init_registry() + resource_manager_registry.init_registry() diff --git a/geonode/upload/api/urls.py b/geonode/upload/api/urls.py deleted file mode 100644 index a59a29f4b0c..00000000000 --- a/geonode/upload/api/urls.py +++ /dev/null @@ -1,20 +0,0 @@ -######################################################################### -# -# 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 . -# -######################################################################### - -urlpatterns = [] diff --git a/geonode/upload/apps.py b/geonode/upload/apps.py index c72fa22d1e8..faa99c5c631 100644 --- a/geonode/upload/apps.py +++ b/geonode/upload/apps.py @@ -26,7 +26,6 @@ class UploadAppConfig(AppConfig): def ready(self): """Finalize setup""" init_feature_validators_registry() - run_setup_hooks() super(UploadAppConfig, self).ready() @@ -34,26 +33,3 @@ def init_feature_validators_registry(): from geonode.upload.registry import feature_validators_registry feature_validators_registry.init_registry() - - -def run_setup_hooks(*args, **kwargs): - """ - Run basic setup configuration for the importer app. - Here we are overriding the upload API url - """ - from geonode.urls import urlpatterns - from django.urls import re_path, include - - url_already_injected = any( - [ - "geonode.upload.urls" in x.urlconf_name.__name__ - for x in urlpatterns - if hasattr(x, "urlconf_name") and not isinstance(x.urlconf_name, list) - ] - ) - - if not url_already_injected: - urlpatterns.insert( - 0, - re_path(r"^api/v2/", include("geonode.upload.api.urls")), - ) diff --git a/geonode/urls.py b/geonode/urls.py index 3e2b8ef441b..3c5e0d472e5 100644 --- a/geonode/urls.py +++ b/geonode/urls.py @@ -63,9 +63,6 @@ re_path(r"^sitemap\.xml$", sitemap, {"sitemaps": sitemaps}, name="sitemap"), re_path(r"^robots\.txt$", TemplateView.as_view(template_name="robots.txt"), name="robots"), re_path(r"(.*version\.txt)$", version.version, name="version"), -] - -urlpatterns += [ # ResourceBase views re_path(r"^base/", include("geonode.base.urls")), re_path(r"^resources/", include("geonode.base.base_urls")), @@ -95,7 +92,6 @@ re_path(r"^account/", include("allauth.urls")), re_path(r"^invitations/", include("geonode.invitations.urls", namespace="geonode.invitations")), re_path(r"^people/", include("geonode.people.urls")), - re_path(r"^api/v2/users/", include("geonode.people.api.urls")), re_path(r"^avatar/", include("avatar.urls")), re_path(r"^activity/", include("actstream.urls")), re_path(r"^announcements/", include("announcements.urls")), @@ -126,14 +122,16 @@ ResourceImporter.as_view({"put": "copy"}), name="importer_resource_copy", ), - re_path(r"^api/v2/", include(router.urls)), + # API v2 (resource, harvesting used to self-register from AppConfig.ready(); hardcoded now, like every other app) + re_path(r"^api/v2/users/", include("geonode.people.api.urls")), + re_path(r"^api/v2/", include("geonode.resource.api.urls")), + re_path(r"^api/v2/", include("geonode.harvesting.api.urls")), re_path(r"^api/v2/", include("geonode.api.urls")), - re_path(r"^api/v2/", include("geonode.management_commands_http.urls")), + re_path(r"^api/v2/management/", include("geonode.management_commands_http.urls")), re_path(r"^api/v2/api-auth/", include("rest_framework.urls", namespace="geonode_rest_framework")), - re_path(r"^api/v2/", include("geonode.facets.urls")), + re_path(r"^api/v2/facets", include("geonode.facets.urls")), re_path(r"^api/v2/", include("geonode.assets.urls")), - # metadata views - re_path(r"^api/v2/", include("geonode.metadata.api.urls")), + re_path(r"^api/v2/metadata/", include("geonode.metadata.api.urls")), re_path(r"", include(api.urls)), re_path( r"uploads/upload", @@ -157,9 +155,6 @@ urlpatterns += [ re_path(r"^i18n/", include(django.conf.urls.i18n), name="i18n"), re_path(r"^jsi18n/$", JavaScriptCatalog.as_view(), js_info_dict, name="javascript-catalog"), -] - -urlpatterns += [ # '', re_path(r"^showmetadata/", include("geonode.catalogue.metadataxsl.urls")), ] @@ -178,6 +173,11 @@ re_path(r"^gs/", include("geonode.geoserver.urls")), ] +# router.urls is a cached property: keep this after every router.register()-triggering include above. +urlpatterns += [ + re_path(r"^api/v2/", include(router.urls)), +] + if settings.NOTIFICATIONS_MODULE in settings.INSTALLED_APPS: notifications_urls = f"{settings.NOTIFICATIONS_MODULE}.urls" urlpatterns += [ # '', @@ -195,7 +195,6 @@ urlpatterns += staticfiles_urlpatterns() urlpatterns += static(settings.LOCAL_MEDIA_URL, document_root=settings.MEDIA_ROOT) -# Internationalization Javascript urlpatterns += [ re_path(r"^metadata_update_redirect$", views.metadata_update_redirect, name="metadata_update_redirect"), ]