Browse Source

[master] Merge branch 'trac495'

Conflicts:
	src/lib/Makefile.am
	src/lib/asiolink/Makefile.am
	src/lib/asiolink/tcp_socket.h
	src/lib/asiolink/tests/Makefile.am
	src/lib/nsas/asiolink.h
	src/lib/resolve/recursive_query.cc
Jelte Jansen 14 years ago
parent
commit
76022a7e9f
34 changed files with 640 additions and 273 deletions
  1. 1 1
      ext/asio/asio/detail/epoll_reactor.hpp
  2. 1 0
      src/bin/resolver/Makefile.am
  3. 49 4
      src/bin/resolver/main.cc
  4. 24 4
      src/bin/resolver/resolver.cc
  5. 21 0
      src/bin/resolver/resolver.h
  6. 1 0
      src/bin/resolver/tests/Makefile.am
  7. 1 1
      src/lib/Makefile.am
  8. 2 6
      src/lib/asiolink/Makefile.am
  9. 0 1
      src/lib/asiolink/asiolink.h
  10. 2 2
      src/lib/asiolink/tcp_socket.h
  11. 0 2
      src/lib/asiolink/tests/Makefile.am
  12. 1 0
      src/lib/cache/tests/Makefile.am
  13. 2 2
      src/lib/nsas/address_entry.h
  14. 0 37
      src/lib/nsas/asiolink.h
  15. 3 1
      src/lib/nsas/nameserver_address.cc
  16. 13 0
      src/lib/nsas/nameserver_address_store.cc
  17. 7 0
      src/lib/nsas/nameserver_address_store.h
  18. 6 3
      src/lib/nsas/nameserver_entry.cc
  19. 1 0
      src/lib/nsas/tests/Makefile.am
  20. 2 2
      src/lib/nsas/tests/address_entry_unittest.cc
  21. 1 1
      src/lib/nsas/tests/nameserver_address_unittest.cc
  22. 2 2
      src/lib/nsas/tests/nameserver_entry_unittest.cc
  23. 16 16
      src/lib/nsas/tests/zone_entry_unittest.cc
  24. 12 0
      src/lib/nsas/zone_entry.cc
  25. 9 0
      src/lib/nsas/zone_entry.h
  26. 1 0
      src/lib/resolve/Makefile.am
  27. 324 160
      src/lib/asiolink/recursive_query.cc
  28. 8 6
      src/lib/asiolink/recursive_query.h
  29. 7 3
      src/lib/resolve/response_classifier.cc
  30. 1 0
      src/lib/resolve/response_classifier.h
  31. 7 0
      src/lib/resolve/tests/Makefile.am
  32. 80 15
      src/lib/asiolink/tests/recursive_query_unittest.cc
  33. 18 4
      src/lib/asiolink/tests/recursive_query_unittest_2.cc
  34. 17 0
      src/lib/resolve/tests/response_classifier_unittest.cc

+ 1 - 1
ext/asio/asio/detail/epoll_reactor.hpp

@@ -207,7 +207,7 @@ public:
   // Cancel all operations associated with the given descriptor. The
   // handlers associated with the descriptor will be invoked with the
   // operation_aborted error.
-  void cancel_ops(socket_type descriptor, per_descriptor_data& descriptor_data)
+  void cancel_ops(socket_type, per_descriptor_data& descriptor_data)
   {
     mutex::scoped_lock descriptor_lock(descriptor_data->mutex_);
 

+ 1 - 0
src/bin/resolver/Makefile.am

@@ -51,6 +51,7 @@ b10_resolver_LDADD += $(top_builddir)/src/lib/log/liblog.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/server_common/libserver_common.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/cache/libcache.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/nsas/libnsas.la
+b10_resolver_LDADD += $(top_builddir)/src/lib/resolve/libresolve.la
 b10_resolver_LDADD += $(top_builddir)/src/bin/auth/change_user.o
 b10_resolver_LDFLAGS = -pthread
 

+ 49 - 4
src/bin/resolver/main.cc

@@ -45,6 +45,9 @@
 #include <resolver/spec_config.h>
 #include <resolver/resolver.h>
 
+#include <cache/resolver_cache.h>
+#include <nsas/nameserver_address_store.h>
+
 #include <log/dummylog.h>
 
 using namespace std;
@@ -59,7 +62,7 @@ namespace {
 static const string PROGRAM = "Resolver";
 
 IOService io_service;
-static Resolver *resolver;
+static boost::shared_ptr<Resolver> resolver;
 
 ConstElementPtr
 my_config_handler(ConstElementPtr new_config) {
@@ -135,15 +138,58 @@ main(int argc, char* argv[]) {
             specfile = string(RESOLVER_SPECFILE_LOCATION);
         }
 
-        resolver = new Resolver();
+        resolver = boost::shared_ptr<Resolver>(new Resolver());
         dlog("Server created.");
 
         SimpleCallback* checkin = resolver->getCheckinProvider();
         DNSLookup* lookup = resolver->getDNSLookupProvider();
         DNSAnswer* answer = resolver->getDNSAnswerProvider();
 
+        isc::nsas::NameserverAddressStore nsas(resolver);
+        resolver->setNameserverAddressStore(nsas);
+
+        isc::cache::ResolverCache cache;
+        resolver->setCache(cache);
+        
+        // TODO priming query, remove root from direct
+        // Fake a priming query result here (TODO2 how to flag non-expiry?)
+        // propagation to runningquery. And check for forwarder mode?
+        isc::dns::QuestionPtr root_question(new isc::dns::Question(
+                                            isc::dns::Name("."),
+                                            isc::dns::RRClass::IN(),
+                                            isc::dns::RRType::NS()));
+        isc::dns::RRsetPtr root_ns_rrset(new isc::dns::RRset(isc::dns::Name("."), 
+                                         isc::dns::RRClass::IN(),
+                                         isc::dns::RRType::NS(),
+                                         isc::dns::RRTTL(8888)));
+        root_ns_rrset->addRdata(isc::dns::rdata::createRdata(isc::dns::RRType::NS(),
+                                                             isc::dns::RRClass::IN(),
+                                                             "l.root-servers.net."));
+        isc::dns::RRsetPtr root_a_rrset(new isc::dns::RRset(isc::dns::Name("l.root-servers.net"), 
+                                        isc::dns::RRClass::IN(),
+                                        isc::dns::RRType::A(),
+                                        isc::dns::RRTTL(8888)));
+        root_a_rrset->addRdata(isc::dns::rdata::createRdata(isc::dns::RRType::A(),
+                                                             isc::dns::RRClass::IN(),
+                                                             "199.7.83.42"));
+        isc::dns::RRsetPtr root_aaaa_rrset(new isc::dns::RRset(isc::dns::Name("l.root-servers.net"), 
+                                        isc::dns::RRClass::IN(),
+                                        isc::dns::RRType::AAAA(),
+                                        isc::dns::RRTTL(8888)));
+        root_aaaa_rrset->addRdata(isc::dns::rdata::createRdata(isc::dns::RRType::AAAA(),
+                                                             isc::dns::RRClass::IN(),
+                                                             "2001:500:3::42"));
+        isc::dns::MessagePtr priming_result(new isc::dns::Message(isc::dns::Message::RENDER));
+        priming_result->addQuestion(root_question);
+        priming_result->addRRset(isc::dns::Message::SECTION_ANSWER, root_ns_rrset);
+        priming_result->addRRset(isc::dns::Message::SECTION_ADDITIONAL, root_a_rrset);
+        priming_result->addRRset(isc::dns::Message::SECTION_ADDITIONAL, root_aaaa_rrset);
+        cache.update(*priming_result);
+        cache.update(root_ns_rrset);
+        cache.update(root_a_rrset);
+        cache.update(root_aaaa_rrset);
+        
         DNSService dns_service(io_service, checkin, lookup, answer);
-
         resolver->setDNSService(dns_service);
         dlog("IOService created.");
 
@@ -172,7 +218,6 @@ main(int argc, char* argv[]) {
 
     delete config_session;
     delete cc_session;
-    delete resolver;
 
     return (ret);
 }

+ 24 - 4
src/bin/resolver/resolver.cc

@@ -41,6 +41,8 @@
 #include <dns/messagerenderer.h>
 #include <server_common/portconfig.h>
 
+#include <resolve/recursive_query.h>
+
 #include <log/dummylog.h>
 
 #include <resolver/resolver.h>
@@ -74,10 +76,15 @@ public:
         queryShutdown();
     }
 
-    void querySetup(DNSService& dnss) {
+    void querySetup(DNSService& dnss,
+                    isc::nsas::NameserverAddressStore& nsas,
+                    isc::cache::ResolverCache& cache)
+    {
         assert(!rec_query_); // queryShutdown must be called first
         dlog("Query setup");
-        rec_query_ = new RecursiveQuery(dnss, upstream_,
+        rec_query_ = new RecursiveQuery(dnss, 
+                                        nsas, cache,
+                                        upstream_,
                                         upstream_root_,
                                         query_timeout_,
                                         client_timeout_,
@@ -129,7 +136,7 @@ public:
             }
         }
     }
-
+    
     void resolve(const isc::dns::QuestionPtr& question,
         const isc::resolve::ResolverInterface::CallbackPtr& callback);
 
@@ -342,6 +349,19 @@ Resolver::setDNSService(asiolink::DNSService& dnss) {
 }
 
 void
+Resolver::setNameserverAddressStore(isc::nsas::NameserverAddressStore& nsas)
+{
+    nsas_ = &nsas;
+}
+
+void
+Resolver::setCache(isc::cache::ResolverCache& cache)
+{
+    cache_ = &cache;
+}
+
+
+void
 Resolver::setConfigSession(ModuleCCSession* config_session) {
     impl_->config_session_ = config_session;
 }
@@ -544,7 +564,7 @@ Resolver::updateConfig(ConstElementPtr config) {
 
         if (need_query_restart) {
             impl_->queryShutdown();
-            impl_->querySetup(*dnss_);
+            impl_->querySetup(*dnss_, *nsas_, *cache_);
         }
         return (isc::config::createAnswer());
     } catch (const isc::Exception& error) {

+ 21 - 0
src/bin/resolver/resolver.h

@@ -24,6 +24,9 @@
 
 #include <asiolink/asiolink.h>
 
+#include <nsas/nameserver_address_store.h>
+#include <cache/resolver_cache.h>
+
 #include <resolve/resolver_interface.h>
 
 class ResolverImpl;
@@ -86,10 +89,26 @@ public:
 
     /// \brief Assign an ASIO IO Service queue to this Resolver object
     void setDNSService(asiolink::DNSService& dnss);
+    
+    /// \brief Assign a NameserverAddressStore to this Resolver object
+    void setNameserverAddressStore(isc::nsas::NameserverAddressStore &nsas);
+    
+    /// \brief Assign a cache to this Resolver object
+    void setCache(isc::cache::ResolverCache& cache);
 
     /// \brief Return this object's ASIO IO Service queue
     asiolink::DNSService& getDNSService() const { return (*dnss_); }
 
+    /// \brief Returns this object's NSAS
+    isc::nsas::NameserverAddressStore& getNameserverAddressStore() const {
+        return *nsas_;
+    };
+
+    /// \brief Returns this object's ResolverCache
+    isc::cache::ResolverCache& getResolverCache() const {
+        return *cache_;
+    };
+    
     /// \brief Return pointer to the DNS Lookup callback function
     asiolink::DNSLookup* getDNSLookupProvider() { return (dns_lookup_); }
 
@@ -208,6 +227,8 @@ private:
     asiolink::SimpleCallback* checkin_;
     asiolink::DNSLookup* dns_lookup_;
     asiolink::DNSAnswer* dns_answer_;
+    isc::nsas::NameserverAddressStore* nsas_;
+    isc::cache::ResolverCache* cache_;
 };
 
 #endif // __RESOLVER_H

+ 1 - 0
src/bin/resolver/tests/Makefile.am

@@ -40,6 +40,7 @@ run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la
 run_unittests_LDADD += $(top_builddir)/src/lib/server_common/libserver_common.la
 run_unittests_LDADD += $(top_builddir)/src/lib/cache/libcache.la
 run_unittests_LDADD += $(top_builddir)/src/lib/nsas/libnsas.la
+run_unittests_LDADD += $(top_builddir)/src/lib/resolve/libresolve.la
 
 # Note the ordering matters: -Wno-... must follow -Wextra (defined in
 # B10_CXXFLAGS

+ 1 - 1
src/lib/Makefile.am

@@ -1,2 +1,2 @@
 SUBDIRS = exceptions dns cc config datasrc python xfr bench log \
-          resolve nsas cache asiolink testutils server_common
+          asiolink nsas cache resolve testutils server_common

+ 2 - 6
src/lib/asiolink/Makefile.am

@@ -27,10 +27,9 @@ libasiolink_la_SOURCES += io_endpoint.cc io_endpoint.h
 libasiolink_la_SOURCES += io_error.h
 libasiolink_la_SOURCES += io_fetch.cc io_fetch.h
 libasiolink_la_SOURCES += io_message.h
-libasiolink_la_SOURCES += io_service.cc io_service.h
-libasiolink_la_SOURCES += io_socket.cc io_socket.h
 libasiolink_la_SOURCES += qid_gen.cc qid_gen.h
-libasiolink_la_SOURCES += recursive_query.cc recursive_query.h
+libasiolink_la_SOURCES += io_service.h io_service.cc
+libasiolink_la_SOURCES += io_socket.h io_socket.cc
 libasiolink_la_SOURCES += simple_callback.h
 libasiolink_la_SOURCES += tcp_endpoint.h
 libasiolink_la_SOURCES += tcp_server.cc tcp_server.h
@@ -44,9 +43,6 @@ EXTRA_DIST = asiodef.msg
 # Note: the ordering matters: -Wno-... must follow -Wextra (defined in
 # B10_CXXFLAGS)
 libasiolink_la_CXXFLAGS = $(AM_CXXFLAGS)
-if USE_GXX
-libasiolink_la_CXXFLAGS += -Wno-unused-parameter
-endif
 if USE_CLANGPP
 # Same for clang++, but we need to turn off -Werror completely.
 libasiolink_la_CXXFLAGS += -Wno-error

+ 0 - 1
src/lib/asiolink/asiolink.h

@@ -25,7 +25,6 @@
 #include <asiolink/dns_lookup.h>
 #include <asiolink/dns_answer.h>
 #include <asiolink/simple_callback.h>
-#include <asiolink/recursive_query.h>
 #include <asiolink/interval_timer.h>
 
 #include <asiolink/io_address.h>

+ 2 - 2
src/lib/asiolink/tcp_socket.h

@@ -255,8 +255,8 @@ TCPSocket<C>::open(const IOEndpoint* endpoint, C& callback) {
 // an exception if this is the case.
 
 template <typename C> void
-TCPSocket<C>::asyncSend(const void* data, size_t length, const IOEndpoint*,
-                        C& callback)
+TCPSocket<C>::asyncSend(const void* data, size_t length,
+    const IOEndpoint*, C& callback)
 {
     if (isopen_) {
 

+ 0 - 2
src/lib/asiolink/tests/Makefile.am

@@ -25,8 +25,6 @@ run_unittests_SOURCES += io_fetch_unittest.cc
 run_unittests_SOURCES += io_socket_unittest.cc
 run_unittests_SOURCES += io_service_unittest.cc
 run_unittests_SOURCES += interval_timer_unittest.cc
-run_unittests_SOURCES += recursive_query_unittest.cc
-run_unittests_SOURCES += recursive_query_unittest_2.cc
 run_unittests_SOURCES += tcp_endpoint_unittest.cc
 run_unittests_SOURCES += tcp_socket_unittest.cc
 run_unittests_SOURCES += udp_endpoint_unittest.cc

+ 1 - 0
src/lib/cache/tests/Makefile.am

@@ -53,6 +53,7 @@ endif
 run_unittests_LDADD += $(top_builddir)/src/lib/cache/libcache.la
 run_unittests_LDADD += $(top_builddir)/src/lib/nsas/libnsas.la
 run_unittests_LDADD += $(top_builddir)/src/lib/dns/libdns++.la
+run_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
 run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
 endif
 

+ 2 - 2
src/lib/nsas/address_entry.h

@@ -21,7 +21,7 @@
 /// convenience methods for accessing and updating the information.
 
 #include <stdint.h>
-#include "asiolink.h"
+#include <asiolink/io_address.h>
 
 namespace isc {
 namespace nsas {
@@ -39,7 +39,7 @@ public:
     {}
 
     /// \return Address object
-    asiolink::IOAddress getAddress() const {
+    const asiolink::IOAddress& getAddress() const {
         return address_;
     }
 

+ 0 - 37
src/lib/nsas/asiolink.h

@@ -18,41 +18,4 @@
 #include <string>
 #include <sys/socket.h>
 
-namespace asiolink {
-
-/// \brief IO Address Dummy Class
-///
-/// As part of ther resolver, Evan has written the asiolink.h file, which
-/// encapsulates some of the boost::asio classes.  Until these are checked
-/// into trunk and merged with this branch, these dummy classes should fulfill
-/// their function.
-
-class IOAddress {
-public:
-    /// \param address_str String representing the address
-    IOAddress(const std::string& address_str) : address_(address_str)
-    {}
-
-    /// \param Just a virtual destructor
-    virtual ~ IOAddress() { }
-
-    /// \return Textual representation of the address
-    std::string toText() const
-    {return address_;}
-
-    /// \return Address family of the address
-    virtual short getFamily() const {
-        return ((address_.find(".") != std::string::npos) ? AF_INET : AF_INET6);
-    }
-
-    /// \return true if two addresses are equal
-    bool equal(const IOAddress& address) const
-    {return (toText() == address.toText());}
-
-private:
-    std::string     address_;       ///< Address represented
-};
-
-}   // namespace asiolink
-
 #endif // __ASIOLINK_H

+ 3 - 1
src/lib/nsas/nameserver_address.cc

@@ -23,7 +23,9 @@ namespace nsas {
 void
 NameserverAddress::updateRTT(uint32_t rtt) const {
     // We delegate it to the address entry inside the nameserver entry
-    ns_->updateAddressRTT(rtt, address_.getAddress(), family_);
+    if (ns_) {
+        ns_->updateAddressRTT(rtt, address_.getAddress(), family_);
+    }
 }
 
 } // namespace nsas

+ 13 - 0
src/lib/nsas/nameserver_address_store.cc

@@ -93,5 +93,18 @@ NameserverAddressStore::lookup(const string& zone, const RRClass& class_code,
     zone_obj.second->addCallback(callback, family);
 }
 
+void
+NameserverAddressStore::cancel(const string& zone,
+    const RRClass& class_code,
+    const boost::shared_ptr<AddressRequestCallback>& callback,
+    AddressFamily family)
+{
+    boost::shared_ptr<ZoneEntry> entry(zone_hash_->get(HashKey(zone,
+                                                               class_code)));
+    if (entry) {
+        entry->removeCallback(callback, family);
+    }
+}
+
 } // namespace nsas
 } // namespace isc

+ 7 - 0
src/lib/nsas/nameserver_address_store.h

@@ -87,6 +87,13 @@ public:
         boost::shared_ptr<AddressRequestCallback> callback, AddressFamily
         family = ANY_OK);
 
+    /// \brief cancel the given lookup action
+    ///
+    /// \param callback Callback object that would be called
+    void cancel(const std::string& zone, const dns::RRClass& class_code,
+                const boost::shared_ptr<AddressRequestCallback>& callback,
+                AddressFamily family = ANY_OK);
+
     /// \brief Protected Members
     ///
     /// These members should be private.  However, with so few public methods

+ 6 - 3
src/lib/nsas/nameserver_entry.cc

@@ -35,6 +35,8 @@
 #include <dns/question.h>
 #include <resolve/resolver_interface.h>
 
+#include <asiolink/io_address.h>
+
 #include "address_entry.h"
 #include "nameserver_address.h"
 #include "nameserver_entry.h"
@@ -140,7 +142,7 @@ NameserverEntry::setAddressRTT(const IOAddress& address, uint32_t rtt) {
     AddressFamily family(V4_ONLY);
     for (;;) {
         BOOST_FOREACH(AddressEntry& entry, addresses_[family]) {
-            if (entry.getAddress().equal(address)) {
+            if (entry.getAddress().equals(address)) {
                 entry.setRTT(rtt);
                 return;
             }
@@ -181,7 +183,7 @@ NameserverEntry::updateAddressRTT(uint32_t rtt,
 {
     Lock lock(mutex_);
     for (size_t i(0); i < addresses_[family].size(); ++ i) {
-        if (addresses_[family][i].getAddress().equal(address)) {
+        if (addresses_[family][i].getAddress().equals(address)) {
             updateAddressRTTAtIndex(rtt, i, family);
             return;
         }
@@ -226,8 +228,9 @@ class NameserverEntry::ResolverCallback :
                 response_message->getRcode() != isc::dns::Rcode::NOERROR() ||
                 response_message->getRRCount(isc::dns::Message::SECTION_ANSWER) == 0) {
                 failureInternal(lock);
+                return;
             }
-                
+            
             isc::dns::RRsetIterator rrsi =
                 response_message->beginSection(isc::dns::Message::SECTION_ANSWER);
             const isc::dns::RRsetPtr response = *rrsi;

+ 1 - 0
src/lib/nsas/tests/Makefile.am

@@ -54,6 +54,7 @@ endif
 
 run_unittests_LDADD += $(top_builddir)/src/lib/nsas/libnsas.la
 run_unittests_LDADD += $(top_builddir)/src/lib/dns/libdns++.la
+run_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
 run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
 endif
 

+ 2 - 2
src/lib/nsas/tests/address_entry_unittest.cc

@@ -24,12 +24,12 @@
 #include <stdint.h>
 
 
-#include "../asiolink.h"
+#include <asiolink/io_address.h>
 #include "../address_entry.h"
 
 static std::string V4A_TEXT("1.2.3.4");
 static std::string V4B_TEXT("5.6.7.8");
-static std::string V6A_TEXT("2001:dead:beef::0");
+static std::string V6A_TEXT("2001:dead:beef::");
 static std::string V6B_TEXT("1984:1985::1986:1987");
 
 using namespace asiolink;

+ 1 - 1
src/lib/nsas/tests/nameserver_address_unittest.cc

@@ -100,7 +100,7 @@ protected:
 
 // Test that the address is equal to the address in NameserverEntry
 TEST_F(NameserverAddressTest, Address) {
-    EXPECT_TRUE(ns_address_.getAddress().equal( ns_sample_.getAddressAtIndex(TEST_ADDRESS_INDEX)));
+    EXPECT_TRUE(ns_address_.getAddress().equals( ns_sample_.getAddressAtIndex(TEST_ADDRESS_INDEX)));
 
     boost::shared_ptr<NameserverEntry> empty_ne((NameserverEntry*)NULL);
     // It will throw an NullNameserverEntryPointer exception with the empty NameserverEntry shared pointer

+ 2 - 2
src/lib/nsas/tests/nameserver_entry_unittest.cc

@@ -153,7 +153,7 @@ TEST_F(NameserverEntryTest, SetRTT) {
     int matchcount = 0;
     for (NameserverEntry::AddressVectorIterator i = newvec.begin();
         i != newvec.end(); ++i) {
-        if (i->getAddress().equal(first_address)) {
+        if (i->getAddress().equals(first_address)) {
             ++matchcount;
             EXPECT_EQ(i->getAddressEntry().getRTT(), new_rtt);
         }
@@ -188,7 +188,7 @@ TEST_F(NameserverEntryTest, Unreachable) {
     int matchcount = 0;
     for (NameserverEntry::AddressVectorIterator i = newvec.begin();
         i != newvec.end(); ++i) {
-        if (i->getAddress().equal(first_address)) {
+        if (i->getAddress().equals(first_address)) {
             ++matchcount;
             EXPECT_TRUE(i->getAddressEntry().isUnreachable());
         }

+ 16 - 16
src/lib/nsas/tests/zone_entry_unittest.cc

@@ -165,9 +165,9 @@ protected:
         EXPECT_EQ(failure_count, callback_->unreachable_count_);
         EXPECT_EQ(success_count, callback_->successes_.size());
         for (size_t i = 0; i < callback_->successes_.size(); ++ i) {
-            EXPECT_TRUE(IOAddress("192.0.2.1").equal(
+            EXPECT_TRUE(IOAddress("192.0.2.1").equals(
                 callback_->successes_[i].getAddress()) ||
-                IOAddress("2001:db8::1").equal(
+                IOAddress("2001:db8::1").equals(
                 callback_->successes_[i].getAddress()));
         }
     }
@@ -234,7 +234,7 @@ TEST_F(ZoneEntryTest, ChangedNS) {
     EXPECT_NO_THROW(resolver_->answer(1, ns_name_, RRType::A(),
         rdata::in::A("192.0.2.1")));
     ASSERT_EQ(1, callback_->successes_.size());
-    EXPECT_TRUE(IOAddress("192.0.2.1").equal(
+    EXPECT_TRUE(IOAddress("192.0.2.1").equals(
         callback_->successes_[0].getAddress()));
     EXPECT_NO_THROW(resolver_->answer(2, ns_name_, RRType::AAAA(),
         rdata::in::AAAA("2001:db8::1")));
@@ -257,7 +257,7 @@ TEST_F(ZoneEntryTest, ChangedNS) {
     EXPECT_NO_THROW(resolver_->answer(4, different_name, RRType::A(),
         rdata::in::A("192.0.2.2")));
     ASSERT_EQ(2, callback_->successes_.size());
-    EXPECT_TRUE(IOAddress("192.0.2.2").equal(
+    EXPECT_TRUE(IOAddress("192.0.2.2").equals(
         callback_->successes_[1].getAddress()));
 
     // And now, switch back, as it timed out again
@@ -270,7 +270,7 @@ TEST_F(ZoneEntryTest, ChangedNS) {
     EXPECT_EQ(7, resolver_->requests.size());
     EXPECT_EQ(Fetchable::READY, zone->getState());
     ASSERT_EQ(3, callback_->successes_.size());
-    EXPECT_TRUE(IOAddress("192.0.2.1").equal(
+    EXPECT_TRUE(IOAddress("192.0.2.1").equals(
         callback_->successes_[0].getAddress()));
 }
 
@@ -306,9 +306,9 @@ TEST_F(ZoneEntryTest, CallbacksAnswered) {
          rdata::in::A("192.0.2.1")));
     // Two are answered (ANY and V4)
     ASSERT_EQ(2, callback_->successes_.size());
-    EXPECT_TRUE(IOAddress("192.0.2.1").equal(
+    EXPECT_TRUE(IOAddress("192.0.2.1").equals(
         callback_->successes_[0].getAddress()));
-    EXPECT_TRUE(IOAddress("192.0.2.1").equal(
+    EXPECT_TRUE(IOAddress("192.0.2.1").equals(
         callback_->successes_[1].getAddress()));
     // None are rejected
     EXPECT_EQ(0, callback_->unreachable_count_);
@@ -318,7 +318,7 @@ TEST_F(ZoneEntryTest, CallbacksAnswered) {
     // This should answer the third callback
     EXPECT_EQ(0, callback_->unreachable_count_);
     ASSERT_EQ(3, callback_->successes_.size());
-    EXPECT_TRUE(IOAddress("2001:db8::1").equal(
+    EXPECT_TRUE(IOAddress("2001:db8::1").equals(
         callback_->successes_[2].getAddress()));
     // It should think it is ready
     EXPECT_EQ(Fetchable::READY, zone->getState());
@@ -326,7 +326,7 @@ TEST_F(ZoneEntryTest, CallbacksAnswered) {
     zone->addCallback(callback_, V4_ONLY);
     EXPECT_EQ(3, resolver_->requests.size());
     ASSERT_EQ(4, callback_->successes_.size());
-    EXPECT_TRUE(IOAddress("192.0.2.1").equal(
+    EXPECT_TRUE(IOAddress("192.0.2.1").equals(
         callback_->successes_[3].getAddress()));
     EXPECT_EQ(0, callback_->unreachable_count_);
 }
@@ -366,9 +366,9 @@ TEST_F(ZoneEntryTest, CallbacksAOnly) {
     EXPECT_NO_THROW(resolver_->answer(1, ns_name_, RRType::A(),
         rdata::in::A("192.0.2.1")));
     ASSERT_EQ(2, callback_->successes_.size());
-    EXPECT_TRUE(IOAddress("192.0.2.1").equal(
+    EXPECT_TRUE(IOAddress("192.0.2.1").equals(
         callback_->successes_[0].getAddress()));
-    EXPECT_TRUE(IOAddress("192.0.2.1").equal(
+    EXPECT_TRUE(IOAddress("192.0.2.1").equals(
         callback_->successes_[1].getAddress()));
     EXPECT_EQ(1, callback_->unreachable_count_);
     // Everything arriwed, so we are ready
@@ -377,7 +377,7 @@ TEST_F(ZoneEntryTest, CallbacksAOnly) {
     zone->addCallback(callback_, V4_ONLY);
     EXPECT_EQ(3, resolver_->requests.size());
     ASSERT_EQ(3, callback_->successes_.size());
-    EXPECT_TRUE(IOAddress("192.0.2.1").equal(
+    EXPECT_TRUE(IOAddress("192.0.2.1").equals(
         callback_->successes_[2].getAddress()));
     EXPECT_EQ(1, callback_->unreachable_count_);
 
@@ -439,9 +439,9 @@ TEST_F(ZoneEntryTest, CallbackTwoNS) {
     // The other callbacks should be answered now
     EXPECT_EQ(2, callback_->unreachable_count_);
     ASSERT_EQ(2, callback_->successes_.size());
-    EXPECT_TRUE(IOAddress("2001:db8::1").equal(
+    EXPECT_TRUE(IOAddress("2001:db8::1").equals(
         callback_->successes_[0].getAddress()));
-    EXPECT_TRUE(IOAddress("2001:db8::1").equal(
+    EXPECT_TRUE(IOAddress("2001:db8::1").equals(
         callback_->successes_[1].getAddress()));
 }
 
@@ -534,7 +534,7 @@ TEST_F(ZoneEntryTest, AddressTimeout) {
          rdata::in::A("192.0.2.1"), 0));
     // It answers, not rejects
     ASSERT_EQ(1, callback_->successes_.size());
-    EXPECT_TRUE(IOAddress("192.0.2.1").equal(
+    EXPECT_TRUE(IOAddress("192.0.2.1").equals(
         callback_->successes_[0].getAddress()));
     EXPECT_EQ(0, callback_->unreachable_count_);
     // As well with IPv6
@@ -551,7 +551,7 @@ TEST_F(ZoneEntryTest, AddressTimeout) {
          rdata::in::A("192.0.2.1"), 0));
     EXPECT_EQ(0, callback_->unreachable_count_);
     ASSERT_EQ(2, callback_->successes_.size());
-    EXPECT_TRUE(IOAddress("192.0.2.1").equal(
+    EXPECT_TRUE(IOAddress("192.0.2.1").equals(
         callback_->successes_[1].getAddress()));
 }
 

+ 12 - 0
src/lib/nsas/zone_entry.cc

@@ -261,6 +261,18 @@ ZoneEntry::addCallback(CallbackPtr callback, AddressFamily family) {
     }
 }
 
+void
+ZoneEntry::removeCallback(const CallbackPtr& callback, AddressFamily family) {
+    Lock lock(mutex_);
+    std::vector<boost::shared_ptr<AddressRequestCallback> >::iterator i = 
+        callbacks_[family].begin();
+    for (; i != callbacks_[family].end(); ++i) {
+        if (*i == callback) {
+            callbacks_[family].erase(i);
+        }
+    }
+}
+
 namespace {
 
 // This just moves items from one container to another

+ 9 - 0
src/lib/nsas/zone_entry.h

@@ -101,6 +101,15 @@ public:
     void addCallback(boost::shared_ptr<AddressRequestCallback>
         callback, AddressFamily family);
 
+    /**
+     * \short Remove a callback from the list
+     *
+     * \param callback The callback itself.
+     * \param family Which address family is acceptable as an answer?
+     */
+    void removeCallback(const boost::shared_ptr<AddressRequestCallback>&
+                        callback, AddressFamily family);
+
     /// \short Protected members, so they can be accessed by tests.
     //@{
 protected:

+ 1 - 0
src/lib/resolve/Makefile.am

@@ -14,5 +14,6 @@ libresolve_la_SOURCES = resolve.h resolve.cc
 libresolve_la_SOURCES += resolver_interface.h
 libresolve_la_SOURCES += resolver_callback.h resolver_callback.cc
 libresolve_la_SOURCES += response_classifier.cc response_classifier.h
+libresolve_la_SOURCES += recursive_query.cc recursive_query.h
 libresolve_la_LIBADD = $(top_builddir)/src/lib/dns/libdns++.la
 libresolve_la_LIBADD += $(top_builddir)/src/lib/exceptions/libexceptions.la

+ 324 - 160
src/lib/asiolink/recursive_query.cc

@@ -30,12 +30,14 @@
 
 #include <resolve/resolve.h>
 #include <cache/resolver_cache.h>
+#include <nsas/address_request_callback.h>
+#include <nsas/nameserver_address.h>
 
 #include <asio.hpp>
 #include <asiolink/dns_service.h>
 #include <asiolink/io_fetch.h>
 #include <asiolink/io_service.h>
-#include <asiolink/recursive_query.h>
+#include <resolve/recursive_query.h>
 
 using isc::log::dlog;
 using namespace isc::dns;
@@ -49,16 +51,21 @@ typedef std::vector<std::pair<std::string, uint16_t> > AddressVector;
 // We can probably use a typedef, but need to move it to a central
 // location and use it consistently.
 RecursiveQuery::RecursiveQuery(DNSService& dns_service,
+    isc::nsas::NameserverAddressStore& nsas,
+    isc::cache::ResolverCache& cache,
     const std::vector<std::pair<std::string, uint16_t> >& upstream,
     const std::vector<std::pair<std::string, uint16_t> >& upstream_root,
     int query_timeout, int client_timeout, int lookup_timeout,
     unsigned retries) :
-    dns_service_(dns_service), upstream_(new AddressVector(upstream)),
+    dns_service_(dns_service),
+    nsas_(nsas), cache_(cache),
+    upstream_(new AddressVector(upstream)),
     upstream_root_(new AddressVector(upstream_root)),
     test_server_("", 0),
     query_timeout_(query_timeout), client_timeout_(client_timeout),
     lookup_timeout_(lookup_timeout), retries_(retries)
-{}
+{
+}
 
 // Set the test server - only used for unit testing.
 
@@ -83,6 +90,31 @@ typedef std::pair<std::string, uint16_t> addr_t;
  * Used by RecursiveQuery::sendQuery.
  */
 class RunningQuery : public IOFetch::Callback {
+
+class ResolverNSASCallback : public isc::nsas::AddressRequestCallback {
+public:
+    ResolverNSASCallback(RunningQuery* rq) : rq_(rq) {}
+    
+    void success(const isc::nsas::NameserverAddress& address) {
+        dlog("Found a nameserver, sending query to " + address.getAddress().toText());
+        rq_->nsasCallbackCalled();
+        rq_->sendTo(address);
+    }
+    
+    void unreachable() {
+        dlog("Nameservers unreachable");
+        // Drop query or send servfail?
+        rq_->nsasCallbackCalled();
+        rq_->makeSERVFAIL();
+        rq_->callCallback(true);
+        rq_->stop();
+    }
+
+private:
+    RunningQuery* rq_;
+};
+
+
 private:
     // The io service to handle async calls
     IOService& io_;
@@ -97,18 +129,15 @@ private:
     // we should differentiate between forwarding and resolving
     boost::shared_ptr<AddressVector> upstream_;
 
-    // root servers...just copied over to the zone_servers_
-    boost::shared_ptr<AddressVector> upstream_root_;
-
     // Test server - only used for testing.  This takes precedence over all
     // other servers if the port is non-zero.
     std::pair<std::string, uint16_t> test_server_;
 
-    // Buffer to store the result.
+    // Buffer to store the intermediate results.
     OutputBufferPtr buffer_;
 
-    // Server to notify when we succeed or fail
-    //shared_ptr<DNSServer> server_;
+    // The callback will be called when we have either decided we
+    // are done, or when we give up
     isc::resolve::ResolverInterface::CallbackPtr resolvercallback_;
 
     // Protocol used for the last query.  This is set to IOFetch::UDP when a
@@ -127,16 +156,12 @@ private:
      * TODO Do something more clever with timeouts. In the long term, some
      *     computation of average RTT, increase with each retry, etc.
      */
-    // Timeout information
+    // Timeout information for outgoing queries
     int query_timeout_;
     unsigned retries_;
 
     // normal query state
 
-    // Not using NSAS at this moment, so we keep a list
-    // of 'current' zone servers
-    std::vector<addr_t> zone_servers_;
-
     // Update the question that will be sent to the server
     void setQuestion(const Question& new_question) {
         question_ = new_question;
@@ -146,18 +171,56 @@ private:
     asio::deadline_timer client_timer;
     asio::deadline_timer lookup_timer;
 
-    size_t queries_out_;
-    
     // If we timed out ourselves (lookup timeout), stop issuing queries
     bool done_;
 
-    // If we have a client timeout, we send back an answer, but don't
-    // stop. We use this variable to make sure we don't send another
-    // answer if we do find one later (or if we have a lookup_timeout)
-    bool answer_sent_;
+    // If we have a client timeout, we call back with a failure message,
+    // but we do not stop yet. We use this variable to make sure we 
+    // don't call back a second time later
+    bool callback_called_;
+
+    // Reference to our NSAS
+    isc::nsas::NameserverAddressStore& nsas_;
 
     // Reference to our cache
     isc::cache::ResolverCache& cache_;
+    
+    // the 'current' zone we are in (i.e.) we start out at the root,
+    // and for each delegation this gets updated with the zone the
+    // delegation points to.
+    // TODO: make this a Name (it is a string right now because most
+    // of the call we use it in take a string, we need update those
+    // too).
+    std::string cur_zone_;
+    
+    // This is the handler we pass on to the NSAS; it is called when
+    // the NSAS has an address for us to query
+    boost::shared_ptr<ResolverNSASCallback> nsas_callback_;
+
+    // this is set to true if we have asked the nsas to give us
+    // an address and we are waiting for it to call us back.
+    // We use is to cancel the outstanding callback in case we
+    // have a lookup timeout and decide to give up
+    bool nsas_callback_out_;
+
+    // This is the nameserver we have an outstanding query to.
+    // It is used to update the RTT once the query returns
+    isc::nsas::NameserverAddress current_ns_address;
+
+    // The moment in time we sent a query to the nameserver above.
+    struct timeval current_ns_qsent_time;
+    
+    // RunningQuery deletes itself when it is done. In order for us
+    // to do this safely, we must make sure that there are no events
+    // that might call back to it. There are two types of events in
+    // this sense; the timers we set ourselves (lookup and client),
+    // and outstanding queries to nameservers. When each of these is
+    // started, we increase this value. When they fire, it is decreased
+    // again. We cannot delete ourselves until this value is back to 0.
+    //
+    // Note that the NSAS callback is *not* seen as an outstanding
+    // event; we can cancel the NSAS callback safely.
+    size_t outstanding_events_;
 
     // perform a single lookup; first we check the cache to see
     // if we have a response for our query stored already. if
@@ -168,62 +231,90 @@ private:
         isc::resolve::initResponseMessage(question_, cached_message);
         if (cache_.lookup(question_.getName(), question_.getType(),
                           question_.getClass(), cached_message)) {
-            dlog("Message found in cache, returning that");
-            handleRecursiveAnswer(cached_message);
+            dlog("Message found in cache, continuing with that");
+            // Should these be set by the cache too?
+            cached_message.setOpcode(Opcode::QUERY());
+            cached_message.setRcode(Rcode::NOERROR());
+            cached_message.setHeaderFlag(Message::HEADERFLAG_QR);
+            if (handleRecursiveAnswer(cached_message)) {
+                callCallback(true);
+                stop();
+            }
         } else {
+            cur_zone_ = ".";
             send();
         }
         
     }
 
-    // (re)send the query to the server.
-    //
-    // \param protocol Protocol to use for the fetch (default is UDP)
+    // Send the current question to the given nameserver address
+    void sendTo(const isc::nsas::NameserverAddress& address) {
+        // We need to keep track of the Address, so that we can update
+        // the RTT
+        current_ns_address = address;
+        gettimeofday(&current_ns_qsent_time, NULL);
+        ++outstanding_events_;
+        IOFetch query(protocol_, io_, question_,
+            current_ns_address.getAddress(),
+            53, buffer_, this,
+            query_timeout_);
+        io_.get_io_service().post(query);
+    }
+    
+    // 'general' send; if we are in forwarder mode, send a query to
+    // a random nameserver in our forwarders list. If we are in
+    // recursive mode, ask the NSAS to give us an address.
     void send(IOFetch::Protocol protocol = IOFetch::UDP) {
+        // If are in forwarder mode, send it to a random
+        // forwarder. If not, ask the NSAS for an address
         const int uc = upstream_->size();
-        const int zs = zone_servers_.size();
         protocol_ = protocol;   // Store protocol being used for this
-        buffer_->clear();
         if (test_server_.second != 0) {
             dlog("Sending upstream query (" + question_.toText() +
                  ") to test server at " + test_server_.first);
+            ++outstanding_events_;
             IOFetch query(protocol, io_, question_,
                 test_server_.first,
                 test_server_.second, buffer_, this,
                 query_timeout_);
-            ++queries_out_;
             io_.get_io_service().post(query);
         } else if (uc > 0) {
+            // TODO: use boost, or rand()-utility function we provide
             int serverIndex = rand() % uc;
             dlog("Sending upstream query (" + question_.toText() +
                 ") to " + upstream_->at(serverIndex).first);
+            ++outstanding_events_;
             IOFetch query(protocol, io_, question_,
                 upstream_->at(serverIndex).first,
                 upstream_->at(serverIndex).second, buffer_, this,
                 query_timeout_);
-            ++queries_out_;
-            io_.get_io_service().post(query);
-        } else if (zs > 0) {
-            int serverIndex = rand() % zs;
-            dlog("Sending query to zone server (" + question_.toText() +
-                ") to " + zone_servers_.at(serverIndex).first);
-            IOFetch query(protocol, io_, question_,
-                zone_servers_.at(serverIndex).first,
-                zone_servers_.at(serverIndex).second, buffer_, this,
-                query_timeout_);
-            ++queries_out_;
             io_.get_io_service().post(query);
         } else {
-            dlog("Error, no upstream servers to send to.");
+            // Ask the NSAS for an address for the current zone,
+            // the callback will call the actual sendTo()
+            dlog("Look up nameserver for " + cur_zone_ + " in NSAS");
+            // Can we have multiple calls to nsas_out? Let's assume not
+            // for now
+            assert(!nsas_callback_out_);
+            nsas_callback_out_ = true;
+            nsas_.lookup(cur_zone_, question_.getClass(), nsas_callback_);
         }
     }
     
-    // This function is called by operator() if there is an actual
-    // answer from a server and we are in recursive mode
-    // depending on the contents, we go on recursing or return
+    // Called by our NSAS callback handler so we know we do not have
+    // an outstanding NSAS call anymore.
+    void nsasCallbackCalled() {
+        nsas_callback_out_ = false;
+    }
+
+    // This function is called by operator() and lookup();
+    // We have an answer either from a nameserver or the cache, and
+    // we do not know yet if this is a final answer we can send back or
+    // that more recursive processing needs to be done.
+    // Depending on the content, we go on recursing or return
     //
-    // Note that the footprint may change as this function may
-    // need to append data to the answer we are building later.
+    // This method also updates the cache, depending on the content
+    // of the message
     //
     // returns true if we are done (either we have an answer or an
     //              error message)
@@ -241,16 +332,13 @@ private:
 
         bool found_ns_address = false;
             
-        // If the packet is OK, store it in the cache
-        if (!isc::resolve::ResponseClassifier::error(category)) {
-            cache_.update(incoming);
-        }
-
         switch (category) {
         case isc::resolve::ResponseClassifier::ANSWER:
         case isc::resolve::ResponseClassifier::ANSWERCNAME:
             // Done. copy and return.
+            dlog("Response is an answer");
             isc::resolve::copyResponseMessage(incoming, answer_message_);
+            cache_.update(*answer_message_);
             return true;
             break;
         case isc::resolve::ResponseClassifier::CNAME:
@@ -261,14 +349,12 @@ private:
             if (cname_count_ >= RESOLVER_MAX_CNAME_CHAIN) {
                 // just give up
                 dlog("CNAME chain too long");
-                isc::resolve::makeErrorMessage(answer_message_,
-                                               Rcode::SERVFAIL());
+                makeSERVFAIL();
                 return true;
             }
 
             answer_message_->appendSection(Message::SECTION_ANSWER,
                                            incoming);
-            setZoneServersToRoot();
 
             question_ = Question(cname_target, question_.getClass(),
                                  question_.getType());
@@ -278,38 +364,40 @@ private:
             return false;
             break;
         case isc::resolve::ResponseClassifier::NXDOMAIN:
+        case isc::resolve::ResponseClassifier::NXRRSET:
+            dlog("Response is NXDOMAIN or NXRRSET");
             // NXDOMAIN, just copy and return.
+            dlog(incoming.toText());
             isc::resolve::copyResponseMessage(incoming, answer_message_);
+            // no negcache yet
+            //cache_.update(*answer_message_);
             return true;
             break;
         case isc::resolve::ResponseClassifier::REFERRAL:
+            dlog("Response is referral");
+            cache_.update(incoming);
             // Referral. For now we just take the first glue address
             // we find and continue with that
-            zone_servers_.clear();
 
-            for (RRsetIterator rrsi = incoming.beginSection(Message::SECTION_ADDITIONAL);
-                 rrsi != incoming.endSection(Message::SECTION_ADDITIONAL) && !found_ns_address;
-                 rrsi++) {
+            // auth section should have at least one RRset
+            // and one of them should be an NS (otherwise
+            // classifier should have error'd)
+            // TODO: should we check if it really is subzone?
+            for (RRsetIterator rrsi = incoming.beginSection(Message::SECTION_AUTHORITY);
+                 rrsi != incoming.endSection(Message::SECTION_AUTHORITY) && !found_ns_address;
+                 ++rrsi) {
                 ConstRRsetPtr rrs = *rrsi;
-                if (rrs->getType() == RRType::A()) {
-                    // found address
-                    RdataIteratorPtr rdi = rrs->getRdataIterator();
-                    // just use the first for now
-                    if (!rdi->isLast()) {
-                        std::string addr_str = rdi->getCurrent().toText();
-                        dlog("[XX] first address found: " + addr_str);
-                        // now we have one address, simply
-                        // resend that exact same query
-                        // to that address and yield, when it
-                        // returns, loop again.
-                        
-                        // TODO should use NSAS
-                        zone_servers_.push_back(addr_t(addr_str, 53));
-                        found_ns_address = true;
-                        break;
-                    }
+                if (rrs->getType() == RRType::NS()) {
+                    // TODO: make cur_zone_ a Name instead of a string
+                    // (this requires a few API changes in related
+                    // libraries, so as not to need many conversions)
+                    cur_zone_ = rrs->getName().toText();
+                    dlog("Referred to zone " + cur_zone_);
+                    found_ns_address = true;
+                    break;
                 }
             }
+
             if (found_ns_address) {
                 // next resolver round
                 // we do NOT use doLookup() here, but send() (i.e. we
@@ -319,7 +407,7 @@ private:
                 send();
                 return false;
             } else {
-                dlog("[XX] no ready-made addresses in additional. need nsas.");
+                dlog("No NS RRset in referral?");
                 // TODO this will result in answering with the delegation. oh well
                 isc::resolve::copyResponseMessage(incoming, answer_message_);
                 return true;
@@ -348,35 +436,35 @@ private:
         case isc::resolve::ResponseClassifier::NOTSINGLE:
         case isc::resolve::ResponseClassifier::OPCODE:
         case isc::resolve::ResponseClassifier::RCODE:
-
             // Should we try a different server rather than SERVFAIL?
-            isc::resolve::makeErrorMessage(answer_message_,
-                                           Rcode::SERVFAIL());
+            makeSERVFAIL();
             return true;
             break;
         }
-        // should not be reached. assert here?
-        dlog("[FATAL] unreachable code");
+
+        // Since we do not have a default in the switch above,
+        // the compiler should have errored on any missing case
+        // statements.
+        assert(false);
         return true;
     }
     
 public:
     RunningQuery(IOService& io,
-        const Question &question,
+        const Question& question,
         MessagePtr answer_message,
         boost::shared_ptr<AddressVector> upstream,
-        boost::shared_ptr<AddressVector> upstream_root,
         std::pair<std::string, uint16_t>& test_server,
         OutputBufferPtr buffer,
         isc::resolve::ResolverInterface::CallbackPtr cb,
         int query_timeout, int client_timeout, int lookup_timeout,
         unsigned retries,
+        isc::nsas::NameserverAddressStore& nsas,
         isc::cache::ResolverCache& cache) :
         io_(io),
         question_(question),
         answer_message_(answer_message),
         upstream_(upstream),
-        upstream_root_(upstream_root),
         test_server_(test_server),
         buffer_(buffer),
         resolvercallback_(cb),
@@ -386,72 +474,65 @@ public:
         retries_(retries),
         client_timer(io.get_io_service()),
         lookup_timer(io.get_io_service()),
-        queries_out_(0),
         done_(false),
-        answer_sent_(false),
-        cache_(cache)
+        callback_called_(false),
+        nsas_(nsas),
+        cache_(cache),
+        nsas_callback_(new ResolverNSASCallback(this)),
+        nsas_callback_out_(false),
+        outstanding_events_(0)
     {
         // Setup the timer to stop trying (lookup_timeout)
         if (lookup_timeout >= 0) {
             lookup_timer.expires_from_now(
                 boost::posix_time::milliseconds(lookup_timeout));
-            lookup_timer.async_wait(boost::bind(&RunningQuery::stop, this, false));
+            ++outstanding_events_;
+            lookup_timer.async_wait(boost::bind(&RunningQuery::lookupTimeout, this));
         }
         
         // Setup the timer to send an answer (client_timeout)
         if (client_timeout >= 0) {
             client_timer.expires_from_now(
                 boost::posix_time::milliseconds(client_timeout));
+            ++outstanding_events_;
             client_timer.async_wait(boost::bind(&RunningQuery::clientTimeout, this));
         }
         
-        // should use NSAS for root servers
-        // Adding root servers if not a forwarder
-        if (upstream_->empty()) {
-            setZoneServersToRoot();
-        }
-
         doLookup();
     }
 
-    void setZoneServersToRoot() {
-        zone_servers_.clear();
-        if (upstream_root_->empty()) { //if no root ips given, use this
-            zone_servers_.push_back(addr_t("192.5.5.241", 53));
-        } else {
-            // copy the list
-            dlog("Size is " + 
-                boost::lexical_cast<std::string>(upstream_root_->size()) + 
-                "\n");
-            for(AddressVector::iterator it = upstream_root_->begin();
-                it < upstream_root_->end(); ++it) {
-            zone_servers_.push_back(addr_t(it->first,it->second));
-            dlog("Put " + zone_servers_.back().first + "into root list\n");
-            }
+    // called if we have a lookup timeout; if our callback has
+    // not been called, call it now. Then stop.
+    void lookupTimeout() {
+        if (!callback_called_) {
+            makeSERVFAIL();
+            callCallback(true);
         }
+        assert(outstanding_events_ > 0);
+        --outstanding_events_;
+        stop();
     }
-    virtual void clientTimeout() {
-        // Return a SERVFAIL, but do not stop until
-        // we have an answer or timeout ourselves
-        isc::resolve::makeErrorMessage(answer_message_,
-                                       Rcode::SERVFAIL());
-        if (!answer_sent_) {
-            answer_sent_ = true;
-            resolvercallback_->success(answer_message_);
+    
+    // called if we have a client timeout; if our callback has
+    // not been called, call it now. But do not stop.
+    void clientTimeout() {
+        if (!callback_called_) {
+            makeSERVFAIL();
+            callCallback(true);
+        }
+        assert(outstanding_events_ > 0);
+        --outstanding_events_;
+        if (outstanding_events_ == 0) {
+            stop();
         }
     }
 
-    virtual void stop(bool resume) {
-        // if we cancel our timers, we will still get an event for
-        // that, so we cannot delete ourselves just yet (those events
-        // would be bound to a deleted object)
-        // cancel them one by one, both cancels should get us back
-        // here again.
-        // same goes if we have an outstanding query (can't delete
-        // until that one comes back to us)
-        done_ = true;
-        if (resume && !answer_sent_) {
-            answer_sent_ = true;
+    // If the callback has not been called yet, call it now
+    // If success is true, we call 'success' with our answer_message
+    // If it is false, we call failure()
+    void callCallback(bool success) {
+        if (!callback_called_) {
+            callback_called_ = true;
 
             // There are two types of messages we could store in the
             // cache;
@@ -470,34 +551,61 @@ public:
             // stores Messages on their question section only, this
             // does mean that we overwrite the messages we stored in
             // the previous iteration if we are following a delegation.
-            cache_.update(*answer_message_);
-
-            resolvercallback_->success(answer_message_);
-        } else {
-            resolvercallback_->failure();
-        }
-        if (lookup_timer.cancel() != 0) {
-            return;
+            if (success) {
+                resolvercallback_->success(answer_message_);
+            } else {
+                resolvercallback_->failure();
+            }
         }
-        if (client_timer.cancel() != 0) {
-            return;
+    }
+
+    // We are done. If there are no more outstanding events, we delete
+    // ourselves. If there are any, we do not.
+    void stop() {
+        done_ = true;
+        if (nsas_callback_out_) {
+            nsas_.cancel(cur_zone_, question_.getClass(), nsas_callback_);
+            nsas_callback_out_ = false;
         }
-        if (queries_out_ > 0) {
+        client_timer.cancel();
+        lookup_timer.cancel();
+        if (outstanding_events_ > 0) {
             return;
+        } else {
+            delete this;
         }
-        delete this;
     }
 
     // This function is used as callback from DNSQuery.
     virtual void operator()(IOFetch::Result result) {
-        --queries_out_;
+        // XXX is this the place for TCP retry?
+        assert(outstanding_events_ > 0);
+        --outstanding_events_;
+        
         if (!done_ && result != IOFetch::TIME_OUT) {
             // we got an answer
+
+            // Update the NSAS with the time it took
+            struct timeval cur_time;
+            gettimeofday(&cur_time, NULL);
+            uint32_t rtt;
+            if (cur_time.tv_sec >= current_ns_qsent_time.tv_sec ||
+                cur_time.tv_usec > current_ns_qsent_time.tv_usec) {
+                rtt = 1000 * (cur_time.tv_sec - current_ns_qsent_time.tv_sec);
+                rtt += (cur_time.tv_usec - current_ns_qsent_time.tv_usec) / 1000;
+            } else {
+                rtt = 1;
+            }
+
+            dlog("RTT: " + boost::lexical_cast<std::string>(rtt));
+            current_ns_address.updateRTT(rtt);
+            
             Message incoming(Message::PARSE);
             InputBuffer ibuf(buffer_->getData(), buffer_->getLength());
             incoming.fromWire(ibuf);
 
-            if (upstream_->size() == 0 &&
+            buffer_->clear();
+            if (recursive_mode() &&
                 incoming.getRcode() == Rcode::NOERROR()) {
                 done_ = handleRecursiveAnswer(incoming);
             } else {
@@ -506,17 +614,42 @@ public:
             }
             
             if (done_) {
-                stop(true);
+                callCallback(true);
+                stop();
             }
         } else if (!done_ && retries_--) {
-            // We timed out, but we have some retries, so send again
-            dlog("Timeout, resending query");
+            // Query timed out, but we have some retries, so send again
+            dlog("Timeout for " + question_.toText() + " to " + current_ns_address.getAddress().toText() + ", resending query");
+            if (recursive_mode()) {
+                current_ns_address.updateRTT(isc::nsas::AddressEntry::UNREACHABLE);
+            }
             send();
         } else {
-            // out of retries, give up for now
-            stop(false);
+            // We are either already done, or out of retries
+            if (recursive_mode() && result == IOFetch::TIME_OUT) {
+                dlog("Timeout for " + question_.toText() + " to " + current_ns_address.getAddress().toText() + ", giving up");
+                current_ns_address.updateRTT(isc::nsas::AddressEntry::UNREACHABLE);
+            }
+            if (!callback_called_) {
+                makeSERVFAIL();
+                callCallback(true);
+            }
+            stop();
         }
     }
+    
+    // Clear the answer parts of answer_message, and set the rcode
+    // to servfail
+    void makeSERVFAIL() {
+        isc::resolve::makeErrorMessage(answer_message_, Rcode::SERVFAIL());
+    }
+    
+    // Returns true if we are in 'recursive' mode
+    // Returns false if we are in 'forwarding' mode
+    // (i.e. if we have anything in upstream_)
+    bool recursive_mode() const {
+        return upstream_->empty();
+    }
 };
 
 }
@@ -532,22 +665,37 @@ RecursiveQuery::resolve(const QuestionPtr& question,
 
     OutputBufferPtr buffer(new OutputBuffer(0));
 
+    dlog("Asked to resolve: " + question->toText());
+    
     dlog("Try out cache first (direct call to resolve)");
     // First try to see if we have something cached in the messagecache
     if (cache_.lookup(question->getName(), question->getType(),
-                      question->getClass(), *answer_message)) {
+                      question->getClass(), *answer_message) &&
+        answer_message->getRRCount(Message::SECTION_ANSWER) > 0) {
         dlog("Message found in cache, returning that");
         // TODO: err, should cache set rcode as well?
         answer_message->setRcode(Rcode::NOERROR());
         callback->success(answer_message);
     } else {
-        dlog("Message not found in cache, starting recursive query");
-        // It will delete itself when it is done
-        new RunningQuery(io, *question, answer_message, upstream_,
-                         upstream_root_, test_server_,
-                         buffer, callback, query_timeout_,
-                         client_timeout_, lookup_timeout_, retries_,
-                         cache_);
+        // Perhaps we only have the one RRset?
+        // TODO: can we do this? should we check for specific types only?
+        RRsetPtr cached_rrset = cache_.lookup(question->getName(),
+                                              question->getType(),
+                                              question->getClass());
+        if (cached_rrset) {
+            dlog("Found single RRset in cache");
+            answer_message->addRRset(Message::SECTION_ANSWER,
+                                     cached_rrset);
+            answer_message->setRcode(Rcode::NOERROR());
+            callback->success(answer_message);
+        } else {
+            dlog("Message not found in cache, starting recursive query");
+            // It will delete itself when it is done
+            new RunningQuery(io, *question, answer_message, upstream_,
+                             test_server_, buffer, callback,
+                             query_timeout_, client_timeout_,
+                             lookup_timeout_, retries_, nsas_, cache_);
+        }
     }
 }
 
@@ -570,21 +718,37 @@ RecursiveQuery::resolve(const Question& question,
     answer_message->setOpcode(isc::dns::Opcode::QUERY());
     answer_message->addQuestion(question);
     
+    dlog("Asked to resolve: " + question.toText());
+    
     // First try to see if we have something cached in the messagecache
     dlog("Try out cache first (started by incoming event)");
     if (cache_.lookup(question.getName(), question.getType(),
-                      question.getClass(), *answer_message)) {
+                      question.getClass(), *answer_message) &&
+        answer_message->getRRCount(Message::SECTION_ANSWER) > 0) {
         dlog("Message found in cache, returning that");
         // TODO: err, should cache set rcode as well?
         answer_message->setRcode(Rcode::NOERROR());
         crs->success(answer_message);
     } else {
-        dlog("Message not found in cache, starting recursive query");
-        // It will delete itself when it is done
-        new RunningQuery(io, question, answer_message, upstream_, upstream_root_,
-                         test_server_,
-                         buffer, crs, query_timeout_, client_timeout_,
-                         lookup_timeout_, retries_, cache_);
+        // Perhaps we only have the one RRset?
+        // TODO: can we do this? should we check for specific types only?
+        RRsetPtr cached_rrset = cache_.lookup(question.getName(),
+                                              question.getType(),
+                                              question.getClass());
+        if (cached_rrset) {
+            dlog("Found single RRset in cache");
+            answer_message->addRRset(Message::SECTION_ANSWER,
+                                     cached_rrset);
+            answer_message->setRcode(Rcode::NOERROR());
+            crs->success(answer_message);
+        } else {
+            dlog("Message not found in cache, starting recursive query");
+            // It will delete itself when it is done
+            new RunningQuery(io, question, answer_message, upstream_,
+                             test_server_, buffer, crs, query_timeout_,
+                             client_timeout_, lookup_timeout_, retries_,
+                             nsas_, cache_);
+        }
     }
 }
 

+ 8 - 6
src/lib/asiolink/recursive_query.h

@@ -12,12 +12,13 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#ifndef __ASIOLINK_RECURSIVE_QUERY_H
-#define __ASIOLINK_RECURSIVE_QUERY_H 1
+#ifndef __RECURSIVE_QUERY_H
+#define __RECURSIVE_QUERY_H 1
 
 #include <asiolink/dns_service.h>
 #include <asiolink/dns_server.h>
 #include <dns/buffer.h>
+#include <nsas/nameserver_address_store.h>
 #include <cache/resolver_cache.h>
 
 namespace asiolink {
@@ -52,6 +53,8 @@ public:
     /// \param retries how many times we try again (0 means just send and
     ///     and return if it returs).
     RecursiveQuery(DNSService& dns_service,
+                   isc::nsas::NameserverAddressStore& nsas,
+                   isc::cache::ResolverCache& cache,
                    const std::vector<std::pair<std::string, uint16_t> >&
                    upstream, 
                    const std::vector<std::pair<std::string, uint16_t> >&
@@ -113,6 +116,8 @@ public:
     
 private:
     DNSService& dns_service_;
+    isc::nsas::NameserverAddressStore& nsas_;
+    isc::cache::ResolverCache& cache_;
     boost::shared_ptr<std::vector<std::pair<std::string, uint16_t> > >
         upstream_;
     boost::shared_ptr<std::vector<std::pair<std::string, uint16_t> > >
@@ -122,10 +127,7 @@ private:
     int client_timeout_;
     int lookup_timeout_;
     unsigned retries_;
-    // Cache. TODO: I think we want this initialized in Resolver class,
-    // not here
-    isc::cache::ResolverCache cache_;
 };
 
 }      // namespace asiolink
-#endif // __ASIOLINK_RECURSIVE_QUERY_H
+#endif // __RECURSIVE_QUERY_H

+ 7 - 3
src/lib/resolve/response_classifier.cc

@@ -114,13 +114,17 @@ ResponseClassifier::Category ResponseClassifier::classify(
             );
 
     // If there is nothing in the answer section, it is a referral - unless
-    // there is nothing in the authority section
+    // there is no NS in the authority section
     if (answer.empty()) {
         if (authority.empty()) {
             return (EMPTY);
-        } else {
-            return (REFERRAL);
         }
+        for (int i = 0; i < authority.size(); ++i) {
+            if (authority[i]->getType() == RRType::NS()) {
+                return (REFERRAL);
+            }
+        }
+        return (NXRRSET);
     }
 
     // Look at two cases - one RRset in the answer and multiple RRsets in

+ 1 - 0
src/lib/resolve/response_classifier.h

@@ -53,6 +53,7 @@ public:
         ANSWERCNAME,        ///< Response was a CNAME chain ending in an answer
         CNAME,              ///< Response was a CNAME
         NXDOMAIN,           ///< Response was an NXDOMAIN
+        NXRRSET,            ///< Response was name exists, but type does not
         REFERRAL,           ///< Response contains a referral
 
         // Codes indicating that a message is invalid.  Note that the error()

+ 7 - 0
src/lib/resolve/tests/Makefile.am

@@ -14,12 +14,19 @@ TESTS += run_unittests
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
 run_unittests_SOURCES = run_unittests.cc
+run_unittests_SOURCES += $(top_srcdir)/src/lib/dns/tests/unittest_util.h
+run_unittests_SOURCES += $(top_srcdir)/src/lib/dns/tests/unittest_util.cc
 run_unittests_SOURCES += resolve_unittest.cc
 run_unittests_SOURCES += resolver_callback_unittest.cc
 run_unittests_SOURCES += response_classifier_unittest.cc
+run_unittests_SOURCES += recursive_query_unittest.cc
+run_unittests_SOURCES += recursive_query_unittest_2.cc
 
 run_unittests_LDADD = $(GTEST_LDADD)
 run_unittests_LDADD +=  $(top_builddir)/src/lib/exceptions/libexceptions.la
+run_unittests_LDADD +=  $(top_builddir)/src/lib/nsas/libnsas.la
+run_unittests_LDADD +=  $(top_builddir)/src/lib/cache/libcache.la
+run_unittests_LDADD +=  $(top_builddir)/src/lib/asiolink/libasiolink.la
 run_unittests_LDADD +=  $(top_builddir)/src/lib/resolve/libresolve.la
 run_unittests_LDADD +=  $(top_builddir)/src/lib/dns/libdns++.la
 

+ 80 - 15
src/lib/asiolink/tests/recursive_query_unittest.cc

@@ -33,6 +33,9 @@
 #include <dns/buffer.h>
 #include <dns/message.h>
 
+#include <nsas/nameserver_address_store.h>
+#include <cache/resolver_cache.h>
+
 // IMPORTANT: We shouldn't directly use ASIO definitions in this test.
 // In particular, we must not include asio.hpp in this file.
 // The asiolink module is primarily intended to be a wrapper that hide the
@@ -41,7 +44,7 @@
 // if we include asio.hpp unless we specify a special compiler option.
 // If we need to test something at the level of underlying ASIO and need
 // their definition, that test should go to asiolink/internal/tests.
-#include <asiolink/recursive_query.h>
+#include <resolve/recursive_query.h>
 #include <asiolink/io_socket.h>
 #include <asiolink/io_service.h>
 #include <asiolink/io_message.h>
@@ -343,6 +346,12 @@ protected:
         private:
             bool* done_;
     };
+    
+    class MockResolver : public isc::resolve::ResolverInterface {
+        void resolve(const QuestionPtr& question,
+                     const ResolverInterface::CallbackPtr& callback) {
+        }
+    };
 
     // This version of mock server just stops the io_service when it is resumed
     // the second time. (Used in the clientTimeout test, where resume
@@ -403,6 +412,8 @@ protected:
     // need to recreate a new one within one onstance of this class
     IOService* io_service_;
     DNSService* dns_service_;
+    isc::nsas::NameserverAddressStore* nsas_;
+    isc::cache::ResolverCache cache_;
     ASIOCallBack* callback_;
     int callback_protocol_;
     int callback_native_;
@@ -418,6 +429,8 @@ RecursiveQueryTest::RecursiveQueryTest() :
 {
     io_service_ = new IOService();
     setDNSService(true, true);
+    boost::shared_ptr<MockResolver>mock_resolver(new MockResolver());
+    nsas_ = new isc::nsas::NameserverAddressStore(mock_resolver);
 }
 
 TEST_F(RecursiveQueryTest, v6UDPSend) {
@@ -519,6 +532,7 @@ TEST_F(RecursiveQueryTest, recursiveSetupV4) {
     setDNSService(true, false);
     uint16_t port = boost::lexical_cast<uint16_t>(TEST_CLIENT_PORT);
     EXPECT_NO_THROW(RecursiveQuery(*dns_service_,
+                                   *nsas_, cache_,
                                    singleAddress(TEST_IPV4_ADDR, port),
                                    singleAddress(TEST_IPV4_ADDR, port)));
 }
@@ -527,6 +541,7 @@ TEST_F(RecursiveQueryTest, recursiveSetupV6) {
     setDNSService(false, true);
     uint16_t port = boost::lexical_cast<uint16_t>(TEST_CLIENT_PORT);
     EXPECT_NO_THROW(RecursiveQuery(*dns_service_,
+                                   *nsas_, cache_,
                                    singleAddress(TEST_IPV6_ADDR, port),
                                    singleAddress(TEST_IPV6_ADDR,port)));
 }
@@ -545,6 +560,7 @@ TEST_F(RecursiveQueryTest, forwarderSend) {
 
     MockServer server(*io_service_);
     RecursiveQuery rq(*dns_service_,
+                      *nsas_, cache_,
                       singleAddress(TEST_IPV4_ADDR, port),
                       singleAddress(TEST_IPV4_ADDR, port));
 
@@ -632,6 +648,7 @@ TEST_F(RecursiveQueryTest, forwardQueryTimeout) {
     // Do the answer
     const uint16_t port = boost::lexical_cast<uint16_t>(TEST_CLIENT_PORT);
     RecursiveQuery query(*dns_service_,
+                         *nsas_, cache_,
                          singleAddress(TEST_IPV4_ADDR, port),
                          singleAddress(TEST_IPV4_ADDR, port),
                          10, 4000, 3000, 2);
@@ -649,8 +666,8 @@ TEST_F(RecursiveQueryTest, forwardQueryTimeout) {
     int num = 0;
     bool read_success = tryRead(sock_, recv_options, 3, &num);
 
-    // The query should fail
-    EXPECT_FALSE(done);
+    // The query should 'succeed' with an error response
+    EXPECT_TRUE(done);
     EXPECT_EQ(3, num);
     EXPECT_TRUE(read_success);
 }
@@ -666,8 +683,7 @@ TEST_F(RecursiveQueryTest, forwardClientTimeout) {
 
     // Prepare the server
     bool done1(true);
-    bool done2(true);
-    MockServerStop2 server(*io_service_, &done1, &done2);
+    MockServerStop server(*io_service_, &done1);
 
     MessagePtr answer(new Message(Message::RENDER));
 
@@ -677,6 +693,7 @@ TEST_F(RecursiveQueryTest, forwardClientTimeout) {
     // Since the lookup timer has not fired, it should retry
     // four times
     RecursiveQuery query(*dns_service_,
+                         *nsas_, cache_,
                          singleAddress(TEST_IPV4_ADDR, port),
                          singleAddress(TEST_IPV4_ADDR, port),
                          200, 480, 4000, 4);
@@ -690,15 +707,14 @@ TEST_F(RecursiveQueryTest, forwardClientTimeout) {
     // we know it'll fail, so make it a shorter timeout
     int recv_options = setSocketTimeout(sock_, 1, 0);
 
-    // Try to read 5 times
+    // Try to read 4 times
     int num = 0;
-    bool read_success = tryRead(sock_, recv_options, 5, &num);
+    bool read_success = tryRead(sock_, recv_options, 4, &num);
 
-    // The query should fail, but we should have kept on trying
+    // The query should fail
     EXPECT_TRUE(done1);
-    EXPECT_FALSE(done2);
-    EXPECT_EQ(5, num);
-    EXPECT_TRUE(read_success);
+    EXPECT_EQ(3, num);
+    EXPECT_FALSE(read_success);
 }
 
 // If we set lookup timeout to lower than querytimeout*retries, we should
@@ -721,6 +737,7 @@ TEST_F(RecursiveQueryTest, forwardLookupTimeout) {
     // Set up the test so that it will retry 5 times, but the lookup
     // timeout will fire after only 3 normal timeouts
     RecursiveQuery query(*dns_service_,
+                         *nsas_, cache_,
                          singleAddress(TEST_IPV4_ADDR, port),
                          singleAddress(TEST_IPV4_ADDR, port),
                          200, 4000, 480, 5);
@@ -737,12 +754,55 @@ TEST_F(RecursiveQueryTest, forwardLookupTimeout) {
     int num = 0;
     bool read_success = tryRead(sock_, recv_options, 5, &num);
 
-    // The query should fail
-    EXPECT_FALSE(done);
+    // The query should fail and respond with an error
+    EXPECT_TRUE(done);
     EXPECT_EQ(3, num);
     EXPECT_FALSE(read_success);
 }
 
+// Set everything very low and see if this doesn't cause weird
+// behaviour
+TEST_F(RecursiveQueryTest, lowtimeouts) {
+    // Prepare the service (we do not use the common setup, we do not answer
+    setDNSService();
+
+    // Prepare the socket
+    sock_ = createTestSocket();
+
+    // Prepare the server
+    bool done(true);
+    MockServerStop server(*io_service_, &done);
+
+    MessagePtr answer(new Message(Message::RENDER));
+
+    // Do the answer
+    const uint16_t port = boost::lexical_cast<uint16_t>(TEST_CLIENT_PORT);
+    // Set up the test so that it will retry 5 times, but the lookup
+    // timeout will fire after only 3 normal timeouts
+    RecursiveQuery query(*dns_service_,
+                         *nsas_, cache_,
+                         singleAddress(TEST_IPV4_ADDR, port),
+                         singleAddress(TEST_IPV4_ADDR, port),
+                         1, 1, 1, 1);
+    Question question(Name("example.net"), RRClass::IN(), RRType::A());
+    OutputBufferPtr buffer(new OutputBuffer(0));
+    query.resolve(question, answer, buffer, &server);
+
+    // Run the test
+    io_service_->run();
+
+    int recv_options = setSocketTimeout(sock_, 1, 0);
+
+    // Try to read 5 times, should stop after 3 reads
+    int num = 0;
+    bool read_success = tryRead(sock_, recv_options, 5, &num);
+
+    // The query should fail and respond with an error
+    EXPECT_TRUE(done);
+    EXPECT_EQ(1, num);
+    EXPECT_FALSE(read_success);
+}
+
 // as mentioned above, we need a more better framework for this,
 // in addition to that, this sends out queries into the world
 // (which we should catch somehow and fake replies for)
@@ -755,7 +815,8 @@ TEST_F(RecursiveQueryTest, DISABLED_recursiveSendOk) {
     
     MockServerStop server(*io_service_, &done);
     vector<pair<string, uint16_t> > empty_vector;
-    RecursiveQuery rq(*dns_service_, empty_vector, empty_vector, 10000, 0);
+    RecursiveQuery rq(*dns_service_, *nsas_, cache_, empty_vector,
+                      empty_vector, 10000, 0);
 
     Question q(Name("www.isc.org"), RRClass::IN(), RRType::A());
     OutputBufferPtr buffer(new OutputBuffer(0));
@@ -780,7 +841,8 @@ TEST_F(RecursiveQueryTest, DISABLED_recursiveSendNXDOMAIN) {
     
     MockServerStop server(*io_service_, &done);
     vector<pair<string, uint16_t> > empty_vector;
-    RecursiveQuery rq(*dns_service_, empty_vector, empty_vector, 10000, 0);
+    RecursiveQuery rq(*dns_service_, *nsas_, cache_, empty_vector,
+                      empty_vector, 10000, 0);
 
     Question q(Name("wwwdoesnotexist.isc.org"), RRClass::IN(), RRType::A());
     OutputBufferPtr buffer(new OutputBuffer(0));
@@ -793,4 +855,7 @@ TEST_F(RecursiveQueryTest, DISABLED_recursiveSendNXDOMAIN) {
     EXPECT_EQ(0, answer->getRRCount(Message::SECTION_ANSWER));
 }
 
+// TODO: add tests that check whether the cache is updated on succesfull
+// responses, and not updated on failures.
+
 }

+ 18 - 4
src/lib/asiolink/tests/recursive_query_unittest_2.cc

@@ -42,7 +42,7 @@
 #include <asiolink/io_endpoint.h>
 #include <asiolink/io_fetch.h>
 #include <asiolink/io_service.h>
-#include <asiolink/recursive_query.h>
+#include <resolve/recursive_query.h>
 #include <resolve/resolver_interface.h>
 
 using namespace asio;
@@ -84,6 +84,14 @@ const char* WWW_EXAMPLE_ORG = "192.0.2.254";    ///< Address of www.example.org
 // enable them.
 const bool DEBUG_PRINT = false;
 
+class MockResolver : public isc::resolve::ResolverInterface {
+    void resolve(const QuestionPtr& question,
+                 const ResolverInterface::CallbackPtr& callback) {
+    }
+};
+
+
+
 /// \brief Test fixture for the RecursiveQuery Test
 class RecursiveQueryTest2 : public virtual ::testing::Test
 {
@@ -109,6 +117,8 @@ public:
     QueryStatus     last_;                      ///< What was the last state
     QueryStatus     expected_;                  ///< Expected next state
     OutputBufferPtr question_buffer_;           ///< Question we expect to receive
+    isc::nsas::NameserverAddressStore* nsas_;
+    isc::cache::ResolverCache cache_;
 
     // Data for TCP Server
     size_t          tcp_cumulative_;            ///< Cumulative TCP data received
@@ -145,7 +155,10 @@ public:
         udp_receive_buffer_(),
         udp_send_buffer_(new OutputBuffer(BUFFER_SIZE)),
         udp_socket_(service_.get_io_service(), udp::v4())
-    {}
+    {
+        boost::shared_ptr<MockResolver>mock_resolver(new MockResolver());
+        nsas_ = new isc::nsas::NameserverAddressStore(mock_resolver);
+    }
 
     /// \brief Set Common Message Bits
     ///
@@ -617,12 +630,13 @@ TEST_F(RecursiveQueryTest2, Resolve) {
     // Set up the RecursiveQuery object.
     std::vector<std::pair<std::string, uint16_t> > upstream;         // Empty
     std::vector<std::pair<std::string, uint16_t> > upstream_root;    // Empty
-    RecursiveQuery query(dns_service_, upstream, upstream_root);
+    RecursiveQuery query(dns_service_, *nsas_, cache_,
+                         upstream, upstream_root);
     query.setTestServer(TEST_ADDRESS, TEST_PORT);
 
     // Set up callback for the tor eceive notification that the query has
     // completed.
-    ResolverInterface::CallbackPtr
+    isc::resolve::ResolverInterface::CallbackPtr
         resolver_callback(new ResolverCallback(service_));
 
     // Kick off the resolution process.  We expect the first question to go to

+ 17 - 0
src/lib/resolve/tests/response_classifier_unittest.cc

@@ -80,6 +80,8 @@ public:
             RRType::CNAME(), RRTTL(300))),
         rrs_in_ns_(new RRset(Name("example.com"), RRClass::IN(),
             RRType::NS(), RRTTL(300))),
+        rrs_in_soa_(new RRset(Name("example.com"), RRClass::IN(),
+            RRType::SOA(), RRTTL(300))),
         rrs_in_txt_www(new RRset(Name("www.example.com"), RRClass::IN(),
             RRType::TXT(), RRTTL(300))),
         cname_target("."),
@@ -115,6 +117,9 @@ public:
         // Set up an imaginary NS RRset for an authority section
         rrs_in_ns_->addRdata(ConstRdataPtr(new NS(Name("ns0.isc.org"))));
         rrs_in_ns_->addRdata(ConstRdataPtr(new NS(Name("ns0.example.org"))));
+        
+        // And an imaginary SOA
+        rrs_in_soa_->addRdata(ConstRdataPtr(new SOA(Name("ns0.example.org"), Name("root.example.org"), 1, 2, 3, 4, 5)));
 
         // Set up the records for the www host
         rrs_in_a_www->addRdata(ConstRdataPtr(new A("1.2.3.4")));
@@ -146,6 +151,7 @@ public:
     RRsetPtr    rrs_in_cname_www1;  // www1.example.com IN CNAME
     RRsetPtr    rrs_in_cname_www2;  // www2.example.com IN CNAME
     RRsetPtr    rrs_in_ns_;         // example.com IN NS
+    RRsetPtr    rrs_in_soa_;        // example.com IN SOA
     RRsetPtr    rrs_in_txt_www;     // www.example.com IN TXT
     Name        cname_target;       // Used in response classifier to
                                     // store the target of a possible
@@ -349,6 +355,17 @@ TEST_F(ResponseClassifierTest, EmptyAnswerReferral) {
 
 }
 
+// Test if we get a NOERROR answer that contains neither an actual
+// answer nor a delegation
+TEST_F(ResponseClassifierTest, NoErrorNoData) {
+
+    msg_a.addRRset(Message::SECTION_AUTHORITY, rrs_in_soa_);
+    EXPECT_EQ(ResponseClassifier::NXRRSET,
+        ResponseClassifier::classify(qu_in_a_www, msg_a, cname_target,
+                                     cname_count));
+
+}
+
 // Check the case where we have a simple answer in the answer section.  This
 // occurs when the QNAME/QTYPE/QCLASS matches one of the RRsets in the
 // answer section - expect when the QTYPE is ANY, in which case the match