Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(storage): rename host variable used for xml reads #8160

Merged
merged 3 commits into from Jun 27, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions storage/http_client.go
Expand Up @@ -49,7 +49,7 @@ import (
type httpStorageClient struct {
creds *google.Credentials
hc *http.Client
readHost string
xmlHost string
raw *raw.Service
scheme string
settings *settings
Expand Down Expand Up @@ -123,7 +123,7 @@ func newHTTPStorageClient(ctx context.Context, opts ...storageOption) (storageCl
if err != nil {
return nil, fmt.Errorf("storage client: %w", err)
}
// Update readHost and scheme with the chosen endpoint.
// Update xmlHost and scheme with the chosen endpoint.
u, err := url.Parse(ep)
if err != nil {
return nil, fmt.Errorf("supplied endpoint %q is not valid: %w", ep, err)
Expand All @@ -132,7 +132,7 @@ func newHTTPStorageClient(ctx context.Context, opts ...storageOption) (storageCl
return &httpStorageClient{
creds: creds,
hc: hc,
readHost: u.Host,
xmlHost: u.Host,
raw: rawService,
scheme: u.Scheme,
settings: s,
Expand Down Expand Up @@ -792,7 +792,7 @@ func (c *httpStorageClient) NewRangeReader(ctx context.Context, params *newRange
func (c *httpStorageClient) newRangeReaderXML(ctx context.Context, params *newRangeReaderParams, s *settings) (r *Reader, err error) {
u := &url.URL{
Scheme: c.scheme,
Host: c.readHost,
Host: c.xmlHost,
Path: fmt.Sprintf("/%s/%s", params.bucket, params.object),
RawPath: fmt.Sprintf("/%s/%s", params.bucket, url.PathEscape(params.object)),
}
Expand Down
18 changes: 9 additions & 9 deletions storage/storage.go
Expand Up @@ -109,8 +109,8 @@ type Client struct {
raw *raw.Service
// Scheme describes the scheme under the current host.
scheme string
// ReadHost is the default host used on the reader.
readHost string
// xmlHost is the default host used for XML requests.
xmlHost string
// May be nil.
creds *google.Credentials
retry *retryConfig
Expand Down Expand Up @@ -199,7 +199,7 @@ func NewClient(ctx context.Context, opts ...option.ClientOption) (*Client, error
if err != nil {
return nil, fmt.Errorf("storage client: %w", err)
}
// Update readHost and scheme with the chosen endpoint.
// Update xmlHost and scheme with the chosen endpoint.
u, err := url.Parse(ep)
if err != nil {
return nil, fmt.Errorf("supplied endpoint %q is not valid: %w", ep, err)
Expand All @@ -211,12 +211,12 @@ func NewClient(ctx context.Context, opts ...option.ClientOption) (*Client, error
}

return &Client{
hc: hc,
raw: rawService,
scheme: u.Scheme,
readHost: u.Host,
creds: creds,
tc: tc,
hc: hc,
raw: rawService,
scheme: u.Scheme,
xmlHost: u.Host,
creds: creds,
tc: tc,
}, nil
}

Expand Down
38 changes: 19 additions & 19 deletions storage/storage_test.go
Expand Up @@ -2016,95 +2016,95 @@ func TestEmulatorWithCredentialsFile(t *testing.T) {

// Create a client using a combination of custom endpoint and
// STORAGE_EMULATOR_HOST env variable and verify that raw.BasePath (used
// for writes) and readHost and scheme (used for reads) are all set correctly.
// for writes) and xmlHost and scheme (used for reads) are all set correctly.
func TestWithEndpoint(t *testing.T) {
originalStorageEmulatorHost := os.Getenv("STORAGE_EMULATOR_HOST")
testCases := []struct {
desc string
CustomEndpoint string
StorageEmulatorHost string
WantRawBasePath string
WantReadHost string
WantXMLHost string
WantScheme string
}{
{
desc: "No endpoint or emulator host specified",
CustomEndpoint: "",
StorageEmulatorHost: "",
WantRawBasePath: "https://storage.googleapis.com/storage/v1/",
WantReadHost: "storage.googleapis.com",
WantXMLHost: "storage.googleapis.com",
WantScheme: "https",
},
{
desc: "With specified https endpoint, no specified emulator host",
CustomEndpoint: "https://fake.gcs.com:8080/storage/v1",
StorageEmulatorHost: "",
WantRawBasePath: "https://fake.gcs.com:8080/storage/v1",
WantReadHost: "fake.gcs.com:8080",
WantXMLHost: "fake.gcs.com:8080",
WantScheme: "https",
},
{
desc: "With specified http endpoint, no specified emulator host",
CustomEndpoint: "http://fake.gcs.com:8080/storage/v1",
StorageEmulatorHost: "",
WantRawBasePath: "http://fake.gcs.com:8080/storage/v1",
WantReadHost: "fake.gcs.com:8080",
WantXMLHost: "fake.gcs.com:8080",
WantScheme: "http",
},
{
desc: "Emulator host specified, no specified endpoint",
CustomEndpoint: "",
StorageEmulatorHost: "http://emu.com",
WantRawBasePath: "http://emu.com/storage/v1/",
WantReadHost: "emu.com",
WantXMLHost: "emu.com",
WantScheme: "http",
},
{
desc: "Emulator host specified without scheme",
CustomEndpoint: "",
StorageEmulatorHost: "emu.com",
WantRawBasePath: "http://emu.com/storage/v1/",
WantReadHost: "emu.com",
WantXMLHost: "emu.com",
WantScheme: "http",
},
{
desc: "Emulator host specified as host:port",
CustomEndpoint: "",
StorageEmulatorHost: "localhost:9000",
WantRawBasePath: "http://localhost:9000/storage/v1/",
WantReadHost: "localhost:9000",
WantXMLHost: "localhost:9000",
WantScheme: "http",
},
{
desc: "Endpoint overrides emulator host when both are specified - https",
CustomEndpoint: "https://fake.gcs.com:8080/storage/v1",
StorageEmulatorHost: "http://emu.com",
WantRawBasePath: "https://fake.gcs.com:8080/storage/v1",
WantReadHost: "fake.gcs.com:8080",
WantXMLHost: "fake.gcs.com:8080",
WantScheme: "https",
},
{
desc: "Endpoint overrides emulator host when both are specified - http",
CustomEndpoint: "http://fake.gcs.com:8080/storage/v1",
StorageEmulatorHost: "https://emu.com",
WantRawBasePath: "http://fake.gcs.com:8080/storage/v1",
WantReadHost: "fake.gcs.com:8080",
WantXMLHost: "fake.gcs.com:8080",
WantScheme: "http",
},
{
desc: "Endpoint overrides emulator host when host is specified as scheme://host:port",
CustomEndpoint: "http://localhost:8080/storage/v1",
StorageEmulatorHost: "https://localhost:9000",
WantRawBasePath: "http://localhost:8080/storage/v1",
WantReadHost: "localhost:8080",
WantXMLHost: "localhost:8080",
WantScheme: "http",
},
{
desc: "Endpoint overrides emulator host when host is specified as host:port",
CustomEndpoint: "http://localhost:8080/storage/v1",
StorageEmulatorHost: "localhost:9000",
WantRawBasePath: "http://localhost:8080/storage/v1",
WantReadHost: "localhost:8080",
WantXMLHost: "localhost:8080",
WantScheme: "http",
},
}
Expand All @@ -2119,8 +2119,8 @@ func TestWithEndpoint(t *testing.T) {
if c.raw.BasePath != tc.WantRawBasePath {
t.Errorf("%s: raw.BasePath not set correctly\n\tgot %v, want %v", tc.desc, c.raw.BasePath, tc.WantRawBasePath)
}
if c.readHost != tc.WantReadHost {
t.Errorf("%s: readHost not set correctly\n\tgot %v, want %v", tc.desc, c.readHost, tc.WantReadHost)
if c.xmlHost != tc.WantXMLHost {
t.Errorf("%s: xmlHost not set correctly\n\tgot %v, want %v", tc.desc, c.xmlHost, tc.WantXMLHost)
}
if c.scheme != tc.WantScheme {
t.Errorf("%s: scheme not set correctly\n\tgot %v, want %v", tc.desc, c.scheme, tc.WantScheme)
Expand All @@ -2132,7 +2132,7 @@ func TestWithEndpoint(t *testing.T) {
// Create a client using a combination of custom endpoint and STORAGE_EMULATOR_HOST
// env variable and verify that the client hits the correct endpoint for several
// different operations performe in sequence.
// Verifies also that raw.BasePath, readHost and scheme are not changed
// Verifies also that raw.BasePath, xmlHost and scheme are not changed
// after running the operations.
func TestOperationsWithEndpoint(t *testing.T) {
originalStorageEmulatorHost := os.Getenv("STORAGE_EMULATOR_HOST")
Expand Down Expand Up @@ -2214,7 +2214,7 @@ func TestOperationsWithEndpoint(t *testing.T) {
return
}
originalRawBasePath := c.raw.BasePath
originalReadHost := c.readHost
originalXMLHost := c.xmlHost
originalScheme := c.scheme

operations := []struct {
Expand Down Expand Up @@ -2298,9 +2298,9 @@ func TestOperationsWithEndpoint(t *testing.T) {
t.Errorf("raw.BasePath changed\n\tgot:\t\t%v\n\toriginal:\t%v",
c.raw.BasePath, originalRawBasePath)
}
if c.readHost != originalReadHost {
t.Errorf("readHost changed\n\tgot:\t\t%v\n\toriginal:\t%v",
c.readHost, originalReadHost)
if c.xmlHost != originalXMLHost {
t.Errorf("xmlHost changed\n\tgot:\t\t%v\n\toriginal:\t%v",
c.xmlHost, originalXMLHost)
}
if c.scheme != originalScheme {
t.Errorf("scheme changed\n\tgot:\t\t%v\n\toriginal:\t%v",
Expand Down