swh/web/conftest.py

@@ -38,7 +38,6 @@ from swh.model.hashutil import (
 from swh.model.model import Content, Directory, Revision
 from swh.model.swhids import CoreSWHID, ObjectType
 from swh.scheduler.tests.common import TASK_TYPES
-from swh.storage.algos.origin import origin_get_latest_visit_status
 from swh.storage.algos.revisions_walker import get_revisions_walker
 from swh.storage.algos.snapshot import (
     snapshot_get_all_branches,
@@ -1010,23 +1009,17 @@ class _ArchiveData:
                 visit_status = self.storage.origin_visit_status_get_latest(
                     origin_url, visit.visit
                 )
-                visits.append(
-                    converters.from_origin_visit(
-                        {**visit_status.to_dict(), "type": visit.type}
-                    )
-                )
+                visits.append(converters.from_origin_visit(visit, visit_status))
             if not next_page_token:
                 break
         return visits
 
-    def origin_visit_get_by(self, origin_url: str, visit_id: int) -> OriginVisitInfo:
+    def origin_visit_get_by(
+        self, origin_url: str, visit_id: int
+    ) -> Optional[OriginVisitInfo]:
         visit = self.storage.origin_visit_get_by(origin_url, visit_id)
-        assert visit is not None
         visit_status = self.storage.origin_visit_status_get_latest(origin_url, visit_id)
-        assert visit_status is not None
-        return converters.from_origin_visit(
-            {**visit_status.to_dict(), "type": visit.type}
-        )
+        return converters.from_origin_visit(visit, visit_status)
 
     def origin_visit_status_get_latest(
         self,
@@ -1035,17 +1028,20 @@ class _ArchiveData:
         allowed_statuses: Optional[List[str]] = None,
         require_snapshot: bool = False,
     ):
-        visit_status = origin_get_latest_visit_status(
-            self.storage,
+        visit = self.storage.origin_visit_get_latest(
             origin_url,
             type=type,
             allowed_statuses=allowed_statuses,
             require_snapshot=require_snapshot,
         )
+        visit_status = self.storage.origin_visit_status_get_latest(
+            origin_url,
+            visit.visit,
+            allowed_statuses=allowed_statuses,
+            require_snapshot=require_snapshot,
+        )
         return (
-            converters.from_origin_visit(visit_status.to_dict())
-            if visit_status
-            else None
+            converters.from_origin_visit(visit, visit_status) if visit_status else None
         )
 
     def snapshot_get(self, snapshot_id):

swh/web/save_bulk/api_views.py

@@ -26,7 +26,7 @@ from swh.web.auth.utils import API_SAVE_BULK_PERMISSION
 from swh.web.config import get_config, scheduler
 from swh.web.save_bulk.models import SaveBulkOrigin, SaveBulkRequest
 from swh.web.save_code_now.origin_save import validate_origin_url
-from swh.web.utils import reverse
+from swh.web.utils import datetime_to_utc, reverse
 from swh.web.utils.exc import BadInputExc, ForbiddenExc, NotFoundExc, UnauthorizedExc
 
 save_bulk_api_urls = APIUrls()
@@ -452,7 +452,9 @@ def api_origin_save_bulk_request_info(request: Request, request_id: UUID):
             (listed_origin.url, listed_origin.visit_type): listed_origin
             for listed_origin in scheduler()
             .get_listed_origins(
-                lister.id, urls=[origin.origin_url for origin in page.object_list]
+                lister.id,
+                urls=[origin.origin_url for origin in page.object_list],
+                limit=per_page,
             )
             .results
         }
@@ -514,10 +516,14 @@ def api_origin_save_bulk_request_info(request: Request, request_id: UUID):
                 visit_type=origin.visit_type,
                 status=status,
                 last_scheduling_date=(
-                    last_scheduled.isoformat() if last_scheduled else None
+                    datetime_to_utc(last_scheduled).isoformat()
+                    if last_scheduled
+                    else None
                 ),
                 last_visit_date=(
-                    last_visit_date.isoformat() if last_visit_date else None
+                    datetime_to_utc(last_visit_date).isoformat()
+                    if last_visit_date
+                    else None
                 ),
                 last_visit_status=(
                     last_visit_status.value if last_visit_status else None

swh/web/save_bulk/tests/test_api_views.py

@@ -401,7 +401,7 @@ def test_save_bulk_request_info_invalid_user(api_client, save_bulk_user):
     }
 
 
-NB_SUBMITTED_ORIGINS = 1000
+NB_SUBMITTED_ORIGINS = 2000
 
 
 @pytest.fixture
@@ -506,6 +506,7 @@ def test_save_bulk_request_info_successful_before_lister_processing(
     url = reverse(
         "api-1-save-origin-bulk-request-info",
         url_args={"request_id": request_id},
+        query_params={"per_page": NB_SUBMITTED_ORIGINS},
     )
     resp = check_api_get_responses(api_client, url, status_code=200)
 
@@ -527,7 +528,36 @@ def save_bulk_request_info_user(request):
     return request.getfixturevalue(request.param)
 
 
-def test_save_bulk_request_info_successful_after_lister_processing(
+def test_save_bulk_request_info_successful_after_lister_processing_no_pagination(
+    api_client, save_bulk_request_info_user, save_bulk_request_info_lister_data
+):
+    api_client.force_login(save_bulk_request_info_user)
+    request_id, accepted_origins, rejected_origins = save_bulk_request_info_lister_data
+
+    url = reverse(
+        "api-1-save-origin-bulk-request-info",
+        url_args={"request_id": request_id},
+        query_params={"per_page": NB_SUBMITTED_ORIGINS},
+    )
+    resp = check_api_get_responses(api_client, url, status_code=200)
+    submitted_origins_info = resp.data
+
+    assert len(submitted_origins_info) == len(accepted_origins) + len(rejected_origins)
+
+    assert {
+        origin_info["origin_url"]
+        for origin_info in submitted_origins_info
+        if origin_info["status"] == "accepted"
+    } == {origin["origin_url"] for origin in accepted_origins}
+
+    assert {
+        origin_info["origin_url"]
+        for origin_info in submitted_origins_info
+        if origin_info["status"] == "rejected"
+    } == {origin["origin_url"] for origin in rejected_origins}
+
+
+def test_save_bulk_request_info_successful_after_lister_processing_pagination(
     api_client, save_bulk_request_info_user, save_bulk_request_info_lister_data
 ):
     request_id, accepted_origins, rejected_origins = save_bulk_request_info_lister_data

swh/web/utils/archive.py

@@ -24,7 +24,6 @@ from swh.model.hashutil import hash_to_bytes, hash_to_hex
 from swh.model.model import ExtID, Origin, Revision
 from swh.model.swhids import CoreSWHID, ObjectType, QualifiedSWHID
 from swh.storage.algos import diff, revisions_walker
-from swh.storage.algos.origin import origin_get_latest_visit_status
 from swh.storage.algos.snapshot import snapshot_get_latest, snapshot_resolve_alias
 from swh.storage.interface import ListOrder, OriginVisitWithStatuses
 from swh.vault.exc import NotFoundExc as VaultNotFoundExc
@@ -1228,7 +1227,9 @@ def lookup_origin_visits(
 
     """
     for visit in _lookup_origin_visits(origin, last_visit=last_visit, limit=per_page):
-        yield converters.from_origin_visit(visit.statuses[-1].to_dict())
+        origin_visit = converters.from_origin_visit(visit.visit, visit.statuses[-1])
+        assert origin_visit
+        yield origin_visit
 
 
 def lookup_origin_visit_latest(
@@ -1261,16 +1262,23 @@ def lookup_origin_visit_latest(
         "url"
     ]
 
-    visit_status = origin_get_latest_visit_status(
-        storage,
+    visit = storage.origin_visit_get_latest(
         origin_url,
         type=type,
         allowed_statuses=allowed_statuses,
         require_snapshot=require_snapshot,
     )
-    return (
-        converters.from_origin_visit(visit_status.to_dict()) if visit_status else None
+    visit_status = (
+        storage.origin_visit_status_get_latest(
+            origin_url,
+            visit.visit,
+            allowed_statuses=allowed_statuses,
+            require_snapshot=require_snapshot,
+        )
+        if visit and visit.visit
+        else None
     )
+    return converters.from_origin_visit(visit, visit_status)
 
 
 def lookup_origin_visit(
@@ -1303,12 +1311,9 @@ def lookup_origin_visit(
             f"Origin {origin_url} or its visit with id {visit_id} not found!"
         )
     visit_status = storage.origin_visit_status_get_latest(origin_url, visit_id)
-    if visit_status:
-        return converters.from_origin_visit(
-            {**visit_status.to_dict(), "type": visit.type}
-        )
-    else:
-        return converters.from_origin_visit(visit.to_dict())
+    origin_visit = converters.from_origin_visit(visit, visit_status)
+    assert origin_visit
+    return origin_visit
 
 
 def origin_visit_find_by_date(
@@ -1331,6 +1336,7 @@ def origin_visit_find_by_date(
 
     """
     visit = storage.origin_visit_find_by_date(origin_url, visit_date, type)
+    visit_status = None
     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
@@ -1344,24 +1350,15 @@ def origin_visit_find_by_date(
             )
 
         visit_id = visit.visit
-        visit = None
         for next_visit in iter_origin_visits_from(str(visit_id)):
             if next_visit.date >= visit_date and (
                 type is None or next_visit.type == type
             ):
                 visit = next_visit
                 break
-
     if visit and visit.visit:
         visit_status = storage.origin_visit_status_get_latest(origin_url, visit.visit)
-        if visit_status:
-            return converters.from_origin_visit(
-                {**visit_status.to_dict(), "type": visit.type}
-            )
-        else:
-            return converters.from_origin_visit(visit.to_dict())
-    else:
-        return None
+    return converters.from_origin_visit(visit, visit_status)
 
 
 def lookup_snapshot_sizes(

swh/web/utils/converters.py

@@ -5,13 +5,15 @@
 
 import datetime
 import json
-from typing import Any, Dict, Mapping, Union
+from typing import Any, Dict, Mapping, Optional, Union
 
 from django.core.serializers.json import DjangoJSONEncoder
 
 from swh.core.utils import decode_with_escape
 from swh.model import hashutil
 from swh.model.model import (
+    OriginVisit,
+    OriginVisitStatus,
     RawExtrinsicMetadata,
     Release,
     Revision,
@@ -321,18 +323,26 @@ def from_person(person):
     return from_swh(person, bytess={"name", "fullname", "email"})
 
 
-def from_origin_visit(visit: Dict[str, Any]) -> OriginVisitInfo:
+def from_origin_visit(
+    visit: Optional[OriginVisit], visit_status: Optional[OriginVisitStatus] = None
+) -> Optional[OriginVisitInfo]:
     """Convert swh origin_visit to serializable origin_visit dictionary."""
-    ov = from_swh(
-        visit,
+    if visit is None:
+        return None
+    if visit_status is not None:
+        origin_visit = visit_status.to_dict()
+        # override visit status date with visit date
+        origin_visit.update(visit.to_dict())
+    else:
+        origin_visit = visit.to_dict()
+    return from_swh(
+        origin_visit,
         hashess={"target", "snapshot"},
         bytess={"branch"},
         dates={"date"},
         empty_dict={"metadata"},
     )
 
-    return ov
-
 
 def from_snapshot(snapshot):
     """Convert swh snapshot to serializable (partial) snapshot dictionary."""

swh/web/utils/tests/test_converters.py

-# Copyright (C) 2015-2021  The Software Heritage developers
+# Copyright (C) 2015-2024  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
@@ -9,6 +9,8 @@ import hashlib
 from swh.model import hashutil
 from swh.model.model import (
     ObjectType,
+    OriginVisit,
+    OriginVisitStatus,
     Person,
     Release,
     Revision,
@@ -178,35 +180,40 @@ def test_from_origin():
     assert actual_origin == expected_origin
 
 
-def test_from_origin_visit():
-    snap_hash = "b5f0b7f716735ebffe38505c60145c4fd9da6ca3"
+def test_from_origin_visit(snapshot):
+    visit_date = datetime.datetime(2015, 1, 1, 22, 0, 0, tzinfo=datetime.timezone.utc)
+    visit_status_date = visit_date + datetime.timedelta(minutes=10)
 
-    for snap in [snap_hash, None]:
+    for snap in [snapshot, None]:
         visit = {
-            "date": {
-                "timestamp": datetime.datetime(
-                    2015, 1, 1, 22, 0, 0, tzinfo=datetime.timezone.utc
-                ).timestamp(),
-                "offset": 0,
-                "negative_utc": False,
-            },
-            "origin": 10,
+            "date": visit_date,
+            "origin": "https://git.example.org/foo",
+            "visit": 100,
+            "type": "git",
+        }
+        visit_status = {
+            "date": visit_status_date,
+            "origin": "https://git.example.org/foo",
             "visit": 100,
             "metadata": None,
             "status": "full",
-            "snapshot": hashutil.hash_to_bytes(snap) if snap else snap,
+            "snapshot": hashutil.hash_to_bytes(snap) if snap else None,
+            "type": "git",
         }
 
         expected_visit = {
-            "date": "2015-01-01T22:00:00+00:00",
-            "origin": 10,
+            "date": visit_date.isoformat(),
+            "origin": "https://git.example.org/foo",
             "visit": 100,
             "metadata": {},
             "status": "full",
-            "snapshot": snap_hash if snap else snap,
+            "snapshot": snap if snap else None,
+            "type": "git",
         }
 
-        actual_visit = converters.from_origin_visit(visit)
+        actual_visit = converters.from_origin_visit(
+            OriginVisit.from_dict(visit), OriginVisitStatus.from_dict(visit_status)
+        )
 
         assert actual_visit == expected_visit