Skip to content

Commit 4049a8d

Browse files
committed
refactor(client)!: pivot to context-only request building
Drop the legacy no-context BuildHTTP entry point and consolidate around BuildHTTPContext. Public-facing change: Runtime.CreateHttpRequest is deprecated in favor of the new Runtime.CreateHTTPRequestContext. Callers that set a per-request timeout via ClientRequestWriter must migrate, otherwise the cancel func is dropped and the timer leaks; previously such timeouts were silently ignored on this path (only Submit honored them), so the behavior change is explicit and documented inline. Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
1 parent 5115002 commit 4049a8d

5 files changed

Lines changed: 139 additions & 120 deletions

File tree

client/content_negotiation_test.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ func runBuildHTTPCases(t *testing.T, cases iter.Seq[buildHTTPCase]) {
9595

9696
func runBuildHTTPCase(tc buildHTTPCase) func(*testing.T) {
9797
return func(t *testing.T) {
98+
ctx := t.Context()
9899
method := tc.method
99100
if method == "" {
100101
method = http.MethodPost
@@ -110,7 +111,9 @@ func runBuildHTTPCase(tc buildHTTPCase) func(*testing.T) {
110111

111112
r := request.New(method, "/", writer)
112113
r.SetConsumes(tc.consumes)
113-
req, err := r.BuildHTTP(tc.mediaType, "/", producers, strfmt.Default, nil)
114+
req, cancel, err := r.BuildHTTPContext(ctx, tc.mediaType, "/", producers, strfmt.Default, nil)
115+
defer cancel()
116+
114117
if tc.wantErr != "" {
115118
require.Error(t, err)
116119
assert.Contains(t, err.Error(), tc.wantErr)

client/internal/request/consts_test.go

Lines changed: 0 additions & 14 deletions
This file was deleted.

client/internal/request/request.go

Lines changed: 24 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ var _ runtime.ClientRequest = new(Request) // ensure compliance to the interface
3737
//
3838
// # Request binding
3939
//
40-
// The binding of parameters is carried out by method [request.BuildHTTP].
40+
// The binding of parameters is carried out by method [Request.BuildHTTPContext].
4141
//
4242
// It analyzes parameters, which may come in different flavors:
4343
//
@@ -52,8 +52,8 @@ var _ runtime.ClientRequest = new(Request) // ensure compliance to the interface
5252
// - file, multipart form or io.Reader body: a streaming request with an attached go routine that consumes the [io.Reader].
5353
// - buffered body: a simple request
5454
//
55-
// In all cases, it is left to the caller to set the request's [context.Context]: [request.BuildHTTP] only builds
56-
// requests with [context.Background].
55+
// The caller passes the parent [context.Context] to [Request.BuildHTTPContext] and receives back a cancel
56+
// function to release the resources held by the derived request context once the response is consumed.
5757
//
5858
// # Authentication
5959
//
@@ -289,7 +289,9 @@ func (r *Request) SetConsumes(consumers []string) {
289289
r.consumes = consumers
290290
}
291291

292-
// BuildHTTP dispatches to one of two end-to-end builders based on whether:
292+
// BuildHTTPContext binds the request parameters and returns a ready-to-send [http.Request].
293+
//
294+
// Dispatch picks one of two end-to-end builders based on whether:
293295
//
294296
// - the body source is a stream (multipart pipe or stream payload)
295297
// - or a buffer (urlencoded form, producer output, or no body)
@@ -299,52 +301,46 @@ func (r *Request) SetConsumes(consumers []string) {
299301
//
300302
// The split mirrors the auth question: streaming bodies require a lazy body-copy closure during AuthenticateRequest,
301303
// whereas buffered bodies do not.
302-
func (r *Request) BuildHTTP(mediaType, basePath string, producers map[string]runtime.Producer, registry strfmt.Registry, auth runtime.ClientAuthInfoWriter) (*http.Request, error) {
303-
if err := r.writer.WriteToRequest(r, registry); err != nil {
304-
return nil, err
305-
}
306-
return r.assembleHTTPBody(context.Background(), mediaType, basePath, producers, registry, auth)
307-
}
308-
309-
// BuildHTTPContext is the context-aware variant of [Request.BuildHTTP].
310304
//
311305
// The returned [http.Request] carries a context derived from parentCtx that:
312306
//
313307
// - inherits any deadline or cancellation already set on parentCtx;
314308
// - additionally honors the per-request timeout set via [Request.SetTimeout]
315-
// (the [ClientRequestWriter] may override the runtime default during
309+
// (the [runtime.ClientRequestWriter] may override the runtime default during
316310
// WriteToRequest, which is why the derivation happens here rather than
317311
// at the call site).
318312
//
319313
// The returned cancel must be invoked by the caller (typically deferred)
320314
// once the response has been fully read; otherwise resources held by the
321315
// derived context — including any timeout timer — are leaked.
322316
//
323-
// On error the cancel is invoked internally and a nil cancel is returned.
317+
// On error the cancel is invoked internally and a no-op cancel is returned,
318+
// so callers can defer cancel unconditionally.
324319
func (r *Request) BuildHTTPContext(parentCtx context.Context, mediaType, basePath string, producers map[string]runtime.Producer, registry strfmt.Registry, auth runtime.ClientAuthInfoWriter) (*http.Request, context.CancelFunc, error) {
325320
if err := r.writer.WriteToRequest(r, registry); err != nil {
326-
return nil, nil, err
321+
return nil, noop, err
327322
}
323+
328324
ctx, cancel := deriveRequestContext(parentCtx, r.timeout)
329-
httpReq, err := r.assembleHTTPBody(ctx, mediaType, basePath, producers, registry, auth)
325+
r.buf = bytes.NewBuffer(nil)
326+
327+
var (
328+
httpReq *http.Request
329+
err error
330+
)
331+
if r.usesStreamingBody(mediaType) {
332+
httpReq, err = r.buildStreamingRequest(ctx, mediaType, basePath, producers, registry, auth)
333+
} else {
334+
httpReq, err = r.buildBufferedRequest(ctx, mediaType, basePath, producers, registry, auth)
335+
}
330336
if err != nil {
331337
cancel()
332-
return nil, nil, err
338+
return nil, noop, err
333339
}
334340
return httpReq, cancel, nil
335341
}
336342

337-
// assembleHTTPBody is the post-WriteToRequest path shared by [Request.BuildHTTP]
338-
// and [Request.BuildHTTPContext]: it dispatches to the buffered or streaming
339-
// flow and threads ctx down to [http.NewRequestWithContext].
340-
func (r *Request) assembleHTTPBody(ctx context.Context, mediaType, basePath string, producers map[string]runtime.Producer, registry strfmt.Registry, auth runtime.ClientAuthInfoWriter) (*http.Request, error) {
341-
r.buf = bytes.NewBuffer(nil)
342-
343-
if r.usesStreamingBody(mediaType) {
344-
return r.buildStreamingRequest(ctx, mediaType, basePath, producers, registry, auth)
345-
}
346-
return r.buildBufferedRequest(ctx, mediaType, basePath, producers, registry, auth)
347-
}
343+
func noop() {}
348344

349345
// deriveRequestContext returns a child of parent bounded by timeout.
350346
// If timeout == 0 the child is only canceled when the caller invokes

0 commit comments

Comments
 (0)