[svn] commit: r3782 - in /branches/trac347: ./ src/bin/auth/ src/bin/auth/benchmarks/ src/bin/auth/tests/

BIND 10 source code commits bind10-changes at lists.isc.org
Thu Dec 9 09:38:04 UTC 2010


Author: y-aharen
Date: Thu Dec  9 09:38:03 2010
New Revision: 3782

Log:
Address some review comments [comment:ticket:347:5]

* Timer call back scheme of submitting statistics changed.
* Avoid dynamic allocations.
* Some files and classes were renamed.
  - stats.{cc,h} -> statistics.{cc,h}
  - class Counter -> class QueryCounters
* Remove unnecessary includes.
* Some test cases were appended.
  - class IntervalTimer
  - Submitted values from QueryCounters::submitStatistics()
  - Counter increment with handling query
* Remove namespace 'statistics'.
* Argument to constructor of IntervalTimer was changed
  to asio_link::IOService&.
* Error handling improved
  - Parameter check
  - Exception handling
* Documentation and comments were appended.


Added:
    branches/trac347/src/bin/auth/statistics.cc   (contents, props changed)
      - copied, changed from r3689, branches/trac347/src/bin/auth/stats.cc
    branches/trac347/src/bin/auth/statistics.h   (contents, props changed)
      - copied, changed from r3689, branches/trac347/src/bin/auth/stats.h
    branches/trac347/src/bin/auth/tests/statistics_unittest.cc   (with props)
Removed:
    branches/trac347/src/bin/auth/stats.cc
    branches/trac347/src/bin/auth/stats.h
Modified:
    branches/trac347/ChangeLog
    branches/trac347/src/bin/auth/Makefile.am
    branches/trac347/src/bin/auth/asio_link.cc
    branches/trac347/src/bin/auth/asio_link.h
    branches/trac347/src/bin/auth/auth.spec.pre.in
    branches/trac347/src/bin/auth/auth_srv.cc
    branches/trac347/src/bin/auth/auth_srv.h
    branches/trac347/src/bin/auth/benchmarks/Makefile.am
    branches/trac347/src/bin/auth/main.cc
    branches/trac347/src/bin/auth/tests/Makefile.am
    branches/trac347/src/bin/auth/tests/asio_link_unittest.cc
    branches/trac347/src/bin/auth/tests/auth_srv_unittest.cc

Modified: branches/trac347/ChangeLog
==============================================================================
--- branches/trac347/ChangeLog (original)
+++ branches/trac347/ChangeLog Thu Dec  9 09:38:03 2010
@@ -1,8 +1,11 @@
   TBD.  [func]		y-aharen
 	src/bin/auth: Added a feature to count query and send counter
-	values to b10-stats periodically. To support it, added wrapping
+	values to statistics periodically. To support it, added wrapping
 	class of asio::deadline_timer to use as interval timer.
 	Added a command 'sendstats' to send counter values at once.
