From c0ae5e697c3c4fdf81f0cfc199d18aacd3b8ecba Mon Sep 17 00:00:00 2001 From: Alexey Kachayev Date: Thu, 26 Apr 2018 09:04:01 +0300 Subject: [PATCH] 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/DnsAddressResolverGroup.java | 24 +++-- .../resolver/dns/DnsNameResolverBuilder.java | 93 ++++++++++++++++++- .../RoundRobinDnsAddressResolverGroup.java | 4 + .../resolver/dns/DnsNameResolverTest.java | 17 ++++ 4 files changed, 129 insertions(+), 9 deletions(-) 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 index fb30608232..303c42a098 100644 --- a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsAddressResolverGroup.java +++ b/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 { - private final ChannelFactory channelFactory; - private final DnsServerAddressStreamProvider nameServerProvider; + private final DnsNameResolverBuilder dnsResolverBuilder; private final ConcurrentMap> resolvesInProgress = newConcurrentHashMap(); private final ConcurrentMap>> resolveAllsInProgress = newConcurrentHashMap(); + public DnsAddressResolverGroup(DnsNameResolverBuilder dnsResolverBuilder) { + this.dnsResolverBuilder = dnsResolverBuilder.copy(); + } + public DnsAddressResolverGroup( Class channelType, DnsServerAddressStreamProvider nameServerProvider) { - this(new ReflectiveChannelFactory(channelType), nameServerProvider); + this(new DnsNameResolverBuilder()); + dnsResolverBuilder.channelType(channelType).nameServerProvider(nameServerProvider); } public DnsAddressResolverGroup( ChannelFactory channelFactory, DnsServerAddressStreamProvider nameServerProvider) { - this.channelFactory = channelFactory; - this.nameServerProvider = nameServerProvider; + this(new DnsNameResolverBuilder()); + dnsResolverBuilder.channelFactory(channelFactory).nameServerProvider(nameServerProvider); } @SuppressWarnings("deprecation") @@ -70,7 +74,11 @@ public class DnsAddressResolverGroup extends AddressResolverGroup 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(); 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 index 8ccf7c308f..a5df3409df 100644 --- a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverBuilder.java +++ b/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 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 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; + } } diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/RoundRobinDnsAddressResolverGroup.java b/resolver-dns/src/main/java/io/netty/resolver/dns/RoundRobinDnsAddressResolverGroup.java index 1fc072baa7..68c14fec61 100644 --- a/resolver-dns/src/main/java/io/netty/resolver/dns/RoundRobinDnsAddressResolverGroup.java +++ b/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 channelType, DnsServerAddressStreamProvider nameServerProvider) { 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 index 893ffa4d22..0ffe40412a 100644 --- a/resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverTest.java +++ b/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 channelFactory = + new ReflectiveChannelFactory(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 newChannelFactory = + new ReflectiveChannelFactory(NioDatagramChannel.class); + builder.channelFactory(newChannelFactory); + assertEquals(channelFactory, copiedBuilder.channelFactory()); + assertEquals(newChannelFactory, builder.channelFactory()); + } }