Fixing spanner headers consistency issue.

vam-google · vam-google · commit 4644e73e5ac7 · 2017-06-23T12:32:39.000-07:00
Previously spanner client was sending (example valuess):
user-agent: google-cloud-java/0.20.0-beta grpc-java-netty/1.2.0
x-goog-api-client: google-cloud-java/0.20.0-beta

After the changes (example values):
user-agent: grpc-java-netty/1.2.0,
x-goog-api-client: gl-java/1.8.0_112-google-v7 gccl/1.2.1, gax/1.3.1 grpc/1.2.0
diff --git a/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/BigQueryOptionsTest.java b/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/BigQueryOptionsTest.java
@@ -17,6 +17,7 @@
 package com.google.cloud.bigquery;
 
 import com.google.cloud.TransportOptions;
+import org.easymock.EasyMock;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -28,7 +29,7 @@ public class BigQueryOptionsTest {
   @Test
   public void testInvalidTransport() {
     thrown.expect(IllegalArgumentException.class);
-    BigQueryOptions.newBuilder().setTransportOptions(new TransportOptions() {});
+    BigQueryOptions.newBuilder().setTransportOptions(EasyMock.createMock(TransportOptions.class));
   }
 
 }
diff --git a/google-cloud-compute/src/test/java/com/google/cloud/compute/ComputeOptionsTest.java b/google-cloud-compute/src/test/java/com/google/cloud/compute/ComputeOptionsTest.java
@@ -17,6 +17,7 @@
 package com.google.cloud.compute;
 
 import com.google.cloud.TransportOptions;
+import org.easymock.EasyMock;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -29,7 +30,7 @@ public class ComputeOptionsTest {
   @Test
   public void testInvalidTransport() {
     thrown.expect(IllegalArgumentException.class);
-    ComputeOptions.newBuilder().setTransportOptions(new TransportOptions() {});
+    ComputeOptions.newBuilder().setTransportOptions(EasyMock.createMock(TransportOptions.class));
   }
 
 }
diff --git a/google-cloud-core-grpc/src/main/java/com/google/cloud/grpc/GrpcTransportOptions.java b/google-cloud-core-grpc/src/main/java/com/google/cloud/grpc/GrpcTransportOptions.java
@@ -21,8 +21,10 @@
 import com.google.api.core.InternalApi;
 import com.google.api.gax.core.FixedCredentialsProvider;
 import com.google.api.gax.core.CredentialsProvider;
+import com.google.api.gax.core.GaxProperties;
 import com.google.api.gax.core.NoCredentialsProvider;
 import com.google.api.gax.grpc.ChannelProvider;
+import com.google.api.gax.grpc.GaxGrpcProperties;
 import com.google.api.gax.grpc.InstantiatingChannelProvider;
 import com.google.api.gax.grpc.UnaryCallSettings;
 import com.google.api.gax.retrying.RetrySettings;
@@ -34,6 +36,8 @@
 import io.grpc.internal.SharedResourceHolder.Resource;
 import java.io.IOException;
 import java.io.ObjectInputStream;
+import java.util.Locale;
+import java.util.Locale.Category;
 import java.util.Objects;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.ScheduledExecutorService;
@@ -71,6 +75,16 @@ public void close(ScheduledExecutorService instance) {
         }
       };
 