+	The counter value can be seen with sending 'sendstats' command to
+	statistics module via bindctl like:
+	  ... "auth.queries.tcp": 1, "auth.queries.udp": 1 ...
 	(Trac #347, svn rxxxx)
 
   122.  [func]		stephen

Modified: branches/trac347/src/bin/auth/Makefile.am
==============================================================================
--- branches/trac347/src/bin/auth/Makefile.am (original)
+++ branches/trac347/src/bin/auth/Makefile.am Thu Dec  9 09:38:03 2010
@@ -58,7 +58,7 @@
 b10_auth_SOURCES = auth_srv.cc auth_srv.h
 b10_auth_SOURCES += change_user.cc change_user.h
 b10_auth_SOURCES += common.h
-b10_auth_SOURCES += stats.cc stats.h
+b10_auth_SOURCES += statistics.cc statistics.h
 b10_auth_SOURCES += main.cc
 b10_auth_LDADD =  $(top_builddir)/src/lib/datasrc/libdatasrc.la
 b10_auth_LDADD += $(top_builddir)/src/lib/dns/libdns++.la

Modified: branches/trac347/src/bin/auth/asio_link.cc
==============================================================================
--- branches/trac347/src/bin/auth/asio_link.cc (original)
+++ branches/trac347/src/bin/auth/asio_link.cc Thu Dec  9 09:38:03 2010
@@ -669,64 +669,74 @@
 
 class IntervalTimerImpl {
 private:
-    // prohibit copy to avoid the function be called twice
+    // prohibit copy
     IntervalTimerImpl(const IntervalTimerImpl& source);
     IntervalTimerImpl& operator=(const IntervalTimerImpl& source);
 public:
-    IntervalTimerImpl(asio::io_service& io_service);
+    IntervalTimerImpl(IOService& io_service);
     ~IntervalTimerImpl();
-    bool setupTimer(const IntervalTimer::Callback& cbfunc,
+    void setupTimer(const IntervalTimer::Callback& cbfunc,
                     const uint32_t interval);
     void callback(const asio::error_code& error);
 private:
+    // a function to update timer_ when it expires
     void updateTimer();
+    // a function to call back when timer_ expires
     IntervalTimer::Callback cbfunc_;
     // interval in seconds
     uint32_t interval_;
+    // asio timer
     asio::deadline_timer timer_;
 };
 
-IntervalTimerImpl::IntervalTimerImpl(asio::io_service& io_service) :
-    timer_(io_service)
+IntervalTimerImpl::IntervalTimerImpl(IOService& io_service) :
+    timer_(io_service.get_io_service())
 {}
 
-IntervalTimerImpl::~IntervalTimerImpl() {
-    timer_.cancel();
-}
-
-bool
+IntervalTimerImpl::~IntervalTimerImpl()
+{}
+
+void
 IntervalTimerImpl::setupTimer(const IntervalTimer::Callback& cbfunc,
-                              const uint32_t interval)
+                                    const uint32_t interval)
 {
-    // interval value must be positive
-    assert(interval > 0);
+    // Interval should not be 0.
+    if (interval == 0) {
+        isc_throw(isc::BadValue, "Interval should not be 0");
+    }
+    // Call back function should not be empty.
+    if (cbfunc.empty()) {
+        isc_throw(isc::InvalidParameter, "Callback function is empty");
+    }
     cbfunc_ = cbfunc;
     interval_ = interval;
-    // start timer
+    // Set initial expire time.
+    // At this point the timer is not running yet and will not expire.
+    // After calling IOService::run(), the timer will expire.
     updateTimer();
-    return (true);
+    return;
 }
 
 void
 IntervalTimerImpl::updateTimer() {
-    // update expire time (current time + interval_)
+    // Update expire time to (current time + interval_).
     timer_.expires_from_now(boost::posix_time::seconds(interval_));
-    // restart timer
+    // Reset timer.
     timer_.async_wait(boost::bind(&IntervalTimerImpl::callback, this, _1));
 }
 
 void
 IntervalTimerImpl::callback(const asio::error_code& cancelled) {
-    assert(!cbfunc_.empty());
-    // skip function call in case the timer was cancelled
+    // Do not call cbfunc_ in case the timer was cancelled.
+    // The timer will be canelled in the destructor of asio::deadline_timer.
     if (!cancelled) {
         cbfunc_();
-        // restart timer
+        // Set next expire time.
         updateTimer();
     }
 }
 
-IntervalTimer::IntervalTimer(asio::io_service& io_service) {
+IntervalTimer::IntervalTimer(IOService& io_service) {
     impl_ = new IntervalTimerImpl(io_service);
 }
 
@@ -734,7 +744,7 @@
     delete impl_;
 }
 
-bool
+void
 IntervalTimer::setupTimer(const Callback& cbfunc, const uint32_t interval) {
     return (impl_->setupTimer(cbfunc, interval));
 }

Modified: branches/trac347/src/bin/auth/asio_link.h
==============================================================================
--- branches/trac347/src/bin/auth/asio_link.h (original)
+++ branches/trac347/src/bin/auth/asio_link.h Thu Dec  9 09:38:03 2010
@@ -446,15 +446,45 @@
     IOServiceImpl* impl_;
 };
 
