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

fix: Return resolved endpoint from StubSettings' Builder #2715

Merged
merged 4 commits into from
May 2, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,8 @@ private GrpcCallContext(
this.retryableCodes = retryableCodes == null ? null : ImmutableSet.copyOf(retryableCodes);
// Attempt to create an empty, non-functioning EndpointContext by default. The client will have
// a valid EndpointContext with user configurations after the client has been initialized.
try {
this.endpointContext =
endpointContext == null ? EndpointContext.newBuilder().build() : endpointContext;
} catch (IOException ex) {
throw new RuntimeException(ex);
}
this.endpointContext =
endpointContext == null ? EndpointContext.getDefaultInstance() : endpointContext;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,8 @@ private HttpJsonCallContext(
defaultRetryableCodes == null ? null : ImmutableSet.copyOf(defaultRetryableCodes);
// Attempt to create an empty, non-functioning EndpointContext by default. The client will have
// a valid EndpointContext with user configurations after the client has been initialized.
try {
this.endpointContext =
endpointContext == null ? EndpointContext.newBuilder().build() : endpointContext;
} catch (IOException ex) {
throw new RuntimeException(ex);
}
this.endpointContext =
endpointContext == null ? EndpointContext.getDefaultInstance() : endpointContext;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,24 +126,20 @@ public abstract class ClientContext {

/** Create a new ClientContext with default values */
public static Builder newBuilder() {
try {
return new AutoValue_ClientContext.Builder()
.setBackgroundResources(Collections.<BackgroundResource>emptyList())
.setExecutor(Executors.newScheduledThreadPool(0))
.setHeaders(Collections.<String, String>emptyMap())
.setInternalHeaders(Collections.<String, String>emptyMap())
.setClock(NanoClock.getDefaultClock())
.setStreamWatchdog(null)
.setStreamWatchdogCheckInterval(Duration.ZERO)
.setTracerFactory(BaseApiTracerFactory.getInstance())
.setQuotaProjectId(null)
.setGdchApiAudience(null)
// Attempt to create an empty, non-functioning EndpointContext by default. This is
// not exposed to the user via getters/setters.
.setEndpointContext(EndpointContext.newBuilder().build());
} catch (IOException e) {
throw new RuntimeException(e);
}
return new AutoValue_ClientContext.Builder()
.setBackgroundResources(Collections.<BackgroundResource>emptyList())
.setExecutor(Executors.newScheduledThreadPool(0))
.setHeaders(Collections.<String, String>emptyMap())
.setInternalHeaders(Collections.<String, String>emptyMap())
.setClock(NanoClock.getDefaultClock())
.setStreamWatchdog(null)
.setStreamWatchdogCheckInterval(Duration.ZERO)
.setTracerFactory(BaseApiTracerFactory.getInstance())
.setQuotaProjectId(null)
.setGdchApiAudience(null)
// Attempt to create an empty, non-functioning EndpointContext by default. This is
// not exposed to the user via getters/setters.
.setEndpointContext(EndpointContext.getDefaultInstance());
}

public abstract Builder toBuilder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,28 @@
@InternalApi
@AutoValue
public abstract class EndpointContext {

private static final EndpointContext INSTANCE;

// static block initialization for exception handling
static {
try {
INSTANCE = EndpointContext.newBuilder().setServiceName("").build();
} catch (IOException e) {
throw new RuntimeException("Unable to create a default empty EndpointContext", e);
}
}

public static final String GOOGLE_CLOUD_UNIVERSE_DOMAIN = "GOOGLE_CLOUD_UNIVERSE_DOMAIN";
public static final String INVALID_UNIVERSE_DOMAIN_ERROR_TEMPLATE =
"The configured universe domain (%s) does not match the universe domain found in the credentials (%s). If you haven't configured the universe domain explicitly, `googleapis.com` is the default.";
public static final String UNABLE_TO_RETRIEVE_CREDENTIALS_ERROR_MESSAGE =
"Unable to retrieve the Universe Domain from the Credentials.";

public static EndpointContext getDefaultInstance() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can this be package private?

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'm using this inside GrpcCallContext (gax-grpc) and HttpJsonCallContext (gax-httpsjon) and can't access it otherwise.

return INSTANCE;
}

/**
* ServiceName is host URI for Google Cloud Services. It follows the format of
* `{ServiceName}.googleapis.com`. For example, speech.googleapis.com would have a ServiceName of
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@ public abstract static class Builder<
@Nonnull private ApiTracerFactory tracerFactory;
private boolean deprecatedExecutorProviderSet;
private String universeDomain;
private final EndpointContext endpointContext;

/**
* Indicate when creating transport whether it is allowed to use mTLS endpoint instead of the
Expand Down Expand Up @@ -300,6 +301,9 @@ protected Builder(StubSettings settings) {
this.switchToMtlsEndpointAllowed =
settings.getEndpointContext().switchToMtlsEndpointAllowed();
this.universeDomain = settings.getEndpointContext().universeDomain();
// Store the EndpointContext that was already created. This is used to return the state
// of the EndpointContext prior to any new modifications
this.endpointContext = settings.getEndpointContext();
}

/** Get Quota Project ID from Client Context * */
Expand Down Expand Up @@ -327,16 +331,22 @@ protected Builder(ClientContext clientContext) {
this.headerProvider = new NoHeaderProvider();
this.internalHeaderProvider = new NoHeaderProvider();
this.clock = NanoClock.getDefaultClock();
this.clientSettingsEndpoint = null;
this.transportChannelProviderEndpoint = null;
this.mtlsEndpoint = null;
this.quotaProjectId = null;
this.streamWatchdogProvider = InstantiatingWatchdogProvider.create();
this.streamWatchdogCheckInterval = Duration.ofSeconds(10);
this.tracerFactory = BaseApiTracerFactory.getInstance();
this.deprecatedExecutorProviderSet = false;
this.gdchApiAudience = null;

this.clientSettingsEndpoint = null;
this.transportChannelProviderEndpoint = null;
this.mtlsEndpoint = null;
this.switchToMtlsEndpointAllowed = false;
this.universeDomain = null;
// Attempt to create an empty, non-functioning EndpointContext by default. The client will
// have
// a valid EndpointContext with user configurations after the client has been initialized.
this.endpointContext = EndpointContext.getDefaultInstance();
} else {
ExecutorProvider fixedExecutorProvider =
FixedExecutorProvider.create(clientContext.getExecutor());
Expand Down Expand Up @@ -365,6 +375,9 @@ protected Builder(ClientContext clientContext) {
this.switchToMtlsEndpointAllowed =
clientContext.getEndpointContext().switchToMtlsEndpointAllowed();
this.universeDomain = clientContext.getEndpointContext().universeDomain();
// Store the EndpointContext that was already created. This is used to return the state
// of the EndpointContext prior to any new modifications
this.endpointContext = clientContext.getEndpointContext();
}
}

Expand Down Expand Up @@ -584,8 +597,13 @@ public ApiClock getClock() {
return clock;
}

/**
* @return the resolved endpoint when the Builder was created. If other parameters are then set
* in the builder, the resolved endpoint is not automatically updated. The resolved endpoint
* will only be recomputed when the StubSettings is built again.
*/
public String getEndpoint() {
return clientSettingsEndpoint;
return endpointContext.resolvedEndpoint();
Copy link
Collaborator

Choose a reason for hiding this comment

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

A fully compatible solution might be

// For newBuilder() scenario.
if (endpointContext.equals(EndpointContext.getDefaultInstance())) {
  return clientSettingsEndpoint;
}
return endpointContext.resolvedEndpoint();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I think you are right. A valid StubSettings converted to a Builder will still be able to have the resolved endpoint.

}

public String getMtlsEndpoint() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import java.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -88,12 +87,8 @@ private FakeCallContext(
this.tracer = tracer;
this.retrySettings = retrySettings;
this.retryableCodes = retryableCodes == null ? null : ImmutableSet.copyOf(retryableCodes);
try {
this.endpointContext =
endpointContext == null ? EndpointContext.newBuilder().build() : endpointContext;
} catch (IOException e) {
throw new RuntimeException(e);
}
this.endpointContext =
endpointContext == null ? EndpointContext.getDefaultInstance() : endpointContext;
}

public static FakeCallContext createDefault() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -416,17 +416,36 @@ public void universeDomainValidation_noCredentials_userSetUniverseDomain() throw
@Test
public void endpointResolution_defaultViaBuilder() {
EchoSettings.Builder echoSettingsBuilder = EchoSettings.newBuilder();
// The getter in the builder returns the user set value. No configuration
// means the getter will return null
Truth.assertThat(echoSettingsBuilder.getEndpoint()).isEqualTo(null);
// Showcase clients do not have a serviceName. The resolved endpoint will use `""`
// as the serviceName.
Truth.assertThat(echoSettingsBuilder.getEndpoint()).isEqualTo(".googleapis.com:443");
}

// User configuration in Builder
@Test
public void endpointResolution_userConfigurationViaBuilder() {
String customEndpoint = "test.com:123";
EchoSettings.Builder echoSettingsBuilder =
EchoSettings.newBuilder().setEndpoint(customEndpoint);
Truth.assertThat(echoSettingsBuilder.getEndpoint()).isEqualTo(customEndpoint);
EchoSettings.newBuilder().setEndpoint("test.com:123");
// EndpointContext is not automatically recomputed for every setter called in the Builder.
// It is initially computed when the Builder is created.
Truth.assertThat(echoSettingsBuilder.getEndpoint()).isEqualTo(".googleapis.com:443");
}

@Test
public void endpointResolution_builderBuilderBackToBuilder() throws IOException {
String customEndpoint = "test.com:123";
EchoStubSettings.Builder echoStubSettingsBuilder =
EchoStubSettings.newBuilder().setEndpoint(customEndpoint);
// EndpointContext is not automatically recomputed for every setter called in the Builder.
// It is initially computed when the Builder is created.
Truth.assertThat(echoStubSettingsBuilder.getEndpoint()).isEqualTo(".googleapis.com:443");

// EndpointContext is recomputed when the Builder is re-built
EchoStubSettings echoStubSettings = echoStubSettingsBuilder.build();
Truth.assertThat(echoStubSettings.getEndpoint()).isEqualTo(customEndpoint);

// Calling toBuilder on StubSettings keeps the configurations the same
echoStubSettingsBuilder = echoStubSettings.toBuilder();
Truth.assertThat(echoStubSettingsBuilder.getEndpoint()).isEqualTo(customEndpoint);
}
}
Loading