]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#1127] removed shared callout manager
authorRazvan Becheriu <razvan@isc.org>
Thu, 14 May 2020 13:24:26 +0000 (16:24 +0300)
committerRazvan Becheriu <razvan@isc.org>
Fri, 15 May 2020 18:21:41 +0000 (21:21 +0300)
src/bin/dhcp4/tests/hooks_unittest.cc
src/bin/dhcp6/tests/hooks_unittest.cc
src/lib/hooks/hooks_manager.cc
src/lib/hooks/hooks_manager.h
src/lib/hooks/library_manager_collection.cc
src/lib/hooks/tests/hooks_manager_unittest.cc

index 9c4203e3316a5129352bd8d0f28a71391c066cc7..a7a0abee21c7dbc21fd1b15a485cf28bde81a961 100644 (file)
@@ -105,6 +105,7 @@ public:
 
     /// @brief creates Dhcpv4Srv and prepares buffers for callouts
     HooksDhcpv4SrvTest() {
+        HooksManager::getHooksManager().setTestMode(false);
 
         // Allocate new DHCPv6 Server
         srv_ = new NakedDhcpv4Srv(0);
@@ -129,8 +130,8 @@ public:
         HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts("lease4_decline");
         HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts("host4_identifier");
 
-        HooksManager::getHooksManager().setSharedCalloutManager();
         delete srv_;
+        HooksManager::getHooksManager().setTestMode(false);
     }
 
     /// @brief creates an option with specified option code
@@ -2377,14 +2378,12 @@ TEST_F(HooksDhcpv4SrvTest, HooksDecline) {
     IfaceMgrTestConfig test_config(true);
     IfaceMgr::instance().openSockets4();
 
-    // Libraries will be reloaded later
-    HooksManager::getHooksManager().setSharedCalloutManager(
-        boost::shared_ptr<CalloutManager>(new CalloutManager(0)));
-
     // Install a callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
                         "lease4_decline", lease4_decline_callout));
 
+    HooksManager::getHooksManager().setTestMode(true);
+
     // Conduct the actual DORA + Decline.
     Dhcp4Client client(Dhcp4Client::SELECTING);
     acquireAndDecline(client, "01:02:03:04:05:06", "12:14",
@@ -2430,14 +2429,12 @@ TEST_F(HooksDhcpv4SrvTest, HooksDeclineSkip) {
     IfaceMgrTestConfig test_config(true);
     IfaceMgr::instance().openSockets4();
 
-    // Libraries will be reloaded later
-    HooksManager::getHooksManager().setSharedCalloutManager(
-        boost::shared_ptr<CalloutManager>(new CalloutManager(0)));
-
     // Install a callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
                         "lease4_decline", lease4_decline_skip_callout));
 
+    HooksManager::getHooksManager().setTestMode(true);
+
     // Conduct the actual DORA + Decline. The DECLINE should fail, as the
     // hook will set the status to SKIP.
     Dhcp4Client client(Dhcp4Client::SELECTING);
@@ -2482,14 +2479,12 @@ TEST_F(HooksDhcpv4SrvTest, HooksDeclineDrop) {
     IfaceMgrTestConfig test_config(true);
     IfaceMgr::instance().openSockets4();
 
-    // Libraries will be reloaded later
-    HooksManager::getHooksManager().setSharedCalloutManager(
-        boost::shared_ptr<CalloutManager>(new CalloutManager(0)));
-
     // Install a callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
                         "lease4_decline", lease4_decline_drop_callout));
 
+    HooksManager::getHooksManager().setTestMode(true);
+
     // Conduct the actual DORA + Decline. The DECLINE should fail, as the
     // hook will set the status to DROP.
     Dhcp4Client client(Dhcp4Client::SELECTING);
index 11a2d0face2790d53fba402985833473e495dbd2..c25f55c35a21a3ceb3d035ef0859d92d02fa664f 100644 (file)
@@ -122,6 +122,8 @@ public:
     HooksDhcpv6SrvTest()
         : Dhcpv6SrvTest() {
 
+        HooksManager::getHooksManager().setTestMode(false);
+
         // Allocate new DHCPv6 Server
         srv_.reset(new NakedDhcpv6Srv(0));
 
@@ -136,10 +138,7 @@ public:
 
     /// @brief destructor (deletes Dhcpv6Srv)
     ~HooksDhcpv6SrvTest() {
-
-        // Clear shared manager
-        HooksManager::getHooksManager().setSharedCalloutManager();
-
+        HooksManager::getHooksManager().setTestMode(false);
     }
 
     /// @brief creates an option with specified option code
@@ -4417,14 +4416,12 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRebindPrefix) {
 TEST_F(HooksDhcpv6SrvTest, basicLease6Decline) {
     IfaceMgrTestConfig test_config(true);
 
-    // Libraries will be reloaded later
-    HooksManager::getHooksManager().setSharedCalloutManager(
-        boost::shared_ptr<CalloutManager>(new CalloutManager(0)));
-
     // Install lease6_decline callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
                         "lease6_decline", lease6_decline_callout));
 
