Skip to content

Commit

Permalink
spanner: remove redundant x-goog-api-client header
Browse files Browse the repository at this point in the history
The Spanner client now uses the gapic client, which automatically
sets the x-goog-api-client header, so this header should no longer
be set by the client itself.

Fixes #1524

Change-Id: Ib7523878a7d94f7e5a05b5caec6fc3312316c72c
Reviewed-on: https://code-review.googlesource.com/c/gocloud/+/43671
Reviewed-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
  • Loading branch information
olavloite committed Aug 8, 2019
1 parent e4bd323 commit 4b1db5f
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 14 deletions.
10 changes: 1 addition & 9 deletions spanner/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"time"

"cloud.google.com/go/internal/trace"
"cloud.google.com/go/internal/version"
vkit "cloud.google.com/go/spanner/apiv1"
"cloud.google.com/go/spanner/internal/backoff"
"google.golang.org/api/option"
Expand All @@ -41,10 +40,6 @@ const (
// resourcePrefixHeader is the name of the metadata header used to indicate
// the resource being operated on.
resourcePrefixHeader = "google-cloud-resource-prefix"

// xGoogHeaderKey is the name of the metadata header used to indicate client
// information.
xGoogHeaderKey = "x-goog-api-client"
)

const (
Expand All @@ -57,7 +52,6 @@ const (

var (
validDBPattern = regexp.MustCompile("^projects/[^/]+/instances/[^/]+/databases/[^/]+$")
xGoogHeaderVal = fmt.Sprintf("gl-go/%s gccl/%s grpc/%s", version.Go(), version.Repo, grpc.Version)
)

func validDatabaseName(db string) error {
Expand Down Expand Up @@ -125,9 +119,7 @@ func NewClient(ctx context.Context, database string, opts ...option.ClientOption
func NewClientWithConfig(ctx context.Context, database string, config ClientConfig, opts ...option.ClientOption) (c *Client, err error) {
c = &Client{
database: database,
md: metadata.Pairs(
resourcePrefixHeader, database,
xGoogHeaderKey, xGoogHeaderVal),
md: metadata.Pairs(resourcePrefixHeader, database),
}

// Make a copy of labels.
Expand Down
49 changes: 49 additions & 0 deletions spanner/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ import (
"cloud.google.com/go/spanner/internal/benchserver"
"cloud.google.com/go/spanner/internal/testutil"
"google.golang.org/api/iterator"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/metadata"
gstatus "google.golang.org/grpc/status"
)

Expand Down Expand Up @@ -435,3 +437,50 @@ func TestNewClient_ConnectToEmulator(t *testing.T) {
t.Fatal(err)
}
}

func TestClient_ApiClientHeader(t *testing.T) {
t.Parallel()
server, client := newSpannerInMemTestServerWithInterceptor(t, func(
ctx context.Context,
method string,
req interface{},
reply interface{},
cc *grpc.ClientConn,
invoker grpc.UnaryInvoker,
opts ...grpc.CallOption,
) error {
// Check that the x-goog-api-client is set.
headers, ok := metadata.FromOutgoingContext(ctx)
if !ok {
return spannerErrorf(codes.Internal, "could not get outgoing metadata")
}
token, ok := headers["x-goog-api-client"]
if !ok {
return spannerErrorf(codes.Internal, "could not get api client token")
}
if token == nil {
return spannerErrorf(codes.Internal, "nil-value for outgoing api client token")
}
if len(token) != 1 {
return spannerErrorf(codes.Internal, "unexpected number of api client token headers: %v", len(token))
}
if !strings.HasPrefix(token[0], "gl-go/") {
return spannerErrorf(codes.Internal, "unexpected api client token: %v", token[0])
}
return invoker(ctx, method, req, reply, cc, opts...)
},
)
defer server.teardown(client)
ctx := context.Background()
iter := client.Single().Query(ctx, NewStatement(selectSingerIDAlbumIDAlbumTitleFromAlbums))
defer iter.Stop()
for {
_, err := iter.Next()
if err == iterator.Done {
break
}
if err != nil {
t.Fatal(err)
}
}
}
21 changes: 16 additions & 5 deletions spanner/mocked_inmem_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,18 +56,25 @@ func newSpannerInMemTestServer(t *testing.T) (*spannerInMemTestServer, *Client)
return s, client
}

// Create a spannerInMemTestServer with default configuration and a client interceptor.
func newSpannerInMemTestServerWithInterceptor(t *testing.T, interceptor grpc.UnaryClientInterceptor) (*spannerInMemTestServer, *Client) {
s := &spannerInMemTestServer{}
client := s.setupWithConfig(t, ClientConfig{}, interceptor)
return s, client
}

// Create a spannerInMemTestServer with the specified configuration.
func newSpannerInMemTestServerWithConfig(t *testing.T, config ClientConfig) (*spannerInMemTestServer, *Client) {
s := &spannerInMemTestServer{}
client := s.setupWithConfig(t, config)
client := s.setupWithConfig(t, config, nil)
return s, client
}

func (s *spannerInMemTestServer) setup(t *testing.T) *Client {
return s.setupWithConfig(t, ClientConfig{})
return s.setupWithConfig(t, ClientConfig{}, nil)
}

func (s *spannerInMemTestServer) setupWithConfig(t *testing.T, config ClientConfig) *Client {
func (s *spannerInMemTestServer) setupWithConfig(t *testing.T, config ClientConfig, interceptor grpc.UnaryClientInterceptor) *Client {
s.testSpanner = testutil.NewInMemSpannerServer()
s.setupFooResults()
s.setupSingersResults()
Expand All @@ -83,11 +90,15 @@ func (s *spannerInMemTestServer) setupWithConfig(t *testing.T, config ClientConf
serverAddress := lis.Addr().String()
ctx := context.Background()
var formattedDatabase = fmt.Sprintf("projects/%s/instances/%s/databases/%s", "[PROJECT]", "[INSTANCE]", "[DATABASE]")
client, err := NewClientWithConfig(ctx, formattedDatabase, config,
opts := []option.ClientOption{
option.WithEndpoint(serverAddress),
option.WithGRPCDialOption(grpc.WithInsecure()),
option.WithoutAuthentication(),
)
}
if interceptor != nil {
opts = append(opts, option.WithGRPCDialOption(grpc.WithUnaryInterceptor(interceptor)))
}
client, err := NewClientWithConfig(ctx, formattedDatabase, config, opts...)
if err != nil {
t.Fatal(err)
}
Expand Down

0 comments on commit 4b1db5f

Please sign in to comment.