Changeset View
Changeset View
Standalone View
Standalone View
swh/web/browse/views/origin.py
| # Copyright (C) 2017-2020 The Software Heritage developers | # Copyright (C) 2017-2020 The Software Heritage developers | ||||
| # See the AUTHORS file at the top-level directory of this distribution | # See the AUTHORS file at the top-level directory of this distribution | ||||
| # License: GNU Affero General Public License version 3, or any later version | # License: GNU Affero General Public License version 3, or any later version | ||||
| # See top-level LICENSE file for more information | # See top-level LICENSE file for more information | ||||
| import functools | |||||
| from django.shortcuts import redirect, render | from django.shortcuts import redirect, render | ||||
| from django.urls import resolve | |||||
| from swh.web.browse.browseurls import browse_route | from swh.web.browse.browseurls import browse_route | ||||
| from swh.web.browse.snapshot_context import ( | from swh.web.browse.snapshot_context import ( | ||||
| browse_snapshot_branches, | |||||
| browse_snapshot_content, | browse_snapshot_content, | ||||
| browse_snapshot_directory, | browse_snapshot_directory, | ||||
| browse_snapshot_log, | |||||
| browse_snapshot_releases, | browse_snapshot_releases, | ||||
| get_snapshot_context, | get_snapshot_context, | ||||
| ) | ) | ||||
| from swh.web.common import archive | 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.origin_visits import get_origin_visits | ||||
| from swh.web.common.utils import format_utc_iso_date, parse_iso8601_date_to_utc, reverse | 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 routes | |||||
| "browse-origin-log": "browse-snapshot-log", | |||||
| "browse-origin-log-legacy": "browse-snapshot-log", | |||||
anlambert: I think it will be better to add a `view_redirect` parameter to the decorator instead of… | |||||
| # branches routes | |||||
| "browse-origin-branches": "browse-snapshot-branches", | |||||
| "browse-origin-branches-legacy": "browse-snapshot-branches", | |||||
| } | |||||
| @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/<url:url-val>/log?path=test | |||||
| # will be send as /log?url=<url-val>&path=test | |||||
| args = {**request_path.kwargs, **{k: v for k, v in request.GET.items()}} | |||||
| return redirect( | |||||
Done Inline ActionsCould you remove the decorator wrapping and simply call redirect_to_new_route in views implementation ? anlambert: Could you remove the decorator wrapping and simply call `redirect_to_new_route` in views… | |||||
Done Inline ActionsOk, do you think adding an extra line in the doc string in each deprecated function would be enough?. It would be like "This route is deprecated, use <alternate name> instead" ? jayeshv: Ok, do you think adding an extra line in the doc string in each deprecated function would be… | |||||
Done Inline ActionsYes this should be enough, you can reference the correct view to use in sphinx using :http:get:<url>, see how its is done for the vault. anlambert: Yes this should be enough, you can reference the correct view to use in sphinx using `:http:get… | |||||
Done Inline ActionsOk, Thanks. In fact, I have that doc string added in the coming diffs (for branches and releases). So, I will move that change to this diff and keep the decorator jayeshv: Ok, Thanks. In fact, I have that doc string added in the coming diffs (for branches and… | |||||
| reverse(deprecation_mapping[request_path.url_name], query_params=args) | |||||
| ) | |||||
| return redirect_to_new_route | |||||
| @browse_route( | @browse_route( | ||||
| r"origin/directory/", view_name="browse-origin-directory", | r"origin/directory/", view_name="browse-origin-directory", | ||||
| ) | ) | ||||
| def origin_directory_browse(request): | def origin_directory_browse(request): | ||||
| """Django view for browsing the content of a directory associated | """Django view for browsing the content of a directory associated | ||||
| to an origin for a given visit. | to an origin for a given visit. | ||||
| The URL that points to it is :http:get:`/browse/origin/directory/` | The URL that points to it is :http:get:`/browse/origin/directory/` | ||||
| ▲ Show 20 Lines • Show All 74 Lines • ▼ Show 20 Lines | return browse_snapshot_content( | ||||
| path=path, | path=path, | ||||
| selected_language=request.GET.get("language"), | selected_language=request.GET.get("language"), | ||||
| ) | ) | ||||
| @browse_route( | @browse_route( | ||||
| r"origin/log/", view_name="browse-origin-log", | r"origin/log/", view_name="browse-origin-log", | ||||
| ) | ) | ||||
| @deprecated_route | |||||
| def origin_log_browse(request): | def origin_log_browse(request): | ||||
| """Django view that produces an HTML display of revisions history (aka | """ | ||||
Done Inline ActionsUse http:get:`/browse/snapshot/log` to link to endpoint doc
anlambert: %%%Use http:get:`/browse/snapshot/log` to link to endpoint doc
%%% | |||||
| This route is deprecated, use /snapshot-log/ instead | |||||
| Django view that produces an HTML display of revisions history (aka | |||||
| the commit log) associated to a software origin. | the commit log) associated to a software origin. | ||||
| The URL that points to it is :http:get:`/browse/origin/log/` | 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( | @browse_route( | ||||
| r"origin/(?P<origin_url>.+)/visit/(?P<timestamp>.+)/log/", | r"origin/(?P<origin_url>.+)/visit/(?P<timestamp>.+)/log/", | ||||
| r"origin/(?P<origin_url>.+)/log/", | r"origin/(?P<origin_url>.+)/log/", | ||||
| view_name="browse-origin-log-legacy", | view_name="browse-origin-log-legacy", | ||||
| ) | ) | ||||
| @deprecated_route | |||||
| def origin_log_browse_legacy(request, origin_url, timestamp=None): | def origin_log_browse_legacy(request, origin_url, timestamp=None): | ||||
| """Django view that produces an HTML display of revisions history (aka | """ | ||||
Done Inline Actionssame here anlambert: same here | |||||
| This route is deprecated, use /snapshot-log/ instead | |||||
| Django view that produces an HTML display of revisions history (aka | |||||
| the commit log) associated to a software origin. | the commit log) associated to a software origin. | ||||
| The URLs that point to it are | The URLs that point to it are | ||||
| :http:get:`/browse/origin/(origin_url)/log/` and | :http:get:`/browse/origin/(origin_url)/log/` and | ||||
| :http:get:`/browse/origin/(origin_url)/visit/(timestamp)/log/` | :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( | @browse_route( | ||||
| r"origin/branches/", view_name="browse-origin-branches", | r"origin/branches/", view_name="browse-origin-branches", | ||||
| ) | ) | ||||
| @deprecated_route | |||||
| def origin_branches_browse(request): | def origin_branches_browse(request): | ||||
| """Django view that produces an HTML display of the list of branches | """ | ||||
| This route is deprecated, use /snapshot-branches/ instead | |||||
| Django view that produces an HTML display of the list of branches | |||||
| associated to an origin for a given visit. | associated to an origin for a given visit. | ||||
| The URL that points to it is :http:get:`/browse/origin/branches/` | The URL that points to it is :http:get:`/browse/origin/branches/` | ||||
| """ | """ | ||||
| return browse_snapshot_branches( | |||||
| request, | |||||
| origin_url=request.GET.get("origin_url"), | |||||
| snapshot_id=request.GET.get("snapshot"), | |||||
| timestamp=request.GET.get("timestamp"), | |||||
| branch_name_include=request.GET.get("name_include"), | |||||
| ) | |||||
| @browse_route( | @browse_route( | ||||
| r"origin/(?P<origin_url>.+)/visit/(?P<timestamp>.+)/branches/", | r"origin/(?P<origin_url>.+)/visit/(?P<timestamp>.+)/branches/", | ||||
| r"origin/(?P<origin_url>.+)/branches/", | r"origin/(?P<origin_url>.+)/branches/", | ||||
| view_name="browse-origin-branches-legacy", | view_name="browse-origin-branches-legacy", | ||||
| ) | ) | ||||
| @deprecated_route | |||||
| def origin_branches_browse_legacy(request, origin_url, timestamp=None): | def origin_branches_browse_legacy(request, origin_url, timestamp=None): | ||||
| """Django view that produces an HTML display of the list of branches | """ | ||||
| This route is deprecated, use /snapshot-branches/ instead | |||||
| Django view that produces an HTML display of the list of branches | |||||
| associated to an origin for a given visit. | associated to an origin for a given visit. | ||||
| The URLs that point to it are | The URLs that point to it are | ||||
| :http:get:`/browse/origin/(origin_url)/branches/` and | :http:get:`/browse/origin/(origin_url)/branches/` and | ||||
| :http:get:`/browse/origin/(origin_url)/visit/(timestamp)/branches/` | :http:get:`/browse/origin/(origin_url)/visit/(timestamp)/branches/` | ||||
| """ | """ | ||||
| return browse_snapshot_branches( | |||||
| request, | |||||
| origin_url=origin_url, | |||||
| snapshot_id=request.GET.get("snapshot"), | |||||
| timestamp=timestamp, | |||||
| ) | |||||
| @browse_route( | @browse_route( | ||||
| r"origin/releases/", view_name="browse-origin-releases", | r"origin/releases/", view_name="browse-origin-releases", | ||||
| ) | ) | ||||
| def origin_releases_browse(request): | def origin_releases_browse(request): | ||||
| """Django view that produces an HTML display of the list of releases | """Django view that produces an HTML display of the list of releases | ||||
| associated to an origin for a given visit. | associated to an origin for a given visit. | ||||
| ▲ Show 20 Lines • Show All 122 Lines • Show Last 20 Lines | |||||
I think it will be better to add a view_redirect parameter to the decorator instead of storing the deprecation_mapping dict in it.
It will make the code more readable and the decorator more generic.