fix(gitea): handle single-object response in ListContents #74
@@ -434,6 +434,8 @@ type ContentEntry struct {
|
||||
|
||||
// ListContents lists files and directories at a given path in a repo.
|
||||
// Pass an empty path to list the repository root.
|
||||
// If the path points to a file (not a directory), Gitea returns a single
|
||||
// object instead of an array; this method normalizes both cases to a slice.
|
||||
func (c *Client) ListContents(ctx context.Context, owner, repo, path string) ([]ContentEntry, error) {
|
||||
// Normalize "." to empty string — Gitea API rejects "." with 500
|
||||
if path == "." {
|
||||
@@ -451,7 +453,12 @@ func (c *Client) ListContents(ctx context.Context, owner, repo, path string) ([]
|
||||
}
|
||||
var entries []ContentEntry
|
||||
if err := json.Unmarshal(body, &entries); err != nil {
|
||||
return nil, fmt.Errorf("parse contents JSON: %w", err)
|
||||
// Gitea returns a single object (not an array) when path is a file
|
||||
|
|
||||
var single ContentEntry
|
||||
gpt-review-bot
commented
[NIT] When both array and single-object unmarshals fail, the returned error wraps only the first error. Consider including details from the second error (e.g., via errors.Join or adding err2 to the message) to aid debugging malformed responses. **[NIT]** When both array and single-object unmarshals fail, the returned error wraps only the first error. Consider including details from the second error (e.g., via errors.Join or adding err2 to the message) to aid debugging malformed responses.
|
||||
if err2 := json.Unmarshal(body, &single); err2 != nil {
|
||||
sonnet-review-bot
commented
[MINOR] The fallback tries to unmarshal into a zero-value **[MINOR]** The fallback tries to unmarshal into a zero-value `ContentEntry`. If Gitea returns an empty JSON object `{}`, the unmarshal will succeed and return an entry with all zero/empty fields (`Name: "", Path: "", Type: ""`), which would be silently added to the results. This is an edge case but could cause confusing downstream behavior. A guard like `if single.Name == "" && single.Path == ""` before wrapping would be more defensive, though arguably over-engineering for this specific Gitea API.
sonnet-review-bot
commented
[NIT] The variable **[NIT]** The variable `err2` shadows the outer `err` in a way that's slightly awkward — `err` (from the array unmarshal) is preserved for the outer error return while `err2` is used only for the single-object attempt. The naming is fine, but a comment noting why `err` (not `err2`) is returned in the failure case would aid future readers: the original error is preferred because it describes the primary expected shape.
|
||||
return nil, fmt.Errorf("parse contents JSON: %w", err)
|
||||
}
|
||||
|
sonnet-review-bot
commented
[NIT] The guard condition **[NIT]** The guard condition `single.Name == "" && single.Path == ""` uses AND — a response where Name is populated but Path is empty (or vice versa) would pass the guard and produce a partially-valid entry. Using OR (`single.Name == "" || single.Path == ""`) would be more defensive, though in practice Gitea always returns both fields for a valid file entry.
|
||||
entries = []ContentEntry{single}
|
||||
}
|
||||
return entries, nil
|
||||
}
|
||||
|
||||
@@ -304,11 +304,40 @@ func TestListContents_DotPath(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestListContents_FilePath(t *testing.T) {
|
||||
// Gitea returns a single object (not an array) when path is a file
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.URL.Path != "/api/v1/repos/owner/repo/contents/README.md" {
|
||||
t.Errorf("unexpected path: %s", r.URL.Path)
|
||||
}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
// Single object, not an array
|
||||
fmt.Fprintf(w, `{"name":"README.md","path":"README.md","type":"file"}`)
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := NewClient(server.URL, "test-token")
|
||||
entries, err := client.ListContents(context.Background(), "owner", "repo", "README.md")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if len(entries) != 1 {
|
||||
t.Fatalf("expected 1 entry, got %d", len(entries))
|
||||
}
|
||||
if entries[0].Name != "README.md" {
|
||||
t.Errorf("expected README.md, got %s", entries[0].Name)
|
||||
}
|
||||
if entries[0].Type != "file" {
|
||||
t.Errorf("expected type file, got %s", entries[0].Type)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetAllFilesInPath_File(t *testing.T) {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.URL.Path == "/api/v1/repos/owner/repo/contents/README.md" {
|
||||
// Gitea returns 404 for contents API on files (it's not a dir)
|
||||
http.NotFound(w, r)
|
||||
// Gitea returns a single object (not array) when path is a file
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
fmt.Fprintf(w, `{"name":"README.md","path":"README.md","type":"file"}`)
|
||||
return
|
||||
}
|
||||
if r.URL.Path == "/api/v1/repos/owner/repo/raw/README.md" {
|
||||
|
||||
[NIT] The inner error
err2from the single-object unmarshal attempt is silently discarded when returning the original array-unmarshal error. In diagnostic scenarios this loses information about why both parses failed. Consider wrapping or loggingerr2, e.g.fmt.Errorf("parse contents JSON (array: %v; object: %v)", err, err2). Low priority since the original error is the more meaningful one.