[2974] Added the LibraryHandleCollection object.

src/lib/util/hooks/library_handle.cc

@@ -79,18 +79,19 @@ LibraryHandle::calloutsPresent(int index) const {
 // Call all the callouts for a given hook.
 
 int
-LibraryHandle::callCallouts(int index, CalloutHandle& handle) {
+LibraryHandle::callCallouts(int index, CalloutHandle& callout_handle) {
 
     // Validate the hook index.
     checkHookIndex(index);
 
-    // Call all the callouts, stopping if a non-zero status is returned.
-    // @todo also need to stop if the callout handle "skip" flag is set.
+    // Call all the callouts, stopping if a non-zero status is returned or if
+    // the "skip" flag is set.
     int status = 0;
     for (int i = 0;
-         (i < hook_vector_[index].size()) && !handle.getSkip() && (status == 0);
+         (i < hook_vector_[index].size()) && !callout_handle.getSkip() &&
+         (status == 0);
           ++i) {
-        status = (*hook_vector_[index][i])(handle);
+        status = (*hook_vector_[index][i])(callout_handle);
     }
 
     return (status);
@@ -150,5 +151,54 @@ LibraryHandle::getContextNames() const {
     return (names);
 }
 
+// LibraryHandleCollection methods.
+
+// Check if a callout is present on a hook in any of the libraries.
+
+bool
+LibraryHandleCollection::calloutsPresent(int index) const {
+
+    // Method returns false if no LibraryHandles are present.  Otherwise,
+    // the validity of the index is checked by the calloutsPresent() method
+    // on the first handle processed.
+    bool present = false;
+    for (int i = 0; (i < handles_.size()) && !present; ++i) {
+        present = handles_[i]->calloutsPresent(index);
+    }
+
+    return (present);
+}
+
+// Call all the callouts for a given hook.
+
+int
+LibraryHandleCollection::callCallouts(int index,
+                                      CalloutHandle& callout_handle) {
+
+    // Don't validate the hook index, that is checked in the call to the
+    // callCallouts() method of the first library handle.
+
+    // Clear the skip flag before we start so that no state from a previous
+    // call of a hook accidentally leaks through.
+    callout_handle.setSkip(false);
+
+    // Call all the callouts, stopping if a non-zero status is returned or if
+    // the "skip" flag is set.  Note that we iterate using the current index
+    // as the counter (so that the callout handle object retrieves the
+    // correct LibraryHandle from the collection).
+    int status = 0;
+    for (curidx_ = 0;
+         (curidx_ < handles_.size()) && !callout_handle.getSkip() &&
+         (status == 0); ++curidx_) {
+        status = handles_[curidx_]->callCallouts(index, callout_handle);
+    }
+
+    // Reset current index to an invalid value as we are no longer calling
+    // the callouts.
+    curidx_ = -1;
+
+    return (status);
+}
+
 } // namespace util
 } // namespace isc

src/lib/util/hooks/library_handle.h

@@ -80,10 +80,8 @@ public:
     /// is used by the CalloutHandle object to access appropriate context
     ///
     /// @param hooks Pointer to the hooks registered by the server.
-    /// @param index Index of this library in the list of loaded libraries.
-    LibraryHandle(boost::shared_ptr<ServerHooks>& hooks, int index)
-        : context_(), hooks_(hooks), hook_vector_(hooks->getCount()),
-          index_(index)
+    LibraryHandle(boost::shared_ptr<ServerHooks>& hooks)
+        : context_(), hooks_(hooks), hook_vector_(hooks->getCount())
     {}
 
     /// @brief Set context
@@ -116,7 +114,7 @@ public:
         ContextCollection::const_iterator element_ptr = context_.find(name);
         if (element_ptr == context_.end()) {
             isc_throw(NoSuchContext, "unable to find library context datum " <<
-                      name << " in library at index " << index_);
+                      name << " in library handle");
         }
 
         value = boost::any_cast<T>(element_ptr->second);
@@ -204,12 +202,12 @@ public:
     /// Calls the callouts associated with the given hook index.
     ///
     /// @param index Index of the hook to call.
-    /// @param handle Reference to the CalloutHandle object for the current
-    ///        object being processed.
+    /// @param callout_handle Reference to the CalloutHandle object for the
+    ///        current object being processed.
     ///
     /// @return Status return.
     ///
-    int callCallouts(int index, CalloutHandle& handle);
+    int callCallouts(int index, CalloutHandle& callout_handle);
 
 private:
     /// @brief Check hook index
@@ -239,10 +237,6 @@ private:
     /// @throw Unexpected Index not valid for the hook vector.
     int getHookIndex(const std::string& name) const;
 
-    /// @brief Callout pointers equal
-    ///
-    /// Unary predicate to 
-
     // Member variables
 
     /// Context - mapping of names variables that can be of different types.
@@ -254,9 +248,87 @@ private:
     /// Each element in the following vector corresponds to a single hook and
     /// is an ordered list of callouts for that hook.
     std::vector<std::vector<CalloutPtr> >  hook_vector_;
