]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#2707] parseAnswer() cleaned up
authorTomek Mrugalski <tomek@isc.org>
Thu, 22 Jun 2023 10:48:50 +0000 (12:48 +0200)
committerTomek Mrugalski <tomek@isc.org>
Thu, 22 Jun 2023 14:23:38 +0000 (16:23 +0200)
 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
src/bin/dhcp4/tests/config_parser_unittest.cc
src/bin/dhcp4/tests/dhcp4_test_utils.cc
src/bin/dhcp6/tests/config_backend_unittest.cc
src/bin/dhcp6/tests/config_parser_unittest.cc
src/lib/cc/command_interpreter.cc
src/lib/cc/command_interpreter.h
src/lib/cc/tests/command_interpreter_unittests.cc

index 510c2bffccf7859502504922786b3774a9c48b14..f3aa486976318f49257ec4f6a264a483c4d0a2d7 100644 (file)
@@ -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();
 
index e297063b3e1821d0581bb445e3da16f1653b98b7..9e30f2358086c26ea0e04c56bce9af77edd39dc8 100644 (file)
@@ -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_);
         }
     }
index c4a4db593bf410e343b0bd82af3c458d016acccf..b33c34f531f2afc82adb1b316b532563e0fb56b1 100644 (file)
@@ -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.
index 6475ee832bcfa78f1deaec63134d1a75404310b3..e2d87762d23d0fde1a3e9647fdb8f5af58ebc5b0 100644 (file)
@@ -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();
 
index 59c17abb87f09e8450747ed517b2067c7373127a..7eb5186d3c4ae371e5e929bf6abb3c7c4489d0f1 100644 (file)
@@ -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());
 
index 5cb1747105ba6ef85d5bb1ac321c55dd3bee994f..0ea03bcd48421a547c879364489beaf68a4b3e87 100644 (file)
@@ -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);
-        }
+        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) {
index 6a2b2cb3ae032b2309785cd6d104803ab61194df..aea0552f43e1cc35eb4eadf59b3c653778b8ee15 100644 (file)
@@ -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.
-isc::data::ConstElementPtr parseAnswer(int &status_code,
-                                       const isc::data::ConstElementPtr& msg);
+/// @return The optional argument in the message (or null)
+isc::data::ConstElementPtr
+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
 ///
index 38910fb441a2e091b20962418989b24fe3afce5f..8738b925c59da1e924c1b78cc76bb9cb98859358 100644 (file)
@@ -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);
-    EXPECT_EQ("error", arg->stringValue());
+    ASSERT_TRUE(arg);
+    EXPECT_EQ(3, rcode);
+    EXPECT_EQ("[ \"some\", \"data\" ]", arg->str());
+}
 
-    answer = el("{ \"result\": 0, \"arguments\": [ \"just\", \"some\", \"data\" ] }");
-    arg = parseAnswer(rcode, answer);
-    EXPECT_EQ(0, rcode);
-    EXPECT_EQ("[ \"just\", \"some\", \"data\" ]", arg->str());
+// Checks if parseAnswerText can return the text
+TEST(CommandInterpreterTest, parseAnswerText) {
+    ConstElementPtr answer;
+    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.