Browse Source

[2974] Corrected constructor of CalloutHandle

This now takes a pointer to a LibraryHandleCollection instead
of a pointer to a HookManager: such a change cleanly separates
the server and client code.
Stephen Morris 12 years ago
parent
commit
ba0004b009

+ 8 - 8
src/lib/util/hooks/callout_handle.h

@@ -36,8 +36,8 @@ public:
         isc::Exception(file, line, what) {}
 };
 
-// Forward declaration of the hook manager class
-class HookManager;
+// Forward declaration of the handle collection class.
+class LibraryHandleCollection;
 
 /// @brief Per-Packet callout handle
 ///
@@ -54,10 +54,9 @@ public:
 
     /// @brief Constructor
     ///
-    /// @param manager Pointer to the HookManager object controlling the
-    ///        the operations of the hooks.
-    CalloutHandle(boost::shared_ptr<HookManager>& manager)
-        : arguments_(), manager_(manager), skip_(false)
+    /// @param manager Pointer to the collection of library handles.
+    CalloutHandle(boost::shared_ptr<LibraryHandleCollection>& collection)
+        : arguments_(), collection_(collection), skip_(false)
     {}
 
     /// @brief Set argument
@@ -148,8 +147,9 @@ private:
     /// Collection of arguments passed to the callouts
     ArgumentCollection arguments_;
 
-    /// Controlling hook manager
-    boost::shared_ptr<HookManager> manager_;   ///< Controlling hook manager
+    /// Library handle collection, used to obtain the correct library handle
+    /// during a call to a callout.
+    boost::shared_ptr<LibraryHandleCollection> collection_;
 
     /// "Skip" flag, indicating if the caller should bypass remaining callouts.
     bool skip_;

+ 2 - 1
src/lib/util/hooks/library_handle.h

@@ -295,7 +295,8 @@ public:
     /// @return Pointer to current library handle. This is the handle for
     ///         which a callout is being called.
     boost::shared_ptr<LibraryHandle> getLibraryHandle() const {
-        return (boost::shared_ptr<LibraryHandle>(curidx_));
+        return (boost::shared_ptr<LibraryHandle>());
+        /// @todo Return the appropriate handle
     }
 
     /// @brief Checks if callouts are present

+ 13 - 26
src/lib/util/tests/callout_handle_unittest.cc

@@ -23,19 +23,6 @@
 using namespace isc::util;
 using namespace std;
 
