flasn4nt
/
grpc-java
mirror of https://github.com/grpc/grpc-java.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

core: introduce NameResolver.Helper and deprecate the params on newNameResolver() (#5345) 

Context: [#4159 (comment)](https://github.com/grpc/grpc-java/issues/4159#issuecomment-415827641)

`Attributes` is appropriate for plumbing optional objects, especially useful for a long plumbing path where components in the middle may not care or see all objects in the container. It's not the case for the `params` on `newNewResolver()`. Both the default port and the proxy detector are guaranteed to be there and the plumbing path is very short. In this case, a first-class object is more appropriate and easier to use.

The `Helper` will also have `getSynchronizationContext()` (#2649) and a method to parse and validate service config. We we also considering merging `Listener` into the `Helper`, to make `NameResolver` match the `LoadBalancer` API.
This commit is contained in:
Kun Zhang 2019-02-12 10:14:59 -08:00 committed by GitHub
parent 1bead99241
commit e875a8c6a3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
19 changed files with 293 additions and 142 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
core/src/main/java/io/grpc/NameResolver.java
View File

@ -94,16 +94,24 @@ public abstract class NameResolver {
* The port number used in case the target or the underlying naming system doesn't provide a
* port number.
*
* @deprecated this will be deleted along with {@link #newNameResolver(URI, Attributes)} in
* a future release.
*
* @since 1.0.0
*/
@Deprecated
public static final Attributes.Key<Integer> PARAMS_DEFAULT_PORT =
Attributes.Key.create("params-default-port");
/**
* If the NameResolver wants to support proxy, it should inquire this {@link ProxyDetector}.
* See documentation on {@link ProxyDetector} about how proxies work in gRPC.
*
* @deprecated this will be deleted along with {@link #newNameResolver(URI, Attributes)} in
* a future release
*/
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/5113")
@Deprecated
public static final Attributes.Key<ProxyDetector> PARAMS_PROXY_DETECTOR =
Attributes.Key.create("params-proxy-detector");
@ -114,11 +122,39 @@ public abstract class NameResolver {
*
* @param targetUri the target URI to be resolved, whose scheme must not be {@code null}
* @param params optional parameters. Canonical keys are defined as {@code PARAMS_*} fields in
* {@link Factory}.
* {@link Factory}.
*
* @deprecated Implement {@link #newNameResolver(URI, NameResolver.Helper)} instead. This is
* going to be deleted in a future release.
*
* @since 1.0.0
*/
@Nullable
public abstract NameResolver newNameResolver(URI targetUri, Attributes params);
@Deprecated
public NameResolver newNameResolver(URI targetUri, Attributes params) {
throw new UnsupportedOperationException("This method is going to be deleted");
}
/**
* Creates a {@link NameResolver} for the given target URI, or {@code null} if the given URI
* cannot be resolved by this factory. The decision should be solely based on the scheme of the
* URI.
*
* @param targetUri the target URI to be resolved, whose scheme must not be {@code null}
* @param helper utility that may be used by the NameResolver implementation
*
* @since 1.19.0
*/
// TODO(zhangkun83): make this abstract when the other override is deleted
@Nullable
public NameResolver newNameResolver(URI targetUri, Helper helper) {
return newNameResolver(
targetUri,
Attributes.newBuilder()
.set(PARAMS_DEFAULT_PORT, helper.getDefaultPort())
.set(PARAMS_PROXY_DETECTOR, helper.getProxyDetector())
.build());
}
/**
* Returns the default scheme, which will be used to construct a URI when {@link
@ -170,4 +206,21 @@ public abstract class NameResolver {
@Retention(RetentionPolicy.SOURCE)
@Documented
public @interface ResolutionResultAttr {}
/**
* A utility object passed to {@link Factory#newNameResolver(URI, NameResolver.Helper)}.
*/
public abstract static class Helper {
/**
* The port number used in case the target or the underlying naming system doesn't provide a
* port number.
*/
public abstract int getDefaultPort();
/**
* If the NameResolver wants to support proxy, it should inquire this {@link ProxyDetector}.
* See documentation on {@link ProxyDetector} about how proxies work in gRPC.
*/
public abstract ProxyDetector getProxyDetector();
}
}

5
core/src/main/java/io/grpc/NameResolverProvider.java
View File

@ -44,6 +44,7 @@ public abstract class NameResolverProvider extends NameResolver.Factory {
* @since 1.0.0
*/
@SuppressWarnings("unused") // Avoids outside callers accidentally depending on the super class.
@Deprecated
public static final Attributes.Key<Integer> PARAMS_DEFAULT_PORT =
NameResolver.Factory.PARAMS_DEFAULT_PORT;
@ -106,10 +107,10 @@ public abstract class NameResolverProvider extends NameResolver.Factory {
@Override
@Nullable
public NameResolver newNameResolver(URI targetUri, Attributes params) {
public NameResolver newNameResolver(URI targetUri, NameResolver.Helper helper) {
checkForProviders();
for (NameResolverProvider provider : providers) {
NameResolver resolver = provider.newNameResolver(targetUri, params);
NameResolver resolver = provider.newNameResolver(targetUri, helper);
if (resolver != null) {
return resolver;
}

9
core/src/main/java/io/grpc/ProxyDetector.java
View File

@ -31,11 +31,10 @@ import javax.annotation.Nullable;
* <p>In order for gRPC to use a proxy, {@link NameResolver}, {@link ProxyDetector} and the
* underlying transport need to work together.
*
* <p>The {@link NameResolver} should invoke the {@link ProxyDetector} retrieved from the {@code
* params} of {@link NameResolver.Factory#newNameResolver} using {@link
* io.grpc.NameResolver.Factory#PARAMS_PROXY_DETECTOR PARAMS_PROXY_DETECTOR}, and pass the returned
* {@link ProxiedSocketAddress} to {@link NameResolver.Listener#onAddresses}. The DNS name resolver
* shipped with gRPC is already doing so.
* <p>The {@link NameResolver} should invoke the {@link ProxyDetector} retrieved from the {@link
* NameResolver.Helper#getProxyDetector}, and pass the returned {@link ProxiedSocketAddress} to
* {@link NameResolver.Listener#onAddresses}. The DNS name resolver shipped with gRPC is already
* doing so.
*
* <p>The default {@code ProxyDetector} uses Java's standard {@link java.net.ProxySelector} and
* {@link java.net.Authenticator} to detect proxies and authentication credentials and produce

12
core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java
View File

@ -488,12 +488,12 @@ public abstract class AbstractManagedChannelImplBuilder
protected abstract ClientTransportFactory buildTransportFactory();
/**
* Subclasses can override this method to provide additional parameters to {@link
* NameResolver.Factory#newNameResolver}. The default implementation returns {@link
* Attributes#EMPTY}.
* Subclasses can override this method to provide a default port to {@link NameResolver} for use
* in cases where the target string doesn't include a port. The default implementation returns
* {@link GrpcUtil.DEFAULT_PORT_SSL}.
*/
protected Attributes getNameResolverParams() {
return Attributes.EMPTY;
protected int getDefaultPort() {
return GrpcUtil.DEFAULT_PORT_SSL;
}
/**
@ -517,7 +517,7 @@ public abstract class AbstractManagedChannelImplBuilder
}
@Override
public NameResolver newNameResolver(URI notUsedUri, Attributes params) {
public NameResolver newNameResolver(URI notUsedUri, NameResolver.Helper helper) {
return new NameResolver() {
@Override
public String getServiceAuthority() {

15
core/src/main/java/io/grpc/internal/DnsNameResolver.java
View File

@ -149,9 +149,8 @@ final class DnsNameResolver extends NameResolver {
private final Runnable resolveRunnable;
DnsNameResolver(@Nullable String nsAuthority, String name, Attributes params,
Resource<Executor> executorResource, ProxyDetector proxyDetector,
Stopwatch stopwatch, boolean isAndroid) {
DnsNameResolver(@Nullable String nsAuthority, String name, Helper helper,
Resource<Executor> executorResource, Stopwatch stopwatch, boolean isAndroid) {
// TODO: if a DNS server is provided as nsAuthority, use it.
// https://www.captechconsulting.com/blogs/accessing-the-dusty-corners-of-dns-with-java
this.executorResource = executorResource;
@ -163,17 +162,11 @@ final class DnsNameResolver extends NameResolver {
"nameUri (%s) doesn't have an authority", nameUri);
host = nameUri.getHost();
if (nameUri.getPort() == -1) {
Integer defaultPort = params.get(NameResolver.Factory.PARAMS_DEFAULT_PORT);
if (defaultPort != null) {
port = defaultPort;
} else {
throw new IllegalArgumentException(
"name '" + name + "' doesn't contain a port, and default port is not set in params");
}
port = helper.getDefaultPort();
} else {
port = nameUri.getPort();
}
this.proxyDetector = proxyDetector;
this.proxyDetector = Preconditions.checkNotNull(helper.getProxyDetector(), "proxyDetector");
this.resolveRunnable = new Resolve(this, stopwatch, getNetworkAddressCacheTtlNanos(isAndroid));
}

11
core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java
View File

@ -18,11 +18,9 @@ package io.grpc.internal;
import com.google.common.base.Preconditions;
import com.google.common.base.Stopwatch;
import io.grpc.Attributes;
import io.grpc.InternalServiceProviders;
import io.grpc.NameResolver.Factory;
import io.grpc.NameResolver;
import io.grpc.NameResolverProvider;
import io.grpc.ProxyDetector;
import java.net.URI;
/**
@ -45,20 +43,17 @@ public final class DnsNameResolverProvider extends NameResolverProvider {
private static final String SCHEME = "dns";
@Override
public DnsNameResolver newNameResolver(URI targetUri, Attributes params) {
public DnsNameResolver newNameResolver(URI targetUri, NameResolver.Helper helper) {
if (SCHEME.equals(targetUri.getScheme())) {
String targetPath = Preconditions.checkNotNull(targetUri.getPath(), "targetPath");
Preconditions.checkArgument(targetPath.startsWith("/"),
"the path component (%s) of the target (%s) must start with '/'", targetPath, targetUri);
String name = targetPath.substring(1);
ProxyDetector proxyDetector = Preconditions
.checkNotNull(params.get(Factory.PARAMS_PROXY_DETECTOR), "proxyDetector");
return new DnsNameResolver(
targetUri.getAuthority(),
name,
params,
helper,
GrpcUtil.SHARED_CHANNEL_EXECUTOR,
proxyDetector,
Stopwatch.createUnstarted(),
InternalServiceProviders.isAndroid(getClass().getClassLoader()));
} else {

39
core/src/main/java/io/grpc/internal/ManagedChannelImpl.java
View File

@ -126,7 +126,7 @@ final class ManagedChannelImpl extends ManagedChannel implements
private final InternalLogId logId;
private final String target;
private final NameResolver.Factory nameResolverFactory;
private final Attributes nameResolverParams;
private final NameResolver.Helper nameResolverHelper;
private final LoadBalancer.Factory loadBalancerFactory;
private final ClientTransportFactory transportFactory;
private final ScheduledExecutorForBalancer scheduledExecutorForBalancer;
@ -136,7 +136,6 @@ final class ManagedChannelImpl extends ManagedChannel implements
private final ExecutorHolder balancerRpcExecutorHolder;
private final TimeProvider timeProvider;
private final int maxTraceEvents;
private final ProxyDetector proxyDetector;
@VisibleForTesting
final SynchronizationContext syncContext = new SynchronizationContext(
@ -320,7 +319,7 @@ final class ManagedChannelImpl extends ManagedChannel implements
nameResolver.shutdown();
nameResolverStarted = false;
if (channelIsActive) {
nameResolver = getNameResolver(target, nameResolverFactory, nameResolverParams);
nameResolver = getNameResolver(target, nameResolverFactory, nameResolverHelper);
} else {
nameResolver = null;
}
@ -545,11 +544,21 @@ final class ManagedChannelImpl extends ManagedChannel implements
this.target = checkNotNull(builder.target, "target");
this.logId = InternalLogId.allocate("Channel", target);
this.nameResolverFactory = builder.getNameResolverFactory();
this.proxyDetector =
final ProxyDetector proxyDetector =
builder.proxyDetector != null ? builder.proxyDetector : GrpcUtil.getDefaultProxyDetector();
this.nameResolverParams = addProxyToAttributes(this.proxyDetector,
checkNotNull(builder.getNameResolverParams(), "nameResolverParams"));
this.nameResolver = getNameResolver(target, nameResolverFactory, nameResolverParams);
final int defaultPort = builder.getDefaultPort();
this.nameResolverHelper = new NameResolver.Helper() {
@Override
public int getDefaultPort() {
return defaultPort;
}
@Override
public ProxyDetector getProxyDetector() {
return proxyDetector;
}
};
this.nameResolver = getNameResolver(target, nameResolverFactory, nameResolverHelper);
this.timeProvider = checkNotNull(timeProvider, "timeProvider");
maxTraceEvents = builder.maxTraceEvents;
channelTracer = new ChannelTracer(
@ -617,19 +626,9 @@ final class ManagedChannelImpl extends ManagedChannel implements
channelz.addRootChannel(this);
}
private static Attributes addProxyToAttributes(ProxyDetector proxyDetector,
Attributes attributes) {
if (attributes.get(NameResolver.Factory.PARAMS_PROXY_DETECTOR) == null) {
return attributes.toBuilder()
.set(NameResolver.Factory.PARAMS_PROXY_DETECTOR, proxyDetector).build();
} else {
return attributes;
}
}
@VisibleForTesting
static NameResolver getNameResolver(String target, NameResolver.Factory nameResolverFactory,
Attributes nameResolverParams) {
NameResolver.Helper nameResolverHelper) {
// Finding a NameResolver. Try using the target string as the URI. If that fails, try prepending
// "dns:///".
URI targetUri = null;
@ -644,7 +643,7 @@ final class ManagedChannelImpl extends ManagedChannel implements
uriSyntaxErrors.append(e.getMessage());
}
if (targetUri != null) {
NameResolver resolver = nameResolverFactory.newNameResolver(targetUri, nameResolverParams);
NameResolver resolver = nameResolverFactory.newNameResolver(targetUri, nameResolverHelper);
if (resolver != null) {
return resolver;
}
@ -662,7 +661,7 @@ final class ManagedChannelImpl extends ManagedChannel implements
// Should not be possible.
throw new IllegalArgumentException(e);
}
NameResolver resolver = nameResolverFactory.newNameResolver(targetUri, nameResolverParams);
NameResolver resolver = nameResolverFactory.newNameResolver(targetUri, nameResolverHelper);
if (resolver != null) {
return resolver;
}

5
core/src/main/java/io/grpc/internal/OverrideAuthorityNameResolverFactory.java
View File

@ -16,7 +16,6 @@
package io.grpc.internal;
import io.grpc.Attributes;
import io.grpc.NameResolver;
import java.net.URI;
import javax.annotation.Nullable;
@ -43,8 +42,8 @@ final class OverrideAuthorityNameResolverFactory extends NameResolver.Factory {
@Nullable
@Override
public NameResolver newNameResolver(URI targetUri, Attributes params) {
final NameResolver resolver = delegate.newNameResolver(targetUri, params);
public NameResolver newNameResolver(URI targetUri, NameResolver.Helper helper) {
final NameResolver resolver = delegate.newNameResolver(targetUri, helper);
// Do not wrap null values. We do not want to impede error signaling.
if (resolver == null) {
return null;

21
core/src/test/java/io/grpc/NameResolverProviderTest.java
View File

@ -22,6 +22,8 @@ import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verifyZeroInteractions;
import io.grpc.internal.DnsNameResolverProvider;
import java.net.URI;
@ -29,6 +31,7 @@ import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.concurrent.Callable;
import org.junit.After;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@ -37,7 +40,13 @@ import org.junit.runners.JUnit4;
@RunWith(JUnit4.class)
public class NameResolverProviderTest {
private final URI uri = URI.create("dns:///localhost");
private final Attributes attributes = Attributes.EMPTY;
private final NameResolver.Helper helper = mock(NameResolver.Helper.class);
@After
public void wrapUp() {
// The helper is not implemented. Make sure it's not used in the test.
verifyZeroInteractions(helper);
}
@Test
public void getDefaultScheme_noProvider() {
@ -56,13 +65,13 @@ public class NameResolverProviderTest {
List<NameResolverProvider> providers = Collections.<NameResolverProvider>singletonList(
new BaseProvider(true, 5) {
@Override
public NameResolver newNameResolver(URI passedUri, Attributes passedAttributes) {
public NameResolver newNameResolver(URI passedUri, NameResolver.Helper passedHelper) {
assertSame(uri, passedUri);
assertSame(attributes, passedAttributes);
assertSame(helper, passedHelper);
return null;
}
});
assertNull(NameResolverProvider.asFactory(providers).newNameResolver(uri, attributes));
assertNull(NameResolverProvider.asFactory(providers).newNameResolver(uri, helper));
}
@Test
@ -70,7 +79,7 @@ public class NameResolverProviderTest {
List<NameResolverProvider> providers = Collections.emptyList();
NameResolver.Factory factory = NameResolverProvider.asFactory(providers);
try {
factory.newNameResolver(uri, attributes);
factory.newNameResolver(uri, helper);
fail("Expected exception");
} catch (RuntimeException ex) {
assertTrue(ex.toString(), ex.getMessage().contains("No NameResolverProviders found"));
@ -142,7 +151,7 @@ public class NameResolverProviderTest {
}
@Override
public NameResolver newNameResolver(URI targetUri, Attributes params) {
public NameResolver newNameResolver(URI targetUri, NameResolver.Helper helper) {
throw new UnsupportedOperationException();
}

21
core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java
View File

@ -20,8 +20,8 @@ import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
import io.grpc.Attributes;
import io.grpc.NameResolver;
import io.grpc.ProxyDetector;
import java.net.URI;
import org.junit.Test;
import org.junit.runner.RunWith;
@ -31,10 +31,17 @@ import org.junit.runners.JUnit4;
@RunWith(JUnit4.class)
public class DnsNameResolverProviderTest {
private static final Attributes ATTRIBUTES =
Attributes.newBuilder()
.set(NameResolver.Factory.PARAMS_PROXY_DETECTOR, GrpcUtil.getDefaultProxyDetector())
.build();
private static final NameResolver.Helper HELPER = new NameResolver.Helper() {
@Override
public int getDefaultPort() {
throw new UnsupportedOperationException("Should not be called");
}
@Override
public ProxyDetector getProxyDetector() {
return GrpcUtil.getDefaultProxyDetector();
}
};
private DnsNameResolverProvider provider = new DnsNameResolverProvider();
@ -46,8 +53,8 @@ public class DnsNameResolverProviderTest {
@Test
public void newNameResolver() {
assertSame(DnsNameResolver.class,
provider.newNameResolver(URI.create("dns:///localhost:443"), ATTRIBUTES).getClass());
provider.newNameResolver(URI.create("dns:///localhost:443"), HELPER).getClass());
assertNull(
provider.newNameResolver(URI.create("notdns:///localhost:443"), ATTRIBUTES));
provider.newNameResolver(URI.create("notdns:///localhost:443"), HELPER));
}
}

41
core/src/test/java/io/grpc/internal/DnsNameResolverTest.java
View File

@ -95,11 +95,17 @@ public class DnsNameResolverTest {
private final Map<String, Object> serviceConfig = new LinkedHashMap<>();
private static final int DEFAULT_PORT = 887;
private static final Attributes NAME_RESOLVER_PARAMS =
Attributes.newBuilder()
.set(NameResolver.Factory.PARAMS_DEFAULT_PORT, DEFAULT_PORT)
.set(NameResolver.Factory.PARAMS_PROXY_DETECTOR, GrpcUtil.getDefaultProxyDetector())
.build();
private static final NameResolver.Helper HELPER = new NameResolver.Helper() {
@Override
public int getDefaultPort() {
return DEFAULT_PORT;
}
@Override
public ProxyDetector getProxyDetector() {
return GrpcUtil.getDefaultProxyDetector();
}
};
private final DnsNameResolverProvider provider = new DnsNameResolverProvider();
private final FakeClock fakeClock = new FakeClock();
@ -146,16 +152,25 @@ public class DnsNameResolverTest {
private DnsNameResolver newResolver(
String name,
int port,
ProxyDetector proxyDetector,
final int port,
final ProxyDetector proxyDetector,
Stopwatch stopwatch,
boolean isAndroid) {
DnsNameResolver dnsResolver = new DnsNameResolver(
null,
name,
Attributes.newBuilder().set(NameResolver.Factory.PARAMS_DEFAULT_PORT, port).build(),
new NameResolver.Helper() {
@Override
public int getDefaultPort() {
return port;
}
@Override
public ProxyDetector getProxyDetector() {
return proxyDetector;
}
},
fakeExecutorResource,
proxyDetector,
stopwatch,
isAndroid);
// By default, using the mocked ResourceResolver to avoid I/O
@ -279,9 +294,7 @@ public class DnsNameResolverTest {
public void resolveAll_failsOnEmptyResult() throws Exception {
String hostname = "dns:///addr.fake:1234";
DnsNameResolver nrf =
new DnsNameResolverProvider().newNameResolver(new URI(hostname), Attributes.newBuilder()
.set(NameResolver.Factory.PARAMS_PROXY_DETECTOR, GrpcUtil.getDefaultProxyDetector())
.build());
new DnsNameResolverProvider().newNameResolver(new URI(hostname), HELPER);
nrf.setAddressResolver(new AddressResolver() {
@Override
public List<InetAddress> resolveAddress(String host) throws Exception {
@ -993,7 +1006,7 @@ public class DnsNameResolverTest {
private void testInvalidUri(URI uri) {
try {
provider.newNameResolver(uri, NAME_RESOLVER_PARAMS);
provider.newNameResolver(uri, HELPER);
fail("Should have failed");
} catch (IllegalArgumentException e) {
// expected
@ -1001,7 +1014,7 @@ public class DnsNameResolverTest {
}
private void testValidUri(URI uri, String exportedAuthority, int expectedPort) {
DnsNameResolver resolver = provider.newNameResolver(uri, NAME_RESOLVER_PARAMS);
DnsNameResolver resolver = provider.newNameResolver(uri, HELPER);
assertNotNull(resolver);
assertEquals(expectedPort, resolver.getPort());
assertEquals(exportedAuthority, resolver.getServiceAuthority());

25
core/src/test/java/io/grpc/internal/ManagedChannelImplGetNameResolverTest.java
View File

@ -20,9 +20,9 @@ import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.fail;
import io.grpc.Attributes;
import io.grpc.NameResolver;
import io.grpc.NameResolver.Factory;
import io.grpc.ProxyDetector;
import java.net.URI;
import org.junit.Test;
import org.junit.runner.RunWith;
@ -31,8 +31,17 @@ import org.junit.runners.JUnit4;
/** Unit tests for {@link ManagedChannelImpl#getNameResolver}. */
@RunWith(JUnit4.class)
public class ManagedChannelImplGetNameResolverTest {
private static final Attributes NAME_RESOLVER_PARAMS =
Attributes.newBuilder().set(NameResolver.Factory.PARAMS_DEFAULT_PORT, 447).build();
private static final NameResolver.Helper NAMERESOLVER_HELPER = new NameResolver.Helper() {
@Override
public int getDefaultPort() {
return 447;
}
@Override
public ProxyDetector getProxyDetector() {
throw new UnsupportedOperationException("Should not be called");
}
};
@Test
public void invalidUriTarget() {
@ -96,7 +105,7 @@ public class ManagedChannelImplGetNameResolverTest {
public void validTargetNoResovler() {
Factory nameResolverFactory = new NameResolver.Factory() {
@Override
public NameResolver newNameResolver(URI targetUri, Attributes params) {
public NameResolver newNameResolver(URI targetUri, NameResolver.Helper helper) {
return null;
}
@ -107,7 +116,7 @@ public class ManagedChannelImplGetNameResolverTest {
};
try {
ManagedChannelImpl.getNameResolver(
"foo.googleapis.com:8080", nameResolverFactory, NAME_RESOLVER_PARAMS);
"foo.googleapis.com:8080", nameResolverFactory, NAMERESOLVER_HELPER);
fail("Should fail");
} catch (IllegalArgumentException e) {
// expected
@ -117,7 +126,7 @@ public class ManagedChannelImplGetNameResolverTest {
private void testValidTarget(String target, String expectedUriString, URI expectedUri) {
Factory nameResolverFactory = new FakeNameResolverFactory(expectedUri.getScheme());
FakeNameResolver nameResolver = (FakeNameResolver) ManagedChannelImpl.getNameResolver(
target, nameResolverFactory, NAME_RESOLVER_PARAMS);
target, nameResolverFactory, NAMERESOLVER_HELPER);
assertNotNull(nameResolver);
assertEquals(expectedUri, nameResolver.uri);
assertEquals(expectedUriString, nameResolver.uri.toString());
@ -128,7 +137,7 @@ public class ManagedChannelImplGetNameResolverTest {
try {
FakeNameResolver nameResolver = (FakeNameResolver) ManagedChannelImpl.getNameResolver(
target, nameResolverFactory, NAME_RESOLVER_PARAMS);
target, nameResolverFactory, NAMERESOLVER_HELPER);
fail("Should have failed, but got resolver with " + nameResolver.uri);
} catch (IllegalArgumentException e) {
// expected
@ -143,7 +152,7 @@ public class ManagedChannelImplGetNameResolverTest {
}
@Override
public NameResolver newNameResolver(URI targetUri, Attributes params) {
public NameResolver newNameResolver(URI targetUri, NameResolver.Helper helper) {
if (expectedScheme.equals(targetUri.getScheme())) {
return new FakeNameResolver(targetUri);
}

4
core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java
View File

@ -142,7 +142,7 @@ public class ManagedChannelImplIdlenessTest {
LoadBalancerRegistry.getDefaultRegistry().register(mockLoadBalancerProvider);
when(mockNameResolver.getServiceAuthority()).thenReturn(AUTHORITY);
when(mockNameResolverFactory
.newNameResolver(any(URI.class), any(Attributes.class)))
.newNameResolver(any(URI.class), any(NameResolver.Helper.class)))
.thenReturn(mockNameResolver);
when(mockTransportFactory.getScheduledExecutorService())
.thenReturn(timer.getScheduledExecutorService());
@ -181,7 +181,7 @@ public class ManagedChannelImplIdlenessTest {
}
servers.add(new EquivalentAddressGroup(addrs));
}
verify(mockNameResolverFactory).newNameResolver(any(URI.class), any(Attributes.class));
verify(mockNameResolverFactory).newNameResolver(any(URI.class), any(NameResolver.Helper.class));
// Verify the initial idleness
verify(mockLoadBalancerProvider, never()).newLoadBalancer(any(Helper.class));
verify(mockTransportFactory, never()).newClientTransport(

105
core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java
View File

@ -89,6 +89,8 @@ import io.grpc.Metadata;
import io.grpc.MethodDescriptor;
import io.grpc.MethodDescriptor.MethodType;
import io.grpc.NameResolver;
import io.grpc.ProxiedSocketAddress;
import io.grpc.ProxyDetector;
import io.grpc.SecurityLevel;
import io.grpc.ServerMethodDefinition;
import io.grpc.Status;
@ -142,11 +144,7 @@ import org.mockito.stubbing.Answer;
/** Unit tests for {@link ManagedChannelImpl}. */
@RunWith(JUnit4.class)
public class ManagedChannelImplTest {
private static final Attributes NAME_RESOLVER_PARAMS =
Attributes.newBuilder()
.set(NameResolver.Factory.PARAMS_DEFAULT_PORT, 447)
.set(NameResolver.Factory.PARAMS_PROXY_DETECTOR, GrpcUtil.getDefaultProxyDetector())
.build();
private static final int DEFAULT_PORT = 447;
private static final MethodDescriptor<String, Integer> method =
MethodDescriptor.<String, Integer>newBuilder()
@ -3175,7 +3173,7 @@ public class ManagedChannelImplTest {
@Nullable
@Override
public NameResolver newNameResolver(URI targetUri, Attributes params) {
public NameResolver newNameResolver(URI targetUri, NameResolver.Helper helper) {
return (resolver = new FakeNameResolver());
}
@ -3240,6 +3238,93 @@ public class ManagedChannelImplTest {
mychannel.shutdownNow();
}
@Test
public void nameResolverHelperPropagation() {
final AtomicReference<NameResolver.Helper> capturedHelper = new AtomicReference<>();
final NameResolver noopResolver = new NameResolver() {
@Override
public String getServiceAuthority() {
return "fake-authority";
}
@Override
public void start(Listener listener) {
}
@Override
public void shutdown() {}
};
ProxyDetector neverProxy = new ProxyDetector() {
@Override
public ProxiedSocketAddress proxyFor(SocketAddress targetAddress) {
return null;
}
};
NameResolver.Factory oldApiFactory = new NameResolver.Factory() {
@Override
public NameResolver newNameResolver(URI targetUri, NameResolver.Helper helper) {
capturedHelper.set(helper);
return noopResolver;
}
@Override
public String getDefaultScheme() {
return "fakescheme";
}
};
channelBuilder.nameResolverFactory(oldApiFactory).proxyDetector(neverProxy);
createChannel();
NameResolver.Helper helper = capturedHelper.get();
assertThat(helper).isNotNull();
assertThat(helper.getDefaultPort()).isEqualTo(DEFAULT_PORT);
assertThat(helper.getProxyDetector()).isSameAs(neverProxy);
}
@Test
@Deprecated
public void nameResolverParams_oldApi() {
final AtomicReference<Attributes> capturedParams = new AtomicReference<>();
final NameResolver noopResolver = new NameResolver() {
@Override
public String getServiceAuthority() {
return "fake-authority";
}
@Override
public void start(Listener listener) {
}
@Override
public void shutdown() {}
};
ProxyDetector neverProxy = new ProxyDetector() {
@Override
public ProxiedSocketAddress proxyFor(SocketAddress targetAddress) {
return null;
}
};
NameResolver.Factory oldApiFactory = new NameResolver.Factory() {
@Override
public NameResolver newNameResolver(URI targetUri, Attributes params) {
capturedParams.set(params);
return noopResolver;
}
@Override
public String getDefaultScheme() {
return "fakescheme";
}
};
channelBuilder.nameResolverFactory(oldApiFactory).proxyDetector(neverProxy);
createChannel();
Attributes attrs = capturedParams.get();
assertThat(attrs).isNotNull();
assertThat(attrs.get(NameResolver.Factory.PARAMS_DEFAULT_PORT)).isEqualTo(DEFAULT_PORT);
assertThat(attrs.get(NameResolver.Factory.PARAMS_PROXY_DETECTOR)).isSameAs(neverProxy);
}
@Test
public void getAuthorityAfterShutdown() throws Exception {
createChannel();
@ -3259,8 +3344,8 @@ public class ManagedChannelImplTest {
throw new UnsupportedOperationException();
}
@Override protected Attributes getNameResolverParams() {
return NAME_RESOLVER_PARAMS;
@Override protected int getDefaultPort() {
return DEFAULT_PORT;
}
}
@ -3300,11 +3385,11 @@ public class ManagedChannelImplTest {
}
@Override
public NameResolver newNameResolver(final URI targetUri, Attributes params) {
public NameResolver newNameResolver(final URI targetUri, NameResolver.Helper helper) {
if (!expectedUri.equals(targetUri)) {
return null;
}
assertSame(NAME_RESOLVER_PARAMS, params);
assertEquals(DEFAULT_PORT, helper.getDefaultPort());
FakeNameResolver resolver = new FakeNameResolver(error);
resolvers.add(resolver);
return resolver;

28
core/src/test/java/io/grpc/internal/OverrideAuthorityNameResolverTest.java
View File

@ -23,8 +23,8 @@ import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import io.grpc.Attributes;
import io.grpc.NameResolver;
import io.grpc.ProxyDetector;
import java.net.URI;
import org.junit.Test;
import org.junit.runner.RunWith;
@ -33,17 +33,28 @@ import org.junit.runners.JUnit4;
/** Unit tests for {@link OverrideAuthorityNameResolverFactory}. */
@RunWith(JUnit4.class)
public class OverrideAuthorityNameResolverTest {
private static final NameResolver.Helper HELPER = new NameResolver.Helper() {
@Override
public int getDefaultPort() {
throw new UnsupportedOperationException("Should not be called");
}
@Override
public ProxyDetector getProxyDetector() {
throw new UnsupportedOperationException("Should not be called");
}
};
@Test
public void overridesAuthority() {
NameResolver nameResolverMock = mock(NameResolver.class);
NameResolver.Factory wrappedFactory = mock(NameResolver.Factory.class);
when(wrappedFactory.newNameResolver(any(URI.class), any(Attributes.class)))
when(wrappedFactory.newNameResolver(any(URI.class), any(NameResolver.Helper.class)))
.thenReturn(nameResolverMock);
String override = "override:5678";
NameResolver.Factory factory =
new OverrideAuthorityNameResolverFactory(wrappedFactory, override);
NameResolver nameResolver = factory.newNameResolver(URI.create("dns:///localhost:443"),
Attributes.EMPTY);
NameResolver nameResolver = factory.newNameResolver(URI.create("dns:///localhost:443"), HELPER);
assertNotNull(nameResolver);
assertEquals(override, nameResolver.getServiceAuthority());
}
@ -51,23 +62,24 @@ public class OverrideAuthorityNameResolverTest {
@Test
public void wontWrapNull() {
NameResolver.Factory wrappedFactory = mock(NameResolver.Factory.class);
when(wrappedFactory.newNameResolver(any(URI.class), any(Attributes.class))).thenReturn(null);
when(wrappedFactory.newNameResolver(any(URI.class), any(NameResolver.Helper.class)))
.thenReturn(null);
NameResolver.Factory factory =
new OverrideAuthorityNameResolverFactory(wrappedFactory, "override:5678");
assertEquals(null,
factory.newNameResolver(URI.create("dns:///localhost:443"), Attributes.EMPTY));
factory.newNameResolver(URI.create("dns:///localhost:443"), HELPER));
}
@Test
public void forwardsNonOverridenCalls() {
NameResolver.Factory wrappedFactory = mock(NameResolver.Factory.class);
NameResolver mockResolver = mock(NameResolver.class);
when(wrappedFactory.newNameResolver(any(URI.class), any(Attributes.class)))
when(wrappedFactory.newNameResolver(any(URI.class), any(NameResolver.Helper.class)))
.thenReturn(mockResolver);
NameResolver.Factory factory =
new OverrideAuthorityNameResolverFactory(wrappedFactory, "override:5678");
NameResolver overrideResolver =
factory.newNameResolver(URI.create("dns:///localhost:443"), Attributes.EMPTY);
factory.newNameResolver(URI.create("dns:///localhost:443"), HELPER);
assertNotNull(overrideResolver);
NameResolver.Listener listener = mock(NameResolver.Listener.class);

8
cronet/src/main/java/io/grpc/cronet/CronetChannelBuilder.java
View File

@ -23,9 +23,7 @@ import static io.grpc.internal.GrpcUtil.DEFAULT_MAX_MESSAGE_SIZE;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.util.concurrent.MoreExecutors;
import io.grpc.Attributes;
import io.grpc.ExperimentalApi;
import io.grpc.NameResolver;
import io.grpc.internal.AbstractManagedChannelImplBuilder;
import io.grpc.internal.ClientTransportFactory;
import io.grpc.internal.ConnectionClientTransport;
@ -193,12 +191,6 @@ public final class CronetChannelBuilder extends
transportTracerFactory.create());
}
@Override
protected Attributes getNameResolverParams() {
return Attributes.newBuilder()
.set(NameResolver.Factory.PARAMS_DEFAULT_PORT, GrpcUtil.DEFAULT_PORT_SSL).build();
}
@VisibleForTesting
static class CronetTransportFactory implements ClientTransportFactory {
private final ScheduledExecutorService timeoutService;

12
netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java
View File

@ -30,7 +30,6 @@ import io.grpc.EquivalentAddressGroup;
import io.grpc.ExperimentalApi;
import io.grpc.HttpConnectProxiedSocketAddress;
import io.grpc.Internal;
import io.grpc.NameResolver;
import io.grpc.internal.AbstractManagedChannelImplBuilder;
import io.grpc.internal.AtomicBackoff;
import io.grpc.internal.ClientTransportFactory;
@ -415,21 +414,16 @@ public final class NettyChannelBuilder
@Override
@CheckReturnValue
protected Attributes getNameResolverParams() {
int defaultPort;
protected int getDefaultPort() {
switch (negotiationType) {
case PLAINTEXT:
case PLAINTEXT_UPGRADE:
defaultPort = GrpcUtil.DEFAULT_PORT_PLAINTEXT;
break;
return GrpcUtil.DEFAULT_PORT_PLAINTEXT;
case TLS:
defaultPort = GrpcUtil.DEFAULT_PORT_SSL;
break;
return GrpcUtil.DEFAULT_PORT_SSL;
default:
throw new AssertionError(negotiationType + " not handled");
}
return Attributes.newBuilder()
.set(NameResolver.Factory.PARAMS_DEFAULT_PORT, defaultPort).build();
}
void overrideAuthorityChecker(@Nullable OverrideAuthorityChecker authorityChecker) {

13
okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java
View File

@ -23,10 +23,8 @@ import static io.grpc.internal.GrpcUtil.KEEPALIVE_TIME_NANOS_DISABLED;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import io.grpc.Attributes;
import io.grpc.ExperimentalApi;
import io.grpc.Internal;
import io.grpc.NameResolver;
import io.grpc.internal.AbstractManagedChannelImplBuilder;
import io.grpc.internal.AtomicBackoff;
import io.grpc.internal.ClientTransportFactory;
@ -406,20 +404,15 @@ public class OkHttpChannelBuilder extends
}
@Override
protected Attributes getNameResolverParams() {
int defaultPort;
protected int getDefaultPort() {
switch (negotiationType) {
case PLAINTEXT:
defaultPort = GrpcUtil.DEFAULT_PORT_PLAINTEXT;
break;
return GrpcUtil.DEFAULT_PORT_PLAINTEXT;
case TLS:
defaultPort = GrpcUtil.DEFAULT_PORT_SSL;
break;
return GrpcUtil.DEFAULT_PORT_SSL;
default:
throw new AssertionError(negotiationType + " not handled");
}
return Attributes.newBuilder()
.set(NameResolver.Factory.PARAMS_DEFAULT_PORT, defaultPort).build();
}
@VisibleForTesting

4
okhttp/src/test/java/io/grpc/okhttp/OkHttpChannelBuilderTest.java
View File

@ -23,7 +23,6 @@ import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import com.squareup.okhttp.ConnectionSpec;
import io.grpc.NameResolver;
import io.grpc.internal.ClientTransportFactory;
import io.grpc.internal.FakeClock;
import io.grpc.internal.GrpcUtil;
@ -120,8 +119,7 @@ public class OkHttpChannelBuilderTest {
@Test
public void usePlaintextDefaultPort() {
OkHttpChannelBuilder builder = OkHttpChannelBuilder.forAddress("host", 1234).usePlaintext();
assertEquals(GrpcUtil.DEFAULT_PORT_PLAINTEXT,
builder.getNameResolverParams().get(NameResolver.Factory.PARAMS_DEFAULT_PORT).intValue());
assertEquals(GrpcUtil.DEFAULT_PORT_PLAINTEXT, builder.getDefaultPort());
}
@Test