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 @@ -8,6 +8,7 @@ from django.core.cache import cache +from swh.web.common import service from swh.web.common.exc import NotFoundExc from swh.web.common.typing import OriginInfo, OriginVisitInfo from swh.web.common.utils import parse_iso8601_date_to_utc @@ -110,19 +111,17 @@ Raises: swh.web.common.exc.NotFoundExc: if no visit can be found """ - - if not visit_ts and not visit_id and not snapshot_id: - from swh.web.common import service - - # returns the latest full visit with a valid snapshot + # returns the latest full visit with a valid snapshot + visit = service.lookup_origin_visit_latest( + origin_info["url"], allowed_statuses=["full"], require_snapshot=True + ) + if not visit: + # or the latest partial visit with a valid snapshot otherwise visit = service.lookup_origin_visit_latest( - origin_info["url"], allowed_statuses=["full"], require_snapshot=True + origin_info["url"], allowed_statuses=["partial"], require_snapshot=True ) - if not visit: - # or the latest partial visit with a valid snapshot otherwise - visit = service.lookup_origin_visit_latest( - origin_info["url"], allowed_statuses=["partial"], require_snapshot=True - ) + + if not visit_ts and not visit_id and not snapshot_id: if visit: return visit else: @@ -130,6 +129,11 @@ f"No valid visit for origin with url {origin_info['url']} found!" ) + # no need to fetch all visits list and search in it if the latest + # visit matches some criteria + if visit and (visit["snapshot"] == snapshot_id or visit["visit"] == visit_id): + return visit + visits = get_origin_visits(origin_info) if not visits: diff --git a/swh/web/common/service.py b/swh/web/common/service.py --- a/swh/web/common/service.py +++ b/swh/web/common/service.py @@ -21,7 +21,6 @@ from swh.web.common import converters from swh.web.common import query from swh.web.common.exc import BadInputExc, NotFoundExc -from swh.web.common.origin_visits import get_origin_visit from swh.web.common.typing import ( OriginInfo, OriginVisitInfo, @@ -568,6 +567,8 @@ else: raise TypeError('"origin" must be an int or a string.') + from swh.web.common.origin_visits import get_origin_visit + visit = get_origin_visit(origin, visit_ts=timestamp) branch = _get_snapshot_branch(visit["snapshot"], branch_name) rev_id = None 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 @@ -513,12 +513,14 @@ "swh.web.common.origin_visits.get_origin_visits" ) mock_get_origin_visits.return_value = [] + mock_service = mocker.patch("swh.web.common.origin_visits.service") + mock_service.lookup_origin_visit_latest.return_value = None url = reverse("browse-origin-directory", query_params={"origin_url": origin["url"]}) resp = client.get(url) assert resp.status_code == 404 assert_template_used(resp, "error.html") - assert_contains(resp, "No visit", status_code=404) - assert mock_get_origin_visits.called + assert_contains(resp, "No valid visit", status_code=404) + assert not mock_get_origin_visits.called @given(origin()) @@ -557,6 +559,8 @@ "swh.web.common.origin_visits.get_origin_visits" ) mock_get_origin_visits.return_value = [] + mock_service = mocker.patch("swh.web.common.origin_visits.service") + mock_service.lookup_origin_visit_latest.return_value = None url = reverse( "browse-origin-content", query_params={"origin_url": origin["url"], "path": "foo"}, @@ -564,8 +568,8 @@ resp = client.get(url) assert resp.status_code == 404 assert_template_used(resp, "error.html") - assert_contains(resp, "No visit", status_code=404) - assert mock_get_origin_visits.called + assert_contains(resp, "No valid visit", status_code=404) + assert not mock_get_origin_visits.called @given(origin()) 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 @@ -15,7 +15,11 @@ from swh.web.common.exc import NotFoundExc from swh.web.common.origin_visits import get_origin_visits, get_origin_visit from swh.web.common.typing import OriginInfo -from swh.web.tests.strategies import new_origin, new_snapshots +from swh.web.tests.strategies import ( + new_origin, + new_snapshots, + origin_with_multiple_visits, +) @given(new_snapshots(3)) @@ -233,3 +237,22 @@ # should return the last visit expected_visit = archive_data.origin_visit_get_by(new_origin.url, visits[-1]) assert get_origin_visit((OriginInfo(url=new_origin.url))) == expected_visit + + +@given(origin_with_multiple_visits()) +def test_get_origin_visit_latest_snapshot(mocker, origin): + origin_visits = get_origin_visits(origin) + first_visit = origin_visits[0] + latest_visit = origin_visits[-1] + mock_get_origin_visits = mocker.patch( + "swh.web.common.origin_visits.get_origin_visits" + ) + mock_get_origin_visits.return_value = origin_visits + + visit = get_origin_visit(origin, snapshot_id=latest_visit["snapshot"]) + assert visit == latest_visit + assert not mock_get_origin_visits.called + + visit = get_origin_visit(origin, snapshot_id=first_visit["snapshot"]) + assert visit == first_visit + assert mock_get_origin_visits.called