Просмотр исходного кода

fix(backup): page Gitea's tree off what came back, not what we asked for (#2656)

    The pager computed `seen = (page - 1) * 1000 + len(entries)`, taking the
    requested `per_page` as fact. Gitea clamps `per_page` to
    `MAX_RESPONSE_ITEMS`, which defaults to 50. So on a default install page 1
    returns 50 entries and sets `seen` to 50, then page 2 sets it to 1050 —
    which clears any `total_count` under 1050. The loop returns `success: true`
    holding the first 100 entries of a much larger tree.

    The restore then reads every missing path as "category not present in this
    commit" and skips it silently, which is precisely the failure this override
    was written to prevent. Same class as the GitLab pager fix, in the one
    direction that got left behind.

    Fix: accumulate `seen += len(entries)`. A genuinely over-cap tree still
    hard-fails rather than truncating; the cap is a page count, not a file
    count, because the page size is the server's choice.

    Test: a 120-entry tree served 50 at a time reaches its last entry, in three
    requests. Confirmed failing against the pre-fix backend — it stopped after
    two pages and reported 100 entries as the whole tree.
maziggy 3 недель назад
Родитель
Сommit
2d56ac9215

+ 15 - 3
backend/app/services/git_providers/gitea.py

@@ -121,9 +121,12 @@ class GiteaBackend(GitHubBackend):
         exact failure the GitHub version refuses to allow, so this pages instead.
 
         The cap mirrors GitLab's: reaching it means there are more pages, and
-        that is a failure rather than a partial result.
+        that is a failure rather than a partial result. Because the page size is
+        the server's choice rather than ours (see below), the cap is a page count
+        and not a file count.
         """
         blobs: dict[str, str] = {}
+        seen = 0
         page = 1
         while page <= 50:
             response = await client.get(
@@ -155,9 +158,18 @@ class GiteaBackend(GitHubBackend):
                     blobs[path] = sha
 
             # total_count counts every entry, trees included, so compare against
-            # what this page returned rather than against len(blobs).
+            # what came back rather than against len(blobs).
+            #
+            # Count what the server actually returned, never the per_page we
+            # asked for: Gitea clamps per_page to MAX_RESPONSE_ITEMS, which
+            # defaults to 50. Deriving the offset from the requested 1000 made
+            # page 2 report 1050 entries seen, which clears any total_count below
+            # that — so the loop stopped and returned the first two pages of a
+            # much larger tree as a success. The restore then read every missing
+            # path as "category not present in this commit" and skipped it
+            # silently, the exact failure this override exists to prevent.
             total = data.get("total_count")
-            seen = (page - 1) * 1000 + len(entries)
+            seen += len(entries)
             if not isinstance(total, int) or seen >= total or not entries:
                 return blobs, ""
             page += 1

+ 36 - 0
backend/tests/unit/test_git_providers_restore.py

@@ -425,6 +425,42 @@ class TestGiteaAndForgejoInheritReads:
         assert result["paths"] == ["a.json"]
         assert client.get.await_count == 1
 
+    @pytest.mark.asyncio
+    @pytest.mark.parametrize("backend_cls", [GiteaBackend, ForgejoBackend])
+    async def test_a_clamped_page_size_is_still_read_to_the_end(self, backend_cls):
+        """Gitea clamps per_page to MAX_RESPONSE_ITEMS — 50 by default (#2656).
+
+        Paging off the *requested* 1000 made page 2 believe it had seen 1050
+        entries, which clears any total_count below that. The loop then returned
+        the first 100 entries of a 120-entry tree as a success, and the restore
+        reported the categories it could not see as absent from the commit.
+        """
+        clamped = 50
+        total = 120
+        pages = []
+        for start in range(0, total, clamped):
+            count = min(clamped, total - start)
+            pages.append(
+                _make_mock_response(
+                    200,
+                    {
+                        "tree": [
+                            {"type": "blob", "path": f"f{i}.json", "sha": f"s{i}"} for i in range(start, start + count)
+                        ],
+                        "total_count": total,
+                    },
+                )
+            )
+        client = AsyncMock()
+        client.get = AsyncMock(side_effect=pages)
+
+        result = await backend_cls().list_tree("https://git.example.com/owner/repo", "tok", "abc1234", client)
+
+        assert result["success"] is True
+        assert client.get.await_count == 3
+        assert len(result["paths"]) == total
+        assert "f119.json" in result["paths"], "the tail of the tree is what a clamped pager loses"
+
     @pytest.mark.asyncio
     async def test_a_tree_beyond_the_page_cap_fails_rather_than_truncating(self):
         page = {"tree": [{"type": "blob", "path": f"f{i}.json", "sha": f"s{i}"} for i in range(1000)]}