[#3074] internal opt type refactor

diff --git a/doc/examples/kea4/all-options.json b/doc/examples/kea4/all-options.json
index 7f6d0d1d0e739c9ddc24c3eb5e749a41565d6fe1..a997a5ce1626c75b297a543cf088aae42a0d0472 100644 (file)
--- a/doc/examples/kea4/all-options.json
+++ b/doc/examples/kea4/all-options.json
@@ -1318,7 +1318,7 @@
       Router 1...N          The IP address of the router that should
                             be used to reach that destination.
       */
-      // Type: custom
+      // Type: internal
       {
         "code": 121,
         // please mind the convenience notation used:

diff --git a/doc/sphinx/arm/dhcp4-srv.rst b/doc/sphinx/arm/dhcp4-srv.rst
index b4393a8dcbe628677c647a45999fae70745c229f..fd2fdb1b57791c4848bc417294070fb122f5d560 100644 (file)
--- a/doc/sphinx/arm/dhcp4-srv.rst
+++ b/doc/sphinx/arm/dhcp4-srv.rst
@@ -2022,7 +2022,7 @@ types are given in :ref:`dhcp-types`.
    +----------------------------------------+------+---------------------------+-------------+-------------+
    | domain-search                          | 119  | fqdn                      | true        | false       |
    +----------------------------------------+------+---------------------------+-------------+-------------+
-   | classless-static-route                 | 121  | custom                    | false       | false       |
+   | classless-static-route                 | 121  | internal                  | false       | false       |
    +----------------------------------------+------+---------------------------+-------------+-------------+
    | vivco-suboptions                       | 124  | record (uint32, binary)   | false       | false       |
    +----------------------------------------+------+---------------------------+-------------+-------------+
@@ -2121,10 +2121,6 @@ what values are accepted for them.
    | boolean         | A boolean value with allowed                          |
    |                 | values true or false.                                 |
    +-----------------+-------------------------------------------------------+
-   | custom          | A custom value which is intended to be used only      |
-   |                 | internally. It is meant for options which provide     |
-   |                 | custom configuration syntax for users convenience.    |
-   +-----------------+-------------------------------------------------------+
    | empty           | No value; data is carried in                          |
    |                 | sub-options.                                          |
    +-----------------+-------------------------------------------------------+

diff --git a/src/lib/dhcp/option_data_types.cc b/src/lib/dhcp/option_data_types.cc
index 4e2a17b964f88b326aeba405e3001dd7d3cc1070..46e49bfd6db839e21fcffe1d23670c1a53e8e37f 100644 (file)
--- a/src/lib/dhcp/option_data_types.cc
+++ b/src/lib/dhcp/option_data_types.cc
@@ -50,7 +50,7 @@ OptionDataTypeUtil::OptionDataTypeUtil() {
     data_types_["string"] = OPT_STRING_TYPE;
     data_types_["tuple"] = OPT_TUPLE_TYPE;
     data_types_["fqdn"] = OPT_FQDN_TYPE;
-    data_types_["custom"] = OPT_CUSTOM_TYPE;
+    data_types_["internal"] = OPT_INTERNAL_TYPE;
     data_types_["record"] = OPT_RECORD_TYPE;
 
     data_type_names_[OPT_EMPTY_TYPE] = "empty";
@@ -69,7 +69,7 @@ OptionDataTypeUtil::OptionDataTypeUtil() {
     data_type_names_[OPT_STRING_TYPE] = "string";
     data_type_names_[OPT_TUPLE_TYPE] = "tuple";
     data_type_names_[OPT_FQDN_TYPE] = "fqdn";
-    data_type_names_[OPT_CUSTOM_TYPE] = "custom";
+    data_type_names_[OPT_INTERNAL_TYPE] = "internal";
     data_type_names_[OPT_RECORD_TYPE] = "record";
     // The "unknown" data type is declared here so as
     // it can be returned by reference by a getDataTypeName

diff --git a/src/lib/dhcp/option_data_types.h b/src/lib/dhcp/option_data_types.h
index 5ef1220b0f961711f63fd7901466fe3afb91608a..021ec6ee5f8721de503cef536f4f07f013560827 100644 (file)
--- a/src/lib/dhcp/option_data_types.h
+++ b/src/lib/dhcp/option_data_types.h
@@ -59,7 +59,8 @@ enum OptionDataType {
     OPT_STRING_TYPE,
     OPT_TUPLE_TYPE,
     OPT_FQDN_TYPE,
-    OPT_CUSTOM_TYPE,
+    // Type to be used only internally. Allows convenient notation of the option config.
+    OPT_INTERNAL_TYPE,
     OPT_RECORD_TYPE,
     OPT_UNKNOWN_TYPE
 };

diff --git a/src/lib/dhcp/option_definition.cc b/src/lib/dhcp/option_definition.cc
index 2961faf222201968523a49d6ac9a87711d6a6a1a..5f111be1620e737650bca0f64eee65b25a161a19 100644 (file)
--- a/src/lib/dhcp/option_definition.cc
+++ b/src/lib/dhcp/option_definition.cc
@@ -187,7 +187,7 @@ OptionDefinition::optionFactory(Option::Universe u,
                                 uint16_t type,
                                 OptionBufferConstIter begin,
                                 OptionBufferConstIter end,
-                                bool convenient_format) const {
+                                bool convenient_notation) const {
 
     try {
         // Some of the options are represented by the specialized classes derived
@@ -196,7 +196,7 @@ OptionDefinition::optionFactory(Option::Universe u,
         // type to be returned. Therefore, we first check that if we are dealing
         // with such an option. If the instance is returned we just exit at this
         // point. If not, we will search for a generic option type to return.
-        OptionPtr option = factorySpecialFormatOption(u, begin, end, convenient_format);
+        OptionPtr option = factorySpecialFormatOption(u, begin, end, convenient_notation);
         if (option) {
             return (option);
         }
@@ -210,9 +210,9 @@ OptionDefinition::optionFactory(Option::Universe u,
             }
 
         case OPT_BINARY_TYPE:
-        // If this is Custom type, and it wasn't handled by factorySpecialFormatOption() before,
+        // If this is Internal type, and it wasn't handled by factorySpecialFormatOption() before,
         // let's treat it like normal Binary type.
-        case OPT_CUSTOM_TYPE:
+        case OPT_INTERNAL_TYPE:
             return (factoryGeneric(u, type, begin, end));
 
         case OPT_UINT8_TYPE:
@@ -312,10 +312,11 @@ OptionDefinition::optionFactory(Option::Universe u, uint16_t type,
             if (type_ != OPT_EMPTY_TYPE) {
                 isc_throw(InvalidOptionValue, "no option value specified");
             }
-        } else if (type_ == OPT_CUSTOM_TYPE) {
-            // If Custom type is used together with csv-format=true, let's treat it
-            // like String type. optionFactory() will be called with custom_data flag set to true,
-            // so that the factory will have a chance to handle it in a custom way.
+        } else if (type_ == OPT_INTERNAL_TYPE) {
+            // If an Option of type Internal is configured using csv-format=true, it means it is
+            // convenient notation option config that needs special parsing. Let's treat it like
+            // String type. optionFactory() will be called with convenient_notation flag set to
+            // true, so that the factory will have a chance to handle it in a special way.
             writeToBuffer(u, boost::algorithm::join(values, ","), OPT_STRING_TYPE, buf);
         } else {
             writeToBuffer(u, util::str::trim(values[0]), type_, buf);
@@ -341,7 +342,7 @@ OptionDefinition::optionFactory(Option::Universe u, uint16_t type,
             }
         }
     }
-    return (optionFactory(u, type, buf.begin(), buf.end(), (type_ == OPT_CUSTOM_TYPE)));
+    return (optionFactory(u, type, buf.begin(), buf.end(), (type_ == OPT_INTERNAL_TYPE)));
 }
 
 void
@@ -413,9 +414,9 @@ OptionDefinition::validate() const {
                             << " an option record.";
                     break;
                 }
-                // Custom type is not allowed within a record.
-                if (*it == OPT_CUSTOM_TYPE) {
-                    err_str << "custom data type can't be stored as a field in"
+                // Internal type is not allowed within a record.
+                if (*it == OPT_INTERNAL_TYPE) {
+                    err_str << "internal data type can't be stored as a field in"
                             << " an option record.";
                     break;
                 }
@@ -448,8 +449,8 @@ OptionDefinition::validate() const {
             err_str << "array of empty value is not"
                     << " a valid option definition.";
 
-        } else if (type_ == OPT_CUSTOM_TYPE) {
-            err_str << "array of custom type value is not"
+        } else if (type_ == OPT_INTERNAL_TYPE) {
+            err_str << "array of internal type value is not"
                     << " a valid option definition.";
 
         }
@@ -846,7 +847,7 @@ OptionPtr
 OptionDefinition::factorySpecialFormatOption(Option::Universe u,
                                              OptionBufferConstIter begin,
                                              OptionBufferConstIter end,
-                                             bool convenient_format) const {
+                                             bool convenient_notation) const {
     if ((u == Option::V6) && haveSpace(DHCP6_OPTION_SPACE)) {
         switch (getCode()) {
         case D6O_IA_NA:
@@ -908,7 +909,7 @@ OptionDefinition::factorySpecialFormatOption(Option::Universe u,
             return (OptionPtr(new Option4ClientFqdn(begin, end)));
 
         case DHO_CLASSLESS_STATIC_ROUTE:
-            return (OptionPtr(new OptionClasslessStaticRoute(begin, end, convenient_format)));
+            return (OptionPtr(new OptionClasslessStaticRoute(begin, end, convenient_notation)));
 
         case DHO_VIVCO_SUBOPTIONS:
             // Record of uint32 followed by binary.

diff --git a/src/lib/dhcp/option_definition.h b/src/lib/dhcp/option_definition.h
index 12955308eba09665641ef3696d6e621fe9cbd835..f1208ca3dd5232d5d0e6bcbbfc79829c4d63e1eb 100644 (file)
--- a/src/lib/dhcp/option_definition.h
+++ b/src/lib/dhcp/option_definition.h
@@ -433,10 +433,10 @@ public:
     /// @param type option type.
     /// @param begin beginning of the option buffer.
     /// @param end end of the option buffer.
-    /// @param convenient_format flag which indicates that the buffer contains option data
-    ///                          as a string formatted in user-friendly, convenient way.
-    ///                          The flag is propagated to the option constructor, so that
-    ///                          the data could be parsed properly. Defaults to false.
+    /// @param convenient_notation flag which indicates that the buffer contains option data
+    ///                            as a string formatted in user-friendly, convenient way.
+    ///                            The flag is propagated to the option constructor, so that
+    ///                            the data could be parsed properly. Defaults to false.
     ///
     /// @return instance of the DHCP option.
     /// @throw InvalidOptionValue if data for the option is invalid.
@@ -444,7 +444,7 @@ public:
                             uint16_t type,
                             OptionBufferConstIter begin,
                             OptionBufferConstIter end,
-                            bool convenient_format = false) const;
+                            bool convenient_notation = false) const;
 
     /// @brief Option factory.
     ///
@@ -676,10 +676,10 @@ private:
     /// @param u A universe (V4 or V6).
     /// @param begin beginning of the option buffer.
     /// @param end end of the option buffer.
-    /// @param convenient_format flag which indicates that the buffer contains option data
-    ///                          as a string formatted in user-friendly, convenient way.
-    ///                          The flag is propagated to the option constructor, so that
-    ///                          the data could be parsed properly. Defaults to false.
+    /// @param convenient_notation flag which indicates that the buffer contains option data
+    ///                            as a string formatted in user-friendly, convenient way.
+    ///                            The flag is propagated to the option constructor, so that
+    ///                            the data could be parsed properly. Defaults to false.
     ///
     /// @return An instance of the option having special format or NULL if
     /// such an option can't be created because an option with the given
@@ -687,7 +687,7 @@ private:
     OptionPtr factorySpecialFormatOption(Option::Universe u,
                                          OptionBufferConstIter begin,
                                          OptionBufferConstIter end,
-                                         bool convenient_format = false) const;
+                                         bool convenient_notation = false) const;
 
     /// @brief Check if specified type matches option definition type.
     ///

diff --git a/src/lib/dhcp/std_option_defs.h b/src/lib/dhcp/std_option_defs.h
index db2bded0e5b3c0acedf9c203ce83dc0ecf7f9fa5..5e8acec8b6599ab18f02d8f4c0fc014f16cef7a3 100644 (file)
--- a/src/lib/dhcp/std_option_defs.h
+++ b/src/lib/dhcp/std_option_defs.h
@@ -332,7 +332,7 @@ const OptionDefParams STANDARD_V4_OPTION_DEFINITIONS[] = {
       OPT_IPV4_ADDRESS_TYPE, false, NO_RECORD_DEF, "" },
     { "domain-search", DHO_DOMAIN_SEARCH, DHCP4_OPTION_SPACE, OPT_FQDN_TYPE,
       true, NO_RECORD_DEF, "" },
-    { "classless-static-route", DHO_CLASSLESS_STATIC_ROUTE, DHCP4_OPTION_SPACE, OPT_CUSTOM_TYPE,
+    { "classless-static-route", DHO_CLASSLESS_STATIC_ROUTE, DHCP4_OPTION_SPACE, OPT_INTERNAL_TYPE,
       false, NO_RECORD_DEF, "" },
     { "vivco-suboptions", DHO_VIVCO_SUBOPTIONS, DHCP4_OPTION_SPACE,
       OPT_RECORD_TYPE, false, RECORD_DEF(VIVCO_RECORDS), "" },

diff --git a/src/lib/dhcp/tests/option_definition_unittest.cc b/src/lib/dhcp/tests/option_definition_unittest.cc
index 647533da6a5bf0a66cd0e067a065127c9ea71d05..f1f51add064a69105b10809eb66b39856019c864 100644 (file)
--- a/src/lib/dhcp/tests/option_definition_unittest.cc
+++ b/src/lib/dhcp/tests/option_definition_unittest.cc
@@ -2106,13 +2106,14 @@ TEST_F(OptionDefinitionTest, tuple6ArrayTokenized) {
     EXPECT_EQ("world", tuple2.getText());
 }
 
-// The purpose of this test is to verify creation of OPT_CUSTOM_TYPE option
+// The purpose of this test is to verify creation of OPT_INTERNAL_TYPE option
 // definition. OptionFactory is used which takes vector of string values as
-// data for the option. This is special case for OPT_CUSTOM_TYPE because it means
-// that custom data is provided in string values. Custom parsing is verified in this test.
-TEST_F(OptionDefinitionTest, customOptionTypeString) {
+// data for the option. This is special case for OPT_INTERNAL_TYPE because it means
+// that convenient notation option config is provided as string that needs special parsing.
+// Custom parsing is verified in this test.
+TEST_F(OptionDefinitionTest, internalOptionTypeString) {
     OptionDefinition opt_def("classless-static-route", DHO_CLASSLESS_STATIC_ROUTE,
-                             DHCP4_OPTION_SPACE, "custom", false);
+                             DHCP4_OPTION_SPACE, "internal", false);
 
     OptionPtr option;
 
@@ -2148,12 +2149,12 @@ TEST_F(OptionDefinitionTest, customOptionTypeString) {
               option_cast->toText());
 }
 
-// The purpose of this test is to verify creation of OPT_CUSTOM_TYPE option
+// The purpose of this test is to verify creation of OPT_INTERNAL_TYPE option
 // definition. OptionFactory is used which takes OptionBuffer as
 // data for the option. Binary data unpack is verified in this test.
-TEST_F(OptionDefinitionTest, customOptionTypeBinary) {
+TEST_F(OptionDefinitionTest, internalOptionTypeBinary) {
     OptionDefinition opt_def("classless-static-route", DHO_CLASSLESS_STATIC_ROUTE,
-                             DHCP4_OPTION_SPACE, "custom", false);
+                             DHCP4_OPTION_SPACE, "internal", false);
 
     OptionPtr option;