Skip to content

Commit

Permalink
DnsResolverBuilder methods should make it clear that these are for Da… (
Browse files Browse the repository at this point in the history
#14379)

…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
  • Loading branch information
normanmaurer authored Sep 30, 2024
1 parent e87ce47 commit 232a5ab
Show file tree
Hide file tree
Showing 9 changed files with 72 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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());
}

Expand Down Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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.
*
Expand All @@ -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.
*
Expand All @@ -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.
*
Expand All @@ -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.
*
Expand Down Expand Up @@ -603,7 +638,7 @@ public DnsNameResolver build() {

return new DnsNameResolver(
eventLoop,
channelFactory,
datagramChannelFactory,
socketChannelFactory,
retryOnTimeout,
resolveCache,
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 232a5ab

Please sign in to comment.