Skip to content

Commit

Permalink
fix: Endpoint resolution uses user set endpoint from ClientSettings (#…
Browse files Browse the repository at this point in the history
…2429)

* fix: Endpoint resolution uses the user set endpoint

* chore: Rename to userSetEndpoint()

* chore: Empty commit

* chore: Fix showcase tests
  • Loading branch information
lqiu96 committed Jan 29, 2024
1 parent 363e35e commit 46b0a85
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ public static ClientContext create(StubSettings settings) throws IOException {
EndpointContext.newBuilder()
.setServiceName(settings.getServiceName())
.setUniverseDomain(settings.getUniverseDomain())
.setClientSettingsEndpoint(settings.getEndpoint())
.setClientSettingsEndpoint(settings.getUserSetEndpoint())
.setTransportChannelProviderEndpoint(
settings.getTransportChannelProvider().getEndpoint())
.setMtlsEndpoint(settings.getMtlsEndpoint())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,17 @@ public String getEndpoint() {
return endpoint;
}

/**
* This is an internal api meant to either return the user set endpoint or null. The difference
* between this method and {@link #getEndpoint()}} is that {@link #getEndpoint()} is reimplemented
* by the child class and will return the default service endpoint if the user did not set an
* endpoint (does not return null).
*/
@InternalApi
String getUserSetEndpoint() {
return endpoint;
}

public final String getMtlsEndpoint() {
return mtlsEndpoint;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,23 @@ public class ITEndpointContext {
public static final String SHOWCASE_DEFAULT_ENDPOINT = "localhost:7469";

// Default (no configuration)
// This test is very similar to `endpointResolution_userConfiguration`. This test is kept
// as future enhancements could allow this test to not have to explicitly set the endpoint.
@Test
public void endpointResolution_default() throws InterruptedException, IOException {
EchoClient echoClient = null;
try {
// The default usage is EchoClient.create(), but for showcase tests run in CI, the
// This is not how a client is created by default:
// 1. The default usage is EchoClient.create(), but for showcase tests run in CI, the
// client must be supplied with Credentials.
// 2. The default configuration does not set an endpoint. Showcase clients do not have
// a serviceName (and this cannot be configured by the user). Set the endpoint
// to simulate the endpointContext creating it with a proper serviceName.
EchoSettings echoSettings =
EchoSettings.newBuilder().setCredentialsProvider(NoCredentialsProvider.create()).build();
EchoSettings.newBuilder()
.setCredentialsProvider(NoCredentialsProvider.create())
.setEndpoint(SHOWCASE_DEFAULT_ENDPOINT)
.build();
echoClient = EchoClient.create(echoSettings);
Truth.assertThat(echoClient.getSettings().getEndpoint()).isEqualTo(SHOWCASE_DEFAULT_ENDPOINT);
} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ public static ComplianceClient createGrpcComplianceClient(List<ClientInterceptor
.setChannelConfigurator(ManagedChannelBuilder::usePlaintext)
.setInterceptorProvider(() -> interceptorList)
.build())
.setEndpoint("localhost:7469")
.build();
return ComplianceClient.create(grpcComplianceSettings);
}
Expand Down

0 comments on commit 46b0a85

Please sign in to comment.