diff --git a/internal/http_client.go b/internal/http_client.go index 9d1257bc..2a5b22b2 100644 --- a/internal/http_client.go +++ b/internal/http_client.go @@ -29,6 +29,7 @@ import ( "time" "google.golang.org/api/option" + "google.golang.org/api/option/internaloption" "google.golang.org/api/transport" ) @@ -65,7 +66,13 @@ func NewHTTPClient(ctx context.Context, opts ...option.ClientOption) (*HTTPClien return nil, "", err } - return WithDefaultRetryConfig(hc), endpoint, nil + client := &HTTPClient{Client: hc} + if retryConfig, ok := retryConfigFromOptions(opts...); ok { + client.RetryConfig = retryConfig + } else { + client.RetryConfig = defaultRetryConfig() + } + return client, endpoint, nil } // WithDefaultRetryConfig creates a new HTTPClient using the provided client and the default @@ -75,17 +82,44 @@ func NewHTTPClient(ctx context.Context, opts ...option.ClientOption) (*HTTPClien // ServiceUnavailable (503) error. Repeatedly failing requests are retried up to 4 times // with exponential backoff. Retry delay is never longer than 2 minutes. func WithDefaultRetryConfig(hc *http.Client) *HTTPClient { - twoMinutes := time.Duration(2) * time.Minute return &HTTPClient{ - Client: hc, - RetryConfig: &RetryConfig{ - MaxRetries: 4, - CheckForRetry: retryNetworkAndHTTPErrors( - http.StatusServiceUnavailable, - ), - ExpBackoffFactor: 0.5, - MaxDelay: &twoMinutes, - }, + Client: hc, + RetryConfig: defaultRetryConfig(), + } +} + +// CloneHTTPClient returns a copy of the given HTTPClient. +// +// Slice and pointer fields are copied to avoid accidental cross-client mutations. +func CloneHTTPClient(client *HTTPClient) *HTTPClient { + if client == nil { + return nil + } + + clone := *client + if client.Opts != nil { + clone.Opts = append([]HTTPOption{}, client.Opts...) + } + if client.RetryConfig != nil { + retryConfig := *client.RetryConfig + if client.RetryConfig.MaxDelay != nil { + maxDelay := *client.RetryConfig.MaxDelay + retryConfig.MaxDelay = &maxDelay + } + clone.RetryConfig = &retryConfig + } + + return &clone +} + +// WithRetryConfig creates a ClientOption that can be used to configure HTTP retries. +// +// The option can be passed into NewApp() and is propagated to service clients. +// If this option is provided with a nil RetryConfig, retries are disabled. +func WithRetryConfig(retryConfig *RetryConfig) option.ClientOption { + return &withRetryConfigOption{ + EmbeddableAdapter: &internaloption.EmbeddableAdapter{}, + retryConfig: retryConfig, } } @@ -371,6 +405,41 @@ type RetryConfig struct { MaxDelay *time.Duration } +type withRetryConfigOption struct { + *internaloption.EmbeddableAdapter + retryConfig *RetryConfig +} + +func (w *withRetryConfigOption) getRetryConfig() (*RetryConfig, bool) { + return w.retryConfig, true +} + +type retryConfigOption interface { + getRetryConfig() (*RetryConfig, bool) +} + +func retryConfigFromOptions(opts ...option.ClientOption) (*RetryConfig, bool) { + for idx := len(opts) - 1; idx >= 0; idx-- { + if rcOpt, ok := opts[idx].(retryConfigOption); ok { + return rcOpt.getRetryConfig() + } + } + + return nil, false +} + +func defaultRetryConfig() *RetryConfig { + twoMinutes := time.Duration(2) * time.Minute + return &RetryConfig{ + MaxRetries: 4, + CheckForRetry: retryNetworkAndHTTPErrors( + http.StatusServiceUnavailable, + ), + ExpBackoffFactor: 0.5, + MaxDelay: &twoMinutes, + } +} + // RetryCondition determines if an HTTP request should be retried depending on its last outcome. type RetryCondition func(resp *http.Response, networkErr error) bool diff --git a/internal/http_client_test.go b/internal/http_client_test.go index 22b498cf..710afe98 100644 --- a/internal/http_client_test.go +++ b/internal/http_client_test.go @@ -693,6 +693,79 @@ func TestNewHTTPClient(t *testing.T) { } } +func TestNewHTTPClientWithRetryConfigOption(t *testing.T) { + wantRetry := &RetryConfig{ + MaxRetries: 1, + ExpBackoffFactor: 1.25, + } + client, _, err := NewHTTPClient( + context.Background(), + tokenSourceOpt, + WithRetryConfig(wantRetry), + ) + if err != nil { + t.Fatal(err) + } + if client.RetryConfig != wantRetry { + t.Errorf("NewHTTPClient().RetryConfig = %p; want = %p", client.RetryConfig, wantRetry) + } +} + +func TestNewHTTPClientWithRetryConfigOptionNil(t *testing.T) { + client, _, err := NewHTTPClient( + context.Background(), + tokenSourceOpt, + WithRetryConfig(nil), + ) + if err != nil { + t.Fatal(err) + } + if client.RetryConfig != nil { + t.Errorf("NewHTTPClient().RetryConfig = %v; want = nil", client.RetryConfig) + } +} + +func TestCloneHTTPClient(t *testing.T) { + delay := 5 * time.Second + original := &HTTPClient{ + Client: &http.Client{}, + RetryConfig: &RetryConfig{ + MaxRetries: 3, + CheckForRetry: retryNetworkAndHTTPErrors(http.StatusServiceUnavailable), + ExpBackoffFactor: 0.5, + MaxDelay: &delay, + }, + Opts: []HTTPOption{ + WithHeader("X-Test", "value"), + }, + } + + cloned := CloneHTTPClient(original) + if cloned == original { + t.Fatalf("CloneHTTPClient() returned the original instance") + } + if len(cloned.Opts) != len(original.Opts) { + t.Fatalf("len(Opts) = %d; want = %d", len(cloned.Opts), len(original.Opts)) + } + if cloned.RetryConfig == original.RetryConfig { + t.Errorf("RetryConfig pointer should be copied") + } + if cloned.RetryConfig.MaxDelay == original.RetryConfig.MaxDelay { + t.Errorf("RetryConfig.MaxDelay pointer should be copied") + } + + cloned.Opts = append(cloned.Opts, WithHeader("X-Test-2", "value-2")) + if len(original.Opts) != 1 { + t.Errorf("len(original.Opts) = %d; want = 1", len(original.Opts)) + } + + newDelay := 10 * time.Second + cloned.RetryConfig.MaxDelay = &newDelay + if *original.RetryConfig.MaxDelay != delay { + t.Errorf("original RetryConfig.MaxDelay = %v; want = %v", *original.RetryConfig.MaxDelay, delay) + } +} + func TestNewHTTPClientRetryOnNetworkErrors(t *testing.T) { client, _, err := NewHTTPClient(context.Background(), tokenSourceOpt) if err != nil { diff --git a/messaging/messaging.go b/messaging/messaging.go index 67b94f66..94e138cd 100644 --- a/messaging/messaging.go +++ b/messaging/messaging.go @@ -28,7 +28,6 @@ import ( "time" "firebase.google.com/go/v4/internal" - "google.golang.org/api/transport" ) const ( @@ -910,7 +909,7 @@ func NewClient(ctx context.Context, c *internal.MessagingConfig) (*Client, error return nil, errors.New("project ID is required to access Firebase Cloud Messaging client") } - hc, messagingEndpoint, err := transport.NewHTTPClient(ctx, c.Opts...) + baseHTTPClient, messagingEndpoint, err := internal.NewHTTPClient(ctx, c.Opts...) if err != nil { return nil, err } @@ -923,8 +922,8 @@ func NewClient(ctx context.Context, c *internal.MessagingConfig) (*Client, error } return &Client{ - fcmClient: newFCMClient(hc, c, messagingEndpoint, batchEndpoint), - iidClient: newIIDClient(hc, c), + fcmClient: newFCMClient(baseHTTPClient, c, messagingEndpoint, batchEndpoint), + iidClient: newIIDClient(baseHTTPClient, c), }, nil } @@ -936,16 +935,16 @@ type fcmClient struct { httpClient *internal.HTTPClient } -func newFCMClient(hc *http.Client, conf *internal.MessagingConfig, messagingEndpoint string, batchEndpoint string) *fcmClient { - client := internal.WithDefaultRetryConfig(hc) +func newFCMClient(base *internal.HTTPClient, conf *internal.MessagingConfig, messagingEndpoint string, batchEndpoint string) *fcmClient { + client := internal.CloneHTTPClient(base) client.CreateErrFn = handleFCMError version := fmt.Sprintf("fire-admin-go/%s", conf.Version) - client.Opts = []internal.HTTPOption{ + client.Opts = append(client.Opts, internal.WithHeader(apiFormatVersionHeader, apiFormatVersion), internal.WithHeader(firebaseClientHeader, version), internal.WithHeader("x-goog-api-client", internal.GetMetricsHeader(conf.Version)), - } + ) return &fcmClient{ fcmEndpoint: messagingEndpoint, diff --git a/messaging/messaging_test.go b/messaging/messaging_test.go index 5ba548af..9fcab766 100644 --- a/messaging/messaging_test.go +++ b/messaging/messaging_test.go @@ -1364,6 +1364,83 @@ func TestSendWithCustomEndpoint(t *testing.T) { } } +func TestNewClientWithRetryConfigOption(t *testing.T) { + ctx := context.Background() + + customRetry := &internal.RetryConfig{ + MaxRetries: 2, + ExpBackoffFactor: 0.25, + } + + conf := *testMessagingConfig + conf.Opts = append(conf.Opts, internal.WithRetryConfig(customRetry)) + + client, err := NewClient(ctx, &conf) + if err != nil { + t.Fatal(err) + } + + if client.fcmClient.httpClient.RetryConfig == nil { + t.Fatal("fcm retry config = nil; want non-nil") + } + if client.iidClient.httpClient.RetryConfig == nil { + t.Fatal("iid retry config = nil; want non-nil") + } + if !reflect.DeepEqual(client.fcmClient.httpClient.RetryConfig, customRetry) { + t.Errorf("fcm retry config = %#v; want = %#v", client.fcmClient.httpClient.RetryConfig, customRetry) + } + if !reflect.DeepEqual(client.iidClient.httpClient.RetryConfig, customRetry) { + t.Errorf("iid retry config = %#v; want = %#v", client.iidClient.httpClient.RetryConfig, customRetry) + } +} + +func TestNewClientWithNilRetryConfigOption(t *testing.T) { + ctx := context.Background() + + conf := *testMessagingConfig + conf.Opts = append(conf.Opts, internal.WithRetryConfig(nil)) + + client, err := NewClient(ctx, &conf) + if err != nil { + t.Fatal(err) + } + + if client.fcmClient.httpClient.RetryConfig != nil { + t.Errorf("fcm retry config = %v; want = nil", client.fcmClient.httpClient.RetryConfig) + } + if client.iidClient.httpClient.RetryConfig != nil { + t.Errorf("iid retry config = %v; want = nil", client.iidClient.httpClient.RetryConfig) + } +} + +func TestMessagingClientPreservesBaseHTTPOptions(t *testing.T) { + base := &internal.HTTPClient{ + Client: &http.Client{}, + Opts: []internal.HTTPOption{ + internal.WithHeader("X-Base-Header", "base"), + }, + } + + conf := &internal.MessagingConfig{ + ProjectID: "test-project", + Version: "test-version", + } + + fcm := newFCMClient(base, conf, defaultMessagingEndpoint, defaultBatchEndpoint) + iid := newIIDClient(base, conf) + + if len(base.Opts) != 1 { + t.Fatalf("len(base.Opts) = %d; want = 1", len(base.Opts)) + } + + if len(fcm.httpClient.Opts) != 4 { + t.Errorf("len(fcm.httpClient.Opts) = %d; want = 4", len(fcm.httpClient.Opts)) + } + if len(iid.httpClient.Opts) != 3 { + t.Errorf("len(iid.httpClient.Opts) = %d; want = 3", len(iid.httpClient.Opts)) + } +} + func TestSendDryRun(t *testing.T) { var tr *http.Request var b []byte diff --git a/messaging/topic_mgt.go b/messaging/topic_mgt.go index 0e7e9d0c..d49b2bf2 100644 --- a/messaging/topic_mgt.go +++ b/messaging/topic_mgt.go @@ -63,13 +63,13 @@ type iidClient struct { httpClient *internal.HTTPClient } -func newIIDClient(hc *http.Client, conf *internal.MessagingConfig) *iidClient { - client := internal.WithDefaultRetryConfig(hc) +func newIIDClient(base *internal.HTTPClient, conf *internal.MessagingConfig) *iidClient { + client := internal.CloneHTTPClient(base) client.CreateErrFn = handleIIDError - client.Opts = []internal.HTTPOption{ + client.Opts = append(client.Opts, internal.WithHeader("access_token_auth", "true"), internal.WithHeader("x-goog-api-client", internal.GetMetricsHeader(conf.Version)), - } + ) return &iidClient{ iidEndpoint: iidEndpoint, httpClient: client, diff --git a/retry_options.go b/retry_options.go new file mode 100644 index 00000000..61006c73 --- /dev/null +++ b/retry_options.go @@ -0,0 +1,31 @@ +// Copyright 2017 Google LLC All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package firebase + +import ( + "firebase.google.com/go/v4/internal" + "google.golang.org/api/option" +) + +// RetryConfig specifies how Admin SDK HTTP clients should retry failing requests. +type RetryConfig = internal.RetryConfig + +// WithRetryConfig creates a ClientOption that configures HTTP retry behavior. +// +// Pass this option to NewApp() to configure retries for service clients. +// If set with a nil RetryConfig, retries are disabled. +func WithRetryConfig(retryConfig *RetryConfig) option.ClientOption { + return internal.WithRetryConfig(retryConfig) +}