[3409] Store position of the data element in the value storage.

src/lib/cc/data.h

@@ -98,6 +98,10 @@ public:
         uint32_t line_;    ///< Line number.
         uint32_t pos_;     ///< Position within the line.
 
+        /// \brief Default constructor.
+        Position() : file_(""), line_(0), pos_(0) {
+        }
+
         /// \brief Constructor.
         ///
         /// \param file File name.

src/lib/dhcpsrv/dhcp_parsers.cc

@@ -125,6 +125,8 @@ DebugParser::commit() {
 // **************************** BooleanParser  *************************
 
 template<> void ValueParser<bool>::build(isc::data::ConstElementPtr value) {
+    // Invoke common code for all specializations of build().
+    buildCommon(value);
     // The Config Manager checks if user specified a
     // valid value for a boolean parameter: True or False.
     // We should have a boolean Element, use value directly
@@ -139,6 +141,9 @@ template<> void ValueParser<bool>::build(isc::data::ConstElementPtr value) {
 // **************************** Uin32Parser  *************************
 
 template<> void ValueParser<uint32_t>::build(ConstElementPtr value) {
+    // Invoke common code for all specializations of build().
+    buildCommon(value);
+
     int64_t check;
     string x = value->str();
     try {
@@ -163,6 +168,9 @@ template<> void ValueParser<uint32_t>::build(ConstElementPtr value) {
 // **************************** StringParser  *************************
 
 template <> void ValueParser<std::string>::build(ConstElementPtr value) {
+    // Invoke common code for all specializations of build().
+    buildCommon(value);
+
     value_ = value->str();
     boost::erase_all(value_, "\"");
 }

src/lib/dhcpsrv/dhcp_parsers.h

@@ -51,86 +51,127 @@ typedef boost::shared_ptr<std::vector<std::string> > HooksLibsStoragePtr;
 
 /// @brief A template class that stores named elements of a given data type.
 ///
-/// This template class is provides data value storage for configuration parameters
-/// of a given data type.  The values are stored by parameter name and as instances
-/// of type "ValueType".
+/// This template class is provides data value storage for configuration
+/// parameters of a given data type.  The values are stored by parameter name
+/// and as instances of type "ValueType". Each value held in the storage has
+/// a corresponding position within a configuration string (file) specified
+/// as a: file name, line number and position within the line. The position
+/// information is used for logging when the particular configuration value
+/// causes a configuration error.
 ///
-/// @param ValueType is the data type of the elements to store.
+/// @tparam ValueType is the data type of the elements to store.
 template<typename ValueType>
 class ValueStorage {
-    public:
-        /// @brief  Stores the the parameter and its value in the store.
-        ///
-        /// If the parameter does not exist in the store, then it will be added,
-        /// otherwise its data value will be updated with the given value.
-        ///
-        /// @param name is the name of the paramater to store.
-        /// @param value is the data value to store.
-        void setParam(const std::string& name, const ValueType& value) {
-            values_[name] = value;
-        }
+public:
+    /// @brief  Stores the the parameter, its value and the position in the
+    /// store.
+    ///
+    /// If the parameter does not exist in the store, then it will be added,
+    /// otherwise its data value and the position will be updated with the
+    /// given values.
+    ///
+    /// @param name is the name of the paramater to store.
+    /// @param value is the data value to store.
+    /// @param position is the position of the data element within a
+    /// configuration string (file).
+    void setParam(const std::string& name, const ValueType& value,
+                  const data::Element::Position& position) {
+        values_[name] = value;
+        positions_[name] = position;
+    }
 
-        /// @brief Returns the data value for the given parameter.
-        ///
-        /// Finds and returns the data value for the given parameter.
-        /// @param name is the name of the parameter for which the data
-        /// value is desired.
-        ///
-        /// @return The paramater's data value of type @c ValueType.
-        /// @throw DhcpConfigError if the parameter is not found.
-        ValueType getParam(const std::string& name) const {
-            typename std::map<std::string, ValueType>::const_iterator param
-                = values_.find(name);
-
-            if (param == values_.end()) {
-                isc_throw(DhcpConfigError, "Missing parameter '"
-                       << name << "'");
-            }
-
-            return (param->second);
-        }
+    /// @brief Returns the data value for the given parameter.
+    ///
+    /// Finds and returns the data value for the given parameter.
+    /// @param name is the name of the parameter for which the data
+    /// value is desired.
+    ///
+    /// @return The paramater's data value of type @c ValueType.
+    /// @throw DhcpConfigError if the parameter is not found.
+    ValueType getParam(const std::string& name) const {
+        typename std::map<std::string, ValueType>::const_iterator param
+            = values_.find(name);
 
-        /// @brief Returns the data value for an optional parameter.
-        ///
-        /// Finds and returns the data value for the given parameter or
-        /// a supplied default value if it is not found.
-        ///
-        /// @param name is the name of the parameter for which the data
-        /// value is desired.
-        /// @param default_value value to use the default
-        ///
-        /// @return The paramater's data value of type @c ValueType.
-        ValueType getOptionalParam(const std::string& name,
-                                   const ValueType& default_value) const {
-            typename std::map<std::string, ValueType>::const_iterator param
-                = values_.find(name);
-
-            if (param == values_.end()) {
-                return (default_value);
-            }
-
-            return (param->second);
+        if (param == values_.end()) {
+            isc_throw(DhcpConfigError, "Missing parameter '"
+                      << name << "'");
         }
 
-        /// @brief  Remove the parameter from the store.
-        ///
-        /// Deletes the entry for the given parameter from the store if it
-        /// exists.
-        ///
-        /// @param name is the name of the paramater to delete.
-        void delParam(const std::string& name) {
-            values_.erase(name);
+        return (param->second);
+    }
+
+    /// @brief Returns position of the data element in the configuration string.
+    ///
+    /// The returned object comprises file name, line number and the position
+    /// within the particular line of the configuration string where the data
+    /// element holding a particular value is located.
+    ///
+    /// @param name is the name of the parameter which position is desired.
+    ///
+    /// @return Position of the data element or the position holding empty
+    /// file name and two zeros if the position hasn't been specified for the
+    /// particular value.
+    const data::Element::Position& getPosition(const std::string& name) const {
+        typename std::map<std::string, data::Element::Position>::const_iterator
+            pos = positions_.find(name);
+        if (pos == positions_.end()) {
+            return (data::Element::ZERO_POSITION());
         }
 
-        /// @brief Deletes all of the entries from the store.
-        ///
-        void clear() {
-            values_.clear();
+        return (pos->second);
+    }
+
+    /// @brief Returns the data value for an optional parameter.
+    ///
+    /// Finds and returns the data value for the given parameter or
+    /// a supplied default value if it is not found.
+    ///
+    /// @param name is the name of the parameter for which the data
+    /// value is desired.
+    /// @param default_value value to use the default
+    ///
+    /// @return The paramater's data value of type @c ValueType.
+    ValueType getOptionalParam(const std::string& name,
+                               const ValueType& default_value) const {
+        typename std::map<std::string, ValueType>::const_iterator param
+            = values_.find(name);
+
+        if (param == values_.end()) {
+            return (default_value);
         }
 
-    private:
-        /// @brief An std::map of the data values, keyed by parameter names.
-        std::map<std::string, ValueType> values_;
+        return (param->second);
+    }
+
+    /// @brief  Remove the parameter from the store.
+    ///
+    /// Deletes the entry for the given parameter from the store if it
+    /// exists.
+    ///
+    /// @param name is the name of the paramater to delete.
+    void delParam(const std::string& name) {
+        values_.erase(name);
+        positions_.erase(name);
+    }
+
+    /// @brief Deletes all of the entries from the store.
+    ///
+    void clear() {
+        values_.clear();
+        positions_.clear();
+    }
+
+private:
+    /// @brief An std::map of the data values, keyed by parameter names.
+    std::map<std::string, ValueType> values_;
+
+    /// @brief An std::map holding positions of the data elements in the
+    /// configuration, which values are held in @c values_.
+    ///
+    /// The position is used for logging, when the particular value
+    /// causes a configuration error.
+    std::map<std::string, data::Element::Position> positions_;
+
 };
 
 
@@ -237,7 +278,7 @@ public:
     /// @throw isc::dhcp::DhcpConfigError if storage is null.
     ValueParser(const std::string& param_name,
         boost::shared_ptr<ValueStorage<ValueType> > storage)
-        : storage_(storage), param_name_(param_name), value_() {
+        : storage_(storage), param_name_(param_name), value_(), pos_() {
         // Empty parameter name is invalid.
         if (param_name_.empty()) {
             isc_throw(isc::dhcp::DhcpConfigError, "parser logic error:"
@@ -251,7 +292,6 @@ public:
         }
     }
 
-
     /// @brief Parse a given element into a value of type @c ValueType
     ///
     /// @param value a value to be parsed.
@@ -264,10 +304,23 @@ public:
     void commit() {
         // If a given parameter already exists in the storage we override
         // its value. If it doesn't we insert a new element.
-        storage_->setParam(param_name_, value_);
+        storage_->setParam(param_name_, value_, pos_);
     }
 
 private:
+
+    /// @brief Performs operations common for all specializations of the
+    /// @c build function.
+    ///
+    /// This method should be called by all specializations of the @c build
+    /// method.
+    ///
+    /// @param value a value being parsed.
+    void buildCommon(isc::data::ConstElementPtr value) {
+        // Remember position of the data element.
+        pos_ = value->getPosition();
+    }
+
     /// Pointer to the storage where committed value is stored.
     boost::shared_ptr<ValueStorage<ValueType> > storage_;
 
@@ -276,6 +329,8 @@ private:
 
     /// Parsed value.
     ValueType value_;
+
+    data::Element::Position pos_;
 };
 
 /// @brief typedefs for simple data type parsers

src/lib/dhcpsrv/tests/cfgmgr_unittest.cc

@@ -29,6 +29,7 @@
 
 using namespace std;
 using namespace isc::asiolink;
+using namespace isc::data;
 using namespace isc::dhcp;
 using namespace isc::dhcp::test;
 using namespace isc::util;
@@ -40,34 +41,67 @@ using boost::scoped_ptr;
 
 namespace {
 
+template <typename Storage>
+bool isZeroPosition(const Storage& storage, const std::string& param_name) {
+    Element::Position position = storage.getPosition(param_name);
+    return ((position.line_ == 0) && (position.pos_ == 0) &&
+            (position.file_.empty()));
+}
+
 // This test verifies that BooleanStorage functions properly.
 TEST(ValueStorageTest, BooleanTesting) {
     BooleanStorage testStore;
 
     // Verify that we can add and retrieve parameters.
-    testStore.setParam("firstBool", false);
-    testStore.setParam("secondBool", true);
+    testStore.setParam("firstBool", false, Element::Position("kea.conf", 123, 234));
+    testStore.setParam("secondBool", true, Element::Position("keax.conf", 10, 20));
 
     EXPECT_FALSE(testStore.getParam("firstBool"));
     EXPECT_TRUE(testStore.getParam("secondBool"));
 
+    EXPECT_EQ(123, testStore.getPosition("firstBool").line_);
+    EXPECT_EQ(234, testStore.getPosition("firstBool").pos_);
+    EXPECT_EQ("kea.conf", testStore.getPosition("firstBool").file_);
+
+    EXPECT_EQ(10, testStore.getPosition("secondBool").line_);
+    EXPECT_EQ(20, testStore.getPosition("secondBool").pos_);
+    EXPECT_EQ("keax.conf", testStore.getPosition("secondBool").file_);
+
     // Verify that we can update parameters.
-    testStore.setParam("firstBool", true);
-    testStore.setParam("secondBool", false);
+    testStore.setParam("firstBool", true, Element::Position("keax.conf", 555, 111));
+    testStore.setParam("secondBool", false, Element::Position("kea.conf", 1, 3));
 
     EXPECT_TRUE(testStore.getParam("firstBool"));
     EXPECT_FALSE(testStore.getParam("secondBool"));
 
+    EXPECT_EQ(555, testStore.getPosition("firstBool").line_);
+    EXPECT_EQ(111, testStore.getPosition("firstBool").pos_);
+    EXPECT_EQ("keax.conf", testStore.getPosition("firstBool").file_);
+
+    EXPECT_EQ(1, testStore.getPosition("secondBool").line_);
+    EXPECT_EQ(3, testStore.getPosition("secondBool").pos_);
+    EXPECT_EQ("kea.conf", testStore.getPosition("secondBool").file_);
+
     // Verify that we can delete a parameter and it will no longer be found.
     testStore.delParam("firstBool");
     EXPECT_THROW(testStore.getParam("firstBool"), isc::dhcp::DhcpConfigError);
 
+    // Verify that the "zero" position is returned when parameter doesn't exist.
+    EXPECT_TRUE(isZeroPosition(testStore, "firstBool"));
+
     // Verify that the delete was safe and the store still operates.
     EXPECT_FALSE(testStore.getParam("secondBool"));
 
+    EXPECT_EQ(1, testStore.getPosition("secondBool").line_);
+    EXPECT_EQ(3, testStore.getPosition("secondBool").pos_);
+    EXPECT_EQ("kea.conf", testStore.getPosition("secondBool").file_);
+
     // Verify that looking for a parameter that never existed throws.
     ASSERT_THROW(testStore.getParam("bogusBool"), isc::dhcp::DhcpConfigError);
 
+    // Verify that the "zero" position is returned when parameter doesn't exist.
+    EXPECT_TRUE(isZeroPosition(testStore, "bogusBool"));
+
     // Verify that attempting to delete a parameter that never existed does not throw.
     EXPECT_NO_THROW(testStore.delParam("bogusBool"));
 
@@ -75,35 +109,60 @@ TEST(ValueStorageTest, BooleanTesting) {
     testStore.clear();
     EXPECT_THROW(testStore.getParam("secondBool"), isc::dhcp::DhcpConfigError);
 
+    // Verify that the "zero" position is returned when parameter doesn't exist.
+    EXPECT_TRUE(isZeroPosition(testStore, "secondBool"));
 }
 
 // This test verifies that Uint32Storage functions properly.
 TEST(ValueStorageTest, Uint32Testing) {
     Uint32Storage testStore;
 
-    uint32_t intOne = 77;
-    uint32_t intTwo = 33;
+    uint32_t int_one = 77;
+    uint32_t int_two = 33;
 
     // Verify that we can add and retrieve parameters.
-    testStore.setParam("firstInt", intOne);
-    testStore.setParam("secondInt", intTwo);
+    testStore.setParam("firstInt", int_one, Element::Position("kea.conf", 123, 234));
+    testStore.setParam("secondInt", int_two, Element::Position("keax.conf", 10, 20));
+
+    EXPECT_EQ(testStore.getParam("firstInt"), int_one);
+    EXPECT_EQ(testStore.getParam("secondInt"), int_two);
 
-    EXPECT_EQ(testStore.getParam("firstInt"), intOne);
-    EXPECT_EQ(testStore.getParam("secondInt"), intTwo);
+    EXPECT_EQ(123, testStore.getPosition("firstInt").line_);
+    EXPECT_EQ(234, testStore.getPosition("firstInt").pos_);
+    EXPECT_EQ("kea.conf", testStore.getPosition("firstInt").file_);
+
+    EXPECT_EQ(10, testStore.getPosition("secondInt").line_);
+    EXPECT_EQ(20, testStore.getPosition("secondInt").pos_);
+    EXPECT_EQ("keax.conf", testStore.getPosition("secondInt").file_);
 
     // Verify that we can update parameters.
-    testStore.setParam("firstInt", --intOne);
-    testStore.setParam("secondInt", ++intTwo);
+    testStore.setParam("firstInt", --int_one, Element::Position("keax.conf", 555, 111));
+    testStore.setParam("secondInt", ++int_two, Element::Position("kea.conf", 1, 3));
+
+    EXPECT_EQ(testStore.getParam("firstInt"), int_one);
+    EXPECT_EQ(testStore.getParam("secondInt"), int_two);
+
+    EXPECT_EQ(555, testStore.getPosition("firstInt").line_);
+    EXPECT_EQ(111, testStore.getPosition("firstInt").pos_);
+    EXPECT_EQ("keax.conf", testStore.getPosition("firstInt").file_);
 
-    EXPECT_EQ(testStore.getParam("firstInt"), intOne);
-    EXPECT_EQ(testStore.getParam("secondInt"), intTwo);
+    EXPECT_EQ(1, testStore.getPosition("secondInt").line_);
+    EXPECT_EQ(3, testStore.getPosition("secondInt").pos_);
+    EXPECT_EQ("kea.conf", testStore.getPosition("secondInt").file_);
 
     // Verify that we can delete a parameter and it will no longer be found.
     testStore.delParam("firstInt");
     EXPECT_THROW(testStore.getParam("firstInt"), isc::dhcp::DhcpConfigError);
 
+    // Verify that the "zero" position is returned when parameter doesn't exist.
+    EXPECT_TRUE(isZeroPosition(testStore, "firstInt"));
+
     // Verify that the delete was safe and the store still operates.
-    EXPECT_EQ(testStore.getParam("secondInt"), intTwo);
+    EXPECT_EQ(testStore.getParam("secondInt"), int_two);
+
+    EXPECT_EQ(1, testStore.getPosition("secondInt").line_);
+    EXPECT_EQ(3, testStore.getPosition("secondInt").pos_);
+    EXPECT_EQ("kea.conf", testStore.getPosition("secondInt").file_);
 
     // Verify that looking for a parameter that never existed throws.
     ASSERT_THROW(testStore.getParam("bogusInt"), isc::dhcp::DhcpConfigError);
@@ -111,41 +170,74 @@ TEST(ValueStorageTest, Uint32Testing) {
     // Verify that attempting to delete a parameter that never existed does not throw.
     EXPECT_NO_THROW(testStore.delParam("bogusInt"));
 
+    // Verify that the "zero" position is returned when parameter doesn't exist.
+    EXPECT_TRUE(isZeroPosition(testStore, "bogusInt"));
+
     // Verify that we can empty the list.
     testStore.clear();
     EXPECT_THROW(testStore.getParam("secondInt"), isc::dhcp::DhcpConfigError);
+
+    // Verify that the "zero" position is returned when parameter doesn't exist.
+    EXPECT_TRUE(isZeroPosition(testStore, "secondInt"));
 }
 
 // This test verifies that StringStorage functions properly.
 TEST(ValueStorageTest, StringTesting) {
     StringStorage testStore;
 
-    std::string stringOne = "seventy-seven";
-    std::string stringTwo = "thirty-three";
+    std::string string_one = "seventy-seven";
+    std::string string_two = "thirty-three";
 
     // Verify that we can add and retrieve parameters.
-    testStore.setParam("firstString", stringOne);
-    testStore.setParam("secondString", stringTwo);
+    testStore.setParam("firstString", string_one,
+                       Element::Position("kea.conf", 123, 234));
+    testStore.setParam("secondString", string_two,
+                       Element::Position("keax.conf", 10, 20));
+
+    EXPECT_EQ(testStore.getParam("firstString"), string_one);
+    EXPECT_EQ(testStore.getParam("secondString"), string_two);
+
+    EXPECT_EQ(123, testStore.getPosition("firstString").line_);
+    EXPECT_EQ(234, testStore.getPosition("firstString").pos_);
+    EXPECT_EQ("kea.conf", testStore.getPosition("firstString").file_);
 
-    EXPECT_EQ(testStore.getParam("firstString"), stringOne);
-    EXPECT_EQ(testStore.getParam("secondString"), stringTwo);
+    EXPECT_EQ(10, testStore.getPosition("secondString").line_);
+    EXPECT_EQ(20, testStore.getPosition("secondString").pos_);
+    EXPECT_EQ("keax.conf", testStore.getPosition("secondString").file_);
 
     // Verify that we can update parameters.
-    stringOne.append("-boo");
-    stringTwo.append("-boo");
+    string_one.append("-boo");
+    string_two.append("-boo");
 
-    testStore.setParam("firstString", stringOne);
-    testStore.setParam("secondString", stringTwo);
+    testStore.setParam("firstString", string_one,
+                       Element::Position("kea.conf", 555, 111));
+    testStore.setParam("secondString", string_two,
+                       Element::Position("keax.conf", 1, 3));
 
-    EXPECT_EQ(testStore.getParam("firstString"), stringOne);
-    EXPECT_EQ(testStore.getParam("secondString"), stringTwo);
+    EXPECT_EQ(testStore.getParam("firstString"), string_one);
+    EXPECT_EQ(testStore.getParam("secondString"), string_two);
+
+    EXPECT_EQ(555, testStore.getPosition("firstString").line_);
+    EXPECT_EQ(111, testStore.getPosition("firstString").pos_);
+    EXPECT_EQ("kea.conf", testStore.getPosition("firstString").file_);
+
+    EXPECT_EQ(1, testStore.getPosition("secondString").line_);
+    EXPECT_EQ(3, testStore.getPosition("secondString").pos_);
+    EXPECT_EQ("keax.conf"