Decouple DnsCache and DnsCacheEntry

Motivation:
DnsCache (an interface) is coupled to DnsCacheEntry (a final class). This means that DnsCache implementations can't implement their own DnsCacheEntry objects if the default behavior isn't appropriate.

Modifications:
- DnsCacheEntry should be moved to DefaultDnsCache as it is an implementation detail
- DnsCache#cache(..) should return a new DnsCacheEntry
- The methods which from DnsCacheEntry that were used outside the scope of DefaultDnsCache should be moved into an interface

Result:
DnsCache is more extensible and not tightly coupled to a default implementation of DnsCacheEntry.
This commit is contained in:
Scott Mitchell 2017-08-18 19:16:51 -07:00
parent 2beb5fc8ee
commit 566566db3a
7 changed files with 152 additions and 106 deletions

View File

@ -17,6 +17,7 @@ package io.netty.resolver.dns;
import io.netty.channel.EventLoop;
import io.netty.handler.codec.dns.DnsRecord;
import io.netty.util.concurrent.ScheduledFuture;
import io.netty.util.internal.PlatformDependent;
import io.netty.util.internal.UnstableApi;
@ -38,7 +39,8 @@ import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero;
@UnstableApi
public class DefaultDnsCache implements DnsCache {
private final ConcurrentMap<String, List<DnsCacheEntry>> resolveCache = PlatformDependent.newConcurrentHashMap();
private final ConcurrentMap<String, List<DefaultDnsCacheEntry>> resolveCache =
PlatformDependent.newConcurrentHashMap();
private final int minTtl;
private final int maxTtl;
private final int negativeTtl;
@ -95,8 +97,9 @@ public class DefaultDnsCache implements DnsCache {
@Override
public void clear() {
for (Iterator<Map.Entry<String, List<DnsCacheEntry>>> i = resolveCache.entrySet().iterator(); i.hasNext();) {
final Map.Entry<String, List<DnsCacheEntry>> e = i.next();
for (Iterator<Map.Entry<String, List<DefaultDnsCacheEntry>>> i = resolveCache.entrySet().iterator();
i.hasNext();) {
final Map.Entry<String, List<DefaultDnsCacheEntry>> e = i.next();
i.remove();
cancelExpiration(e.getValue());
}
@ -106,8 +109,9 @@ public class DefaultDnsCache implements DnsCache {
public boolean clear(String hostname) {
checkNotNull(hostname, "hostname");
boolean removed = false;
for (Iterator<Map.Entry<String, List<DnsCacheEntry>>> i = resolveCache.entrySet().iterator(); i.hasNext();) {
final Map.Entry<String, List<DnsCacheEntry>> e = i.next();
for (Iterator<Map.Entry<String, List<DefaultDnsCacheEntry>>> i = resolveCache.entrySet().iterator();
i.hasNext();) {
final Map.Entry<String, List<DefaultDnsCacheEntry>> e = i.next();
if (e.getKey().equals(hostname)) {
i.remove();
cancelExpiration(e.getValue());
@ -122,7 +126,7 @@ public class DefaultDnsCache implements DnsCache {
}
@Override
public List<DnsCacheEntry> get(String hostname, DnsRecord[] additionals) {
public List<? extends DnsCacheEntry> get(String hostname, DnsRecord[] additionals) {
checkNotNull(hostname, "hostname");
if (!emptyAdditionals(additionals)) {
return null;
@ -130,11 +134,11 @@ public class DefaultDnsCache implements DnsCache {
return resolveCache.get(hostname);
}
private List<DnsCacheEntry> cachedEntries(String hostname) {
List<DnsCacheEntry> oldEntries = resolveCache.get(hostname);
final List<DnsCacheEntry> entries;
private List<DefaultDnsCacheEntry> cachedEntries(String hostname) {
List<DefaultDnsCacheEntry> oldEntries = resolveCache.get(hostname);
final List<DefaultDnsCacheEntry> entries;
if (oldEntries == null) {
List<DnsCacheEntry> newEntries = new ArrayList<DnsCacheEntry>(8);
List<DefaultDnsCacheEntry> newEntries = new ArrayList<DefaultDnsCacheEntry>(8);
oldEntries = resolveCache.putIfAbsent(hostname, newEntries);
entries = oldEntries != null? oldEntries : newEntries;
} else {
@ -144,21 +148,21 @@ public class DefaultDnsCache implements DnsCache {
}
@Override
public void cache(String hostname, DnsRecord[] additionals,
InetAddress address, long originalTtl, EventLoop loop) {
public DnsCacheEntry cache(String hostname, DnsRecord[] additionals,
InetAddress address, long originalTtl, EventLoop loop) {
checkNotNull(hostname, "hostname");
checkNotNull(address, "address");
checkNotNull(loop, "loop");
final DefaultDnsCacheEntry e = new DefaultDnsCacheEntry(hostname, address);
if (maxTtl == 0 || !emptyAdditionals(additionals)) {
return;
return e;
}
final int ttl = Math.max(minTtl, (int) Math.min(maxTtl, originalTtl));
final List<DnsCacheEntry> entries = cachedEntries(hostname);
final DnsCacheEntry e = new DnsCacheEntry(hostname, address);
final List<DefaultDnsCacheEntry> entries = cachedEntries(hostname);
synchronized (entries) {
if (!entries.isEmpty()) {
final DnsCacheEntry firstEntry = entries.get(0);
final DefaultDnsCacheEntry firstEntry = entries.get(0);
if (firstEntry.cause() != null) {
assert entries.size() == 1;
firstEntry.cancelExpiration();
@ -169,19 +173,20 @@ public class DefaultDnsCache implements DnsCache {
}
scheduleCacheExpiration(entries, e, ttl, loop);
return e;
}
@Override
public void cache(String hostname, DnsRecord[] additionals, Throwable cause, EventLoop loop) {
public DnsCacheEntry cache(String hostname, DnsRecord[] additionals, Throwable cause, EventLoop loop) {
checkNotNull(hostname, "hostname");
checkNotNull(cause, "cause");
checkNotNull(loop, "loop");
final DefaultDnsCacheEntry e = new DefaultDnsCacheEntry(hostname, cause);
if (negativeTtl == 0 || !emptyAdditionals(additionals)) {
return;
return e;
}
final List<DnsCacheEntry> entries = cachedEntries(hostname);
final DnsCacheEntry e = new DnsCacheEntry(hostname, cause);
final List<DefaultDnsCacheEntry> entries = cachedEntries(hostname);
synchronized (entries) {
final int numEntries = entries.size();
@ -193,17 +198,18 @@ public class DefaultDnsCache implements DnsCache {
}
scheduleCacheExpiration(entries, e, negativeTtl, loop);
return e;
}
private static void cancelExpiration(List<DnsCacheEntry> entries) {
private static void cancelExpiration(List<DefaultDnsCacheEntry> entries) {
final int numEntries = entries.size();
for (int i = 0; i < numEntries; i++) {
entries.get(i).cancelExpiration();
}
}
private void scheduleCacheExpiration(final List<DnsCacheEntry> entries,
final DnsCacheEntry e,
private void scheduleCacheExpiration(final List<DefaultDnsCacheEntry> entries,
final DefaultDnsCacheEntry e,
int ttl,
EventLoop loop) {
e.scheduleExpiration(loop, new Runnable() {
@ -229,4 +235,58 @@ public class DefaultDnsCache implements DnsCache {
.append(resolveCache.size()).append(")")
.toString();
}
private static final class DefaultDnsCacheEntry implements DnsCacheEntry {
private final String hostname;
private final InetAddress address;
private final Throwable cause;
private volatile ScheduledFuture<?> expirationFuture;
DefaultDnsCacheEntry(String hostname, InetAddress address) {
this.hostname = checkNotNull(hostname, "hostname");
this.address = checkNotNull(address, "address");
cause = null;
}
DefaultDnsCacheEntry(String hostname, Throwable cause) {
this.hostname = checkNotNull(hostname, "hostname");
this.cause = checkNotNull(cause, "cause");
address = null;
}
@Override
public InetAddress address() {
return address;
}
@Override
public Throwable cause() {
return cause;
}
String hostname() {
return hostname;
}
void scheduleExpiration(EventLoop loop, Runnable task, long delay, TimeUnit unit) {
assert expirationFuture == null : "expiration task scheduled already";
expirationFuture = loop.schedule(task, delay, unit);
}
void cancelExpiration() {
ScheduledFuture<?> expirationFuture = this.expirationFuture;
if (expirationFuture != null) {
expirationFuture.cancel(false);
}
}
@Override
public String toString() {
if (cause != null) {
return hostname + '/' + cause;
} else {
return address.toString();
}
}
}
}

View File

@ -49,17 +49,19 @@ public interface DnsCache {
* @param additionals the additional records
* @return the cached entries
*/
List<DnsCacheEntry> get(String hostname, DnsRecord[] additionals);
List<? extends DnsCacheEntry> get(String hostname, DnsRecord[] additionals);
/**
* Cache a resolved address for a given hostname.
* Create a new {@link DnsCacheEntry} and cache a resolved address for a given hostname.
* @param hostname the hostname
* @param additionals the additional records
* @param address the resolved address
* @param originalTtl the TLL as returned by the DNS server
* @param loop the {@link EventLoop} used to register the TTL timeout
* @return The {@link DnsCacheEntry} corresponding to this cache entry.
*/
void cache(String hostname, DnsRecord[] additionals, InetAddress address, long originalTtl, EventLoop loop);
DnsCacheEntry cache(String hostname, DnsRecord[] additionals, InetAddress address, long originalTtl,
EventLoop loop);
/**
* Cache the resolution failure for a given hostname.
@ -67,6 +69,8 @@ public interface DnsCache {
* @param additionals the additional records
* @param cause the resolution failure
* @param loop the {@link EventLoop} used to register the TTL timeout
* @return The {@link DnsCacheEntry} corresponding to this cache entry, or {@code null} if this cache doesn't
* support caching failed responses.
*/
void cache(String hostname, DnsRecord[] additionals, Throwable cause, EventLoop loop);
DnsCacheEntry cache(String hostname, DnsRecord[] additionals, Throwable cause, EventLoop loop);
}

View File

@ -1,5 +1,5 @@
/*
* Copyright 2015 The Netty Project
* Copyright 2017 The Netty Project
*
* The Netty Project licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
@ -13,71 +13,28 @@
* License for the specific language governing permissions and limitations
* under the License.
*/
package io.netty.resolver.dns;
import static io.netty.util.internal.ObjectUtil.checkNotNull;
import io.netty.channel.EventLoop;
import io.netty.util.concurrent.ScheduledFuture;
import io.netty.util.internal.UnstableApi;
import java.net.InetAddress;
import java.util.concurrent.TimeUnit;
/**
* Entry in {@link DnsCache}.
* Represents the results from a previous DNS query which can be cached.
*/
@UnstableApi
public final class DnsCacheEntry {
public interface DnsCacheEntry {
/**
* Get the resolved address.
* <p>
* This may be null if the resolution failed, and in that case {@link #cause()} will describe the failure.
* @return the resolved address.
*/
InetAddress address();
private final String hostname;
private final InetAddress address;
private final Throwable cause;
private volatile ScheduledFuture<?> expirationFuture;
public DnsCacheEntry(String hostname, InetAddress address) {
this.hostname = checkNotNull(hostname, "hostname");
this.address = checkNotNull(address, "address");
cause = null;
}
public DnsCacheEntry(String hostname, Throwable cause) {
this.hostname = checkNotNull(hostname, "hostname");
this.cause = checkNotNull(cause, "cause");
address = null;
}
public String hostname() {
return hostname;
}
public InetAddress address() {
return address;
}
public Throwable cause() {
return cause;
}
void scheduleExpiration(EventLoop loop, Runnable task, long delay, TimeUnit unit) {
assert expirationFuture == null: "expiration task scheduled already";
expirationFuture = loop.schedule(task, delay, unit);
}
void cancelExpiration() {
ScheduledFuture<?> expirationFuture = this.expirationFuture;
if (expirationFuture != null) {
expirationFuture.cancel(false);
}
}
@Override
public String toString() {
if (cause != null) {
return hostname + '/' + cause;
} else {
return address.toString();
}
}
/**
* If the DNS query failed this will provide the rational.
* @return the rational for why the DNS query failed, or {@code null} if the query hasn't failed.
*/
Throwable cause();
}

View File

@ -599,7 +599,7 @@ public class DnsNameResolver extends InetNameResolver {
DnsRecord[] additionals,
Promise<InetAddress> promise,
DnsCache resolveCache) {
final List<DnsCacheEntry> cachedEntries = resolveCache.get(hostname, additionals);
final List<? extends DnsCacheEntry> cachedEntries = resolveCache.get(hostname, additionals);
if (cachedEntries == null || cachedEntries.isEmpty()) {
return false;
}
@ -729,7 +729,7 @@ public class DnsNameResolver extends InetNameResolver {
DnsRecord[] additionals,
Promise<List<InetAddress>> promise,
DnsCache resolveCache) {
final List<DnsCacheEntry> cachedEntries = resolveCache.get(hostname, additionals);
final List<? extends DnsCacheEntry> cachedEntries = resolveCache.get(hostname, additionals);
if (cachedEntries == null || cachedEntries.isEmpty()) {
return false;
}

View File

@ -236,7 +236,7 @@ abstract class DnsNameResolverContext<T> {
}
idx = idx2;
List<DnsCacheEntry> entries = parent.authoritativeDnsServerCache().get(hostname, additionals);
List<? extends DnsCacheEntry> entries = parent.authoritativeDnsServerCache().get(hostname, additionals);
if (entries != null && !entries.isEmpty()) {
return DnsServerAddresses.sequential(new DnsCacheIterable(entries)).stream();
}
@ -244,16 +244,16 @@ abstract class DnsNameResolverContext<T> {
}
private final class DnsCacheIterable implements Iterable<InetSocketAddress> {
private final List<DnsCacheEntry> entries;
private final List<? extends DnsCacheEntry> entries;
DnsCacheIterable(List<DnsCacheEntry> entries) {
DnsCacheIterable(List<? extends DnsCacheEntry> entries) {
this.entries = entries;
}
@Override
public Iterator<InetSocketAddress> iterator() {
return new Iterator<InetSocketAddress>() {
Iterator<DnsCacheEntry> entryIterator = entries.iterator();
Iterator<? extends DnsCacheEntry> entryIterator = entries.iterator();
@Override
public boolean hasNext() {
@ -484,9 +484,8 @@ abstract class DnsNameResolverContext<T> {
resolvedEntries = new ArrayList<DnsCacheEntry>(8);
}
final DnsCacheEntry e = new DnsCacheEntry(hostname, resolved);
resolveCache.cache(hostname, additionals, resolved, r.timeToLive(), parent.ch.eventLoop());
resolvedEntries.add(e);
resolvedEntries.add(
resolveCache.cache(hostname, additionals, resolved, r.timeToLive(), parent.ch.eventLoop()));
found = true;
// Note that we do not break from the loop here, so we decode/cache all A/AAAA records.

View File

@ -47,21 +47,46 @@ public final class NoopDnsCache implements DnsCache {
}
@Override
public List<DnsCacheEntry> get(String hostname, DnsRecord[] additionals) {
public List<? extends DnsCacheEntry> get(String hostname, DnsRecord[] additionals) {
return Collections.emptyList();
}
@Override
public void cache(String hostname, DnsRecord[] additional,
InetAddress address, long originalTtl, EventLoop loop) {
public DnsCacheEntry cache(String hostname, DnsRecord[] additional,
InetAddress address, long originalTtl, EventLoop loop) {
return new NoopDnsCacheEntry(address);
}
@Override
public void cache(String hostname, DnsRecord[] additional, Throwable cause, EventLoop loop) {
public DnsCacheEntry cache(String hostname, DnsRecord[] additional, Throwable cause, EventLoop loop) {
return null;
}
@Override
public String toString() {
return NoopDnsCache.class.getSimpleName();
}
private static final class NoopDnsCacheEntry implements DnsCacheEntry {
private final InetAddress address;
NoopDnsCacheEntry(InetAddress address) {
this.address = address;
}
@Override
public InetAddress address() {
return address;
}
@Override
public Throwable cause() {
return null;
}
@Override
public String toString() {
return address.toString();
}
}
}

View File

@ -971,7 +971,7 @@ public class DnsNameResolverTest {
if (cache) {
assertNull(nsCache.cache.get("io.", null));
assertNull(nsCache.cache.get("netty.io.", null));
List<DnsCacheEntry> entries = nsCache.cache.get("record.netty.io.", null);
List<? extends DnsCacheEntry> entries = nsCache.cache.get("record.netty.io.", null);
assertEquals(1, entries.size());
assertNull(nsCache.cache.get(hostname, null));
@ -1159,7 +1159,8 @@ public class DnsNameResolverTest {
private static final class TestDnsCache implements DnsCache {
private final DnsCache cache;
final Map<String, List<DnsCacheEntry>> cacheHits = new HashMap<String, List<DnsCacheEntry>>();
final Map<String, List<? extends DnsCacheEntry>> cacheHits = new HashMap<String,
List<? extends DnsCacheEntry>>();
TestDnsCache(DnsCache cache) {
this.cache = cache;
@ -1176,22 +1177,22 @@ public class DnsNameResolverTest {
}
@Override
public List<DnsCacheEntry> get(String hostname, DnsRecord[] additionals) {
List<DnsCacheEntry> cacheEntries = cache.get(hostname, additionals);
public List<? extends DnsCacheEntry> get(String hostname, DnsRecord[] additionals) {
List<? extends DnsCacheEntry> cacheEntries = cache.get(hostname, additionals);
cacheHits.put(hostname, cacheEntries);
return cacheEntries;
}
@Override
public void cache(
public DnsCacheEntry cache(
String hostname, DnsRecord[] additionals, InetAddress address, long originalTtl, EventLoop loop) {
cache.cache(hostname, additionals, address, originalTtl, loop);
return cache.cache(hostname, additionals, address, originalTtl, loop);
}
@Override
public void cache(
public DnsCacheEntry cache(
String hostname, DnsRecord[] additionals, Throwable cause, EventLoop loop) {
cache.cache(hostname, additionals, cause, loop);
return cache.cache(hostname, additionals, cause, loop);
}
}