Jelajahi Sumber

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.
jmoore-skild 1 bulan lalu
induk
melakukan
582a6b18bc

+ 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)]}