Skip to content
29 changes: 29 additions & 0 deletions api/src/main/java/io/grpc/Grpc.java
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,35 @@ public static ManagedChannelBuilder<?> newChannelBuilder(
return ManagedChannelRegistry.getDefaultRegistry().newChannelBuilder(target, creds);
}

/**
* Creates a channel builder with a target string, credentials, and a specific
* name resolver registry.
*
* <p>This method uses the {@link ManagedChannelRegistry#getDefaultRegistry()} to
Copy link
Member

Choose a reason for hiding this comment

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

Let's not mention the ManagedChannelRegistry, as users shouldn't care. We didn't mention it in the newChannelBuilder(String, ChannelCredentials) version and I don't think it hurt users at all.

* find an appropriate underlying transport provider based on the target and credentials.
* The provided {@code nameResolverRegistry} is used to resolve the target address
* into physical addresses (e.g., DNS or custom schemes).
*
* @param target the target URI for the channel, such as {@code "localhost:8080"}
* or {@code "dns:///example.com"}
* @param creds the channel credentials to use for secure communication
* @param nameResolverRegistry the registry used to look up {@link NameResolver}
* providers for the target
* @return a {@link ManagedChannelBuilder} instance configured with the given parameters
* @throws IllegalArgumentException if no provider is available for the given target
* or credentials
* @since 1.79.0
*/
public static ManagedChannelBuilder<?> newChannelBuilder(
Copy link
Member

Choose a reason for hiding this comment

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

Add @ExperimentalApi

String target,
ChannelCredentials creds,
NameResolverRegistry nameResolverRegistry) {
return ManagedChannelRegistry.getDefaultRegistry().newChannelBuilder(
nameResolverRegistry,
target,
creds);
}

/**
* Creates a channel builder from a host, port, and credentials. The host and port are combined to
* form an authority string and then passed to {@link #newChannelBuilder(String,
Expand Down
25 changes: 25 additions & 0 deletions api/src/main/java/io/grpc/ManagedChannelProvider.java
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,31 @@ protected NewChannelBuilderResult newChannelBuilder(String target, ChannelCreden
return NewChannelBuilderResult.error("ChannelCredentials are unsupported");
}

/**
* Creates a channel builder using the provided target, credentials, and resolution
* components.
*
* <p>This method allows for fine-grained control over name resolution by providing
* both a {@link NameResolverRegistry} and a specific {@link NameResolverProvider}.
* Unlike the public factory methods, this returns a {@link NewChannelBuilderResult},
Copy link
Member

Choose a reason for hiding this comment

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

It is pretty unclear what "public factory methods" means. (I figured it out, but it wasn't clear.)

* which may contain an error string if the provided credentials or target are
* not supported by this provider.
*
* @param target the target URI for the channel
* @param creds the channel credentials to use
* @param nameResolverRegistry the registry used for looking up name resolvers
* @param nameResolverProvider a specific provider to use, or {@code null} to
* search the registry
* @return a {@link NewChannelBuilderResult} containing either the builder or an
* error description
* @since 1.79.0
*/
protected NewChannelBuilderResult newChannelBuilder(String target, ChannelCredentials creds,
NameResolverRegistry nameResolverRegistry,
NameResolverProvider nameResolverProvider) {
return newChannelBuilder(target, creds);
}

/**
* Returns the {@link SocketAddress} types this ManagedChannelProvider supports.
*/
Expand Down
3 changes: 1 addition & 2 deletions api/src/main/java/io/grpc/ManagedChannelRegistry.java
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ ManagedChannelBuilder<?> newChannelBuilder(String target, ChannelCredentials cre
return newChannelBuilder(NameResolverRegistry.getDefaultRegistry(), target, creds);
}

@VisibleForTesting
Copy link
Member

Choose a reason for hiding this comment

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

Why was this removed?

