[svn] commit: r4015 - in /branches/trac347/src/bin/auth: asio_link.cc asio_link.h auth_srv.cc auth_srv.h statistics.cc statistics.h tests/asio_link_unittest.cc tests/auth_srv_unittest.cc tests/statistics_unittest.cc

BIND 10 source code commits bind10-changes at lists.isc.org
Mon Dec 27 04:34:02 UTC 2010


Author: y-aharen
Date: Mon Dec 27 04:34:02 2010
New Revision: 4015

Log:
Address review comments on [comment:ticket:347:14]

- enum QueryType was renamed to enum CounterType.
- COUNTER_{UDP,TCP} were renamed to COUNTER_{UDP,TCP}_QUERY.
- Catch an exception by const reference.
- Documentation and comments were appended.
- Removed unnecessary test; the result is obvious.


Modified:
    branches/trac347/src/bin/auth/asio_link.cc
    branches/trac347/src/bin/auth/asio_link.h
    branches/trac347/src/bin/auth/auth_srv.cc
    branches/trac347/src/bin/auth/auth_srv.h
    branches/trac347/src/bin/auth/statistics.cc
    branches/trac347/src/bin/auth/statistics.h
    branches/trac347/src/bin/auth/tests/asio_link_unittest.cc
    branches/trac347/src/bin/auth/tests/auth_srv_unittest.cc
    branches/trac347/src/bin/auth/tests/statistics_unittest.cc

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 Mon Dec 27 04:34:02 2010
@@ -722,7 +722,7 @@
     try {
         // Update expire time to (current time + interval_).
         timer_.expires_from_now(boost::posix_time::seconds(interval_));
-    } catch (asio::system_error& e) {
+    } catch (const asio::system_error& e) {
         isc_throw(isc::Unexpected, "Failed to update timer");
     }
     // Reset timer.

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 Mon Dec 27 04:34:02 2010
@@ -461,6 +461,9 @@
 /// The function calls the call back function set by \c setupTimer()
 /// and updates the timer to expire in (now + interval) seconds.
 /// The type of call back function is \c void(void).
+/// 
+/// The call back function will not be called if the instance of this
+/// class is destructed before the timer is expired.
 ///
 /// Note: Destruction of an instance of this class while call back
 /// is pending causes throwing an exception from \c IOService.
@@ -515,7 +518,7 @@
 
     /// \brief Register timer callback function and interval.
     ///