-/// \brief The \c IntervalTimer class is a wrapper for the ASIO \c deadline_timer
-/// class.
-///
-/// This class is implemented to use boost::deadline_timer as interval timer.
-/// Copy of this class is prohibited not to call the callback function twice.
+/// \brief The \c IntervalTimer class is a wrapper for the ASIO
+/// \c asio::deadline_timer class.
+///
+/// This class is implemented to use \c asio::deadline_timer as
+/// interval timer.
+///
+/// \c setupTimer() sets a timer to expire on (now + interval) and
+/// a call back function.
+///
+/// \c IntervalTimerImpl::callback() is called by the timer when
+/// it expires.
+///
+/// The function calls the call back function set by \c setupTimer()
+/// and update the timer to expire on (now + interval).
+/// The type of call back function is \c void(void).
+///
+/// This class is mainly designed to use for calling
+/// \c QueryCounters::submitStatistics() periodically.
+///
+/// Note: Destruction of the instance of this class while call back
+/// is pending causes throwing an exception from IOService.
+///
+/// Sample code:
+/// \code
+///  void function_to_call_back() {
+///      // this function will called periodically
+///  }
+///  int interval_in_seconds = 1;
+///  IOService io_service;
+///
+///  IntervalTimer intervalTimer(io_service);
+///  intervalTimer.setupTimer(function_to_call_back, interval_in_seconds);
+///  io_service.run();
+/// \endcode
+///
 class IntervalTimer {
 public:
     /// \name The type of timer callback function
-    typedef boost::function<void(void)> Callback;
+    typedef boost::function<void()> Callback;
 
     ///
     /// \name Constructors and Destructor
@@ -466,22 +496,43 @@
     IntervalTimer(const IntervalTimer& source);
     IntervalTimer& operator=(const IntervalTimer& source);
 public:
-    /// \brief The constructor with asio::io_service.
-    ///
-    /// \param io_service A reference to an instance of asio::io_service
-    IntervalTimer(asio::io_service& io_service);
+    /// \brief The constructor with \c IOService.
+    ///
+    /// This constructor may throw a standard exception if
+    /// memory allocation fails inside the method.
+    /// This constructor may also throw \c asio::system_error.
+    ///
+    /// \param io_service A reference to an instance of IOService
+    ///
+    IntervalTimer(IOService& io_service);
     /// \brief The destructor.
+    ///
+    /// This destructor never throws an exception.
+    ///
+    /// On the destruction of this class the timer will be cancelled
+    /// inside \c asio::deadline_timer.
+    ///
     ~IntervalTimer();
     //@}
 
-    /// \brief Register timer callback function
-    ///
-    /// \param cbfunc A reference to a function to call back
+    /// \brief Register timer callback function and interval
+    ///
+    /// This function sets call back function and interval in seconds.
+    /// Timer will actually start after calling \c IOService::run().
+    ///
+    /// \param cbfunc A reference to a function \c void(void) to call back
     /// when the timer is expired
-    /// \param interval Interval in seconds
-    ///
-    /// \return \c true on success
-    bool setupTimer(const Callback& cbfunc, const uint32_t interval);
+    /// \param interval Interval in seconds (greater than 0)
+    ///
+    /// Note: IntervalTimer will not pass asio::error_code to
+    /// call back function. In case the timer is cancelled, the function
+    /// will not be called.
+    ///
+    /// \throw isc::InvalidParameter cbfunc is empty
+    /// \throw isc::BadValue interval is 0
+    /// \throw asio::system_error ASIO library error
+    ///
+    void setupTimer(const Callback& cbfunc, const uint32_t interval);
 private:
     IntervalTimerImpl* impl_;
 };

Modified: branches/trac347/src/bin/auth/auth.spec.pre.in
==============================================================================
--- branches/trac347/src/bin/auth/auth.spec.pre.in (original)
+++ branches/trac347/src/bin/auth/auth.spec.pre.in Thu Dec  9 09:38:03 2010
@@ -17,7 +17,7 @@
       },
       {
         "command_name": "sendstats",
-        "command_description": "Send statistics data to b10-stats at once",
+        "command_description": "Send data to a statistics module at once",
         "command_args": []
       }
     ]

Modified: branches/trac347/src/bin/auth/auth_srv.cc
==============================================================================
--- branches/trac347/src/bin/auth/auth_srv.cc (original)
+++ branches/trac347/src/bin/auth/auth_srv.cc Thu Dec  9 09:38:03 2010
@@ -50,6 +50,7 @@
 #include <auth/common.h>
 #include <auth/auth_srv.h>
 #include <auth/asio_link.h>
+#include <auth/statistics.h>
 
 using namespace std;
 
@@ -99,6 +100,9 @@
 
     /// Hot spot cache
     isc::datasrc::HotCache cache_;
