Merge pull request 'nisse-review: match repository patterns against Gitea's search' (#154) from review-patterns-no-org-scope into main

docs/Architecture.md

@@ -882,9 +882,9 @@ else) towards better changes.
   a draft, and not marked work in progress. Any author's: the operator's,
   `nisse-dev`'s, or another tool's. Its repositories are the ones its Gitea
   account was given, or, when configured, those matching patterns (`dev/*`,
-  `*/*`) among all it can read, public ones included: Gitea leaves a public
+  `*/*`) among all it can read, public ones included, so a public repository
-  repository's Read collaborators out of an account's own list, so access to
+  needs no grant in Gitea: access to public repositories is managed in
-  public repositories is managed in configuration, and to private ones in Gitea.
+  configuration, and to private ones in Gitea.
 - **A work item per PR,** scoped to its repository (L1). It closes when the PR
   merges or closes.
 - **What starts a run:** the PR becoming ready, a new push, or a new comment or

docs/Development-Plan.md

@@ -608,9 +608,9 @@ waits until two exist to compare. Moved, unchanged in behaviour:
   ready unless a draft or titled WIP; closed when the PR merges or closes.
   Every open PR is reviewed, those open when it starts included (operator).
   Since #153, `repositories` holds patterns (`dev/*`, `*/*`) matched against