+  public String getXGoogApiClientHeader(String libraryVersion) {
+    return String.format(Locale.US,
+        "gl-java/%s %s/%s gax/%s grpc/%s",
+        firstNonNull(Runtime.class.getPackage().getImplementationVersion(), ""),
+        ServiceOptions.getGoogApiClientLibName(),
+        libraryVersion,
+        GaxProperties.getGaxVersion(),
+        GaxGrpcProperties.getGrpcVersion());
+  }
+
   /**
    * An interface for {@link ExecutorService} factories. Implementations of this interface can be
    * used to provide an user-defined executor to execute requests. Any implementation of this
diff --git a/google-cloud-core-grpc/src/test/java/com/google/cloud/grpc/GrpcTransportOptionsTest.java b/google-cloud-core-grpc/src/test/java/com/google/cloud/grpc/GrpcTransportOptionsTest.java
@@ -24,6 +24,7 @@
 import com.google.cloud.grpc.GrpcTransportOptions.DefaultExecutorFactory;
 import com.google.cloud.grpc.GrpcTransportOptions.ExecutorFactory;
 import java.util.concurrent.ScheduledExecutorService;
+import java.util.regex.Pattern;
 import org.easymock.EasyMock;
 import org.junit.Test;
 
@@ -70,4 +71,12 @@ public void testDefaultExecutorFactory() {
     ScheduledExecutorService executorService = executorFactory.get();
     assertSame(executorService, executorFactory.get());
   }
+
+  @Test
+  public void testHeader() {
+    String expectedHeaderPattern = "^gl-java/.+ gccl/0.0.0+ gax/.+ grpc/.+";
+    assertTrue(Pattern.compile(expectedHeaderPattern)
+        .matcher(OPTIONS.getXGoogApiClientHeader("0.0.0"))
+        .find());
+  }
 }
diff --git a/google-cloud-core-http/src/main/java/com/google/cloud/http/HttpTransportOptions.java b/google-cloud-core-http/src/main/java/com/google/cloud/http/HttpTransportOptions.java
@@ -24,7 +24,6 @@
 import com.google.api.client.http.HttpRequestInitializer;
 import com.google.api.client.http.HttpTransport;
 import com.google.api.client.http.javanet.NetHttpTransport;
-import com.google.api.core.InternalApi;
 import com.google.auth.Credentials;
 import com.google.auth.http.HttpCredentialsAdapter;
 import com.google.auth.http.HttpTransportFactory;
@@ -33,6 +32,7 @@
 import com.google.cloud.TransportOptions;
 import java.io.IOException;
 import java.io.ObjectInputStream;
+import java.util.Locale;
 import java.util.Objects;
 
 /**
@@ -163,22 +163,18 @@ public void initialize(HttpRequest httpRequest) throws IOException {
         }
 
         HttpHeaders headers = httpRequest.getHeaders();
-        headers.set("x-goog-api-client", getXGoogApiClientHeader(serviceOptions));
+        headers.set(
+            "x-goog-api-client", getXGoogApiClientHeader(serviceOptions.getLibraryVersion()));
       }
     };
   }
 
-  String getXGoogApiClientHeader(ServiceOptions<?, ?> serviceOptions) {
-    return String.format(
+  String getXGoogApiClientHeader(String libraryVersion) {
+    return String.format(Locale.US,
         "gl-java/%s %s/%s",
-        getJavaVersion(),
+        firstNonNull(Runtime.class.getPackage().getImplementationVersion(), ""),
         ServiceOptions.getGoogApiClientLibName(),
-        serviceOptions.getLibraryVersion());
-  }
-
-  private static String getJavaVersion() {
-    String javaVersion = Runtime.class.getPackage().getImplementationVersion();
-    return javaVersion != null ? javaVersion : "";
+        libraryVersion);
   }
 
   /**
diff --git a/google-cloud-core-http/src/test/java/com/google/cloud/http/HttpTransportOptionsTest.java b/google-cloud-core-http/src/test/java/com/google/cloud/http/HttpTransportOptionsTest.java
@@ -22,7 +22,6 @@
 import static org.junit.Assert.assertTrue;
 
 import com.google.auth.http.HttpTransportFactory;
-import com.google.cloud.ServiceOptions;
 import com.google.cloud.http.HttpTransportOptions.DefaultHttpTransportFactory;
 import java.util.regex.Pattern;
 import org.easymock.EasyMock;
@@ -65,12 +64,9 @@ public void testBaseHashCode() {
 
   @Test
   public void testHeader() {
-    String expectedHeaderPattern = "^gl-java/.* gccl/0.0.0";
-    final ServiceOptions mockOptions = EasyMock.createMock(ServiceOptions.class);
-    EasyMock.expect(mockOptions.getLibraryVersion()).andReturn("0.0.0");
-    EasyMock.replay(mockOptions);
+    String expectedHeaderPattern = "^gl-java/.+ gccl/0.0.0";
     assertTrue(Pattern.compile(expectedHeaderPattern)
-        .matcher(OPTIONS.getXGoogApiClientHeader(mockOptions))
+        .matcher(OPTIONS.getXGoogApiClientHeader("0.0.0"))
         .find());
   }
 }
diff --git a/google-cloud-core/src/main/java/com/google/cloud/TransportOptions.java b/google-cloud-core/src/main/java/com/google/cloud/TransportOptions.java
@@ -21,5 +21,4 @@
  * An abstraction for transport-specific options, e.g. for http1.1 vs grpc.
  */
 public interface TransportOptions extends Serializable {
-
 }
diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java
@@ -89,6 +89,6 @@ public void testToBuilder() throws Exception {
   @Test
   public void testInvalidTransport() {
     thrown.expect(IllegalArgumentException.class);
-    DatastoreOptions.newBuilder().setTransportOptions(new TransportOptions() {});
+    DatastoreOptions.newBuilder().setTransportOptions(EasyMock.createMock(TransportOptions.class));
   }
 }
diff --git a/google-cloud-dns/src/test/java/com/google/cloud/dns/DnsOptionsTest.java b/google-cloud-dns/src/test/java/com/google/cloud/dns/DnsOptionsTest.java
@@ -17,6 +17,7 @@
 package com.google.cloud.dns;
 
 import com.google.cloud.TransportOptions;
+import org.easymock.EasyMock;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -29,7 +30,7 @@ public class DnsOptionsTest {
   @Test
   public void testInvalidTransport() {
     thrown.expect(IllegalArgumentException.class);
-    DnsOptions.newBuilder().setTransportOptions(new TransportOptions() {});
+    DnsOptions.newBuilder().setTransportOptions(EasyMock.createMock(TransportOptions.class));
   }
 
 }
diff --git a/google-cloud-examples/src/main/java/com/google/cloud/examples/spanner/snippets/DatabaseSelect.java b/google-cloud-examples/src/main/java/com/google/cloud/examples/spanner/snippets/DatabaseSelect.java
@@ -0,0 +1,67 @@
+/*
+ * Copyright 2017 Google Inc. 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.
+ */
+
+/*
+ * EDITING INSTRUCTIONS
+ * This file is referenced in READMEs. Any change to this file should be reflected in
+ * the project's READMEs.
+ */
+
+package com.google.cloud.examples.spanner.snippets;
+
+import com.google.cloud.spanner.DatabaseClient;
+import com.google.cloud.spanner.DatabaseId;
+import com.google.cloud.spanner.ResultSet;
+import com.google.cloud.spanner.Spanner;
+import com.google.cloud.spanner.SpannerOptions;
+import com.google.cloud.spanner.Statement;
+
+/**
+ * A quick start code for Cloud Spanner. It demonstrates how to setup the Cloud Spanner client and
+ * execute a simple query using it against an existing database.
+ */
+public class DatabaseSelect {
+  public static void main(String... args) throws Exception {
+
+    if (args.length != 2) {
+      System.err.println("Usage: QuickStartSample <instance_id> <database_id>");
+      return;
+    }
+    // Instantiates a client
+    SpannerOptions options = SpannerOptions.newBuilder().build();
+    Spanner spanner = options.getService();
+
+    // Name of your instance & database.
+    String instanceId = args[0];
+    String databaseId = args[1];
+    try {
+      // Creates a database client
+      DatabaseClient dbClient = spanner.getDatabaseClient(DatabaseId.of(
+          options.getProjectId(), instanceId, databaseId));
+      // Queries the database
+      ResultSet resultSet = dbClient.singleUse().executeQuery(Statement.of("SELECT 1"));
+
+      System.out.println("\n\nResults:");
+      // Prints the results
+      while (resultSet.next()) {
+        System.out.printf("%d\n\n", resultSet.getLong(0));
+      }
+    } finally {
+      // Closes the client which will free up the resources used
+      spanner.close();
+    }
+  }
+}
diff --git a/google-cloud-logging/src/test/java/com/google/cloud/logging/LoggingOptionsTest.java b/google-cloud-logging/src/test/java/com/google/cloud/logging/LoggingOptionsTest.java
@@ -17,6 +17,7 @@
 package com.google.cloud.logging;
 
 import com.google.cloud.TransportOptions;
