feat: Add Gitea repository operations endpoints#69
feat: Add Gitea repository operations endpoints#69jaysomani wants to merge 3 commits intoutopia-php:mainfrom
Conversation
- Implement searchRepositories with client-side owner filtering - Implement listBranches to list all branches in repository - Implement getOwnerName (returns installationId for Gitea) - Add comprehensive tests for all three methods - Tests include try/finally cleanup and timing delays for branch creation
WalkthroughThis pull request implements three public methods in the Gitea VCS adapter: Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e33e09576
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/VCS/Adapter/Git/Gitea.php
Outdated
| return [ | ||
| 'items' => $filteredRepos, | ||
| 'total' => count($filteredRepos), |
There was a problem hiding this comment.
This computes items/total from only the current /repos/search?page=...&limit=... slice, then applies owner filtering locally, so results become incorrect whenever other owners' repos occupy that page. In that case, a page can be empty even though matching repos exist on later pages, and total undercounts to at most the current page size instead of all matching repos for the owner.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/VCS/Adapter/GiteaTest.php (1)
563-576: Prefer polling with a timeout over fixed sleeps here.These two
sleep(1)calls add a guaranteed 2s to the test and still leave it flaky if branch visibility takes longer on CI. A short retry loop aroundlistBranches()with a deadline will make the test both faster and more reliable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/VCS/Adapter/GiteaTest.php` around lines 563 - 576, Replace the two fixed sleep(1) calls after calling createBranch with a short polling loop that repeatedly calls listBranches(self::$owner, $repositoryName) until the expected branches ('feature-1' and 'feature-2' plus 'main') appear or a deadline/timeout (e.g. a few seconds) is reached; implement this in the test method surrounding the calls to $this->vcsAdapter->createBranch and $this->vcsAdapter->listBranches so it asserts success when the branches list contains the expected names and fails with a clear message if the timeout elapses.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/VCS/Adapter/Git/Gitea.php`:
- Around line 405-414: getOwnerName currently just echoes the provided
installationId instead of resolving the repository owner; update the Gitea
adapter (class Gitea) to actually return a stored owner or resolve it via the
Gitea API: add a private property (e.g. $owner) and populate it during adapter
initialization (constructor or init method) from config or by calling the Gitea
API using the existing HTTP client, and then have getOwnerName(string
$installationId): string return $this->owner (or the owner resolved from the
API) rather than the raw installationId; ensure the resolution path handles
missing owner by throwing or returning a clear default and reference the
getOwnerName and constructor/init method names when making the change.
- Around line 438-447: The code currently iterates $responseBody from the
$this->call(self::METHOD_GET, $url, ...) without validating the HTTP status or
ensuring $responseBody is an array of branch items; update the logic around the
$response/$responseBody (from the call to METHOD_GET) to first check the
response status (e.g. $response['status'] or equivalent) and that $responseBody
is an array of arrays (use is_array and optionally check each item has a 'name'
key) before the foreach; on non-200 responses or unexpected shapes, return an
appropriate error/empty result or throw the adapter error so the runtime type
error is avoided (adjust callers if necessary) — change references in this block
($response, $responseBody, the foreach that reads $branch['name']) accordingly.
- Around line 132-160: The current implementation paginates the remote search
request via $queryParams ('page' => $page, 'limit' => $per_page) and then
filters that single page by owner, causing incomplete results and an incorrect
total; change the logic in the method that builds $query/$url and calls
$this->call(...) so you fetch the full candidate set (remove or increase remote
pagination to retrieve all pages or iterate until no more results), collect
$responseBody['data'] into a single $allRepos array, apply the owner filter to
produce $filteredRepos = array_values(array_filter($allRepos, ...)), compute
$total = count($filteredRepos), then paginate the filtered result by slicing
with offset = ($page - 1) * $per_page (e.g., array_slice) and return 'items' =>
$pagedRepos and 'total' => $total; keep existing symbols $queryParams,
$responseBody, $allRepos, $filteredRepos, $page, and $per_page when implementing
this change.
---
Nitpick comments:
In `@tests/VCS/Adapter/GiteaTest.php`:
- Around line 563-576: Replace the two fixed sleep(1) calls after calling
createBranch with a short polling loop that repeatedly calls
listBranches(self::$owner, $repositoryName) until the expected branches
('feature-1' and 'feature-2' plus 'main') appear or a deadline/timeout (e.g. a
few seconds) is reached; implement this in the test method surrounding the calls
to $this->vcsAdapter->createBranch and $this->vcsAdapter->listBranches so it
asserts success when the branches list contains the expected names and fails
with a clear message if the timeout elapses.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f032ea6f-d670-4e04-8bc6-b4f7b6aa0634
📒 Files selected for processing (2)
src/VCS/Adapter/Git/Gitea.phptests/VCS/Adapter/GiteaTest.php
| /** | ||
| * Get owner name | ||
| * @param string $installationId In Gitea context, this is the owner name itself | ||
| * @return string Owner name | ||
| */ | ||
| public function getOwnerName(string $installationId): string | ||
| { | ||
| throw new Exception("Not implemented yet"); | ||
| // Gitea doesn't have GitHub App installation concept | ||
| // Return the installationId as-is since it represents the owner | ||
| return $installationId; |
There was a problem hiding this comment.
getOwnerName() does not currently resolve an owner name.
This just returns the input string unchanged. The shared adapter contract expects this method to derive an owner name from an installation identifier, but this adapter never stores owner identity during initialization, so callers that do not already know the owner get no useful result. The new test passes only because it feeds the owner value back in as installationId.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/VCS/Adapter/Git/Gitea.php` around lines 405 - 414, getOwnerName currently
just echoes the provided installationId instead of resolving the repository
owner; update the Gitea adapter (class Gitea) to actually return a stored owner
or resolve it via the Gitea API: add a private property (e.g. $owner) and
populate it during adapter initialization (constructor or init method) from
config or by calling the Gitea API using the existing HTTP client, and then have
getOwnerName(string $installationId): string return $this->owner (or the owner
resolved from the API) rather than the raw installationId; ensure the resolution
path handles missing owner by throwing or returning a clear default and
reference the getOwnerName and constructor/init method names when making the
change.
- searchRepositories: Fetch all pages before filtering to get accurate totals - listBranches: Add HTTP status checks and array validation
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/VCS/Adapter/Git/Gitea.php`:
- Around line 148-155: The code in Gitea.php currently treats non-OK or
malformed /repos/search responses as empty by assigning $repos =
$responseBody['data'] ?? [] and breaking out, hiding failures; update the logic
that follows the call(self::METHOD_GET, $url, ['Authorization' => "token
$this->accessToken"]) to validate the HTTP response and payload like the GitHub
adapter does: if the response status is not a 2xx or the $response['body'] is
missing or does not contain the expected 'data' key, throw an exception (with a
clear message including status/body) instead of silently setting $repos to [];
ensure the check references the same symbols ($response, $responseBody, $repos)
so callers receive an error on auth/outage/malformed payloads rather than total
= 0.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2d3f2ebe-1fbe-49d3-860e-1a04f4c48f92
📒 Files selected for processing (1)
src/VCS/Adapter/Git/Gitea.php
- Check HTTP status code in pagination loop - Throw exception if data key is missing - Prevent silent failures from being treated as empty results
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/VCS/Adapter/Git/Gitea.php (1)
436-446: Design decision is valid but warrants explicit adapter documentation.The implementation correctly adapts to Gitea's OAuth model where there's no "installation" concept. Returning
$installationIddirectly makes sense since callers must pass the owner name as this parameter.However, this semantic shift (the parameter represents different things per adapter) could confuse API consumers. Consider adding a note in the class-level docblock or a higher-level adapter documentation explaining that Gitea callers should pass the owner name as
installationIdfor methods likesearchRepositories()andgetOwnerName().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/VCS/Adapter/Git/Gitea.php` around lines 436 - 446, Add explicit adapter documentation in the Gitea adapter explaining the semantic overload of the $installationId parameter: update the class-level docblock in class Gitea (file Gitea.php) to state that Gitea has no installation concept and callers must supply the owner name as the $installationId for methods like getOwnerName() and searchRepositories(); mention this behavior so consumers know to pass owner names instead of installation IDs and reference the affected methods (getOwnerName, searchRepositories) in the docblock.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/VCS/Adapter/Git/Gitea.php`:
- Around line 436-446: Add explicit adapter documentation in the Gitea adapter
explaining the semantic overload of the $installationId parameter: update the
class-level docblock in class Gitea (file Gitea.php) to state that Gitea has no
installation concept and callers must supply the owner name as the
$installationId for methods like getOwnerName() and searchRepositories();
mention this behavior so consumers know to pass owner names instead of
installation IDs and reference the affected methods (getOwnerName,
searchRepositories) in the docblock.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: da2b5af9-2f00-4bae-967d-625b31fa8c3f
📒 Files selected for processing (1)
src/VCS/Adapter/Git/Gitea.php
1 participant
Summary
Implements repository search, branch listing, and owner name retrieval for Gitea adapter.
Changes
Methods Implemented (3 total)
searchRepositories()- Search repos by owner with client-side filteringlistBranches()- List all branches in a repositorygetOwnerName()- Return owner name (adapted for Gitea's OAuth model)Tests Added (3 total)
testSearchRepositories()- Tests repo search with and without filterstestListBranches()- Tests branch listing with multiple branchestestGetOwnerName()- Tests owner name retrievalImplementation Notes
/repos/searchAPI with client-side owner filtering (Gitea doesn't support server-side owner filter)sleep()delays to handle Gitea's async branch creationSummary by CodeRabbit
New Features
Tests