Skip to content

Commit

Permalink
core: make newNameResolver() change backward compatible for callers. (#…
Browse files Browse the repository at this point in the history
…5560) (#5564)

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 #5556
  • Loading branch information
zhangkun83 committed Apr 9, 2019
1 parent 230e6f1 commit 7f3fa9b
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
Expand Up @@ -124,6 +124,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 @@ -140,8 +144,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 @@ -162,6 +182,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
Expand Up @@ -3371,6 +3371,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 7f3fa9b

Please sign in to comment.