[svn] commit: r1368 - in /trunk/src: bin/auth/auth_srv.cc bin/auth/main.cc lib/config/ccsession.cc lib/config/ccsession.h lib/config/tests/ccsession_unittests.cc
BIND 10 source code commits
bind10-changes at lists.isc.org
Fri Mar 12 17:00:22 UTC 2010
Author: jelte
Date: Fri Mar 12 17:00:22 2010
New Revision: 1368
Log:
doxygen for createAnswer and family of helper functions
added sanity checks within those functions (and tests for that)
changes createAnswer(int rcode) to createAnswer() since rcode should only be 0 if there is not argument
Modified:
trunk/src/bin/auth/auth_srv.cc
trunk/src/bin/auth/main.cc
trunk/src/lib/config/ccsession.cc
trunk/src/lib/config/ccsession.h
trunk/src/lib/config/tests/ccsession_unittests.cc
Modified: trunk/src/bin/auth/auth_srv.cc
==============================================================================
--- trunk/src/bin/auth/auth_srv.cc (original)
+++ trunk/src/bin/auth/auth_srv.cc Fri Mar 12 17:00:22 2010
@@ -248,7 +248,7 @@
// delete and swap operations which should not fail.
DataSrcPtr datasrc_ptr(DataSrcPtr(new Sqlite3DataSrc));
datasrc_ptr->init(config);
- ElementPtr answer = isc::config::createAnswer(0);
+ ElementPtr answer = isc::config::createAnswer();
data_sources_.addDataSrc(datasrc_ptr);
// The following code should be exception free.
@@ -263,7 +263,7 @@
ElementPtr
AuthSrv::updateConfig(isc::data::ElementPtr new_config) {
try {
- ElementPtr answer = isc::config::createAnswer(0);
+ ElementPtr answer = isc::config::createAnswer();
if (new_config != NULL) {
// the ModuleCCSession has already checked if we have
// the correct ElementPtr type as specified in our .spec file
Modified: trunk/src/bin/auth/main.cc
==============================================================================
--- trunk/src/bin/auth/main.cc (original)
+++ trunk/src/bin/auth/main.cc Fri Mar 12 17:00:22 2010
@@ -90,7 +90,7 @@
static ElementPtr
my_command_handler(const string& command, const ElementPtr args) {
- ElementPtr answer = createAnswer(0);
+ ElementPtr answer = createAnswer();
if (command == "print_message") {
cout << args << endl;
Modified: trunk/src/lib/config/ccsession.cc
==============================================================================
--- trunk/src/lib/config/ccsession.cc (original)
+++ trunk/src/lib/config/ccsession.cc Fri Mar 12 17:00:22 2010
@@ -56,17 +56,20 @@
/// Creates a standard config/command protocol answer message
ElementPtr
-createAnswer(const int rcode)
+createAnswer()
{
ElementPtr answer = Element::createFromString("{\"result\": [] }");
ElementPtr answer_content = answer->get("result");
- answer_content->add(Element::create(rcode));
+ answer_content->add(Element::create(0));
return answer;
}
ElementPtr
createAnswer(const int rcode, const ElementPtr arg)
{
+ if (rcode != 0 && (!arg || arg->getType() != Element::string)) {
+ isc_throw(CCSessionError, "Bad or no argument for rcode != 0");
+ }
ElementPtr answer = Element::createFromString("{\"result\": [] }");
ElementPtr answer_content = answer->get("result");
answer_content->add(Element::create(rcode));
@@ -87,23 +90,38 @@
ElementPtr
parseAnswer(int &rcode, const ElementPtr msg)
{
- if (!msg->contains("result")) {
- // TODO: raise CCSessionError exception
- isc_throw(CCSessionError, "No result in answer message");
- } else {
+ if (msg &&
+ msg->getType() == Element::map &&
+ msg->contains("result")) {
ElementPtr result = msg->get("result");
- if (result->get(0)->getType() != Element::integer) {
+ if (result->getType() != Element::list) {
+ isc_throw(CCSessionError, "Result element in answer message is not a list");
+ } else if (result->get(0)->getType() != Element::integer) {
isc_throw(CCSessionError, "First element of result is not an rcode in answer message");
- } else if (result->get(0)->intValue() != 0 && result->get(1)->getType() != Element::string) {
- isc_throw(CCSessionError, "Rcode in answer message is non-zero, but other argument is not a StringElement");
}
rcode = result->get(0)->intValue();
if (result->size() > 1) {
- return result->get(1);
+ if (rcode == 0 || result->get(1)->getType() == Element::string) {
+ return result->get(1);
+ } else {
+ isc_throw(CCSessionError, "Error description in result with rcode != 0 is not a string");
+ }
} else {
- return ElementPtr();
- }
- }
+ if (rcode == 0) {
+ return ElementPtr();
+ } else {
+ isc_throw(CCSessionError, "Result with rcode != 0 does not have an error description");
+ }
+ }
+ } else {
+ isc_throw(CCSessionError, "No result part in answer message");
+ }
+}
+
+ElementPtr
+createCommand(const std::string& command)
+{
+ return createCommand(command, ElementPtr());
}
ElementPtr
@@ -124,7 +142,8 @@
const std::string
parseCommand(ElementPtr& arg, const ElementPtr command)
{
- if (command->getType() == Element::map &&
+ if (command &&
+ command->getType() == Element::map &&
command->contains("command")) {
ElementPtr cmd = command->get("command");
if (cmd->getType() == Element::list &&
@@ -136,10 +155,12 @@
arg = ElementPtr();
}
return cmd->get(0)->stringValue();
- }
- }
- arg = ElementPtr();
- return "";
+ } else {
+ isc_throw(CCSessionError, "Command part in command message missing, empty, or not a list");
+ }
+ } else {
+ isc_throw(CCSessionError, "Command Element empty or not a map with \"command\"");
+ }
}
ModuleSpec
@@ -302,29 +323,33 @@
/* ignore result messages (in case we're out of sync, to prevent
* pingpongs */
- if (!data->getType() == Element::map || data->contains("result")) {
+ if (data->getType() != Element::map || data->contains("result")) {
return 0;
}
ElementPtr arg;
- std::string cmd_str = parseCommand(arg, data);
ElementPtr answer;
- if (cmd_str == "config_update") {
- std::string target_module = routing->get("group")->stringValue();
- if (target_module == module_name_) {
- answer = handleConfigUpdate(arg);
+ try {
+ std::string cmd_str = parseCommand(arg, data);
+ if (cmd_str == "config_update") {
+ std::string target_module = routing->get("group")->stringValue();
+ if (target_module == module_name_) {
+ answer = handleConfigUpdate(arg);
+ } else {
+ // ok this update is not for us, if we have this module
+ // in our remote config list, update that
+ updateRemoteConfig(target_module, arg);
+ // we're not supposed to answer to this, so return
+ return 0;
+ }
} else {
- // ok this update is not for us, if we have this module
- // in our remote config list, update that
- updateRemoteConfig(target_module, arg);
- // we're not supposed to answer to this, so return
- return 0;
+ if (command_handler_) {
+ answer = command_handler_(cmd_str, arg);
+ } else {
+ answer = createAnswer(1, "Command given but no command handler for module");
+ }
}
- } else {
- if (command_handler_) {
- answer = command_handler_(cmd_str, arg);
- } else {
- answer = createAnswer(1, "Command given but no command handler for module");
- }
+ } catch (CCSessionError re) {
+ answer = createAnswer(1, re.what());
}
session_.reply(routing, answer);
}
Modified: trunk/src/lib/config/ccsession.h
==============================================================================
--- trunk/src/lib/config/ccsession.h (original)
+++ trunk/src/lib/config/ccsession.h Fri Mar 12 17:00:22 2010
@@ -33,12 +33,72 @@
namespace isc {
namespace config {
-ElementPtr createAnswer(const int rcode);
+///
+/// \brief Creates a standard config/command level success answer message
+/// (i.e. of the form { "result": [ 0 ] }
+/// \return Standard command/config success answer message
+ElementPtr createAnswer();
+
+///
+/// \brief Creates a standard config/command level answer message
+/// (i.e. of the form { "result": [ rcode, arg ] }
+/// If rcode != 0, arg must be a StringElement
+///
+/// \param rcode The return code (0 for success)
+/// \param arg For rcode == 0, this is an optional argument of any
+/// Element type. For rcode == 1, this argument is mandatory,
+/// and must be a StringElement containing an error description
+/// \return Standard command/config answer message
ElementPtr createAnswer(const int rcode, const ElementPtr arg);
+
+///
+/// \brief Creates a standard config/command level answer message
+/// (i.e. of the form { "result": [ rcode, arg ] }
+///
+/// \param rcode The return code (0 for success)
+/// \param arg A string to put into the StringElement argument
+/// \return Standard command/config answer message
ElementPtr createAnswer(const int rcode, const std::string& arg);
+
+///
+/// Parses a standard config/command level answer message
+///
+/// \param rcode This value will be set to the return code contained in
+/// the message
+/// \param msg The message to parse
+/// \return The optional argument in the message, or an empty ElementPtr
+/// if there was no argument. If rcode != 0, this contains a
+/// StringElement with the error description.
ElementPtr parseAnswer(int &rcode, const ElementPtr msg);
+
+///
+/// \brief Creates a standard config/command command message with no
+/// argument (of the form { "command": [ "my_command" ] }
+///
+/// \param command The command string
+/// \return The created message
+ElementPtr createCommand(const std::string& command);
+
+///
+/// \brief Creates a standard config/command command message with the
+/// given argument (of the form { "command": [ "my_command", arg ] }
+///
+/// \param command The command string
+/// \param arg The optional argument for the command. This can be of
+/// any Element type, but it should conform to the .spec file.
+/// \return The created message
ElementPtr createCommand(const std::string& command, ElementPtr arg);
+
+///
+/// \brief Parses the given command into a string containing the actual
+/// command and an ElementPtr containing the optional argument.
+///
+/// \param arg This value will be set to the ElementPtr pointing to
+/// the argument, or to an empty ElementPtr if there was none.
+/// \param command The command message containing the command (as made
+/// by createCommand()
+/// \return The command string
const std::string parseCommand(ElementPtr& arg, const ElementPtr command);
Modified: trunk/src/lib/config/tests/ccsession_unittests.cc
==============================================================================
--- trunk/src/lib/config/tests/ccsession_unittests.cc (original)
+++ trunk/src/lib/config/tests/ccsession_unittests.cc Fri Mar 12 17:00:22 2010
@@ -49,7 +49,7 @@
initial_messages = el("[]");
msg_queue = el("[]");
subscriptions = el("[]");
- initial_messages->add(createAnswer(0));
+ initial_messages->add(createAnswer());
}
void
@@ -63,14 +63,13 @@
TEST(CCSession, createAnswer)
{
ElementPtr answer;
- // todo: this should fail (actually int rcode should probably be removed)
- answer = createAnswer(1);
- answer = createAnswer(0);
+ answer = createAnswer();
EXPECT_EQ("{\"result\": [ 0 ]}", answer->str());
answer = createAnswer(1, "error");
EXPECT_EQ("{\"result\": [ 1, \"error\" ]}", answer->str());
- // todo: this should fail
- answer = createAnswer(0, "error");
+
+ EXPECT_THROW(createAnswer(1, ElementPtr()), CCSessionError);
+ EXPECT_THROW(createAnswer(1, Element::create(1)), CCSessionError);
ElementPtr arg = el("[ \"just\", \"some\", \"data\" ]");
answer = createAnswer(0, arg);
@@ -83,20 +82,21 @@
ElementPtr arg;
int rcode;
- // todo: these should throw CCSessionError
- //answer = el("1");
- //arg = parseAnswer(rcode, ElementPtr());
- //arg = parseAnswer(rcode, answer);
- // etc
+ EXPECT_THROW(parseAnswer(rcode, ElementPtr()), CCSessionError);
+ EXPECT_THROW(parseAnswer(rcode, el("1")), CCSessionError);
+ EXPECT_THROW(parseAnswer(rcode, el("[]")), CCSessionError);
+ EXPECT_THROW(parseAnswer(rcode, el("{}")), CCSessionError);
+ EXPECT_THROW(parseAnswer(rcode, el("{ \"something\": 1 }")), CCSessionError);
+ EXPECT_THROW(parseAnswer(rcode, el("{ \"result\": 0 }")), CCSessionError);
+ EXPECT_THROW(parseAnswer(rcode, el("{ \"result\": 1 }")), CCSessionError);
+ EXPECT_THROW(parseAnswer(rcode, el("{ \"result\": [ 1 ]}")), CCSessionError);
+ EXPECT_THROW(parseAnswer(rcode, el("{ \"result\": [ 1, 1 ]}")), CCSessionError);
answer = el("{\"result\": [ 0 ]}");
arg = parseAnswer(rcode, answer);
EXPECT_EQ(0, rcode);
EXPECT_TRUE(isNull(arg));
- // todo: this should throw
- answer = el("{\"result\": [ 1 ]}");
-
answer = el("{\"result\": [ 1, \"error\"]}");
arg = parseAnswer(rcode, answer);
EXPECT_EQ(1, rcode);
@@ -113,9 +113,8 @@
ElementPtr command;
ElementPtr arg;
- // todo: add overload for no arg
- //command = createCommand("my_command");
- //ASSERT_EQ("{\"command\": [ \"my_command\" ]", command->str());
+ command = createCommand("my_command");
+ ASSERT_EQ("{\"command\": [ \"my_command\" ]}", command->str());
arg = el("1");
command = createCommand("my_command", arg);
@@ -136,14 +135,17 @@
std::string cmd;
// should throw
- //parseCommand(arg, ElementPtr());
- parseCommand(arg, el("1"));
- parseCommand(arg, el("{}"));
- parseCommand(arg, el("{\"not a command\": 1 }"));
- parseCommand(arg, el("{\"command\": 1 }"));
- parseCommand(arg, el("{\"command\": [] }"));
- parseCommand(arg, el("{\"command\": [ 1 ] }"));
- parseCommand(arg, el("{\"command\": [ \"command\" ] }"));
+ EXPECT_THROW(parseCommand(arg, ElementPtr()), CCSessionError);
+ EXPECT_THROW(parseCommand(arg, el("1")), CCSessionError);
+ EXPECT_THROW(parseCommand(arg, el("{}")), CCSessionError);
+ EXPECT_THROW(parseCommand(arg, el("{\"not a command\": 1 }")), CCSessionError);
+ EXPECT_THROW(parseCommand(arg, el("{\"command\": 1 }")), CCSessionError);
+ EXPECT_THROW(parseCommand(arg, el("{\"command\": [] }")), CCSessionError);
+ EXPECT_THROW(parseCommand(arg, el("{\"command\": [ 1 ] }")), CCSessionError);
+
+ cmd = parseCommand(arg, el("{\"command\": [ \"my_command\" ] }"));
+ EXPECT_EQ("my_command", cmd);
+ EXPECT_TRUE(isNull(arg));
cmd = parseCommand(arg, el("{\"command\": [ \"my_command\", 1 ] }"));
EXPECT_EQ("my_command", cmd);
@@ -197,13 +199,13 @@
new_config->get("item1")->intValue() == 5) {
return createAnswer(6, "I do not like the number 5");
}
- return createAnswer(0);
+ return createAnswer();
}
ElementPtr my_command_handler(const std::string& command, ElementPtr arg UNUSED_PARAM)
{
if (command == "good_command") {
- return createAnswer(0);
+ return createAnswer();
} else if (command == "command_with_arg") {
if (arg) {
if (arg->getType() == Element::integer) {
@@ -266,10 +268,10 @@
result = mccs.checkCommand();
EXPECT_EQ(0, result);
- // todo: type check in message handling in checkCommand
- //addMessage(el("1"), "Spec2", "*");
- //result = mccs.checkCommand();
- //EXPECT_EQ(0, result);
+ // not a command, should be ignored
+ addMessage(el("1"), "Spec2", "*");
+ result = mccs.checkCommand();
+ EXPECT_EQ(0, result);
addMessage(el("{ \"command\": [ \"good_command\" ] }"), "Spec2", "*");
result = mccs.checkCommand();
@@ -279,6 +281,13 @@
EXPECT_EQ(0, result);
addMessage(el("{ \"command\": \"bad_command\" }"), "Spec2", "*");
+ result = mccs.checkCommand();
+ EXPECT_EQ(1, msg_queue->size());
+ msg = getFirstMessage(group, to);
+ EXPECT_EQ("{\"result\": [ 1, \"Command part in command message missing, empty, or not a list\" ]}", msg->str());
+ EXPECT_EQ(0, result);
+
+ addMessage(el("{ \"command\": [ \"bad_command\" ] }"), "Spec2", "*");
result = mccs.checkCommand();
EXPECT_EQ(1, msg_queue->size());
msg = getFirstMessage(group, to);
More information about the bind10-changes
mailing list