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

feat: Parse Host Service Name #2300

Merged
merged 14 commits into from Dec 18, 2023
Merged
Show file tree
Hide file tree
Changes from 11 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
Expand Up @@ -155,6 +155,8 @@ public abstract class AbstractServiceStubSettingsClassComposer implements ClassC
protected static final VariableExpr DEFAULT_SERVICE_SCOPES_VAR_EXPR =
createDefaultServiceScopesVarExpr();

private static final VariableExpr HOST_SERVICE_NAME_VAR_EXPR = createHostServiceNameVarExpr();

protected AbstractServiceStubSettingsClassComposer(TransportContext transportContext) {
this.transportContext = transportContext;
}
Expand Down Expand Up @@ -492,6 +494,23 @@ private static List<Statement> createClassStatements(
Function<Expr, Statement> exprToStatementFn = e -> ExprStatement.withExpr(e);
List<Statement> statements = new ArrayList<>();

VariableExpr hostServiceNameVarExpr =
HOST_SERVICE_NAME_VAR_EXPR
.toBuilder()
.setIsDecl(true)
.setScope(ScopeNode.PRIVATE)
.setIsStatic(true)
.setIsFinal(true)
.build();

statements.add(
exprToStatementFn.apply(
AssignmentExpr.builder()
.setVariableExpr(hostServiceNameVarExpr)
.setValueExpr(
ValueExpr.withValue(StringObjectValue.withValue(service.hostServiceName())))
.build()));

// Assign DEFAULT_SERVICE_SCOPES.
statements.add(SettingsCommentComposer.DEFAULT_SCOPES_COMMENT);
VariableExpr defaultServiceScopesDeclVarExpr =
Expand Down Expand Up @@ -1899,6 +1918,12 @@ protected MethodDefinition createNestedClassCreateDefaultMethod(
ValueExpr.withValue(
PrimitiveValue.builder().setType(TypeNode.BOOLEAN).setValue("true").build()))
.build());
bodyExprs.add(
MethodInvocationExpr.builder()
.setExprReferenceExpr(builderVarExpr)
.setMethodName("setHostServiceName")
.setArguments(HOST_SERVICE_NAME_VAR_EXPR)
.build());
bodyStatements.addAll(
bodyExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList()));
bodyStatements.add(EMPTY_LINE_STATEMENT);
Expand Down Expand Up @@ -2190,6 +2215,11 @@ private static VariableExpr createNestedRetryParamDefinitionsVarExpr() {
.build());
}

private static VariableExpr createHostServiceNameVarExpr() {
return VariableExpr.withVariable(
Variable.builder().setName("HOST_SERVICE_NAME").setType(TypeNode.STRING).build());
}

private static String getPagedResponseTypeName(String methodName) {
return String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, JavaStyle.toUpperCamelCase(methodName));
}
Expand Down
Expand Up @@ -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
Copy link
Collaborator

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?

Copy link
Contributor Author

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.

I remember we discussed that the service name could come from the proto or service yaml, is that still true?

It is still true. Checks the proto's default_host value first, then the service yaml file, and fails otherwise

// Parser will fail if the default host is not supplied
return parseHostServiceName(defaultHost());
}