+};
+
+
+/// @brief Collection of Library Handles
+///
+/// This simple class is a collection of handles for all libraries loaded.
+/// It is pointed to by the CalloutHandle object and is used by that object
+/// to locate the correct LibraryHandle to pass to a callout function. To
+/// do this, the class contains an index indicating the "current" handle.  This
+/// is used in the calling of callouts: prior to calling a callout associated
+/// with a LibraryHandle, the index is updated to point to that handle.
+/// If the callout requests access to the LibraryHandle, it is passed a
+/// reference to the correct one.
+
+class LibraryHandleCollection {
+private:
+    /// Private typedef to abbreviate statements in class methods.
+    typedef std::vector<boost::shared_ptr<LibraryHandle> > HandleVector;
+public:
+
+    /// @brief Constructor
+    ///
+    /// Initializes member variables, in particular setting the current index
+    /// to an invalid value.
+    LibraryHandleCollection() : curidx_(-1), handles_()
+    {}
+
+    /// @brief Add library handle
+    ///
+    /// Adds a library handle to the collection.  The collection is ordered,
+    /// and this adds a library handle to the end of the current list.
+    ///
+    /// @param library_handle Pointer to the a library handle to be added.
+    void addLibraryHandle(boost::shared_ptr<LibraryHandle>& handle) {
+        handles_.push_back(handle);
+    }
+
+    /// @brief Get current library handle
+    ///
+    /// Returns a pointer to the current library handle.  This method can
+    /// only be called while the code is iterating through the list of
+    /// library handles: calling it at any other time is meaningless and will
+    /// cause an exception to be thrown.
+    ///
+    /// @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_));
+    }
+
+    /// @brief Checks if callouts are present
+    ///
+    /// Checks all loaded libraries and returns true if at least one callout
+    /// has been registered for a hook in any of them.
+    ///
+    /// @param index Hook index for which callouts are checked.
+    ///
+    /// @return true if callouts are present, false if not.
+    ///
+    /// @throw NoSuchHook Thrown if the index is not valid.
+    bool calloutsPresent(int index) const;
+
+    /// @brief Calls the callouts for a given hook
+    ///
+    /// Iterates through the libray handles and calls the callouts associated
+    /// with the given hook index.
+    ///
+    /// @param index Index of the hook to call.
+    /// @param callout_handle Reference to the CalloutHandle object for the
+    ///        current object being processed.
+    ///
+    /// @return Status return.
+    ///
+    int callCallouts(int index, CalloutHandle& callout_handle);
+
+private:
+    /// Index of the current library handle during iteration.
+    int curidx_;
 
-    /// Index of this library in the list of libraries
-    int index_;
+    /// Vector of pointers to library handles.
+    HandleVector handles_;
 };
 
 } // namespace util

src/lib/util/tests/Makefile.am

@@ -32,6 +32,7 @@ run_unittests_SOURCES += filename_unittest.cc
 run_unittests_SOURCES += hex_unittest.cc
 run_unittests_SOURCES += io_utilities_unittest.cc
 run_unittests_SOURCES += library_handle_unittest.cc
+run_unittests_SOURCES += library_handle_collection_unittest.cc
 run_unittests_SOURCES += lru_list_unittest.cc
 run_unittests_SOURCES += memory_segment_local_unittest.cc
 if USE_SHARED_MEMORY

src/lib/util/tests/library_handle_collection_unittest.cc

