diff --git a/swh/web/browse/snapshot_context.py b/swh/web/browse/snapshot_context.py --- a/swh/web/browse/snapshot_context.py +++ b/swh/web/browse/snapshot_context.py @@ -1,4 +1,4 @@ -# Copyright (C) 2018-2021 The Software Heritage developers +# Copyright (C) 2018-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 @@ -723,11 +723,12 @@ """ _check_origin_url(snapshot_id, origin_url) + visit_id = int(request.GET.get("visit_id", 0)) snapshot_context = get_snapshot_context( snapshot_id=snapshot_id, origin_url=origin_url, timestamp=timestamp, - visit_id=request.GET.get("visit_id"), + visit_id=visit_id or None, path=path, browse_context="directory", branch_name=request.GET.get("branch"), @@ -937,11 +938,12 @@ """ _check_origin_url(snapshot_id, origin_url) + visit_id = int(request.GET.get("visit_id", 0)) snapshot_context = get_snapshot_context( snapshot_id=snapshot_id, origin_url=origin_url, timestamp=timestamp, - visit_id=request.GET.get("visit_id"), + visit_id=visit_id or None, browse_context="log", branch_name=request.GET.get("branch"), release_name=request.GET.get("release"), @@ -1082,11 +1084,12 @@ """ _check_origin_url(snapshot_id, origin_url) + visit_id = int(request.GET.get("visit_id", 0)) snapshot_context = get_snapshot_context( snapshot_id=snapshot_id, origin_url=origin_url, timestamp=timestamp, - visit_id=request.GET.get("visit_id"), + visit_id=visit_id or None, ) branches_bc = request.GET.get("branches_breadcrumbs", "") @@ -1198,11 +1201,12 @@ """ _check_origin_url(snapshot_id, origin_url) + visit_id = int(request.GET.get("visit_id", 0)) snapshot_context = get_snapshot_context( snapshot_id=snapshot_id, origin_url=origin_url, timestamp=timestamp, - visit_id=request.GET.get("visit_id"), + visit_id=visit_id or None, ) rel_bc = request.GET.get("releases_breadcrumbs", "") diff --git a/swh/web/browse/views/content.py b/swh/web/browse/views/content.py --- a/swh/web/browse/views/content.py +++ b/swh/web/browse/views/content.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 @@ -180,12 +180,14 @@ raise BadInputExc( "The origin_url or snapshot query parameters must be provided." ) + + visit_id = int(request.GET.get("visit_id", 0)) snapshot_context = get_snapshot_context( snapshot_id=snapshot, origin_url=origin_url, path=path, timestamp=request.GET.get("timestamp"), - visit_id=request.GET.get("visit_id"), + visit_id=visit_id or None, branch_name=request.GET.get("branch"), release_name=request.GET.get("release"), browse_context="content", @@ -238,11 +240,12 @@ snapshot_context = None if origin_url is not None or snapshot_id is not None: try: + visit_id = int(request.GET.get("visit_id", 0)) snapshot_context = get_snapshot_context( origin_url=origin_url, snapshot_id=snapshot_id, timestamp=request.GET.get("timestamp"), - visit_id=request.GET.get("visit_id"), + visit_id=visit_id or None, branch_name=request.GET.get("branch"), release_name=request.GET.get("release"), revision_id=request.GET.get("revision"), diff --git a/swh/web/browse/views/release.py b/swh/web/browse/views/release.py --- a/swh/web/browse/views/release.py +++ b/swh/web/browse/views/release.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 @@ -47,14 +47,14 @@ if not origin_url: origin_url = request.GET.get("origin") timestamp = request.GET.get("timestamp") - visit_id = request.GET.get("visit_id") + visit_id = int(request.GET.get("visit_id", 0)) if origin_url: try: snapshot_context = get_snapshot_context( snapshot_id, origin_url, timestamp, - visit_id, + visit_id or None, release_name=release["name"], ) except NotFoundExc as e: diff --git a/swh/web/browse/views/revision.py b/swh/web/browse/views/revision.py --- a/swh/web/browse/views/revision.py +++ b/swh/web/browse/views/revision.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 @@ -197,11 +197,12 @@ snapshot_id = request.GET.get("snapshot") snapshot_context = None if origin_url or snapshot_id: + visit_id = int(request.GET.get("visit_id", 0)) snapshot_context = get_snapshot_context( snapshot_id=snapshot_id, origin_url=origin_url, timestamp=request.GET.get("timestamp"), - visit_id=request.GET.get("visit_id"), + visit_id=visit_id or None, branch_name=request.GET.get("branch"), release_name=request.GET.get("release"), revision_id=sha1_git, @@ -309,7 +310,7 @@ if not origin_url: origin_url = request.GET.get("origin") timestamp = request.GET.get("timestamp") - visit_id = request.GET.get("visit_id") + visit_id = int(request.GET.get("visit_id", 0)) snapshot_id = request.GET.get("snapshot_id") if not snapshot_id: snapshot_id = request.GET.get("snapshot") @@ -323,7 +324,7 @@ snapshot_id=snapshot_id, origin_url=origin_url, timestamp=timestamp, - visit_id=visit_id, + visit_id=visit_id or None, branch_name=request.GET.get("branch"), release_name=request.GET.get("release"), revision_id=sha1_git, diff --git a/swh/web/common/archive.py b/swh/web/common/archive.py --- a/swh/web/common/archive.py +++ b/swh/web/common/archive.py @@ -1067,20 +1067,22 @@ def origin_visit_find_by_date( - origin_url: str, visit_date: datetime.datetime + origin_url: str, visit_date: datetime.datetime, greater_or_equal: bool = True ) -> Optional[OriginVisitInfo]: """Retrieve origin visit status whose date is most recent than the provided visit_date. Args: origin_url: origin concerned by the visit visit_date: provided visit date + greater_or_equal: ensure returned visit has a date greater or equal + than the one passed as parameter Returns: The dict origin_visit_status matching the criteria if any. """ visit = storage.origin_visit_find_by_date(origin_url, visit_date) - if visit and visit.date < visit_date: + if greater_or_equal and visit and visit.date < visit_date: # when visit is anterior to the provided date, trying to find another one most # recent visits = storage.origin_visit_get( diff --git a/swh/web/common/origin_visits.py b/swh/web/common/origin_visits.py --- a/swh/web/common/origin_visits.py +++ b/swh/web/common/origin_visits.py @@ -1,9 +1,8 @@ -# Copyright (C) 2018-2019 The Software Heritage developers +# Copyright (C) 2018-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 math from typing import List, Optional from django.core.cache import cache @@ -136,6 +135,25 @@ if visit and (visit["snapshot"] == snapshot_id or visit["visit"] == visit_id): return visit + if visit_id: + return archive.lookup_origin_visit(origin_info["url"], visit_id) + + if visit_ts: + visit = archive.origin_visit_find_by_date( + origin_info["url"], + parse_iso8601_date_to_utc(visit_ts), + greater_or_equal=False, + ) + if visit is not None: + return visit + else: + raise NotFoundExc( + ( + "Visit with timestamp %s for origin with " + "url %s not found!" % (visit_ts, origin_info["url"]) + ) + ) + visits = get_origin_visits(origin_info) if not visits: @@ -154,46 +172,4 @@ ) return visits[0] - if visit_id: - visits = [v for v in visits if v["visit"] == int(visit_id)] - if len(visits) == 0: - raise NotFoundExc( - ( - "Visit with id %s for origin with" - " url %s not found!" % (visit_id, origin_info["url"]) - ) - ) - return visits[0] - - if visit_ts: - - target_visit_ts = math.floor(parse_iso8601_date_to_utc(visit_ts).timestamp()) - - # Find the visit with date closest to the target (in absolute value) - (abs_time_delta, visit_idx) = min( - ( - (math.floor(parse_iso8601_date_to_utc(visit["date"]).timestamp()), i) - for (i, visit) in enumerate(visits) - ), - key=lambda ts_and_i: abs(ts_and_i[0] - target_visit_ts), - ) - - if visit_idx is not None: - visit = visits[visit_idx] - # If multiple visits have the same date, select the one with - # the largest id. - while ( - visit_idx < len(visits) - 1 - and visit["date"] == visits[visit_idx + 1]["date"] - ): - visit_idx = visit_idx + 1 - visit = visits[visit_idx] - return visit - else: - raise NotFoundExc( - ( - "Visit with timestamp %s for origin with " - "url %s not found!" % (visit_ts, origin_info["url"]) - ) - ) return visits[-1] 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 @@ -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 @@ -402,22 +402,17 @@ assert not mock_get_origin_visits.called -def test_browse_origin_directory_unknown_visit(client, mocker, origin): - mock_get_origin_visits = mocker.patch( - "swh.web.common.origin_visits.get_origin_visits" - ) - mock_get_origin_visits.return_value = [{"visit": 1}] +def test_browse_origin_directory_unknown_visit(client, origin): url = reverse( "browse-origin-directory", - query_params={"origin_url": origin["url"], "visit_id": 2}, + query_params={"origin_url": origin["url"], "visit_id": 200}, ) resp = check_html_get_response( client, url, status_code=404, template_used="error.html" ) - assert re.search("Visit.*not found", resp.content.decode("utf-8")) - assert mock_get_origin_visits.called + assert re.search("visit.*not found", resp.content.decode("utf-8")) def test_browse_origin_directory_not_found(client, origin): diff --git a/swh/web/tests/common/test_archive.py b/swh/web/tests/common/test_archive.py --- a/swh/web/tests/common/test_archive.py +++ b/swh/web/tests/common/test_archive.py @@ -258,12 +258,15 @@ archive_data.origin_visit_status_add(visit_statuses) # Check correct visit is returned when searching by date - for search_date, expected_visit in [ - (first_visit_date, 1), - (pivot_date, 2), - (second_visit_date, 2), + for search_date, greater_or_equal, expected_visit in [ + (first_visit_date, True, 1), + (pivot_date, True, 2), + (pivot_date, False, 1), + (second_visit_date, True, 2), ]: - origin_visit = archive.origin_visit_find_by_date(new_origin.url, search_date) + origin_visit = archive.origin_visit_find_by_date( + new_origin.url, search_date, greater_or_equal + ) assert origin_visit["visit"] == expected_visit diff --git a/swh/web/tests/common/test_origin_visits.py b/swh/web/tests/common/test_origin_visits.py --- a/swh/web/tests/common/test_origin_visits.py +++ b/swh/web/tests/common/test_origin_visits.py @@ -1,4 +1,4 @@ -# Copyright (C) 2018-2021 The Software Heritage developers +# Copyright (C) 2018-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 @@ -82,8 +82,8 @@ with pytest.raises(NotFoundExc) as e: visit = get_origin_visit(origin_info, visit_id=visit_id) - assert e.match("Visit with id %s" % visit_id) - assert e.match("url %s" % origin_info["url"]) + assert e.match("visit with id %s" % visit_id) + assert e.match("Origin %s" % origin_info["url"]) visit_id = visits[1].visit visit = get_origin_visit(origin_info, visit_id=visit_id)