+    HooksManager::getHooksManager().setTestMode(true);
+
     // Get an address and decline it. DUIDs, IAID match and we send valid
     // address, so the decline procedure should be successful.
     Dhcp6Client client;
@@ -4471,14 +4468,12 @@ TEST_F(HooksDhcpv6SrvTest, basicLease6Decline) {
 TEST_F(HooksDhcpv6SrvTest, lease6DeclineSkip) {
     IfaceMgrTestConfig test_config(true);
 
-    // Libraries will be reloaded later
-    HooksManager::getHooksManager().setSharedCalloutManager(
-        boost::shared_ptr<CalloutManager>(new CalloutManager(0)));
-
     // Install lease6_decline_skip callout. It will set the status to skip
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
                         "lease6_decline", lease6_decline_skip));
 
+    HooksManager::getHooksManager().setTestMode(true);
+
     // Get an address and decline it. DUIDs, IAID match and we send valid
     // address, so the decline procedure should be successful.
     Dhcp6Client client;
@@ -4522,14 +4517,12 @@ TEST_F(HooksDhcpv6SrvTest, lease6DeclineSkip) {
 TEST_F(HooksDhcpv6SrvTest, lease6DeclineDrop) {
     IfaceMgrTestConfig test_config(true);
 
-    // Libraries will be reloaded later
-    HooksManager::getHooksManager().setSharedCalloutManager(
-        boost::shared_ptr<CalloutManager>(new CalloutManager(0)));
-
     // Install lease6_decline_drop callout. It will set the status to drop
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
                         "lease6_decline", lease6_decline_drop));
 
+    HooksManager::getHooksManager().setTestMode(true);
+
     // Get an address and decline it. DUIDs, IAID match and we send valid
     // address, so it would work, but the callout sets status to DROP, so
     // the server should not update the lease and should not send back any
