Compare commits
17 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
 claw
|
6316007eb1 | ||
|
claw
|
b380e7fcae | ||
|
claw
|
30798ff023 | ||
|
claw
|
6e8e744816 | ||
|
claw
|
1194bc758c | ||
|
claw
|
80af5037b2 | ||
|
claw
|
5b2fa0b9af | ||
|
claw
|
491df7cb1f | ||
|
claw
|
1fcc0b738a | ||
|
claw
|
fce5f2d184 | ||
|
claw
|
af72c64b7f | ||
|
claw
|
1bc3f206ba | ||
|
claw
|
c10bb72117 | ||
|
claw
|
ae91c8aef5 | ||
|
claw
|
75f65fbf5d | ||
|
claw
|
5b43afc6d4 | ||
|
claw
|
d1ef1e21e5 |
@@ -1,232 +0,0 @@
|
||||
package gitea
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
|
||||
"gitea.weiker.me/rodin/review-bot/vcs"
|
||||
)
|
||||
|
||||
// Adapter wraps a gitea.Client and satisfies the vcs.Client interface.
|
||||
// It handles translation between GitHub-canonical diff positions and Gitea
|
||||
// line numbers, and between canonical review event strings and Gitea-native values.
|
||||
type Adapter struct {
|
||||
client *Client
|
||||
}
|
||||
|
||||
// Compile-time interface conformance assertion.
|
||||
var _ vcs.Client = (*Adapter)(nil)
|
||||
|
||||
// NewAdapter creates a new Adapter wrapping the given gitea Client.
|
||||
func NewAdapter(client *Client) *Adapter {
|
||||
return &Adapter{client: client}
|
||||
}
|
||||
|
||||
// Underlying returns the wrapped gitea.Client for Gitea-specific operations
|
||||
// that have no vcs.Client equivalent (resolve comment, timeline, supersede flow).
|
||||
func (a *Adapter) Underlying() *Client {
|
||||
return a.client
|
||||
}
|
||||
|
||||
// --- PRReader ---
|
||||
|
||||
// GetPullRequest maps gitea.PullRequest to vcs.PullRequest.
|
||||
func (a *Adapter) GetPullRequest(ctx context.Context, owner, repo string, number int) (*vcs.PullRequest, error) {
|
||||
pr, err := a.client.GetPullRequest(ctx, owner, repo, number)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("get pull request: %w", err)
|
||||
}
|
||||
return &vcs.PullRequest{
|
||||
Number: number,
|
||||
Title: pr.Title,
|
||||
Body: pr.Body,
|
||||
Head: vcs.HeadRef{
|
||||
SHA: pr.Head.Sha,
|
||||
Ref: pr.Head.Ref,
|
||||
},
|
||||
Base: vcs.BaseRef{
|
||||
Ref: pr.Base.Ref,
|
||||
},
|
||||
}, nil
|
||||
}
|
||||
|
||||
// GetPullRequestDiff is a pass-through to the underlying client.
|
||||
func (a *Adapter) GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error) {
|
||||
return a.client.GetPullRequestDiff(ctx, owner, repo, number)
|
||||
}
|
||||
|
||||
// GetPullRequestFiles maps []gitea.ChangedFile to []vcs.ChangedFile.
|
||||
// Patch field is omitted (zero-value) since Gitea's /pulls/{n}/files does not return patch text.
|
||||
func (a *Adapter) GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]vcs.ChangedFile, error) {
|
||||
files, err := a.client.GetPullRequestFiles(ctx, owner, repo, number)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
result := make([]vcs.ChangedFile, len(files))
|
||||
for i, f := range files {
|
||||
result[i] = vcs.ChangedFile{
|
||||
Filename: f.Filename,
|
||||
Status: f.Status,
|
||||
}
|
||||
}
|
||||
return result, nil
|
||||
}
|
||||
|
||||
// GetFileContentAtRef is a pass-through to the underlying client.
|
||||
func (a *Adapter) GetFileContentAtRef(ctx context.Context, owner, repo, path, ref string) (string, error) {
|
||||
return a.client.GetFileContentAtRef(ctx, owner, repo, path, ref)
|
||||
}
|
||||
|
||||
// GetCommitStatuses maps []gitea.CommitStatus to []vcs.CommitStatus.
|
||||
func (a *Adapter) GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]vcs.CommitStatus, error) {
|
||||
statuses, err := a.client.GetCommitStatuses(ctx, owner, repo, sha)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
result := make([]vcs.CommitStatus, len(statuses))
|
||||
for i, s := range statuses {
|
||||
result[i] = vcs.CommitStatus{
|
||||
Status: s.Status,
|
||||
Context: s.Context,
|
||||
Description: s.Description,
|
||||
TargetURL: s.TargetURL,
|
||||
}
|
||||
}
|
||||
return result, nil
|
||||
}
|
||||
|
||||
// --- FileReader ---
|
||||
|
||||
// GetFileContent delegates to the underlying client, routing to the ref-aware
|
||||
// variant when ref is non-empty.
|
||||
func (a *Adapter) GetFileContent(ctx context.Context, owner, repo, path, ref string) (string, error) {
|
||||
if ref != "" {
|
||||
return a.client.GetFileContentRef(ctx, owner, repo, path, ref)
|
||||
}
|
||||
return a.client.GetFileContent(ctx, owner, repo, path)
|
||||
}
|
||||
|
||||
// ListContents maps []gitea.ContentEntry to []vcs.ContentEntry.
|
||||
func (a *Adapter) ListContents(ctx context.Context, owner, repo, path string) ([]vcs.ContentEntry, error) {
|
||||
entries, err := a.client.ListContents(ctx, owner, repo, path)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
result := make([]vcs.ContentEntry, len(entries))
|
||||
for i, e := range entries {
|
||||
result[i] = vcs.ContentEntry{
|
||||
Name: e.Name,
|
||||
Path: e.Path,
|
||||
Type: e.Type,
|
||||
}
|
||||
}
|
||||
return result, nil
|
||||
}
|
||||
|
||||
// --- Reviewer ---
|
||||
|
||||
// translateEvent translates a vcs.ReviewEvent (GitHub-canonical) to a Gitea-native event string.
|
||||
func translateEvent(event vcs.ReviewEvent) string {
|
||||
switch event {
|
||||
case vcs.ReviewEventApprove:
|
||||
return "APPROVED"
|
||||
case vcs.ReviewEventRequestChanges:
|
||||
return "REQUEST_CHANGES"
|
||||
case vcs.ReviewEventComment:
|
||||
return "COMMENT"
|
||||
default:
|
||||
// Unknown events pass through as-is. This is intentional: new event types
|
||||
// added to vcs.ReviewEvent will still be forwarded without a code change here,
|
||||
// and Gitea will reject truly invalid values with a clear API error.
|
||||
return string(event)
|
||||
}
|
||||
}
|
||||
|
||||
// PostReview translates vcs.ReviewRequest to the Gitea-native format.
|
||||
// It fetches the PR diff, builds a position-to-line map, and translates each
|
||||
// ReviewComment.Position (GitHub diff-position) to a Gitea new_position (line number).
|
||||
func (a *Adapter) PostReview(ctx context.Context, owner, repo string, number int, req vcs.ReviewRequest) (*vcs.Review, error) {
|
||||
event := translateEvent(req.Event)
|
||||
|
||||
var giteaComments []ReviewComment
|
||||
if len(req.Comments) > 0 {
|
||||
// Fetch diff to build position → line number map.
|
||||
// The diff is fetched unconditionally when comments exist. This adds latency
|
||||
// for reviews with inline comments but keeps the implementation simple — caching
|
||||
// the diff across calls would add complexity for minimal gain since PostReview
|
||||
// is called at most once per review cycle.
|
||||
diff, err := a.client.GetPullRequestDiff(ctx, owner, repo, number)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("fetch diff for position translation: %w", err)
|
||||
}
|
||||
|
||||
posMap := BuildPositionToLineMap(diff)
|
||||
|
||||
for _, c := range req.Comments {
|
||||
lineNum, err := posMap.Translate(c.Path, c.Position)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("translate position %d in %s: %w", c.Position, c.Path, err)
|
||||
}
|
||||
// CommitID from vcs.ReviewComment is intentionally not forwarded:
|
||||
// Gitea review comments are pinned to the PR head SHA automatically,
|
||||
// and the CreatePullReview API has no per-comment commit_id field.
|
||||
giteaComments = append(giteaComments, ReviewComment{
|
||||
Path: c.Path,
|
||||
NewPosition: int64(lineNum),
|
||||
Body: c.Body,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
review, err := a.client.PostReview(ctx, owner, repo, number, event, req.Body, giteaComments)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("post review: %w", err)
|
||||
}
|
||||
|
||||
return &vcs.Review{
|
||||
ID: review.ID,
|
||||
Body: review.Body,
|
||||
User: vcs.UserInfo{Login: review.User.Login},
|
||||
State: review.State,
|
||||
Stale: review.Stale,
|
||||
CommitID: review.CommitID,
|
||||
}, nil
|
||||
}
|
||||
|
||||
// ListReviews maps []gitea.Review to []vcs.Review.
|
||||
func (a *Adapter) ListReviews(ctx context.Context, owner, repo string, number int) ([]vcs.Review, error) {
|
||||
reviews, err := a.client.ListReviews(ctx, owner, repo, number)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
result := make([]vcs.Review, len(reviews))
|
||||
for i, r := range reviews {
|
||||
result[i] = vcs.Review{
|
||||
ID: r.ID,
|
||||
Body: r.Body,
|
||||
User: vcs.UserInfo{Login: r.User.Login},
|
||||
State: r.State,
|
||||
Stale: r.Stale,
|
||||
CommitID: r.CommitID,
|
||||
}
|
||||
}
|
||||
return result, nil
|
||||
}
|
||||
|
||||
// DeleteReview is a pass-through to the underlying client.
|
||||
func (a *Adapter) DeleteReview(ctx context.Context, owner, repo string, number int, reviewID int64) error {
|
||||
return a.client.DeleteReview(ctx, owner, repo, number, reviewID)
|
||||
}
|
||||
|
||||
// DismissReview deletes the review. Gitea supports full deletion of any review state.
|
||||
// The message parameter is intentionally unused — Gitea deletion has no dismissal message.
|
||||
func (a *Adapter) DismissReview(ctx context.Context, owner, repo string, number int, reviewID int64, message string) error {
|
||||
return a.client.DeleteReview(ctx, owner, repo, number, reviewID)
|
||||
}
|
||||
|
||||
// --- Identity ---
|
||||
|
||||
// GetAuthenticatedUser is a pass-through to the underlying client.
|
||||
func (a *Adapter) GetAuthenticatedUser(ctx context.Context) (string, error) {
|
||||
return a.client.GetAuthenticatedUser(ctx)
|
||||
}
|
||||
@@ -1,388 +0,0 @@
|
||||
package gitea_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"gitea.weiker.me/rodin/review-bot/gitea"
|
||||
"gitea.weiker.me/rodin/review-bot/vcs"
|
||||
)
|
||||
|
||||
func TestAdapter_GetPullRequest(t *testing.T) {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode(map[string]any{
|
||||
"title": "Test PR",
|
||||
"body": "PR body",
|
||||
"head": map[string]any{
|
||||
"sha": "abc123",
|
||||
"ref": "feature-branch",
|
||||
},
|
||||
"base": map[string]any{
|
||||
"ref": "main",
|
||||
},
|
||||
})
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := gitea.NewClient(server.URL, "token")
|
||||
adapter := gitea.NewAdapter(client)
|
||||
|
||||
pr, err := adapter.GetPullRequest(context.Background(), "owner", "repo", 42)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if pr.Number != 42 {
|
||||
t.Errorf("Number = %d, want 42", pr.Number)
|
||||
}
|
||||
if pr.Title != "Test PR" {
|
||||
t.Errorf("Title = %q, want %q", pr.Title, "Test PR")
|
||||
}
|
||||
if pr.Body != "PR body" {
|
||||
t.Errorf("Body = %q, want %q", pr.Body, "PR body")
|
||||
}
|
||||
if pr.Head.SHA != "abc123" {
|
||||
t.Errorf("Head.SHA = %q, want %q", pr.Head.SHA, "abc123")
|
||||
}
|
||||
if pr.Head.Ref != "feature-branch" {
|
||||
t.Errorf("Head.Ref = %q, want %q", pr.Head.Ref, "feature-branch")
|
||||
}
|
||||
if pr.Base.Ref != "main" {
|
||||
t.Errorf("Base.Ref = %q, want %q", pr.Base.Ref, "main")
|
||||
}
|
||||
}
|
||||
|
||||
func TestAdapter_GetPullRequestFiles(t *testing.T) {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode([]map[string]any{
|
||||
{"filename": "main.go", "status": "modified"},
|
||||
{"filename": "new.go", "status": "added"},
|
||||
})
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := gitea.NewClient(server.URL, "token")
|
||||
adapter := gitea.NewAdapter(client)
|
||||
|
||||
files, err := adapter.GetPullRequestFiles(context.Background(), "owner", "repo", 1)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if len(files) != 2 {
|
||||
t.Fatalf("got %d files, want 2", len(files))
|
||||
}
|
||||
if files[0].Filename != "main.go" || files[0].Status != "modified" {
|
||||
t.Errorf("files[0] = %+v", files[0])
|
||||
}
|
||||
if files[1].Filename != "new.go" || files[1].Status != "added" {
|
||||
t.Errorf("files[1] = %+v", files[1])
|
||||
}
|
||||
}
|
||||
|
||||
func TestAdapter_ListReviews(t *testing.T) {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode([]map[string]any{
|
||||
{
|
||||
"id": 1,
|
||||
"body": "LGTM",
|
||||
"user": map[string]any{"login": "reviewer1"},
|
||||
"state": "APPROVED",
|
||||
"stale": false,
|
||||
"commit_id": "abc123",
|
||||
},
|
||||
{
|
||||
"id": 2,
|
||||
"body": "Needs work",
|
||||
"user": map[string]any{"login": "reviewer2"},
|
||||
"state": "REQUEST_CHANGES",
|
||||
"stale": true,
|
||||
"commit_id": "def456",
|
||||
},
|
||||
})
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := gitea.NewClient(server.URL, "token")
|
||||
adapter := gitea.NewAdapter(client)
|
||||
|
||||
reviews, err := adapter.ListReviews(context.Background(), "owner", "repo", 1)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if len(reviews) != 2 {
|
||||
t.Fatalf("got %d reviews, want 2", len(reviews))
|
||||
}
|
||||
if reviews[0].ID != 1 || reviews[0].Body != "LGTM" || reviews[0].User.Login != "reviewer1" {
|
||||
t.Errorf("reviews[0] = %+v", reviews[0])
|
||||
}
|
||||
if reviews[0].State != "APPROVED" || reviews[0].Stale || reviews[0].CommitID != "abc123" {
|
||||
t.Errorf("reviews[0] state/stale/commit = %v/%v/%v", reviews[0].State, reviews[0].Stale, reviews[0].CommitID)
|
||||
}
|
||||
if reviews[1].ID != 2 || !reviews[1].Stale || reviews[1].State != "REQUEST_CHANGES" {
|
||||
t.Errorf("reviews[1] = %+v", reviews[1])
|
||||
}
|
||||
}
|
||||
|
||||
func TestAdapter_GetCommitStatuses(t *testing.T) {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode([]map[string]any{
|
||||
{
|
||||
"status": "success",
|
||||
"context": "ci/test",
|
||||
"description": "All tests pass",
|
||||
"target_url": "https://ci.example.com/1",
|
||||
},
|
||||
})
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := gitea.NewClient(server.URL, "token")
|
||||
adapter := gitea.NewAdapter(client)
|
||||
|
||||
statuses, err := adapter.GetCommitStatuses(context.Background(), "owner", "repo", "abc123")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if len(statuses) != 1 {
|
||||
t.Fatalf("got %d statuses, want 1", len(statuses))
|
||||
}
|
||||
if statuses[0].Status != "success" {
|
||||
t.Errorf("Status = %q, want %q", statuses[0].Status, "success")
|
||||
}
|
||||
if statuses[0].Context != "ci/test" {
|
||||
t.Errorf("Context = %q, want %q", statuses[0].Context, "ci/test")
|
||||
}
|
||||
if statuses[0].Description != "All tests pass" {
|
||||
t.Errorf("Description = %q, want %q", statuses[0].Description, "All tests pass")
|
||||
}
|
||||
if statuses[0].TargetURL != "https://ci.example.com/1" {
|
||||
t.Errorf("TargetURL = %q, want %q", statuses[0].TargetURL, "https://ci.example.com/1")
|
||||
}
|
||||
}
|
||||
|
||||
func TestAdapter_PostReview_EventTranslation(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
event vcs.ReviewEvent
|
||||
wantEvent string
|
||||
}{
|
||||
{"APPROVE becomes APPROVED", vcs.ReviewEventApprove, "APPROVED"},
|
||||
{"REQUEST_CHANGES stays", vcs.ReviewEventRequestChanges, "REQUEST_CHANGES"},
|
||||
{"COMMENT stays", vcs.ReviewEventComment, "COMMENT"},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
var gotEvent string
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
var payload struct {
|
||||
Event string `json:"event"`
|
||||
}
|
||||
json.NewDecoder(r.Body).Decode(&payload)
|
||||
gotEvent = payload.Event
|
||||
json.NewEncoder(w).Encode(map[string]any{
|
||||
"id": 1,
|
||||
"body": "test",
|
||||
"user": map[string]any{"login": "bot"},
|
||||
})
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := gitea.NewClient(server.URL, "token")
|
||||
adapter := gitea.NewAdapter(client)
|
||||
|
||||
_, err := adapter.PostReview(context.Background(), "owner", "repo", 1, vcs.ReviewRequest{
|
||||
Body: "test",
|
||||
Event: tt.event,
|
||||
// No comments → no diff fetch needed
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if gotEvent != tt.wantEvent {
|
||||
t.Errorf("event = %q, want %q", gotEvent, tt.wantEvent)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestAdapter_PostReview_WithComments_PositionTranslation(t *testing.T) {
|
||||
diff := `diff --git a/main.go b/main.go
|
||||
--- a/main.go
|
||||
+++ b/main.go
|
||||
@@ -1,3 +1,4 @@
|
||||
package main
|
||||
|
||||
+// new comment at line 3
|
||||
func main() {}
|
||||
`
|
||||
var gotComments []struct {
|
||||
Path string `json:"path"`
|
||||
NewPosition int64 `json:"new_position"`
|
||||
Body string `json:"body"`
|
||||
}
|
||||
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
if strings.HasSuffix(r.URL.Path, ".diff") {
|
||||
// Diff request
|
||||
w.Write([]byte(diff))
|
||||
return
|
||||
}
|
||||
if strings.HasSuffix(r.URL.Path, "/reviews") {
|
||||
// Review post
|
||||
var payload struct {
|
||||
Comments []struct {
|
||||
Path string `json:"path"`
|
||||
NewPosition int64 `json:"new_position"`
|
||||
Body string `json:"body"`
|
||||
} `json:"comments"`
|
||||
}
|
||||
json.NewDecoder(r.Body).Decode(&payload)
|
||||
gotComments = payload.Comments
|
||||
json.NewEncoder(w).Encode(map[string]any{
|
||||
"id": 1,
|
||||
"body": "review",
|
||||
"user": map[string]any{"login": "bot"},
|
||||
})
|
||||
return
|
||||
}
|
||||
t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path)
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := gitea.NewClient(server.URL, "token")
|
||||
adapter := gitea.NewAdapter(client)
|
||||
|
||||
// Position 4 in this diff is "+// new comment at line 3" → new line 3
|
||||
_, err := adapter.PostReview(context.Background(), "owner", "repo", 1, vcs.ReviewRequest{
|
||||
Body: "review",
|
||||
Event: vcs.ReviewEventRequestChanges,
|
||||
Comments: []vcs.ReviewComment{
|
||||
{
|
||||
Path: "main.go",
|
||||
Position: 4,
|
||||
CommitID: "abc123",
|
||||
Body: "needs fix",
|
||||
},
|
||||
},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
|
||||
if len(gotComments) != 1 {
|
||||
t.Fatalf("got %d comments, want 1", len(gotComments))
|
||||
}
|
||||
if gotComments[0].Path != "main.go" {
|
||||
t.Errorf("path = %q, want %q", gotComments[0].Path, "main.go")
|
||||
}
|
||||
if gotComments[0].NewPosition != 3 {
|
||||
t.Errorf("new_position = %d, want 3", gotComments[0].NewPosition)
|
||||
}
|
||||
if gotComments[0].Body != "needs fix" {
|
||||
t.Errorf("body = %q, want %q", gotComments[0].Body, "needs fix")
|
||||
}
|
||||
}
|
||||
|
||||
func TestAdapter_DismissReview(t *testing.T) {
|
||||
var deleteCalled bool
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Method == http.MethodDelete {
|
||||
deleteCalled = true
|
||||
w.WriteHeader(204)
|
||||
return
|
||||
}
|
||||
w.WriteHeader(404)
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := gitea.NewClient(server.URL, "token")
|
||||
adapter := gitea.NewAdapter(client)
|
||||
|
||||
err := adapter.DismissReview(context.Background(), "owner", "repo", 1, 99, "stale review")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if !deleteCalled {
|
||||
t.Error("expected delete to be called")
|
||||
}
|
||||
}
|
||||
|
||||
func TestAdapter_Underlying(t *testing.T) {
|
||||
client := gitea.NewClient("http://example.com", "token")
|
||||
adapter := gitea.NewAdapter(client)
|
||||
if adapter.Underlying() != client {
|
||||
t.Error("Underlying() should return the wrapped client")
|
||||
}
|
||||
}
|
||||
|
||||
func TestAdapter_ListContents(t *testing.T) {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode([]map[string]any{
|
||||
{"name": "main.go", "path": "src/main.go", "type": "file"},
|
||||
{"name": "util", "path": "src/util", "type": "dir"},
|
||||
})
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := gitea.NewClient(server.URL, "token")
|
||||
adapter := gitea.NewAdapter(client)
|
||||
|
||||
entries, err := adapter.ListContents(context.Background(), "owner", "repo", "src")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if len(entries) != 2 {
|
||||
t.Fatalf("got %d entries, want 2", len(entries))
|
||||
}
|
||||
if entries[0].Name != "main.go" || entries[0].Type != "file" {
|
||||
t.Errorf("entries[0] = %+v", entries[0])
|
||||
}
|
||||
if entries[1].Name != "util" || entries[1].Type != "dir" {
|
||||
t.Errorf("entries[1] = %+v", entries[1])
|
||||
}
|
||||
}
|
||||
|
||||
func TestAdapter_GetFileContent_RefRouting(t *testing.T) {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
// When ref is provided, the URL should contain ?ref=
|
||||
if r.URL.RawQuery != "" && strings.Contains(r.URL.RawQuery, "ref=") {
|
||||
w.Write([]byte("content-at-ref"))
|
||||
} else {
|
||||
w.Write([]byte("content-default"))
|
||||
}
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := gitea.NewClient(server.URL, "token")
|
||||
adapter := gitea.NewAdapter(client)
|
||||
|
||||
// Empty ref → routes to GetFileContent (no ?ref= query param)
|
||||
got, err := adapter.GetFileContent(context.Background(), "owner", "repo", "main.go", "")
|
||||
if err != nil {
|
||||
t.Fatalf("GetFileContent(ref=\"\"): %v", err)
|
||||
}
|
||||
if got != "content-default" {
|
||||
t.Errorf("GetFileContent(ref=\"\") = %q, want %q", got, "content-default")
|
||||
}
|
||||
|
||||
// Non-empty ref → routes to GetFileContentRef (with ?ref= query param)
|
||||
got, err = adapter.GetFileContent(context.Background(), "owner", "repo", "main.go", "abc123")
|
||||
if err != nil {
|
||||
t.Fatalf("GetFileContent(ref=\"abc123\"): %v", err)
|
||||
}
|
||||
if got != "content-at-ref" {
|
||||
t.Errorf("GetFileContent(ref=\"abc123\") = %q, want %q", got, "content-at-ref")
|
||||
}
|
||||
}
|
||||
@@ -86,9 +86,6 @@ type PullRequest struct {
|
||||
Sha string `json:"sha"`
|
||||
Ref string `json:"ref"`
|
||||
} `json:"head"`
|
||||
Base struct {
|
||||
Ref string `json:"ref"`
|
||||
} `json:"base"`
|
||||
}
|
||||
|
||||
// CommitStatus represents a single CI status entry.
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
//go:build phase2
|
||||
|
||||
package gitea_test
|
||||
|
||||
import (
|
||||
@@ -5,6 +7,19 @@ import (
|
||||
"gitea.weiker.me/rodin/review-bot/vcs"
|
||||
)
|
||||
|
||||
// Compile-time interface conformance assertion.
|
||||
// The Adapter (not the raw Client) satisfies the full vcs.Client interface.
|
||||
var _ vcs.Client = (*gitea.Adapter)(nil)
|
||||
// Compile-time interface conformance assertions.
|
||||
// These will verify gitea.Client satisfies vcs interfaces once the Phase 2
|
||||
// adapter bridges the method signature gaps:
|
||||
//
|
||||
// - PRReader: GetPullRequest returns *gitea.PullRequest (needs *vcs.PullRequest)
|
||||
// - PRReader: GetPullRequestFiles returns []gitea.ChangedFile (needs []vcs.ChangedFile)
|
||||
// - FileReader: GetFileContent lacks ref parameter
|
||||
// - Reviewer: PostReview uses (event, body, comments) instead of vcs.ReviewRequest
|
||||
//
|
||||
// Remove the phase2 build tag once the adapter is complete.
|
||||
var (
|
||||
_ vcs.PRReader = (*gitea.Client)(nil)
|
||||
_ vcs.FileReader = (*gitea.Client)(nil)
|
||||
_ vcs.Reviewer = (*gitea.Client)(nil)
|
||||
_ vcs.Identity = (*gitea.Client)(nil)
|
||||
)
|
||||
|
||||
@@ -1,190 +0,0 @@
|
||||
package gitea
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"strconv"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// PositionMap holds a per-file mapping of GitHub diff-position to new-file line number.
|
||||
// Position is a 1-indexed offset from the @@ hunk header line in the unified diff.
|
||||
type PositionMap struct {
|
||||
// files maps filename → (position → new-file line number).
|
||||
// Deletion lines are mapped to -1 (no new-file line).
|
||||
files map[string]map[int]int
|
||||
// maxPositions caches the highest position number per file,
|
||||
// tracked during construction to avoid O(n) scans at translate time.
|
||||
maxPositions map[string]int
|
||||
}
|
||||
|
||||
// Translate converts a GitHub diff-position to a new-file line number for a given file.
|
||||
// Returns an error if the file is not in the diff or the position is out of range.
|
||||
// If the position targets a deletion line, it maps to the nearest non-deletion line below;
|
||||
// if no such line exists, returns an error.
|
||||
func (pm *PositionMap) Translate(file string, position int) (int, error) {
|
||||
if pm == nil || pm.files == nil {
|
||||
return 0, fmt.Errorf("empty position map")
|
||||
}
|
||||
|
||||
fileMap, ok := pm.files[file]
|
||||
if !ok {
|
||||
return 0, fmt.Errorf("file %q not found in diff", file)
|
||||
}
|
||||
|
||||
if position < 1 {
|
||||
return 0, fmt.Errorf("position %d out of range (must be >= 1)", position)
|
||||
}
|
||||
|
||||
lineNum, ok := fileMap[position]
|
||||
if !ok {
|
||||
return 0, fmt.Errorf("position %d out of range for file %q", position, file)
|
||||
}
|
||||
|
||||
// lineNum == -1 means this position is a deletion line.
|
||||
// Map to the nearest non-deletion line below.
|
||||
if lineNum == -1 {
|
||||
maxPos := pm.maxPosition(file)
|
||||
for p := position + 1; p <= maxPos; p++ {
|
||||
if ln, exists := fileMap[p]; exists && ln > 0 {
|
||||
return ln, nil
|
||||
}
|
||||
}
|
||||
return 0, fmt.Errorf("position %d targets a deletion line with no subsequent new-file line in %q", position, file)
|
||||
}
|
||||
|
||||
return lineNum, nil
|
||||
}
|
||||
|
||||
// maxPosition returns the highest position number for a file.
|
||||
// O(1) — the maximum is tracked during map construction.
|
||||
func (pm *PositionMap) maxPosition(file string) int {
|
||||
return pm.maxPositions[file]
|
||||
}
|
||||
|
||||
// BuildPositionToLineMap parses a unified diff and builds a PositionMap
|
||||
// mapping diff-position → new-file line number per file.
|
||||
//
|
||||
// Diff-position counting rules (GitHub spec):
|
||||
// - The @@ hunk header line is position 1 for the file's first hunk
|
||||
// - Every subsequent line increments position by 1 — context, additions, AND deletions
|
||||
// - A new @@ hunk within the same file continues incrementing (does not reset)
|
||||
// - Position maps to the new file line number for additions and context lines
|
||||
// - Deletion lines have a position but no new-file line number (stored as -1)
|
||||
func BuildPositionToLineMap(diff string) *PositionMap {
|
||||
pm := &PositionMap{
|
||||
files: make(map[string]map[int]int),
|
||||
maxPositions: make(map[string]int),
|
||||
}
|
||||
|
||||
lines := strings.Split(diff, "\n")
|
||||
var currentFile string
|
||||
var position int
|
||||
var newLine int
|
||||
|
||||
for _, line := range lines {
|
||||
// Detect new file in diff.
|
||||
// "+++ b/" is checked before "+++ /dev/null" — the two prefixes are
|
||||
// non-overlapping ("+++ /dev/null" does not start with "+++ b/"), so
|
||||
// ordering is independent. Checking the common case first for clarity.
|
||||
if strings.HasPrefix(line, "+++ b/") {
|
||||
currentFile = strings.TrimPrefix(line, "+++ b/")
|
||||
position = 0
|
||||
newLine = 0
|
||||
if pm.files[currentFile] == nil {
|
||||
pm.files[currentFile] = make(map[int]int)
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
// Deleted file: +++ /dev/null means the file is being deleted
|
||||
if strings.HasPrefix(line, "+++ /dev/null") {
|
||||
currentFile = ""
|
||||
continue
|
||||
}
|
||||
|
||||
// Skip --- lines (old file header)
|
||||
if strings.HasPrefix(line, "--- ") {
|
||||
continue
|
||||
}
|
||||
|
||||
// Skip diff --git lines
|
||||
if strings.HasPrefix(line, "diff --git") {
|
||||
continue
|
||||
}
|
||||
|
||||
// Skip index lines
|
||||
if strings.HasPrefix(line, "index ") {
|
||||
continue
|
||||
}
|
||||
|
||||
// Binary file detection
|
||||
if strings.HasPrefix(line, "Binary files") {
|
||||
currentFile = ""
|
||||
continue
|
||||
}
|
||||
|
||||
// Parse hunk headers
|
||||
if strings.HasPrefix(line, "@@") && currentFile != "" {
|
||||
position++
|
||||
pm.maxPositions[currentFile] = position
|
||||
newLine = parseHunkStart(line)
|
||||
continue
|
||||
}
|
||||
|
||||
if currentFile == "" {
|
||||
continue
|
||||
}
|
||||
|
||||
// Skip "\ No newline at end of file" markers
|
||||
if strings.HasPrefix(line, `\`) {
|
||||
continue
|
||||
}
|
||||
|
||||
// Process diff content lines
|
||||
if strings.HasPrefix(line, "+") {
|
||||
// Addition: has a new-file line number
|
||||
position++
|
||||
pm.files[currentFile][position] = newLine
|
||||
pm.maxPositions[currentFile] = position
|
||||
newLine++
|
||||
} else if strings.HasPrefix(line, "-") {
|
||||
// Deletion: has a position but no new-file line number
|
||||
position++
|
||||
pm.files[currentFile][position] = -1
|
||||
pm.maxPositions[currentFile] = position
|
||||
} else if strings.HasPrefix(line, " ") {
|
||||
// Context line
|
||||
position++
|
||||
pm.files[currentFile][position] = newLine
|
||||
pm.maxPositions[currentFile] = position
|
||||
newLine++
|
||||
}
|
||||
}
|
||||
|
||||
return pm
|
||||
}
|
||||
|
||||
// parseHunkStart extracts the new-file starting line number from a hunk header.
|
||||
// Format: @@ -old_start[,old_count] +new_start[,new_count] @@
|
||||
func parseHunkStart(hunkLine string) int {
|
||||
plusIdx := strings.Index(hunkLine, "+")
|
||||
if plusIdx < 0 {
|
||||
return 1
|
||||
}
|
||||
rest := hunkLine[plusIdx+1:]
|
||||
|
||||
endIdx := 0
|
||||
for endIdx < len(rest) && rest[endIdx] >= '0' && rest[endIdx] <= '9' {
|
||||
endIdx++
|
||||
}
|
||||
|
||||
if endIdx == 0 {
|
||||
return 1
|
||||
}
|
||||
|
||||
n, err := strconv.Atoi(rest[:endIdx])
|
||||
if err != nil {
|
||||
return 1
|
||||
}
|
||||
return n
|
||||
}
|
||||
@@ -1,274 +0,0 @@
|
||||
package gitea
|
||||
|
||||
import (
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestBuildPositionToLineMap_SingleHunk(t *testing.T) {
|
||||
// @@ -16,4 +16,5 @@ ← position 1
|
||||
// context ← position 2, new line 16
|
||||
//-deleted ← position 3, no new line
|
||||
//+added ← position 4, new line 17
|
||||
// context ← position 5, new line 18
|
||||
diff := `diff --git a/file.go b/file.go
|
||||
index abc..def 100644
|
||||
--- a/file.go
|
||||
+++ b/file.go
|
||||
@@ -16,4 +16,5 @@ func example() {
|
||||
context line
|
||||
-deleted line
|
||||
+added line
|
||||
context after
|
||||
`
|
||||
pm := BuildPositionToLineMap(diff)
|
||||
|
||||
tests := []struct {
|
||||
pos int
|
||||
wantLine int
|
||||
}{
|
||||
{2, 16}, // context line -> new line 16
|
||||
{4, 17}, // added line -> new line 17
|
||||
{5, 18}, // context after -> new line 18
|
||||
}
|
||||
for _, tt := range tests {
|
||||
got, err := pm.Translate("file.go", tt.pos)
|
||||
if err != nil {
|
||||
t.Errorf("Translate(file.go, %d): unexpected error: %v", tt.pos, err)
|
||||
continue
|
||||
}
|
||||
if got != tt.wantLine {
|
||||
t.Errorf("Translate(file.go, %d) = %d, want %d", tt.pos, got, tt.wantLine)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildPositionToLineMap_MultipleHunks(t *testing.T) {
|
||||
diff := `diff --git a/file.go b/file.go
|
||||
--- a/file.go
|
||||
+++ b/file.go
|
||||
@@ -1,3 +1,3 @@ package main
|
||||
line1
|
||||
-old
|
||||
+new
|
||||
@@ -10,3 +10,4 @@ func foo() {
|
||||
func foo() {
|
||||
+ // added
|
||||
return
|
||||
}
|
||||
`
|
||||
pm := BuildPositionToLineMap(diff)
|
||||
|
||||
tests := []struct {
|
||||
pos int
|
||||
wantLine int
|
||||
}{
|
||||
// First hunk: @@ is pos 1
|
||||
{2, 1}, // " line1" -> new line 1
|
||||
{4, 2}, // "+new" -> new line 2
|
||||
// Second hunk: @@ is pos 5 (continues from 4)
|
||||
// Wait: first hunk has pos 1(@@ hdr), 2(" line1"), 3("-old"), 4("+new")
|
||||
// Second hunk @@ is pos 5
|
||||
{6, 10}, // " func foo() {" -> new line 10
|
||||
{7, 11}, // "+\t// added" -> new line 11
|
||||
{8, 12}, // " \treturn" -> new line 12
|
||||
{9, 13}, // " }" -> new line 13
|
||||
}
|
||||
for _, tt := range tests {
|
||||
got, err := pm.Translate("file.go", tt.pos)
|
||||
if err != nil {
|
||||
t.Errorf("Translate(file.go, %d): unexpected error: %v", tt.pos, err)
|
||||
continue
|
||||
}
|
||||
if got != tt.wantLine {
|
||||
t.Errorf("Translate(file.go, %d) = %d, want %d", tt.pos, got, tt.wantLine)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildPositionToLineMap_DeletionTargeted(t *testing.T) {
|
||||
diff := `diff --git a/file.go b/file.go
|
||||
--- a/file.go
|
||||
+++ b/file.go
|
||||
@@ -1,4 +1,3 @@ package main
|
||||
line1
|
||||
-deleted
|
||||
line3
|
||||
`
|
||||
pm := BuildPositionToLineMap(diff)
|
||||
|
||||
// Position 3 is the deletion line "-deleted" — should map to nearest below
|
||||
// Position 4 is " line3" which is new line 2
|
||||
got, err := pm.Translate("file.go", 3)
|
||||
if err != nil {
|
||||
t.Fatalf("Translate(file.go, 3): unexpected error: %v", err)
|
||||
}
|
||||
if got != 2 {
|
||||
t.Errorf("Translate(file.go, 3) = %d, want 2 (nearest non-deletion below)", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildPositionToLineMap_DeletionAtEnd(t *testing.T) {
|
||||
// If a deletion line is at the end with no subsequent non-deletion line, error
|
||||
diff := `diff --git a/file.go b/file.go
|
||||
--- a/file.go
|
||||
+++ b/file.go
|
||||
@@ -1,3 +1,2 @@ package main
|
||||
line1
|
||||
line2
|
||||
-deleted at end
|
||||
`
|
||||
pm := BuildPositionToLineMap(diff)
|
||||
|
||||
_, err := pm.Translate("file.go", 4)
|
||||
if err == nil {
|
||||
t.Error("expected error for deletion at end with no subsequent line")
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildPositionToLineMap_NewFile(t *testing.T) {
|
||||
diff := `diff --git a/new.go b/new.go
|
||||
new file mode 100644
|
||||
--- /dev/null
|
||||
+++ b/new.go
|
||||
@@ -0,0 +1,3 @@
|
||||
+package main
|
||||
+
|
||||
+func init() {}
|
||||
`
|
||||
pm := BuildPositionToLineMap(diff)
|
||||
|
||||
tests := []struct {
|
||||
pos int
|
||||
wantLine int
|
||||
}{
|
||||
{2, 1}, // "+package main" -> line 1
|
||||
{3, 2}, // "+" (empty line) -> line 2
|
||||
{4, 3}, // "+func init() {}" -> line 3
|
||||
}
|
||||
for _, tt := range tests {
|
||||
got, err := pm.Translate("new.go", tt.pos)
|
||||
if err != nil {
|
||||
t.Errorf("Translate(new.go, %d): unexpected error: %v", tt.pos, err)
|
||||
continue
|
||||
}
|
||||
if got != tt.wantLine {
|
||||
t.Errorf("Translate(new.go, %d) = %d, want %d", tt.pos, got, tt.wantLine)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildPositionToLineMap_DeletedFile(t *testing.T) {
|
||||
diff := `diff --git a/old.go b/old.go
|
||||
deleted file mode 100644
|
||||
--- a/old.go
|
||||
+++ /dev/null
|
||||
@@ -1,3 +0,0 @@
|
||||
-package main
|
||||
-
|
||||
-func old() {}
|
||||
`
|
||||
pm := BuildPositionToLineMap(diff)
|
||||
|
||||
// Deleted file has no new-file lines; positions should error
|
||||
_, err := pm.Translate("old.go", 2)
|
||||
if err == nil {
|
||||
t.Error("expected error for deleted file position")
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildPositionToLineMap_BinaryFile(t *testing.T) {
|
||||
diff := `diff --git a/image.png b/image.png
|
||||
Binary files /dev/null and b/image.png differ
|
||||
diff --git a/code.go b/code.go
|
||||
--- a/code.go
|
||||
+++ b/code.go
|
||||
@@ -1,2 +1,3 @@
|
||||
package main
|
||||
+// added
|
||||
func main() {}
|
||||
`
|
||||
pm := BuildPositionToLineMap(diff)
|
||||
|
||||
// Binary file should not be in the map
|
||||
_, err := pm.Translate("image.png", 1)
|
||||
if err == nil {
|
||||
t.Error("expected error for binary file")
|
||||
}
|
||||
|
||||
// code.go should still work
|
||||
got, err := pm.Translate("code.go", 3)
|
||||
if err != nil {
|
||||
t.Fatalf("Translate(code.go, 3): unexpected error: %v", err)
|
||||
}
|
||||
if got != 2 {
|
||||
t.Errorf("Translate(code.go, 3) = %d, want 2", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildPositionToLineMap_OutOfRange(t *testing.T) {
|
||||
diff := `diff --git a/file.go b/file.go
|
||||
--- a/file.go
|
||||
+++ b/file.go
|
||||
@@ -1,2 +1,2 @@
|
||||
line1
|
||||
-old
|
||||
+new
|
||||
`
|
||||
pm := BuildPositionToLineMap(diff)
|
||||
|
||||
// Position 0 is invalid
|
||||
_, err := pm.Translate("file.go", 0)
|
||||
if err == nil {
|
||||
t.Error("expected error for position 0")
|
||||
}
|
||||
|
||||
// Position 5 is out of range (only positions 1-4 exist)
|
||||
_, err = pm.Translate("file.go", 5)
|
||||
if err == nil {
|
||||
t.Error("expected error for position 5 (out of range)")
|
||||
}
|
||||
|
||||
// Unknown file
|
||||
_, err = pm.Translate("unknown.go", 1)
|
||||
if err == nil {
|
||||
t.Error("expected error for unknown file")
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildPositionToLineMap_MultipleFiles(t *testing.T) {
|
||||
diff := `diff --git a/a.go b/a.go
|
||||
--- a/a.go
|
||||
+++ b/a.go
|
||||
@@ -1,2 +1,3 @@
|
||||
package a
|
||||
+// file a
|
||||
func aFunc() {}
|
||||
diff --git a/b.go b/b.go
|
||||
--- a/b.go
|
||||
+++ b/b.go
|
||||
@@ -1,2 +1,3 @@
|
||||
package b
|
||||
+// file b
|
||||
func bFunc() {}
|
||||
`
|
||||
pm := BuildPositionToLineMap(diff)
|
||||
|
||||
// a.go: pos 3 is "+// file a" -> new line 2
|
||||
got, err := pm.Translate("a.go", 3)
|
||||
if err != nil {
|
||||
t.Fatalf("Translate(a.go, 3): %v", err)
|
||||
}
|
||||
if got != 2 {
|
||||
t.Errorf("Translate(a.go, 3) = %d, want 2", got)
|
||||
}
|
||||
|
||||
// b.go: pos 3 is "+// file b" -> new line 2
|
||||
// Note: position resets per file
|
||||
got, err = pm.Translate("b.go", 3)
|
||||
if err != nil {
|
||||
t.Fatalf("Translate(b.go, 3): %v", err)
|
||||
}
|
||||
if got != 2 {
|
||||
t.Errorf("Translate(b.go, 3) = %d, want 2", got)
|
||||
}
|
||||
}
|
||||
+16
-2
@@ -47,6 +47,13 @@ func (e *APIError) Error() string {
|
||||
return fmt.Sprintf("HTTP %d: %s", e.StatusCode, body)
|
||||
}
|
||||
|
||||
// SafeError returns the error string without response body content,
|
||||
// suitable for logging in contexts where upstream response data should
|
||||
// not be exposed.
|
||||
func (e *APIError) SafeError() string {
|
||||
return fmt.Sprintf("HTTP %d", e.StatusCode)
|
||||
}
|
||||
|
||||
// IsNotFound reports whether an error is an API 404 response.
|
||||
func IsNotFound(err error) bool {
|
||||
if apiErr, ok := asAPIError(err); ok {
|
||||
@@ -172,6 +179,12 @@ func (c *Client) SetHTTPClient(hc *http.Client) {
|
||||
Timeout: 30 * time.Second,
|
||||
CheckRedirect: defaultCheckRedirect,
|
||||
}
|
||||
} else if hc.CheckRedirect == nil {
|
||||
// Enforce safe redirect policy when caller provides a client without one.
|
||||
// The default net/http behavior follows up to 10 redirects and forwards
|
||||
// all headers (including Authorization) to any host, which can leak
|
||||
// credentials on cross-host redirects.
|
||||
hc.CheckRedirect = defaultCheckRedirect
|
||||
}
|
||||
c.httpClient = hc
|
||||
}
|
||||
@@ -252,10 +265,11 @@ func (c *Client) doRequest(ctx context.Context, method, reqURL string, accept st
|
||||
|
||||
resp, err := c.httpClient.Do(req)
|
||||
if err != nil {
|
||||
// Transport errors (DNS, TLS, timeout) yield nil resp; no body to close.
|
||||
return nil, fmt.Errorf("do request: %w", err)
|
||||
}
|
||||
|
||||
body, done, err := c.handleResponse(resp, maxResponseBytes, maxErrorBodyBytes)
|
||||
body, done, err := handleResponse(resp, maxResponseBytes, maxErrorBodyBytes)
|
||||
if done {
|
||||
return body, err
|
||||
}
|
||||
@@ -300,7 +314,7 @@ func (c *Client) doRequest(ctx context.Context, method, reqURL string, accept st
|
||||
// handleResponse reads and closes the response body, returning the result.
|
||||
// It uses defer to ensure the body is always closed regardless of code path.
|
||||
// Returns (body, done, err) where done=true means the caller should return immediately.
|
||||
func (c *Client) handleResponse(resp *http.Response, maxRespBytes int, maxErrBytes int) ([]byte, bool, error) {
|
||||
func handleResponse(resp *http.Response, maxRespBytes int, maxErrBytes int) ([]byte, bool, error) {
|
||||
defer resp.Body.Close()
|
||||
|
||||
if resp.StatusCode >= 200 && resp.StatusCode < 300 {
|
||||
|
||||
@@ -554,3 +554,46 @@ func TestSetHTTPClient_NilRestoresDefault(t *testing.T) {
|
||||
t.Fatal("expected CheckRedirect policy after SetHTTPClient(nil)")
|
||||
}
|
||||
}
|
||||
|
||||
func TestSetHTTPClient_NilCheckRedirectEnforcesDefault(t *testing.T) {
|
||||
c := NewClient("token", "https://api.github.com")
|
||||
// Provide a client with nil CheckRedirect — should get default policy enforced.
|
||||
hc := &http.Client{Timeout: 5 * time.Second}
|
||||
c.SetHTTPClient(hc)
|
||||
if c.httpClient.CheckRedirect == nil {
|
||||
t.Fatal("expected CheckRedirect to be enforced when caller provides nil")
|
||||
}
|
||||
if c.httpClient.Timeout != 5*time.Second {
|
||||
t.Errorf("expected caller's timeout preserved, got %v", c.httpClient.Timeout)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSetHTTPClient_PreservesCustomCheckRedirect(t *testing.T) {
|
||||
c := NewClient("token", "https://api.github.com")
|
||||
called := false
|
||||
hc := &http.Client{
|
||||
CheckRedirect: func(req *http.Request, via []*http.Request) error {
|
||||
called = true
|
||||
return nil
|
||||
},
|
||||
}
|
||||
c.SetHTTPClient(hc)
|
||||
// Invoke the redirect to verify original is preserved
|
||||
_ = c.httpClient.CheckRedirect(nil, []*http.Request{{}})
|
||||
if !called {
|
||||
t.Fatal("expected custom CheckRedirect to be preserved")
|
||||
}
|
||||
}
|
||||
|
||||
func TestAPIError_SafeError(t *testing.T) {
|
||||
e := &APIError{StatusCode: 403, Body: "some sensitive body content"}
|
||||
got := e.SafeError()
|
||||
if got != "HTTP 403" {
|
||||
t.Errorf("SafeError() = %q, want %q", got, "HTTP 403")
|
||||
}
|
||||
// Ensure Error() still includes body
|
||||
full := e.Error()
|
||||
if full != "HTTP 403: some sensitive body content" {
|
||||
t.Errorf("Error() = %q, unexpected", full)
|
||||
}
|
||||
}
|
||||
|
||||
+1
-1
@@ -81,7 +81,7 @@ func (c *Client) ListContents(ctx context.Context, owner, repo, path string) ([]
|
||||
if err := json.Unmarshal(body, &entries); err != nil {
|
||||
var single entry
|
||||
if err2 := json.Unmarshal(body, &single); err2 != nil {
|
||||
return nil, fmt.Errorf("parse contents JSON: as array: %v; as object: %w", err, err2)
|
||||
return nil, fmt.Errorf("parse contents JSON: as array: %w; as object: %w", err, err2)
|
||||
}
|
||||
// Guard against empty objects ({}) or unexpected shapes that
|
||||
// unmarshal successfully but carry no useful data.
|
||||
|
||||
+20
-4
@@ -1,3 +1,5 @@
|
||||
//go:build phase2
|
||||
|
||||
package vcs_test
|
||||
|
||||
import (
|
||||
@@ -5,7 +7,21 @@ import (
|
||||
"gitea.weiker.me/rodin/review-bot/vcs"
|
||||
)
|
||||
|
||||
// Compile-time assertion: the gitea.Adapter satisfies vcs.Client.
|
||||
// (The raw gitea.Client does NOT satisfy vcs.Client due to signature differences;
|
||||
// the Adapter bridges them.)
|
||||
var _ vcs.Client = (*gitea.Adapter)(nil)
|
||||
// Compile-time assertion: documents the gap between gitea.Client and vcs.Client.
|
||||
// Guarded by the "phase2" build tag — enable once the Gitea adapter bridges these gaps:
|
||||
//
|
||||
// 1. PostReview signature mismatch:
|
||||
// gitea.Client: PostReview(ctx, owner, repo, number, event, body string, comments []gitea.ReviewComment)
|
||||
// vcs.Reviewer: PostReview(ctx, owner, repo, number, req vcs.ReviewRequest)
|
||||
//
|
||||
// 2. GetFileContent signature mismatch:
|
||||
// gitea.Client: GetFileContent(ctx, owner, repo, filepath string) [no ref; uses default branch]
|
||||
// vcs.FileReader: GetFileContent(ctx, owner, repo, path, ref string)
|
||||
// (gitea.Client has GetFileContentRef for the ref variant)
|
||||
//
|
||||
// 3. ReviewComment type mismatch:
|
||||
// gitea.ReviewComment uses NewPosition int64 (Gitea line-number convention)
|
||||
// vcs.ReviewComment uses Position int (GitHub diff-position convention)
|
||||
//
|
||||
// The Gitea adapter (Phase 2) will wrap gitea.Client to bridge these gaps.
|
||||
var _ vcs.Client = (*gitea.Client)(nil)
|
||||
|
||||
Reference in New Issue
Block a user