+
+    /// Query counters for statistics
+    QueryCounters counters_;
 };
 
 AuthSrvImpl::AuthSrvImpl(const bool use_cache,
@@ -106,7 +110,8 @@
     config_session_(NULL), verbose_mode_(false),
     xfrin_session_(NULL),
     xfrout_connected_(false),
-    xfrout_client_(xfrout_client)
+    xfrout_client_(xfrout_client),
+    counters_(verbose_mode_)
 {
     // cur_datasrc_ is automatically initialized by the default constructor,
     // effectively being an empty (sqlite) data source.  once ccsession is up
@@ -128,13 +133,10 @@
 
 AuthSrv::AuthSrv(const bool use_cache, AbstractXfroutClient& xfrout_client) :
     impl_(new AuthSrvImpl(use_cache, xfrout_client))
-{
-    counter = new statistics::Counter(impl_->verbose_mode_);
-}
+{}
 
 AuthSrv::~AuthSrv() {
     delete impl_;
-    delete counter;
 }
 
 namespace {
@@ -219,7 +221,7 @@
 
 void
 AuthSrv::setStatsSession(AbstractSession* stats_session) {
-    counter->setStatsSession(stats_session);
+    impl_->counters_.setStatsSession(stats_session);
 }
 
 ModuleCCSession*
@@ -276,15 +278,13 @@
 
     // increment Query Counter
     if (io_message.getSocket().getProtocol() == IPPROTO_UDP) {
-        counter->inc(statistics::Counter::COUNTER_UDP);
+        impl_->counters_.inc(QueryCounters::COUNTER_UDP);
     } else if (io_message.getSocket().getProtocol() == IPPROTO_TCP) {
-        counter->inc(statistics::Counter::COUNTER_TCP);
+        impl_->counters_.inc(QueryCounters::COUNTER_TCP);
     } else {
         // unknown protocol
-        if (impl_->verbose_mode_) {
-            cerr << "[b10-auth] Unknown protocol: " <<
-                     io_message.getSocket().getProtocol() << endl;
-        }
+        isc_throw(Unexpected, "Unknown protocol: " <<
+                  io_message.getSocket().getProtocol());
     }
 
     // Perform further protocol-level validation.
@@ -565,9 +565,11 @@
     }
 }
 
-asio_link::IntervalTimer::Callback
-AuthSrv::getStatsCallback() {
-    // just invoke statistics::Counter::getStatsCallback()
-    // and return its return value
-    return (counter->getCallback());
-}
+bool AuthSrv::submitStatistics() {
+    return (impl_->counters_.submitStatistics());
+}
+
+const std::vector<uint64_t>&
+AuthSrv::getCounters() const {
+    return (impl_->counters_.getCounters());
+}

Modified: branches/trac347/src/bin/auth/auth_srv.h
==============================================================================
--- branches/trac347/src/bin/auth/auth_srv.h (original)
+++ branches/trac347/src/bin/auth/auth_srv.h Thu Dec  9 09:38:03 2010
@@ -21,12 +21,6 @@
 
 #include <cc/data.h>
 #include <config/ccsession.h>
-
-/// for the typedef of asio_link::IntervalTimer::Callback
-#include <auth/asio_link.h>
-
-// for class statistics::Counter
-#include <auth/stats.h>
 
 namespace isc {
 namespace dns {
@@ -208,25 +202,50 @@
 
     /// \brief Set the communication session with Statistics.
     ///
+    /// This function never throws an exception as far as
+    /// QueryCounters::setStatsSession() doesn't throw.
+    ///
+    /// Note: this interface is tentative.  We'll revisit the ASIO and
+    /// session frameworks, at which point the session will probably
+    /// be passed on construction of the server.
+    ///
+    /// \param stats_session A Session object over which statistics
+    /// information is exchanged with statistics module.
+    /// The session must be established before setting in the server
+    /// object.
+    /// Ownership isn't transferred: the caller is responsible for keeping
+    /// this object to be valid while the server object is working and for
+    /// disconnecting the session and destroying the object when the server
+    /// is shutdown.
+    ///
     void setStatsSession(isc::cc::AbstractSession* stats_session);
 
-    /// \brief Return the function that sends statistics information
-    /// to Statistics module.
+    /// \brief Submit statistics counters to statistics module.
+    ///
+    /// This function can throw an exception from
+    /// QueryCounters::submitStatistics().
+    ///
+    /// \return true on success, false on failure (e.g. session timeout,
+    /// session error).
+    ///
+    bool submitStatistics();
+
+    /// \brief Get counters in the QueryCounters.
     /// 
-    /// This function returns the return value of
-    /// statistics::Counter::getCallback().
-    ///
-    /// \return \c boost::function which contains the procedure
-    /// to send statistics.
-    asio_link::IntervalTimer::Callback getStatsCallback();
+    /// This function calls QueryCounters::getCounters() and
+    /// returns its return velue, a reference to the counters.
+    ///
+    /// This function never throws an exception as far as
+    /// QueryCounters::getCounters() doesn't throw.
+    /// 
+    /// Note: Currently this function is for testing purpose only.
+    /// This function should not be called except from tests.
+    ///
+    /// \return a reference to the counters.
+    ///
+    const std::vector<uint64_t>& getCounters() const;
 private:
     AuthSrvImpl* impl_;
-
-    // TODO: consider where to put the counter.
-    // Currently, count-up is in AuthSrv::processMessage.
-    // In the future, count-up will be in AuthSrvImpl::process*Query
-    // and this declaration will be moved into AuthSrvImpl.
-    statistics::Counter *counter;
 };
 
 #endif // __AUTH_SRV_H

Modified: branches/trac347/src/bin/auth/benchmarks/Makefile.am
==============================================================================
--- branches/trac347/src/bin/auth/benchmarks/Makefile.am (original)
+++ branches/trac347/src/bin/auth/benchmarks/Makefile.am Thu Dec  9 09:38:03 2010
@@ -9,7 +9,7 @@
 noinst_PROGRAMS = query_bench
 query_bench_SOURCES = query_bench.cc
 query_bench_SOURCES += ../auth_srv.h ../auth_srv.cc
-query_bench_SOURCES += ../stats.h ../stats.cc
+query_bench_SOURCES += ../statistics.h ../statistics.cc
 
 query_bench_LDADD = $(top_builddir)/src/lib/dns/libdns++.la
 query_bench_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la

Modified: branches/trac347/src/bin/auth/main.cc
==============================================================================
--- branches/trac347/src/bin/auth/main.cc (original)
+++ branches/trac347/src/bin/auth/main.cc Thu Dec  9 09:38:03 2010
@@ -26,6 +26,7 @@
 #include <iostream>
 
 #include <boost/foreach.hpp>
+#include <boost/bind.hpp>
 
 #include <exceptions/exceptions.h>
 
@@ -89,9 +90,8 @@
         if (verbose_mode) {
             cerr << "[b10-auth] command 'sendstats' received" << endl;
         }
-        if (auth_server != NULL) {
-            auth_server->getStatsCallback()();
-        }
+        assert(auth_server != NULL);
+        auth_server->submitStatistics();
     }
     
     return (answer);
@@ -103,6 +103,12 @@
     exit(1);
 }
 } // end of anonymous namespace
