Merge pull request lightbody#667 from jekh/deprecate-dnsjava-resolver

Deprecate dnsjava

Author: jekh

browsermob-core/src/main/java/net/lightbody/bmp/client/ClientUtil.java

@@ -42,6 +42,7 @@ public static AdvancedHostResolver createNativeResolver() {
      * calling {@link net.lightbody.bmp.BrowserMobProxy#setHostNameResolver(net.lightbody.bmp.proxy.dns.AdvancedHostResolver)}.
      *
      * @return a new DnsJavaResolver
+     * @deprecated The dnsjava resolver has been deprecated in favor of the standard JVM resolver and will be removed in BMP >2.1.
      */
     public static AdvancedHostResolver createDnsJavaResolver() {
         return new DnsJavaResolver();
@@ -53,6 +54,7 @@ public static AdvancedHostResolver createDnsJavaResolver() {
      * Can be used when calling {@link net.lightbody.bmp.BrowserMobProxy#setHostNameResolver(net.lightbody.bmp.proxy.dns.AdvancedHostResolver)}.
      *
      * @return a new ChainedHostResolver that resolves addresses first using a DnsJavaResolver, then using a NativeCacheManipulatingResolver
+     * @deprecated The dnsjava resolver has been deprecated in favor of the standard JVM resolver and will be removed in BMP >2.1.
      */
     public static AdvancedHostResolver createDnsJavaWithNativeFallbackResolver() {
         return new ChainedHostResolver(ImmutableList.of(new DnsJavaResolver(), new NativeCacheManipulatingResolver()));
@@ -74,7 +76,7 @@ public static org.openqa.selenium.Proxy createSeleniumProxy(BrowserMobProxy brow
      * Creates a Selenium Proxy object from the BrowserMobProxy instance, using the specified connectableAddress as the Selenium Proxy object's
      * proxy address. Determines the port using {@link net.lightbody.bmp.BrowserMobProxy#getPort()}. The BrowserMobProxy must be started.
      *
-     * @param browserMobProxy started BrowserMobProxy instance to read the port from
+     * @param browserMobProxy    started BrowserMobProxy instance to read the port from
      * @param connectableAddress the network address the Selenium Proxy will use to reach this BrowserMobProxy instance
      * @return a Selenium Proxy instance, configured to use the BrowserMobProxy instance as its proxy server
      * @throws java.lang.IllegalStateException if the proxy has not been started.
@@ -105,6 +107,7 @@ public static org.openqa.selenium.Proxy createSeleniumProxy(InetSocketAddress co
      * Attempts to retrieve a "connectable" address for this device that other devices on the network can use to connect to a local proxy.
      * This is a "reasonable guess" that is suitable in many (but not all) common scenarios.
      * TODO: define the algorithm used to discover a "connectable" local host
+     *
      * @return a "reasonable guess" at an address that can be used by other machines on the network to reach this host
      */
     public static InetAddress getConnectableAddress() {

browsermob-core/src/main/java/net/lightbody/bmp/proxy/dns/DnsJavaResolver.java

@@ -23,6 +23,8 @@
 /**
  * An {@link net.lightbody.bmp.proxy.dns.AdvancedHostResolver} that uses dnsjava to perform DNS lookups. This implementation provides full
  * cache manipulation capabilities.
+ *
+ * @deprecated The dnsjava resolver has been deprecated in favor of the standard JVM resolver and will be removed in BMP >2.1.
  */
 public class DnsJavaResolver extends AbstractHostNameRemapper implements AdvancedHostResolver {
     private static final Logger log = LoggerFactory.getLogger(DnsJavaResolver.class);

browsermob-core/src/test/java/net/lightbody/bmp/proxy/dns/AdvancedHostResolverCacheTest.java

@@ -12,6 +12,7 @@
 import java.net.InetAddress;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Random;
 import java.util.concurrent.TimeUnit;
 
@@ -29,7 +30,7 @@ public class AdvancedHostResolverCacheTest {
     public static Collection<Object[]> data() {
         return Arrays.asList(new Object[][]{
                 // skip DNS cache operations for NativeResolver
-                {DnsJavaResolver.class}, {NativeCacheManipulatingResolver.class}, {ChainedHostResolver.class}
+                {NativeCacheManipulatingResolver.class}, {ChainedHostResolver.class}
         });
     }
 
@@ -38,10 +39,7 @@ public static Collection<Object[]> data() {
     public AdvancedHostResolverCacheTest(Class<AdvancedHostResolver> resolverClass) throws IllegalAccessException, InstantiationException {
         // this is a hacky way to allow us to test the ChainedHostResolver, even though it doesn't have a no-arg constructor
         if (resolverClass.equals(ChainedHostResolver.class)) {
-            // don't use the NativecacheManipulatingResolver on Windows, since it is unsupported
-            this.resolver = new ChainedHostResolver(
-                    NewProxyServerTestUtil.isWindows() ? ImmutableList.of(new DnsJavaResolver())
-                                                : ImmutableList.of(new NativeCacheManipulatingResolver(), new DnsJavaResolver()));
+            this.resolver = new ChainedHostResolver(ImmutableList.of(new NativeCacheManipulatingResolver()));
         } else {
             this.resolver = resolverClass.newInstance();
         }
@@ -54,10 +52,11 @@ public void skipForTravisCi() {
     }
 
     @Before
-    public void skipForNativeDnsCacheOnWindows() {
-        // the NativecacheManipulatingResolver does not work on Windows because Java seems to use to the OS-level cache
+    public void skipOnWindows() {
+        // DNS cache-manipulating features are not available on Windows, because the NativeCacheManipulatingResolver does
+        // not work, since Java seems to use to the OS-level cache.
         assumeFalse("NativeCacheManipulatingResolver does not support cache manipulation on Windows",
-                NewProxyServerTestUtil.isWindows() && this.resolver instanceof NativeCacheManipulatingResolver);
+                NewProxyServerTestUtil.isWindows());
     }
 
     @Test

browsermob-core/src/test/java/net/lightbody/bmp/proxy/dns/AdvancedHostResolverTest.java

@@ -28,7 +28,7 @@ public class AdvancedHostResolverTest {
     @Parameterized.Parameters
     public static Collection<Object[]> data() {
         return Arrays.asList(new Object[][]{
-                {DnsJavaResolver.class}, {NativeResolver.class}, {NativeCacheManipulatingResolver.class}, {ChainedHostResolver.class}
+                {NativeResolver.class}, {NativeCacheManipulatingResolver.class}, {ChainedHostResolver.class}
         });
     }
 
@@ -37,7 +37,7 @@ public static Collection<Object[]> data() {
     public AdvancedHostResolverTest(Class<AdvancedHostResolver> resolverClass) throws IllegalAccessException, InstantiationException {
         // this is a hacky way to allow us to test the ChainedHostResolver, even though it doesn't have a no-arg constructor
         if (resolverClass.equals(ChainedHostResolver.class)) {
-            this.resolver = new ChainedHostResolver(ImmutableList.of(new DnsJavaResolver(), new NativeResolver(), new NativeCacheManipulatingResolver()));
+            this.resolver = new ChainedHostResolver(ImmutableList.of(new NativeResolver(), new NativeCacheManipulatingResolver()));
         } else {
             this.resolver = resolverClass.newInstance();
         }
@@ -96,18 +96,11 @@ public void testResolveIPv4AndIPv6Addresses() {
 
         // disabling this assert to prevent test failures on systems without ipv6 access, or when the DNS server does not return IPv6 addresses
         //assertTrue("Expected to find at least one IPv6 address for www.google.com", foundIPv6);
-        // update: since the dnsjava resolver now returns ipv6 addresses *only if there are no ipv4 addresses*, it will generally not return
-        // any ipv6 addresses for most hostnames
 
     }
 
     @Test
     public void testResolveLocalhost() {
-        // DnsJavaResolver cannot resolve localhost, since it does not look up entries in the hosts file
-        if (resolver.getClass() == DnsJavaResolver.class) {
-            return;
-        }
-
         Collection<InetAddress> addresses = resolver.resolve("localhost");
 
         assertNotNull("Collection of resolved addresses should never be null", addresses);