From: Marcin Siodelski <marcin@isc.org>
Date: Tue, 26 Mar 2019 10:19:58 +0000 (+0100)
Subject: [#490,!284] Use inheritance for all optional parameters in Network.
X-Git-Tag: Kea-1.6.0-beta~304
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=70e95490cf31be69badd10b024f413b95f99bb69;p=thirdparty%2Fkea.git

[#490,!284] Use inheritance for all optional parameters in Network.
---

diff --git a/src/lib/dhcpsrv/network.cc b/src/lib/dhcpsrv/network.cc
index 2c13a4779c..59f599bf92 100644
--- a/src/lib/dhcpsrv/network.cc
+++ b/src/lib/dhcpsrv/network.cc
@@ -107,8 +107,8 @@ Network::toElement() const {
     contextToElement(map);
 
     // Set interface
-    if (!getIface().unspecified()) {
-        map->set("interface", Element::create(getIface().get()));
+    if (!iface_name_.unspecified()) {
+        map->set("interface", Element::create(iface_name_.get()));
     }
 
     ElementPtr relay_map = Element::createMap();
@@ -122,8 +122,8 @@ Network::toElement() const {
     map->set("relay", relay_map);
 
     // Set client-class
-    if (!getClientClass().unspecified()) {
-        map->set("client-class", Element::create(getClientClass().get()));
+    if (!client_class_.unspecified()) {
+        map->set("client-class", Element::create(client_class_.get()));
     }
 
     // Set require-client-classes
@@ -140,22 +140,21 @@ Network::toElement() const {
     // T1, T2, and Valid are optional for SharedNetworks, and
     // T1 and T2 are optional for Subnet4 thus we will only
     // output them if they are marked as specified.
-    if (!getT1().unspecified()) {
+    if (!t1_.unspecified()) {
         map->set("renew-timer",
-                 Element::create(static_cast<long long>(getT1().get())));
+                 Element::create(static_cast<long long>(t1_.get())));
     }
 
     // Set rebind-timer
-    if (!getT2().unspecified()) {
+    if (!t2_.unspecified()) {
         map->set("rebind-timer",
-                 Element::create(static_cast<long long>(getT2().get())));
+                 Element::create(static_cast<long long>(t2_.get())));
     }
 
     // Set valid-lifetime
-    if (!getValid().unspecified()) {
+    if (!valid_.unspecified()) {
         map->set("valid-lifetime",
-                 Element::create(static_cast<long long>
-                                 (getValid().get())));
+                 Element::create(static_cast<long long>(valid_.get())));
     }
 
     // Set reservation mode
@@ -186,20 +185,17 @@ Network::toElement() const {
     ConstCfgOptionPtr opts = getCfgOption();
     map->set("option-data", opts->toElement());
 
-    // Output calcualte-tee-times and percentages if calculation is enabled.
-    auto calc_tee_times = getCalculateTeeTimes();
-    if (!calc_tee_times.unspecified()) {
-        map->set("calculate-tee-times", Element::create(calc_tee_times));
+    // Output calculate-tee-times and percentages if calculation is enabled.
+    if (!calculate_tee_times_.unspecified()) {
+        map->set("calculate-tee-times", Element::create(calculate_tee_times_));
     }
 
-    auto t1_percent = getT1Percent();
-    if (!t1_percent.unspecified()) {
-        map->set("t1-percent", Element::create(getT1Percent()));
+    if (!t1_percent_.unspecified()) {
+        map->set("t1-percent", Element::create(t1_percent_));
     }
 
-    auto t2_percent = getT2Percent();
-    if (!t2_percent.unspecified()) {
-        map->set("t2-percent", Element::create(getT2Percent()));
+    if (!t2_percent_.unspecified()) {
+        map->set("t2-percent", Element::create(t2_percent_));
     }
 
     return (map);
@@ -214,38 +210,23 @@ Network4::setSiaddr(const Optional<IOAddress>& siaddr) {
     siaddr_ = siaddr;
 }
 
-const Optional<IOAddress>&
-Network4::getSiaddr() const {
-    return (siaddr_);
-}
-
 void
 Network4::setSname(const Optional<std::string>& sname) {
     sname_ = sname;
 }
 
-const Optional<std::string>&
-Network4::getSname() const {
-    return (sname_);
-}
-
 void
 Network4::setFilename(const Optional<std::string>& filename) {
     filename_ = filename;
 }
 
-const Optional<std::string>&
-Network4::getFilename() const {
-    return (filename_);
-}
-
 ElementPtr
 Network4::toElement() const {
     ElementPtr map = Network::toElement();
 
     // Set match-client-id
-    if (!getMatchClientId().unspecified()) {
-        map->set("match-client-id", Element::create(getMatchClientId().get()));
+    if (!match_client_id_.unspecified()) {
+        map->set("match-client-id", Element::create(match_client_id_.get()));
     }
 
     // Set authoritative
@@ -254,18 +235,18 @@ Network4::toElement() const {
     }
 
     // Set next-server
-    if (!getSiaddr().unspecified()) {
-        map->set("next-server", Element::create(getSiaddr().get().toText()));
+    if (!siaddr_.unspecified()) {
+        map->set("next-server", Element::create(siaddr_.get().toText()));
     }
 
     // Set server-hostname
-    if (!getSname().unspecified()) {
-        map->set("server-hostname", Element::create(getSname().get()));
+    if (!sname_.unspecified()) {
+        map->set("server-hostname", Element::create(sname_.get()));
     }
 
     // Set boot-file-name
-    if (!getFilename().unspecified()) {
-        map->set("boot-file-name",Element::create(getFilename().get()));
+    if (!filename_.unspecified()) {
+        map->set("boot-file-name",Element::create(filename_.get()));
     }
 
     return (map);
diff --git a/src/lib/dhcpsrv/network.h b/src/lib/dhcpsrv/network.h
index f3d3d9f059..583960745c 100644
--- a/src/lib/dhcpsrv/network.h
+++ b/src/lib/dhcpsrv/network.h
@@ -311,13 +311,13 @@ public:
     /// returned it is valid.
     ///
     /// @return client class @ref client_class_
-    const util::Optional<ClientClass>& getClientClass() const {
-        return (client_class_);
+    util::Optional<ClientClass> getClientClass() const {
+        return (getProperty<Network>(&Network::getClientClass, client_class_));
     }
 
     /// @brief Return valid-lifetime for addresses in that prefix
     Triplet<uint32_t> getValid() const {
-        return (valid_);
+        return (getProperty<Network>(&Network::getValid, valid_, "valid-lifetime"));
     }
 
     /// @brief Sets new valid lifetime for a network.
@@ -329,7 +329,7 @@ public:
 
     /// @brief Returns T1 (renew timer), expressed in seconds
     Triplet<uint32_t> getT1() const {
-        return (t1_);
+        return (getProperty<Network>(&Network::getT1, t1_, "renew-timer"));
     }
 
     /// @brief Sets new renew timer for a network.
@@ -341,7 +341,7 @@ public:
 
     /// @brief Returns T2 (rebind timer), expressed in seconds
     Triplet<uint32_t> getT2() const {
-        return (t2_);
+        return (getProperty<Network>(&Network::getT2, t2_, "rebind-timer"));
     }
 
     /// @brief Sets new rebind timer for a network.
@@ -388,7 +388,9 @@ public:
 
     /// @brief Returns whether or not T1/T2 calculation is enabled.
     util::Optional<bool> getCalculateTeeTimes() const {
-        return (calculate_tee_times_);
+        return (getProperty<Network>(&Network::getCalculateTeeTimes,
+                                     calculate_tee_times_,
+                                     "calculate-tee-times"));
     }
 
     /// @brief Sets whether or not T1/T2 calculation is enabled.
@@ -400,7 +402,9 @@ public:
 
     /// @brief Returns percentage to use when calculating the T1 (renew timer).
     util::Optional<double> getT1Percent() const {
-        return (t1_percent_);
+        return (getProperty<Network>(&Network::getT1Percent,
+                                     t1_percent_,
+                                     "t1-percent"));
     }
 
     /// @brief Sets new precentage for calculating T1 (renew timer).
@@ -412,7 +416,9 @@ public:
 
     /// @brief Returns percentage to use when calculating the T2 (rebind timer).
     util::Optional<double> getT2Percent() const {
-        return (t2_percent_);
+        return (getProperty<Network>(&Network::getT2Percent,
+                                     t2_percent_,
+                                     "t2-percent"));
     }
 
     /// @brief Sets new precentage for calculating T2 (rebind timer).
@@ -430,9 +436,10 @@ public:
 protected:
 
     template<typename BaseType, typename ReturnType>
-    util::Optional<ReturnType>
-    getProperty(util::Optional<ReturnType>(BaseType::*MethodPointer)() const,
-                util::Optional<ReturnType> property, const std::string& global_name) const {
+    ReturnType
+    getProperty(ReturnType(BaseType::*MethodPointer)() const,
+                ReturnType property,
+                const std::string& global_name = "") const {
         if (property.unspecified()) {
             auto parent = boost::dynamic_pointer_cast<BaseType>(parent_network_.lock());
             if (parent) {
@@ -443,12 +450,12 @@ protected:
                 }
             }
 
-            if (fetch_globals_fn_) {
+            if (!global_name.empty() && fetch_globals_fn_) {
                 data::ConstElementPtr globals = fetch_globals_fn_();
                 if (globals && (globals->getType() == data::Element::map)) {
                     data::ConstElementPtr global_param = globals->get(global_name);
                     if (global_param) {
-                        return (data::ElementExtractor<ReturnType>()(global_param));
+                        return (data::ElementExtractor<typename ReturnType::ValueType>()(global_param));
                     }
                 }
             }
@@ -532,7 +539,9 @@ public:
     ///
     /// @return true if client identifiers should be used, false otherwise.
     util::Optional<bool> getMatchClientId() const {
-        return (match_client_id_);
+        return (getProperty<Network4>(&Network4::getMatchClientId,
+                                      match_client_id_,
+                                      "match-client-id"));
     }
 
     /// @brief Sets the flag indicating if the client identifier should be
@@ -573,7 +582,9 @@ public:
     /// @brief Returns siaddr for this network.
     ///
     /// @return siaddr value
-    const util::Optional<asiolink::IOAddress>& getSiaddr() const;
+    util::Optional<asiolink::IOAddress> getSiaddr() const {
+        return (siaddr_);
+    }
 
     /// @brief Sets server hostname for the network.
     ///
@@ -583,7 +594,9 @@ public:
     /// @brief Returns server hostname for this network.
     ///
     /// @return server hostname value
-    const util::Optional<std::string>& getSname() const;
+    util::Optional<std::string> getSname() const {
+        return (sname_);
+    }
 
     /// @brief Sets boot file name for the network.
     ///
@@ -593,7 +606,9 @@ public:
     /// @brief Returns boot file name for this subnet
     ///
     /// @return boot file name value
-    const util::Optional<std::string>& getFilename() const;
+    util::Optional<std::string> getFilename() const {
+        return (filename_);
+    }
 
     /// @brief Unparses network object.
     ///
diff --git a/src/lib/dhcpsrv/tests/network_unittest.cc b/src/lib/dhcpsrv/tests/network_unittest.cc
index 996a9ee7b6..a9d9a609f3 100644
--- a/src/lib/dhcpsrv/tests/network_unittest.cc
+++ b/src/lib/dhcpsrv/tests/network_unittest.cc
@@ -42,6 +42,10 @@ public:
         return (globals_);
     }
 
+    template<typename>
+    void testNetworkInheritance() {
+    }
+
     ElementPtr globals_;
 };
 
diff --git a/src/lib/util/optional.h b/src/lib/util/optional.h
index d75787a74b..5d1cc98e0d 100644
--- a/src/lib/util/optional.h
+++ b/src/lib/util/optional.h
@@ -36,6 +36,9 @@ template<typename T>
 class Optional {
 public:
 
+    /// @brief Type of the encapsulated value.
+    typedef T ValueType;
+
     /// @brief Assigns a new value value and marks it "specified".
     ///
     /// @tparam A Type of the value to be assigned. Typically this is @c T, but