+
+void
+statisticsTimerCallback(AuthSrv* auth_server) {
+    assert(auth_server != NULL);
+    auth_server->submitStatistics();
+}
 
 int
 main(int argc, char* argv[]) {
@@ -240,10 +246,11 @@
         auth_server->updateConfig(ElementPtr());
 
         // create interval timer instance
-        itimer = new asio_link::IntervalTimer(io_service->get_io_service());
+        itimer = new asio_link::IntervalTimer(*io_service);
         // set up interval timer
         // register function to send statistics with interval
-        itimer->setupTimer(auth_server->getStatsCallback(),
+        itimer->setupTimer(boost::bind(statisticsTimerCallback,
+                                       auth_server),
                            STATS_SEND_INTERVAL_SEC);
         cout << "[b10-auth] Interval timer set to send stats." << endl;
 

Modified: branches/trac347/src/bin/auth/tests/Makefile.am
==============================================================================
--- branches/trac347/src/bin/auth/tests/Makefile.am (original)
+++ branches/trac347/src/bin/auth/tests/Makefile.am Thu Dec  9 09:38:03 2010
@@ -22,10 +22,11 @@
 run_unittests_SOURCES += $(top_srcdir)/src/lib/dns/tests/unittest_util.cc
 run_unittests_SOURCES += ../auth_srv.h ../auth_srv.cc
 run_unittests_SOURCES += ../change_user.h ../change_user.cc
-run_unittests_SOURCES += ../stats.h ../stats.cc
+run_unittests_SOURCES += ../statistics.h ../statistics.cc
 run_unittests_SOURCES += auth_srv_unittest.cc
 run_unittests_SOURCES += change_user_unittest.cc
 run_unittests_SOURCES += asio_link_unittest.cc
+run_unittests_SOURCES += statistics_unittest.cc
 run_unittests_SOURCES += run_unittests.cc
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)

Modified: branches/trac347/src/bin/auth/tests/asio_link_unittest.cc
==============================================================================
--- branches/trac347/src/bin/auth/tests/asio_link_unittest.cc (original)
+++ branches/trac347/src/bin/auth/tests/asio_link_unittest.cc Thu Dec  9 09:38:03 2010
@@ -32,6 +32,8 @@
 
 #include <auth/asio_link.h>
 
+#include <boost/date_time/posix_time/posix_time_types.hpp>
+
 using isc::UnitTestUtil;
 using namespace std;
 using namespace asio_link;
@@ -44,6 +46,9 @@
 // two octets encode the length of the rest of the data.  This is crucial
 // for the tests below.
 const uint8_t test_data[] = {0, 4, 1, 2, 3, 4};
+// TODO: Consider this mergin
+const boost::posix_time::time_duration TIMER_MERGIN_MSEC =
+    boost::posix_time::milliseconds(50);
 
 TEST(IOAddressTest, fromText) {
     IOAddress io_address_v4("192.0.2.1");
@@ -252,15 +257,92 @@
                             callback_data_.size(),
                             expected_data, expected_datasize);
     }
