From: Tomek Mrugalski <tomasz@isc.org>
Date: Thu, 2 Mar 2017 01:36:09 +0000 (+0100)
Subject: [5134_rebase] Changes after review:
X-Git-Tag: trac5137_base~3^2~2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9b37ab357429f20221e713010ebb0e0d1feaa12f;p=thirdparty%2Fkea.git

[5134_rebase] Changes after review:

 - stub implementation for parse added
 - unit-test for simpleParseConfig added
 - isc::hooks namespace added
---

diff --git a/src/lib/hooks/hooks_parser.cc b/src/lib/hooks/hooks_parser.cc
index 5c50d9997b..4c4d9057fe 100644
--- a/src/lib/hooks/hooks_parser.cc
+++ b/src/lib/hooks/hooks_parser.cc
@@ -20,6 +20,9 @@ using namespace isc::data;
 using namespace isc::hooks;
 using namespace isc::dhcp;
 
+namespace isc {
+namespace hooks {
+
 // ******************** HooksLibrariesParser *************************
 void
 HooksLibrariesParser::parse(ConstElementPtr value) {
@@ -152,3 +155,6 @@ void
 HooksLibrariesParser::getLibraries(isc::hooks::HookLibsCollection& libraries) {
     libraries = libraries_;
 }
+
+}
+}
diff --git a/src/lib/process/d_cfg_mgr.cc b/src/lib/process/d_cfg_mgr.cc
index 5ba2e6e22a..0442e5878e 100644
--- a/src/lib/process/d_cfg_mgr.cc
+++ b/src/lib/process/d_cfg_mgr.cc
@@ -258,13 +258,12 @@ DCfgMgrBase::parseConfig(isc::data::ConstElementPtr config_set) {
 isc::data::ConstElementPtr
 DCfgMgrBase::simpleParseConfig(isc::data::ConstElementPtr config_set,
                                bool check_only) {
-    LOG_DEBUG(dctl_logger, DBGLVL_COMMAND,
-                DCTL_CONFIG_START).arg(config_set->str());
-
     if (!config_set) {
         return (isc::config::createAnswer(1,
                                     std::string("Can't parse NULL config")));
     }
+    LOG_DEBUG(dctl_logger, DBGLVL_COMMAND,
+                DCTL_CONFIG_START).arg(config_set->str());
 
     // The parsers implement data inheritance by directly accessing
     // configuration context. For this reason the data parsers must store
diff --git a/src/lib/process/tests/d_cfg_mgr_unittests.cc b/src/lib/process/tests/d_cfg_mgr_unittests.cc
index fe37fe5e6c..0877f9a010 100644
--- a/src/lib/process/tests/d_cfg_mgr_unittests.cc
+++ b/src/lib/process/tests/d_cfg_mgr_unittests.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -477,5 +477,30 @@ TEST_F(DStubCfgMgrTest, paramPosition) {
     EXPECT_EQ(pos.file_, isc::data::Element::ZERO_POSITION().file_);
 }
 
+// This tests if some aspects of simpleParseConfig are behaving properly.
+// Thorough testing is only possible for specific implementations. This
+// is done for control agent (see CtrlAgentControllerTest tests in
+// src/bin/agent/tests/ctrl_agent_controller_unittest.cc for example).
+// Also, shell tests in src/bin/agent/ctrl_agent_process_tests.sh test
+// the whole CA process that uses simpleParseConfig. The alternative
+// would be to implement whole parser that would set the context
+// properly. The ROI for this is not worth the effort.
+TEST_F(DStubCfgMgrTest, simpleParseConfig) {
+    using namespace isc::data;
+
+    // Passing just null pointer should result in error return code
+    answer_ = cfg_mgr_->simpleParseConfig(ConstElementPtr(), false);
+    EXPECT_TRUE(checkAnswer(1));
+
+    // Ok, now try with a dummy, but valid json code
+    string config = "{ \"bool_test\": true , \n"
+                    "  \"uint32_test\": 77 , \n"
+                    "  \"string_test\": \"hmmm chewy\" }";
+    ASSERT_NO_THROW(fromJSON(config));
+
+    answer_ = cfg_mgr_->simpleParseConfig(config_set_, false);
+    EXPECT_TRUE(checkAnswer(0));
+}
+
 
 } // end of anonymous namespace
diff --git a/src/lib/process/testutils/d_test_stubs.cc b/src/lib/process/testutils/d_test_stubs.cc
index 4d9f34caef..501198f483 100644
--- a/src/lib/process/testutils/d_test_stubs.cc
+++ b/src/lib/process/testutils/d_test_stubs.cc
@@ -9,6 +9,7 @@
 #include <process/d_log.h>
 #include <process/spec_config.h>
 #include <process/testutils/d_test_stubs.h>
+#include <cc/command_interpreter.h>
 
 using namespace boost::asio;
 
@@ -428,5 +429,10 @@ DStubCfgMgr::createConfigParser(const std::string& element_id,
     return (parser);
 }
 
+isc::data::ConstElementPtr
+DStubCfgMgr::parse(isc::data::ConstElementPtr /*config*/, bool /*check_only*/) {
+    return (isc::config::createAnswer(0, "It all went fine. I promise"));
+}
+
 }; // namespace isc::process
 }; // namespace isc
diff --git a/src/lib/process/testutils/d_test_stubs.h b/src/lib/process/testutils/d_test_stubs.h
index e6e6d62335..d8fa797889 100644
--- a/src/lib/process/testutils/d_test_stubs.h
+++ b/src/lib/process/testutils/d_test_stubs.h
@@ -706,6 +706,19 @@ public:
                        const isc::data::Element::Position& pos
                        = isc::data::Element::Position());
 
+    /// @brief Pretends to parse the config
+    ///
+    /// This method pretends to parse the configuration specified on input
+    /// and returns a positive answer. The check_only flag is currently ignored.
+    ///
+    /// @param config configuration specified
+    /// @param check_only whether it's real configuration (false) or just
+    ///                configuration check (true)
+    /// @return always positive answer
+    ///
+    isc::data::ConstElementPtr
+    parse(isc::data::ConstElementPtr config, bool check_only);
+
     /// @brief Returns a summary of the configuration in the textual format.
     ///
     /// @return Always an empty string.