public String apiShortName() {
if (!Strings.isNullOrEmpty(defaultHost())) {
return parseApiShortName(defaultHost());
Expand Down Expand Up @@ -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) {
Copy link
Collaborator

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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".
Expand Down
Expand Up @@ -69,6 +69,7 @@ import org.threeten.bp.Duration;
@Deprecated
@Generated("by gapic-generator-java")
public class DeprecatedServiceStubSettings extends StubSettings<DeprecatedServiceStubSettings> {
private static final String HOST_SERVICE_NAME = "";
/** The default scopes of the service. */
private static final ImmutableList<String> DEFAULT_SERVICE_SCOPES =
ImmutableList.<String>builder().add("https://www.googleapis.com/auth/cloud-platform").build();
Expand Down Expand Up @@ -238,6 +239,7 @@ public class DeprecatedServiceStubSettings extends StubSettings<DeprecatedServic
builder.setEndpoint(getDefaultEndpoint());
builder.setMtlsEndpoint(getDefaultMtlsEndpoint());
builder.setSwitchToMtlsEndpointAllowed(true);
builder.setHostServiceName(HOST_SERVICE_NAME);

return initDefaults(builder);
}
Expand Down
Expand Up @@ -91,6 +91,7 @@ import org.threeten.bp.Duration;
@BetaApi
@Generated("by gapic-generator-java")
public class EchoStubSettings extends StubSettings<EchoStubSettings> {
private static final String HOST_SERVICE_NAME = "";
/** The default scopes of the service. */
private static final ImmutableList<String> DEFAULT_SERVICE_SCOPES =
ImmutableList.<String>builder().add("https://www.googleapis.com/auth/cloud-platform").build();
Expand Down Expand Up @@ -487,6 +488,7 @@ public class EchoStubSettings extends StubSettings<EchoStubSettings> {
builder.setEndpoint(getDefaultEndpoint());
builder.setMtlsEndpoint(getDefaultMtlsEndpoint());
builder.setSwitchToMtlsEndpointAllowed(true);
builder.setHostServiceName(HOST_SERVICE_NAME);

return initDefaults(builder);
}
Expand Down
Expand Up @@ -98,6 +98,7 @@ import org.threeten.bp.Duration;
*/
@Generated("by gapic-generator-java")
public class LoggingServiceV2StubSettings extends StubSettings<LoggingServiceV2StubSettings> {
private static final String HOST_SERVICE_NAME = "logging";
/** The default scopes of the service. */
private static final ImmutableList<String> DEFAULT_SERVICE_SCOPES =
ImmutableList.<String>builder()
Expand Down Expand Up @@ -623,6 +624,7 @@ public class LoggingServiceV2StubSettings extends StubSettings<LoggingServiceV2S
builder.setEndpoint(getDefaultEndpoint());
builder.setMtlsEndpoint(getDefaultMtlsEndpoint());
builder.setSwitchToMtlsEndpointAllowed(true);
builder.setHostServiceName(HOST_SERVICE_NAME);

return initDefaults(builder);
}
Expand Down
Expand Up @@ -98,6 +98,7 @@ import org.threeten.bp.Duration;
*/
@Generated("by gapic-generator-java")
public class PublisherStubSettings extends StubSettings<PublisherStubSettings> {
private static final String HOST_SERVICE_NAME = "pubsub";
/** The default scopes of the service. */
private static final ImmutableList<String> DEFAULT_SERVICE_SCOPES =
ImmutableList.<String>builder()
Expand Down Expand Up @@ -664,6 +665,7 @@ public class PublisherStubSettings extends StubSettings<PublisherStubSettings> {
builder.setEndpoint(getDefaultEndpoint());
builder.setMtlsEndpoint(getDefaultMtlsEndpoint());
builder.setSwitchToMtlsEndpointAllowed(true);
builder.setHostServiceName(HOST_SERVICE_NAME);

return initDefaults(builder);
}
Expand Down
Expand Up @@ -96,6 +96,7 @@ import org.threeten.bp.Duration;
@BetaApi
@Generated("by gapic-generator-java")
public class EchoStubSettings extends StubSettings<EchoStubSettings> {
private static final String HOST_SERVICE_NAME = "";
/** The default scopes of the service. */
private static final ImmutableList<String> DEFAULT_SERVICE_SCOPES =
ImmutableList.<String>builder().add("https://www.googleapis.com/auth/cloud-platform").build();
Expand Down Expand Up @@ -515,6 +516,7 @@ public class EchoStubSettings extends StubSettings<EchoStubSettings> {
builder.setEndpoint(getDefaultEndpoint());
builder.setMtlsEndpoint(getDefaultMtlsEndpoint());
builder.setSwitchToMtlsEndpointAllowed(true);
builder.setHostServiceName(HOST_SERVICE_NAME);

return initDefaults(builder);
}
Expand All @@ -528,6 +530,7 @@ public class EchoStubSettings extends StubSettings<EchoStubSettings> {
builder.setEndpoint(getDefaultEndpoint());
builder.setMtlsEndpoint(getDefaultMtlsEndpoint());
builder.setSwitchToMtlsEndpointAllowed(true);
builder.setHostServiceName(HOST_SERVICE_NAME);

return initDefaults(builder);
}
Expand Down
Expand Up @@ -65,6 +65,7 @@ import javax.annotation.Generated;
@BetaApi
@Generated("by gapic-generator-java")
public class WickedStubSettings extends StubSettings<WickedStubSettings> {
private static final String HOST_SERVICE_NAME = "";
/** The default scopes of the service. */
private static final ImmutableList<String> DEFAULT_SERVICE_SCOPES =
ImmutableList.<String>builder().build();
Expand Down Expand Up @@ -234,6 +235,7 @@ public class WickedStubSettings extends StubSettings<WickedStubSettings> {
builder.setEndpoint(getDefaultEndpoint());
builder.setMtlsEndpoint(getDefaultMtlsEndpoint());
builder.setSwitchToMtlsEndpointAllowed(true);
builder.setHostServiceName(HOST_SERVICE_NAME);

return initDefaults(builder);
}
Expand Down
Expand Up @@ -66,6 +66,7 @@ import javax.annotation.Generated;
@BetaApi
@Generated("by gapic-generator-java")
public class ComplianceStubSettings extends StubSettings<ComplianceStubSettings> {
private static final String HOST_SERVICE_NAME = "";
/** The default scopes of the service. */
private static final ImmutableList<String> DEFAULT_SERVICE_SCOPES =
ImmutableList.<String>builder().build();
Expand Down Expand Up @@ -305,6 +306,7 @@ public class ComplianceStubSettings extends StubSettings<ComplianceStubSettings>
builder.setEndpoint(getDefaultEndpoint());
builder.setMtlsEndpoint(getDefaultMtlsEndpoint());
builder.setSwitchToMtlsEndpointAllowed(true);
builder.setHostServiceName(HOST_SERVICE_NAME);

return initDefaults(builder);
}
Expand Down
Expand Up @@ -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();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we call this field serviceName in Gax and generated stubs? It makes sense to call ithostServiceName in the generator as serviceName could be confused with Service.name(), but in Gax I think it makes more sense to just call it serviceName.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'll rename to serviceName.


@Nullable
public abstract String getEndpoint();

Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand Down
Expand Up @@ -40,6 +40,15 @@
@InternalApi
@AutoValue
public abstract class EndpointContext {
/**
* HostServiceName is HostName for Google Cloud Services. It follows the format of
* `{HostServiceName}.googleapis.com`. For example, speech.googleapis.com would have a
* HostServiceName of speech and cloudasset.googleapis.com would have a HostServiceName of
* cloudasset.
*/
@Nullable
public abstract String hostServiceName();

/**
* ClientSettingsEndpoint is the endpoint value set via the ClientSettings/StubSettings classes.
*/
Expand Down Expand Up @@ -117,6 +126,14 @@ public String getResolvedEndpoint() {

@AutoValue.Builder
public abstract static class Builder {
/**
* HostServiceName is HostName for Google Cloud Services. It follows the format of
* `{HostServiceName}.googleapis.com`. For example, speech.googleapis.com would have a
* HostServiceName of speech and cloudasset.googleapis.com would have a HostServiceName of
* cloudasset.
*/
public abstract Builder setHostServiceName(String hostServiceName);

/**
* ClientSettingsEndpoint is the endpoint value set via the ClientSettings/StubSettings classes.
*/
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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");
Expand Down Expand Up @@ -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) {
Copy link
Collaborator

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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;
Expand Down
Expand Up @@ -103,6 +103,7 @@
@BetaApi
@Generated("by gapic-generator-java")
public class ComplianceStubSettings extends StubSettings<ComplianceStubSettings> {
private static final String HOST_SERVICE_NAME = "";
/** The default scopes of the service. */
private static final ImmutableList<String> DEFAULT_SERVICE_SCOPES =
ImmutableList.<String>builder().build();
Expand Down Expand Up @@ -519,6 +520,7 @@ private static Builder createDefault() {
builder.setEndpoint(getDefaultEndpoint());
builder.setMtlsEndpoint(getDefaultMtlsEndpoint());
builder.setSwitchToMtlsEndpointAllowed(true);
builder.setHostServiceName(HOST_SERVICE_NAME);

return initDefaults(builder);
}
Expand All @@ -532,6 +534,7 @@ private static Builder createHttpJsonDefault() {
builder.setEndpoint(getDefaultEndpoint());
builder.setMtlsEndpoint(getDefaultMtlsEndpoint());
builder.setSwitchToMtlsEndpointAllowed(true);
builder.setHostServiceName(HOST_SERVICE_NAME);

return initDefaults(builder);
}
Expand Down