[#2707] parseAnswer() cleaned up

There are now two versions of this func:
  - parseAnswer() - returns parameters (if present) or text
  - parseanswerText() - return text status

 Previously they were both bundled into one parseAnswer()

src/bin/dhcp4/tests/config_backend_unittest.cc

@@ -131,7 +131,7 @@ public:
         ASSERT_TRUE(status);
 
         int rcode;
-        ConstElementPtr comment = parseAnswer(rcode, status);
+        ConstElementPtr comment = parseAnswerText(rcode, status);
         ASSERT_EQ(expected_code, rcode) << " comment: "
                     << comment->stringValue();
 

src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -294,7 +294,7 @@ public:
     // Also stores result in rcode_ and comment_.
     void checkResult(ConstElementPtr status, int expected_code) {
         ASSERT_TRUE(status);
-        comment_ = parseAnswer(rcode_, status);
+        comment_ = parseAnswerText(rcode_, status);
         EXPECT_EQ(expected_code, rcode_) << "error text:" << comment_->stringValue();
     }
 
@@ -332,10 +332,11 @@ public:
         ASSERT_TRUE(status);
 
         int rcode;
-        ConstElementPtr comment = parseAnswer(rcode, status);
+        ConstElementPtr comment = parseAnswerText(rcode, status);
         EXPECT_EQ(expected_code, rcode);
 
         string text;
+        ASSERT_TRUE(comment);
         ASSERT_NO_THROW(text = comment->stringValue());
 
         if (expected_code != rcode) {
@@ -665,7 +666,7 @@ public:
         // Store the answer if we need it.
 
         // Returned value should be 0 (configuration success)
-        comment_ = parseAnswer(rcode_, status);
+        comment_ = parseAnswerText(rcode_, status);
         if (rcode_ != 0) {
             string reason = "";
             if (comment_) {
@@ -2327,6 +2328,7 @@ TEST_F(Dhcp4ParserTest, badSubnetValues) {
             ConstElementPtr status;
             EXPECT_NO_THROW(status = Dhcpv4SrvTest::configure(*srv_, config));
             checkResult(status, 1);
+            ASSERT_TRUE(comment_);
             EXPECT_EQ(comment_->stringValue(), (*scenario).exp_error_msg_);
         }
     }

src/bin/dhcp4/tests/dhcp4_test_utils.cc

@@ -854,7 +854,7 @@ Dhcpv4SrvTest::configure(const std::string& config,
     EXPECT_NO_THROW(status = configureDhcp4Server(srv, json, test));
     ASSERT_TRUE(status);
     int rcode;
-    ConstElementPtr comment = config::parseAnswer(rcode, status);
+    ConstElementPtr comment = config::parseAnswerText(rcode, status);
     ASSERT_EQ(0, rcode) << "configuration failed, test is broken: "
         << comment->str();
 
@@ -927,7 +927,7 @@ Dhcpv4SrvTest::configureWithStatus(const std::string& config, NakedDhcpv4Srv& sr
     }
 
     int rcode;
-    ConstElementPtr comment = config::parseAnswer(rcode, status);
+    ConstElementPtr comment = config::parseAnswerText(rcode, status);
     EXPECT_EQ(exp_rcode, rcode) << comment->stringValue();
 
     // Use specified lease database backend.

src/bin/dhcp6/tests/config_backend_unittest.cc

@@ -134,7 +134,7 @@ public:
         ASSERT_TRUE(status);
 
         int rcode;
-        ConstElementPtr comment = parseAnswer(rcode, status);
+        ConstElementPtr comment = parseAnswerText(rcode, status);
         ASSERT_EQ(expected_code, rcode) << " comment: "
                     << comment->stringValue();
 

src/bin/dhcp6/tests/config_parser_unittest.cc

@@ -405,7 +405,7 @@ public:
     // Also stores result in rcode_ and comment_.
     void checkResult(ConstElementPtr status, int expected_code) {
         ASSERT_TRUE(status);
-        comment_ = parseAnswer(rcode_, status);
+        comment_ = parseAnswerText(rcode_, status);
         EXPECT_EQ(expected_code, rcode_);
         if (expected_code != rcode_) {
             cout << "The comment returned was: [" << comment_->stringValue() << "]" << endl;
@@ -418,7 +418,7 @@ public:
     void checkResult(ConstElementPtr status, int expected_code,
                      string expected_txt) {
         ASSERT_TRUE(status);
-        comment_ = parseAnswer(rcode_, status);
+        comment_ = parseAnswerText(rcode_, status);
         EXPECT_EQ(expected_code, rcode_) << "error text:" << comment_->stringValue();
         ASSERT_TRUE(comment_);
         ASSERT_EQ(Element::string, comment_->getType());
@@ -442,10 +442,11 @@ public:
         ASSERT_TRUE(status);
 
         int rcode;
-        ConstElementPtr comment = parseAnswer(rcode, status);
+        ConstElementPtr comment = parseAnswerText(rcode, status);
         EXPECT_EQ(expected_code, rcode);
 
         string text;
+        ASSERT_TRUE(comment);
         ASSERT_NO_THROW(text = comment->stringValue());
 
         if (expected_code != rcode) {
@@ -708,7 +709,7 @@ public:
         // Store the answer if we need it.
 
         // Returned value should be 0 (configuration success)
-        comment_ = parseAnswer(rcode_, status);
+        comment_ = parseAnswerText(rcode_, status);
         if (rcode_ != 0) {
             string reason = "";
             if (comment_) {
@@ -6169,7 +6170,7 @@ TEST_F(Dhcp6ParserTest, notExistDataDir) {
 
     // returned value should be 1 (error)
     int rcode;
-    ConstElementPtr comment = parseAnswer(rcode, status);
+    ConstElementPtr comment = parseAnswerText(rcode, status);
     EXPECT_EQ(1, rcode);
     string text;
     ASSERT_NO_THROW(text = comment->stringValue());
@@ -6191,7 +6192,7 @@ TEST_F(Dhcp6ParserTest, notDirDataDir) {
 
     // returned value should be 1 (error)
     int rcode;
-    ConstElementPtr comment = parseAnswer(rcode, status);
+    ConstElementPtr comment = parseAnswerText(rcode, status);
     EXPECT_EQ(1, rcode);
     string text;
     ASSERT_NO_THROW(text = comment->stringValue());
@@ -6646,7 +6647,7 @@ TEST_F(Dhcp6ParserTest, invalidPoolRange) {
     EXPECT_NO_THROW(status = Dhcpv6SrvTest::configure(srv_, json));
     ASSERT_TRUE(status);
     int rcode;
-    ConstElementPtr comment = parseAnswer(rcode, status);
+    ConstElementPtr comment = parseAnswerText(rcode, status);
     string text;
     ASSERT_NO_THROW(text = comment->stringValue());
 
@@ -6677,7 +6678,7 @@ TEST_F(Dhcp6ParserTest, outsideSubnetPool) {
     EXPECT_NO_THROW(status = Dhcpv6SrvTest::configure(srv_, json));
     ASSERT_TRUE(status);
     int rcode;
-    ConstElementPtr comment = parseAnswer(rcode, status);
+    ConstElementPtr comment = parseAnswerText(rcode, status);
     string text;
     ASSERT_NO_THROW(text = comment->stringValue());
 

src/lib/cc/command_interpreter.cc

@@ -65,6 +65,33 @@ createAnswer(const int status_code, const ConstElementPtr& arg) {
     return (createAnswer(status_code, "", arg));
 }
 
+ConstElementPtr
+parseAnswerText(int &rcode, const ConstElementPtr& msg) {
+    if (!msg) {
+        isc_throw(CtrlChannelError, "invalid answer: no answer specified");
+    }
+    if (msg->getType() != Element::map) {
+        isc_throw(CtrlChannelError, "invalid answer: expected toplevel entry to be a map, got "
+                  << Element::typeToName(msg->getType()) << " instead");
+    }
+    if (!msg->contains(CONTROL_RESULT)) {
+        isc_throw(CtrlChannelError,
+                  "invalid answer: does not contain mandatory '" << CONTROL_RESULT << "'");
+    }
+
+    ConstElementPtr result = msg->get(CONTROL_RESULT);
+    if (result->getType() != Element::integer) {
+        isc_throw(CtrlChannelError, "invalid answer: expected '" << CONTROL_RESULT
+                  << "' to be an integer, got "
+                  << Element::typeToName(result->getType()) << " instead");
+    }
+
+    rcode = result->intValue();
+
+    // If there are arguments, return them.
+    return (msg->get(CONTROL_TEXT));
+}
+
 ConstElementPtr
 parseAnswer(int &rcode, const ConstElementPtr& msg) {
     if (!msg) {
@@ -91,17 +118,14 @@ parseAnswer(int &rcode, const ConstElementPtr& msg) {
     // If there are arguments, return them.
     ConstElementPtr args = msg->get(CONTROL_ARGUMENTS);
     if (args) {
-        // If the arguments contain only a hash (used in config-set/config-get), we can ignore them.
-        // We don't want to return arguments with just a hash.
-        if ( (args->getType()!=isc::data::Element::map) || (args->size() > 1) || !args->get("hash") ) {
         return (args);
     }
-    }
 
     // There are no arguments, let's try to return just the text status
     return (msg->get(CONTROL_TEXT));
 }
 
+
 std::string
 answerToText(const ConstElementPtr& msg) {
     if (!msg) {

src/lib/cc/command_interpreter.h

@@ -101,14 +101,29 @@ isc::data::ConstElementPtr createAnswer(const int status_code,
                                         const std::string& status,
                                         const isc::data::ConstElementPtr& arg);
 
-/// @brief Parses a standard config/command level answer message.
+/// @brief Parses a standard config/command level answer and returns arguments
+/// or text status code.
+///
+/// If you need to get the text status, please use @ref parseAnswerText.
 ///
 /// @param status_code 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.
+/// @return The optional argument in the message (or null)
-isc::data::ConstElementPtr parseAnswer(int &status_code,
+isc::data::ConstElementPtr
-                                       const isc::data::ConstElementPtr& msg);
+parseAnswer(int &status_code, const isc::data::ConstElementPtr& msg);
+
+/// @brief Parses a standard config/command level answer and returns text status.
+///
+/// This method returns the text status. If you need to get the arguments provided,
+/// please use @ref parseAnswer.
+///
+/// @param status_code 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 null)
+isc::data::ConstElementPtr
+parseAnswerText(int &rcode, const isc::data::ConstElementPtr& msg);
 
 /// @brief Converts answer to printable text
 ///

src/lib/cc/tests/command_interpreter_unittests.cc

@@ -78,15 +78,24 @@ TEST(CommandInterpreterTest, parseAnswer) {
     EXPECT_EQ(0, rcode);
     EXPECT_TRUE(isNull(arg));
 
-    answer = el("{ \"result\": 1, \"text\": \"error\" }");
+    answer = el("{ \"result\": 3, \"text\": \"error\", \"arguments\": [ \"some\", \"data\" ] }");
     arg = parseAnswer(rcode, answer);
-    EXPECT_EQ(1, rcode);
+    ASSERT_TRUE(arg);
-    EXPECT_EQ("error", arg->stringValue());
+    EXPECT_EQ(3, rcode);
+    EXPECT_EQ("[ \"some\", \"data\" ]", arg->str());
+}
 
-    answer = el("{ \"result\": 0, \"arguments\": [ \"just\", \"some\", \"data\" ] }");
+// Checks if parseAnswerText can return the text
-    arg = parseAnswer(rcode, answer);
+TEST(CommandInterpreterTest, parseAnswerText) {
-    EXPECT_EQ(0, rcode);
+    ConstElementPtr answer;
-    EXPECT_EQ("[ \"just\", \"some\", \"data\" ]", arg->str());
+    ConstElementPtr arg;
+    int rcode;
+
+    answer = el("{ \"result\": 5, \"text\": \"error\", \"arguments\": [ \"some\", \"data\" ] }");
+    arg = parseAnswerText(rcode, answer);
+    ASSERT_TRUE(arg);
+    EXPECT_EQ(5, rcode);
+    EXPECT_EQ("error", arg->stringValue());
 }
 
 // This checks whether we can convert an answer to easily printable form.