client: Client.buildRequest, jsonEncode improve handling of content

- add early returns for `nil` body, `http.NoBody`, and `json.RawMessage`
- use `http.NoBody` instead of `nil` for empty bodies; it's more clear
  on intent.
- use json.Encode instead of json.Encoder.Encode(), as we're marshaling
  a single JSON document; this also avoid adding a trailing newline.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

client/request.go

@@ -67,31 +67,22 @@ func (cli *Client) delete(ctx context.Context, path string, query url.Values, he
 // prepareJSONRequest encodes the given body to JSON and returns it as an [io.Reader], and sets the Content-Type
 // header. If body is nil, or a nil-interface, a "nil" body is returned without
 // error.
-//
-// TODO(thaJeztah): should this return an error if a different Content-Type is already set?
-// TODO(thaJeztah): is "nil" the appropriate approach for an empty body, or should we use [http.NoBody] (or similar)?
 func prepareJSONRequest(body any, headers http.Header) (io.Reader, http.Header, error) {
-	if body == nil {
-		return nil, headers, nil
-	}
-	// encoding/json encodes a nil pointer as the JSON document `null`,
-	// irrespective of whether the type implements json.Marshaler or encoding.TextMarshaler.
-	// That is almost certainly not what the caller intended as the request body.
-	//
-	// TODO(thaJeztah): consider moving this to jsonEncode, which would also allow returning an (empty) reader instead of nil.
-	if reflect.TypeOf(body).Kind() == reflect.Ptr && reflect.ValueOf(body).IsNil() {
-		return nil, headers, nil
-	}
-
 	jsonBody, err := jsonEncode(body)
 	if err != nil {
 		return nil, headers, err
 	}
+	if jsonBody == nil || jsonBody == http.NoBody {
+		// no content-type is set on empty requests.
+		return jsonBody, headers, nil
+	}
+
 	hdr := http.Header{}
 	if headers != nil {
 		hdr = headers.Clone()
 	}
 
+	// TODO(thaJeztah): should this return an error if a different Content-Type is already set?
 	hdr.Set("Content-Type", "application/json")
 	return jsonBody, hdr, nil
 }
@@ -330,13 +321,31 @@ func (cli *Client) addHeaders(req *http.Request, headers http.Header) *http.Requ
 }
 
 func jsonEncode(data any) (io.Reader, error) {
-	var params bytes.Buffer
-	if data != nil {
-		if err := json.NewEncoder(&params).Encode(data); err != nil {
-			return nil, err
+	switch x := data.(type) {
+	case nil:
+		return http.NoBody, nil
+	case io.Reader:
+		// http.NoBody or other readers
+		return x, nil
+	case json.RawMessage:
+		if len(x) == 0 {
+			return http.NoBody, nil
 		}
+		return bytes.NewReader(x), nil
 	}
-	return &params, nil
+
+	// encoding/json encodes a nil pointer as the JSON document `null`,
+	// irrespective of whether the type implements json.Marshaler or encoding.TextMarshaler.
+	// That is almost certainly not what the caller intended as the request body.
+	if v := reflect.ValueOf(data); v.Kind() == reflect.Ptr && v.IsNil() {
+		return http.NoBody, nil
+	}
+
+	b, err := json.Marshal(data)
+	if err != nil {
+		return nil, err
+	}
+	return bytes.NewReader(b), nil
 }
 
 func ensureReaderClosed(response *http.Response) {

client/request_test.go

@@ -7,7 +7,6 @@ import (
 	"io"
 	"math/rand"
 	"net/http"
-	"strings"
 	"testing"
 	"time"
 
@@ -266,26 +265,25 @@ func TestPrepareJSONRequest(t *testing.T) {
 		body       any
 		headers    http.Header
 		expBody    string
-		expNilBody bool
 		expHeaders http.Header
 	}{
 		{
-			doc:        "nil body",
-			body:       nil,
-			headers:    http.Header{"Something": []string{"something"}},
-			expNilBody: true,
+			doc:     "nil body",
+			body:    nil,
+			headers: http.Header{"Something": []string{"something"}},
+			expBody: "",
 			expHeaders: http.Header{
-				// currently, no content-type is set on empty requests.
+				// no content-type is set on empty requests.
 				"Something": []string{"something"},
 			},
 		},
 		{
-			doc:        "nil interface body",
-			body:       (*struct{})(nil),
-			headers:    http.Header{"Something": []string{"something"}},
-			expNilBody: true,
+			doc:     "nil interface body",
+			body:    (*struct{})(nil),
+			headers: http.Header{"Something": []string{"something"}},
+			expBody: "",
 			expHeaders: http.Header{
-				// currently, no content-type is set on empty requests.
+				// no content-type is set on empty requests.
 				"Something": []string{"something"},
 			},
 		},
@@ -308,12 +306,16 @@ func TestPrepareJSONRequest(t *testing.T) {
 			},
 		},
 		{
-			doc:     "empty body",
-			body:    http.NoBody,
-			expBody: `{}`,
-			expHeaders: http.Header{
-				"Content-Type": []string{"application/json"},
-			},
+			doc:        "empty json raw message",
+			body:       json.RawMessage(""),
+			expBody:    "",
+			expHeaders: nil, // no content-type is set on empty requests.
+		},
+		{
+			doc:        "empty body",
+			body:       http.NoBody,
+			expBody:    "",
+			expHeaders: nil, // no content-type is set on empty requests.
 		},
 	}
 
@@ -322,16 +324,9 @@ func TestPrepareJSONRequest(t *testing.T) {
 			req, hdr, err := prepareJSONRequest(tc.body, tc.headers)
 			assert.NilError(t, err)
 
-			var body string
-			if tc.expNilBody {
-				assert.Check(t, is.Nil(req))
-			} else {
-				assert.Assert(t, req != nil)
-
-				resp, err := io.ReadAll(req)
-				assert.NilError(t, err)
-				body = strings.TrimSpace(string(resp))
-			}
+			resp, err := io.ReadAll(req)
+			assert.NilError(t, err)
+			body := string(resp)
 
 			assert.Check(t, is.Equal(body, tc.expBody))
 			assert.Check(t, is.DeepEqual(hdr, tc.expHeaders))