@@ -0,0 +1,469 @@
+// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <exceptions/exceptions.h>
+#include <util/hooks/callout_handle.h>
+#include <util/hooks/library_handle.h>
+#include <util/hooks/server_hooks.h>
+
+#include <gtest/gtest.h>
+
+#include <algorithm>
+#include <string>
+#include <vector>
+
+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
+///
+/// Thrown if an attempt it made to obtain an invalid library handle.
+class InvalidIndex : public isc::Exception {
+public:
+    InvalidIndex(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) {}
+};
+
+class LibraryHandleCollectionTest : public ::testing::Test {
+public:
+    /// @brief Constructor
+    ///
+    /// Sets up a collection of three LibraryHandle objects to use in the test.
+    LibraryHandleCollectionTest()
+        : collection_(new LibraryHandleCollection()), handles_(),
+          hooks_(new ServerHooks()), manager_(new HookManager()) {
+
+        // Set up the server hooks
+        hooks_->registerHook("one");
+        hooks_->registerHook("two");
+        hooks_->registerHook("three");
+        hooks_->registerHook("four");
+
+        // Set up the library handles and collection.
+        for (int i = 0; i < 4; ++i) {
+            boost::shared_ptr<LibraryHandle> handle(new LibraryHandle(hooks_));
+            handles_.push_back(handle);
+            collection_->addLibraryHandle(handle);
+        }
+
+        callout_value_ = 0;
+    }
+
+    /// @brief Obtain constructed server hooks
+    ///
+    /// @return Reference to shared pointer pointing to server hooks object.
+    boost::shared_ptr<ServerHooks>& getServerHooks() {
+        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
+    boost::shared_ptr<LibraryHandleCollection>& getLibraryHandleCollection() {
+        return (collection_);
+    }
+
+    /// @brief Obtain individual LibraryHandle.
+    ///
+    /// @param i Index of the library handle required.
+    ///
+    /// @return Reference to shared pointer pointing to the relevant handle.
+    ///
+    /// @throws InvalidIndex if the requeste dindex is not valid.
+    boost::shared_ptr<LibraryHandle>& getLibraryHandle(int i) {
+        if ((i < 0) || (i >= handles_.size())) {
+            isc_throw(InvalidIndex, "handle index of " << i << " not valid for "
+                      " size of handle vector (" << handles_.size() << ")");
+        }
+        return (handles_[i]);
+    }
+
+    /// Variable for callouts test. This is public and static to allow non-
+    /// member functions to access it.  It is initialized every time a
+    /// new test starts.
+    static int callout_value_;
+
+private:
+
+    /// Library handle collection and the individual handles (as the
+    /// collection has no method for accessing an individual member).
+    boost::shared_ptr<LibraryHandleCollection> collection_;
+    std::vector<boost::shared_ptr<LibraryHandle> > handles_;
+
+    /// Server hooks and hooks manager
+    boost::shared_ptr<ServerHooks> hooks_;
+    boost::shared_ptr<HookManager> manager_;
+};
+
+// Definition of the static variable.
+int LibraryHandleCollectionTest::callout_value_ = 0;
+
+// *** Callout Tests ***
+//
+// The next set of tests check that callouts can be called.
+
+// Supply callouts structured in such a way that we can determine the order
+// that they are called and whether they are called at all. The method used
+// is simple - after a sequence of callouts, the digits in the value, reading
+// left to right, determines the order of the callouts and whether they were
+// called at all.  So:
+//
+// * one followed by two, the resulting value is 12
+// * two followed by one, the resuling value is 21
+// * one and two is not called, the resulting value is 1
+// * two and one is not called, the resulting value is 2
+// * neither called, the resulting value is 0
+//
+// ... and extending beyond two callouts:
+//
+// * one followed by two followed by three followed by two followed by one
+//   results in a value of 12321.
+//
+// Functions return a zero indicating success.
+
+extern "C" {
+int collection_one(CalloutHandle&) {
+    LibraryHandleCollectionTest::callout_value_ =
+        10 * LibraryHandleCollectionTest::callout_value_ + 1;
+    return (0);
+}
+
+int collection_two(CalloutHandle&) {
+    LibraryHandleCollectionTest::callout_value_ =
+        10 * LibraryHandleCollectionTest::callout_value_ + 2;
+    return (0);
+}
+
+int collection_three(CalloutHandle&) {
+    LibraryHandleCollectionTest::callout_value_ =
+        10 * LibraryHandleCollectionTest::callout_value_ + 3;
+    return (0);
+}
+
+int collection_four(CalloutHandle&) {
+    LibraryHandleCollectionTest::callout_value_ =
+        10 * LibraryHandleCollectionTest::callout_value_ + 4;
+    return (0);
+}
+
+// The next functions are duplicates of the above, but return an error.
+
+int collection_one_error(CalloutHandle& handle) {
+    (void) collection_one(handle);
+    return (1);
+}
+
+int collection_two_error(CalloutHandle& handle) {
+    (void) collection_two(handle);
+    return (1);
+}
+
+int collection_three_error(CalloutHandle& handle) {
+    (void) collection_three(handle);
+    return (1);
+}
+
+int collection_four_error(CalloutHandle& handle) {
+    (void) collection_four(handle);
+    return (1);
+}
+
+// The next functions are duplicates of the above, but set the skip flag.
+
+int collection_one_skip(CalloutHandle& handle) {
+    handle.setSkip(true);
+    return (collection_one(handle));
+}
+
+int collection_two_skip(CalloutHandle& handle) {
+    handle.setSkip(true);
+    return (collection_two(handle));
+}
+
+int collection_three_skip(CalloutHandle& handle) {
+    handle.setSkip(true);
+    return (collection_three(handle));
+}
+
+int collection_four_skip(CalloutHandle& handle) {
+    handle.setSkip(true);
+    return (collection_four(handle));
+}
+
+};  // extern "C"
+
+// Check that we know which hooks have callouts attached to them.
+// Note: as we needed to use the addHandleMethod() to set up the handles to
+// which the callouts are attached, this can also be construed as a test
+// of the addLibraryHandle method as well.
+
+TEST_F(LibraryHandleCollectionTest, CalloutsPresent) {
+    const int one_index = getServerHooks()->getIndex("one");
+    const int two_index = getServerHooks()->getIndex("two");
+    const int three_index = getServerHooks()->getIndex("three");
+    const int four_index = getServerHooks()->getIndex("four");
+
+    // Ensure that no callouts are attached to any of the hooks.
+    EXPECT_FALSE(getLibraryHandleCollection()->calloutsPresent(one_index));
+    EXPECT_FALSE(getLibraryHandleCollection()->calloutsPresent(two_index));
+    EXPECT_FALSE(getLibraryHandleCollection()->calloutsPresent(three_index));
+    EXPECT_FALSE(getLibraryHandleCollection()->calloutsPresent(four_index));
+                 
+    // Set up so that hooks "one", "two" and "four" have callouts attached
+    // to them, and callout  "three" does not. (In the statements below, the
+    // exact callouts attached to a hook are not relevant - only the fact
+    // that some callouts are).
+
+    getLibraryHandle(0)->registerCallout("one", collection_one);
+
+    getLibraryHandle(1)->registerCallout("one", collection_two);
+    getLibraryHandle(1)->registerCallout("two", collection_two);
+
+    getLibraryHandle(3)->registerCallout("one", collection_four);
+    getLibraryHandle(3)->registerCallout("four", collection_two);
+
+    // Check all is as expected.
+    EXPECT_TRUE(getLibraryHandleCollection()->calloutsPresent(one_index));
+    EXPECT_TRUE(getLibraryHandleCollection()->calloutsPresent(two_index));
+    EXPECT_FALSE(getLibraryHandleCollection()->calloutsPresent(three_index));
+    EXPECT_TRUE(getLibraryHandleCollection()->calloutsPresent(four_index));
+
+    // Check we fail on an invalid index.
+    EXPECT_THROW(getLibraryHandleCollection()->calloutsPresent(42), NoSuchHook);
+    EXPECT_THROW(getLibraryHandleCollection()->calloutsPresent(-1), NoSuchHook);
+
+    // Check we get a negative result on an empty collection.
+    LibraryHandleCollection empty_collection;
+    EXPECT_FALSE(empty_collection.calloutsPresent(-1));
+}
+
+// Test that the callouts are called in order.
+
+TEST_F(LibraryHandleCollectionTest, CallCalloutsSuccess) {
+    const int one_index = getServerHooks()->getIndex("one");
+    const int two_index = getServerHooks()->getIndex("two");
+    const int three_index = getServerHooks()->getIndex("three");
+    const int four_index = getServerHooks()->getIndex("four");
+
+    // Ensure that no callouts are attached to any of the hooks.
+    EXPECT_FALSE(getLibraryHandleCollection()->calloutsPresent(one_index));
+    EXPECT_FALSE(getLibraryHandleCollection()->calloutsPresent(two_index));
+    EXPECT_FALSE(getLibraryHandleCollection()->calloutsPresent(three_index));
+    EXPECT_FALSE(getLibraryHandleCollection()->calloutsPresent(four_index));
+                 
+    // Set up different sequences of callouts on different handles.
+    CalloutHandle callout_handle(getHookManager());
+    int status;
+
+    // Each library contributing one callout on hook "one".
+    callout_value_ = 0;
+    getLibraryHandle(0)->registerCallout("one", collection_one);
+    getLibraryHandle(1)->registerCallout("one", collection_two);
+    getLibraryHandle(2)->registerCallout("one", collection_three);
+    getLibraryHandle(3)->registerCallout("one", collection_four);
+    status = getLibraryHandleCollection()->callCallouts(one_index,
+                                                        callout_handle);
+    EXPECT_EQ(0, status);
+    EXPECT_EQ(1234, callout_value_);
+
+    // Do a random selection of callouts on hook "two".
+    callout_value_ = 0;
+    getLibraryHandle(0)->registerCallout("two", collection_one);
+    getLibraryHandle(0)->registerCallout("two", collection_one);
+    getLibraryHandle(1)->registerCallout("two", collection_two);
+    getLibraryHandle(3)->registerCallout("two", collection_four);
+    status = getLibraryHandleCollection()->callCallouts(two_index,
+                                                        callout_handle);
+    EXPECT_EQ(0, status);
+    EXPECT_EQ(1124, callout_value_);
+
+    // Ensure that calling the callouts on a hook with no callouts works,
+    // even though it does not return any value.
+    callout_value_ = 0;
+    status = getLibraryHandleCollection()->callCallouts(three_index,
+                                                        callout_handle);
+    EXPECT_EQ(0, status);
+    EXPECT_EQ(0, callout_value_);
+}
+
+// Test that the callouts are called in order, but not after a callout
+// returing an error code.
+
+TEST_F(LibraryHandleCollectionTest, CallCalloutsError) {
+    const int one_index = getServerHooks()->getIndex("one");
+    const int two_index = getServerHooks()->getIndex("two");
+    const int three_index = getServerHooks()->getIndex("three");
+    const int four_index = getServerHooks()->getIndex("four");
+
+    // Ensure that no callouts are attached to any of the hooks.
+    EXPECT_FALSE(getLibraryHandleCollection()->calloutsPresent(one_index));
+    EXPECT_FALSE(getLibraryHandleCollection()->calloutsPresent(two_index));
+    EXPECT_FALSE(getLibraryHandleCollection()->calloutsPresent(three_index));
+    EXPECT_FALSE(getLibraryHandleCollection()->calloutsPresent(four_index));
+                 
+    // Set up different sequences of callouts on different handles.
+    CalloutHandle callout_handle(getHookManager());
+    int status;
+
+    // Each library contributing one callout on hook "one". First callout
+    // returns an error.
+    callout_value_ = 0;
+    getLibraryHandle(0)->registerCallout("one", collection_one_error);
+    getLibraryHandle(1)->registerCallout("one", collection_two);
+    getLibraryHandle(2)->registerCallout("one", collection_three);
+    getLibraryHandle(3)->registerCallout("one", collection_four);
+    status = getLibraryHandleCollection()->callCallouts(one_index,
+                                                        callout_handle);
+    EXPECT_EQ(1, status);
+    EXPECT_EQ(1, callout_value_);
+
+    // Each library contributing multiple callouts on hook "two". Last callout
+    // on first library returns an error after updating the value.
+    callout_value_ = 0;
+    getLibraryHandle(0)->registerCallout("two", collection_one);
+    getLibraryHandle(0)->registerCallout("two", collection_one_error);
+    getLibraryHandle(1)->registerCallout("two", collection_two);
+    getLibraryHandle(1)->registerCallout("two", collection_two);
+    getLibraryHandle(1)->registerCallout("two", collection_three);
+    getLibraryHandle(1)->registerCallout("two", collection_three);
+    getLibraryHandle(3)->registerCallout("two", collection_four);
+    getLibraryHandle(3)->registerCallout("two", collection_four);
+    status = getLibraryHandleCollection()->callCallouts(two_index,
+                                                        callout_handle);
+    EXPECT_EQ(1, status);
+    EXPECT_EQ(11, callout_value_);
+
+    // Random callout returns an error.
+    callout_value_ = 0;
+    getLibraryHandle(0)->registerCallout("three", collection_one);
+    getLibraryHandle(0)->registerCallout("three", collection_one);
+    getLibraryHandle(1)->registerCallout("three", collection_two);
+    getLibraryHandle(1)->registerCallout("three", collection_two);
+    getLibraryHandle(3)->registerCallout("three", collection_four_error);
+    getLibraryHandle(3)->registerCallout("three", collection_four);
+    status = getLibraryHandleCollection()->callCallouts(three_index,
+                                                        callout_handle);
+    EXPECT_EQ(1, status);
+    EXPECT_EQ(11224, callout_value_);
+
+    // Last callout returns an error.
+    callout_value_ = 0;
+    getLibraryHandle(0)->registerCallout("four", collection_one);
+    getLibraryHandle(0)->registerCallout("four", collection_one);
+    getLibraryHandle(1)->registerCallout("four", collection_two);
+    getLibraryHandle(1)->registerCallout("four", collection_two);
+    getLibraryHandle(2)->registerCallout("four", collection_three);
+    getLibraryHandle(2)->registerCallout("four", collection_three);
+    getLibraryHandle(3)->registerCallout("four", collection_four);
+    getLibraryHandle(3)->registerCallout("four", collection_four_error);
+    status = getLibraryHandleCollection()->callCallouts(four_index,
+                                                        callout_handle);
+    EXPECT_EQ(1, status);
+    EXPECT_EQ(11223344, callout_value_);
+}
+
+// Same test as CallCalloutsSucess, but with functions returning a "skip"
+// instead.
+
+TEST_F(LibraryHandleCollectionTest, CallCalloutsSkip) {
+    const int one_index = getServerHooks()->getIndex("one");
+    const int two_index = getServerHooks()->getIndex("two");
+    const int three_index = getServerHooks()->getIndex("three");
+    const int four_index = getServerHooks()->getIndex("four");
+
+    // Ensure that no callouts are attached to any of the hooks.
+    EXPECT_FALSE(getLibraryHandleCollection()->calloutsPresent(one_index));
+    EXPECT_FALSE(getLibraryHandleCollection()->calloutsPresent(two_index));
+    EXPECT_FALSE(getLibraryHandleCollection()->calloutsPresent(three_index));
+    EXPECT_FALSE(getLibraryHandleCollection()->calloutsPresent(four_index));
+                 
+    // Set up different sequences of callouts on different handles.
+    CalloutHandle callout_handle(getHookManager());
+    int status;
+
+    // Each library contributing one callout on hook "one". First callout
+    // returns an error.
+    callout_value_ = 0;
+    getLibraryHandle(0)->registerCallout("one", collection_one_skip);
+    getLibraryHandle(1)->registerCallout("one", collection_two);
+    getLibraryHandle(2)->registerCallout("one", collection_three);
+    getLibraryHandle(3)->registerCallout("one", collection_four);
+    status = getLibraryHandleCollection()->callCallouts(one_index,
+                                                        callout_handle);
+    EXPECT_EQ(0, status);
+    EXPECT_EQ(1, callout_value_);
+
+    // Each library contributing multiple callouts on hook "two". Last callout
+    // on first library returns an error after updating the value.
+    callout_value_ = 0;
+    getLibraryHandle(0)->registerCallout("two", collection_one);
+    getLibraryHandle(0)->registerCallout("two", collection_one_skip);
+    getLibraryHandle(1)->registerCallout("two", collection_two);
+    getLibraryHandle(1)->registerCallout("two", collection_two);
+    getLibraryHandle(1)->registerCallout("two", collection_three);
+    getLibraryHandle(1)->registerCallout("two", collection_three);
+    getLibraryHandle(3)->registerCallout("two", collection_four);
+    getLibraryHandle(3)->registerCallout("two", collection_four);
+    status = getLibraryHandleCollection()->callCallouts(two_index,
+                                                        callout_handle);
+    EXPECT_EQ(0, status);
+    EXPECT_EQ(11, callout_value_);
+
+    // Random callout returns an error.
+    callout_value_ = 0;
+    getLibraryHandle(0)->registerCallout("three", collection_one);
+    getLibraryHandle(0)->registerCallout("three", collection_one);
+    getLibraryHandle(1)->registerCallout("three", collection_two);
+    getLibraryHandle(1)->registerCallout("three", collection_two);
+    getLibraryHandle(3)->registerCallout("three", collection_four_skip);
+    getLibraryHandle(3)->registerCallout("three", collection_four);
+    status = getLibraryHandleCollection()->callCallouts(three_index,
+                                                        callout_handle);
+    EXPECT_EQ(0, status);
+    EXPECT_EQ(11224, callout_value_);
+
+    // Last callout returns an error.
+    callout_value_ = 0;
+    getLibraryHandle(0)->registerCallout("four", collection_one);
+    getLibraryHandle(0)->registerCallout("four", collection_one);
+    getLibraryHandle(1)->registerCallout("four", collection_two);
+    getLibraryHandle(1)->registerCallout("four", collection_two);
+    getLibraryHandle(2)->registerCallout("four", collection_three);
+    getLibraryHandle(2)->registerCallout("four", collection_three);
+    getLibraryHandle(3)->registerCallout("four", collection_four);
+    getLibraryHandle(3)->registerCallout("four", collection_four_skip);
+    status = getLibraryHandleCollection()->callCallouts(four_index,
+                                                        callout_handle);
+    EXPECT_EQ(0, status);
+    EXPECT_EQ(11223344, callout_value_);
+}
+
+} // Anonymous namespace