-    /// This function sets call back function and interval in seconds.
+    /// This function sets callback 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

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 Mon Dec 27 04:34:02 2010
@@ -500,9 +500,9 @@
 AuthSrvImpl::incCounter(int protocol) {
     // Increment query counter.
     if (protocol == IPPROTO_UDP) {
-        counters_.inc(AuthCounters::COUNTER_UDP);
+        counters_.inc(AuthCounters::COUNTER_UDP_QUERY);
     } else if (protocol == IPPROTO_TCP) {
-        counters_.inc(AuthCounters::COUNTER_TCP);
+        counters_.inc(AuthCounters::COUNTER_TCP_QUERY);
     } else {
         // unknown protocol
         isc_throw(Unexpected, "Unknown protocol: " << protocol);
@@ -581,6 +581,6 @@
 }
 
 uint64_t
-AuthSrv::getCounter(const AuthCounters::QueryType type) const {
+AuthSrv::getCounter(const AuthCounters::CounterType type) const {
     return (impl_->counters_.getCounter(type));
 }

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 Mon Dec 27 04:34:02 2010
@@ -249,7 +249,7 @@
     ///
     /// \return the value of the counter.
     ///
-    uint64_t getCounter(const AuthCounters::QueryType type) const;
+    uint64_t getCounter(const AuthCounters::CounterType type) const;
 private:
     AuthSrvImpl* impl_;
 };

Modified: branches/trac347/src/bin/auth/statistics.cc
==============================================================================
--- branches/trac347/src/bin/auth/statistics.cc (original)
+++ branches/trac347/src/bin/auth/statistics.cc Mon Dec 27 04:34:02 2010
@@ -36,11 +36,11 @@
     // after we have logging framework
     AuthCountersImpl(const bool& verbose_mode);
     ~AuthCountersImpl();
-    void inc(const AuthCounters::QueryType type);
+    void inc(const AuthCounters::CounterType type);
     bool submitStatistics() const;
     void setStatisticsSession(isc::cc::AbstractSession* statistics_session);
     // Currently for testing purpose only
-    uint64_t getCounter(const AuthCounters::QueryType type) const;
+    uint64_t getCounter(const AuthCounters::CounterType type) const;
 private:
     std::vector<uint64_t> counters_;
     isc::cc::AbstractSession* statistics_session_;
@@ -59,7 +59,7 @@
 {}
 
 void
-AuthCountersImpl::inc(const AuthCounters::QueryType type) {
+AuthCountersImpl::inc(const AuthCounters::CounterType type) {
     ++counters_.at(type);
 }
 
@@ -77,9 +77,9 @@
     statistics_string << "{\"command\": [\"set\","
                       <<   "{ \"stats_data\": "
                       <<     "{ \"auth.queries.udp\": "
-                      <<     counters_.at(AuthCounters::COUNTER_UDP)
+                      <<     counters_.at(AuthCounters::COUNTER_UDP_QUERY)
                       <<     ", \"auth.queries.tcp\": "
-                      <<     counters_.at(AuthCounters::COUNTER_TCP)
+                      <<     counters_.at(AuthCounters::COUNTER_TCP_QUERY)
                       <<   " }"
                       <<   "}"
                       << "]}";
@@ -127,7 +127,7 @@
 
 // Currently for testing purpose only
 uint64_t
-AuthCountersImpl::getCounter(const AuthCounters::QueryType type) const {
+AuthCountersImpl::getCounter(const AuthCounters::CounterType type) const {
     return (counters_.at(type));
 }
 
@@ -140,7 +140,7 @@
 }
 
 void
-AuthCounters::inc(const AuthCounters::QueryType type) {
+AuthCounters::inc(const AuthCounters::CounterType type) {
     impl_->inc(type);
 }
 
@@ -157,6 +157,6 @@
 }
 
 uint64_t
-AuthCounters::getCounter(const AuthCounters::QueryType type) const {
+AuthCounters::getCounter(const AuthCounters::CounterType type) const {
     return (impl_->getCounter(type));
 }

Modified: branches/trac347/src/bin/auth/statistics.h
==============================================================================
--- branches/trac347/src/bin/auth/statistics.h (original)
+++ branches/trac347/src/bin/auth/statistics.h Mon Dec 27 04:34:02 2010
@@ -34,7 +34,7 @@
 /// Call \c setStatisticsSession() to set a session to communicate with
 /// statistics module like Xfrin session.
 /// Call \c inc() to increment a counter for specific type of query in
-/// the query processing function. use \c enum \c QueryType to specify
+/// the query processing function. use \c enum \c CounterType to specify
 /// the type of query.
 /// Call \c submitStatistics() to submit statistics information to statistics
 /// module with statistics_session, periodically or at a time the command
@@ -55,9 +55,9 @@
     AuthCountersImpl* impl_;
 public:
     // Enum for the type of counter
-    enum QueryType {
-        COUNTER_UDP = 0,  ///< COUNTER_UDP: counter for UDP queries
-        COUNTER_TCP = 1,  ///< COUNTER_TCP: counter for TCP queries
+    enum CounterType {
+        COUNTER_UDP_QUERY = 0,  ///< COUNTER_UDP_QUERY: counter for UDP queries
+        COUNTER_TCP_QUERY = 1,  ///< COUNTER_TCP_QUERY: counter for TCP queries
         COUNTER_TYPES = 2 ///< The number of defined counters
     };
     /// The constructor.
@@ -83,9 +83,9 @@
     ///
     /// \throw std::out_of_range \a type is unknown.
     ///
-    /// usage: counter.inc(QueryType::COUNTER_UDP);
+    /// usage: counter.inc(CounterType::COUNTER_UDP_QUERY);
     /// 
-    void inc(const QueryType type);
+    void inc(const CounterType type);
 
     /// \brief Submit statistics counters to statistics module.
     ///
@@ -136,7 +136,7 @@
     ///
     /// \return the value of the counter specified by \a type.
     ///
-    uint64_t getCounter(const AuthCounters::QueryType type) const;
+    uint64_t getCounter(const AuthCounters::CounterType type) const;
 };
 
 #endif // __STATISTICS_H

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 Mon Dec 27 04:34:02 2010
@@ -492,23 +492,6 @@
     EXPECT_TRUE(delta < TIMER_MERGIN_MSEC);
 }
 
-TEST_F(IntervalTimerTest, deleteIntervalTimerBeforeStart) {
-    // Note: This code isn't exception safe, but we'd rather keep the code
-    // simpler and more readable as this is only for tests and if it throws
-    // the program would immediately terminate anyway.
-
-    // Create asio_link::IntervalTimer and delete before starting timer.
-    // Test if the callback function is not called.
-    IntervalTimer* itimer = new IntervalTimer(io_service_);
-    timer_called_ = false;
-    // setup timer...
-    itimer->setupTimer(TimerCallBack(this), 1);
-    // and delete
-    delete itimer;
-    // expect the callback function is not called
-    EXPECT_FALSE(timer_called_);
-}
-
 TEST_F(IntervalTimerTest, destructIntervalTimer) {
     // Note: This test currently takes 6 seconds. The timer should have
     // finer granularity and timer periods in this test should be shorter

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 Mon Dec 27 04:34:02 2010
@@ -774,38 +774,38 @@
 // Submit UDP normal query and check query counter
 TEST_F(AuthSrvTest, queryCounterUDPNormal) {
     // The counter should be initialized to 0.
-    EXPECT_EQ(0, server.getCounter(AuthCounters::COUNTER_UDP));
+    EXPECT_EQ(0, server.getCounter(AuthCounters::COUNTER_UDP_QUERY));
     createRequestPacket(opcode, Name("example.com"), RRClass::IN(),
                         RRType::NS(), IPPROTO_UDP);
     EXPECT_TRUE(server.processMessage(*io_message, parse_message,
                                       response_renderer));
     // After processing UDP query, the counter should be 1.
-    EXPECT_EQ(1, server.getCounter(AuthCounters::COUNTER_UDP));
+    EXPECT_EQ(1, server.getCounter(AuthCounters::COUNTER_UDP_QUERY));
 }
 
 // Submit TCP normal query and check query counter
 TEST_F(AuthSrvTest, queryCounterTCPNormal) {
     // The counter should be initialized to 0.
-    EXPECT_EQ(0, server.getCounter(AuthCounters::COUNTER_TCP));
+    EXPECT_EQ(0, server.getCounter(AuthCounters::COUNTER_TCP_QUERY));
     createRequestPacket(opcode, Name("example.com"), RRClass::IN(),
                         RRType::NS(), IPPROTO_TCP);
     EXPECT_TRUE(server.processMessage(*io_message, parse_message,
                                       response_renderer));
     // After processing TCP query, the counter should be 1.
-    EXPECT_EQ(1, server.getCounter(AuthCounters::COUNTER_TCP));
+    EXPECT_EQ(1, server.getCounter(AuthCounters::COUNTER_TCP_QUERY));
 }
 
 // Submit TCP AXFR query and check query counter
 TEST_F(AuthSrvTest, queryCounterTCPAXFR) {
     // The counter should be initialized to 0.
-    EXPECT_EQ(0, server.getCounter(AuthCounters::COUNTER_TCP));
+    EXPECT_EQ(0, server.getCounter(AuthCounters::COUNTER_TCP_QUERY));
     createRequestPacket(opcode, Name("example.com"), RRClass::IN(),
                         RRType::AXFR(), IPPROTO_TCP);
     // It returns false. see AXFRSuccess test.
     EXPECT_FALSE(server.processMessage(*io_message, parse_message,
                                       response_renderer));
     // After processing TCP AXFR query, the counter should be 1.
-    EXPECT_EQ(1, server.getCounter(AuthCounters::COUNTER_TCP));
+    EXPECT_EQ(1, server.getCounter(AuthCounters::COUNTER_TCP_QUERY));
 }
 
 // class for queryCounterUnexpected test
@@ -825,8 +825,12 @@
     return (socket);
 }
 
-// Submit unexpected type of query and check it throws IPPROTO_IP
+// Submit unexpected type of query and check it throws isc::Unexpected
 TEST_F(AuthSrvTest, queryCounterUnexpected) {
+    // This code isn't exception safe, but we'd rather keep the code
+    // simpler and more readable as this is only for tests and if it throws
+    // the program would immediately terminate anyway.
+
     // Create UDP query packet.
     createRequestPacket(opcode, Name("example.com"), RRClass::IN(),
                         RRType::NS(), IPPROTO_UDP);

Modified: branches/trac347/src/bin/auth/tests/statistics_unittest.cc
==============================================================================
--- branches/trac347/src/bin/auth/tests/statistics_unittest.cc (original)
+++ branches/trac347/src/bin/auth/tests/statistics_unittest.cc Mon Dec 27 04:34:02 2010
@@ -144,18 +144,18 @@
 
 TEST_F(AuthCountersTest, incrementUDPCounter) {
     // The counter should be initialized to 0.
-    EXPECT_EQ(0, counters.getCounter(AuthCounters::COUNTER_UDP));
-    EXPECT_NO_THROW(counters.inc(AuthCounters::COUNTER_UDP));
+    EXPECT_EQ(0, counters.getCounter(AuthCounters::COUNTER_UDP_QUERY));
+    EXPECT_NO_THROW(counters.inc(AuthCounters::COUNTER_UDP_QUERY));
     // After increment, the counter should be 1.
-    EXPECT_EQ(1, counters.getCounter(AuthCounters::COUNTER_UDP));
+    EXPECT_EQ(1, counters.getCounter(AuthCounters::COUNTER_UDP_QUERY));
 }
 
 TEST_F(AuthCountersTest, incrementTCPCounter) {
     // The counter should be initialized to 0.
-    EXPECT_EQ(0, counters.getCounter(AuthCounters::COUNTER_TCP));
-    EXPECT_NO_THROW(counters.inc(AuthCounters::COUNTER_TCP));
+    EXPECT_EQ(0, counters.getCounter(AuthCounters::COUNTER_TCP_QUERY));
+    EXPECT_NO_THROW(counters.inc(AuthCounters::COUNTER_TCP_QUERY));
     // After increment, the counter should be 1.
-    EXPECT_EQ(1, counters.getCounter(AuthCounters::COUNTER_TCP));
+    EXPECT_EQ(1, counters.getCounter(AuthCounters::COUNTER_TCP_QUERY));
 }
 
 TEST_F(AuthCountersTest, incrementInvalidCounter) {
@@ -189,14 +189,14 @@
     // Validate if it submits correct data.
 
     // Counters should be initialized to 0.
-    EXPECT_EQ(0, counters.getCounter(AuthCounters::COUNTER_UDP));
-    EXPECT_EQ(0, counters.getCounter(AuthCounters::COUNTER_TCP));
+    EXPECT_EQ(0, counters.getCounter(AuthCounters::COUNTER_UDP_QUERY));
+    EXPECT_EQ(0, counters.getCounter(AuthCounters::COUNTER_TCP_QUERY));
 
     // UDP query counter is set to 2.
-    counters.inc(AuthCounters::COUNTER_UDP);
-    counters.inc(AuthCounters::COUNTER_UDP);
+    counters.inc(AuthCounters::COUNTER_UDP_QUERY);
+    counters.inc(AuthCounters::COUNTER_UDP_QUERY);
     // TCP query counter is set to 1.
-    counters.inc(AuthCounters::COUNTER_TCP);
+    counters.inc(AuthCounters::COUNTER_TCP_QUERY);
     counters.submitStatistics();
 
     // Destination is "Stats".




More information about the bind10-changes mailing list