ManagedChannelBuilder<?> newChannelBuilder(NameResolverRegistry nameResolverRegistry,
String target, ChannelCredentials creds) {
NameResolverProvider nameResolverProvider = null;
Expand Down Expand Up @@ -198,7 +197,7 @@ ManagedChannelBuilder<?> newChannelBuilder(NameResolverRegistry nameResolverRegi
continue;
}
ManagedChannelProvider.NewChannelBuilderResult result
= provider.newChannelBuilder(target, creds);
= provider.newChannelBuilder(target, creds, nameResolverRegistry, nameResolverProvider);
if (result.getChannelBuilder() != null) {
return result.getChannelBuilder();
}
Expand Down
2 changes: 1 addition & 1 deletion api/src/main/java/io/grpc/NameResolver.java
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ public abstract static class Factory {
*/
public NameResolver newNameResolver(Uri targetUri, final Args args) {
// Not every io.grpc.Uri can be converted but in the ordinary ManagedChannel creation flow,
// any IllegalArgumentException thrown here would happened anyway, just earlier. That's
// any IllegalArgumentException thrown here would have happened anyway, just earlier. That's
// because parse/toString is transparent so java.net.URI#create here sees the original target
// string just like it did before the io.grpc.Uri migration.
//
Expand Down
47 changes: 47 additions & 0 deletions api/src/test/java/io/grpc/ManagedChannelRegistryTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,53 @@ public NewChannelBuilderResult newChannelBuilder(
mcb);
}

@Test
public void newChannelBuilder_propagatesRegistry() {
final NameResolverRegistry nameResolverRegistry = new NameResolverRegistry();
class SocketAddress1 extends SocketAddress {
}

ManagedChannelRegistry registry = new ManagedChannelRegistry();
class MockChannelBuilder extends ForwardingChannelBuilder2<MockChannelBuilder> {
@Override
public ManagedChannelBuilder<?> delegate() {
throw new UnsupportedOperationException();
}
}

final ManagedChannelBuilder<?> mcb = new MockChannelBuilder();
registry.register(new BaseProvider(true, 4) {
@Override
protected Collection<Class<? extends SocketAddress>> getSupportedSocketAddressTypes() {
return Collections.singleton(SocketAddress1.class);
}

@Override
public NewChannelBuilderResult newChannelBuilder(
String passedTarget, ChannelCredentials passedCreds,
NameResolverRegistry passedRegistry, NameResolverProvider passedProvider) {
assertThat(passedRegistry).isSameInstanceAs(nameResolverRegistry);
return NewChannelBuilderResult.channelBuilder(mcb);
}
});

// ManagedChannelRegistry.newChannelBuilder(NameResolverRegistry, String, ChannelCredentials)
// gets the scheme from target. Then it gets NameResolverProvider from registry for that scheme.
// Then it gets producedSocketAddressTypes from that provider.
// Then it finds a ManagedChannelProvider that supports those types.
// So we need a registered NameResolverProvider for the scheme.
nameResolverRegistry.register(new BaseNameResolverProvider(true, 5, "sc1") {
@Override
public Collection<Class<? extends SocketAddress>> getProducedSocketAddressTypes() {
return Collections.singleton(SocketAddress1.class);
}
});

assertThat(
registry.newChannelBuilder(nameResolverRegistry, "sc1:" + target, creds)).isSameInstanceAs(
mcb);
}

@Test
public void newChannelBuilder_unsupportedSocketAddressTypes() {
NameResolverRegistry nameResolverRegistry = new NameResolverRegistry();
Expand Down
81 changes: 72 additions & 9 deletions core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,9 @@ public static ManagedChannelBuilder<?> forTarget(String target) {
private final List<ClientInterceptor> interceptors = new ArrayList<>();
NameResolverRegistry nameResolverRegistry = NameResolverRegistry.getDefaultRegistry();

@Nullable
NameResolverProvider nameResolverProvider;

final List<ClientTransportFilter> transportFilters = new ArrayList<>();

final String target;
Expand Down Expand Up @@ -307,6 +310,49 @@ public ManagedChannelImplBuilder(
InternalConfiguratorRegistry.configureChannelBuilder(this);
}

/**
* Creates a new managed channel builder with a target string, which can be
* either a valid {@link io.grpc.NameResolver}-compliant URI, or an authority
* string. Transport
* implementors must provide client transport factory builder, and may set
* custom channel default
* port provider.
*
* @param channelCreds The ChannelCredentials provided by the user.
* These may be used when
* creating derivative channels.
* @param nameResolverRegistry the registry used to look up name resolvers.
* @param nameResolverProvider the provider used to look up name resolvers.
*/
public ManagedChannelImplBuilder(
String target, @Nullable ChannelCredentials channelCreds, @Nullable CallCredentials callCreds,
ClientTransportFactoryBuilder clientTransportFactoryBuilder,
@Nullable ChannelBuilderDefaultPortProvider channelBuilderDefaultPortProvider,
@Nullable NameResolverRegistry nameResolverRegistry,
@Nullable NameResolverProvider nameResolverProvider) {
this.target = checkNotNull(target, "target");
this.channelCredentials = channelCreds;
this.callCredentials = callCreds;
this.clientTransportFactoryBuilder = checkNotNull(clientTransportFactoryBuilder,
"clientTransportFactoryBuilder");
this.directServerAddress = null;

if (channelBuilderDefaultPortProvider != null) {
this.channelBuilderDefaultPortProvider = channelBuilderDefaultPortProvider;
} else {
this.channelBuilderDefaultPortProvider = new ManagedChannelDefaultPortProvider();
}
if (nameResolverRegistry != null) {
this.nameResolverRegistry = nameResolverRegistry;
}
if (nameResolverProvider != null) {
Copy link
Member

Choose a reason for hiding this comment

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

Why a null check?

this.nameResolverProvider = nameResolverProvider;
}

// TODO(dnvindhya): Move configurator to all the individual builders
InternalConfiguratorRegistry.configureChannelBuilder(this);
}

/**
* Returns a target string for the SocketAddress. It is only used as a placeholder, because
* DirectAddressNameResolverProvider will not actually try to use it. However, it must be a valid
Expand Down Expand Up @@ -422,6 +468,7 @@ public ManagedChannelImplBuilder nameResolverFactory(NameResolver.Factory resolv
Preconditions.checkState(directServerAddress == null,
"directServerAddress is set (%s), which forbids the use of NameResolverFactory",
directServerAddress);

if (resolverFactory != null) {
NameResolverRegistry reg = new NameResolverRegistry();
if (resolverFactory instanceof NameResolverProvider) {
Expand Down Expand Up @@ -724,7 +771,7 @@ public ManagedChannel build() {
ResolvedNameResolver resolvedResolver =
InternalFeatureFlags.getRfc3986UrisEnabled()
? getNameResolverProviderRfc3986(target, nameResolverRegistry)
: getNameResolverProvider(target, nameResolverRegistry);
: getNameResolverProvider(target, nameResolverRegistry, nameResolverProvider);
resolvedResolver.checkAddressTypes(clientTransportFactory.getSupportedSocketAddressTypes());
return new ManagedChannelOrphanWrapper(new ManagedChannelImpl(
this,
Expand Down Expand Up @@ -845,7 +892,8 @@ void checkAddressTypes(

@VisibleForTesting
static ResolvedNameResolver getNameResolverProvider(
String target, NameResolverRegistry nameResolverRegistry) {
String target, NameResolverRegistry nameResolverRegistry,
NameResolverProvider nameResolverProvider) {
// Finding a NameResolver. Try using the target string as the URI. If that fails, try prepending
// "dns:///".
NameResolverProvider provider = null;
Expand All @@ -860,19 +908,34 @@ static ResolvedNameResolver getNameResolverProvider(
if (targetUri != null) {
// For "localhost:8080" this would likely cause provider to be null, because "localhost" is
// parsed as the scheme. Will hit the next case and try "dns:///localhost:8080".
provider = nameResolverRegistry.getProviderForScheme(targetUri.getScheme());
provider = nameResolverProvider;
if (provider == null) {
provider = nameResolverRegistry.getProviderForScheme(targetUri.getScheme());
}
}

if (provider == null && !URI_PATTERN.matcher(target).matches()) {
Copy link
Member

Choose a reason for hiding this comment

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

Why go through this code block if you will ignore the result?

Copy link
Member

Choose a reason for hiding this comment

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

I still don't understand why this block was modified to check if provider is set. It looks like it can only break things.

// It doesn't look like a URI target. Maybe it's an authority string. Try with the default
// scheme from the registry.
if (!URI_PATTERN.matcher(target).matches()) {
// It doesn't look like a URI target. Maybe it's an authority string. Try with
// the default scheme from the registry (if provider is not specified) or
// the provider's default scheme (if provider is specified).
String scheme = (provider != null)
? provider.getDefaultScheme()
: (nameResolverProvider != null
? nameResolverProvider.getDefaultScheme()
: nameResolverRegistry.getDefaultScheme());
try {
targetUri = new URI(nameResolverRegistry.getDefaultScheme(), "", "/" + target, null);
targetUri = new URI(scheme, "", "/" + target, null);
} catch (URISyntaxException e) {
// Should not be possible.
// Should not happen because we just validated the URI.
throw new IllegalArgumentException(e);
}
provider = nameResolverRegistry.getProviderForScheme(targetUri.getScheme());
if (provider == null) {
if (nameResolverProvider != null) {
provider = nameResolverProvider;
} else {
provider = nameResolverRegistry.getProviderForScheme(targetUri.getScheme());
}
}
}

if (provider == null) {
Expand Down
Loading