diff --git a/swh/web/add_forge_now/api_views.py b/swh/web/add_forge_now/api_views.py --- a/swh/web/add_forge_now/api_views.py +++ b/swh/web/add_forge_now/api_views.py @@ -22,7 +22,7 @@ from swh.web.add_forge_now.models import RequestHistory as AddForgeNowRequestHistory from swh.web.add_forge_now.models import RequestStatus as AddForgeNowRequestStatus from swh.web.api.apidoc import api_doc, format_docstring -from swh.web.api.apiurls import api_route +from swh.web.api.apiurls import APIUrls, api_route from swh.web.auth.utils import is_add_forge_now_moderator from swh.web.utils import reverse from swh.web.utils.exc import BadInputExc @@ -102,10 +102,14 @@ fields = ("id", "date", "new_status", "actor_role") +add_forge_now_api_urls = APIUrls() + + @api_route( r"/add-forge/request/create/", "api-1-add-forge-request-create", methods=["POST"], + api_urls=add_forge_now_api_urls, ) @api_doc("/add-forge/request/create", category="Request archival") @format_docstring() @@ -189,6 +193,7 @@ r"/add-forge/request/(?P[0-9]+)/update/", "api-1-add-forge-request-update", methods=["POST"], + api_urls=add_forge_now_api_urls, ) @api_doc("/add-forge/request/update", category="Request archival", tags=["hidden"]) @format_docstring() @@ -280,6 +285,7 @@ r"/add-forge/request/list/", "api-1-add-forge-request-list", methods=["GET"], + api_urls=add_forge_now_api_urls, ) @api_doc("/add-forge/request/list", category="Request archival") @format_docstring() @@ -351,6 +357,7 @@ r"/add-forge/request/(?P[0-9]+)/get/", "api-1-add-forge-request-get", methods=["GET"], + api_urls=add_forge_now_api_urls, ) @api_doc("/add-forge/request/get", category="Request archival") @format_docstring() diff --git a/swh/web/add_forge_now/urls.py b/swh/web/add_forge_now/urls.py --- a/swh/web/add_forge_now/urls.py +++ b/swh/web/add_forge_now/urls.py @@ -11,7 +11,7 @@ ) # register Web API endpoints -import swh.web.add_forge_now.api_views # noqa +from swh.web.add_forge_now.api_views import add_forge_now_api_urls from swh.web.add_forge_now.views import ( add_forge_request_list_datatables, create_request_create, @@ -44,4 +44,4 @@ add_forge_now_request_dashboard, name="add-forge-now-request-dashboard", ), -] +] + add_forge_now_api_urls.get_url_patterns() diff --git a/swh/web/api/apidoc.py b/swh/web/api/apidoc.py --- a/swh/web/api/apidoc.py +++ b/swh/web/api/apidoc.py @@ -19,7 +19,7 @@ from rest_framework.decorators import api_view from swh.web.api.apiresponse import make_api_response -from swh.web.api.apiurls import APIUrls, CategoryId +from swh.web.api.apiurls import CategoryId, api_urls from swh.web.utils import parse_rst, reverse @@ -352,7 +352,7 @@ if "hidden" not in tags_set: doc_data = get_doc_data(f, route, noargs) doc_desc = doc_data["description"] - APIUrls.add_doc_route( + api_urls.add_doc_route( route, category, re.split(r"\.\s", doc_desc)[0], @@ -372,7 +372,7 @@ urlpattern = f"^api/{api_version}{route}doc/$" view_name = "api-%s-%s" % (api_version, route_name) - APIUrls.add_url_pattern(urlpattern, doc_view, view_name) + api_urls.add_url_pattern(urlpattern, doc_view, view_name) # for backward compatibility as previous apidoc URLs were missing # the /api prefix @@ -383,7 +383,7 @@ def old_doc_view(request): return redirect(reverse(view_name)) - APIUrls.add_url_pattern(old_urlpattern, old_doc_view, old_view_name) + api_urls.add_url_pattern(old_urlpattern, old_doc_view, old_view_name) @wraps(f) def documented_view(request, **kwargs): 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 @@ -21,25 +21,15 @@ class APIUrls(UrlsIndex): - """ - Class to manage API documentation URLs. - - - Indexes all routes documented using apidoc's decorators. - - Tracks endpoint/request processing method relationships for use in - generating related urls in API documentation - - """ + """Class to manage API URLs and endpoint documentation URLs.""" - _apidoc_routes: Dict[str, Dict[str, str]] = {} - scope = "api" + apidoc_routes: Dict[str, Dict[str, str]] = {} - @classmethod - def get_app_endpoints(cls) -> Dict[str, Dict[str, str]]: - return cls._apidoc_routes + def get_app_endpoints(self) -> Dict[str, Dict[str, str]]: + return self.apidoc_routes - @classmethod def add_doc_route( - cls, + self, route: str, category: CategoryId, docstring: str, @@ -54,7 +44,7 @@ if not noargs: route_name = "%s-doc" % route_name route_view_name = "api-%s-%s" % (api_version, route_name) - if route not in cls._apidoc_routes: + if route not in self.apidoc_routes: d = { "category": category, "docstring": docstring, @@ -63,7 +53,10 @@ } for k, v in kwargs.items(): d[k] = v - cls._apidoc_routes[route] = d + self.apidoc_routes[route] = d + + +api_urls = APIUrls() def api_route( @@ -74,6 +67,7 @@ api_version: str = "1", checksum_args: Optional[List[str]] = None, never_cache: bool = False, + api_urls: APIUrls = api_urls, ): """ Decorator to ease the registration of an API endpoint @@ -122,10 +116,10 @@ api_view_f.http_method_names = methods # register the route and its view in the endpoints index - APIUrls.add_url_pattern(url_pattern, api_view_f, view_name) + api_urls.add_url_pattern(url_pattern, api_view_f, view_name) if checksum_args: - APIUrls.add_redirect_for_checksum_args( + api_urls.add_redirect_for_checksum_args( view_name, [url_pattern], checksum_args ) return f diff --git a/swh/web/api/urls.py b/swh/web/api/urls.py --- a/swh/web/api/urls.py +++ b/swh/web/api/urls.py @@ -4,7 +4,7 @@ # See top-level LICENSE file for more information -from swh.web.api.apiurls import APIUrls +from swh.web.api.apiurls import api_urls import swh.web.api.views.content # noqa import swh.web.api.views.directory # noqa import swh.web.api.views.graph # noqa @@ -18,4 +18,4 @@ import swh.web.api.views.snapshot # noqa import swh.web.api.views.stat # noqa -urlpatterns = APIUrls.get_url_patterns() +urlpatterns = api_urls.get_url_patterns() 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 @@ -11,7 +11,7 @@ from rest_framework.request import Request from rest_framework.response import Response -from swh.web.api.apiurls import APIUrls, api_route +from swh.web.api.apiurls import api_route, api_urls from swh.web.utils.exc import NotFoundExc EnrichFunction = Callable[[Dict[str, str], Optional[HttpRequest]], Dict[str, str]] @@ -77,14 +77,14 @@ return Response({}, template_name="api.html") -APIUrls.add_url_pattern(r"^api/$", api_home, view_name="api-1-homepage") +api_urls.add_url_pattern(r"^api/$", api_home, view_name="api-1-homepage") @api_route(r"/", "api-1-endpoints") def api_endpoints(request): """Display the list of opened api endpoints.""" routes_by_category = {} - for route, doc in APIUrls.get_app_endpoints().items(): + for route, doc in api_urls.get_app_endpoints().items(): doc["doc_intro"] = doc["docstring"].split("\n\n")[0] routes_by_category.setdefault(doc["category"], []).append(doc) 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 @@ -7,13 +7,7 @@ from swh.web.utils.urlsindex import UrlsIndex - -class BrowseUrls(UrlsIndex): - """ - Class to manage swh-web browse application urls. - """ - - scope = "browse" +browse_urls = UrlsIndex() def browse_route( @@ -36,10 +30,10 @@ def decorator(f): # register the route and its view in the browse endpoints index for url_pattern in url_patterns: - BrowseUrls.add_url_pattern(url_pattern, f, view_name) + browse_urls.add_url_pattern(url_pattern, f, view_name) if checksum_args: - BrowseUrls.add_redirect_for_checksum_args( + browse_urls.add_redirect_for_checksum_args( view_name, url_patterns, checksum_args ) 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 @@ -7,7 +7,7 @@ from django.shortcuts import redirect, render from django.urls import re_path as url -from swh.web.browse.browseurls import BrowseUrls +from swh.web.browse.browseurls import browse_urls from swh.web.browse.identifiers import swhid_browse import swh.web.browse.views.content # noqa import swh.web.browse.views.directory # noqa @@ -71,6 +71,5 @@ swhid_browse, name="browse-swhid", ), + *browse_urls.get_url_patterns(), ] - -urlpatterns += BrowseUrls.get_url_patterns() diff --git a/swh/web/save_code_now/api_views.py b/swh/web/save_code_now/api_views.py --- a/swh/web/save_code_now/api_views.py +++ b/swh/web/save_code_now/api_views.py @@ -9,7 +9,7 @@ from rest_framework.request import Request from swh.web.api.apidoc import api_doc, format_docstring -from swh.web.api.apiurls import api_route +from swh.web.api.apiurls import APIUrls, api_route from swh.web.auth.utils import ( API_SAVE_ORIGIN_PERMISSION, SWH_AMBASSADOR_PERMISSION, @@ -33,12 +33,16 @@ return docstring +save_code_now_api_urls = APIUrls() + + @api_route( r"/origin/save/(?P.+)/url/(?P.+)/", "api-1-save-origin", methods=["GET", "POST"], throttle_scope="swh_save_origin", never_cache=True, + api_urls=save_code_now_api_urls, ) @api_doc("/origin/save/", category="Request archival") @format_docstring(visit_types=_savable_visit_types()) diff --git a/swh/web/save_code_now/urls.py b/swh/web/save_code_now/urls.py --- a/swh/web/save_code_now/urls.py +++ b/swh/web/save_code_now/urls.py @@ -20,7 +20,7 @@ ) # register Web API endpoints -import swh.web.save_code_now.api_views # noqa +from swh.web.save_code_now.api_views import save_code_now_api_urls from swh.web.save_code_now.views import ( origin_save_help_view, origin_save_list_view, @@ -96,4 +96,5 @@ admin_origin_save_request_remove, name="admin-origin-save-request-remove", ), + *save_code_now_api_urls.get_url_patterns(), ] diff --git a/swh/web/save_origin_webhooks/generic_receiver.py b/swh/web/save_origin_webhooks/generic_receiver.py --- a/swh/web/save_origin_webhooks/generic_receiver.py +++ b/swh/web/save_origin_webhooks/generic_receiver.py @@ -9,10 +9,12 @@ from rest_framework.request import Request from swh.web.api.apidoc import api_doc -from swh.web.api.apiurls import api_route +from swh.web.api.apiurls import APIUrls, api_route from swh.web.save_code_now.origin_save import create_save_origin_request from swh.web.utils.exc import BadInputExc +webhooks_api_urls = APIUrls() + class OriginSaveWebhookReceiver(abc.ABC): FORGE_TYPE: str @@ -65,6 +67,7 @@ f"/origin/save/webhook/{self.FORGE_TYPE.lower()}/", f"api-1-origin-save-webhook-{self.FORGE_TYPE.lower()}", methods=["POST"], + api_urls=webhooks_api_urls, )(self) def __call__( diff --git a/swh/web/save_origin_webhooks/urls.py b/swh/web/save_origin_webhooks/urls.py --- a/swh/web/save_origin_webhooks/urls.py +++ b/swh/web/save_origin_webhooks/urls.py @@ -3,9 +3,6 @@ # License: GNU Affero General Public License version 3, or any later version # See top-level LICENSE file for more information -from typing import List, Union - -from django.urls import URLPattern, URLResolver # register Web API endpoints import swh.web.save_origin_webhooks.bitbucket # noqa @@ -14,4 +11,8 @@ import swh.web.save_origin_webhooks.gitlab # noqa import swh.web.save_origin_webhooks.sourceforge # noqa -urlpatterns: List[Union[URLPattern, URLResolver]] = [] +from swh.web.save_origin_webhooks.generic_receiver import ( # isort: skip + webhooks_api_urls, +) + +urlpatterns = webhooks_api_urls.get_url_patterns() 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 @@ -40,24 +40,23 @@ ALLOWED_HOSTS = ["127.0.0.1", "localhost"] + swh_web_config["allowed_hosts"] -# Application definition +# Applications definition SWH_BASE_DJANGO_APPS = [ - "swh.web.webapp", + "swh.web.api", "swh.web.auth", "swh.web.browse", - "swh.web.utils", "swh.web.tests", - "swh.web.api", + "swh.web.utils", + "swh.web.webapp", ] 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 + +SWH_DJANGO_APPS = SWH_BASE_DJANGO_APPS + SWH_EXTRA_DJANGO_APPS INSTALLED_APPS = [ diff --git a/swh/web/tests/save_origin_webhooks/test_app.py b/swh/web/tests/save_origin_webhooks/test_app.py new file mode 100644 --- /dev/null +++ b/swh/web/tests/save_origin_webhooks/test_app.py @@ -0,0 +1,26 @@ +# Copyright (C) 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 + +import pytest + +from django.urls import get_resolver + +from swh.web.save_origin_webhooks.urls import urlpatterns + + +@pytest.mark.django_db +def test_save_origin_webhooks_deactivate(django_settings): + """Check webhooks feature is deactivated when the swh.web.save_origin_webhooks + django application is not in installed apps.""" + + django_settings.SWH_DJANGO_APPS = [ + app + for app in django_settings.SWH_DJANGO_APPS + if app != "swh.web.save_origin_webhooks" + ] + + save_origin_webhooks_view_names = set(urlpattern.name for urlpattern in urlpatterns) + all_view_names = set(get_resolver().reverse_dict.keys()) + assert save_origin_webhooks_view_names & all_view_names == set() diff --git a/swh/web/utils/urlsindex.py b/swh/web/utils/urlsindex.py --- a/swh/web/utils/urlsindex.py +++ b/swh/web/utils/urlsindex.py @@ -3,54 +3,50 @@ # License: GNU Affero General Public License version 3, or any later version # See top-level LICENSE file for more information -from collections import defaultdict -from typing import Dict, List +from typing import Callable, List +from django.http.response import HttpResponseBase from django.shortcuts import redirect from django.urls import URLPattern from django.urls import re_path as url class UrlsIndex: - """ - Simple helper class for centralizing url patterns of a Django - web application. + """Simple helper class for centralizing URL patterns of a Django web application.""" - Derived classes should override the 'scope' class attribute otherwise - all declared patterns will be grouped under the default one. - """ + def __init__(self): + self.urlpatterns: List[URLPattern] = [] - _urlpatterns: Dict[str, List[URLPattern]] = defaultdict(list) - scope = "default" - - @classmethod - def add_url_pattern(cls, url_pattern, view, view_name=None): + def add_url_pattern( + self, + url_pattern: str, + view: Callable[..., HttpResponseBase], + view_name: str = "", + ) -> None: """ - Class method that adds an url pattern to the current scope. + Adds an URL pattern. Args: - url_pattern: regex describing a Django url + url_pattern: regex describing a Django URL view: function implementing the Django view - view_name: name of the view used to reverse the url + view_name: name of the view used to reverse the URL """ - if cls.scope not in cls._urlpatterns: - cls._urlpatterns[cls.scope] = [] if view_name: - cls._urlpatterns[cls.scope].append(url(url_pattern, view, name=view_name)) + self.urlpatterns.append(url(url_pattern, view, name=view_name)) else: - cls._urlpatterns[cls.scope].append(url(url_pattern, view)) + self.urlpatterns.append(url(url_pattern, view)) - @classmethod - def add_redirect_for_checksum_args(cls, view_name, url_patterns, checksum_args): + def add_redirect_for_checksum_args( + self, view_name: str, url_patterns: List[str], checksum_args: List[str] + ) -> None: """ - Class method that redirects to view with lowercase checksums - when upper/mixed case checksums are passed as url arguments. + Adds redirection to view with lowercase checksums when upper/mixed case + checksums are passed as url arguments. Args: - view_name (str): name of the view to redirect requests - url_patterns (List[str]): regexps describing the view urls - checksum_args (List[str]): url argument names corresponding - to checksum values + view_name: name of the view to redirect requests + url_patterns: regexps describing the view URLs + checksum_args: url argument names corresponding to checksum values """ new_view_name = view_name + "-uppercase-checksum" for url_pattern in url_patterns: @@ -62,15 +58,10 @@ kwargs[checksum_arg] = checksum_upper.lower() return redirect(view_name, *args, **kwargs) - cls.add_url_pattern(url_pattern_upper, view_redirect, new_view_name) + self.add_url_pattern(url_pattern_upper, view_redirect, new_view_name) - @classmethod - def get_url_patterns(cls): + def get_url_patterns(self) -> List[URLPattern]: """ - Class method that returns the list of url pattern associated to - the current scope. - - Returns: - The list of url patterns associated to the current scope + Returns the list of registered URL patterns. """ - return cls._urlpatterns[cls.scope] + return self.urlpatterns diff --git a/swh/web/vault/api_views.py b/swh/web/vault/api_views.py --- a/swh/web/vault/api_views.py +++ b/swh/web/vault/api_views.py @@ -12,7 +12,7 @@ from swh.model.hashutil import hash_to_hex from swh.model.swhids import CoreSWHID, ObjectType from swh.web.api.apidoc import api_doc, format_docstring -from swh.web.api.apiurls import api_route +from swh.web.api.apiurls import APIUrls, api_route from swh.web.api.views.utils import api_lookup from swh.web.utils import SWHID_RE, archive, query, reverse from swh.web.utils.exc import BadInputExc @@ -61,6 +61,8 @@ return d +vault_api_urls = APIUrls() + ###################################################### # Flat bundles @@ -71,6 +73,7 @@ methods=["GET", "POST"], throttle_scope="swh_vault_cooking", never_cache=True, + api_urls=vault_api_urls, ) @api_doc("/vault/flat/", category="Batch download") @format_docstring() @@ -146,6 +149,7 @@ checksum_args=["dir_id"], throttle_scope="swh_vault_cooking", never_cache=True, + api_urls=vault_api_urls, ) @api_doc("/vault/directory/", category="Batch download", tags=["deprecated"]) @format_docstring() @@ -172,6 +176,7 @@ @api_route( f"/vault/flat/(?P{SWHID_RE})/raw/", "api-1-vault-fetch-flat", + api_urls=vault_api_urls, ) @api_doc("/vault/flat/raw/", category="Batch download") def api_vault_fetch_flat(request: Request, swhid: str): @@ -211,6 +216,7 @@ r"/vault/directory/(?P[0-9a-f]+)/raw/", "api-1-vault-fetch-directory", checksum_args=["dir_id"], + api_urls=vault_api_urls, ) @api_doc( "/vault/directory/raw/", category="Batch download", tags=["hidden", "deprecated"] @@ -240,6 +246,7 @@ methods=["GET", "POST"], throttle_scope="swh_vault_cooking", never_cache=True, + api_urls=vault_api_urls, ) @api_doc("/vault/gitfast/", category="Batch download") @format_docstring() @@ -315,6 +322,7 @@ checksum_args=["rev_id"], throttle_scope="swh_vault_cooking", never_cache=True, + api_urls=vault_api_urls, ) @api_doc("/vault/revision/gitfast/", category="Batch download", tags=["deprecated"]) @format_docstring() @@ -341,6 +349,7 @@ @api_route( f"/vault/gitfast/(?P{SWHID_RE})/raw/", "api-1-vault-fetch-gitfast", + api_urls=vault_api_urls, ) @api_doc("/vault/gitfast/raw/", category="Batch download") def api_vault_fetch_revision_gitfast(request: Request, swhid: str): @@ -380,6 +389,7 @@ r"/vault/revision/(?P[0-9a-f]+)/gitfast/raw/", "api-1-vault-fetch-revision_gitfast", checksum_args=["rev_id"], + api_urls=vault_api_urls, ) @api_doc( "/vault/revision_gitfast/raw/", @@ -408,6 +418,7 @@ methods=["GET", "POST"], throttle_scope="swh_vault_cooking", never_cache=True, + api_urls=vault_api_urls, ) @api_doc("/vault/git-bare/", category="Batch download") @format_docstring() @@ -483,6 +494,7 @@ @api_route( f"/vault/git-bare/(?P{SWHID_RE})/raw/", "api-1-vault-fetch-git-bare", + api_urls=vault_api_urls, ) @api_doc("/vault/git-bare/raw/", category="Batch download") def api_vault_fetch_revision_git_bare(request: Request, swhid: str): diff --git a/swh/web/vault/urls.py b/swh/web/vault/urls.py --- a/swh/web/vault/urls.py +++ b/swh/web/vault/urls.py @@ -8,7 +8,7 @@ from django.urls import re_path as url # register Web API endpoints -import swh.web.vault.api_views # noqa +from swh.web.vault.api_views import vault_api_urls def vault_view(request: HttpRequest) -> HttpResponse: @@ -27,4 +27,5 @@ url(r"^vault/$", vault_view, name="vault"), # for backward compatibility url(r"^browse/vault/$", browse_vault_view, name="browse-vault"), + *vault_api_urls.get_url_patterns(), ]