diff --git a/swh/web/browse/views/origin.py b/swh/web/browse/views/origin.py --- a/swh/web/browse/views/origin.py +++ b/swh/web/browse/views/origin.py @@ -3,23 +3,50 @@ # License: GNU Affero General Public License version 3, or any later version # See top-level LICENSE file for more information +import functools + from django.shortcuts import redirect, render +from django.urls import resolve from swh.web.browse.browseurls import browse_route from swh.web.browse.snapshot_context import ( browse_snapshot_branches, browse_snapshot_content, browse_snapshot_directory, - browse_snapshot_log, browse_snapshot_releases, get_snapshot_context, ) from swh.web.common import archive -from swh.web.common.exc import BadInputExc +from swh.web.common.exc import BadInputExc, NotFoundExc from swh.web.common.origin_visits import get_origin_visits from swh.web.common.utils import format_utc_iso_date, parse_iso8601_date_to_utc, reverse +def deprecated_route(func): + # A mapping to relate deprecated URls to the new ones + deprecation_mapping = { + # log URLs + "browse-origin-log": "browse-snapshot-log", + "browse-origin-log-legacy": "browse-snapshot-log", + } + + @functools.wraps(func) + def redirect_to_new_route(request, *args, **kwargs): + request_path = resolve(request.path_info) + if request_path.url_name not in deprecation_mapping: + raise NotFoundExc("Invalid deprecated route") + # Send all the url_args and the request_args as query params + # eg /origin//log?path=test + # will be send as /log?url=&path=test + args = {**request_path.kwargs, **{k: v for k, v in request.GET.items()}} + return redirect( + reverse(deprecation_mapping[request_path.url_name], query_params=args), + permanent=True, + ) + + return redirect_to_new_route + + @browse_route( r"origin/directory/", view_name="browse-origin-directory", ) @@ -110,18 +137,16 @@ @browse_route( r"origin/log/", view_name="browse-origin-log", ) +@deprecated_route def origin_log_browse(request): - """Django view that produces an HTML display of revisions history (aka + """ + This route is deprecated; use /browse/snapshot/log instead + + Django view that produces an HTML display of revisions history (aka the commit log) associated to a software origin. The URL that points to it is :http:get:`/browse/origin/log/` """ - return browse_snapshot_log( - request, - origin_url=request.GET.get("origin_url"), - snapshot_id=request.GET.get("snapshot"), - timestamp=request.GET.get("timestamp"), - ) @browse_route( @@ -129,8 +154,12 @@ r"origin/(?P.+)/log/", view_name="browse-origin-log-legacy", ) +@deprecated_route def origin_log_browse_legacy(request, origin_url, timestamp=None): - """Django view that produces an HTML display of revisions history (aka + """ + This route is deprecated; use /browse/snapshot/log instead + + Django view that produces an HTML display of revisions history (aka the commit log) associated to a software origin. The URLs that point to it are @@ -138,12 +167,6 @@ :http:get:`/browse/origin/(origin_url)/visit/(timestamp)/log/` """ - return browse_snapshot_log( - request, - origin_url=origin_url, - snapshot_id=request.GET.get("snapshot"), - timestamp=timestamp, - ) @browse_route( diff --git a/swh/web/browse/views/snapshot.py b/swh/web/browse/views/snapshot.py --- a/swh/web/browse/views/snapshot.py +++ b/swh/web/browse/views/snapshot.py @@ -13,10 +13,22 @@ browse_snapshot_directory, browse_snapshot_log, browse_snapshot_releases, + get_snapshot_context, ) +from swh.web.common.exc import BadInputExc from swh.web.common.utils import reverse +def get_snapshot_from_request(request): + args = {key: value for key, value in request.GET.items()} + snapshot = args.get("snapshot") + if snapshot: + return snapshot + if args.get("origin_url") is None: + raise BadInputExc("An origin URL must be provided as a query parameter.") + return get_snapshot_context(**args)["snapshot_id"] + + @browse_route( r"snapshot/(?P[0-9a-f]+)/", view_name="browse-snapshot", @@ -115,17 +127,34 @@ @browse_route( r"snapshot/(?P[0-9a-f]+)/log/", + r"snapshot/log/", view_name="browse-snapshot-log", checksum_args=["snapshot_id"], ) -def snapshot_log_browse(request, snapshot_id): +def snapshot_log_browse(request, snapshot_id=None): """Django view that produces an HTML display of revisions history (aka the commit log) collected in a snapshot. The url that points to it is :http:get:`/browse/snapshot/(snapshot_id)/log/` """ - return browse_snapshot_log(request, snapshot_id=snapshot_id) + if snapshot_id is None: + # This case happens when redirected from /origin/log + snapshot_id = get_snapshot_from_request(request) + # Redirect to the same route with snapshot_id + return redirect( + reverse( + "browse-snapshot-log", + url_args={"snapshot_id": snapshot_id}, + query_params=request.GET, + ), + ) + return browse_snapshot_log( + request, + snapshot_id=snapshot_id, + origin_url=request.GET.get("origin_url"), + timestamp=request.GET.get("timestamp"), + ) @browse_route( diff --git a/swh/web/tests/browse/views/test_origin.py b/swh/web/tests/browse/views/test_origin.py --- a/swh/web/tests/browse/views/test_origin.py +++ b/swh/web/tests/browse/views/test_origin.py @@ -854,21 +854,31 @@ for browse_context in ( "content", "directory", - "log", "branches", "releases", "visits", ): url = reverse(f"browse-origin-{browse_context}") - resp = check_html_get_response( client, url, status_code=400, template_used="error.html" ) assert_contains( - resp, "An origin URL must be provided as query parameter.", status_code=400 + resp, "An origin URL must be provided as query parameter.", status_code=400, ) +@given(origin()) +@pytest.mark.parametrize("browse_context", ["log"]) +def test_origin_view_redirects(client, browse_context, origin): + query_params = {"origin_url": origin["url"]} + url = reverse(f"browse-origin-{browse_context}", query_params=query_params) + + resp = check_html_get_response(client, url, status_code=301) + assert resp["location"] == reverse( + f"browse-snapshot-{browse_context}", query_params=query_params + ) + + def _origin_content_view_test_helper( client, archive_data,