-    void doTimerTest(asio_link::IntervalTimer* itimer) {
-        // interval timer test
-        // set test_obj_->timerCallBack() as callback function
-        // and check that the function was called
-        timer_called_ = false;
-        EXPECT_TRUE(itimer->setupTimer(TimerCallBack(this), 1));
-        io_service_->run();
-        EXPECT_TRUE(timer_called_);
-    }
+    class TimerCallBack : public std::unary_function<void, void> {
+    public:
+        TimerCallBack(ASIOLinkTest* test_obj) : test_obj_(test_obj) {}
+        void operator()() const {
+            test_obj_->timer_called_ = true;
+            test_obj_->io_service_->stop();
+            return;
+        }
+    private:
+        ASIOLinkTest* test_obj_;
+    };
+    class TimerCallBackCounter : public std::unary_function<void, void> {
+    public:
+        TimerCallBackCounter(ASIOLinkTest* test_obj) : test_obj_(test_obj) {
+            counter_ = 0;
+        }
+        void operator()() {
+            ++counter_;
+            return;
+        }
+        int counter_;
+    private:
+        ASIOLinkTest* test_obj_;
+    };
+    class TimerCallBackCancelDeleter : public std::unary_function<void, void> {
+    public:
+        TimerCallBackCancelDeleter(ASIOLinkTest* test_obj,
+                                   IntervalTimer* timer,
+                                   TimerCallBackCounter& counter)
+            : test_obj_(test_obj), timer_(timer), counter_(counter), count_(0)
+        {}
+        void operator()() {
+            ++count_;
+            if (count_ == 1) {
+                // First time of call back.
+                // Store the value of counter_.counter_.
+                prev_counter_ = counter_.counter_;
+                delete timer_;
+            } else if (count_ == 2) {
+                // Second time of call back.
+                // Stop io_service to stop all timers.
+                test_obj_->io_service_->stop();
+                // Compare the value of counter_.counter_ with stored one.
+                // If TimerCallBackCounter was not called (expected behavior),
+                // they are same.
+                if (counter_.counter_ == prev_counter_) {
+                    test_obj_->timer_cancel_success_ = true;
+                }
+            }
+            return;
+        }
+    private:
+        ASIOLinkTest* test_obj_;
+        IntervalTimer* timer_;
+        TimerCallBackCounter& counter_;
+        int count_;
+        int prev_counter_;
+    };
+    class TimerCallBackOverwriter : public std::unary_function<void, void> {
+    public:
+        TimerCallBackOverwriter(ASIOLinkTest* test_obj,
+                                IntervalTimer& timer)
+            : test_obj_(test_obj), timer_(timer), count_(0)
+        {}
+        void operator()() {
+            ++count_;
+            if (count_ == 1) {
+                // First time of call back.
+                // Call setupTimer() to update callback function
+                // to TimerCallBack.
+                test_obj_->timer_called_ = false;
+                timer_.setupTimer(IntervalTimer::Callback(
+                                       TimerCallBack(test_obj_)), 1);
+            } else if (count_ == 2) {
+                // Second time of call back.
+                // If it reaches here, re-setupTimer() is failed (unexpected).
+                // We should stop here.
+                test_obj_->io_service_->stop();
+            }
+            return;
+        }
+    private:
+        ASIOLinkTest* test_obj_;
+        IntervalTimer& timer_;
+        int count_;
+    };
 private:
     class ASIOCallBack : public std::unary_function<IOMessage, void> {
     public:
@@ -282,20 +364,6 @@
             io_message.getDataSize());
         io_service_->stop();
     }
-private:
-    class TimerCallBack : public std::unary_function<void, void> {
-    public:
-        TimerCallBack(ASIOLinkTest* test_obj) : test_obj_(test_obj) {}
-        void operator()(void) const {
-            test_obj_->timerCallBack();
-        }
-    private:
-        ASIOLinkTest* test_obj_;
-    };
-    void timerCallBack() {
-        timer_called_ = true;
-        io_service_->stop();
-    }
 protected:
     IOService* io_service_;
     int callback_protocol_;
@@ -303,6 +371,7 @@
     string callback_address_;
     vector<uint8_t> callback_data_;
     bool timer_called_;
+    bool timer_cancel_success_;
     int sock_;
 private:
     struct addrinfo* res_;
@@ -378,13 +447,146 @@
     EXPECT_THROW(sendTCP(AF_INET6), IOError);
 }
 
