diff --git a/backend/app/services/git_providers/base.py b/backend/app/services/git_providers/base.py index cb5f89744..34bbef855 100644 --- a/backend/app/services/git_providers/base.py +++ b/backend/app/services/git_providers/base.py @@ -97,6 +97,18 @@ class GitProviderBackend(ABC): Returns ``{"success", "message", "commits": [{"sha", "message", "author", "date"}]}``. """ + @abstractmethod + async def get_commit(self, repo_url: str, token: str, ref: str, client: httpx.AsyncClient) -> dict: + """Read one commit's display metadata by SHA. + + ``list_commits`` only reaches back as far as its limit, so a ref outside + that window has no entry to describe it. This is the direct lookup for + that case. + + Returns ``{"success", "message", "commit": {"sha", "message", "author", + "date"} | None}``. + """ + @abstractmethod async def list_tree( self, @@ -112,7 +124,10 @@ class GitProviderBackend(ABC): one being restored are provably the same commit even if a scheduled backup lands in between. - Returns ``{"success", "message", "paths": [str]}``. + Returns ``{"success", "message", "paths": [str], "blob_shas": + {path: sha}}``. ``blob_shas`` is the path -> blob SHA map the listing + already had to build, offered so :meth:`fetch_files` need not fetch the + same tree again; providers that read files by path return ``{}``. """ @abstractmethod @@ -123,6 +138,7 @@ class GitProviderBackend(ABC): ref: str, paths: list[str], client: httpx.AsyncClient, + blob_shas: dict[str, str] | None = None, ) -> dict: """Read several files' decoded UTF-8 text at ``ref``. @@ -130,6 +146,11 @@ class GitProviderBackend(ABC): listing to map path -> blob SHA can do that lookup once for the whole restore instead of per file. + ``blob_shas`` is the map :meth:`list_tree` returned for the same ref, if + the caller has one. Passing it saves a second recursive tree GET; a + provider that reads by path ignores it, and one that needs it fetches + the tree itself when it is absent. + Returns ``{"success", "message", "files": {path: text}}``. Paths absent from the commit are simply missing from ``files`` — that is not an error, since which categories a given backup contains varies by config. diff --git a/backend/app/services/git_providers/gitea.py b/backend/app/services/git_providers/gitea.py index 57c2ed017..5b7ea024a 100644 --- a/backend/app/services/git_providers/gitea.py +++ b/backend/app/services/git_providers/gitea.py @@ -100,6 +100,73 @@ class GiteaBackend(GitHubBackend): headers["Accept"] = "application/json" return headers + async def _blob_shas_at( + self, + client: httpx.AsyncClient, + headers: dict, + api_base: str, + owner: str, + repo: str, + ref: str, + ) -> tuple[dict[str, str] | None, str]: + """Paged override of GitHub's single-GET tree read (#2656). + + Divergence four, alongside the three in the class docstring. GitHub's + recursive trees endpoint is not paginated and signals overflow with + ``truncated: true``, which the inherited implementation hard-fails on. + Gitea and Forgejo *do* page the same endpoint — ``page``/``per_page``, + with ``total_count`` alongside the tree — so the inherited version would + read only the first page and then report every category beyond it as + absent from the commit. A restore that silently skips categories is the + 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. + """ + blobs: dict[str, str] = {} + page = 1 + while page <= 50: + response = await client.get( + f"{api_base}/repos/{owner}/{repo}/git/trees/{ref}", + headers=headers, + params={"recursive": "true", "page": page, "per_page": 1000}, + ) + if response.status_code == 404: + return None, f"Commit or tree '{ref}' not found in the repository" + if response.status_code != 200: + return None, ( + f"Failed to list tree (HTTP {response.status_code}): {self._truncated_response_text(response)}" + ) + try: + data = response.json() + except ValueError: + return None, "Non-JSON response listing tree" + if not isinstance(data, dict): + return None, "Unexpected shape listing tree" + + entries = data.get("tree") + if not isinstance(entries, list): + entries = [] + for item in entries: + if not isinstance(item, dict) or item.get("type") != "blob": + continue + path, sha = item.get("path"), item.get("sha") + if isinstance(path, str) and isinstance(sha, str) and path and sha: + blobs[path] = sha + + # total_count counts every entry, trees included, so compare against + # what this page returned rather than against len(blobs). + total = data.get("total_count") + seen = (page - 1) * 1000 + len(entries) + if not isinstance(total, int) or seen >= total or not entries: + return blobs, "" + page += 1 + + return None, ( + "Repository tree exceeds the listing limit, so the backup contents cannot be " + "enumerated reliably. Rotate the backup repository." + ) + async def push_files( self, repo_url: str, diff --git a/backend/app/services/git_providers/github.py b/backend/app/services/git_providers/github.py index dd0e99a82..78d06deab 100644 --- a/backend/app/services/git_providers/github.py +++ b/backend/app/services/git_providers/github.py @@ -229,6 +229,39 @@ class GitHubBackend(GitProviderBackend): blobs[path] = sha return blobs, "" + async def get_commit(self, repo_url: str, token: str, ref: str, client: httpx.AsyncClient) -> dict: + """Read one commit's metadata directly, for refs outside the list window.""" + try: + owner, repo = self.parse_repo_url(repo_url) + api_base = self.get_api_base(repo_url) + headers = self.get_headers(token) + + response = await client.get(f"{api_base}/repos/{owner}/{repo}/commits/{ref}", headers=headers) + if response.status_code == 404: + return {"success": False, "message": f"Commit '{ref}' not found in the repository", "commit": None} + if response.status_code != 200: + msg = f"Failed to read commit (HTTP {response.status_code}): {self._truncated_response_text(response)}" + logger.warning("get_commit %s/%s ref=%s: %s", owner, repo, ref, msg) + return {"success": False, "message": msg, "commit": None} + + try: + data = response.json() + except ValueError: + return {"success": False, "message": "Non-JSON response reading commit", "commit": None} + if not isinstance(data, dict): + return {"success": False, "message": "Unexpected shape reading commit", "commit": None} + + # Same entry shape as list_commits, so callers can treat the two + # interchangeably. + parsed = self._parse_commit_entries([data], 1) + if not parsed: + return {"success": False, "message": "Commit response carried no SHA", "commit": None} + return {"success": True, "message": "OK", "commit": parsed[0]} + + except Exception as e: + logger.exception("get_commit failed for %s ref=%s", repo_url, ref) + return {"success": False, "message": f"{type(e).__name__}: {str(e)[:200]}", "commit": None} + async def list_tree( self, repo_url: str, @@ -245,13 +278,15 @@ class GitHubBackend(GitProviderBackend): blobs, error = await self._blob_shas_at(client, headers, api_base, owner, repo, ref) if blobs is None: logger.warning("list_tree %s/%s ref=%s: %s", owner, repo, ref, error) - return {"success": False, "message": error, "paths": []} + return {"success": False, "message": error, "paths": [], "blob_shas": {}} - return {"success": True, "message": "OK", "paths": sorted(blobs)} + # The map is handed back so fetch_files does not GET the same + # recursive tree a second time for the same ref. + return {"success": True, "message": "OK", "paths": sorted(blobs), "blob_shas": blobs} except Exception as e: logger.exception("list_tree failed for %s ref=%s", repo_url, ref) - return {"success": False, "message": f"{type(e).__name__}: {str(e)[:200]}", "paths": []} + return {"success": False, "message": f"{type(e).__name__}: {str(e)[:200]}", "paths": [], "blob_shas": {}} async def fetch_files( self, @@ -260,6 +295,7 @@ class GitHubBackend(GitProviderBackend): ref: str, paths: list[str], client: httpx.AsyncClient, + blob_shas: dict[str, str] | None = None, ) -> dict: """Read ``paths`` at ``ref`` via the Git Data blobs API. @@ -273,10 +309,12 @@ class GitHubBackend(GitProviderBackend): api_base = self.get_api_base(repo_url) headers = self.get_headers(token) - blobs, error = await self._blob_shas_at(client, headers, api_base, owner, repo, ref) + blobs = blob_shas if blobs is None: - logger.warning("fetch_files %s/%s ref=%s: %s", owner, repo, ref, error) - return {"success": False, "message": error, "files": {}} + blobs, error = await self._blob_shas_at(client, headers, api_base, owner, repo, ref) + if blobs is None: + logger.warning("fetch_files %s/%s ref=%s: %s", owner, repo, ref, error) + return {"success": False, "message": error, "files": {}} files: dict[str, str] = {} for path in paths: diff --git a/backend/app/services/git_providers/gitlab.py b/backend/app/services/git_providers/gitlab.py index e4ee61eb7..cedfd5383 100644 --- a/backend/app/services/git_providers/gitlab.py +++ b/backend/app/services/git_providers/gitlab.py @@ -185,6 +185,48 @@ class GitLabBackend(GitProviderBackend): logger.exception("list_commits failed for %s branch=%s", repo_url, branch) return {"success": False, "message": f"{type(e).__name__}: {str(e)[:200]}", "commits": []} + async def get_commit(self, repo_url: str, token: str, ref: str, client: httpx.AsyncClient) -> dict: + """Read one commit's metadata directly, for refs outside the list window.""" + try: + api_base = self.get_api_base(repo_url) + headers = self.get_headers(token) + encoded_path = self._encoded_project(repo_url) + + response = await client.get( + f"{api_base}/projects/{encoded_path}/repository/commits/{urllib.parse.quote(ref, safe='')}", + headers=headers, + ) + if response.status_code == 404: + return {"success": False, "message": f"Commit '{ref}' not found in the repository", "commit": None} + if response.status_code != 200: + msg = f"Failed to read commit (HTTP {response.status_code}): {self._truncated_response_text(response)}" + logger.warning("get_commit %s ref=%s: %s", repo_url, ref, msg) + return {"success": False, "message": msg, "commit": None} + + try: + data = response.json() + except ValueError: + return {"success": False, "message": "Non-JSON response reading commit", "commit": None} + sha = data.get("id") if isinstance(data, dict) else None + if not isinstance(sha, str) or not sha: + return {"success": False, "message": "Commit response carried no SHA", "commit": None} + + # GitLab flattens author/date onto the commit, as in list_commits. + return { + "success": True, + "message": "OK", + "commit": { + "sha": sha, + "message": data.get("message") or "", + "author": data.get("author_name") or "", + "date": data.get("committed_date") or data.get("created_at") or "", + }, + } + + except Exception as e: + logger.exception("get_commit failed for %s ref=%s", repo_url, ref) + return {"success": False, "message": f"{type(e).__name__}: {str(e)[:200]}", "commit": None} + async def list_tree( self, repo_url: str, @@ -200,9 +242,11 @@ class GitLabBackend(GitProviderBackend): paths: list[str] = [] page = 1 + complete = False # GitLab's tree endpoint paginates instead of exposing a "truncated" # flag, so walk pages until one comes back short. The page cap stops - # a malformed X-Next-Page loop from spinning forever. + # a malformed X-Next-Page loop from spinning forever — and reaching + # it is a failure, not a result: see the check after the loop. while page <= 50: response = await client.get( f"{api_base}/projects/{encoded_path}/repository/tree", @@ -214,20 +258,21 @@ class GitLabBackend(GitProviderBackend): "success": False, "message": f"Commit or tree '{ref}' not found in the repository", "paths": [], + "blob_shas": {}, } if response.status_code != 200: msg = ( f"Failed to list tree (HTTP {response.status_code}): {self._truncated_response_text(response)}" ) logger.warning("list_tree %s ref=%s: %s", repo_url, ref, msg) - return {"success": False, "message": msg, "paths": []} + return {"success": False, "message": msg, "paths": [], "blob_shas": {}} try: data = response.json() except ValueError: - return {"success": False, "message": "Non-JSON response listing tree", "paths": []} + return {"success": False, "message": "Non-JSON response listing tree", "paths": [], "blob_shas": {}} if not isinstance(data, list): - return {"success": False, "message": "Unexpected shape listing tree", "paths": []} + return {"success": False, "message": "Unexpected shape listing tree", "paths": [], "blob_shas": {}} for item in data: if isinstance(item, dict) and item.get("type") == "blob": @@ -236,14 +281,29 @@ class GitLabBackend(GitProviderBackend): paths.append(path) if len(data) < 100: + complete = True break page += 1 - return {"success": True, "message": "OK", "paths": sorted(paths)} + if not complete: + # Falling out of the loop means the last page was full and there + # are more. Returning success here would hand the restore a + # silently partial path list, and it would then report the + # categories it could not see as "not present in this commit" — + # the same failure GitHub's truncated=true check refuses to allow. + msg = ( + "Repository tree exceeds the listing limit (more than 5000 files), so the backup " + "contents cannot be enumerated reliably. Rotate the backup repository." + ) + logger.warning("list_tree %s ref=%s: %s", repo_url, ref, msg) + return {"success": False, "message": msg, "paths": [], "blob_shas": {}} + + # GitLab reads files by path, so there is no blob-SHA map to share. + return {"success": True, "message": "OK", "paths": sorted(paths), "blob_shas": {}} except Exception as e: logger.exception("list_tree failed for %s ref=%s", repo_url, ref) - return {"success": False, "message": f"{type(e).__name__}: {str(e)[:200]}", "paths": []} + return {"success": False, "message": f"{type(e).__name__}: {str(e)[:200]}", "paths": [], "blob_shas": {}} async def fetch_files( self, @@ -252,8 +312,14 @@ class GitLabBackend(GitProviderBackend): ref: str, paths: list[str], client: httpx.AsyncClient, + blob_shas: dict[str, str] | None = None, ) -> dict: - """Read ``paths`` at ``ref`` via /repository/files/{path}.""" + """Read ``paths`` at ``ref`` via /repository/files/{path}. + + ``blob_shas`` is accepted for interface parity and ignored: this backend + addresses files by path, so it never needed the tree listing that makes + the map worth passing. + """ try: api_base = self.get_api_base(repo_url) headers = self.get_headers(token) diff --git a/backend/app/services/github_restore.py b/backend/app/services/github_restore.py index f49450e28..09dadc981 100644 --- a/backend/app/services/github_restore.py +++ b/backend/app/services/github_restore.py @@ -307,21 +307,54 @@ class GitHubRestoreService: result["branch"] = config.branch return result - async def _resolve_ref(self, config: GitHubBackupConfig, ref: str) -> tuple[str | None, str]: + async def _resolve_ref(self, config: GitHubBackupConfig, ref: str) -> tuple[str | None, str, dict | None]: """Turn ``HEAD`` into a concrete commit SHA. Done once up front so a preview and the restore that follows it act on the same commit even if a scheduled backup lands in between. + + The third element is the commit entry, when resolving already fetched + one. ``preview`` displays it, and taking it from here means the ``HEAD`` + case — by far the common one — costs one ``list_commits`` call rather + than two. """ if ref and ref.upper() != "HEAD": - return ref, "" + return ref, "", None result = await self.list_commits(config, limit=1) if not result.get("success"): - return None, result.get("message") or "Could not read the backup repository" + return None, result.get("message") or "Could not read the backup repository", None commits = result.get("commits") or [] if not commits: - return None, f"Branch '{config.branch}' has no commits to restore from" - return commits[0]["sha"], "" + return None, f"Branch '{config.branch}' has no commits to restore from", None + return commits[0]["sha"], "", commits[0] + + async def _describe_commit(self, config: GitHubBackupConfig, resolved: str) -> dict | None: + """Find the display metadata for one commit SHA. + + Two things used to leave ``commit: null`` in a preview, and the second is + the one that bit in practice: + + * the commit is older than the 20 the picker lists, so it is not in the + scan at all — that is what ``get_commit`` is for; + * ``REF_PATTERN`` accepts a 7-character ref while providers return the + full 40, so an exact ``==`` never matched an abbreviated SHA *even when + the commit was in the window*. Hence the prefix comparison. + + Best-effort throughout: this is a subject line and a date, so a failure + returns None and the preview renders without them rather than failing. + """ + commits = (await self.list_commits(config, limit=20)).get("commits") or [] + for entry in commits: + sha = entry.get("sha") or "" + if sha == resolved or sha.startswith(resolved) or resolved.startswith(sha): + return entry + + backend = get_provider_backend(config.provider) + client = await self._get_client() + result = await backend.get_commit( + repo_url=config.repository_url, token=config.access_token, ref=resolved, client=client + ) + return result.get("commit") if result.get("success") else None def _category_paths(self, category: RestoreCategory, available: list[str]) -> list[str]: """Return the paths in ``available`` that belong to ``category``.""" @@ -429,7 +462,7 @@ class GitHubRestoreService: Takes a session because the settings count depends on local state — see ``_plan_settings``. ``ref`` stays keyword-friendly for callers. """ - resolved, error = await self._resolve_ref(config, ref) + resolved, error, commit_info = await self._resolve_ref(config, ref) if resolved is None: return {"success": False, "message": error, "ref": ref, "categories": []} @@ -449,7 +482,14 @@ class GitHubRestoreService: wanted.extend(self._category_paths(category, available)) fetched = await backend.fetch_files( - repo_url=config.repository_url, token=config.access_token, ref=resolved, paths=wanted, client=client + repo_url=config.repository_url, + token=config.access_token, + ref=resolved, + paths=wanted, + client=client, + # The listing above already built this map; without it the GitHub + # family would GET the same recursive tree a second time. + blob_shas=tree.get("blob_shas") or None, ) if not fetched.get("success"): return { @@ -485,12 +525,8 @@ class GitHubRestoreService: count, detail = await self._count_items(db, category, parsed) categories.append(self._category_entry(category, True, count, detail)) - commit_info = None - commits = (await self.list_commits(config, limit=20)).get("commits") or [] - for entry in commits: - if entry["sha"] == resolved: - commit_info = entry - break + if commit_info is None: + commit_info = await self._describe_commit(config, resolved) return { "success": True, @@ -622,7 +658,7 @@ class GitHubRestoreService: return {"success": False, "message": "Configuration not found", "results": {}} self._progress = "Resolving commit..." - resolved, error = await self._resolve_ref(config, ref) + resolved, error, _ = await self._resolve_ref(config, ref) if resolved is None: return {"success": False, "message": error, "results": {}} @@ -706,7 +742,12 @@ class GitHubRestoreService: self._progress = "Downloading backup files..." fetched = await backend.fetch_files( - repo_url=config.repository_url, token=config.access_token, ref=ref, paths=wanted, client=client + repo_url=config.repository_url, + token=config.access_token, + ref=ref, + paths=wanted, + client=client, + blob_shas=tree.get("blob_shas") or None, ) if not fetched.get("success"): return {}, fetched.get("message") or "Could not read the commit contents" diff --git a/backend/tests/unit/test_git_providers_restore.py b/backend/tests/unit/test_git_providers_restore.py index 109a1a4b6..67ab773cc 100644 --- a/backend/tests/unit/test_git_providers_restore.py +++ b/backend/tests/unit/test_git_providers_restore.py @@ -115,6 +115,67 @@ class TestGitHubListCommits: assert "Unexpected shape" in result["message"] +class TestGetCommit: + """A ref older than the list window still needs a subject line and a date.""" + + @pytest.mark.asyncio + async def test_github_reads_one_commit_by_sha(self): + client = AsyncMock() + client.get = AsyncMock(return_value=_make_mock_response(200, _github_commit("abc1234567"))) + + result = await GitHubBackend().get_commit("https://github.com/owner/repo", "tok", "abc1234567", client) + + assert result["success"] is True + assert result["commit"] == { + "sha": "abc1234567", + "message": "Bambuddy backup", + "author": "Bambuddy", + "date": "2026-07-01T10:00:00Z", + } + assert "repos/owner/repo/commits/abc1234567" in client.get.await_args.args[0] + + @pytest.mark.asyncio + async def test_github_404_names_the_ref(self): + client = AsyncMock() + client.get = AsyncMock(return_value=_make_mock_response(404, {})) + + result = await GitHubBackend().get_commit("https://github.com/owner/repo", "tok", "deadbee", client) + + assert result["success"] is False + assert result["commit"] is None + assert "deadbee" in result["message"] + + @pytest.mark.asyncio + async def test_gitlab_reads_its_flattened_shape(self): + client = AsyncMock() + client.get = AsyncMock( + return_value=_make_mock_response( + 200, + { + "id": "abc1234567", + "message": "Bambuddy backup", + "author_name": "Bambuddy", + "committed_date": "2026-07-02T10:00:00Z", + }, + ) + ) + + result = await GitLabBackend().get_commit("https://gitlab.com/owner/repo", "tok", "abc1234567", client) + + assert result["commit"]["author"] == "Bambuddy" + assert result["commit"]["date"] == "2026-07-02T10:00:00Z" + + @pytest.mark.asyncio + async def test_gitlab_404_names_the_ref(self): + client = AsyncMock() + client.get = AsyncMock(return_value=_make_mock_response(404, {})) + + result = await GitLabBackend().get_commit("https://gitlab.com/owner/repo", "tok", "deadbee", client) + + assert result["success"] is False + assert "deadbee" in result["message"] + + class TestGitHubListTree: def setup_method(self): self.backend = GitHubBackend() @@ -221,6 +282,40 @@ class TestGitHubFetchFiles: assert result["files"] == {"a.json": "1", "b.json": "2"} assert client.get.await_count == 3 + @pytest.mark.asyncio + async def test_a_supplied_blob_map_skips_the_second_tree_read(self): + """list_tree already fetched this; fetching it again was a wasted GET.""" + client = AsyncMock() + client.get = AsyncMock(return_value=_make_mock_response(200, {"content": _b64("1"), "encoding": "base64"})) + + result = await self.backend.fetch_files( + self.repo_url, self.token, "abc1234", ["a.json"], client, blob_shas={"a.json": "sha-a"} + ) + + assert result["files"] == {"a.json": "1"} + # The blob read and nothing else. + assert client.get.await_count == 1 + assert "git/blobs/sha-a" in client.get.await_args.args[0] + + @pytest.mark.asyncio + async def test_list_tree_hands_back_the_map_it_built(self): + client = AsyncMock() + client.get = AsyncMock( + return_value=_make_mock_response( + 200, + { + "tree": [ + {"type": "blob", "path": "a.json", "sha": "sha-a"}, + {"type": "tree", "path": "dir", "sha": "sha-d"}, + ] + }, + ) + ) + + result = await self.backend.list_tree(self.repo_url, self.token, "abc1234", client) + + assert result["blob_shas"] == {"a.json": "sha-a"} + @pytest.mark.asyncio async def test_missing_path_is_skipped_not_an_error(self): """Which categories a backup contains varies by config, so an absent @@ -273,13 +368,85 @@ class TestGitHubFetchFiles: class TestGiteaAndForgejoInheritReads: - """Gitea overrides the *write* path only; reads come from GitHubBackend.""" + """Gitea overrides the *write* path, plus the one read that genuinely differs.""" @pytest.mark.parametrize("backend_cls", [GiteaBackend, ForgejoBackend]) def test_read_methods_are_not_overridden(self, backend_cls): - for method in ("list_commits", "list_tree", "fetch_files"): + for method in ("list_commits", "list_tree", "fetch_files", "get_commit"): assert getattr(backend_cls, method) is getattr(GitHubBackend, method) + @pytest.mark.parametrize("backend_cls", [GiteaBackend, ForgejoBackend]) + def test_the_tree_read_is_paged_rather_than_inherited(self, backend_cls): + """GitHub's trees endpoint is not paginated; Gitea's is (#2656).""" + assert backend_cls._blob_shas_at is not GitHubBackend._blob_shas_at + + @pytest.mark.asyncio + @pytest.mark.parametrize("backend_cls", [GiteaBackend, ForgejoBackend]) + async def test_a_paged_tree_is_read_to_the_end(self, backend_cls): + """Inheriting GitHub's single GET read only the first page. + + The rest of the backup then looked absent from the commit, and the + preview reported those categories as "not present" — a restore silently + skipping data, which is exactly what GitHub's truncated=true check + exists to prevent. + """ + page1 = { + "tree": [{"type": "blob", "path": f"f{i}.json", "sha": f"s{i}"} for i in range(1000)], + "total_count": 1002, + } + page2 = { + "tree": [ + {"type": "blob", "path": "settings/app_settings.json", "sha": "sx"}, + {"type": "tree", "path": "settings", "sha": "dx"}, + ], + "total_count": 1002, + } + client = AsyncMock() + client.get = AsyncMock(side_effect=[_make_mock_response(200, page1), _make_mock_response(200, page2)]) + + 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 == 2 + assert "settings/app_settings.json" in result["paths"] + assert len(result["paths"]) == 1001 + + @pytest.mark.asyncio + async def test_a_single_page_tree_costs_one_request(self): + client = AsyncMock() + client.get = AsyncMock( + return_value=_make_mock_response( + 200, {"tree": [{"type": "blob", "path": "a.json", "sha": "s1"}], "total_count": 1} + ) + ) + + result = await GiteaBackend().list_tree("https://git.example.com/owner/repo", "tok", "abc1234", client) + + assert result["paths"] == ["a.json"] + assert client.get.await_count == 1 + + @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)]} + page["total_count"] = 10_000_000 + client = AsyncMock() + client.get = AsyncMock(return_value=_make_mock_response(200, page)) + + result = await GiteaBackend().list_tree("https://git.example.com/owner/repo", "tok", "abc1234", client) + + assert result["success"] is False + assert "listing limit" in result["message"] + + @pytest.mark.asyncio + async def test_a_missing_ref_is_still_named(self): + client = AsyncMock() + client.get = AsyncMock(return_value=_make_mock_response(404, {})) + + result = await GiteaBackend().list_tree("https://git.example.com/owner/repo", "tok", "deadbee", client) + + assert result["success"] is False + assert "deadbee" in result["message"] + @pytest.mark.asyncio async def test_gitea_list_commits_uses_its_own_api_base(self): backend = GiteaBackend() @@ -299,6 +466,7 @@ class TestGiteaAndForgejoInheritReads: client = AsyncMock() client.get = AsyncMock(return_value=_make_mock_response(200, {"tree": []})) + client.get = AsyncMock(return_value=_make_mock_response(200, {"tree": [], "total_count": 0})) await backend.list_tree("https://example.com/git/owner/repo", "tok", "abc1234", client) url = client.get.await_args.args[0] @@ -398,6 +566,47 @@ class TestGitLabReads: assert len(result["paths"]) == 101 assert "last.json" in result["paths"] + @pytest.mark.asyncio + async def test_hitting_the_page_cap_is_a_failure_not_a_partial_list(self): + """The mirror image of GitHub's truncated=true check. + + Falling out of the `while page <= 50` condition used to return + success: True with a silently partial path list, which the restore then + reported as "those categories are not present in this commit" — data + skipped without anyone being told. + """ + full_page = [{"type": "blob", "path": f"f{i}.json"} for i in range(100)] + client = AsyncMock() + client.get = AsyncMock(return_value=_make_mock_response(200, full_page)) + + result = await self.backend.list_tree(self.repo_url, self.token, "abc1234", client) + + assert result["success"] is False + assert result["paths"] == [] + assert "cannot be enumerated reliably" in result["message"] + + @pytest.mark.asyncio + async def test_list_tree_returns_no_blob_map(self): + """GitLab reads files by path, so there is nothing to share.""" + client = AsyncMock() + client.get = AsyncMock(return_value=_make_mock_response(200, [{"type": "blob", "path": "a.json"}])) + + result = await self.backend.list_tree(self.repo_url, self.token, "abc1234", client) + + assert result["blob_shas"] == {} + + @pytest.mark.asyncio + async def test_fetch_files_ignores_a_blob_map(self): + client = AsyncMock() + client.get = AsyncMock(return_value=_make_mock_response(200, {"content": _b64("1"), "encoding": "base64"})) + + result = await self.backend.fetch_files( + self.repo_url, self.token, "abc1234", ["a.json"], client, blob_shas={"a.json": "irrelevant"} + ) + + assert result["files"] == {"a.json": "1"} + assert "repository/files/a.json" in client.get.await_args.args[0] + @pytest.mark.asyncio async def test_fetch_files_decodes_base64(self): client = AsyncMock() diff --git a/backend/tests/unit/test_github_restore.py b/backend/tests/unit/test_github_restore.py index 81bc342f2..020484061 100644 --- a/backend/tests/unit/test_github_restore.py +++ b/backend/tests/unit/test_github_restore.py @@ -1837,10 +1837,12 @@ class TestResolveRef: service.list_commits = AsyncMock() config = MagicMock(branch="main") - resolved, error = await service._resolve_ref(config, "abc1234") + resolved, error, commit = await service._resolve_ref(config, "abc1234") assert resolved == "abc1234" assert error == "" + # Nothing was fetched, so there is no entry to describe it with. + assert commit is None service.list_commits.assert_not_awaited() @pytest.mark.asyncio @@ -1851,10 +1853,13 @@ class TestResolveRef: ) config = MagicMock(branch="main") - resolved, error = await service._resolve_ref(config, "HEAD") + resolved, error, commit = await service._resolve_ref(config, "HEAD") assert resolved == "tipsha1" assert error == "" + # Handed back so preview does not list commits a second time just to + # describe the one it already fetched. + assert commit == {"sha": "tipsha1"} @pytest.mark.asyncio async def test_empty_history_is_an_error(self): @@ -1862,7 +1867,80 @@ class TestResolveRef: service.list_commits = AsyncMock(return_value={"success": True, "commits": []}) config = MagicMock(branch="main") - resolved, error = await service._resolve_ref(config, "HEAD") + resolved, error, commit = await service._resolve_ref(config, "HEAD") assert resolved is None assert "no commits" in error + assert commit is None + + +class TestDescribeCommit: + """A preview that says `commit: null` gives the user no idea what they picked.""" + + def _config(self): + return MagicMock(branch="main", provider="github", repository_url="https://github.com/o/r", access_token="t") + + def _entry(self, sha: str): + return {"sha": sha, "message": "Bambuddy backup", "author": "Bambuddy", "date": "2026-07-01T10:00:00Z"} + + @pytest.mark.asyncio + async def test_an_abbreviated_ref_matches_a_full_sha_in_the_window(self): + """REF_PATTERN accepts 7 characters; providers return 40. + + The old exact `==` therefore never matched an abbreviated ref, even when + the commit was right there in the top 20. + """ + service = _service() + full = "abc1234" + "0" * 33 + service.list_commits = AsyncMock(return_value={"success": True, "commits": [self._entry(full)]}) + + found = await service._describe_commit(self._config(), "abc1234") + + assert found is not None + assert found["sha"] == full + + @pytest.mark.asyncio + async def test_a_full_sha_matches_an_abbreviated_entry(self): + service = _service() + service.list_commits = AsyncMock(return_value={"success": True, "commits": [self._entry("abc1234")]}) + + found = await service._describe_commit(self._config(), "abc1234" + "0" * 33) + + assert found is not None + + @pytest.mark.asyncio + async def test_a_commit_outside_the_window_is_fetched_directly(self): + service = _service() + service.list_commits = AsyncMock(return_value={"success": True, "commits": [self._entry("f" * 40)]}) + backend = MagicMock() + backend.get_commit = AsyncMock(return_value={"success": True, "commit": self._entry("old" + "0" * 37)}) + + with patch("backend.app.services.github_restore.get_provider_backend", return_value=backend): + found = await service._describe_commit(self._config(), "old" + "0" * 37) + + assert found["sha"] == "old" + "0" * 37 + backend.get_commit.assert_awaited_once() + + @pytest.mark.asyncio + async def test_a_direct_lookup_failure_is_not_fatal(self): + """It is a subject line: render the preview without it.""" + service = _service() + service.list_commits = AsyncMock(return_value={"success": True, "commits": []}) + backend = MagicMock() + backend.get_commit = AsyncMock(return_value={"success": False, "message": "boom", "commit": None}) + + with patch("backend.app.services.github_restore.get_provider_backend", return_value=backend): + assert await service._describe_commit(self._config(), "a" * 40) is None + + @pytest.mark.asyncio + async def test_the_window_scan_is_not_run_twice(self): + """_resolve_ref already listed commits for HEAD; preview reuses that.""" + service = _service() + tip = self._entry("t" * 40) + service.list_commits = AsyncMock(return_value={"success": True, "commits": [tip]}) + + resolved, _, commit = await service._resolve_ref(self._config(), "HEAD") + + assert resolved == "t" * 40 + assert commit == tip + assert service.list_commits.await_count == 1