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.



git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac347@4015 e5f2f494-b856-4b98-b285-d166d9295462

src/bin/auth/asio_link.cc

@@ -722,7 +722,7 @@ IntervalTimerImpl::updateTimer() {
     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.

src/bin/auth/asio_link.h

@@ -461,6 +461,9 @@ private:
 /// 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 @@ public:
 
     /// \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

src/bin/auth/auth_srv.cc

@@ -500,9 +500,9 @@ void
 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 @@ bool AuthSrv::submitStatistics() const {
 }
 
 uint64_t
-AuthSrv::getCounter(const AuthCounters::QueryType type) const {
+AuthSrv::getCounter(const AuthCounters::CounterType type) const {
     return (impl_->counters_.getCounter(type));
 }

src/bin/auth/auth_srv.h

@@ -249,7 +249,7 @@ public:
     ///
     /// \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_;
 };

src/bin/auth/statistics.cc

@@ -36,11 +36,11 @@ public:
     // 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 @@ AuthCountersImpl::~AuthCountersImpl()
 {}
 
 void
-AuthCountersImpl::inc(const AuthCounters::QueryType type) {
+AuthCountersImpl::inc(const AuthCounters::CounterType type) {
     ++counters_.at(type);
 }
 
@@ -77,9 +77,9 @@ AuthCountersImpl::submitStatistics() const {
     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 @@ AuthCountersImpl::setStatisticsSession
 
 // 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 @@ AuthCounters::~AuthCounters() {
 }
 
 void
-AuthCounters::inc(const AuthCounters::QueryType type) {
+AuthCounters::inc(const AuthCounters::CounterType type) {
     impl_->inc(type);
 }
 
@@ -157,6 +157,6 @@ AuthCounters::setStatisticsSession
 }
 
 uint64_t
-AuthCounters::getCounter(const AuthCounters::QueryType type) const {
+AuthCounters::getCounter(const AuthCounters::CounterType type) const {
     return (impl_->getCounter(type));
 }

src/bin/auth/statistics.h

@@ -34,7 +34,7 @@ class AuthCountersImpl;
 /// 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 @@ private:
     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 @@ public:
     ///
     /// \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 @@ public:
     ///
     /// \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

src/bin/auth/tests/asio_link_unittest.cc

@@ -492,23 +492,6 @@ TEST_F(IntervalTimerTest, startIntervalTimer) {
     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

src/bin/auth/tests/auth_srv_unittest.cc

@@ -774,38 +774,38 @@ TEST_F(AuthSrvTest, cacheSlots) {
 // 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 @@ getDummyUnknownSocket() {
     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);

src/bin/auth/tests/statistics_unittest.cc

@@ -144,18 +144,18 @@ AuthCountersTest::MockSession::setThrowSessionTimeout(bool flag) {
 
 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 @@ TEST_F(AuthCountersTest, submitStatistics) {
     // 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".