more cname-skip unit-tests and simplification of skip-finder function

diff --git a/src/dns/dnscache.cpp b/src/dns/dnscache.cpp
index 4b8df17..4ad62c4 100644 (file)
--- a/src/dns/dnscache.cpp
+++ b/src/dns/dnscache.cpp
@@ -362,16 +362,12 @@ HostAddressVec DnsCache::get_ips_recursive(const std::string &hostname,
 }
 
 /**
- * from a list of CNAMEs find the first one that is out of date
+ * from a list of CNAMEs find the first one that is out of date or empty
+ *
+ * returns the hostname that is out of date or empty if all CNAMEs are
+ *   up-to-date
  *
  * required in ResolverBase::get_skipper
- * 
- * Assume you have the following situation in cache with TTLs below:
- * hostname --> cname1 --> cname2 --> ... --> cnameN [--> IP]
- *               100         0                                --> return cname2
- *               100        100        100     100            --> return cnameN
- *    ( with N < Config::MaxRetrievalRecursions )
- * hostname --> IP (no cnames involved) --> return hostname
  */
 std::string DnsCache::get_first_outdated_cname(const std::string &hostname,
                                                const uint32_t ttl_thresh)
@@ -385,23 +381,23 @@ std::string DnsCache::get_first_outdated_cname(const std::string &hostname,
         {
             GlobalLogger.warning() << "DnsCache: reached recursion limit of "
                 << n_recursions << " in search of outdated CNAMEs!";
-            break;
-        }
+            return first_outdated;   // not really out of date but currently
+        }                            // our best guess
 
         cname = get_cname(first_outdated);
         if (cname.Host.empty())
-            // reached end of cname list
-            break;
+            // reached end of cname list --> everything was up-to-date
+            return "";
         else if (cname.Ttl.get_updated_value() > ttl_thresh)
             // cname is up to date --> continue looking
             first_outdated = cname.Host;
         else
-        {   // cname is out of date --> return its target
-            first_outdated = cname.Host;
-            break;
-        }
+            // cname is out of date --> return its target
+            return cname.Host;
     }
-    return first_outdated;
+    // reach this point only if cname chain does not end with an IP
+    // --> all are up-to-date
+    return "";
 }
 
 // (created using vim -- the world's best text editor)

diff --git a/test/test_dns.cpp b/test/test_dns.cpp
index 3c60bf3..f215e39 100644 (file)
--- a/test/test_dns.cpp
+++ b/test/test_dns.cpp
@@ -121,20 +121,15 @@ struct GlobalFixture
             Cache->update("host2_3.test", ips);
         }
 
-        {   // cname.test --> host1.test
-            Cname cname("host1.test", 35);
-            Cache->update("cname.test", cname);
-        }
+        // cname.test --> host1.test
+        Cache->update("cname.test", Cname("host1.test", 35) );
 
-        {   // cname2.test --> cname.test --> host1.test
-            Cname cname("cname.test", 33);
-            Cache->update("cname2.test", cname);
-        }
+        // cname2.test --> cname.test --> host1.test
+        Cache->update("cname2.test", Cname("cname.test", 33) );
+
+        // cname3.test --> cname2.test --> cname.test --> host1.test
+        Cache->update("cname3.test", Cname("cname2.test", 37) );
 
-        {   // cname3.test --> cname2.test --> cname.test --> host1.test
-            Cname cname("cname2.test", 37);
-            Cache->update("cname3.test", cname);
-        }
         BOOST_TEST_MESSAGE( "Done filling cache." );
     }
 
@@ -245,32 +240,64 @@ BOOST_AUTO_TEST_CASE( cache_retrieve_recursive2 )
     BOOST_CHECK_EQUAL( ip.get_ttl().get_value(), 33 );
 }
 
