From: Yoshitaka Aharen Date: Mon, 28 Jan 2013 12:36:04 +0000 (+0900) Subject: [2157] use boost::optional for getRequestOpcode() X-Git-Tag: bind10-1.1.0beta1-release~85^2~5^2~32 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=9aaf0f9883c7a67ff2acd8be43afd91e235f7164;p=thirdparty%2Fkea.git [2157] use boost::optional for getRequestOpcode() --- diff --git a/src/bin/auth/statistics.cc.pre b/src/bin/auth/statistics.cc.pre index 63151659a8..d540729f34 100644 --- a/src/bin/auth/statistics.cc.pre +++ b/src/bin/auth/statistics.cc.pre @@ -24,6 +24,8 @@ #include +#include + using namespace isc::dns; using namespace isc::auth; using namespace isc::statistics; @@ -151,14 +153,11 @@ Counters::incRequest(const MessageAttributes& msgattrs) { } // OPCODE - try { - server_msg_counter_.inc( - opcode_to_msgcounter[msgattrs.getRequestOpCode().getCode()]); - } catch (const isc::InvalidOperation& ex) { - // The exception will be thrown if OpCode is not set - // (e.g. message is short). - // Increment MSG_OPCODE_OTHER. - server_msg_counter_.inc(MSG_OPCODE_OTHER); + const boost::optional opcode = + msgattrs.getRequestOpCode(); + // Increment opcode counter only if the opcode exists. + if (opcode) { + server_msg_counter_.inc(opcode_to_msgcounter[opcode.get().getCode()]); } } @@ -198,47 +197,42 @@ Counters::incResponse(const MessageAttributes& msgattrs, server_msg_counter_.inc(MSG_REQUEST_BADEDNSVER); } - try { - if (msgattrs.getRequestOpCode() == Opcode::QUERY()) { - // compound attributes - const unsigned int answer_rrs = - response.getRRCount(Message::SECTION_ANSWER); - const bool is_aa_set = - response.getHeaderFlag(Message::HEADERFLAG_AA); - - if (is_aa_set) { - // QryAuthAns - server_msg_counter_.inc(MSG_QRYAUTHANS); - } else { - // QryNoAuthAns - server_msg_counter_.inc(MSG_QRYNOAUTHANS); - } + const boost::optional opcode = + msgattrs.getRequestOpCode(); + if (opcode && opcode.get() == Opcode::QUERY()) { + // compound attributes + const unsigned int answer_rrs = + response.getRRCount(Message::SECTION_ANSWER); + const bool is_aa_set = + response.getHeaderFlag(Message::HEADERFLAG_AA); + + if (is_aa_set) { + // QryAuthAns + server_msg_counter_.inc(MSG_QRYAUTHANS); + } else { + // QryNoAuthAns + server_msg_counter_.inc(MSG_QRYNOAUTHANS); + } - if (rcode == Rcode::NOERROR_CODE) { - if (answer_rrs > 0) { - // QrySuccess - server_msg_counter_.inc(MSG_QRYSUCCESS); + if (rcode == Rcode::NOERROR_CODE) { + if (answer_rrs > 0) { + // QrySuccess + server_msg_counter_.inc(MSG_QRYSUCCESS); + } else { + if (is_aa_set) { + // QryNxrrset + server_msg_counter_.inc(MSG_QRYNXRRSET); } else { - if (is_aa_set) { - // QryNxrrset - server_msg_counter_.inc(MSG_QRYNXRRSET); - } else { - // QryReferral - server_msg_counter_.inc(MSG_QRYREFERRAL); - } - } - } else if (rcode == Rcode::REFUSED_CODE) { - if (!response.getHeaderFlag(Message::HEADERFLAG_RD)) { - // AuthRej - server_msg_counter_.inc(MSG_QRYREJECT); + // QryReferral + server_msg_counter_.inc(MSG_QRYREFERRAL); } } + } else if (rcode == Rcode::REFUSED_CODE) { + if (!response.getHeaderFlag(Message::HEADERFLAG_RD)) { + // AuthRej + server_msg_counter_.inc(MSG_QRYREJECT); + } } - } catch (const isc::InvalidOperation& ex) { - // The exception will be thrown if OpCode is not set - // (e.g. message is short). - // Do not increment the counters above since they are incremented - // only if Opcode is Query. } } diff --git a/src/bin/auth/statistics.h b/src/bin/auth/statistics.h index 49be6a2907..b0e3f6eb63 100644 --- a/src/bin/auth/statistics.h +++ b/src/bin/auth/statistics.h @@ -77,13 +77,13 @@ public: {} /// \brief Return opcode of the request. - /// \return opcode of the request + /// \return opcode of the request wrapped with boost::optional /// \throw isc::InvalidOperation Opcode is not set - const isc::dns::Opcode& getRequestOpCode() const { + const boost::optional getRequestOpCode() const { if (req_opcode_) { return (req_opcode_.get()); } else { - isc_throw(isc::InvalidOperation, "Opcode is not set"); + return boost::none; } } diff --git a/src/bin/auth/tests/auth_srv_unittest.cc b/src/bin/auth/tests/auth_srv_unittest.cc index f6d357cb00..9bcf7c616d 100644 --- a/src/bin/auth/tests/auth_srv_unittest.cc +++ b/src/bin/auth/tests/auth_srv_unittest.cc @@ -295,7 +295,6 @@ TEST_F(AuthSrvTest, shortMessage) { std::map expect; expect["request.v4"] = 1; expect["request.udp"] = 1; - expect["opcode.other"] = 1; checkStatisticsCounters(stats_after, expect); } @@ -309,7 +308,6 @@ TEST_F(AuthSrvTest, response) { std::map expect; expect["request.v4"] = 3; expect["request.udp"] = 3; - expect["opcode.other"] = 3; checkStatisticsCounters(stats_after, expect); } @@ -335,7 +333,6 @@ TEST_F(AuthSrvTest, ednsBadVers) { expect["request.v4"] = 1; expect["request.badednsver"] = 1; expect["request.udp"] = 1; - expect["opcode.other"] = 1; expect["responses"] = 1; expect["rcode.badvers"] = 1; checkStatisticsCounters(stats_after, expect); diff --git a/src/bin/auth/tests/statistics_unittest.cc.pre b/src/bin/auth/tests/statistics_unittest.cc.pre index d602ba7bfb..45b75953fb 100644 --- a/src/bin/auth/tests/statistics_unittest.cc.pre +++ b/src/bin/auth/tests/statistics_unittest.cc.pre @@ -80,13 +80,13 @@ TEST_F(CountersTest, invalidParameter) { TEST_F(CountersTest, invalidOperationForGetRequestOpCode) { MessageAttributes msgattrs; - // getRequestOpCode() will throw isc::InvalidOperation when called before + // getRequestOpCode() should return boost::none when called before // opcode is set with setRequestOpCode(). - EXPECT_THROW(msgattrs.getRequestOpCode(), isc::InvalidOperation); + EXPECT_FALSE(msgattrs.getRequestOpCode()); msgattrs.setRequestOpCode(Opcode::QUERY()); - // getRequestOpCode() will not throw at this point. - EXPECT_NO_THROW(msgattrs.getRequestOpCode()); + // getRequestOpCode() should be Opcode::QUERY. + EXPECT_EQ(Opcode::QUERY(), msgattrs.getRequestOpCode().get()); } TEST_F(CountersTest, incrementResponse) {