From 693779dba776bb4f685a3d878840d4a45b5cd835 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Mon, 24 Sep 2018 17:32:04 -0700 Subject: [PATCH] alts: Remove usage of TransportCreationParamsFilterFactory Provide a ProtocolNegotiatorFactory instead. TransportCreationParamsFilterFactory is being removed. --- .../java/io/grpc/alts/AltsChannelBuilder.java | 99 +++++----------- .../alts/GoogleDefaultChannelBuilder.java | 107 ++++++------------ .../GoogleDefaultProtocolNegotiator.java | 3 +- .../io/grpc/alts/AltsChannelBuilderTest.java | 14 +-- .../alts/GoogleDefaultChannelBuilderTest.java | 10 +- 5 files changed, 67 insertions(+), 166 deletions(-) diff --git a/alts/src/main/java/io/grpc/alts/AltsChannelBuilder.java b/alts/src/main/java/io/grpc/alts/AltsChannelBuilder.java index ce24861b89..5442a192c4 100644 --- a/alts/src/main/java/io/grpc/alts/AltsChannelBuilder.java +++ b/alts/src/main/java/io/grpc/alts/AltsChannelBuilder.java @@ -16,8 +16,6 @@ package io.grpc.alts; -import static com.google.common.base.Preconditions.checkArgument; - import com.google.common.annotations.VisibleForTesting; import io.grpc.CallOptions; import io.grpc.Channel; @@ -38,14 +36,9 @@ import io.grpc.alts.internal.TsiHandshaker; import io.grpc.alts.internal.TsiHandshakerFactory; import io.grpc.internal.GrpcUtil; import io.grpc.internal.ObjectPool; -import io.grpc.internal.ProxyParameters; import io.grpc.internal.SharedResourcePool; import io.grpc.netty.InternalNettyChannelBuilder; -import io.grpc.netty.InternalNettyChannelBuilder.TransportCreationParamsFilter; -import io.grpc.netty.InternalNettyChannelBuilder.TransportCreationParamsFilterFactory; import io.grpc.netty.NettyChannelBuilder; -import java.net.InetSocketAddress; -import java.net.SocketAddress; import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; @@ -64,9 +57,11 @@ public final class AltsChannelBuilder extends ForwardingChannelBuilder handshakerChannelPool = SharedResourcePool.forResource(HandshakerServiceChannel.SHARED_HANDSHAKER_CHANNEL); - private TcpfFactory tcpfFactoryForTest; private boolean enableUntrustedAlts; + private AltsProtocolNegotiator negotiatorForTest; + private AltsClientOptions handshakerOptionsForTest; + /** "Overrides" the static method in {@link ManagedChannelBuilder}. */ public static final AltsChannelBuilder forTarget(String target) { return new AltsChannelBuilder(target); @@ -85,6 +80,8 @@ public final class AltsChannelBuilder extends ForwardingChannelBuilder { private final NettyChannelBuilder delegate; - private final TcpfFactory tcpfFactory; + private GoogleDefaultProtocolNegotiator negotiatorForTest; private GoogleDefaultChannelBuilder(String target) { delegate = NettyChannelBuilder.forTarget(target); - - final AltsClientOptions handshakerOptions = - new AltsClientOptions.Builder() - .setRpcProtocolVersions(RpcProtocolVersionsUtil.getRpcProtocolVersions()) - .build(); - TsiHandshakerFactory altsHandshakerFactory = - new TsiHandshakerFactory() { - @Override - public TsiHandshaker newHandshaker() { - // Used the shared grpc channel to connecting to the ALTS handshaker service. - // TODO: Release the channel if it is not used. - // https://github.com/grpc/grpc-java/issues/4755. - ManagedChannel channel = - SharedResourceHolder.get(HandshakerServiceChannel.SHARED_HANDSHAKER_CHANNEL); - return AltsTsiHandshaker.newClient( - HandshakerServiceGrpc.newStub(channel), handshakerOptions); - } - }; - SslContext sslContext; - try { - sslContext = GrpcSslContexts.forClient().build(); - } catch (SSLException ex) { - throw new RuntimeException(ex); - } - tcpfFactory = new TcpfFactory( - new GoogleDefaultProtocolNegotiator(altsHandshakerFactory, sslContext)); - InternalNettyChannelBuilder.setDynamicTransportParamsFactory(delegate(), tcpfFactory); + InternalNettyChannelBuilder + .setProtocolNegotiatorFactory(delegate(), new ProtocolNegotiatorFactory()); } /** "Overrides" the static method in {@link ManagedChannelBuilder}. */ @@ -125,48 +93,39 @@ public final class GoogleDefaultChannelBuilder } @VisibleForTesting - TransportCreationParamsFilterFactory getTcpfFactoryForTest() { - return tcpfFactory; + GoogleDefaultProtocolNegotiator getProtocolNegotiatorForTest() { + return negotiatorForTest; } - private static final class TcpfFactory implements TransportCreationParamsFilterFactory { - private final GoogleDefaultProtocolNegotiator negotiator; - - private TcpfFactory(GoogleDefaultProtocolNegotiator negotiator) { - this.negotiator = negotiator; - } - + private final class ProtocolNegotiatorFactory + implements InternalNettyChannelBuilder.ProtocolNegotiatorFactory { @Override - public TransportCreationParamsFilter create( - final SocketAddress serverAddress, - final String authority, - final String userAgent, - final ProxyParameters proxy) { - checkArgument( - serverAddress instanceof InetSocketAddress, - "%s must be a InetSocketAddress", - serverAddress); - return new TransportCreationParamsFilter() { - @Override - public SocketAddress getTargetServerAddress() { - return serverAddress; - } - - @Override - public String getAuthority() { - return authority; - } - - @Override - public String getUserAgent() { - return userAgent; - } - - @Override - public GoogleDefaultProtocolNegotiator getProtocolNegotiator() { - return negotiator; - } - }; + public GoogleDefaultProtocolNegotiator buildProtocolNegotiator() { + final AltsClientOptions handshakerOptions = + new AltsClientOptions.Builder() + .setRpcProtocolVersions(RpcProtocolVersionsUtil.getRpcProtocolVersions()) + .build(); + TsiHandshakerFactory altsHandshakerFactory = + new TsiHandshakerFactory() { + @Override + public TsiHandshaker newHandshaker() { + // Used the shared grpc channel to connecting to the ALTS handshaker service. + // TODO: Release the channel if it is not used. + // https://github.com/grpc/grpc-java/issues/4755. + ManagedChannel channel = + SharedResourceHolder.get(HandshakerServiceChannel.SHARED_HANDSHAKER_CHANNEL); + return AltsTsiHandshaker.newClient( + HandshakerServiceGrpc.newStub(channel), handshakerOptions); + } + }; + SslContext sslContext; + try { + sslContext = GrpcSslContexts.forClient().build(); + } catch (SSLException ex) { + throw new RuntimeException(ex); + } + return negotiatorForTest = + new GoogleDefaultProtocolNegotiator(altsHandshakerFactory, sslContext); } } diff --git a/alts/src/main/java/io/grpc/alts/internal/GoogleDefaultProtocolNegotiator.java b/alts/src/main/java/io/grpc/alts/internal/GoogleDefaultProtocolNegotiator.java index 84288945f3..0f61ff8781 100644 --- a/alts/src/main/java/io/grpc/alts/internal/GoogleDefaultProtocolNegotiator.java +++ b/alts/src/main/java/io/grpc/alts/internal/GoogleDefaultProtocolNegotiator.java @@ -35,7 +35,8 @@ public final class GoogleDefaultProtocolNegotiator implements ProtocolNegotiator @VisibleForTesting GoogleDefaultProtocolNegotiator( - ProtocolNegotiator altsProtocolNegotiator, ProtocolNegotiator tlsProtocolNegotiator) { + ProtocolNegotiator altsProtocolNegotiator, + ProtocolNegotiator tlsProtocolNegotiator) { this.altsProtocolNegotiator = altsProtocolNegotiator; this.tlsProtocolNegotiator = tlsProtocolNegotiator; } diff --git a/alts/src/test/java/io/grpc/alts/AltsChannelBuilderTest.java b/alts/src/test/java/io/grpc/alts/AltsChannelBuilderTest.java index 747992ab63..619726b175 100644 --- a/alts/src/test/java/io/grpc/alts/AltsChannelBuilderTest.java +++ b/alts/src/test/java/io/grpc/alts/AltsChannelBuilderTest.java @@ -21,9 +21,7 @@ import static com.google.common.truth.Truth.assertThat; import io.grpc.alts.internal.AltsClientOptions; import io.grpc.alts.internal.AltsProtocolNegotiator; import io.grpc.alts.internal.TransportSecurityCommon.RpcProtocolVersions; -import io.grpc.netty.InternalNettyChannelBuilder.TransportCreationParamsFilterFactory; import io.grpc.netty.ProtocolNegotiator; -import java.net.InetSocketAddress; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -36,22 +34,18 @@ public final class AltsChannelBuilderTest { AltsChannelBuilder builder = AltsChannelBuilder.forTarget("localhost:8080").enableUntrustedAltsForTesting(); - TransportCreationParamsFilterFactory tcpfFactory = builder.getTcpfFactoryForTest(); + ProtocolNegotiator protocolNegotiator = builder.getProtocolNegotiatorForTest(); AltsClientOptions altsClientOptions = builder.getAltsClientOptionsForTest(); - assertThat(tcpfFactory).isNull(); + assertThat(protocolNegotiator).isNull(); assertThat(altsClientOptions).isNull(); builder.build(); - tcpfFactory = builder.getTcpfFactoryForTest(); + protocolNegotiator = builder.getProtocolNegotiatorForTest(); altsClientOptions = builder.getAltsClientOptionsForTest(); - assertThat(tcpfFactory).isNotNull(); - ProtocolNegotiator protocolNegotiator = - tcpfFactory - .create(new InetSocketAddress(8080), "fakeAuthority", "fakeUserAgent", null) - .getProtocolNegotiator(); + assertThat(protocolNegotiator).isNotNull(); assertThat(protocolNegotiator).isInstanceOf(AltsProtocolNegotiator.class); assertThat(altsClientOptions).isNotNull(); diff --git a/alts/src/test/java/io/grpc/alts/GoogleDefaultChannelBuilderTest.java b/alts/src/test/java/io/grpc/alts/GoogleDefaultChannelBuilderTest.java index b681c733b4..d9bba544ad 100644 --- a/alts/src/test/java/io/grpc/alts/GoogleDefaultChannelBuilderTest.java +++ b/alts/src/test/java/io/grpc/alts/GoogleDefaultChannelBuilderTest.java @@ -19,9 +19,7 @@ package io.grpc.alts; import static com.google.common.truth.Truth.assertThat; import io.grpc.alts.internal.GoogleDefaultProtocolNegotiator; -import io.grpc.netty.InternalNettyChannelBuilder.TransportCreationParamsFilterFactory; import io.grpc.netty.ProtocolNegotiator; -import java.net.InetSocketAddress; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -32,13 +30,9 @@ public final class GoogleDefaultChannelBuilderTest { @Test public void buildsNettyChannel() throws Exception { GoogleDefaultChannelBuilder builder = GoogleDefaultChannelBuilder.forTarget("localhost:8080"); + builder.build(); - TransportCreationParamsFilterFactory tcpfFactory = builder.getTcpfFactoryForTest(); - assertThat(tcpfFactory).isNotNull(); - ProtocolNegotiator protocolNegotiator = - tcpfFactory - .create(new InetSocketAddress(8080), "fakeAuthority", "fakeUserAgent", null) - .getProtocolNegotiator(); + ProtocolNegotiator protocolNegotiator = builder.getProtocolNegotiatorForTest(); assertThat(protocolNegotiator).isInstanceOf(GoogleDefaultProtocolNegotiator.class); } }