-BOOST_AUTO_TEST_CASE( cache_skip_test1 )
+void cname_skip_test(const uint32_t ttl1, const uint32_t ttl2,
+                     const uint32_t ttl3, const uint32_t ttl4,
+                     const DnsCacheItem &cache,
+                     const std::string &correct_host_after_skip)
 {
-    // build a cname chain where first one is out of date
-    {   // skip_chain_first -(120)-> skip_chain_second -(0)-> skip_chain_third
-        //  -(120)-> skip_chain_fourth -(60)-> IPs
-        Cname first("skip_chain_second.test", 120);
-        Cache->update("skip_chain_first.test", first);
-        Cname second("skip_chain_third.test", 0);
-        Cache->update("skip_chain_second.test", second);
-        Cname third("skip_chain_fourth.test", 120);
-        Cache->update("skip_chain_third.test", third);
+    {   // create cname chain:
+        // skip_chain_first -(ttl1)-> skip_chain_second -(ttl2)->
+        //   skip_chain_third -(ttl3)-> skip_chain_fourth -(ttl4)-> IPs
+        cache->update( "skip_chain_first.test",
+                 Cname("skip_chain_second.test", ttl1) );
+        cache->update( "skip_chain_second.test",
+                 Cname("skip_chain_third.test", ttl2) );
+        cache->update( "skip_chain_third.test",
+                 Cname("skip_chain_fourth.test", ttl3) );
         HostAddressVec ips;
-        ips.push_back( HostAddress( address::from_string("192.168.42.4"), 60) );
-        Cache->update("skip_chain_fourth.test", ips);
+        ips.push_back( HostAddress( address::from_string("192.168.42.4"), ttl4) );
+        cache->update("skip_chain_fourth.test", ips);
     }
 
-    // normal recursive call should give nothing since all are outdated
+    // normal recursive call should give nothing since one cname is outdated
     bool check_up_to_date = true;
-    HostAddressVec ips = Cache->get_ips_recursive("skip_chain_first.test",
+    HostAddressVec ips = cache->get_ips_recursive("skip_chain_first.test",
                                                   check_up_to_date);
-    BOOST_CHECK( ips.empty() );
+    bool one_is_out_of_date = (ttl1 < 5) || (ttl2 < 5)
+                           || (ttl3 < 5) || (ttl4 < 5);
+    BOOST_CHECK_EQUAL( ips.empty(), one_is_out_of_date );
 
-    // now try to skip
-    std::string first_outdated = Cache->get_first_outdated_cname(
+    // now find host to resolve after the outdated one
+    std::string first_outdated = cache->get_first_outdated_cname(
                                                     "skip_chain_first.test", 5);
-    BOOST_CHECK_EQUAL( first_outdated, "skip_chain_third.test" );
+    BOOST_CHECK_EQUAL( first_outdated, correct_host_after_skip );
+}
+
+BOOST_AUTO_TEST_CASE( cache_skip_tests )
+{
+    // build a cname chain where first one is out of date
+    cname_skip_test(0, 120, 120, 60, Cache, "skip_chain_second.test");
+
+    // build a cname chain where second one is out of date
+    cname_skip_test(120, 0, 120, 60, Cache, "skip_chain_third.test");
+
+    // build a cname chain where third one is out of date
+    cname_skip_test(120, 120, 0, 120, Cache, "skip_chain_fourth.test");
+
+    // build a cname chain where just IPs are out of date
+    cname_skip_test(120, 120, 120, 0, Cache, "");
+
+    // build a cname chain where all are out of date
+    cname_skip_test(0, 0, 0, 0, Cache, "skip_chain_second.test");
+
+    // build a cname chain where all is up to date
+    cname_skip_test(120, 120, 120, 120, Cache, "");
+
+    // test case with very short cname list that is up-to-date
+    BOOST_CHECK_EQUAL( Cache->get_first_outdated_cname("cname.test", 5), "" );
+
+    // test case where there is no cname at all
+    BOOST_CHECK_EQUAL( Cache->get_first_outdated_cname("host1.test", 5), "" );
 }
 
 
@@ -283,9 +310,6 @@ BOOST_AUTO_TEST_SUITE_END()    // of TestDnsCache
 
 BOOST_FIXTURE_TEST_SUITE( TestDnsResolver, TestFixture )
 
-void resolve_callback(IoServiceItem io_serv, ResolverItem resolver,
-                      const bool was_success, const int cname_count);
-
 BOOST_AUTO_TEST_CASE( create_resolver_v4 )
 {
     // create resolver