Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions go/pkg/basecamp/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -365,8 +365,9 @@ func (c *Client) initGeneratedClient() {
return err
}
req.Header.Set("User-Agent", c.userAgent)
// Only set Content-Type if not already set (preserves binary upload content types)
if req.Header.Get("Content-Type") == "" {
// Content-Type describes a request body, so set the JSON default only
// when a body is present and the caller has not already set one.
if requestHasBody(req) && req.Header.Get("Content-Type") == "" {
req.Header.Set("Content-Type", "application/json")
}
req.Header.Set("Accept", "application/json")
Expand All @@ -382,6 +383,10 @@ func (c *Client) initGeneratedClient() {
})
}

func requestHasBody(req *http.Request) bool {
return req != nil && req.Body != nil && req.Body != http.NoBody
}

// discardHandler is a slog.Handler that discards all log records.
type discardHandler struct{}

Expand Down Expand Up @@ -683,7 +688,9 @@ func (c *Client) singleRequest(ctx context.Context, method, url string, body any
return nil, err
}
req.Header.Set("User-Agent", c.userAgent)
req.Header.Set("Content-Type", "application/json")
if requestHasBody(req) && req.Header.Get("Content-Type") == "" {
req.Header.Set("Content-Type", "application/json")
}
req.Header.Set("Accept", "application/json")

// Add ETag for cached GET requests. Derive cache key from the Authorization
Expand Down
94 changes: 94 additions & 0 deletions go/pkg/basecamp/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,25 @@ package basecamp
import (
"context"
"encoding/json"
"errors"
"net/http"
"net/http/httptest"
"strings"
"testing"
)

type contentTypeAuthStrategy struct {
contentType string
}

func (s contentTypeAuthStrategy) Authenticate(_ context.Context, req *http.Request) error {
if req.Header.Get("Content-Type") != "" {
return errors.New("expected Content-Type to be empty before auth strategy sets it")
}
req.Header.Set("Content-Type", s.contentType)
return nil
}
Comment thread
robzolkos marked this conversation as resolved.

func TestSingleRequest_204ReturnsValidJSON(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusNoContent)
Expand Down Expand Up @@ -125,6 +138,87 @@ func TestSingleRequest_200WithBody(t *testing.T) {
}
}

func TestSingleRequest_GETDoesNotSetContentType(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodGet {
t.Errorf("expected GET, got %s", r.Method)
http.Error(w, "wrong method", http.StatusBadRequest)
return
}
Comment thread
robzolkos marked this conversation as resolved.
if got := r.Header.Get("Content-Type"); got != "" {
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
t.Errorf("expected no Content-Type for bodyless GET, got %q", got)
}
if got := r.Header.Get("Accept"); got != "application/json" {
t.Errorf("expected Accept application/json, got %q", got)
}
Comment thread
robzolkos marked this conversation as resolved.
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte(`{"ok":true}`))
}))
defer server.Close()

cfg := &Config{BaseURL: server.URL, CacheEnabled: false}
client := NewClient(cfg, &StaticTokenProvider{Token: "test-token"})

if _, err := client.Get(context.Background(), "/test.json"); err != nil {
t.Fatalf("unexpected error: %v", err)
}
}

func TestSingleRequest_POSTSetsContentTypeForJSONBody(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost {
t.Errorf("expected POST, got %s", r.Method)
http.Error(w, "wrong method", http.StatusBadRequest)
return
}
if got := r.Header.Get("Content-Type"); got != "application/json" {
t.Errorf("expected Content-Type application/json, got %q", got)
}
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte(`{"ok":true}`))
}))
defer server.Close()

cfg := &Config{BaseURL: server.URL, CacheEnabled: false}
client := NewClient(cfg, &StaticTokenProvider{Token: "test-token"})

if _, err := client.Post(context.Background(), "/test.json", map[string]any{"ok": true}); err != nil {
t.Fatalf("unexpected error: %v", err)
}
}

func TestSingleRequest_POSTPreservesExistingContentType(t *testing.T) {
const customContentType = "application/merge-patch+json"

server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost {
t.Errorf("expected POST, got %s", r.Method)
http.Error(w, "wrong method", http.StatusBadRequest)
return
}
if got := r.Header.Get("Content-Type"); got != customContentType {
t.Errorf("expected Content-Type %q, got %q", customContentType, got)
}
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte(`{"ok":true}`))
}))
defer server.Close()

cfg := &Config{BaseURL: server.URL, CacheEnabled: false}
client := NewClient(
cfg,
&StaticTokenProvider{Token: "test-token"},
WithAuthStrategy(contentTypeAuthStrategy{contentType: customContentType}),
)

if _, err := client.Post(context.Background(), "/test.json", map[string]any{"ok": true}); err != nil {
t.Fatalf("unexpected error: %v", err)
}
}
Comment thread
robzolkos marked this conversation as resolved.

func TestSingleRequest_204Delete(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodDelete {
Expand Down
28 changes: 28 additions & 0 deletions go/pkg/basecamp/search_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,3 +286,31 @@ func TestSearchService_Search_NoSort(t *testing.T) {
t.Fatalf("unexpected error: %v", err)
}
}

func TestSearchService_Search_DoesNotSetContentTypeOnGet(t *testing.T) {
fixture := loadSearchFixture(t, "results.json")
svc := testSearchServer(t, func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodGet {
t.Errorf("expected GET, got %s", r.Method)
http.Error(w, "wrong method", http.StatusBadRequest)
return
}
Comment thread
robzolkos marked this conversation as resolved.
if got := r.Header.Get("Content-Type"); got != "" {
t.Errorf("expected no Content-Type for bodyless GET, got %q", got)
}
if got := r.Header.Get("Accept"); got != "application/json" {
t.Errorf("expected Accept application/json, got %q", got)
}
Comment thread
robzolkos marked this conversation as resolved.
if got := r.URL.Query().Get("q"); got != "leto" {
t.Errorf("expected q=leto, got %q", got)
}
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)
_, _ = w.Write(fixture)
})

_, err := svc.Search(context.Background(), "leto", nil)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
}
Loading