+TEST_F(ASIOLinkTest, invalidArgumentToIntervalTimer) {
+    // Create asio_link::IntervalTimer and setup.
+    setIOService(false, false);
+    IntervalTimer itimer(*io_service_);
+    // expect throw if call back function is empty
+    EXPECT_THROW(itimer.setupTimer(
+                     IntervalTimer::Callback(), 1),
+                     isc::InvalidParameter);
+    // expect throw if interval is 0
+    EXPECT_THROW(itimer.setupTimer(
+                     IntervalTimer::Callback(TimerCallBack(this)), 0),
+                     isc::BadValue);
+}
+
 TEST_F(ASIOLinkTest, startIntervalTimer) {
     // Create asio_link::IntervalTimer and setup.
     // Then run IOService and test if the callback function is called.
     setIOService(false, false);
-    asio_link::IntervalTimer* itimer =
-        new asio_link::IntervalTimer(io_service_->get_io_service());
-    doTimerTest(itimer);
-    delete itimer;
-}
-}
+    IntervalTimer itimer(*io_service_);
+    timer_called_ = false;
+    // store start time
+    boost::posix_time::ptime start;
+    start = boost::posix_time::microsec_clock::universal_time();
+    // setup timer
+    EXPECT_NO_THROW(itimer.setupTimer(
+                        IntervalTimer::Callback(TimerCallBack(this)),
+                        1));
+    io_service_->run();
+    // reaches here after timer expired
+    // delta: difference between elapsed time and 1 second
+    boost::posix_time::time_duration delta =
+        (boost::posix_time::microsec_clock::universal_time() - start)
+         - boost::posix_time::seconds(1);
+    if (delta.is_negative()) {
+        delta.invert_sign();
+    }
+    // expect call back is updated: TimerCallBack is called
+    EXPECT_TRUE(timer_called_);
+    // expect interval is 1 second +/- TIMER_MERGIN_MSEC.
+    EXPECT_TRUE(delta < TIMER_MERGIN_MSEC);
+}
+
+TEST_F(ASIOLinkTest, destructIntervalTimer) {
+    // The call back function will not be called
+    // after the timer destucted.
+    setIOService(false, false);
+    // There are two timers:
+    //  itimer_counter (A)
+    //   (Calls TimerCallBackCounter)
+    //     - increments internal counter in callback function
+    //  itimer_canceller (B)
+    //   (Calls TimerCallBackCancelDeleter)
+    //     - first time of callback, it stores the counter value of
+    //       callback_canceller and destructs itimer_counter
+    //     - second time of callback, it compares the counter value of
+    //       callback_canceller with stored value
+    //       if they are same the timer was not called; expected result
+    //       if they are different the timer was called after destructed
+    //
+    //     0  1  2  3  4  5  6 (s)
+    // (A) i-----+--x
+    //              ^
+    //              |destruct itimer_counter
+    // (B) i--------+--------s
+    //                       ^stop io_service
+    //                        and test itimer_counter have been stopped
+
+    // itimer_counter will be deleted in
+    // TimerCallBackCancelDeleter
+    IntervalTimer* itimer_counter;
+    ASSERT_NO_THROW(itimer_counter = new IntervalTimer(*io_service_));
+    IntervalTimer itimer_canceller(*io_service_);
+    timer_cancel_success_ = false;
+    TimerCallBackCounter callback_canceller(this);
+    itimer_counter->setupTimer(IntervalTimer::Callback(callback_canceller), 2);
+    itimer_canceller.setupTimer(
+        IntervalTimer::Callback(
+            TimerCallBackCancelDeleter(this, itimer_counter,
+                                       callback_canceller)),
+        3);
+    io_service_->run();
+    EXPECT_TRUE(timer_cancel_success_);
+}
+
+TEST_F(ASIOLinkTest, overwriteIntervalTimer) {
+    // Calling setupTimer() multiple times updates call back
+    // function and interval.
+    setIOService(false, false);
+    // There are two timers:
+    //  itimer (A)
+    //   (Calls TimerCallBackCounter / TimerCallBack)
+    //     - increments internal counter in callback function
+    //       (TimerCallBackCounter)
+    //       interval: 2 seconds
+    //     - io_service_->stop() (TimerCallBack)
+    //       interval: 1 second
+    //  itimer_overwriter (B)
+    //   (Calls TimerCallBackOverwriter)
+    //     - first time of callback, it calls setupTimer() to change
+    //       call back function and interval  of itimer to
+    //       TimerCallBack / 1 second
+    //       after 3 + 1 seconds from the beginning of this test,
+    //       TimerCallBack() will be called and io_service_ stops.
+    //     - second time of callback, it means the test fails.
+    //
+    //     0  1  2  3  4  5  6 (s)
+    // (A) i-----+--C--s
+    //              ^  ^stop io_service
+    //              |change call back function
+    // (B) i--------+--------S
+    //                       ^(stop io_service on fail)
+
+    IntervalTimer itimer(*io_service_);
+    IntervalTimer itimer_overwriter(*io_service_);
+    // store start time
+    boost::posix_time::ptime start;
+    start = boost::posix_time::microsec_clock::universal_time();
+    itimer.setupTimer(IntervalTimer::Callback(TimerCallBackCounter(this)), 2);
+    itimer_overwriter.setupTimer(
+        IntervalTimer::Callback(TimerCallBackOverwriter(this, itimer)), 3);
+    io_service_->run();
+    // reaches here after timer expired
+    // if interval is updated, it takes
+    //   3 seconds for TimerCallBackOverwriter
+    //   + 1 second for TimerCallBack (stop)
+    //   = 4 seconds.
+    // otherwise (test fails), it takes
+    //   3 seconds for TimerCallBackOverwriter
+    //   + 3 seconds for TimerCallBackOverwriter (stop)
+    //   = 6 seconds.
+    // delta: difference between elapsed time and 3 + 1 seconds
+    boost::posix_time::time_duration delta =
+        (boost::posix_time::microsec_clock::universal_time() - start)
+         - boost::posix_time::seconds(3 + 1);
+    if (delta.is_negative()) {
+        delta.invert_sign();
+    }
+    // expect call back is updated: TimerCallBack is called
+    EXPECT_TRUE(timer_called_);
+    // expect interval is updated
+    EXPECT_TRUE(delta < TIMER_MERGIN_MSEC);
+}
+}

