DnsResolverBuilder methods should make it clear that these are for Da… (#14379)

normanmaurer · web-flow · commit 232a5abfdd5f · 2024-09-30T14:28:35.000+02:00
…tagramChannel

Motivation:

We should add new methods (and deprecate the old ones) to make it more
clear that these are responsible for creating DatagramChannels.

Modifications:

- Add datagramChannelType(...) and datagramChannelFactory(...) and
deprecate the old channelType(...) and channelFactory(...) methods
- Adjust code to use the new methods in favor of the new deprecated
methods

Result:

Cleanup
diff --git a/handler-ssl-ocsp/src/main/java/io/netty/handler/ssl/ocsp/OcspServerCertificateValidator.java b/handler-ssl-ocsp/src/main/java/io/netty/handler/ssl/ocsp/OcspServerCertificateValidator.java
@@ -122,7 +122,7 @@ public OcspServerCertificateValidator(boolean closeAndThrowIfNotValid, boolean v
     protected static DnsNameResolver createDefaultResolver(final IoTransport ioTransport) {
         return new DnsNameResolverBuilder()
                 .eventLoop(ioTransport.eventLoop())
-                .channelFactory(ioTransport.datagramChannel())
+                .datagramChannelFactory(ioTransport.datagramChannel())
                 .socketChannelFactory(ioTransport.socketChannel())
                 .build();
     }
diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsAddressResolverGroup.java b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsAddressResolverGroup.java
@@ -52,14 +52,14 @@ public DnsAddressResolverGroup(
             Class<? extends DatagramChannel> channelType,
             DnsServerAddressStreamProvider nameServerProvider) {
         this.dnsResolverBuilder = withSharedCaches(new DnsNameResolverBuilder());
-        dnsResolverBuilder.channelType(channelType).nameServerProvider(nameServerProvider);
+        dnsResolverBuilder.datagramChannelType(channelType).nameServerProvider(nameServerProvider);
     }
 
     public DnsAddressResolverGroup(
             ChannelFactory<? extends DatagramChannel> channelFactory,
             DnsServerAddressStreamProvider nameServerProvider) {
         this.dnsResolverBuilder = withSharedCaches(new DnsNameResolverBuilder());
-        dnsResolverBuilder.channelFactory(channelFactory).nameServerProvider(nameServerProvider);
+        dnsResolverBuilder.datagramChannelFactory(channelFactory).nameServerProvider(nameServerProvider);
     }
 
     private static DnsNameResolverBuilder withSharedCaches(DnsNameResolverBuilder dnsResolverBuilder) {
@@ -83,7 +83,7 @@ protected final AddressResolver<InetSocketAddress> newResolver(EventExecutor exe
         // but still keep this to ensure backward compatibility with (potentially) override methods
         EventLoop loop = dnsResolverBuilder.eventLoop;
         return newResolver(loop == null ? (EventLoop) executor : loop,
-                dnsResolverBuilder.channelFactory(),
+                dnsResolverBuilder.datagramChannelFactory(),
                 dnsResolverBuilder.nameServerProvider());
     }
 
@@ -117,7 +117,7 @@ protected NameResolver<InetAddress> newNameResolver(EventLoop eventLoop,
         // once again, channelFactory and nameServerProvider are most probably set in builder already,
         // but I do reassign them again to avoid corner cases with override methods
         return builder.eventLoop(eventLoop)
-                .channelFactory(channelFactory)
+                .datagramChannelFactory(channelFactory)
                 .nameServerProvider(nameServerProvider)
                 .build();
     }
diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverBuilder.java b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverBuilder.java
@@ -47,7 +47,7 @@ public final class DnsNameResolverBuilder {
     static final SocketAddress DEFAULT_LOCAL_ADDRESS = new InetSocketAddress(0);
 
     volatile EventLoop eventLoop;
-    private ChannelFactory<? extends DatagramChannel> channelFactory;
+    private ChannelFactory<? extends DatagramChannel> datagramChannelFactory;
     private ChannelFactory<? extends SocketChannel> socketChannelFactory;
     private boolean retryOnTimeout;
 
@@ -105,40 +105,75 @@ public DnsNameResolverBuilder eventLoop(EventLoop eventLoop) {
         return this;
     }
 
-    protected ChannelFactory<? extends DatagramChannel> channelFactory() {
-        return this.channelFactory;
+    ChannelFactory<? extends DatagramChannel> datagramChannelFactory() {
+        return this.datagramChannelFactory;
     }
 
     /**
      * Sets the {@link ChannelFactory} that will create a {@link DatagramChannel}.
+     * <p>
      * If <a href="https://tools.ietf.org/html/rfc7766">TCP fallback</a> should be supported as well it is required
      * to call the {@link #socketChannelFactory(ChannelFactory) or {@link #socketChannelType(Class)}} method.
      *
-     * @param channelFactory the {@link ChannelFactory}
+     * @param datagramChannelFactory the {@link ChannelFactory}
      * @return {@code this}
+     * @deprecated use {@link #datagramChannelFactory(ChannelFactory)}
      */
-    public DnsNameResolverBuilder channelFactory(ChannelFactory<? extends DatagramChannel> channelFactory) {
-        this.channelFactory = channelFactory;
+    @Deprecated
+    public DnsNameResolverBuilder channelFactory(ChannelFactory<? extends DatagramChannel> datagramChannelFactory) {
+        datagramChannelFactory(datagramChannelFactory);
+        return this;
+    }
+
+    /**
+     * Sets the {@link ChannelFactory} that will create a {@link DatagramChannel}.
+     * <p>
+     * If <a href="https://tools.ietf.org/html/rfc7766">TCP fallback</a> should be supported as well it is required
+     * to call the {@link #socketChannelFactory(ChannelFactory) or {@link #socketChannelType(Class)}} method.
+     *
+     * @param datagramChannelFactory the {@link ChannelFactory}
+     * @return {@code this}
+     */
+    public DnsNameResolverBuilder datagramChannelFactory(
+            ChannelFactory<? extends DatagramChannel> datagramChannelFactory) {
+        this.datagramChannelFactory = datagramChannelFactory;
         return this;
     }
 
     /**
      * Sets the {@link ChannelFactory} as a {@link ReflectiveChannelFactory} of this type.
      * Use as an alternative to {@link #channelFactory(ChannelFactory)}.
+     * <p>
      * If <a href="https://tools.ietf.org/html/rfc7766">TCP fallback</a> should be supported as well it is required
      * to call the {@link #socketChannelFactory(ChannelFactory) or {@link #socketChannelType(Class)}} method.
      *
      * @param channelType the type
      * @return {@code this}
+     * @deprecated use {@link #datagramChannelType(Class)}
      */
+    @Deprecated
     public DnsNameResolverBuilder channelType(Class<? extends DatagramChannel> channelType) {
-        return channelFactory(new ReflectiveChannelFactory<DatagramChannel>(channelType));
+        return datagramChannelFactory(new ReflectiveChannelFactory<DatagramChannel>(channelType));
+    }
+
+    /**
+     * Sets the {@link ChannelFactory} as a {@link ReflectiveChannelFactory} of this type.
+     * Use as an alternative to {@link #datagramChannelFactory(ChannelFactory)}.
+     * <p>
+     * If <a href="https://tools.ietf.org/html/rfc7766">TCP fallback</a> should be supported as well it is required
+     * to call the {@link #socketChannelFactory(ChannelFactory) or {@link #socketChannelType(Class)}} method.
+     *
+     * @param channelType the type
+     * @return {@code this}
+     */
+    public DnsNameResolverBuilder datagramChannelType(Class<? extends DatagramChannel> channelType) {
+        return datagramChannelFactory(new ReflectiveChannelFactory<DatagramChannel>(channelType));
     }
 
     /**
      * Sets the {@link ChannelFactory} that will create a {@link SocketChannel} for
      * <a href="https://tools.ietf.org/html/rfc7766">TCP fallback</a> if needed.
-     *
+     * <p>
      * TCP fallback is <strong>not</strong> enabled by default and must be enabled by providing a non-null
      * {@link ChannelFactory} for this method.
      *
@@ -155,7 +190,7 @@ public DnsNameResolverBuilder socketChannelFactory(ChannelFactory<? extends Sock
      * Sets the {@link ChannelFactory} as a {@link ReflectiveChannelFactory} of this type for
      * <a href="https://tools.ietf.org/html/rfc7766">TCP fallback</a> if needed.
      * Use as an alternative to {@link #socketChannelFactory(ChannelFactory)}.
-     *
+     * <p>
      * TCP fallback is <strong>not</strong> enabled by default and must be enabled by providing a non-null
      * {@code channelType} for this method.
      *
@@ -170,7 +205,7 @@ public DnsNameResolverBuilder socketChannelType(Class<? extends SocketChannel> c
     /**
      * Sets the {@link ChannelFactory} that will create a {@link SocketChannel} for
      * <a href="https://tools.ietf.org/html/rfc7766">TCP fallback</a> if needed.
-     *
+     * <p>
      * TCP fallback is <strong>not</strong> enabled by default and must be enabled by providing a non-null
      * {@link ChannelFactory} for this method.
      *
@@ -193,7 +228,7 @@ public DnsNameResolverBuilder socketChannelFactory(
      * Sets the {@link ChannelFactory} as a {@link ReflectiveChannelFactory} of this type for
      * <a href="https://tools.ietf.org/html/rfc7766">TCP fallback</a> if needed.
      * Use as an alternative to {@link #socketChannelFactory(ChannelFactory)}.
-     *
+     * <p>
      * TCP fallback is <strong>not</strong> enabled by default and must be enabled by providing a non-null
      * {@code channelType} for this method.
      *
@@ -603,7 +638,7 @@ public DnsNameResolver build() {
 
         return new DnsNameResolver(
                 eventLoop,
-                channelFactory,
+                datagramChannelFactory,
                 socketChannelFactory,
                 retryOnTimeout,
                 resolveCache,
@@ -640,8 +675,8 @@ public DnsNameResolverBuilder copy() {
             copiedBuilder.eventLoop(eventLoop);
         }
 
-        if (channelFactory != null) {
-            copiedBuilder.channelFactory(channelFactory);
+        if (datagramChannelFactory != null) {
+            copiedBuilder.datagramChannelFactory(datagramChannelFactory);
         }
 
         copiedBuilder.socketChannelFactory(socketChannelFactory, retryOnTimeout);
diff --git a/resolver-dns/src/test/java/io/netty/resolver/dns/DnsAddressResolverGroupTest.java b/resolver-dns/src/test/java/io/netty/resolver/dns/DnsAddressResolverGroupTest.java
@@ -44,7 +44,7 @@ public void testUseConfiguredEventLoop() throws InterruptedException {
         final EventLoop loop = group.next();
         DefaultEventLoopGroup defaultEventLoopGroup = new DefaultEventLoopGroup(1);
         DnsNameResolverBuilder builder = new DnsNameResolverBuilder()
-                .eventLoop(loop).channelType(NioDatagramChannel.class);
+                .eventLoop(loop).datagramChannelType(NioDatagramChannel.class);
         DnsAddressResolverGroup resolverGroup = new DnsAddressResolverGroup(builder);
         try {
             final Promise<?> promise = loop.newPromise();
@@ -77,7 +77,7 @@ public void testSharedDNSCacheAcrossEventLoops() throws InterruptedException, Ex
         NioEventLoopGroup group = new NioEventLoopGroup(1);
         final EventLoop loop = group.next();
         DnsNameResolverBuilder builder = new DnsNameResolverBuilder()
-                .eventLoop(loop).channelType(NioDatagramChannel.class);
+                .eventLoop(loop).datagramChannelType(NioDatagramChannel.class);
         DnsAddressResolverGroup resolverGroup = new DnsAddressResolverGroup(builder);
         DefaultEventLoopGroup defaultEventLoopGroup = new DefaultEventLoopGroup(2);
         EventLoop eventLoop1 = defaultEventLoopGroup.next();
diff --git a/resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverBuilderTest.java b/resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverBuilderTest.java
@@ -40,7 +40,7 @@ class DnsNameResolverBuilderTest {
 
     @BeforeEach
     void setUp() {
-        builder = new DnsNameResolverBuilder(GROUP.next()).channelType(NioDatagramChannel.class);
+        builder = new DnsNameResolverBuilder(GROUP.next()).datagramChannelType(NioDatagramChannel.class);
     }
 
     @AfterEach
diff --git a/resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverClientSubnetTest.java b/resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverClientSubnetTest.java
@@ -59,7 +59,7 @@ public void testSubnetQuery() throws Exception {
 
     private static DnsNameResolverBuilder newResolver(EventLoopGroup group) {
         return new DnsNameResolverBuilder(group.next())
-                .channelType(NioDatagramChannel.class)
+                .datagramChannelType(NioDatagramChannel.class)
                 .nameServerProvider(
                         new SingletonDnsServerAddressStreamProvider(SocketUtils.socketAddress("8.8.8.8", 53)))
                 .maxQueriesPerResolve(1)
diff --git a/resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverTest.java b/resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverTest.java
@@ -396,7 +396,7 @@ private static DnsNameResolverBuilder newResolver(boolean decodeToUnicode,
                                                       TestDnsServer dnsServer) {
         DnsNameResolverBuilder builder = new DnsNameResolverBuilder(group.next())
                 .dnsQueryLifecycleObserverFactory(new TestRecursiveCacheDnsQueryLifecycleObserverFactory())
-                .channelType(NioDatagramChannel.class)
+                .datagramChannelType(NioDatagramChannel.class)
                 .maxQueriesPerResolve(1)
                 .decodeIdn(decodeToUnicode)
                 .optResourceEnabled(false)
@@ -1155,7 +1155,7 @@ public void testResolveAllMx() {
     @Test
     public void testResolveAllHostsFile() {
         final DnsNameResolver resolver = new DnsNameResolverBuilder(group.next())
-                .channelType(NioDatagramChannel.class)
+                .datagramChannelType(NioDatagramChannel.class)
                 .hostsFileEntriesResolver(new HostsFileEntriesResolver() {
                     @Override
                     public InetAddress address(String inetHost, ResolvedAddressTypes resolvedAddressTypes) {
@@ -1243,7 +1243,7 @@ private static void secondDnsServerShouldBeUsedBeforeCNAME(boolean startDnsServe
             DnsNameResolverBuilder builder = new DnsNameResolverBuilder(group.next())
                     .dnsQueryLifecycleObserverFactory(lifecycleObserverFactory)
                     .resolvedAddressTypes(ResolvedAddressTypes.IPV4_ONLY)
-                    .channelType(NioDatagramChannel.class)
+                    .datagramChannelType(NioDatagramChannel.class)
                     .queryTimeoutMillis(1000) // We expect timeouts if startDnsServer1 is false
                     .optResourceEnabled(false)
                     .ndots(1);
@@ -1292,7 +1292,7 @@ public void aAndAAAAQueryShouldTryFirstDnsServerBeforeSecond() throws IOExceptio
             DnsNameResolverBuilder builder = new DnsNameResolverBuilder(group.next())
                     .resolvedAddressTypes(ResolvedAddressTypes.IPV4_ONLY)
                     .dnsQueryLifecycleObserverFactory(lifecycleObserverFactory)
-                    .channelType(NioDatagramChannel.class)
+                    .datagramChannelType(NioDatagramChannel.class)
                     .optResourceEnabled(false)
                     .ndots(1);
 
@@ -2409,15 +2409,15 @@ public void testDnsNameResolverBuilderCopy() {
         ChannelFactory<DatagramChannel> channelFactory =
                 new ReflectiveChannelFactory<DatagramChannel>(NioDatagramChannel.class);
         DnsNameResolverBuilder builder = new DnsNameResolverBuilder(group.next())
-                .channelFactory(channelFactory);
+                .datagramChannelFactory(channelFactory);
         DnsNameResolverBuilder copiedBuilder = builder.copy();
 
         // change channel factory does not propagate to previously made copy
         ChannelFactory<DatagramChannel> newChannelFactory =
                 new ReflectiveChannelFactory<DatagramChannel>(NioDatagramChannel.class);
-        builder.channelFactory(newChannelFactory);
-        assertEquals(channelFactory, copiedBuilder.channelFactory());
-        assertEquals(newChannelFactory, builder.channelFactory());
+        builder.datagramChannelFactory(newChannelFactory);
+        assertEquals(channelFactory, copiedBuilder.datagramChannelFactory());
+        assertEquals(newChannelFactory, builder.datagramChannelFactory());
     }
 
     @Test
@@ -2771,7 +2771,7 @@ public void testResolveACachedWithDotSearchDomain() throws Exception {
     public void testChannelFactoryException() {
         final IllegalStateException exception = new IllegalStateException();
         try {
-            newResolver().channelFactory(new ChannelFactory<DatagramChannel>() {
+            newResolver().datagramChannelFactory(new ChannelFactory<DatagramChannel>() {
                 @Override
                 public DatagramChannel newChannel() {
                     throw exception;
@@ -3260,7 +3260,7 @@ public DatagramChannel newChannel() {
                     return datagramChannel;
                 }
             };
-            builder.channelFactory(channelFactory);
+            builder.datagramChannelFactory(channelFactory);
             if (tcpFallback) {
                 // If we are configured to use TCP as a fallback also bind a TCP socket
                 serverSocket = startDnsServerAndCreateServerSocket(dnsServer2);
@@ -3423,7 +3423,7 @@ public DatagramChannel newChannel() {
                     return datagramChannel;
                 }
             };
-            builder.channelFactory(channelFactory);
+            builder.datagramChannelFactory(channelFactory);
             serverSocket = startDnsServerAndCreateServerSocket(dnsServer2);
             // If we are configured to use TCP as a fallback also bind a TCP socket
             builder.socketChannelType(NioSocketChannel.class, true);
@@ -3507,7 +3507,7 @@ protected DnsMessage filterMessage(DnsMessage message) {
                                                              dnsServer2.localAddress());
         final DnsNameResolver resolver = new DnsNameResolverBuilder(group.next())
                 .dnsQueryLifecycleObserverFactory(new TestRecursiveCacheDnsQueryLifecycleObserverFactory())
-                .channelType(NioDatagramChannel.class)
+                .datagramChannelType(NioDatagramChannel.class)
                 .optResourceEnabled(false)
                 .nameServerProvider(nameServerProvider)
                 .build();
diff --git a/resolver-dns/src/test/java/io/netty/resolver/dns/SearchDomainTest.java b/resolver-dns/src/test/java/io/netty/resolver/dns/SearchDomainTest.java
@@ -45,7 +45,7 @@ public class SearchDomainTest {
 
     private DnsNameResolverBuilder newResolver() {
         return new DnsNameResolverBuilder(group.next())
-            .channelType(NioDatagramChannel.class)
+            .datagramChannelType(NioDatagramChannel.class)
             .nameServerProvider(new SingletonDnsServerAddressStreamProvider(dnsServer.localAddress()))
             .maxQueriesPerResolve(1)
             .optResourceEnabled(false)
diff --git a/transport-blockhound-tests/src/test/java/io/netty/util/internal/NettyBlockHoundIntegrationTest.java b/transport-blockhound-tests/src/test/java/io/netty/util/internal/NettyBlockHoundIntegrationTest.java
@@ -413,7 +413,7 @@ public void testUnixResolverDnsServerAddressStreamProvider_ParseEtcResolverSearc
         NioEventLoopGroup group = new NioEventLoopGroup();
         try {
             DnsNameResolverBuilder builder = new DnsNameResolverBuilder(group.next())
-                    .channelFactory(NioDatagramChannel::new);
+                    .datagramChannelFactory(NioDatagramChannel::new);
             doTestParseResolverFilesAllowsBlockingCalls(builder::build);
         } finally {
             group.shutdownGracefully();

Original file line number	Diff line number	Diff line change
`@@ -122,7 +122,7 @@ public OcspServerCertificateValidator(boolean closeAndThrowIfNotValid, boolean v`
`122`	`122`	`protected static DnsNameResolver createDefaultResolver(final IoTransport ioTransport) {`
`123`	`123`	`return new DnsNameResolverBuilder()`
`124`	`124`	`.eventLoop(ioTransport.eventLoop())`
`125`		`- .channelFactory(ioTransport.datagramChannel())`
	`125`	`+ .datagramChannelFactory(ioTransport.datagramChannel())`
`126`	`126`	`.socketChannelFactory(ioTransport.socketChannel())`
`127`	`127`	`.build();`
`128`	`128`	`}`
Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,7 @@ class DnsNameResolverBuilderTest {`
`40`	`40`
`41`	`41`	`@BeforeEach`
`42`	`42`	`void setUp() {`
`43`		`- builder = new DnsNameResolverBuilder(GROUP.next()).channelType(NioDatagramChannel.class);`
	`43`	`+ builder = new DnsNameResolverBuilder(GROUP.next()).datagramChannelType(NioDatagramChannel.class);`
`44`	`44`	`}`
`45`	`45`
`46`	`46`	`@AfterEach`