src/lib/util/tests/library_handle_unittest.cc

@@ -47,8 +47,6 @@ public:
         hooks_->registerHook("gamma");
 
         // Also initialize the callout variables.
-        one_count = 0;
-        two_count = 0;
         callout_value = 0;
     }
 
@@ -62,11 +60,9 @@ public:
         return (manager_);
     }
 
-    /// Variables for callouts test. These are public and static to allow non-
-    /// member functions to access them, but declared as class variables to
-    /// allow initialization every time the test starts.
-    static int one_count;
-    static int two_count;
+    /// Variable for callouts test. This is public and static to allow non-
+    /// member functions to access it.  It is initialized every time a
+    /// new test starts.
     static int callout_value;
 
 private:
@@ -74,9 +70,7 @@ private:
     boost::shared_ptr<HookManager> manager_;
 };
 
-// Definition of the static variables.
-int LibraryHandleTest::one_count = 0;
-int LibraryHandleTest::two_count = 0;
+// Definition of the static variable.
 int LibraryHandleTest::callout_value = 0;
 
 // *** Context Tests ***
@@ -88,7 +82,7 @@ int LibraryHandleTest::callout_value = 0;
 // are distinct.
 
 TEST_F(LibraryHandleTest, ContextDistinctSimpleType) {
-    LibraryHandle handle(getServerHooks(), 1);
+    LibraryHandle handle(getServerHooks());
 
     // Store and retrieve an int (random value).
     int a = 42;
@@ -122,7 +116,7 @@ TEST_F(LibraryHandleTest, ContextDistinctSimpleType) {
 // exception.
 
 TEST_F(LibraryHandleTest, ContextUnknownName) {
-    LibraryHandle handle(getServerHooks(), 1);
+    LibraryHandle handle(getServerHooks());
 
     // Set an integer
     int a = 42;
@@ -142,7 +136,7 @@ TEST_F(LibraryHandleTest, ContextUnknownName) {
 // Test that trying to get something with an incorrect type throws an exception.
 
 TEST_F(LibraryHandleTest, ContextIncorrectType) {
-    LibraryHandle handle(getServerHooks(), 1);
+    LibraryHandle handle(getServerHooks());
 
     // Set an integer
     int a = 42;
@@ -171,7 +165,7 @@ struct Beta {
 };
 
 TEST_F(LibraryHandleTest, ComplexTypes) {
-    LibraryHandle handle(getServerHooks(), 1);
+    LibraryHandle handle(getServerHooks());
 
     // Declare two variables of different (complex) types. (Note as to the
     // variable names: aleph and beth are the first two letters of the Hebrew
@@ -210,7 +204,7 @@ TEST_F(LibraryHandleTest, ComplexTypes) {
 // that a "pointer to X" is not the same as a "pointer to const X".
 
 TEST_F(LibraryHandleTest, PointerTypes) {
-    LibraryHandle handle(getServerHooks(), 1);
+    LibraryHandle handle(getServerHooks());
 
     // Declare a couple of variables, const and non-const.
     Alpha aleph(5, 10);
@@ -244,7 +238,7 @@ TEST_F(LibraryHandleTest, PointerTypes) {
 // Check that we can get the names of the context items.
 
 TEST_F(LibraryHandleTest, ContextItemNames) {
-    LibraryHandle handle(getServerHooks(), 1);
+    LibraryHandle handle(getServerHooks());
 
     vector<string> expected_names;
     int value = 42;
@@ -268,7 +262,7 @@ TEST_F(LibraryHandleTest, ContextItemNames) {
 // Check that we can delete one item of context.
 
 TEST_F(LibraryHandleTest, DeleteContext) {
-    LibraryHandle handle(getServerHooks(), 1);
+    LibraryHandle handle(getServerHooks());
 
     int value = 42;
     handle.setContext("faith", value++);
@@ -287,7 +281,7 @@ TEST_F(LibraryHandleTest, DeleteContext) {
 // Delete all all items of context.
 
 TEST_F(LibraryHandleTest, DeleteAllContext) {
-    LibraryHandle handle(getServerHooks(), 1);
+    LibraryHandle handle(getServerHooks());
 
     int value = 42;
     handle.setContext("faith", value++);
@@ -309,40 +303,40 @@ TEST_F(LibraryHandleTest, DeleteAllContext) {
 // The next set of tests check that callouts can be registered.
 
 // Supply callouts structured in such a way that we can determine the order
-// that they are called and whether they are called at all. In particular
-// if the callout order is:
+// that they are called and whether they are called at all. The method used
+// is simple - after a sequence of callouts, the digits in the value, reading
+// left to right, determines the order of the callouts and whether they were
+// called at all.  So:
 //
-// * one followed by two, the resulting value is 20
-// * two followed by one, the resuling value is -10
-// * one and two is not called, the resulting value is 10
-// * two and one is not called, the resulting value is -20
+// * one followed by two, the resulting value is 12
+// * two followed by one, the resuling value is 21
+// * one and two is not called, the resulting value is 1
+// * two and one is not called, the resulting value is 2
 // * neither called, the resulting value is 0
 //
-// The variable xxx_count is the number of times the function has been called
-// in the current test.
+// ... and extending beyond two callouts:
+//
+// * one followed by two followed by three followed by two followed by one
+//   results in a value of 12321.
+//
+// Functions return a zero indicating success.
 
 extern "C" {
 int one(CalloutHandle&) {
-
-    ++LibraryHandleTest::one_count;
-    if (LibraryHandleTest::callout_value == 0) {
-        LibraryHandleTest::callout_value = 10;
-    } else {
-        LibraryHandleTest::callout_value = -10;
-    }
-
+    LibraryHandleTest::callout_value =
+        10 * LibraryHandleTest::callout_value + 1;
     return (0);
 }
 
 int two(CalloutHandle&) {
+    LibraryHandleTest::callout_value =
+        10 * LibraryHandleTest::callout_value + 2;
+    return (0);
+}
 
-    ++LibraryHandleTest::two_count;
-    if (LibraryHandleTest::callout_value == 10) {
-        LibraryHandleTest::callout_value = 20;
-    } else {
-        LibraryHandleTest::callout_value = -20;
-    }
-
+int three(CalloutHandle&) {
+    LibraryHandleTest::callout_value =
+        10 * LibraryHandleTest::callout_value + 3;
     return (0);
 }
 
@@ -356,9 +350,8 @@ int one_error(CalloutHandle& handle) {
 // The next function is a duplicate of "one", but sets the skip flag.
 
 int one_skip(CalloutHandle& handle) {
-    (void) one(handle);
     handle.setSkip(true);
-    return (0);
+    return (one(handle));
 }
 
 };  // extern "C"
@@ -366,7 +359,7 @@ int one_skip(CalloutHandle& handle) {
 // Check that we can register callouts on a particular hook.
 
 TEST_F(LibraryHandleTest, RegisterSingleCallout) {
-    LibraryHandle handle(getServerHooks(), 1);
+    LibraryHandle handle(getServerHooks());
 
     // Register callouts for hooks alpha and see that it is registered.
     EXPECT_FALSE(handle.calloutsPresent(getServerHooks()->getIndex("alpha")));
@@ -384,7 +377,7 @@ TEST_F(LibraryHandleTest, RegisterSingleCallout) {
 // the expected return values.
 
 TEST_F(LibraryHandleTest, CallSingleCallout) {
-    LibraryHandle handle(getServerHooks(), 1);
+    LibraryHandle handle(getServerHooks());
 
     // Register callout for hook alpha...
     EXPECT_FALSE(handle.calloutsPresent(getServerHooks()->getIndex("alpha")));
@@ -392,9 +385,6 @@ TEST_F(LibraryHandleTest, CallSingleCallout) {
     EXPECT_TRUE(handle.calloutsPresent(getServerHooks()->getIndex("alpha")));
 
     // Call it.
-
-    EXPECT_EQ(0, LibraryHandleTest::one_count);
-    EXPECT_EQ(0, LibraryHandleTest::two_count);
     EXPECT_EQ(0, LibraryHandleTest::callout_value);
 
     int index = getServerHooks()->getIndex("alpha");
@@ -402,9 +392,7 @@ TEST_F(LibraryHandleTest, CallSingleCallout) {
     int status = handle.callCallouts(index, callout_handle);
 
     EXPECT_EQ(0, status);
-    EXPECT_EQ(1, LibraryHandleTest::one_count);
-    EXPECT_EQ(0, LibraryHandleTest::two_count);
-    EXPECT_EQ(10, LibraryHandleTest::callout_value);
+    EXPECT_EQ(1, LibraryHandleTest::callout_value);
 
 }
 
@@ -412,15 +400,13 @@ TEST_F(LibraryHandleTest, CallSingleCallout) {
 // in order.
 
 TEST_F(LibraryHandleTest, TwoCallouts) {
-    LibraryHandle handle(getServerHooks(), 1);
+    LibraryHandle handle(getServerHooks());
 
     // Register two callouts for hook alpha...
     handle.registerCallout("alpha", one);
     handle.registerCallout("alpha", two);
 
     // ... and call them.
-    EXPECT_EQ(0, LibraryHandleTest::one_count);
-    EXPECT_EQ(0, LibraryHandleTest::two_count);
     EXPECT_EQ(0, LibraryHandleTest::callout_value);
 
     int index = getServerHooks()->getIndex("alpha");
@@ -428,24 +414,20 @@ TEST_F(LibraryHandleTest, TwoCallouts) {
     int status = handle.callCallouts(index, callout_handle);
 
     EXPECT_EQ(0, status);
-    EXPECT_EQ(1, LibraryHandleTest::one_count);
-    EXPECT_EQ(1, LibraryHandleTest::two_count);
-    EXPECT_EQ(20, LibraryHandleTest::callout_value);
+    EXPECT_EQ(12, LibraryHandleTest::callout_value);
 }
 
 // Check that we can register two callouts for a hook and that the second is not
 // called if the first returns a non-zero status.
 
 TEST_F(LibraryHandleTest, TwoCalloutsWithError) {
-    LibraryHandle handle(getServerHooks(), 1);
+    LibraryHandle handle(getServerHooks());
 
     // Register callout for hook alpha...
     handle.registerCallout("alpha", one_error);
     handle.registerCallout("alpha", two);
 
     // Call them.
-    EXPECT_EQ(0, LibraryHandleTest::one_count);
-    EXPECT_EQ(0, LibraryHandleTest::two_count);
     EXPECT_EQ(0, LibraryHandleTest::callout_value);
 
     int index = getServerHooks()->getIndex("alpha");
@@ -453,24 +435,20 @@ TEST_F(LibraryHandleTest, TwoCalloutsWithError) {
     int status = handle.callCallouts(index, callout_handle);
 
     EXPECT_EQ(1, status);
-    EXPECT_EQ(1, LibraryHandleTest::one_count);
-    EXPECT_EQ(0, LibraryHandleTest::two_count);
-    EXPECT_EQ(10, LibraryHandleTest::callout_value);
+    EXPECT_EQ(1, LibraryHandleTest::callout_value);
 }
 
 // Check that we can register two callouts for a hook and that the second is not
-// called if the first returns a non-zero status.
+// called if the first szets the callout "skip" flag.
 
 TEST_F(LibraryHandleTest, TwoCalloutsWithSkip) {
-    LibraryHandle handle(getServerHooks(), 1);
+    LibraryHandle handle(getServerHooks());
 
     // Register callout for hook alpha...
     handle.registerCallout("alpha", one_skip);
     handle.registerCallout("alpha", two);
 
     // Call them.
-    EXPECT_EQ(0, LibraryHandleTest::one_count);
-    EXPECT_EQ(0, LibraryHandleTest::two_count);
     EXPECT_EQ(0, LibraryHandleTest::callout_value);
 
     int index = getServerHooks()->getIndex("alpha");
@@ -478,15 +456,13 @@ TEST_F(LibraryHandleTest, TwoCalloutsWithSkip) {
     int status = handle.callCallouts(index, callout_handle);
 
     EXPECT_EQ(0, status);
-    EXPECT_EQ(1, LibraryHandleTest::one_count);
-    EXPECT_EQ(0, LibraryHandleTest::two_count);
-    EXPECT_EQ(10, LibraryHandleTest::callout_value);
+    EXPECT_EQ(1, LibraryHandleTest::callout_value);
 }
 
 // Check that a callout can be registered more than once.
 
 TEST_F(LibraryHandleTest, MultipleRegistration) {
-    LibraryHandle handle(getServerHooks(), 1);
+    LibraryHandle handle(getServerHooks());
 
     // Register callouts for hook alpha...
     handle.registerCallout("alpha", one);
@@ -494,8 +470,6 @@ TEST_F(LibraryHandleTest, MultipleRegistration) {
     handle.registerCallout("alpha", one);
 
     // Call them.
-    EXPECT_EQ(0, LibraryHandleTest::one_count);
-    EXPECT_EQ(0, LibraryHandleTest::two_count);
     EXPECT_EQ(0, LibraryHandleTest::callout_value);
 
     int index = getServerHooks()->getIndex("alpha");
@@ -503,15 +477,13 @@ TEST_F(LibraryHandleTest, MultipleRegistration) {
     int status = handle.callCallouts(index, callout_handle);
 
     EXPECT_EQ(0, status);
-    EXPECT_EQ(2, LibraryHandleTest::one_count);
-    EXPECT_EQ(1, LibraryHandleTest::two_count);
-    EXPECT_EQ(-10, LibraryHandleTest::callout_value);
+    EXPECT_EQ(121, LibraryHandleTest::callout_value);
 }
 
 // Check that a callout can be deregistered.
 
 TEST_F(LibraryHandleTest, Deregister) {
-    LibraryHandle handle(getServerHooks(), 1);
+    LibraryHandle handle(getServerHooks());
 
     // Register callouts for hook alpha...
     handle.registerCallout("alpha", one);
@@ -522,8 +494,6 @@ TEST_F(LibraryHandleTest, Deregister) {
     handle.deregisterCallout("alpha", one);
 
     // Call it.
-    EXPECT_EQ(0, LibraryHandleTest::one_count);
-    EXPECT_EQ(0, LibraryHandleTest::two_count);
     EXPECT_EQ(0, LibraryHandleTest::callout_value);
 
     int index = getServerHooks()->getIndex("alpha");
@@ -531,15 +501,13 @@ TEST_F(LibraryHandleTest, Deregister) {
     int status = handle.callCallouts(index, callout_handle);
 
     EXPECT_EQ(0, status);
-    EXPECT_EQ(0, LibraryHandleTest::one_count);
-    EXPECT_EQ(1, LibraryHandleTest::two_count);
-    EXPECT_EQ(-20, LibraryHandleTest::callout_value);
+    EXPECT_EQ(2, LibraryHandleTest::callout_value);
 }
 
 // Check that all callouts can be deregistered.
 
 TEST_F(LibraryHandleTest, DeregisterAll) {
-    LibraryHandle handle(getServerHooks(), 1);
+    LibraryHandle handle(getServerHooks());
 
     // Register callouts for hook alpha...
     EXPECT_FALSE(handle.calloutsPresent(getServerHooks()->getIndex("alpha")));
@@ -556,7 +524,7 @@ TEST_F(LibraryHandleTest, DeregisterAll) {
 // by the constructor, there are five valid hooks, with valid indexes 0 to 4.
 
 TEST_F(LibraryHandleTest, InvalidNameAndIndex) {
-    LibraryHandle handle(getServerHooks(), 1);
+    LibraryHandle handle(getServerHooks());
 
     EXPECT_THROW(handle.registerCallout("omega", one), NoSuchHook);
     EXPECT_THROW(handle.deregisterCallout("omega", one), NoSuchHook);