Add support for custom static headers #432

vam-google · 2017-11-28T20:33:22Z

Remove properties files for version detection (rely on metadata in MANIFEST.MF instead) - greatly simplifies everything, makes things more consistent and reliable.
Split custom and library-defined static headers configuration - prevents accidental overriding of intenal gapic headers by custom headers.
Support prepending User-Agent header with custom value (the only way User-Agent can be modified).
Redesign ApiClientHeaderProvider

codecov-io · 2017-11-28T20:38:33Z

Codecov Report

Merging #432 into master will increase coverage by 1.24%.
The diff coverage is 83.76%.

@@             Coverage Diff              @@
##             master     #432      +/-   ##
============================================
+ Coverage     69.11%   70.35%   +1.24%     
- Complexity      635      685      +50     
============================================
  Files           140      143       +3     
  Lines          3014     3097      +83     
  Branches        230      237       +7     
============================================
+ Hits           2083     2179      +96     
+ Misses          843      831      -12     
+ Partials         88       87       -1

Impacted Files	Coverage Δ	Complexity Δ
...ava/com/google/longrunning/OperationsSettings.java	`75.23% <0%> (+3.41%)`	`8 <0> (ø)`	⬇️
...api/gax/grpc/InstantiatingGrpcChannelProvider.java	`40.17% <0%> (ø)`	`8 <0> (ø)`	⬇️
...ogle/api/gax/httpjson/HttpJsonHeaderEnhancers.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...va/com/google/api/gax/rpc/FixedHeaderProvider.java	`100% <100%> (ø)`	`8 <8> (+6)`	⬆️
...in/java/com/google/api/gax/rpc/ClientSettings.java	`94.44% <100%> (+0.77%)`	`9 <1> (+1)`	⬆️
...ain/java/com/google/api/gax/rpc/ClientContext.java	`93.47% <100%> (+8.47%)`	`8 <0> (+2)`	⬆️
...ava/com/google/api/gax/grpc/GaxGrpcProperties.java	`90% <100%> (+30%)`	`5 <5> (+3)`	⬆️
.../java/com/google/api/gax/rpc/NoHeaderProvider.java	`50% <100%> (-50%)`	`1 <1> (-1)`
...google/api/gax/httpjson/GaxHttpJsonProperties.java	`50% <50%> (ø)`	`2 <2> (?)`
...com/google/api/gax/grpc/GrpcHeaderInterceptor.java	`80.76% <75%> (-4.24%)`	`4 <1> (+1)`
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a13293...8cf21dd. Read the comment docs.

…stent.

…classes)

vam-google · 2017-11-30T22:08:45Z

PTAL

gradle/wrapper/gradle-wrapper.properties

 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
 zipStoreBase=GRADLE_USER_HOME
 zipStorePath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-4.0.1-bin.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-4.0.1-all.zip


gax/src/main/java/com/google/api/gax/rpc/ApiClientHeaderProvider.java