+import org.easymock.EasyMock;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -29,7 +30,7 @@ public class LoggingOptionsTest {
   @Test
   public void testInvalidTransport() {
     thrown.expect(IllegalArgumentException.class);
-    LoggingOptions.newBuilder().setTransportOptions(new TransportOptions() {});
+    LoggingOptions.newBuilder().setTransportOptions(EasyMock.createMock(TransportOptions.class));
   }
 
 }
diff --git a/google-cloud-resourcemanager/src/test/java/com/google/cloud/resourcemanager/ResourceManagerOptionsTest.java b/google-cloud-resourcemanager/src/test/java/com/google/cloud/resourcemanager/ResourceManagerOptionsTest.java
@@ -17,6 +17,7 @@
 package com.google.cloud.resourcemanager;
 
 import com.google.cloud.TransportOptions;
+import org.easymock.EasyMock;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -29,7 +30,8 @@ public class ResourceManagerOptionsTest {
   @Test
   public void testInvalidTransport() {
     thrown.expect(IllegalArgumentException.class);
-    ResourceManagerOptions.newBuilder().setTransportOptions(new TransportOptions() {});
+    ResourceManagerOptions.newBuilder().setTransportOptions(
+        EasyMock.createMock(TransportOptions.class));
   }
 
 }
diff --git a/google-cloud-spanner/README.md b/google-cloud-spanner/README.md
@@ -61,10 +61,14 @@ See the `google-cloud` API [spanner documentation][spanner-api] to learn how to
 interact with Cloud Spanner using this Client Library.
 
 ## Getting Started
