[#2596] Checkpoint for #2595 update

diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index 9fcdcce2ec9e7521eb1803a0e261829a1d22aa08..86996ebf9059ca6293cb278659fbfeb4420b9165 100644 (file)
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@ -1924,7 +1924,7 @@ AllocEngine::reuseExpiredLease(Lease6Ptr& expired, ClientContext6& ctx,
 
     if (!ctx.fake_allocation_) {
         // Add(update) the extended information on the lease.
-        static_cast<void>(updateLease6ExtendedInfo(expired, ctx));
+        updateLease6ExtendedInfo(expired, ctx);
 
         // for REQUEST we do update the lease
         LeaseMgrFactory::instance().updateLease6(expired);
@@ -2085,7 +2085,7 @@ Lease6Ptr AllocEngine::createLease6(ClientContext6& ctx,
 
     if (!ctx.fake_allocation_) {
         // Add(update) the extended information on the lease.
-        static_cast<void>(updateLease6ExtendedInfo(lease, ctx));
+        updateLease6ExtendedInfo(lease, ctx);
 
         // That is a real (REQUEST) allocation
         bool status = LeaseMgrFactory::instance().addLease(lease);
@@ -2401,7 +2401,8 @@ AllocEngine::extendLease6(ClientContext6& ctx, Lease6Ptr lease) {
         }
 
         // @todo should we call storeLease6ExtendedInfo() here ?
-        if (updateLease6ExtendedInfo(lease, ctx)) {
+        updateLease6ExtendedInfo(lease, ctx);
+        if (lease->extended_info_action_ == ExtendedInfoAction::ACTION_UPDATE) {
             changed = true;
         }
 
@@ -3145,7 +3146,10 @@ void AllocEngine::reclaimLeaseInDatabase(const LeasePtrType& lease,
         lease->fqdn_fwd_ = false;
         lease->fqdn_rev_ = false;
         lease->state_ = Lease::STATE_EXPIRED_RECLAIMED;
-        lease->setContext(ElementPtr());
+        if (lease->getContext()) {
+            lease->extended_info_action_ = ExtendedInfoAction::ACTION_DELETE;
+            lease->setContext(ElementPtr());
+        }
         lease_update_fun(lease);
 
     } else {
@@ -4043,7 +4047,7 @@ AllocEngine::createLease4(const ClientContext4& ctx, const IOAddress& addr,
     lease->hostname_ = ctx.hostname_;
 
     // Add(update) the extended information on the lease.
-    static_cast<void>(updateLease4ExtendedInfo(lease, ctx));
+    updateLease4ExtendedInfo(lease, ctx);
 
     // Let's execute all callouts registered for lease4_select
     if (ctx.callout_handle_ &&
@@ -4623,28 +4627,27 @@ AllocEngine::updateLease4Information(const Lease4Ptr& lease,
     }
 
     // Add(update) the extended information on the lease.
-    if (updateLease4ExtendedInfo(lease, ctx)) {
+    updateLease4ExtendedInfo(lease, ctx);
+    if (lease->extended_info_action_ == ExtendedInfoAction::ACTION_UPDATE) {
         changed = true;
     }
 
     return (changed);
 }
 
-bool
+void
 AllocEngine::updateLease4ExtendedInfo(const Lease4Ptr& lease,
                                       const AllocEngine::ClientContext4& ctx) const {
-    bool changed = false;
-
     // If storage is not enabled then punt.
     if (!ctx.subnet_->getStoreExtendedInfo()) {
-        return (changed);
+        return;
     }
 
     // Look for relay agent information option (option 82)
     OptionPtr rai = ctx.query_->getOption(DHO_DHCP_AGENT_OPTIONS);
     if (!rai) {
         // Pkt4 doesn't have it, so nothing to store (or update).
-        return (changed);
+        return;
     }
 
     // Create a StringElement with the hex string for relay-agent-info.
@@ -4693,30 +4696,26 @@ AllocEngine::updateLease4ExtendedInfo(const Lease4Ptr& lease,
     // Add/replace the extended info entry.
     ConstElementPtr old_extended_info = mutable_isc->get("relay-agent-info");
     if (!old_extended_info || (*old_extended_info != *extended_info)) {
-        changed = true;
+        lease->extended_info_action_ = ExtendedInfoAction::ACTION_UPDATE;
         mutable_isc->set("relay-agent-info", extended_info);
         mutable_user_context->set("ISC", mutable_isc);
     }
 
     // Update the lease's user_context.
     lease->setContext(mutable_user_context);
-
-    return (changed);
 }
 
 bool
 AllocEngine::updateLease6ExtendedInfo(const Lease6Ptr& lease,
                                       const AllocEngine::ClientContext6& ctx) const {
-    bool changed = false;
-
     // If storage is not enabled then punt.
     if (!ctx.subnet_->getStoreExtendedInfo()) {
-        return (changed);
+        return;
     }
 
     // If we do not have relay information, then punt.
     if (ctx.query_->relay_info_.empty()) {
-        return (changed);
+        return;
     }
 
     // We need to convert the vector of RelayInfo instances in
@@ -4802,15 +4801,13 @@ AllocEngine::updateLease6ExtendedInfo(const Lease6Ptr& lease,
     // Add/replace the extended info entry.
     ConstElementPtr old_extended_info = mutable_isc->get("relay-info");
     if (!old_extended_info || (*old_extended_info != *extended_info)) {
-        changed = true;
+        lease->extended_info_action_ = ExtendedInfoAction::ACTION_UPDATE;
         mutable_isc->set("relay-info", extended_info);
         mutable_user_context->set("ISC", mutable_isc);
     }
 
     // Update the lease's user context.
     lease->setContext(mutable_user_context);
-
-    return (changed);
 }
 
 void

diff --git a/src/lib/dhcpsrv/alloc_engine.h b/src/lib/dhcpsrv/alloc_engine.h
index 681475f545aa93da61110f6c7b3a71bb84c3fa1c..07257145a1b967999c5d2064e288d0ea6b84cae2 100644 (file)
--- a/src/lib/dhcpsrv/alloc_engine.h
+++ b/src/lib/dhcpsrv/alloc_engine.h
@@ -1898,9 +1898,7 @@ protected:
     /// @param [out] lease A pointer to the lease to be updated.
     /// @param ctx A context containing information from the server about the
     /// client and its message.
-    /// @return True if there was a significant (e.g. other than cltt) change,
-    /// false otherwise.
-    bool updateLease4ExtendedInfo(const Lease4Ptr& lease,
+    void updateLease4ExtendedInfo(const Lease4Ptr& lease,
                                   const ClientContext4& ctx) const;
 
     /// @brief Stores additional client query parameters on a V6 lease
@@ -1918,9 +1916,7 @@ protected:
     /// @param [out] lease A pointer to the lease to be updated.
     /// @param ctx A context containing information from the server about the
     /// client and its message.
-    /// @return True if there was a significant (e.g. other than cltt) change,
-    /// false otherwise.
-    bool updateLease6ExtendedInfo(const Lease6Ptr& lease,
+    void updateLease6ExtendedInfo(const Lease6Ptr& lease,
                                   const ClientContext6& ctx) const;
 
 private:

diff --git a/src/lib/dhcpsrv/lease.cc b/src/lib/dhcpsrv/lease.cc
index d14c53b273dc60bfa70105f98f365cfe891712b6..0d529cdd20b1de1e7c85ac378122ed6cdb4d1dcd 100644 (file)
--- a/src/lib/dhcpsrv/lease.cc
+++ b/src/lib/dhcpsrv/lease.cc
@@ -44,7 +44,8 @@ Lease::Lease(const isc::asiolink::IOAddress& addr,
       reuseable_valid_lft_(0),
       cltt_(cltt), current_cltt_(cltt), subnet_id_(subnet_id),
       hostname_(boost::algorithm::to_lower_copy(hostname)), fqdn_fwd_(fqdn_fwd),
-      fqdn_rev_(fqdn_rev), hwaddr_(hwaddr), state_(STATE_DEFAULT) {
+      fqdn_rev_(fqdn_rev), hwaddr_(hwaddr), state_(STATE_DEFAULT),
+      extended_info_action_(ExtendedInfoAction::ACTION_IGNORE) {
 }
 
 

diff --git a/src/lib/dhcpsrv/lease.h b/src/lib/dhcpsrv/lease.h
index 0c2c7919038c4d276a2b10deb07683fbd9c5d00a..73a913e8d449220de75422841467d89d800de2af 100644 (file)
--- a/src/lib/dhcpsrv/lease.h
+++ b/src/lib/dhcpsrv/lease.h
@@ -184,6 +184,16 @@ struct Lease : public isc::data::UserContext, public isc::data::CfgToElement {
     /// belonging to this class.
     uint32_t state_;
 
+    /// @brief Action on extended info tables.
+    typedef enum {
+        ACTION_IGNORE, ///< ignore extended info,
+        ACTION_DELETE, ///< delete reference to the lease
+        ACTION_UPDATE  ///< update extended info tables.
+    } ExtendedInfoAction;
+
+    /// @brief Record the action on extended info tables in the lease.
+    ExtendedInfoAction extended_info_action_;
+
     /// @brief Convert Lease to Printable Form
     ///
     /// @return String form of the lease

diff --git a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
index 7e6ec5dfba72e8d4bfa70f0e088268ccdfa5964b..3afce6887b916dad28160806cc529b14c0a99ebd 100644 (file)
--- a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
@@ -3851,8 +3851,10 @@ TEST_F(AllocEngine4Test, updateExtendedInfo4) {
         }
 
         // Call AllocEngine::updateLease4ExtendeInfo().
-        bool ret = false;
-        ASSERT_NO_THROW_LOG(ret = engine.callUpdateLease4ExtendedInfo(lease, ctx));
+        ASSERT_NO_THROW_LOG(engine.callUpdateLease4ExtendedInfo(lease, ctx));
+        bool ret = (lease->extended_info_action_ == ExtendedInfoAction::ACTION_UPDATE);
+        // Reset the lease action.
+        lease->extended_info_action_ = ExtendedInfoAction::ACTION_IGNORE;
         ASSERT_EQ(scenario.exp_ret, ret);
 
         // Verify the lease has the expected user context content.

diff --git a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
index 74182e145c816c4bbac5775e28f6b78bd93e8020..5f8f5d802eb61c53afbcbcb4e928a3a2aed433c9 100644 (file)
--- a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
@@ -4299,8 +4299,10 @@ TEST_F(AllocEngine6ExtendedInfoTest, updateExtendedInfo6) {
         ctx.query_->relay_info_ = scenario.relays_;
 
         // Call AllocEngine::updateLease6ExtendeInfo().
-        bool ret;
-        ASSERT_NO_THROW_LOG(ret = engine_.callUpdateLease6ExtendedInfo(lease, ctx));
+        ASSERT_NO_THROW_LOG(engine_.callUpdateLease6ExtendedInfo(lease, ctx));
+        bool ret = (lease->extended_info_action_ == ExtendedInfoAction::ACTION_UPDATE);
+        // Reset the lease action.
+        lease->extended_info_action_ = ExtendedInfoAction::ACTION_IGNORE;
         ASSERT_EQ(scenario.exp_ret, ret);
 
         // Verify the lease has the expected user context content.

diff --git a/src/lib/dhcpsrv/tests/alloc_engine_utils.h b/src/lib/dhcpsrv/tests/alloc_engine_utils.h
index 2e4ffbef107e34b99e9a846f140bafaa884522e8..be8152419e69ddab261070869e6eb33311781d1f 100644 (file)
--- a/src/lib/dhcpsrv/tests/alloc_engine_utils.h
+++ b/src/lib/dhcpsrv/tests/alloc_engine_utils.h
@@ -93,8 +93,7 @@ public:
     /// @brief Wrapper method for invoking AllocEngine4::updateLease4ExtendedInfo().
     /// @param lease lease to update
     /// @param ctx current packet processing context
-    /// @return true if extended information was changed
-    bool callUpdateLease4ExtendedInfo(const Lease4Ptr& lease,
+    void callUpdateLease4ExtendedInfo(const Lease4Ptr& lease,
                                       AllocEngine::ClientContext4& ctx) const {
         return (updateLease4ExtendedInfo(lease, ctx));
     }
@@ -102,8 +101,7 @@ public:
     /// @brief Wrapper method for invoking AllocEngine6::updateLease6ExtendedInfo().
     /// @param lease lease to update
     /// @param ctx current packet processing context
-    /// @return true if extended information was changed
-    bool callUpdateLease6ExtendedInfo(const Lease6Ptr& lease,
+    void callUpdateLease6ExtendedInfo(const Lease6Ptr& lease,
                                       AllocEngine::ClientContext6& ctx) const {
         return (updateLease6ExtendedInfo(lease, ctx));
     }