-      this.clientLibName = name;
-      this.clientLibVersion = version;
-      return this;
+    public String getJvmToken() {


gax/src/main/java/com/google/api/gax/rpc/ApiClientHeaderProvider.java

+      transportToken = null;
+
+      resourceHeaderKey = "google-cloud-resource-prefix";
+      resourceToken = null;


gax/src/main/java/com/google/api/gax/core/GaxProperties.java


  /** Returns the current version of GAX. */
  public static String getGaxVersion() {
-    String version = PropertiesProvider.loadProperty(gaxProperties, GAX_PROPERTY_FILE, "version");
-    return version != null ? version : DEFAULT_VERSION;
+    return GAX_VERSION;


gax/build.gradle

-classes {
-  dependsOn createProperties
-}
+}


gax-grpc/src/test/java/com/google/api/gax/grpc/GaxGrpcPropertiesTest.java

+  @Test
+  public void testGaxGrpcVersion() {
+    String gaxGrpcVersion = GaxGrpcProperties.getGaxGrpcVersion();
+    assertNotNull(gaxGrpcVersion);


gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcHeaderInterceptor.java

+          Metadata.Key.of(header.getKey(), Metadata.ASCII_STRING_MARSHALLER);
+
+      // User-Agent is overridden on gRPC level. The custom User-Agent is supposed to be provided
+      // differently and only merging with gRPC default value for User-Agent is permitted.


gax-grpc/build.gradle

-classes {
-  dependsOn createProperties
-}
+}


gax/src/main/java/com/google/api/gax/core/PropertiesProvider.java

-
-/** Provides meta-data properties stored in a properties file. */
-@BetaApi
-public class PropertiesProvider {


gax/src/test/java/com/google/api/gax/rpc/ApiClientHeaderProviderTest.java

-import java.util.Collections;
-import java.util.List;
-import java.util.regex.Pattern;
+import static com.google.common.truth.Truth.*;


rdayal · 2017-12-07T22:33:16Z

Ping. Update on status here?

Dependent on googleapis/gax-java#432 Usage: // For manual/semi-manual clients: // Resource Manager // Translate // Compute // Storage // Datastore // Dns // BigQuery // // Logging // Firestore // Spanner HeaderProvider headerProvider = FixedHeaderProvider.create("user-agent", "my-client-name", "custom-header", "custom stuff"); LanguageServiceSettings settings = LanguageServiceSettings.newBuilder().setHeaderProvider(headerProvider).build(); LanguageServiceClient client = LanguageServiceClient.create(settings); // For generated clients: // All others, except Pub/Sub HeaderProvider headerProvider = FixedHeaderProvider.create("user-agent", "my-client-name", "custom-header", "custom stuff"); DatastoreOptions options = DatastoreOptions.newBuilder().setHeaderProvider(headerProvider).build(); Datastore datastore = options.getService(); // Note, PubSub is supported only partially. Full support is dependent on migration of PubSub from Grpc stubs to Gapic stubs

…ayer

vam-google · 2017-12-08T00:54:50Z

@rdayal there the second part of this change (much bigger than expected originally): googleapis/google-cloud-java#2690

garrettjonesgoogle

Some initial comments on your latest iteration

gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcClientHeaderProvider.java

+  public static class Builder extends ApiClientHeaderProvider.Builder {
+    public Builder() {
+      super();
+      setTransportToken("grpc", GaxGrpcProperties.getGrpcVersion());


gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpHeadersUtils.java

+          if (listElementType instanceof Class) {
+            headerValue = ImmutableList.of(fromString((Class) listElementType, value));
+          } else {
+            throw new IllegalArgumentException();


gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpHeadersUtils.java

+
+    // It seems only String and Long have chances of being used. Not adding other
+    // classes to avoid having dead code, even though without them this looks incomplete.
+    throw new IllegalArgumentException();


gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpHeadersUtilsTest.java

+import java.util.Map;
+import org.junit.Test;
+
+/*


gax/src/main/java/com/google/api/gax/rpc/ApiClientHeaderProvider.java

      return this;
    }

    public String getGeneratedLibToken() {
      return generatedLibToken;
    }

-    public Builder setGeneratedLibToken(String generatedLibToken) {
-      this.generatedLibToken = generatedLibToken;
+    public Builder setGeneratedLibToken(String name, String version) {


vam-google · 2017-12-08T22:41:45Z

@garrettjonesgoogle PTAL

…s, sent by generated clients.

vam-google · 2017-12-12T22:34:00Z

@garrettjonesgoogle PTAL

gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcClientHeaderProvider.java

+ * client library making API calls.
+ */
+@BetaApi("The surface for customizing headers is not stable yet and may change in the future.")
+public class GrpcClientHeaderProvider extends ApiClientHeaderProvider {


gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcHeaderInterceptor.java

+          Metadata.Key.of(header.getKey(), Metadata.ASCII_STRING_MARSHALLER);
+
+      // User-Agent is overridden on gRPC level. The custom User-Agent is supposed to be provided
+      // differently and only merging with gRPC default value for User-Agent is permitted.


gax/src/main/java/com/google/api/gax/rpc/ApiClientHeaderProvider.java

+      return jvmToken;
+    }
+
+    public Builder setJvmToken(String version) {


gax-grpc/src/main/java/com/google/longrunning/OperationsSettings.java

-    }
-    return gapicVersion;
+    return GrpcClientHeaderProvider.newBuilder()
+        .setGeneratedLibToken("gapic", GaxProperties.getLibraryVersion(OperationsSettings.class));


gax-grpc/src/test/java/com/google/api/gax/grpc/testing/LocalChannelProvider.java

+    return new LocalChannelProvider(new LocalAddress(addressString), null);
+  }
+
+  public boolean validateSentHeader(String headerKey, Pattern headerPattern) {


gax-grpc/src/test/java/com/google/api/gax/grpc/testing/LocalChannelProvider.java

+  }
+
+  private static class LocalHeaderInterceptor implements ClientInterceptor {
+    private final ClientInterceptor delegate;


gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonClientHeaderProvider.java

+  }
+
+  public static Builder newBuilder() {
+    return ApiClientHeaderProvider.newBuilder().setTransportToken("httpjson", "");


gax/src/main/java/com/google/api/gax/rpc/ApiClientHeaderProvider.java

+      return generatedRuntimeToken;
+    }
+
+    public Builder setGeneratedRuntimeToken(String version) {


gax/src/main/java/com/google/api/gax/rpc/ApiClientHeaderProvider.java

+    }
+
+    if (builder.getResourceHeaderKey() != null && builder.getResourceToken() != null) {
+      headersBuilder.put(builder.getResourceHeaderKey(), builder.getResourceToken());


gax/src/main/java/com/google/api/gax/rpc/ClientContext.java

@@ -73,6 +73,9 @@
  @BetaApi("The surface for customizing headers is not stable yet and may change in the future.")
  public abstract Map<String, String> getHeaders();

+  @BetaApi("The surface for customizing headers is not stable yet and may change in the future.")
+  protected abstract Map<String, String> getInternalHeaders();


vam-google · 2017-12-14T02:38:04Z

PTAL

garrettjonesgoogle

LGTM (of course, as usual, wait until the toolkit and google-cloud-java PRs are approved before merging this)

gax/src/main/java/com/google/api/gax/rpc/ApiClientHeaderProvider.java

+      return jvmToken;
+    }
+
+    public Builder setJvmToken(String version) {


Removed most of the scary reflection methods, since I'm not sure how performant they are (the setHeaders() method will be called for each request). The simplified version is pretty ugly and not generric, but should be ok for practical usage.

Add support for custom static headers

e86653d

vam-google requested a review from garrettjonesgoogle November 28, 2017 20:36

vam-google added 3 commits November 28, 2017 13:11

Make x-goog-api-client and google-cloud-resource-prefix headers consi…

64278c8

…stent.

Cleanup headers logic (Braking changes for @internalapi and @BetaApi …

54de195

…classes)

reformat

f3f9544

vam-google added 3 commits November 30, 2017 15:37

Fix grpc version retrieval

6924ba8

More headers cleanup

b935ec0

s/Google Inc./Google LLC

079c8c7

garrettjonesgoogle reviewed Dec 1, 2017

View reviewed changes

Address review feedback, make other modifications to support manual l…

4cf7656

…ayer

vam-google mentioned this pull request Dec 8, 2017

Add support for custom static headers googleapis/google-cloud-java#2690

Merged

garrettjonesgoogle reviewed Dec 8, 2017

View reviewed changes

vam-google added 2 commits December 8, 2017 12:04

Merge remote-tracking branch 'upstream/master'

1f29c9d

Address PR feedback

f165df9

googleJavaFormat

69c599c

vam-google mentioned this pull request Dec 11, 2017

Add support for custom static headers googleapis/gapic-generator#1741

Merged

vam-google added 4 commits December 11, 2017 16:24

.

5b045e8

.

15ccdae

Address review feedback. Support automated testing of internal header…

df1fb64

…s, sent by generated clients.

googleJavaFormat

abb6a02

Merge remote-tracking branch 'upstream/master'

70799a4

garrettjonesgoogle reviewed Dec 13, 2017

View reviewed changes

Address PR feedback (ranamed/moved a bunch of stuff).

f3f0a7e

vam-google added 3 commits December 13, 2017 18:11

googleJavaFormat

e8c6841

.

8145f53

.

3b8d1a5

garrettjonesgoogle approved these changes Dec 14, 2017

View reviewed changes

gax/src/main/java/com/google/api/gax/rpc/ApiClientHeaderProvider.java

return jvmToken;

}

public Builder setJvmToken(String version) {

This comment was marked as spam.

Sign in to view

vam-google added 2 commits December 14, 2017 12:06

Simplify HttpHeadersUtils.

12e6f05

Removed most of the scary reflection methods, since I'm not sure how performant they are (the setHeaders() method will be called for each request). The simplified version is pretty ugly and not generric, but should be ok for practical usage.

.

8cf21dd

vam-google merged commit f2e7142 into googleapis:master Dec 14, 2017

garrettjonesgoogle mentioned this pull request Feb 14, 2018

Warnings at Class Loading Time #376

Closed

Add support for custom static headers #432

Add support for custom static headers #432

Conversation

vam-google commented Nov 28, 2017 • edited

codecov-io commented Nov 28, 2017 • edited

Codecov Report

vam-google commented Nov 30, 2017

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

rdayal commented Dec 7, 2017

vam-google commented Dec 8, 2017

garrettjonesgoogle left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

vam-google commented Dec 8, 2017

vam-google commented Dec 12, 2017

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

vam-google commented Dec 14, 2017

garrettjonesgoogle left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

vam-google commented Nov 28, 2017 •

edited

codecov-io commented Nov 28, 2017 •

edited