DnsAddressResolverGroup to use pluggable DnsNameResolverBuilder (#7793)

Motivation:

Right now to customize DNS name resolver when using DnsAddressResolverGroup
one should subclass implementation and override newNameResolver method when
in fact it's possible to collect all settings in a DnsNameResolverBuilder
instance. Described in #7749.

Modifications:

- Added new constructor for DnsNameResolverBuilder in order to delay
  EventLoop specification

- Added copy() method to DnsNameResolverBuilder to provide an immutable
  copy of the builder

- Added new single-argument constructor for DnsAddressResolverGroup and
  RoundRobinDnsAddressResolverGroup accepting DnsNameResolverBuilder
  instance

- DnsAddressResolverGroup to build a new resolver using DnsNameResolverBuilder
  given instead of creating a new one

- Test cases to check that changing channelFactory after the builder was passed
  to create a DnsNameResolverGroup would not propagate to the name resolver

Result:

Much easier to customize DNS settings w/o subclassing DnsAddressResolverGroup

resolver-dns/src/main/java/io/netty/resolver/dns/DnsAddressResolverGroup.java

@@ -42,23 +42,27 @@ import static io.netty.util.internal.PlatformDependent.newConcurrentHashMap;
 @UnstableApi
 public class DnsAddressResolverGroup extends AddressResolverGroup<InetSocketAddress> {
 
-    private final ChannelFactory<? extends DatagramChannel> channelFactory;
+    private final DnsNameResolverBuilder dnsResolverBuilder;
-    private final DnsServerAddressStreamProvider nameServerProvider;
 
     private final ConcurrentMap<String, Promise<InetAddress>> resolvesInProgress = newConcurrentHashMap();
     private final ConcurrentMap<String, Promise<List<InetAddress>>> resolveAllsInProgress = newConcurrentHashMap();
 
+    public DnsAddressResolverGroup(DnsNameResolverBuilder dnsResolverBuilder) {
+        this.dnsResolverBuilder = dnsResolverBuilder.copy();
+    }
+
     public DnsAddressResolverGroup(
             Class<? extends DatagramChannel> channelType,
             DnsServerAddressStreamProvider nameServerProvider) {
-        this(new ReflectiveChannelFactory<DatagramChannel>(channelType), nameServerProvider);
+        this(new DnsNameResolverBuilder());
+        dnsResolverBuilder.channelType(channelType).nameServerProvider(nameServerProvider);
     }
 
     public DnsAddressResolverGroup(
             ChannelFactory<? extends DatagramChannel> channelFactory,
             DnsServerAddressStreamProvider nameServerProvider) {
-        this.channelFactory = channelFactory;
+        this(new DnsNameResolverBuilder());
-        this.nameServerProvider = nameServerProvider;
+        dnsResolverBuilder.channelFactory(channelFactory).nameServerProvider(nameServerProvider);
     }
 
     @SuppressWarnings("deprecation")
@@ -70,7 +74,11 @@ public class DnsAddressResolverGroup extends AddressResolverGroup<InetSocketAddr
                     " (expected: " + StringUtil.simpleClassName(EventLoop.class));
         }
 