-
+#### Prerequisites
 Please refer to the [getting
 started](https://cloud.google.com/spanner/docs/getting-started/java/) guide.
 
+#### Complete source code
+
+In [DatabaseSelect.java](../google-cloud-examples/src/main/java/com/google/cloud/examples/spanner/snippets/DatabaseSelect.java) we put a quick start example, which shows how you can run a simple select statement.   
+
 ## Troubleshooting
 
 To get help, follow the instructions in the [shared Troubleshooting
diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java
@@ -48,8 +48,7 @@ public class SpannerOptions extends ServiceOptions<Spanner, SpannerOptions> {
           "https://www.googleapis.com/auth/spanner.admin",
           "https://www.googleapis.com/auth/spanner.data");
   private static final int MAX_CHANNELS = 256;
-  private static final RpcChannelFactory DEFAULT_RPC_CHANNEL_FACTORY =
-      new NettyRpcChannelFactory(GrpcSpannerRpc.API_CLIENT);
+  private static final RpcChannelFactory DEFAULT_RPC_CHANNEL_FACTORY = new NettyRpcChannelFactory();
 
   /** Default implementation of {@code SpannerFactory}. */
   private static class DefaultSpannerFactory implements SpannerFactory {
@@ -84,7 +83,7 @@ private SpannerOptions(Builder builder) {
     RpcChannelFactory defaultRpcChannelFactory =
         userAgent == null
             ? DEFAULT_RPC_CHANNEL_FACTORY
-            : new NettyRpcChannelFactory(userAgent + " " + GrpcSpannerRpc.API_CLIENT);
+            : new NettyRpcChannelFactory(userAgent);
     rpcChannels =
         createChannels(
             getHost(),
@@ -259,6 +258,10 @@ static class NettyRpcChannelFactory implements RpcChannelFactory {
     private final String userAgent;
     private final List<ClientInterceptor> interceptors;
 
+    NettyRpcChannelFactory() {
+      this(null);
+    }
+
     NettyRpcChannelFactory(String userAgent) {
       this(userAgent, ImmutableList.<ClientInterceptor>of());
     }
@@ -270,12 +273,15 @@ static class NettyRpcChannelFactory implements RpcChannelFactory {
 
     @Override
     public ManagedChannel newChannel(String host, int port) {
-      return NettyChannelBuilder.forAddress(host, port)
-          .sslContext(newSslContext())
-          .intercept(interceptors)
-          .maxMessageSize(MAX_MESSAGE_SIZE)
-          .userAgent(userAgent)
-          .build();
+      NettyChannelBuilder builder =
+          NettyChannelBuilder.forAddress(host, port)
+            .sslContext(newSslContext())
+            .intercept(interceptors)
+            .maxMessageSize(MAX_MESSAGE_SIZE);
+      if (userAgent != null) {
+        builder.userAgent(userAgent);
+      }
+      return builder.build();
     }
 
     private static SslContext newSslContext() {
diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GrpcSpannerRpc.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GrpcSpannerRpc.java
diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/GceTestEnvConfig.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/GceTestEnvConfig.java
diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java
diff --git a/google-cloud-translate/src/test/java/com/google/cloud/translate/TranslateOptionsTest.java b/google-cloud-translate/src/test/java/com/google/cloud/translate/TranslateOptionsTest.java
diff --git a/google-cloud/pom.xml b/google-cloud/pom.xml

Original file line number	Diff line number	Diff line change
`@@ -17,6 +17,7 @@`
`17`	`17`	`package com.google.cloud.bigquery;`
`18`	`18`
`19`	`19`	`import com.google.cloud.TransportOptions;`
	`20`	`+import org.easymock.EasyMock;`
`20`	`21`	`import org.junit.Rule;`
`21`	`22`	`import org.junit.Test;`
`22`	`23`	`import org.junit.rules.ExpectedException;`
`@@ -28,7 +29,7 @@ public class BigQueryOptionsTest {`
`28`	`29`	`@Test`
`29`	`30`	`public void testInvalidTransport() {`
`30`	`31`	`thrown.expect(IllegalArgumentException.class);`
`31`		`- BigQueryOptions.newBuilder().setTransportOptions(new TransportOptions() {});`
	`32`	`+ BigQueryOptions.newBuilder().setTransportOptions(EasyMock.createMock(TransportOptions.class));`
`32`	`33`	`}`
`33`	`34`
`34`	`35`	`}`
Original file line number	Diff line number	Diff line change
`@@ -21,5 +21,4 @@`
`21`	`21`	`* An abstraction for transport-specific options, e.g. for http1.1 vs grpc.`
`22`	`22`	`*/`
`23`	`23`	`public interface TransportOptions extends Serializable {`
`24`		`-`
`25`	`24`	`}`
Original file line number	Diff line number	Diff line change
`@@ -89,6 +89,6 @@ public void testToBuilder() throws Exception {`
`89`	`89`	`@Test`
`90`	`90`	`public void testInvalidTransport() {`
`91`	`91`	`thrown.expect(IllegalArgumentException.class);`
`92`		`- DatastoreOptions.newBuilder().setTransportOptions(new TransportOptions() {});`
	`92`	`+ DatastoreOptions.newBuilder().setTransportOptions(EasyMock.createMock(TransportOptions.class));`
`93`	`93`	`}`
`94`	`94`	`}`