From 15f9a2c7cd2fb41aeeb198271d9808286cfc578a Mon Sep 17 00:00:00 2001 From: Joel Hendrix Date: Mon, 21 Aug 2023 10:04:48 -0700 Subject: [PATCH] Customer APIs should not be in the runtime package The runtime package is intended for consumption by SDK authors and generated code. Renamed context key to match its helper function. Added examples some missing examples. --- sdk/azcore/CHANGELOG.md | 5 ++ sdk/azcore/internal/shared/shared.go | 4 +- sdk/azcore/policy/examples_test.go | 46 ++++++++++++++++ sdk/azcore/policy/policy.go | 21 ++++++++ sdk/azcore/policy/policy_test.go | 53 +++++++++++++++++++ sdk/azcore/runtime/examples_test.go | 25 --------- sdk/azcore/runtime/policy_http_header.go | 3 +- sdk/azcore/runtime/policy_include_response.go | 5 +- .../runtime/policy_include_response_test.go | 2 +- sdk/azcore/runtime/policy_retry.go | 3 +- 10 files changed, 135 insertions(+), 32 deletions(-) create mode 100644 sdk/azcore/policy/examples_test.go create mode 100644 sdk/azcore/policy/policy_test.go delete mode 100644 sdk/azcore/runtime/examples_test.go diff --git a/sdk/azcore/CHANGELOG.md b/sdk/azcore/CHANGELOG.md index 8987eab09015..7fb01df91409 100644 --- a/sdk/azcore/CHANGELOG.md +++ b/sdk/azcore/CHANGELOG.md @@ -12,6 +12,11 @@ ### Other Changes +* The following functions in the `runtime` package are now exposed from the `policy` package, and the `runtime` versions have been deprecated. + * `WithCaptureResponse` + * `WithHTTPHeader` + * `WithRetryOptions` + ## 1.8.0-beta.2 (2023-08-14) ### Features Added diff --git a/sdk/azcore/internal/shared/shared.go b/sdk/azcore/internal/shared/shared.go index f347072f926f..b81b84672c28 100644 --- a/sdk/azcore/internal/shared/shared.go +++ b/sdk/azcore/internal/shared/shared.go @@ -24,8 +24,8 @@ type CtxWithHTTPHeaderKey struct{} // CtxWithRetryOptionsKey is used as a context key for adding/retrieving RetryOptions. type CtxWithRetryOptionsKey struct{} -// CtxIncludeResponseKey is used as a context key for retrieving the raw response. -type CtxIncludeResponseKey struct{} +// CtxWithCaptureResponse is used as a context key for retrieving the raw response. +type CtxWithCaptureResponse struct{} // CtxWithTracingTracer is used as a context key for adding/retrieving tracing.Tracer. type CtxWithTracingTracer struct{} diff --git a/sdk/azcore/policy/examples_test.go b/sdk/azcore/policy/examples_test.go new file mode 100644 index 000000000000..6b056f119c68 --- /dev/null +++ b/sdk/azcore/policy/examples_test.go @@ -0,0 +1,46 @@ +//go:build go1.16 +// +build go1.16 + +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package policy_test + +import ( + "context" + "net/http" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" +) + +func ExampleWithCaptureResponse() { + // policy.WithCaptureResponse provides a mechanism for obtaining an API's underlying *http.Response + var respFromCtx *http.Response + ctx := policy.WithCaptureResponse(context.TODO(), &respFromCtx) + // make some client method call using the updated context + // resp, err := client.SomeMethod(ctx, ...) + // *respFromCtx contains the raw *http.Response returned during the client method call. + // if the HTTP transport didn't return a response due to an error then *respFromCtx will be nil. + _ = ctx +} + +func ExampleWithHTTPHeader() { + // policy.WithHTTPHeader allows callers to augment API calls with custom headers + customHeaders := http.Header{} + customHeaders.Add("key", "value") + ctx := policy.WithHTTPHeader(context.TODO(), customHeaders) + // make some client method call using the updated context + // resp, err := client.SomeMethod(ctx, ...) + // the underlying HTTP request will include the values in customHeaders + _ = ctx +} + +func ExampleWithRetryOptions() { + // policy.WithRetryOptions contains a [policy.RetryOptions] that can be used to customize the retry policy on a per-API call basis + customRetryOptions := policy.RetryOptions{ /* populate with custom values */ } + ctx := policy.WithRetryOptions(context.TODO(), customRetryOptions) + // make some client method call using the updated context + // resp, err := client.SomeMethod(ctx, ...) + // the behavior of the retry policy will correspond to the values provided in customRetryPolicy + _ = ctx +} diff --git a/sdk/azcore/policy/policy.go b/sdk/azcore/policy/policy.go index 2d7ad45f6167..2a6229305a75 100644 --- a/sdk/azcore/policy/policy.go +++ b/sdk/azcore/policy/policy.go @@ -7,11 +7,13 @@ package policy import ( + "context" "net/http" "time" "github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud" "github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/shared" "github.com/Azure/azure-sdk-for-go/sdk/azcore/tracing" ) @@ -163,3 +165,22 @@ type AuthorizationHandler struct { // the policy will return any 401 response to the client. OnChallenge func(*Request, *http.Response, func(TokenRequestOptions) error) error } + +// WithCaptureResponse applies the HTTP response retrieval annotation to the parent context. +// The resp parameter will contain the HTTP response after the request has completed. +func WithCaptureResponse(parent context.Context, resp **http.Response) context.Context { + return context.WithValue(parent, shared.CtxWithCaptureResponse{}, resp) +} + +// WithHTTPHeader adds the specified http.Header to the parent context. +// Use this to specify custom HTTP headers at the API-call level. +// Any overlapping headers will have their values replaced with the values specified here. +func WithHTTPHeader(parent context.Context, header http.Header) context.Context { + return context.WithValue(parent, shared.CtxWithHTTPHeaderKey{}, header) +} + +// WithRetryOptions adds the specified RetryOptions to the parent context. +// Use this to specify custom RetryOptions at the API-call level. +func WithRetryOptions(parent context.Context, options RetryOptions) context.Context { + return context.WithValue(parent, shared.CtxWithRetryOptionsKey{}, options) +} diff --git a/sdk/azcore/policy/policy_test.go b/sdk/azcore/policy/policy_test.go new file mode 100644 index 000000000000..72701ba1aaad --- /dev/null +++ b/sdk/azcore/policy/policy_test.go @@ -0,0 +1,53 @@ +//go:build go1.18 +// +build go1.18 + +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package policy + +import ( + "context" + "math" + "net/http" + "testing" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/shared" + "github.com/stretchr/testify/require" +) + +func TestWithCaptureResponse(t *testing.T) { + var httpResp *http.Response + ctx := WithCaptureResponse(context.Background(), &httpResp) + require.NotNil(t, ctx) + raw := ctx.Value(shared.CtxWithCaptureResponse{}) + resp, ok := raw.(**http.Response) + require.True(t, ok) + require.Same(t, &httpResp, resp) +} + +func TestWithHTTPHeader(t *testing.T) { + const ( + key = "some" + val = "thing" + ) + input := http.Header{} + input.Set(key, val) + ctx := WithHTTPHeader(context.Background(), input) + require.NotNil(t, ctx) + raw := ctx.Value(shared.CtxWithHTTPHeaderKey{}) + header, ok := raw.(http.Header) + require.True(t, ok) + require.EqualValues(t, val, header.Get(key)) +} + +func TestWithRetryOptions(t *testing.T) { + ctx := WithRetryOptions(context.Background(), RetryOptions{ + MaxRetries: math.MaxInt32, + }) + require.NotNil(t, ctx) + raw := ctx.Value(shared.CtxWithRetryOptionsKey{}) + opts, ok := raw.(RetryOptions) + require.True(t, ok) + require.EqualValues(t, math.MaxInt32, opts.MaxRetries) +} diff --git a/sdk/azcore/runtime/examples_test.go b/sdk/azcore/runtime/examples_test.go deleted file mode 100644 index 425e9e3fbbdf..000000000000 --- a/sdk/azcore/runtime/examples_test.go +++ /dev/null @@ -1,25 +0,0 @@ -//go:build go1.16 -// +build go1.16 - -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -package runtime_test - -import ( - "context" - "net/http" - - "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" -) - -func ExampleWithCaptureResponse() { - var respFromCtx *http.Response - ctx := context.TODO() - ctx = runtime.WithCaptureResponse(ctx, &respFromCtx) - // make some client method call using the updated context - // resp, err := client.SomeMethod(ctx, ...) - // *respFromCtx contains the raw *http.Response returned during the client method call. - // if the HTTP transport didn't return a response due to an error then *respFromCtx will be nil. - _ = ctx -} diff --git a/sdk/azcore/runtime/policy_http_header.go b/sdk/azcore/runtime/policy_http_header.go index 770e0a2b6a64..c230af0afa89 100644 --- a/sdk/azcore/runtime/policy_http_header.go +++ b/sdk/azcore/runtime/policy_http_header.go @@ -34,6 +34,7 @@ func httpHeaderPolicy(req *policy.Request) (*http.Response, error) { // WithHTTPHeader adds the specified http.Header to the parent context. // Use this to specify custom HTTP headers at the API-call level. // Any overlapping headers will have their values replaced with the values specified here. +// Deprecated: use [policy.WithHTTPHeader] instead. func WithHTTPHeader(parent context.Context, header http.Header) context.Context { - return context.WithValue(parent, shared.CtxWithHTTPHeaderKey{}, header) + return policy.WithHTTPHeader(parent, header) } diff --git a/sdk/azcore/runtime/policy_include_response.go b/sdk/azcore/runtime/policy_include_response.go index 4714baa30cd6..bb00f6c2fdb7 100644 --- a/sdk/azcore/runtime/policy_include_response.go +++ b/sdk/azcore/runtime/policy_include_response.go @@ -20,7 +20,7 @@ func includeResponsePolicy(req *policy.Request) (*http.Response, error) { if resp == nil { return resp, err } - if httpOutRaw := req.Raw().Context().Value(shared.CtxIncludeResponseKey{}); httpOutRaw != nil { + if httpOutRaw := req.Raw().Context().Value(shared.CtxWithCaptureResponse{}); httpOutRaw != nil { httpOut := httpOutRaw.(**http.Response) *httpOut = resp } @@ -29,6 +29,7 @@ func includeResponsePolicy(req *policy.Request) (*http.Response, error) { // WithCaptureResponse applies the HTTP response retrieval annotation to the parent context. // The resp parameter will contain the HTTP response after the request has completed. +// Deprecated: use [policy.WithCaptureResponse] instead. func WithCaptureResponse(parent context.Context, resp **http.Response) context.Context { - return context.WithValue(parent, shared.CtxIncludeResponseKey{}, resp) + return policy.WithCaptureResponse(parent, resp) } diff --git a/sdk/azcore/runtime/policy_include_response_test.go b/sdk/azcore/runtime/policy_include_response_test.go index 7c3ed73b5c5a..afa8b881410d 100644 --- a/sdk/azcore/runtime/policy_include_response_test.go +++ b/sdk/azcore/runtime/policy_include_response_test.go @@ -21,7 +21,7 @@ func TestIncludeResponse(t *testing.T) { var respFromCtx *http.Response ctx := WithCaptureResponse(context.Background(), &respFromCtx) require.NotNil(t, ctx) - raw := ctx.Value(shared.CtxIncludeResponseKey{}) + raw := ctx.Value(shared.CtxWithCaptureResponse{}) _, ok := raw.(**http.Response) require.Truef(t, ok, "unexpected type %T", raw) require.Nil(t, respFromCtx) diff --git a/sdk/azcore/runtime/policy_retry.go b/sdk/azcore/runtime/policy_retry.go index 09fb3233faeb..04d7bb4ecbc6 100644 --- a/sdk/azcore/runtime/policy_retry.go +++ b/sdk/azcore/runtime/policy_retry.go @@ -201,8 +201,9 @@ func (p *retryPolicy) Do(req *policy.Request) (resp *http.Response, err error) { // WithRetryOptions adds the specified RetryOptions to the parent context. // Use this to specify custom RetryOptions at the API-call level. +// Deprecated: use [policy.WithRetryOptions] instead. func WithRetryOptions(parent context.Context, options policy.RetryOptions) context.Context { - return context.WithValue(parent, shared.CtxWithRetryOptionsKey{}, options) + return policy.WithRetryOptions(parent, options) } // ********** The following type/methods implement the retryableRequestBody (a ReadSeekCloser)