-        return newResolver((EventLoop) executor, channelFactory, nameServerProvider);
+        // we don't really need to pass channelFactory and nameServerProvider separately,
+        // but still keep this to ensure backward compatibility with (potentially) override methods
+        return newResolver((EventLoop) executor,
+                dnsResolverBuilder.channelFactory(),
+                dnsResolverBuilder.nameServerProvider());
     }
 
     /**
@@ -98,7 +106,9 @@ public class DnsAddressResolverGroup extends AddressResolverGroup<InetSocketAddr
                                                         ChannelFactory<? extends DatagramChannel> channelFactory,
                                                         DnsServerAddressStreamProvider nameServerProvider)
             throws Exception {
-        return new DnsNameResolverBuilder(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 dnsResolverBuilder.eventLoop(eventLoop)
                 .channelFactory(channelFactory)
                 .nameServerProvider(nameServerProvider)
                 .build();

resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverBuilder.java

@@ -25,6 +25,7 @@ import io.netty.resolver.ResolvedAddressTypes;
 import io.netty.util.internal.UnstableApi;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 
 import static io.netty.resolver.dns.DnsServerAddressStreamProviders.platformDefault;
@@ -36,7 +37,7 @@ import static io.netty.util.internal.ObjectUtil.intValue;
  */
 @UnstableApi
 public final class DnsNameResolverBuilder {
-    private final EventLoop eventLoop;
+    private EventLoop eventLoop;
     private ChannelFactory<? extends DatagramChannel> channelFactory;
     private DnsCache resolveCache;
     private DnsCache authoritativeDnsServerCache;
@@ -58,14 +59,35 @@ public final class DnsNameResolverBuilder {
     private int ndots = -1;
     private boolean decodeIdn = true;
 
+    /**
+     * Creates a new builder.
+     */
+    public DnsNameResolverBuilder() {
+    }
+
     /**
      * Creates a new builder.
      *
-     * @param eventLoop the {@link EventLoop} the {@link EventLoop} which will perform the communication with the DNS
+     * @param eventLoop the {@link EventLoop} which will perform the communication with the DNS
      * servers.
      */
     public DnsNameResolverBuilder(EventLoop eventLoop) {
+        eventLoop(eventLoop);
+    }
+
+    /**
+     * Sets the {@link EventLoop} which will perform the communication with the DNS servers.
+     *
+     * @param eventLoop the {@link EventLoop}
+     * @return {@code this}
+     */
+    public DnsNameResolverBuilder eventLoop(EventLoop eventLoop) {
         this.eventLoop = eventLoop;
+        return this;
+    }
+
+    protected ChannelFactory<? extends DatagramChannel> channelFactory() {
+        return this.channelFactory;
     }
 
     /**
@@ -274,6 +296,10 @@ public final class DnsNameResolverBuilder {
         return this;
     }
 
+    protected DnsServerAddressStreamProvider nameServerProvider() {
+        return this.dnsServerAddressStreamProvider;
+    }
+
     /**
      * Set the {@link DnsServerAddressStreamProvider} which is used to determine which DNS server is used to resolve
      * each hostname.
@@ -347,6 +373,10 @@ public final class DnsNameResolverBuilder {
      * @return a {@link DnsNameResolver}
      */
     public DnsNameResolver build() {
+        if (eventLoop == null) {
+            throw new IllegalStateException("eventLoop should be specified to build a DnsNameResolver.");
+        }
+
         if (resolveCache != null && (minTtl != null || maxTtl != null || negativeTtl != null)) {
             throw new IllegalStateException("resolveCache and TTLs are mutually exclusive");
         }
@@ -377,4 +407,63 @@ public final class DnsNameResolverBuilder {
                 ndots,
                 decodeIdn);
     }
+
+    /**
+     * Creates a copy of this {@link DnsNameResolverBuilder}
+     *
+     * @return {@link DnsNameResolverBuilder}
+     */
+    public DnsNameResolverBuilder copy() {
+        DnsNameResolverBuilder copiedBuilder = new DnsNameResolverBuilder();
+
+        if (eventLoop != null) {
+            copiedBuilder.eventLoop(eventLoop);
+        }
+
+        if (channelFactory != null) {
+            copiedBuilder.channelFactory(channelFactory);
+        }
+
+        if (resolveCache != null) {
+            copiedBuilder.resolveCache(resolveCache);
+        }
+
+        if (maxTtl != null && minTtl != null) {
+            copiedBuilder.ttl(minTtl, maxTtl);
+        }
+
+        if (negativeTtl != null) {
+            copiedBuilder.negativeTtl(negativeTtl);
+        }
+
+        if (authoritativeDnsServerCache != null) {
+            copiedBuilder.authoritativeDnsServerCache(authoritativeDnsServerCache);
+        }
+
+        if (dnsQueryLifecycleObserverFactory != null) {
+            copiedBuilder.dnsQueryLifecycleObserverFactory(dnsQueryLifecycleObserverFactory);
+        }
+
+        copiedBuilder.queryTimeoutMillis(queryTimeoutMillis);
+        copiedBuilder.resolvedAddressTypes(resolvedAddressTypes);
+        copiedBuilder.recursionDesired(recursionDesired);
+        copiedBuilder.maxQueriesPerResolve(maxQueriesPerResolve);
+        copiedBuilder.traceEnabled(traceEnabled);
+        copiedBuilder.maxPayloadSize(maxPayloadSize);
+        copiedBuilder.optResourceEnabled(optResourceEnabled);
+        copiedBuilder.hostsFileEntriesResolver(hostsFileEntriesResolver);
+
+        if (dnsServerAddressStreamProvider != null) {
+            copiedBuilder.nameServerProvider(dnsServerAddressStreamProvider);
+        }
+
+        if (searchDomains != null) {
+            copiedBuilder.searchDomains(Arrays.asList(searchDomains));
+        }
+
+        copiedBuilder.ndots(ndots);
+        copiedBuilder.decodeIdn(decodeIdn);
+
+        return copiedBuilder;
+    }
 }

resolver-dns/src/main/java/io/netty/resolver/dns/RoundRobinDnsAddressResolverGroup.java

@@ -36,6 +36,10 @@ import java.net.InetSocketAddress;
 @UnstableApi
 public class RoundRobinDnsAddressResolverGroup extends DnsAddressResolverGroup {
 
+    public RoundRobinDnsAddressResolverGroup(DnsNameResolverBuilder dnsResolverBuilder) {
+        super(dnsResolverBuilder);
+    }
+
     public RoundRobinDnsAddressResolverGroup(
             Class<? extends DatagramChannel> channelType,
             DnsServerAddressStreamProvider nameServerProvider) {

resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverTest.java

@@ -18,6 +18,7 @@ package io.netty.resolver.dns;
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.ByteBufHolder;
 import io.netty.channel.AddressedEnvelope;
+import io.netty.channel.ChannelFactory;
 import io.netty.channel.ChannelFuture;
 import io.netty.channel.EventLoop;
 import io.netty.channel.EventLoopGroup;
@@ -1620,4 +1621,20 @@ public class DnsNameResolverTest {
         assertTrue(DnsNameResolver.isTransportOrTimeoutError(cause));
         resolver.close();
     }
+
+    @Test
+    public void testDnsNameResolverBuilderCopy() {
+        ChannelFactory<DatagramChannel> channelFactory =
+                new ReflectiveChannelFactory<DatagramChannel>(NioDatagramChannel.class);
+        DnsNameResolverBuilder builder = new DnsNameResolverBuilder(group.next())
+                .channelFactory(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());
+    }
 }