Modified: branches/trac347/src/bin/auth/tests/auth_srv_unittest.cc
==============================================================================
--- branches/trac347/src/bin/auth/tests/auth_srv_unittest.cc (original)
+++ branches/trac347/src/bin/auth/tests/auth_srv_unittest.cc Thu Dec  9 09:38:03 2010
@@ -34,6 +34,7 @@
 
 #include <auth/auth_srv.h>
 #include <auth/asio_link.h>
+#include <auth/statistics.h>
 
 #include <dns/tests/unittest_util.h>
 
@@ -44,7 +45,6 @@
 using namespace isc::data;
 using namespace isc::xfr;
 using namespace asio_link;
-using namespace statistics;
 
 namespace {
 const char* const CONFIG_TESTDB =
@@ -121,8 +121,7 @@
                     qclass(RRClass::IN()), qtype(RRType::A()),
                     io_message(NULL), endpoint(NULL), request_obuffer(0),
                     request_renderer(request_obuffer),
-                    response_obuffer(0), response_renderer(response_obuffer),
-                    counter(counter_verbose)
+                    response_obuffer(0), response_renderer(response_obuffer)
     {
         server.setXfrinSession(&notify_session);
         server.setStatsSession(&stats_session);
@@ -149,12 +148,6 @@
     OutputBuffer response_obuffer;
     MessageRenderer response_renderer;
     vector<uint8_t> data;
-    // for Counter unittest
-    // TODO: consider where to put Counter
-    // AuthSrvTest is now includes a test for Counter
-    // In future make a test class CounterTest
-    bool counter_verbose;
-    Counter counter;
 
     void createDataFromFile(const char* const datafile, int protocol);
     void createRequestMessage(const Opcode& opcode, const Name& request_name,
@@ -778,51 +771,21 @@
     EXPECT_EQ(00, server.getCacheSlots());
 }
 
-TEST_F(AuthSrvTest, statsCallback) {
-    // getStatsCallback() test
-    // expect returning a valid function
-    asio_link::IntervalTimer::Callback cbFunc;
-    cbFunc = server.getStatsCallback();
-    EXPECT_FALSE(cbFunc.empty());
-}
-
-TEST_F(AuthSrvTest, sendStatsWithoutSession) {
-    // to cover the code path in case the stats session is not set
-    // expect to put an error message
-    server.setStatsSession(NULL);
-    bool verbose = server.getVerbose();
-    server.setVerbose(true);
-    server.getStatsCallback()();
-    server.setVerbose(verbose);
-}
-
-//
-// statistics::Counter unittest
-
-TEST_F(AuthSrvTest, counter_incUDP) {
-    counter.inc(Counter::COUNTER_UDP);
-}
-
-TEST_F(AuthSrvTest, counter_incTCP) {
-    counter.inc(Counter::COUNTER_TCP);
-}
-
-TEST_F(AuthSrvTest, counter_incUnknown) {
-    EXPECT_THROW(counter.inc(Counter::COUNTER_TYPES), std::out_of_range);
-}
-
-TEST_F(AuthSrvTest, counter_getCallback) {
-    // getCallback() test
-    // expect returning a valid function
-    asio_link::IntervalTimer::Callback cbFunc;
-    cbFunc = counter.getCallback();
-    EXPECT_FALSE(cbFunc.empty());
-}
-
-TEST_F(AuthSrvTest, counter_sendStatsWithSession) {
-    // Test the function to send statistics information to b10-stats
-    // expect to run without throwing any exception
-    counter.setStatsSession(&stats_session);
-    counter.getCallback()();
-}
-}
+TEST_F(AuthSrvTest, queryCounterUDP) {
+    // submit UDP query and check query counter
+    createRequestPacket(opcode, Name("example.com"), RRClass::IN(),
+                        RRType::NS(), IPPROTO_UDP);
+    EXPECT_TRUE(server.processMessage(*io_message, parse_message,
+                                      response_renderer));
+    EXPECT_EQ(1, server.getCounters().at(QueryCounters::COUNTER_UDP));
+}
+
+TEST_F(AuthSrvTest, queryCounterTCP) {
+    // submit TCP query and check query counter
+    createRequestPacket(opcode, Name("example.com"), RRClass::IN(),
+                        RRType::NS(), IPPROTO_TCP);
+    EXPECT_TRUE(server.processMessage(*io_message, parse_message,
+                                      response_renderer));
+    EXPECT_EQ(1, server.getCounters().at(QueryCounters::COUNTER_TCP));
+}
+}




More information about the bind10-changes mailing list