-  every repository it can read, public ones included (Gitea's own list for the
+  every repository it can read, public ones included, through Gitea's search
-  account leaves out a public repository's Read collaborators, found at
+  (`/repos/search`, which needs no `read:organization` scope); the read-only wall
-  bring-up); the read-only wall is checked every round, not only at startup.
+  is checked every round, not only at startup.
 - **Reading (R3):** the repository at the PR's head and base, from Gitea's
   archives, unpacked into a scratch folder and read in Python (`read_file`,
   `list_files`, `search_code`), and its issues (`read_issue`,

nisse-core/src/nisse_core/gitea.py

@@ -421,14 +421,6 @@ class GiteaClient:
                 return found
         raise GiteaError("too many repositories to list")
 
-    def owner_repositories(self, owner: str) -> list[str]:
-        """Every repository of an organization (or a user) this account can
-        read, as "owner/name"."""
-        try:
-            return self._paged_names(f"/orgs/{owner}/repos")
-        except GiteaNotFound:
-            return self._paged_names(f"/users/{owner}/repos")
-
     def visible_repositories(self) -> list[str]:
         """Every repository this account can read, public ones included."""
         return self._paged_names("/repos/search", key="data")

nisse-core/tests/test_gitea.py

@@ -361,19 +361,6 @@ def test_readable_repositories_page_to_the_end() -> None:
     assert len(found) == PR_PAGE_SIZE + 2 and found[-1] == "o/r2-1"
 
 
-def test_owner_repositories_try_the_organization_then_the_user() -> None:
-    asked: list[str] = []
-
-    def handler(request: httpx.Request) -> httpx.Response:
-        asked.append(request.url.path)
-        if request.url.path.startswith("/api/v1/orgs/"):
-            return httpx.Response(404, json={"message": "not found"})
-        return httpx.Response(200, json=[{"full_name": "don/r"}])
-
-    assert client(handler).owner_repositories("don") == ["don/r"]
-    assert asked == ["/api/v1/orgs/don/repos", "/api/v1/users/don/repos"]
-
-
 def test_visible_repositories_read_the_search_and_a_404_is_not_found() -> None:
     def handler(request: httpx.Request) -> httpx.Response:
         if request.url.path.endswith("/repos/search"):
@@ -388,6 +375,23 @@ def test_visible_repositories_read_the_search_and_a_404_is_not_found() -> None:
         gitea.repository("d/gone")
 
 
+def test_visible_repositories_page_through_the_search() -> None:
+    # The patterns rest on the search listing everything (#154's review).
+    asked: list[dict[str, str]] = []
+
+    def handler(request: httpx.Request) -> httpx.Response:
+        asked.append(dict(request.url.params))
+        page = int(request.url.params["page"])
+        count = PR_PAGE_SIZE if page == 1 else 3
+        data = [{"full_name": f"o/r{page}-{n}"} for n in range(count)]
+        return httpx.Response(200, json={"ok": True, "data": data})
+
+    found = client(handler).visible_repositories()
+    assert len(found) == PR_PAGE_SIZE + 3 and found[-1] == "o/r2-2"
+    assert [a["page"] for a in asked] == ["1", "2"]
+    assert all(a["limit"] == str(PR_PAGE_SIZE) for a in asked)
+
+
 def test_a_pull_request_carries_what_a_review_needs() -> None:
     def handler(request: httpx.Request) -> httpx.Response:
         if request.url.path.endswith(".diff"):

nisse-review/src/nisse_review/access.py

@@ -3,7 +3,10 @@
 The repositories are worked out again every round: ``repositories`` in the
 configuration is a list of patterns ("dev/nisse", "dev/*", "*/*"), matched
 against what the account can read, public repositories included; unset, it
-is every repository the account was given (``/user/repos``). Access is
+is every repository the account was given (``/user/repos``). Patterns are
+matched against Gitea's search, which needs only the token's repository
+scope: listing an organization's repositories would need
+``read:organization`` too. Access is
 managed in Gitea for private repositories and in the configuration for
 public ones.
 
@@ -35,22 +38,15 @@ def reviewed(
     """The repositories to review, sorted."""
     if patterns is None:
         return sorted(gitea.readable_repositories())
-    found: set[str] = set()
+    found = {p for p in patterns if not _glob(p)}  # ``check`` sees they're readable
-    for pattern in patterns:
+    globs = [p for p in patterns if _glob(p)]
-        owner, _, name = pattern.partition("/")
+    if globs:
-        if not _glob(owner) and not _glob(name):
+        visible = gitea.visible_repositories()
-            found.add(pattern)  # checked by ``check``: it must be readable
+        for pattern in globs:
-            continue
+            matched = {repo for repo in visible if fnmatchcase(repo, pattern)}
-        try:
+            if not matched:
-            candidates = (
+                oplog.emit("access.no_match", cause="matches nothing", pattern=pattern)
-                gitea.visible_repositories()
+            found |= matched
-                if _glob(owner)
-                else gitea.owner_repositories(owner)
-            )
-        except GiteaNotFound:
-            oplog.emit("access.no_owner", cause="no such owner", pattern=pattern)
-            continue
-        found |= {repo for repo in candidates if fnmatchcase(repo, pattern)}
     return sorted(found)
 
 

nisse-review/src/nisse_review/main.py

@@ -9,6 +9,7 @@ from types import FrameType
 
 from nisse_core.config import ConfigError
 from nisse_core.envfile import load_env_file
+from nisse_core.gitea import GiteaError
 from nisse_review.access import AccessRefused
 from nisse_review.build import StartupError, build
 from nisse_review.config import ReviewConfig
@@ -19,7 +20,7 @@ def main() -> int:
     try:
         config = ReviewConfig.load()
         built = build(config)
-    except (ConfigError, StartupError) as exc:
+    except (ConfigError, StartupError, GiteaError) as exc:
         print(f"nisse-review: {exc}", file=sys.stderr)
         return 2
 

nisse-review/src/nisse_review/scheduler.py

@@ -106,8 +106,16 @@ class Scheduler:
             repos = reviewed(self._gitea, self._patterns, self._oplog)
         open_items = {i.id: i for i in self._store.work_items("open")}
         for repo in repos:
+            try:
+                prs = self._gitea.open_pull_requests(repo)
+            except GiteaError as exc:
+                # One repository Gitea won't list (an empty one, one without
+                # pull requests) holds up none of the others; its items stay
+                # as they are.
+                self._oplog.emit("intake.repo_failed", cause=str(exc)[:300], repo=repo)
+                continue
             seen: set[str] = set()
-            for pr in self._gitea.open_pull_requests(repo):
+            for pr in prs:
                 key = item_id(repo, pr.number)
                 seen.add(key)
                 self._see(open_items.get(key), repo, pr)

nisse-review/tests/review_fakes.py

@@ -70,9 +70,9 @@ class FakeGitea:
     comments: list[tuple[str, int, str]] = field(default_factory=list)
     refuse_off_diff: bool = False
     down: bool = False
-    # Readable but not given (public): missing from /user/repos, as Gitea
+    # Readable but not given (public, no grant): missing from /user/repos.
-    # leaves a public repository's Read collaborators out of it.
     public: set[str] = field(default_factory=set)
+    unlisted: set[str] = field(default_factory=set)  # its PRs can't be listed
 
     def _up(self) -> None:
         if self.down:
@@ -89,13 +89,6 @@ class FakeGitea:
         self._up()
         return list(self.repos)
 
-    def owner_repositories(self, owner: str) -> list[str]:
-        self._up()
-        found = [r for r in self.repos if r.startswith(f"{owner}/")]
-        if not found:
-            raise GiteaNotFound(f"no owner {owner}")
-        return found
-
     def repository(self, repo: str) -> dict[str, Any]:
         if repo not in self.repos:
             raise GiteaNotFound(f"no repository {repo}")
@@ -107,6 +100,8 @@ class FakeGitea:
 
     def open_pull_requests(self, repo: str) -> list[PullRequest]:
         self._up()
+        if repo in self.unlisted:
+            raise GiteaNotFound(f"gitea GET /repos/{repo}/pulls returned 404")
         return list(self.repos[repo])
 
     def pull_request(self, repo: str, index: int) -> PullRequest:

nisse-review/tests/test_review.py

@@ -311,9 +311,8 @@ def test_an_account_that_can_write_is_refused(tmp_path: Path) -> None:
 def test_patterns_reach_public_repositories_the_account_was_not_given(
     tmp_path: Path,
 ) -> None:
-    # Gitea leaves a public repository's Read collaborators out of
+    # A public repository the account wasn't given isn't in /user/repos: a
-    # /user/repos: a pattern finds it all the same (option a: the
+    # pattern finds it all the same (option a: the configuration decides).
-    # configuration decides).
     gitea = FakeGitea(
         repos={"dev/nisse": [pr(7)], "dev/wordle": [], "infra/tofu": [pr(3)]},
         public={"dev/nisse", "dev/wordle", "infra/tofu"},
@@ -329,6 +328,20 @@ def test_patterns_reach_public_repositories_the_account_was_not_given(
     assert w["store"].work_item("infra/tofu#3") is None
 
 
+def test_a_repository_whose_prs_cannot_be_listed_holds_up_no_other(
+    tmp_path: Path,
+) -> None:
+    # Found at bring-up: an empty private repository's /pulls was a 404,
+    # and the round stopped there, so no repository was reviewed.
+    gitea = FakeGitea(repos={"a/empty": [], REPO: [pr()]}, unlisted={"a/empty"})
+    out = io.StringIO()
+    w = world(tmp_path, Scripted(*REVIEW), gitea)
+    w["scheduler"]._oplog = OpLog("t", out)
+    assert w["scheduler"].round() is True  # REPO's PR was reviewed
+    assert item(w).position == REVIEWED
+    assert '"intake.repo_failed"' in out.getvalue() and "a/empty" in out.getvalue()
+
+
 def test_a_configured_repository_it_cannot_read_is_left_out() -> None:
     gitea = FakeGitea(repos={REPO: []})
     out = io.StringIO()
@@ -336,7 +349,7 @@ def test_a_configured_repository_it_cannot_read_is_left_out() -> None:
     patterns = {REPO, "o/gone", "nobody/*"}
     assert check(gitea, reviewed(gitea, patterns, log), log) == [REPO]  # type: ignore[arg-type]
     assert '"access.unreadable"' in out.getvalue() and "o/gone" in out.getvalue()
-    assert '"access.no_owner"' in out.getvalue()
+    assert '"access.no_match"' in out.getvalue() and "nobody/*" in out.getvalue()
 
 
 def test_write_access_granted_while_running_stops_it(tmp_path: Path) -> None: