diff --git a/src/lib/resolve/recursive_query.cc b/src/lib/resolve/recursive_query.cc index 68b06b080c..7eae6fe8cf 100644 --- a/src/lib/resolve/recursive_query.cc +++ b/src/lib/resolve/recursive_query.cc @@ -158,6 +158,7 @@ RecursiveQuery::setRttRecorder(boost::shared_ptr& recorder) { rtt_recorder_ = recorder; } +namespace { typedef std::pair addr_t; /* @@ -167,11 +168,11 @@ typedef std::pair addr_t; * * Used by RecursiveQuery::sendQuery. */ -class RunningQuery::RunningQueryImpl : public IOFetch::Callback { +class RunningQuery : public IOFetch::Callback, public AbstractRunningQuery { class ResolverNSASCallback : public isc::nsas::AddressRequestCallback { public: - ResolverNSASCallback(RunningQueryImpl* rq) : rq_(rq) {} + ResolverNSASCallback(RunningQuery* rq) : rq_(rq) {} void success(const isc::nsas::NameserverAddress& address) { // Success callback, send query to found namesever @@ -191,7 +192,7 @@ public: } private: - RunningQueryImpl* rq_; + RunningQuery* rq_; }; @@ -292,7 +293,7 @@ private: // The moment in time we sent a query to the nameserver above. struct timeval current_ns_qsent_time; - // RunningQueryImpl deletes itself when it is done. In order for us + // 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), @@ -669,7 +670,7 @@ SERVFAIL: } public: - RunningQueryImpl(IOService& io, + RunningQuery(IOService& io, const Question& question, MessagePtr answer_message, std::pair& test_server, @@ -712,7 +713,7 @@ public: lookup_timer.expires_from_now( boost::posix_time::milliseconds(lookup_timeout)); ++outstanding_events_; - lookup_timer.async_wait(boost::bind(&RunningQueryImpl::lookupTimeout, this)); + lookup_timer.async_wait(boost::bind(&RunningQuery::lookupTimeout, this)); } // Setup the timer to send an answer (client_timeout) @@ -720,12 +721,14 @@ public: client_timer.expires_from_now( boost::posix_time::milliseconds(client_timeout)); ++outstanding_events_; - client_timer.async_wait(boost::bind(&RunningQueryImpl::clientTimeout, this)); + client_timer.async_wait(boost::bind(&RunningQuery::clientTimeout, this)); } doLookup(); } + virtual ~RunningQuery() {}; + // called if we have a lookup timeout; if our callback has // not been called, call it now. Then stop. void lookupTimeout() { @@ -896,7 +899,7 @@ public: } }; -class ForwardQuery::ForwardQueryImpl : public IOFetch::Callback { +class ForwardQuery : public IOFetch::Callback, public AbstractRunningQuery { private: // The io service to handle async calls IOService& io_; @@ -928,7 +931,7 @@ private: asio::deadline_timer lookup_timer; // Make FowardQuery deletes itself safely. for more information see - // the comments of outstanding_events in RunningQueryImpl. + // the comments of outstanding_events in RunningQuery. size_t outstanding_events_; // If we have a client timeout, we call back with a failure message, @@ -959,7 +962,7 @@ private: } public: - ForwardQueryImpl(IOService& io, + ForwardQuery(IOService& io, ConstMessagePtr query_message, MessagePtr answer_message, boost::shared_ptr upstream, @@ -983,7 +986,7 @@ public: lookup_timer.expires_from_now( boost::posix_time::milliseconds(lookup_timeout)); ++outstanding_events_; - lookup_timer.async_wait(boost::bind(&ForwardQueryImpl::lookupTimeout, this)); + lookup_timer.async_wait(boost::bind(&ForwardQuery::lookupTimeout, this)); } // Setup the timer to send an answer (client_timeout) @@ -991,12 +994,14 @@ public: client_timer.expires_from_now( boost::posix_time::milliseconds(client_timeout)); ++outstanding_events_; - client_timer.async_wait(boost::bind(&ForwardQueryImpl::clientTimeout, this)); + client_timer.async_wait(boost::bind(&ForwardQuery::clientTimeout, this)); } send(); } + virtual ~ForwardQuery() {}; + virtual void lookupTimeout() { if (!callback_called_) { makeSERVFAIL(); @@ -1072,7 +1077,9 @@ public: } }; -RunningQuery* +} + +AbstractRunningQuery* RecursiveQuery::resolve(const QuestionPtr& question, const isc::resolve::ResolverInterface::CallbackPtr callback) { @@ -1125,7 +1132,7 @@ RecursiveQuery::resolve(const QuestionPtr& question, return (NULL); } -RunningQuery* +AbstractRunningQuery* RecursiveQuery::resolve(const Question& question, MessagePtr answer_message, OutputBufferPtr buffer, @@ -1187,7 +1194,7 @@ RecursiveQuery::resolve(const Question& question, return (NULL); } -ForwardQuery* +AbstractRunningQuery* RecursiveQuery::forward(ConstMessagePtr query_message, MessagePtr answer_message, OutputBufferPtr buffer, @@ -1213,47 +1220,9 @@ RecursiveQuery::forward(ConstMessagePtr query_message, // everything throught without interpretation, except // QID, port number. The response will not be cached. // It will delete itself when it is done - return new ForwardQuery(io, query_message, answer_message, - upstream_, buffer, callback, query_timeout_, - client_timeout_, lookup_timeout_); -} - -ForwardQuery::ForwardQuery(IOService& io, - ConstMessagePtr query_message, - MessagePtr answer_message, - boost::shared_ptr upstream, - OutputBufferPtr buffer, - isc::resolve::ResolverInterface::CallbackPtr cb, - int query_timeout, int client_timeout, int lookup_timeout) { - fqi_ = new ForwardQueryImpl(io, query_message, answer_message, - upstream, buffer, cb, query_timeout, - client_timeout, lookup_timeout); -} - -ForwardQuery::~ForwardQuery() { - delete fqi_; -} - -RunningQuery::RunningQuery(isc::asiolink::IOService& io, - const isc::dns::Question question, - isc::dns::MessagePtr answer_message, - std::pair& test_server, - isc::util::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, - boost::shared_ptr& recorder) -{ - rqi_ = new RunningQueryImpl(io, question, answer_message, test_server, - buffer, cb, query_timeout, client_timeout, - lookup_timeout, retries, nsas, cache, - recorder); -} - -RunningQuery::~RunningQuery() { - delete rqi_; + return (new ForwardQuery(io, query_message, answer_message, + upstream_, buffer, callback, query_timeout_, + client_timeout_, lookup_timeout_)); } } // namespace asiodns diff --git a/src/lib/resolve/recursive_query.h b/src/lib/resolve/recursive_query.h index a8eab8d735..7cda83730c 100644 --- a/src/lib/resolve/recursive_query.h +++ b/src/lib/resolve/recursive_query.h @@ -54,63 +54,23 @@ private: typedef std::vector > AddressVector; -/// \brief A forwarded query -/// -/// This class represents an active forwarded query object; -/// i.e. an outstanding query to a different server when operating -/// in forwarder mode. -/// -/// It can not be instantiated directly, but is created by -/// RecursiveQuery::forward(). -/// -/// Its only public method is its destructor, and that should in theory -/// not be called either except in some unit tests. Instances should -/// delete themselves when the query is finished. -class ForwardQuery { - friend class RecursiveQuery; -private: - class ForwardQueryImpl; - ForwardQueryImpl* fqi_; - ForwardQuery(isc::asiolink::IOService& io, - isc::dns::ConstMessagePtr query_message, - isc::dns::MessagePtr answer_message, - boost::shared_ptr upstream, - isc::util::OutputBufferPtr buffer, - isc::resolve::ResolverInterface::CallbackPtr cb, - int query_timeout, int client_timeout, int lookup_timeout); -public: - ~ForwardQuery(); -}; - /// \brief A Running query /// -/// This class represents an active running query object; -/// i.e. an outstanding query to an authoritative name server. +/// This base class represents an active running query object; +/// i.e. an outstanding query to an authoritative name server or +/// upstream server (when running in forwarder mode). /// /// It can not be instantiated directly, but is created by -/// RecursiveQuery::resolve(). +/// RecursiveQuery::resolve() and RecursiveQuery::forward(). /// /// Its only public method is its destructor, and that should in theory /// not be called either except in some unit tests. Instances should /// delete themselves when the query is finished. -class RunningQuery { - friend class RecursiveQuery; -private: - class RunningQueryImpl; - RunningQueryImpl* rqi_; - RunningQuery(isc::asiolink::IOService& io, - const isc::dns::Question question, - isc::dns::MessagePtr answer_message, - std::pair& test_server, - isc::util::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, - boost::shared_ptr& recorder); +class AbstractRunningQuery { +protected: + AbstractRunningQuery() {}; public: - ~RunningQuery(); + virtual ~AbstractRunningQuery() {}; }; /// \brief Recursive Query @@ -186,7 +146,7 @@ public: /// \param question The question being answered /// \param callback Callback object. See /// \c ResolverInterface::Callback for more information - RunningQuery* resolve(const isc::dns::QuestionPtr& question, + AbstractRunningQuery* resolve(const isc::dns::QuestionPtr& question, const isc::resolve::ResolverInterface::CallbackPtr callback); @@ -202,13 +162,14 @@ public: /// \param buffer An output buffer into which the intermediate responses will /// be copied. /// \param server A pointer to the \c DNSServer object handling the client - /// \return A pointer to the active RunningQuery object created by this - /// call (if any); this object should delete itself in normal - /// circumstances, and can normally be ignored by the caller, - /// but a pointer is returned for use-cases such as unit tests. + /// \return A pointer to the active AbstractRunningQuery object + /// created by this call (if any); this object should delete + /// itself in normal circumstances, and can normally be ignored + /// by the caller, but a pointer is returned for use-cases + /// such as unit tests. /// Returns NULL if the data was found internally and no actual /// query was sent. - RunningQuery* resolve(const isc::dns::Question& question, + AbstractRunningQuery* resolve(const isc::dns::Question& question, isc::dns::MessagePtr answer_message, isc::util::OutputBufferPtr buffer, DNSServer* server); @@ -228,7 +189,7 @@ public: /// this object should delete itself in normal circumstances, /// and can normally be ignored by the caller, but a pointer /// is returned for use-cases such as unit tests. - ForwardQuery* forward(isc::dns::ConstMessagePtr query_message, + AbstractRunningQuery* forward(isc::dns::ConstMessagePtr query_message, isc::dns::MessagePtr answer_message, isc::util::OutputBufferPtr buffer, DNSServer* server, diff --git a/src/lib/resolve/tests/recursive_query_unittest.cc b/src/lib/resolve/tests/recursive_query_unittest.cc index 9582ac1772..7e7557bee4 100644 --- a/src/lib/resolve/tests/recursive_query_unittest.cc +++ b/src/lib/resolve/tests/recursive_query_unittest.cc @@ -510,12 +510,13 @@ protected: vector callback_data_; ScopedSocket sock_; boost::shared_ptr resolver_; - RunningQuery* running_query_; + AbstractRunningQuery* running_query_; }; RecursiveQueryTest::RecursiveQueryTest() : dns_service_(NULL), callback_(NULL), callback_protocol_(0), - callback_native_(-1), resolver_(new isc::util::unittests::TestResolver()), running_query_(NULL) + callback_native_(-1), resolver_(new isc::util::unittests::TestResolver()), + running_query_(NULL) { nsas_.reset(new isc::nsas::NameserverAddressStore(resolver_)); } @@ -661,7 +662,7 @@ TEST_F(RecursiveQueryTest, forwarderSend) { OutputBufferPtr buffer(new OutputBuffer(0)); MessagePtr answer(new Message(Message::RENDER)); - ForwardQuery* fq = rq.forward(query_message, answer, buffer, &server); + AbstractRunningQuery* fq = rq.forward(query_message, answer, buffer, &server); char data[4096]; size_t size = sizeof(data); @@ -759,7 +760,7 @@ TEST_F(RecursiveQueryTest, forwardQueryTimeout) { isc::resolve::initResponseMessage(question, *query_message); boost::shared_ptr callback(new MockResolverCallback(&server)); - ForwardQuery* fq = query.forward(query_message, answer, buffer, &server, callback); + AbstractRunningQuery* fq = query.forward(query_message, answer, buffer, &server, callback); // Run the test io_service_.run(); EXPECT_EQ(callback->result, MockResolverCallback::FAILURE); @@ -795,7 +796,7 @@ TEST_F(RecursiveQueryTest, forwardClientTimeout) { isc::resolve::initResponseMessage(q, *query_message); boost::shared_ptr callback(new MockResolverCallback(&server)); - ForwardQuery* fq = query.forward(query_message, answer, buffer, &server, callback); + AbstractRunningQuery* fq = query.forward(query_message, answer, buffer, &server, callback); // Run the test io_service_.run(); EXPECT_EQ(callback->result, MockResolverCallback::FAILURE); @@ -832,7 +833,7 @@ TEST_F(RecursiveQueryTest, forwardLookupTimeout) { isc::resolve::initResponseMessage(question, *query_message); boost::shared_ptr callback(new MockResolverCallback(&server)); - ForwardQuery* fq = query.forward(query_message, answer, buffer, &server, callback); + AbstractRunningQuery* fq = query.forward(query_message, answer, buffer, &server, callback); // Run the test io_service_.run(); EXPECT_EQ(callback->result, MockResolverCallback::FAILURE); @@ -868,7 +869,7 @@ TEST_F(RecursiveQueryTest, lowtimeouts) { isc::resolve::initResponseMessage(question, *query_message); boost::shared_ptr callback(new MockResolverCallback(&server)); - ForwardQuery* fq = query.forward(query_message, answer, buffer, &server, callback); + AbstractRunningQuery* fq = query.forward(query_message, answer, buffer, &server, callback); // Run the test io_service_.run(); EXPECT_EQ(callback->result, MockResolverCallback::FAILURE); diff --git a/src/lib/resolve/tests/recursive_query_unittest_2.cc b/src/lib/resolve/tests/recursive_query_unittest_2.cc index 134d9b1909..40837a5558 100644 --- a/src/lib/resolve/tests/recursive_query_unittest_2.cc +++ b/src/lib/resolve/tests/recursive_query_unittest_2.cc @@ -150,7 +150,7 @@ public: udp::socket udp_socket_; ///< Socket used by UDP server /// TODO: doc - RunningQuery* running_query_; + AbstractRunningQuery* running_query_; /// \brief Constructor RecursiveQueryTest2() : diff --git a/src/lib/resolve/tests/recursive_query_unittest_3.cc b/src/lib/resolve/tests/recursive_query_unittest_3.cc index 6d200b037e..3da2bfff3f 100644 --- a/src/lib/resolve/tests/recursive_query_unittest_3.cc +++ b/src/lib/resolve/tests/recursive_query_unittest_3.cc @@ -133,7 +133,7 @@ public: udp::socket udp_socket_; ///< Socket used by UDP server /// TODO - RunningQuery* running_query_; + AbstractRunningQuery* running_query_; /// \brief Constructor RecursiveQueryTest3() :