diff --git a/swh/web/api/apiurls.py b/swh/web/api/apiurls.py --- a/swh/web/api/apiurls.py +++ b/swh/web/api/apiurls.py @@ -83,7 +83,7 @@ """ - url_pattern = "^" + api_version + url_pattern + "$" + url_pattern = "^api/" + api_version + url_pattern + "$" def decorator(f): # create a DRF view from the wrapped function diff --git a/swh/web/api/views/utils.py b/swh/web/api/views/utils.py --- a/swh/web/api/views/utils.py +++ b/swh/web/api/views/utils.py @@ -77,7 +77,7 @@ return Response({}, template_name="api/api.html") -APIUrls.add_url_pattern(r"^$", api_home, view_name="api-1-homepage") +APIUrls.add_url_pattern(r"^api/$", api_home, view_name="api-1-homepage") @api_route(r"/", "api-1-endpoints") diff --git a/swh/web/browse/browseurls.py b/swh/web/browse/browseurls.py --- a/swh/web/browse/browseurls.py +++ b/swh/web/browse/browseurls.py @@ -30,7 +30,7 @@ view_name: the name of the Django view associated to the routes used to reverse the url """ - url_patterns = tuple("^" + url_pattern + "$" for url_pattern in url_patterns) + url_patterns = tuple("^browse/" + url_pattern + "$" for url_pattern in url_patterns) view_name = view_name def decorator(f): diff --git a/swh/web/browse/urls.py b/swh/web/browse/urls.py --- a/swh/web/browse/urls.py +++ b/swh/web/browse/urls.py @@ -48,16 +48,16 @@ urlpatterns = [ - url(r"^$", _browse_search_view), - url(r"^help/$", _browse_help_view, name="browse-help"), - url(r"^search/$", _browse_search_view, name="browse-search"), - url(r"^vault/$", _browse_vault_view, name="browse-vault"), + url(r"^browse/$", _browse_search_view), + url(r"^browse/help/$", _browse_help_view, name="browse-help"), + url(r"^browse/search/$", _browse_search_view, name="browse-search"), + url(r"^browse/vault/$", _browse_vault_view, name="browse-vault"), # for backward compatibility - url(r"^origin/save/$", _browse_origin_save_view, name="browse-origin-save"), + url(r"^browse/origin/save/$", _browse_origin_save_view, name="browse-origin-save"), url( - r"^(?Pswh:[0-9]+:[a-z]+:[0-9a-f]+.*)/$", + r"^browse/(?Pswh:[0-9]+:[a-z]+:[0-9a-f]+.*)/$", swhid_browse, - name="browse-swhid", + name="browse-swhid-legacy", ), ] diff --git a/swh/web/common/utils.py b/swh/web/common/utils.py --- a/swh/web/common/utils.py +++ b/swh/web/common/utils.py @@ -21,6 +21,7 @@ import requests from requests.auth import HTTPBasicAuth +from django.conf import settings from django.core.cache import cache from django.core.cache.backends.base import DEFAULT_TIMEOUT from django.http import HttpRequest, QueryDict @@ -40,6 +41,7 @@ SWHID_RE = "swh:1:[a-z]{3}:[0-9a-z]{40}" + swh_object_icons = { "alias": "mdi mdi-star", "branch": "mdi mdi-source-branch", @@ -320,6 +322,7 @@ "MAILMAP_ADMIN_PERMISSION": MAILMAP_ADMIN_PERMISSION, "lang": "en", "sidebar_state": request.COOKIES.get("sidebar-state", "expanded"), + "SWH_DJANGO_APPS": settings.SWH_DJANGO_APPS, } diff --git a/swh/web/config.py b/swh/web/config.py --- a/swh/web/config.py +++ b/swh/web/config.py @@ -1,4 +1,4 @@ -# Copyright (C) 2017-2021 The Software Heritage developers +# Copyright (C) 2017-2022 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU Affero General Public License version 3, or any later version # See top-level LICENSE file for more information @@ -155,6 +155,13 @@ "give": ("dict", {"public_key": "", "token": ""}), "features": ("dict", {"add_forge_now": True}), "add_forge_now": ("dict", {"email_address": "add-forge-now@example.com"}), + "swh_extra_django_apps": ( + "list", + [ + "swh.web.inbound_email", + "swh.web.add_forge_now", + ], + ), } swhweb_config: Dict[str, Any] = {} diff --git a/swh/web/settings/common.py b/swh/web/settings/common.py --- a/swh/web/settings/common.py +++ b/swh/web/settings/common.py @@ -8,6 +8,7 @@ Django common settings for swh-web. """ +from importlib.util import find_spec import os import sys from typing import Any, Dict @@ -42,6 +43,21 @@ # Application definition +SWH_BASE_DJANGO_APPS = [ + "swh.web.auth", + "swh.web.browse", + "swh.web.common", + "swh.web.api", +] +SWH_EXTRA_DJANGO_APPS = [ + app + for app in swh_web_config["swh_extra_django_apps"] + if app not in SWH_BASE_DJANGO_APPS +] +# swh.web.api must be the last loaded application due to the way +# its URLS are registered +SWH_DJANGO_APPS = SWH_EXTRA_DJANGO_APPS + SWH_BASE_DJANGO_APPS + INSTALLED_APPS = [ "django.contrib.admin", "django.contrib.auth", @@ -50,16 +66,10 @@ "django.contrib.messages", "django.contrib.staticfiles", "rest_framework", - "swh.web.common", - "swh.web.inbound_email", - "swh.web.api", - "swh.web.auth", - "swh.web.browse", - "swh.web.add_forge_now", "webpack_loader", "django_js_reverse", "corsheaders", -] +] + SWH_DJANGO_APPS MIDDLEWARE = [ "django.middleware.security.SecurityMiddleware", @@ -84,10 +94,23 @@ ROOT_URLCONF = "swh.web.urls" +SWH_APP_TEMPLATES = [os.path.join(PROJECT_DIR, "../templates")] +# Add templates directory from each SWH Django application +for app in SWH_DJANGO_APPS: + try: + app_spec = find_spec(app) + assert app_spec is not None, f"Django application {app} not found !" + assert app_spec.origin is not None + SWH_APP_TEMPLATES.append( + os.path.join(os.path.dirname(app_spec.origin), "templates") + ) + except ModuleNotFoundError: + assert False, f"Django application {app} not found !" + TEMPLATES = [ { "BACKEND": "django.template.backends.django.DjangoTemplates", - "DIRS": [os.path.join(PROJECT_DIR, "../templates")], + "DIRS": SWH_APP_TEMPLATES, "APP_DIRS": True, "OPTIONS": { "context_processors": [ diff --git a/swh/web/tests/conftest.py b/swh/web/tests/conftest.py --- a/swh/web/tests/conftest.py +++ b/swh/web/tests/conftest.py @@ -6,6 +6,7 @@ from collections import defaultdict from datetime import timedelta import functools +from importlib import import_module, reload import json import os import random @@ -17,10 +18,12 @@ from _pytest.python import Function from hypothesis import HealthCheck, settings import pytest +from pytest_django.fixtures import SettingsWrapper from django.contrib.auth.models import User from django.core.cache import cache from django.test.utils import setup_databases +from django.urls import clear_url_caches from rest_framework.test import APIClient, APIRequestFactory from swh.model.hashutil import ( @@ -1202,3 +1205,35 @@ mailmap_user = User.objects.create_user(username="mailmap-user", password="") mailmap_user.user_permissions.add(create_django_permission(MAILMAP_PERMISSION)) return mailmap_user + + +def reload_urlconf(): + from django.conf import settings + + clear_url_caches() + urlconf = settings.ROOT_URLCONF + if urlconf in sys.modules: + reload(sys.modules[urlconf]) + else: + import_module(urlconf) + + +class SwhSettingsWrapper(SettingsWrapper): + def __setattr__(self, attr: str, value) -> None: + super().__setattr__(attr, value) + reload_urlconf() + + def finalize(self) -> None: + super().finalize() + reload_urlconf() + + +@pytest.fixture +def django_settings(): + """Override pytest-django settings fixture in order to reload URLs + when modifying settings in test and after test execution as most of + them depend on installed django apps in swh-web. + """ + settings = SwhSettingsWrapper() + yield settings + settings.finalize() diff --git a/swh/web/tests/test_urls.py b/swh/web/tests/test_urls.py --- a/swh/web/tests/test_urls.py +++ b/swh/web/tests/test_urls.py @@ -3,11 +3,25 @@ # License: GNU Affero General Public License version 3, or any later version # See top-level LICENSE file for more information +import pytest + from django.urls import get_resolver def test_swh_web_urls_have_trailing_slash(): - urls = set(value[1] for value in get_resolver().reverse_dict.values()) + urls = set( + value[1] + for key, value in get_resolver().reverse_dict.items() + if key != "browse-swhid" # (see T3234) + ) for url in urls: if url != "$": assert url.endswith("/$") + + +def test_urls_registration_error_for_not_found_django_app(django_settings): + app_name = "swh.web.foobar" + with pytest.raises( + AssertionError, match=f"Django application {app_name} not found !" + ): + django_settings.SWH_DJANGO_APPS = django_settings.SWH_DJANGO_APPS + [app_name] diff --git a/swh/web/urls.py b/swh/web/urls.py --- a/swh/web/urls.py +++ b/swh/web/urls.py @@ -3,6 +3,7 @@ # License: GNU Affero General Public License version 3, or any later version # See top-level LICENSE file for more information +from importlib.util import find_spec from django_js_reverse.views import urls_js @@ -38,8 +39,6 @@ urlpatterns = [ url(r"^admin/", include("swh.web.admin.urls")), url(r"^favicon\.ico/$", favicon_view), - url(r"^api/", include("swh.web.api.urls")), - url(r"^browse/", include("swh.web.browse.urls")), url(r"^$", _default_view, name="swh-web-homepage"), url(r"^jsreverse/$", urls_js, name="js_reverse"), # keep legacy SWHID resolving URL with trailing slash for backward compatibility @@ -58,6 +57,16 @@ url(r"^logout/$", LogoutView.as_view(template_name="logout.html"), name="logout"), ] +# Register URLs for each SWH Django application +for app in settings.SWH_DJANGO_APPS: + app_urls = app + ".urls" + try: + app_urls_spec = find_spec(app_urls) + if app_urls_spec is not None: + urlpatterns.append(url(r"^", include(app_urls))) + except ModuleNotFoundError: + assert False, f"Django application {app} not found !" + if is_feature_enabled("add_forge_now"): urlpatterns += (url(r"^", include("swh.web.add_forge_now.views")),)