-// Dummy class for testing
-namespace isc {
-namespace util {
-class HookManager {
-public:
-    HookManager() {}
-};
-}
-}
-
-using namespace isc::util;
-using namespace std;
-
 namespace {
 
 class CalloutHandleTest : public ::testing::Test {
@@ -44,16 +31,16 @@ public:
     ///
     /// Sets up an appropriate number of server hooks to pass to the
     /// constructed callout handle objects.
-    CalloutHandleTest() : manager_(new HookManager()) {
+    CalloutHandleTest() : collection_(new LibraryHandleCollection()) {
     }
 
     /// Obtain hook manager
-    boost::shared_ptr<HookManager>& getHookManager() {
-        return (manager_);
+    boost::shared_ptr<LibraryHandleCollection>& getLibraryHandleCollection() {
+        return (collection_);
     }
 
 private:
-    boost::shared_ptr<HookManager> manager_;
+    boost::shared_ptr<LibraryHandleCollection> collection_;
 };
 
 // *** Argument Tests ***
@@ -65,7 +52,7 @@ private:
 // are distinct.
 
 TEST_F(CalloutHandleTest, ArgumentDistinctSimpleType) {
-    CalloutHandle handle(getHookManager());
+    CalloutHandle handle(getLibraryHandleCollection());
 
     // Store and retrieve an int (random value).
     int a = 42;
@@ -99,7 +86,7 @@ TEST_F(CalloutHandleTest, ArgumentDistinctSimpleType) {
 // exception.
 
 TEST_F(CalloutHandleTest, ArgumentUnknownName) {
-    CalloutHandle handle(getHookManager());
+    CalloutHandle handle(getLibraryHandleCollection());
 
     // Set an integer
     int a = 42;
@@ -119,7 +106,7 @@ TEST_F(CalloutHandleTest, ArgumentUnknownName) {
 // Test that trying to get something with an incorrect type throws an exception.
 
 TEST_F(CalloutHandleTest, ArgumentIncorrectType) {
-    CalloutHandle handle(getHookManager());
+    CalloutHandle handle(getLibraryHandleCollection());
 
     // Set an integer
     int a = 42;
@@ -148,7 +135,7 @@ struct Beta {
 };
 
 TEST_F(CalloutHandleTest, ComplexTypes) {
-    CalloutHandle handle(getHookManager());
+    CalloutHandle handle(getLibraryHandleCollection());
 
     // Declare two variables of different (complex) types. (Note as to the
     // variable names: aleph and beth are the first two letters of the Hebrew
@@ -187,7 +174,7 @@ TEST_F(CalloutHandleTest, ComplexTypes) {
 // that a "pointer to X" is not the same as a "pointer to const X".
 
 TEST_F(CalloutHandleTest, PointerTypes) {
-    CalloutHandle handle(getHookManager());
+    CalloutHandle handle(getLibraryHandleCollection());
 
     // Declare a couple of variables, const and non-const.
     Alpha aleph(5, 10);
@@ -221,7 +208,7 @@ TEST_F(CalloutHandleTest, PointerTypes) {
 // Check that we can get the names of the arguments.
 
 TEST_F(CalloutHandleTest, ContextItemNames) {
-    CalloutHandle handle(getHookManager());
+    CalloutHandle handle(getLibraryHandleCollection());
 
     vector<string> expected_names;
     int value = 42;
@@ -245,7 +232,7 @@ TEST_F(CalloutHandleTest, ContextItemNames) {
 // Test that we can delete and argument.
 
 TEST_F(CalloutHandleTest, DeleteArgument) {
-    CalloutHandle handle(getHookManager());
+    CalloutHandle handle(getLibraryHandleCollection());
 
     int one = 1;
     int two = 2;
@@ -287,7 +274,7 @@ TEST_F(CalloutHandleTest, DeleteArgument) {
 // Test that we can delete all arguments
 
 TEST_F(CalloutHandleTest, DeleteAllArguments) {
-    CalloutHandle handle(getHookManager());
+    CalloutHandle handle(getLibraryHandleCollection());
 
     int one = 1;
     int two = 2;
@@ -314,7 +301,7 @@ TEST_F(CalloutHandleTest, DeleteAllArguments) {
 // Test the "skip" flag.
 
 TEST_F(CalloutHandleTest, SkipFlag) {
-    CalloutHandle handle(getHookManager());
+    CalloutHandle handle(getLibraryHandleCollection());
 
     // Should be false on construction.
     EXPECT_FALSE(handle.getSkip());

+ 4 - 19
src/lib/util/tests/library_handle_collection_unittest.cc

@@ -27,13 +27,6 @@ using namespace isc;
 using namespace isc::util;
 using namespace std;
 
-// Dummy class for testing
-namespace isc {
-namespace util {
-class HookManager {};
-}
-}
-
 namespace {
 
 /// @brief No such hook
@@ -52,7 +45,7 @@ public:
     /// Sets up a collection of three LibraryHandle objects to use in the test.
     LibraryHandleCollectionTest()
         : collection_(new LibraryHandleCollection()), handles_(),
-          hooks_(new ServerHooks()), manager_(new HookManager()) {
+          hooks_(new ServerHooks()) {
 
         // Set up the server hooks
         hooks_->registerHook("one");
@@ -77,13 +70,6 @@ public:
         return (hooks_);
     }
 
-    /// @brief Obtain constructed hook manager
-    ///
-    /// @return Reference to shared pointer pointing to hook manager.
-    boost::shared_ptr<HookManager>& getHookManager() {
-        return (manager_);
-    }
-
     /// @brief Obtain LibraryHandleCollection object
     ///
     /// @return Reference to shared pointer pointing to handle collection
@@ -120,7 +106,6 @@ private:
 
     /// Server hooks and hooks manager
     boost::shared_ptr<ServerHooks> hooks_;
-    boost::shared_ptr<HookManager> manager_;
 };
 
 // Definition of the static variable.
@@ -280,7 +265,7 @@ TEST_F(LibraryHandleCollectionTest, CallCalloutsSuccess) {
     EXPECT_FALSE(getLibraryHandleCollection()->calloutsPresent(four_index));
                  
     // Set up different sequences of callouts on different handles.
-    CalloutHandle callout_handle(getHookManager());
+    CalloutHandle callout_handle(getLibraryHandleCollection());
     int status;
 
     // Each library contributing one callout on hook "one".
@@ -330,7 +315,7 @@ TEST_F(LibraryHandleCollectionTest, CallCalloutsError) {
     EXPECT_FALSE(getLibraryHandleCollection()->calloutsPresent(four_index));
                  
     // Set up different sequences of callouts on different handles.
-    CalloutHandle callout_handle(getHookManager());
+    CalloutHandle callout_handle(getLibraryHandleCollection());
     int status;
 
     // Each library contributing one callout on hook "one". First callout
@@ -406,7 +391,7 @@ TEST_F(LibraryHandleCollectionTest, CallCalloutsSkip) {
     EXPECT_FALSE(getLibraryHandleCollection()->calloutsPresent(four_index));
                  
     // Set up different sequences of callouts on different handles.
-    CalloutHandle callout_handle(getHookManager());
+    CalloutHandle callout_handle(getLibraryHandleCollection());
     int status;
 
     // Each library contributing one callout on hook "one". First callout

+ 12 - 18
src/lib/util/tests/library_handle_unittest.cc

@@ -25,13 +25,6 @@
 using namespace isc::util;
 using namespace std;
 
-// Dummy class for testing
-namespace isc {
-namespace util {
-class HookManager {};
-}
-}
-
 namespace {
 
 class LibraryHandleTest : public ::testing::Test {
@@ -41,7 +34,8 @@ public:
     /// Sets up an appropriate number of server hooks to pass to the
     /// constructed callout handle objects.
     LibraryHandleTest()
-        : hooks_(new ServerHooks()), manager_(new HookManager()) {
+        : hooks_(new ServerHooks()),
+          collection_(new LibraryHandleCollection()) {
         hooks_->registerHook("alpha");
         hooks_->registerHook("beta");
         hooks_->registerHook("gamma");
@@ -56,8 +50,8 @@ public:
     }
 
     // Obtain constructed hook manager
-    boost::shared_ptr<HookManager>& getHookManager() {
-        return (manager_);
+    boost::shared_ptr<LibraryHandleCollection>& getLibraryHandleCollection() {
+        return (collection_);
     }
 
     /// Variable for callouts test. This is public and static to allow non-
@@ -67,7 +61,7 @@ public:
 
 private:
     boost::shared_ptr<ServerHooks> hooks_;
-    boost::shared_ptr<HookManager> manager_;
+    boost::shared_ptr<LibraryHandleCollection> collection_;
 };
 
 // Definition of the static variable.
@@ -388,7 +382,7 @@ TEST_F(LibraryHandleTest, CallSingleCallout) {
     EXPECT_EQ(0, LibraryHandleTest::callout_value);
 
     int index = getServerHooks()->getIndex("alpha");
-    CalloutHandle callout_handle(getHookManager());
+    CalloutHandle callout_handle(getLibraryHandleCollection());
     int status = handle.callCallouts(index, callout_handle);
 
     EXPECT_EQ(0, status);
@@ -410,7 +404,7 @@ TEST_F(LibraryHandleTest, TwoCallouts) {
     EXPECT_EQ(0, LibraryHandleTest::callout_value);
 
     int index = getServerHooks()->getIndex("alpha");
-    CalloutHandle callout_handle(getHookManager());
+    CalloutHandle callout_handle(getLibraryHandleCollection());
     int status = handle.callCallouts(index, callout_handle);
 
     EXPECT_EQ(0, status);
@@ -431,7 +425,7 @@ TEST_F(LibraryHandleTest, TwoCalloutsWithError) {
     EXPECT_EQ(0, LibraryHandleTest::callout_value);
 
     int index = getServerHooks()->getIndex("alpha");
-    CalloutHandle callout_handle(getHookManager());
+    CalloutHandle callout_handle(getLibraryHandleCollection());
     int status = handle.callCallouts(index, callout_handle);
 
     EXPECT_EQ(1, status);
@@ -452,7 +446,7 @@ TEST_F(LibraryHandleTest, TwoCalloutsWithSkip) {
     EXPECT_EQ(0, LibraryHandleTest::callout_value);
 
     int index = getServerHooks()->getIndex("alpha");
-    CalloutHandle callout_handle(getHookManager());
+    CalloutHandle callout_handle(getLibraryHandleCollection());
     int status = handle.callCallouts(index, callout_handle);
 
     EXPECT_EQ(0, status);
@@ -473,7 +467,7 @@ TEST_F(LibraryHandleTest, MultipleRegistration) {
     EXPECT_EQ(0, LibraryHandleTest::callout_value);
 
     int index = getServerHooks()->getIndex("alpha");
-    CalloutHandle callout_handle(getHookManager());
+    CalloutHandle callout_handle(getLibraryHandleCollection());
     int status = handle.callCallouts(index, callout_handle);
 
     EXPECT_EQ(0, status);
@@ -497,7 +491,7 @@ TEST_F(LibraryHandleTest, Deregister) {
     EXPECT_EQ(0, LibraryHandleTest::callout_value);
 
     int index = getServerHooks()->getIndex("alpha");
-    CalloutHandle callout_handle(getHookManager());
+    CalloutHandle callout_handle(getLibraryHandleCollection());
     int status = handle.callCallouts(index, callout_handle);
 
     EXPECT_EQ(0, status);
@@ -533,7 +527,7 @@ TEST_F(LibraryHandleTest, InvalidNameAndIndex) {
     EXPECT_THROW(static_cast<void>(handle.calloutsPresent(-1)), NoSuchHook);
     EXPECT_THROW(static_cast<void>(handle.calloutsPresent(5)), NoSuchHook);
 
-    CalloutHandle callout_handle(getHookManager());
+    CalloutHandle callout_handle(getLibraryHandleCollection());
     EXPECT_THROW(static_cast<void>(handle.callCallouts(-1, callout_handle)),
                  NoSuchHook);
     EXPECT_THROW(static_cast<void>(handle.callCallouts(10, callout_handle)),