feat: Add Gitea commit endpoints (getCommit, getLatestCommit)#66
feat: Add Gitea commit endpoints (getCommit, getLatestCommit)#66jaysomani wants to merge 1 commit intoutopia-php:mainfrom
Conversation
WalkthroughTwo new public methods were added to the Gitea VCS adapter class. The Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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`:
- Line 410: The branch value is interpolated directly into the commit URL and
can contain query-significant characters; update the URL construction in
Gitea.php so the branch is URL-encoded (e.g., replace "{$branch}" with
rawurlencode($branch)) or build the query with
http_build_query(['sha'=>$branch,'limit'=>1]) and append that to
"/repos/{$owner}/{$repositoryName}/commits" so $url is safe; modify the
assignment to $url accordingly (referencing the $url variable and $branch).
In `@tests/VCS/Adapter/GiteaTest.php`:
- Around line 420-438: The two tests testGetCommitWithInvalidSha and
testGetLatestCommitWithInvalidBranch create repositories via
$this->vcsAdapter->createRepository(...) but never delete them on the exception
path; wrap the assertions that call $this->vcsAdapter->getCommit(...) and
$this->vcsAdapter->getLatestCommit(...) in try/finally blocks that always call
the cleanup method (e.g. $this->vcsAdapter->deleteRepository(self::$owner,
$repositoryName)) in the finally so the repositoryName created in each test is
removed regardless of the thrown exception.
| { | ||
| throw new Exception("Not implemented yet"); | ||
| } | ||
| $url = "/repos/{$owner}/{$repositoryName}/commits?sha={$branch}&limit=1"; |
There was a problem hiding this comment.
Encode branch when building the query string.
$branch is interpolated directly into the URL, so query-significant characters can change request semantics. Build the query with encoding to avoid malformed/altered requests.
🔧 Proposed fix
- $url = "/repos/{$owner}/{$repositoryName}/commits?sha={$branch}&limit=1";
+ $query = http_build_query([
+ 'sha' => $branch,
+ 'limit' => 1,
+ ]);
+ $url = "/repos/{$owner}/{$repositoryName}/commits?{$query}";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/VCS/Adapter/Git/Gitea.php` at line 410, The branch value is interpolated
directly into the commit URL and can contain query-significant characters;
update the URL construction in Gitea.php so the branch is URL-encoded (e.g.,
replace "{$branch}" with rawurlencode($branch)) or build the query with
http_build_query(['sha'=>$branch,'limit'=>1]) and append that to
"/repos/{$owner}/{$repositoryName}/commits" so $url is safe; modify the
assignment to $url accordingly (referencing the $url variable and $branch).
| public function testGetCommitWithInvalidSha(): void | ||
| { | ||
| $repositoryName = 'test-get-commit-invalid-' . \uniqid(); | ||
| $this->vcsAdapter->createRepository(self::$owner, $repositoryName, false); | ||
| $this->vcsAdapter->createFile(self::$owner, $repositoryName, 'README.md', '# Test'); | ||
|
|
||
| $this->expectException(\Exception::class); | ||
| $this->vcsAdapter->getCommit(self::$owner, $repositoryName, 'invalid-sha-12345'); | ||
| } | ||
|
|
||
| public function testGetLatestCommitWithInvalidBranch(): void | ||
| { | ||
| $repositoryName = 'test-get-latest-commit-invalid-' . \uniqid(); | ||
| $this->vcsAdapter->createRepository(self::$owner, $repositoryName, false); | ||
| $this->vcsAdapter->createFile(self::$owner, $repositoryName, 'README.md', '# Test'); | ||
|
|
||
| $this->expectException(\Exception::class); | ||
| $this->vcsAdapter->getLatestCommit(self::$owner, $repositoryName, 'non-existing-branch'); | ||
| } |
There was a problem hiding this comment.
Ensure cleanup runs in exception-path tests.
Both negative tests create repositories but never delete them after the expected exception. Wrap assertions in try/finally so cleanup always executes.
🧹 Proposed fix
public function testGetCommitWithInvalidSha(): void
{
$repositoryName = 'test-get-commit-invalid-' . \uniqid();
$this->vcsAdapter->createRepository(self::$owner, $repositoryName, false);
$this->vcsAdapter->createFile(self::$owner, $repositoryName, 'README.md', '# Test');
- $this->expectException(\Exception::class);
- $this->vcsAdapter->getCommit(self::$owner, $repositoryName, 'invalid-sha-12345');
+ try {
+ $this->expectException(\Exception::class);
+ $this->vcsAdapter->getCommit(self::$owner, $repositoryName, 'invalid-sha-12345');
+ } finally {
+ $this->vcsAdapter->deleteRepository(self::$owner, $repositoryName);
+ }
}
public function testGetLatestCommitWithInvalidBranch(): void
{
$repositoryName = 'test-get-latest-commit-invalid-' . \uniqid();
$this->vcsAdapter->createRepository(self::$owner, $repositoryName, false);
$this->vcsAdapter->createFile(self::$owner, $repositoryName, 'README.md', '# Test');
- $this->expectException(\Exception::class);
- $this->vcsAdapter->getLatestCommit(self::$owner, $repositoryName, 'non-existing-branch');
+ try {
+ $this->expectException(\Exception::class);
+ $this->vcsAdapter->getLatestCommit(self::$owner, $repositoryName, 'non-existing-branch');
+ } finally {
+ $this->vcsAdapter->deleteRepository(self::$owner, $repositoryName);
+ }
}🤖 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 420 - 438, The two tests
testGetCommitWithInvalidSha and testGetLatestCommitWithInvalidBranch create
repositories via $this->vcsAdapter->createRepository(...) but never delete them
on the exception path; wrap the assertions that call
$this->vcsAdapter->getCommit(...) and $this->vcsAdapter->getLatestCommit(...) in
try/finally blocks that always call the cleanup method (e.g.
$this->vcsAdapter->deleteRepository(self::$owner, $repositoryName)) in the
finally so the repositoryName created in each test is removed regardless of the
thrown exception.
![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: d54667d93b
ℹ️ 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".
| public function getCommit(string $owner, string $repositoryName, string $commitHash): array | ||
| { | ||
| throw new Exception("Not implemented yet"); | ||
| $url = "/repos/{$owner}/{$repositoryName}/git/commits/{$commitHash}"; |
There was a problem hiding this comment.
getCommit calls /git/commits/{sha} but then parses the response as if it were the repository commit schema (body['commit']['author']['name'], body['commit']['message']). In Gitea, the git-data commit payload does not carry those nested fields, so valid commits will silently return fallback values like 'Unknown' and 'No message' instead of real metadata, making this new endpoint functionally incorrect for normal inputs.
Useful? React with 👍 / 👎.
| { | ||
| throw new Exception("Not implemented yet"); | ||
| } | ||
| $url = "/repos/{$owner}/{$repositoryName}/commits?sha={$branch}&limit=1"; |
There was a problem hiding this comment.
The branch is interpolated directly into the query string, so valid ref names containing query-significant characters (for example & or +) will be misparsed by HTTP query decoding and request the wrong SHA filter. This causes getLatestCommit to fail or return commits from an unintended ref for those branch names.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds commit-retrieval support to the Gitea VCS adapter to align with the existing GitHub adapter, along with new integration tests that exercise commit fetching and error paths.
Changes:
- Implement
getCommit(owner, repo, commitHash)in the Gitea adapter. - Implement
getLatestCommit(owner, repo, branch)in the Gitea adapter. - Add PHPUnit integration tests covering commit retrieval and invalid SHA/branch handling.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
src/VCS/Adapter/Git/Gitea.php |
Adds Gitea implementations for getCommit and getLatestCommit. |
tests/VCS/Adapter/GiteaTest.php |
Adds integration tests validating commit retrieval and error handling scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| $repositoryName = 'test-get-latest-commit-invalid-' . \uniqid(); | ||
| $this->vcsAdapter->createRepository(self::$owner, $repositoryName, false); | ||
| $this->vcsAdapter->createFile(self::$owner, $repositoryName, 'README.md', '# Test'); | ||
|
|
||
| $this->expectException(\Exception::class); | ||
| $this->vcsAdapter->getLatestCommit(self::$owner, $repositoryName, 'non-existing-branch'); | ||
| } |
There was a problem hiding this comment.
testGetLatestCommitWithInvalidBranch() creates a repository but never deletes it because the test exits via the expected exception. Ensure repository cleanup runs even when the exception is thrown (e.g., try/finally around the call).
| $this->assertSame($commitHash, $result['commitHash']); | ||
| $this->assertNotEmpty($result['commitMessage']); | ||
|
|
There was a problem hiding this comment.
The assertions in testGetCommit() are currently satisfied by the adapter’s placeholder defaults (e.g. commitMessage being No message). To ensure the endpoint actually returns real commit data, assert that commitMessage contains the commit message used in createFile() (and/or that commitAuthor is not Unknown, and commitUrl is non-empty).
| $this->assertArrayHasKey('commitAuthorAvatar', $result); | ||
| $this->assertArrayHasKey('commitAuthorUrl', $result); | ||
|
|
||
| $this->assertNotEmpty($result['commitHash']); |
There was a problem hiding this comment.
In testGetLatestCommit(), assertNotEmpty($result['commitAuthor']) will still pass if the adapter returns the fallback value Unknown. Consider asserting on the expected commit message (e.g. default Add file or a provided custom message) and/or that commitUrl is non-empty to validate that parsing is correct.
| $this->assertNotEmpty($result['commitHash']); | |
| $this->assertNotEmpty($result['commitHash']); | |
| $this->assertNotEmpty($result['commitMessage']); | |
| $this->assertNotEmpty($result['commitUrl']); |
| $url = "/repos/{$owner}/{$repositoryName}/git/commits/{$commitHash}"; | ||
|
|
||
| $response = $this->call(self::METHOD_GET, $url, ['Authorization' => "token $this->accessToken"]); | ||
|
|
||
| $statusCode = $response['headers']['status-code'] ?? 0; | ||
| if ($statusCode >= 400) { | ||
| throw new Exception("Commit not found or inaccessible"); | ||
| } | ||
|
|
||
| $body = $response['body'] ?? []; | ||
| $commit = $body['commit'] ?? []; | ||
| $author = $body['author'] ?? []; | ||
|
|
||
| return [ | ||
| 'commitAuthor' => $commit['author']['name'] ?? 'Unknown', | ||
| 'commitMessage' => $commit['message'] ?? 'No message', | ||
| 'commitAuthorAvatar' => $author['avatar_url'] ?? '', | ||
| 'commitAuthorUrl' => $author['html_url'] ?? '', | ||
| 'commitHash' => $body['sha'] ?? '', | ||
| 'commitUrl' => $body['html_url'] ?? '', | ||
| ]; |
There was a problem hiding this comment.
getCommit() is calling the /git/commits/{sha} endpoint but then parses the response as if it came from the /commits/{sha} endpoint (expects commit.*, author.*, and html_url). This can lead to silently returning placeholder values (e.g. Unknown / No message / empty URLs) even when the API call succeeds. Align the endpoint and response parsing with getLatestCommit()/GitHub adapter (or adjust parsing to match the actual /git/commits schema) and fail fast if required fields are missing.
| { | ||
| throw new Exception("Not implemented yet"); | ||
| } | ||
| $url = "/repos/{$owner}/{$repositoryName}/commits?sha={$branch}&limit=1"; |
There was a problem hiding this comment.
getLatestCommit() interpolates $branch directly into the query string. If branch names contain characters like &, ?, or #, this can break the request (or allow query parameter injection). URL-encode the branch name (or pass it as a GET param to call() so it’s encoded via http_build_query).
| $url = "/repos/{$owner}/{$repositoryName}/commits?sha={$branch}&limit=1"; | |
| $encodedBranch = rawurlencode($branch); | |
| $url = "/repos/{$owner}/{$repositoryName}/commits?sha={$encodedBranch}&limit=1"; |
| { | ||
| $repositoryName = 'test-get-commit-invalid-' . \uniqid(); | ||
| $this->vcsAdapter->createRepository(self::$owner, $repositoryName, false); | ||
| $this->vcsAdapter->createFile(self::$owner, $repositoryName, 'README.md', '# Test'); | ||
|
|
||
| $this->expectException(\Exception::class); | ||
| $this->vcsAdapter->getCommit(self::$owner, $repositoryName, 'invalid-sha-12345'); | ||
| } |
There was a problem hiding this comment.
testGetCommitWithInvalidSha() creates a repository but never deletes it because the test exits via the expected exception. This can leak repositories across test runs and cause flakiness. Wrap the call in a try/finally (or equivalent) so deleteRepository() always runs.
Changes
Implements commit-related endpoints for Gitea adapter to match GitHub functionality.
Methods Implemented
getCommit(owner, repo, commitHash)- Fetches details of a specific commit by SHAgetLatestCommit(owner, repo, branch)- Fetches the most recent commit of a branchTests Added
testGetCommit- Verifies commit details retrievaltestGetLatestCommit- Verifies latest commit fetchingtestGetCommitWithInvalidSha- Tests error handling for invalid commit SHAtestGetLatestCommitWithInvalidBranch- Tests error handling for non-existent branchtestGetCommitVerifyMessageContent- Verifies custom commit messages are preservedAll tests follow the established pattern with proper setup/teardown and edge case coverage.
Summary by CodeRabbit
New Features
Tests