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
feat: Parse Host Service Name #2300
Changes from 11 commits
629dee4
a709d8a
e227b62
c6c59a6
789d4e2
309d598
4d84c02
3adeee4
3e62052
d07ed96
db04a0b
09a5dfd
3df71f4
a3e27e7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,6 +52,12 @@ public boolean hasDescription() { | |
return !Strings.isNullOrEmpty(description()); | ||
} | ||
|
||
public String hostServiceName() { | ||
// Default Host is guaranteed to exist and be non-null and non-empty | ||
// Parser will fail if the default host is not supplied | ||
return parseHostServiceName(defaultHost()); | ||
} | ||
|
||
public String apiShortName() { | ||
if (!Strings.isNullOrEmpty(defaultHost())) { | ||
return parseApiShortName(defaultHost()); | ||
|
@@ -184,6 +190,17 @@ private static String parseApiVersion(String protoPackage) { | |
return apiVersion; | ||
} | ||
|
||
// Parse the service name from the default host configured in the protos | ||
// or service yaml file. For Google Cloud Services, the default host value | ||
// is expected to contain `.googleapis.com`. Exceptions may exist (i.e. localhost), | ||
// in which case we will return an empty string. | ||
private static String parseHostServiceName(String defaultHost) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know we are already testing this method indirectly through golden tests. Since this is in Service not composers, and we can easily mock the input, it would be great if we can have unit tests testing it directly as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added some tests for parsing, thanks! |
||
if (defaultHost.contains(".googleapis.com")) { | ||
return Iterables.getFirst(Splitter.on(".").split(defaultHost), defaultHost); | ||
} | ||
return ""; | ||
} | ||
|
||
// Parse defaultHost for apiShortName for the RegionTag. Need to account for regional default | ||
// endpoints like | ||
// "us-east1-pubsub.googleapis.com". | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -100,6 +100,10 @@ public abstract class ClientContext { | |
@Nonnull | ||
public abstract Duration getStreamWatchdogCheckInterval(); | ||
|
||
// Package-Private scope for internal use only. Shared between StubSettings and ClientContext | ||
@Nullable | ||
abstract String getHostServiceName(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shall we call this field There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, I'll rename to |
||
|
||
@Nullable | ||
public abstract String getEndpoint(); | ||
|
||
|
@@ -202,6 +206,7 @@ public static ClientContext create(StubSettings settings) throws IOException { | |
} | ||
EndpointContext endpointContext = | ||
EndpointContext.newBuilder() | ||
.setHostServiceName(settings.getHostServiceName()) | ||
.setClientSettingsEndpoint(settings.getEndpoint()) | ||
.setTransportChannelProviderEndpoint( | ||
settings.getTransportChannelProvider().getEndpoint()) | ||
|
@@ -258,6 +263,7 @@ public static ClientContext create(StubSettings settings) throws IOException { | |
.setInternalHeaders(ImmutableMap.copyOf(settings.getInternalHeaderProvider().getHeaders())) | ||
.setClock(clock) | ||
.setDefaultCallContext(defaultCallContext) | ||
.setHostServiceName(settings.getHostServiceName()) | ||
.setEndpoint(settings.getEndpoint()) | ||
.setQuotaProjectId(settings.getQuotaProjectId()) | ||
.setStreamWatchdog(watchdog) | ||
|
@@ -323,6 +329,9 @@ public abstract static class Builder { | |
|
||
public abstract Builder setDefaultCallContext(ApiCallContext defaultCallContext); | ||
|
||
// Package-Private scope for internal use only. Shared between StubSettings and ClientContext | ||
abstract Builder setHostServiceName(String hostServiceName); | ||
|
||
public abstract Builder setEndpoint(String endpoint); | ||
|
||
public abstract Builder setQuotaProjectId(String QuotaProjectId); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,7 @@ | |
import com.google.api.core.ApiClock; | ||
import com.google.api.core.ApiFunction; | ||
import com.google.api.core.BetaApi; | ||
import com.google.api.core.InternalApi; | ||
import com.google.api.core.NanoClock; | ||
import com.google.api.gax.core.CredentialsProvider; | ||
import com.google.api.gax.core.ExecutorProvider; | ||
|
@@ -70,6 +71,7 @@ public abstract class StubSettings<SettingsT extends StubSettings<SettingsT>> { | |
private final HeaderProvider internalHeaderProvider; | ||
private final TransportChannelProvider transportChannelProvider; | ||
private final ApiClock clock; | ||
private final String hostServiceName; | ||
private final String endpoint; | ||
private final String mtlsEndpoint; | ||
private final String quotaProjectId; | ||
|
@@ -96,6 +98,7 @@ protected StubSettings(Builder builder) { | |
this.headerProvider = builder.headerProvider; | ||
this.internalHeaderProvider = builder.internalHeaderProvider; | ||
this.clock = builder.clock; | ||
this.hostServiceName = builder.hostServiceName; | ||
this.endpoint = builder.endpoint; | ||
this.mtlsEndpoint = builder.mtlsEndpoint; | ||
this.switchToMtlsEndpointAllowed = builder.switchToMtlsEndpointAllowed; | ||
|
@@ -137,6 +140,11 @@ public final ApiClock getClock() { | |
return clock; | ||
} | ||
|
||
// Package-Private scope for internal use only. Shared between StubSettings and ClientContext | ||
final String getHostServiceName() { | ||
return hostServiceName; | ||
} | ||
|
||
public final String getEndpoint() { | ||
return endpoint; | ||
} | ||
|
@@ -211,6 +219,7 @@ public abstract static class Builder< | |
private HeaderProvider internalHeaderProvider; | ||
private TransportChannelProvider transportChannelProvider; | ||
private ApiClock clock; | ||
private String hostServiceName; | ||
private String endpoint; | ||
private String mtlsEndpoint; | ||
private String quotaProjectId; | ||
|
@@ -236,6 +245,7 @@ protected Builder(StubSettings settings) { | |
this.headerProvider = settings.headerProvider; | ||
this.internalHeaderProvider = settings.internalHeaderProvider; | ||
this.clock = settings.clock; | ||
this.hostServiceName = settings.hostServiceName; | ||
this.endpoint = settings.endpoint; | ||
this.mtlsEndpoint = settings.mtlsEndpoint; | ||
this.switchToMtlsEndpointAllowed = settings.switchToMtlsEndpointAllowed; | ||
|
@@ -272,6 +282,7 @@ protected Builder(ClientContext clientContext) { | |
this.headerProvider = new NoHeaderProvider(); | ||
this.internalHeaderProvider = new NoHeaderProvider(); | ||
this.clock = NanoClock.getDefaultClock(); | ||
this.hostServiceName = null; | ||
this.endpoint = null; | ||
this.mtlsEndpoint = null; | ||
this.quotaProjectId = null; | ||
|
@@ -292,6 +303,7 @@ protected Builder(ClientContext clientContext) { | |
this.internalHeaderProvider = | ||
FixedHeaderProvider.create(clientContext.getInternalHeaders()); | ||
this.clock = clientContext.getClock(); | ||
this.hostServiceName = clientContext.getHostServiceName(); | ||
this.endpoint = clientContext.getEndpoint(); | ||
if (this.endpoint != null) { | ||
this.mtlsEndpoint = this.endpoint.replace("googleapis.com", "mtls.googleapis.com"); | ||
|
@@ -414,6 +426,14 @@ public B setClock(ApiClock clock) { | |
return self(); | ||
} | ||
|
||
// This meant to be used internally by GAPIC clients to configure an endpoint. | ||
// Users should avoid settings this as it could change their endpoint. | ||
@InternalApi | ||
public B setHostServiceName(String hostServiceName) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have to expose a setter for it? Since the host service name is a fixed String, I think we should implement it in a similar way as the to-be-deprecated default endpoint, that we generate a getter that returns the default host service name. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see, I think that makes sense. I'll update it. |
||
this.hostServiceName = hostServiceName; | ||
return self(); | ||
} | ||
|
||
public B setEndpoint(String endpoint) { | ||
this.endpoint = endpoint; | ||
this.switchToMtlsEndpointAllowed = false; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remember we discussed that the service name could come from the proto or service yaml, is that still true? Or service name will always come from
defaultHost
in the proto?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, comment is misleading. I meant to write that the
hostServiceName is guaranteed
as the generator won't generate a library if it doesn't exist.It is still true. Checks the proto's default_host value first, then the service yaml file, and fails otherwise