Skip to content

Commit 73c94c2

Browse files
committed
fix: suggested changes
1 parent 7fc22d7 commit 73c94c2

5 files changed

Lines changed: 68 additions & 14 deletions

File tree

api/src/main/java/io/grpc/ManagedChannelProvider.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ protected NewChannelBuilderResult newChannelBuilder(String target, ChannelCreden
9898
* search the registry
9999
* @return a {@link NewChannelBuilderResult} containing either the builder or an
100100
* error description
101-
* @since 1.79.0
101+
* @since 1.81.0
102102
*/
103103
protected NewChannelBuilderResult newChannelBuilder(String target, ChannelCredentials creds,
104104
NameResolverRegistry nameResolverRegistry,

api/src/main/java/io/grpc/NameResolverProvider.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public abstract class NameResolverProvider extends NameResolver.Factory {
6565
*
6666
* @since 1.40.0
6767
* */
68-
protected String getScheme() {
68+
public String getScheme() {
6969
return getDefaultScheme();
7070
}
7171

core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -762,7 +762,7 @@ public ManagedChannel build() {
762762
clientTransportFactoryBuilder.buildClientTransportFactory();
763763
ResolvedNameResolver resolvedResolver =
764764
InternalFeatureFlags.getRfc3986UrisEnabled()
765-
? getNameResolverProviderRfc3986(target, nameResolverRegistry)
765+
? getNameResolverProviderRfc3986(target, nameResolverRegistry, nameResolverProvider)
766766
: getNameResolverProvider(target, nameResolverRegistry, nameResolverProvider);
767767
resolvedResolver.checkAddressTypes(clientTransportFactory.getSupportedSocketAddressTypes());
768768
return new ManagedChannelOrphanWrapper(new ManagedChannelImpl(
@@ -902,7 +902,7 @@ static ResolvedNameResolver getNameResolverProvider(
902902
// parsed as the scheme. Will hit the next case and try "dns:///localhost:8080".
903903
// Use the explicit provider if its scheme matches the target URI.
904904
if (nameResolverProvider != null
905-
&& targetUri.getScheme().equals(nameResolverProvider.getDefaultScheme())) {
905+
&& targetUri.getScheme().equals(nameResolverProvider.getScheme())) {
906906
provider = nameResolverProvider;
907907
} else {
908908
provider = nameResolverRegistry.getProviderForScheme(targetUri.getScheme());
@@ -914,7 +914,7 @@ static ResolvedNameResolver getNameResolverProvider(
914914
// the default scheme from the registry (if provider is not specified) or
915915
// the provider's default scheme (if provider is specified).
916916
String scheme = nameResolverProvider != null
917-
? nameResolverProvider.getDefaultScheme()
917+
? nameResolverProvider.getScheme()
918918
: nameResolverRegistry.getDefaultScheme();
919919
try {
920920
targetUri = new URI(scheme, "", "/" + target, null);
@@ -940,7 +940,8 @@ static ResolvedNameResolver getNameResolverProvider(
940940

941941
@VisibleForTesting
942942
static ResolvedNameResolver getNameResolverProviderRfc3986(
943-
String target, NameResolverRegistry nameResolverRegistry) {
943+
String target, NameResolverRegistry nameResolverRegistry,
944+
NameResolverProvider nameResolverProvider) {
944945
// Finding a NameResolver. Try using the target string as the URI. If that fails, try prepending
945946
// "dns:///".
946947
NameResolverProvider provider = null;
@@ -955,15 +956,25 @@ static ResolvedNameResolver getNameResolverProviderRfc3986(
955956
if (targetUri != null) {
956957
// For "localhost:8080" this would likely cause provider to be null, because "localhost" is
957958
// parsed as the scheme. Will hit the next case and try "dns:///localhost:8080".
958-
provider = nameResolverRegistry.getProviderForScheme(targetUri.getScheme());
959+
// Use the explicit provider if its scheme matches the target URI.
960+
if (nameResolverProvider != null
961+
&& targetUri.getScheme().equals(nameResolverProvider.getScheme())) {
962+
provider = nameResolverProvider;
963+
} else {
964+
provider = nameResolverRegistry.getProviderForScheme(targetUri.getScheme());
965+
}
959966
}
960967

961968
if (provider == null && !URI_PATTERN.matcher(target).matches()) {
962-
// It doesn't look like a URI target. Maybe it's an authority string. Try with the default
963-
// scheme from the registry.
969+
// It doesn't look like a URI target. Maybe it's an authority string. Try with
970+
// the default scheme from the registry (if provider is not specified) or
971+
// the provider's default scheme (if provider is specified).
972+
String scheme = nameResolverProvider != null
973+
? nameResolverProvider.getScheme()
974+
: nameResolverRegistry.getDefaultScheme();
964975
targetUri =
965976
Uri.newBuilder()
966-
.setScheme(nameResolverRegistry.getDefaultScheme())
977+
.setScheme(scheme)
967978
.setHost("")
968979
.setPath("/" + target)
969980
.build();

core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -808,6 +808,7 @@ public void getNameResolverProvider_explicitProviderWithIpTarget() {
808808
String target = "127.0.0.1:8080";
809809
NameResolverRegistry registry = new NameResolverRegistry();
810810
NameResolverProvider explicitProvider = mock(NameResolverProvider.class);
811+
when(explicitProvider.getScheme()).thenReturn("dns");
811812
when(explicitProvider.getDefaultScheme()).thenReturn("dns");
812813

813814
ManagedChannelImplBuilder.ResolvedNameResolver resolved;
@@ -823,6 +824,7 @@ public void getNameResolverProvider_explicitProviderWithInvalidUri() {
823824
String target = "::1";
824825
NameResolverRegistry registry = new NameResolverRegistry();
825826
NameResolverProvider explicitProvider = mock(NameResolverProvider.class);
827+
when(explicitProvider.getScheme()).thenReturn("dns");
826828
when(explicitProvider.getDefaultScheme()).thenReturn("dns");
827829

828830
ManagedChannelImplBuilder.ResolvedNameResolver resolved;
@@ -907,4 +909,43 @@ protected int priority() {
907909

908910
assertThat(resolved.provider).isSameInstanceAs(registryProvider);
909911
}
912+
913+
@Test
914+
public void getNameResolverProviderRfc3986_explicitProviderWithAuthorityTarget() {
915+
String target = "authority-target";
916+
NameResolverRegistry registry = new NameResolverRegistry();
917+
NameResolverProvider explicitProvider = new NameResolverProvider() {
918+
@Override
919+
public NameResolver newNameResolver(URI targetUri, NameResolver.Args args) {
920+
return null;
921+
}
922+
923+
@Override
924+
public String getScheme() {
925+
return "customscheme";
926+
}
927+
928+
@Override
929+
public String getDefaultScheme() {
930+
return "customscheme";
931+
}
932+
933+
@Override
934+
protected boolean isAvailable() {
935+
return true;
936+
}
937+
938+
@Override
939+
protected int priority() {
940+
return 5;
941+
}
942+
};
943+
registry.register(explicitProvider);
944+
945+
ManagedChannelImplBuilder.ResolvedNameResolver resolved = ManagedChannelImplBuilder
946+
.getNameResolverProviderRfc3986(target, registry, explicitProvider);
947+
948+
assertThat(resolved.provider).isSameInstanceAs(explicitProvider);
949+
assertThat(resolved.targetUri.toString()).isEqualTo("customscheme:///authority-target");
950+
}
910951
}

core/src/test/java/io/grpc/internal/ManagedChannelImplGetNameResolverRfc3986Test.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ public void validTargetNoProvider() {
136136
NameResolverRegistry nameResolverRegistry = new NameResolverRegistry();
137137
try {
138138
ManagedChannelImplBuilder.getNameResolverProviderRfc3986(
139-
"foo.googleapis.com:8080", nameResolverRegistry);
139+
"foo.googleapis.com:8080", nameResolverRegistry, null);
140140
fail("Should fail");
141141
} catch (IllegalArgumentException e) {
142142
// expected
@@ -148,7 +148,7 @@ public void validTargetProviderAddrTypesNotSupported() {
148148
NameResolverRegistry nameResolverRegistry = getTestRegistry("testscheme");
149149
try {
150150
ManagedChannelImplBuilder.getNameResolverProviderRfc3986(
151-
"testscheme:///foo.googleapis.com:8080", nameResolverRegistry)
151+
"testscheme:///foo.googleapis.com:8080", nameResolverRegistry, null)
152152
.checkAddressTypes(Collections.singleton(CustomSocketAddress.class));
153153
fail("Should fail");
154154
} catch (IllegalArgumentException e) {
@@ -163,7 +163,8 @@ public void validTargetProviderAddrTypesNotSupported() {
163163
private void testValidTarget(String target, String expectedUriString, Uri expectedUri) {
164164
NameResolverRegistry nameResolverRegistry = getTestRegistry(expectedUri.getScheme());
165165
ManagedChannelImplBuilder.ResolvedNameResolver resolved =
166-
ManagedChannelImplBuilder.getNameResolverProviderRfc3986(target, nameResolverRegistry);
166+
ManagedChannelImplBuilder.getNameResolverProviderRfc3986(target, nameResolverRegistry,
167+
null);
167168
assertThat(resolved.provider).isInstanceOf(FakeNameResolverProvider.class);
168169
assertThat(resolved.targetUri).isEqualTo(wrap(expectedUri));
169170
assertThat(resolved.targetUri.toString()).isEqualTo(expectedUriString);
@@ -174,7 +175,8 @@ private void testInvalidTarget(String target) {
174175

175176
try {
176177
ManagedChannelImplBuilder.ResolvedNameResolver resolved =
177-
ManagedChannelImplBuilder.getNameResolverProviderRfc3986(target, nameResolverRegistry);
178+
ManagedChannelImplBuilder.getNameResolverProviderRfc3986(target, nameResolverRegistry,
179+
null);
178180
FakeNameResolverProvider nameResolverProvider = (FakeNameResolverProvider) resolved.provider;
179181
fail("Should have failed, but got resolver provider " + nameResolverProvider);
180182
} catch (IllegalArgumentException e) {

0 commit comments

Comments
 (0)