Skip to content

Commit

Permalink
core: make newNameResolver() change backward compatible for callers.
Browse files Browse the repository at this point in the history
We assumed that we were the only caller.  Turns out there are
forwarding NameResolver too.  Implementing the old override will give
them time to migrate to the new API.

Resolves grpc#5556
  • Loading branch information
zhangkun83 committed Apr 8, 2019
1 parent 483697f commit e63dbd2
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 2 deletions.
25 changes: 23 additions & 2 deletions core/src/main/java/io/grpc/NameResolver.java
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@ public abstract static class Factory {
public static final Attributes.Key<ProxyDetector> PARAMS_PROXY_DETECTOR =
Attributes.Key.create("params-proxy-detector");

@Deprecated
private static final Attributes.Key<SynchronizationContext> PARAMS_SYNC_CONTEXT =
Attributes.Key.create("params-sync-context");

/**
* 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
Expand All @@ -158,8 +162,24 @@ public abstract static class Factory {
*/
@Nullable
@Deprecated
public NameResolver newNameResolver(URI targetUri, Attributes params) {
throw new UnsupportedOperationException("This method is going to be deleted");
public NameResolver newNameResolver(URI targetUri, final Attributes params) {
Helper helper = new Helper() {
@Override
public int getDefaultPort() {
return checkNotNull(params.get(PARAMS_DEFAULT_PORT), "default port not available");
}

@Override
public ProxyDetector getProxyDetector() {
return checkNotNull(params.get(PARAMS_PROXY_DETECTOR), "proxy detector not available");
}

@Override
public SynchronizationContext getSynchronizationContext() {
return checkNotNull(params.get(PARAMS_SYNC_CONTEXT), "sync context not available");
}
};
return newNameResolver(targetUri, helper);
}

/**
Expand All @@ -180,6 +200,7 @@ public NameResolver newNameResolver(URI targetUri, Helper helper) {
Attributes.newBuilder()
.set(PARAMS_DEFAULT_PORT, helper.getDefaultPort())
.set(PARAMS_PROXY_DETECTOR, helper.getProxyDetector())
.set(PARAMS_SYNC_CONTEXT, helper.getSynchronizationContext())
.build());
}

Expand Down
59 changes: 59 additions & 0 deletions core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -3413,6 +3413,65 @@ public String getDefaultScheme() {
assertThat(attrs.get(NameResolver.Factory.PARAMS_PROXY_DETECTOR)).isSameAs(neverProxy);
}

@Test
@Deprecated
public void nameResolverParams_forwardingResolverWithOldApi() {
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;
}
};
final NameResolver.Factory factory = new NameResolver.Factory() {
@Override
public NameResolver newNameResolver(URI targetUri, NameResolver.Helper helper) {
capturedHelper.set(helper);
return noopResolver;
}

@Override
public String getDefaultScheme() {
return "fakescheme";
}
};

// A forwarding factory still with the old API can forward to a delegate factory that has
// migrated to the new API.
NameResolver.Factory oldApiForwardingFactory = new NameResolver.Factory() {
@Override
public NameResolver newNameResolver(URI targetUri, Attributes params) {
return factory.newNameResolver(targetUri, params);
}

@Override
public String getDefaultScheme() {
return factory.getDefaultScheme();
}
};
channelBuilder.nameResolverFactory(oldApiForwardingFactory).proxyDetector(neverProxy);
createChannel();

NameResolver.Helper helper = capturedHelper.get();
assertThat(helper).isNotNull();
assertThat(helper.getDefaultPort()).isEqualTo(DEFAULT_PORT);
assertThat(helper.getProxyDetector()).isSameAs(neverProxy);
assertThat(helper.getSynchronizationContext()).isSameAs(channel.syncContext);
}

@Test
public void getAuthorityAfterShutdown() throws Exception {
createChannel();
Expand Down

0 comments on commit e63dbd2

Please sign in to comment.