index 31238810e67966a28a86b59729c9c93f048ee1d6..04c857f129798860ecaa5c9ba4b3cee4aa390a7d 100644 (file)
@@ -24,9 +24,6 @@ using namespace std;
 namespace isc {
 namespace hooks {
 
-boost::shared_ptr<CalloutManager> HooksManager::shared_callout_manager_;
-bool HooksManager::loaded_;
-
 // Constructor
 
 HooksManager::HooksManager() {
@@ -93,6 +90,9 @@ HooksManager::callCommandHandlers(const std::string& command_name,
 
 bool
 HooksManager::loadLibrariesInternal(const HookLibsCollection& libraries) {
+    if (test_mode_) {
+        return (true);
+    }
     // Unload current set of libraries (if any are loaded).
     unloadLibrariesInternal(false);
 
@@ -109,8 +109,6 @@ HooksManager::loadLibrariesInternal(const HookLibsCollection& libraries) {
         init();
     }
 
-    loaded_ = true;
-
     return (status);
 }
 
@@ -132,8 +130,6 @@ HooksManager::unloadLibrariesInternal(bool initialize) {
         lm_collection_.reset();
         callout_manager_.reset();
     }
-
-    loaded_ = false;
 }
 
 void HooksManager::unloadLibraries() {
@@ -222,17 +218,10 @@ HooksManager::validateLibraries(const std::vector<std::string>& libraries) {
     return (LibraryManagerCollection::validateLibraries(libraries));
 }
 
-boost::shared_ptr<CalloutManager>
-HooksManager::getSharedCalloutManager() {
-    return (shared_callout_manager_);
-}
+bool HooksManager::test_mode_;
 
-void
-HooksManager::setSharedCalloutManager(boost::shared_ptr<CalloutManager> manager) {
-    shared_callout_manager_ = manager;
-    if (!loaded_) {
-        unloadLibraries();
-    }
+void HooksManager::setTestMode(bool mode) {
+    test_mode_ = mode;
 }
 
 } // namespace util
index 6001278f48017c7226b929a1eb6ad90c46584344..8544109e93cdf8bdfd5c2f9074d5445813c4d10b 100644 (file)
@@ -226,21 +226,6 @@ public:
     static const int CONTEXT_CREATE = ServerHooks::CONTEXT_CREATE;
     static const int CONTEXT_DESTROY = ServerHooks::CONTEXT_DESTROY;
 
-    /// @brief Get the shared callout manager
-    ///
-    /// @return A reference to the shared callout manager
-    static boost::shared_ptr<CalloutManager> getSharedCalloutManager();
-
-    /// @brief Set the shared callout manager
-    ///
-    /// This function sets the shared callout manager and explicitly updates
-    /// @ref callout_manager_ (by calling @ref init) if @ref loadLibraries has
-    /// not yet been called.
-    ///
-    /// @param manager The shared callout manager
-    static void setSharedCalloutManager(boost::shared_ptr<CalloutManager> manager =
-                                        boost::shared_ptr<CalloutManager>());
-
     /// @brief Park an object (packet).
     ///
     /// The typical use case for parking an object is when the server needs to
@@ -326,6 +311,12 @@ public:
         getHooksManager().clearParkingLotsInternal();
     }
 
+    /// @brief Set test mode which enables the tests to resister callouts before
+    /// calling @ref loadLibraries and the @ref callout_manager_ is preserved.
+    ///
+    /// @param mode the flag which enables or disabled @ref test_mode_.
+    void setTestMode(bool mode);
+
 private:
 
     /// @brief Constructor
@@ -500,11 +491,9 @@ private:
     /// Callout manager for the set of library managers.
     boost::shared_ptr<CalloutManager> callout_manager_;
 
-    /// Shared callout manager to survive library reloads.
-    static boost::shared_ptr<CalloutManager> shared_callout_manager_;
-
-    /// Loaded flag to indicate if @ref loadLibraries has been called
-    static bool loaded_;
+    /// Test flag to keep @ref callout_manager_ when calling @ref loadLibraries
+    /// from unittests (called by @ref configureDhcp[46]Server)
+    static bool test_mode_;
 };
 
 } // namespace util
index a6d65c032c04391411905925060773a0b6f0adf7..2577871a91adc0c7eb9070a1e216c1d29793873a 100644 (file)
@@ -77,9 +77,6 @@ LibraryManagerCollection::loadLibraries() {
     // where the hooks-libraries clause was empty and is not changed), try
     // to re-use the existing callout manager (so retaining registered pre-
     // and post-library callouts).
-    if (library_names_.empty()) {
-        callout_manager_ = HooksManager::getSharedCalloutManager();
-    }
     if (!library_names_.empty() || !callout_manager_) {
         callout_manager_.reset(new CalloutManager(library_names_.size()));
     }
index 44b39cb6c7105a59e6ac6b397fe49d3842ddbe6c..d8339a524505d6d4903f2cdd350eb199844fa107 100644 (file)
@@ -46,7 +46,6 @@ public:
     /// Unload all libraries and reset the shared manager.
     ~HooksManagerTest() {
         HooksManager::unloadLibraries();
-        HooksManager::getHooksManager().setSharedCalloutManager();
     }
 
 
@@ -440,13 +439,10 @@ TEST_F(HooksManagerTest, PrePostCalloutTest) {
 // Test with a shared manager the pre- and post- callout functions survive
 // a reload
 
-TEST_F(HooksManagerTest, PrePostCalloutShared) {
+TEST_F(HooksManagerTest, DISABLED_PrePostCalloutShared) {
 
     HookLibsCollection library_names;
 
-    // Initialize the shared manager.
-    HooksManager::getHooksManager().setSharedCalloutManager(
-        boost::shared_ptr<CalloutManager>(new CalloutManager(0)));
 
     // Load the pre- and post- callouts.
     HooksManager::preCalloutsLibraryHandle().registerCallout("hookpt_two",
@@ -486,16 +482,12 @@ TEST_F(HooksManagerTest, PrePostCalloutShared) {
 // Test with a shared manager the pre- and post- callout functions survive
 // a reload but not with a not empty list of libraries
 
-TEST_F(HooksManagerTest, PrePostCalloutSharedNotEmpty) {
+TEST_F(HooksManagerTest, DISABLED_PrePostCalloutSharedNotEmpty) {
 
     HookLibsCollection library_names;
     library_names.push_back(make_pair(std::string(FULL_CALLOUT_LIBRARY),
                                       data::ConstElementPtr()));
 
-    // Initialize the shared manager.
-    HooksManager::getHooksManager().setSharedCalloutManager(
-        boost::shared_ptr<CalloutManager>(new CalloutManager(0)));
-
     // Load the pre- and post- callouts.
     HooksManager::preCalloutsLibraryHandle().registerCallout("hookpt_two",
                                                              testPreCallout);
@@ -534,15 +526,11 @@ TEST_F(HooksManagerTest, PrePostCalloutSharedNotEmpty) {
 // Test with a shared manager the pre- and post- callout functions don't
 // survive a reload if the shared manager is initialized too late.
 
-TEST_F(HooksManagerTest, PrePostCalloutSharedTooLate) {
+TEST_F(HooksManagerTest, DISABLED_PrePostCalloutSharedTooLate) {
 
     HookLibsCollection library_names;
     EXPECT_TRUE(HooksManager::loadLibraries(library_names));
 
-    // Initialize the shared manager (after loadLibraries so too late)
-    HooksManager::getHooksManager().setSharedCalloutManager(
-        boost::shared_ptr<CalloutManager>(new CalloutManager(0)));
-
     // Load the pre- and post- callouts.
     HooksManager::preCalloutsLibraryHandle().registerCallout("hookpt_two",
                                                              testPreCallout);