Browse Source

[master] Merge branch 'trac3054'

Stephen Morris 11 years ago
parent
commit
0f845ed94f

+ 13 - 12
src/lib/hooks/callout_manager.cc

@@ -29,6 +29,19 @@ using namespace std;
 namespace isc {
 namespace hooks {
 
+// Constructor
+CalloutManager::CalloutManager(int num_libraries)
+    : current_hook_(-1), current_library_(-1),
+      hook_vector_(ServerHooks::getServerHooks().getCount()),
+      library_handle_(this), pre_library_handle_(this, 0),
+      post_library_handle_(this, INT_MAX), num_libraries_(num_libraries)
+{
+    if (num_libraries < 0) {
+        isc_throw(isc::BadValue, "number of libraries passed to the "
+                  "CalloutManager must be >= 0");
+    }
+}
+
 // Check that the index of a library is valid.  It can range from 1 - n
 // (n is the number of libraries), 0 (pre-user library callouts), or INT_MAX
 // (post-user library callouts).  It can also be -1 to indicate an invalid
@@ -46,18 +59,6 @@ CalloutManager::checkLibraryIndex(int library_index) const {
               num_libraries_ << ")");
 }
 
-// Set the number of libraries handled by the CalloutManager.
-
-void
-CalloutManager::setNumLibraries(int num_libraries) {
-    if (num_libraries < 0) {
-        isc_throw(isc::BadValue, "number of libraries passed to the "
-                  "CalloutManager must be >= 0");
-    }
-
-    num_libraries_ = num_libraries;
-}
-
 // Register a callout for the current library.
 
 void

+ 2 - 29
src/lib/hooks/callout_manager.h

@@ -132,19 +132,8 @@ public:
     ///
     /// @param num_libraries Number of loaded libraries.
     ///
-    /// @throw isc::BadValue if the number of libraries is less than or equal
-    ///        to 0, or if the pointer to the server hooks object is empty.
-    CalloutManager(int num_libraries = 0)
-        : current_hook_(-1), current_library_(-1),
-          hook_vector_(ServerHooks::getServerHooks().getCount()),
-          library_handle_(this), pre_library_handle_(this, 0),
-          post_library_handle_(this, INT_MAX), num_libraries_(num_libraries)
-    {
-        // Check that the number of libraries is OK.  (This does a redundant
-        // set of the number of libraries, but it's only a single assignment
-        // and avoids the need for a separate "check" method.
-        setNumLibraries(num_libraries);
-    }
+    /// @throw isc::BadValue if the number of libraries is less than 0,
+    CalloutManager(int num_libraries = 0);
 
     /// @brief Register a callout on a hook for the current library
     ///
@@ -224,22 +213,6 @@ public:
         return (current_hook_);
     }
 
-    /// @brief Set number of libraries
-    ///
-    /// Sets the number of libraries.  Although the value is passed to the
-    /// constructor, in some cases that is only an estimate and the number
-    /// can only be determined after the CalloutManager is created.
-    ///
-    /// @note If the number if libraries is reset, it must be done *before*
-    ///       any callouts are registered.
-    ///
-    /// @param num_libraries Number of libraries served by this CalloutManager.
-    ///
-    /// @throw BadValue Number of libraries must be >= 0.
-    /// @throw LibraryCountChanged Number of libraries has been changed after
-    ///        callouts have been registered.
-    void setNumLibraries(int num_libraries);
-
     /// @brief Get number of libraries
     ///
     /// Returns the number of libraries that this CalloutManager is expected

+ 29 - 2
src/lib/hooks/hooks_manager.cc

@@ -81,8 +81,15 @@ HooksManager::loadLibrariesInternal(const std::vector<std::string>& libraries) {
     lm_collection_.reset(new LibraryManagerCollection(libraries));
     bool status = lm_collection_->loadLibraries();
 
-    // ... and obtain the callout manager for them.
-    callout_manager_ = lm_collection_->getCalloutManager();
+    if (status) {
+        // ... and obtain the callout manager for them if successful.
+        callout_manager_ = lm_collection_->getCalloutManager();
+    } else {
+        // Unable to load libraries, reset to state before this function was
+        // called.
+        lm_collection_.reset();
+        callout_manager_.reset();
+    }
 
     return (status);
 }
@@ -124,6 +131,19 @@ HooksManager::createCalloutHandle() {
     return (getHooksManager().createCalloutHandleInternal());
 }
 
+// Get the list of the names of loaded libraries.
+
+std::vector<std::string>
+HooksManager::getLibraryNamesInternal() const {
+    return (lm_collection_ ? lm_collection_->getLibraryNames()
+                           : std::vector<std::string>());
+}
+
+std::vector<std::string>
+HooksManager::getLibraryNames() {
+    return (getHooksManager().getLibraryNamesInternal());
+}
+
 // Perform conditional initialization if nothing is loaded.
 
 void
@@ -169,5 +189,12 @@ HooksManager::postCalloutsLibraryHandle() {
     return (getHooksManager().postCalloutsLibraryHandleInternal());
 }
 
+// Validate libraries
+
+std::vector<std::string>
+HooksManager::validateLibraries(const std::vector<std::string>& libraries) {
+    return (LibraryManagerCollection::validateLibraries(libraries));
+}
+
 } // namespace util
 } // namespace isc

+ 40 - 0
src/lib/hooks/hooks_manager.h

@@ -171,6 +171,30 @@ public:
     ///         registered.
     static int registerHook(const std::string& name);
 
+    /// @brief Return list of loaded libraries
+    ///
+    /// Returns the names of the loaded libraries.
+    ///
+    /// @return List of loaded library names.
+    static std::vector<std::string> getLibraryNames();
+
+    /// @brief Validate library list
+    ///
+    /// For each library passed to it, checks that the library can be opened
+    /// and that the "version" function is present and gives the right answer.
+    /// Each library is closed afterwards.
+    ///
+    /// This is used during the configuration parsing - when the list of hooks
+    /// libraries is changed, each of the new libraries is checked before the
+    /// change is committed.
+    ///
+    /// @param List of libraries to be validated.
+    ///
+    /// @return An empty vector if all libraries validated.  Otherwise it
+    ///         holds the names of the libraries that failed validation.
+    static std::vector<std::string> validateLibraries(
+                       const std::vector<std::string>& libraries);
+
     /// Index numbers for pre-defined hooks.
     static const int CONTEXT_CREATE = ServerHooks::CONTEXT_CREATE;
     static const int CONTEXT_DESTROY = ServerHooks::CONTEXT_DESTROY;
@@ -188,6 +212,17 @@ private:
     /// but actually do the work on the singleton instance of the HooksManager.
     /// See the descriptions of the static methods for more details.
 
+    /// @brief Validate library list
+    ///
+    /// @param List of libraries to be validated.
+    ///
+    /// @return An empty string if all libraries validated.  Otherwise it is
+    ///         the name of the first library that failed validation.  The
+    ///         configuration code can return this to bindctl as an indication
+    ///         of the problem.
+    std::string validateLibrariesInternal(
+                       const std::vector<std::string>& libraries) const;
+
     /// @brief Load and reload libraries
     ///
     /// @param libraries List of libraries to be loaded.  The order is
@@ -234,6 +269,11 @@ private:
     ///         registration.
     LibraryHandle& postCalloutsLibraryHandleInternal();
 
+    /// @brief Return list of loaded libraries
+    ///
+    /// @return List of loaded library names.
+    std::vector<std::string> getLibraryNamesInternal() const;
+
     //@}
 
     /// @brief Initialization to No Libraries

+ 88 - 27
src/lib/hooks/library_manager.cc

@@ -31,6 +31,42 @@ namespace isc {
 namespace hooks {
 
 
+// Constructor (used by external agency)
+LibraryManager::LibraryManager(const std::string& name, int index,
+                               const boost::shared_ptr<CalloutManager>& manager)
+        : dl_handle_(NULL), index_(index), manager_(manager),
+          library_name_(name)
+{
+    if (!manager) {
+        isc_throw(NoCalloutManager, "must specify a CalloutManager when "
+                  "instantiating a LibraryManager object");
+    }
+}
+
+// Constructor (used by "validate" for library validation).  Note that this
+// sets "manager_" to not point to anything, which means that methods such as
+// registerStandardCallout() will fail, probably with a segmentation fault.
+// There are no checks for this condition in those methods: this constructor
+// is declared "private", so can only be executed by a method in this class.
+// The only method to do so is "validateLibrary", which takes care not to call
+// methods requiring a non-NULL manager.
+LibraryManager::LibraryManager(const std::string& name)
+        : dl_handle_(NULL), index_(-1), manager_(), library_name_(name)
+{}
+
+// Destructor.  
+LibraryManager::~LibraryManager() {
+    if (manager_) {
+        // LibraryManager instantiated to load a library, so ensure that
+        // it is unloaded before exiting.
+        static_cast<void>(unloadLibrary());
+    } else {
+        // LibraryManager instantiated to validate a library, so just ensure
+        // that it is closed before exiting.
+        static_cast<void>(closeLibrary());
+    }
+}
+
 // Open the library
 
 bool
@@ -118,8 +154,9 @@ LibraryManager::registerStandardCallouts() {
             // Found a symbol, so register it.
             manager_->getLibraryHandle().registerCallout(hook_names[i],
                                                          pc.calloutPtr());
-            LOG_DEBUG(hooks_logger, HOOKS_DBG_CALLS, HOOKS_STD_CALLOUT_REGISTERED)
-                .arg(library_name_).arg(hook_names[i]).arg(dlsym_ptr);
+            LOG_DEBUG(hooks_logger, HOOKS_DBG_CALLS,
+                      HOOKS_STD_CALLOUT_REGISTERED).arg(library_name_)
+                      .arg(hook_names[i]).arg(dlsym_ptr);
 
         }
     }
@@ -251,41 +288,65 @@ LibraryManager::loadLibrary() {
 }
 
 // The library unloading function.  Call the unload() function (if present),
-// remove callouts from the callout manager, then close the library.
+// remove callouts from the callout manager, then close the library.  This is
+// only run if the library is still loaded and is a no-op if the library is
+// not open.
 
 bool
 LibraryManager::unloadLibrary() {
-    LOG_DEBUG(hooks_logger, HOOKS_DBG_TRACE, HOOKS_LIBRARY_UNLOADING)
-        .arg(library_name_);
+    bool result = true;
+    if (dl_handle_ != NULL) {
+        LOG_DEBUG(hooks_logger, HOOKS_DBG_TRACE, HOOKS_LIBRARY_UNLOADING)
+            .arg(library_name_);
 
-    // Call the unload() function if present.  Note that this is done first -
-    // operations take place in the reverse order to which they were done when
-    // the library was loaded.
-    bool result = runUnload();
+        // Call the unload() function if present.  Note that this is done first
+        // - operations take place in the reverse order to which they were done
+        // when the library was loaded.
+        result = runUnload();
+
+        // Regardless of status, remove all callouts associated with this
+        // library on all hooks.
+        vector<string> hooks = ServerHooks::getServerHooks().getHookNames();
+        manager_->setLibraryIndex(index_);
+        for (int i = 0; i < hooks.size(); ++i) {
+            bool removed = manager_->deregisterAllCallouts(hooks[i]);
+            if (removed) {
+                LOG_DEBUG(hooks_logger, HOOKS_DBG_CALLS, HOOKS_CALLOUTS_REMOVED)
+                    .arg(hooks[i]).arg(library_name_);
+            }
+        }
 
-    // Regardless of status, remove all callouts associated with this library
-    // on all hooks.
-    vector<string> hooks = ServerHooks::getServerHooks().getHookNames();
-    manager_->setLibraryIndex(index_);
-    for (int i = 0; i < hooks.size(); ++i) {
-        bool removed = manager_->deregisterAllCallouts(hooks[i]);
-        if (removed) {
-            LOG_DEBUG(hooks_logger, HOOKS_DBG_CALLS, HOOKS_CALLOUTS_REMOVED)
-                .arg(hooks[i]).arg(library_name_);
+        // ... and close the library.
+        result = closeLibrary() && result;
+        if (result) {
+
+            // Issue the informational message only if the library was unloaded
+            // with no problems.  If there was an issue, an error message would
+            // have been issued.
+            LOG_INFO(hooks_logger, HOOKS_LIBRARY_UNLOADED).arg(library_name_);
         }
     }
+    return (result);
+}
 
-    // ... and close the library.
-    result = closeLibrary() && result;
-    if (result) {
+// Validate the library.  We must be able to open it, and the version function
+// must both exist and return the right number.  Note that this is a static
+// method.
 
-        // Issue the informational message only if the library was unloaded
-        // with no problems.  If there was an issue, an error message would
-        // have been issued.
-        LOG_INFO(hooks_logger, HOOKS_LIBRARY_UNLOADED).arg(library_name_);
-    }
+bool
+LibraryManager::validateLibrary(const std::string& name) {
+    // Instantiate a library manager for the validation.  We use the private
+    // constructor as we don't supply a CalloutManager.
+    LibraryManager manager(name);
 
-    return (result);
+    // Try to open it and, if we succeed, check the version.
+    bool validated = manager.openLibrary() && manager.checkVersion();
+
+    // Regardless of whether the version checked out, close the library. (This
+    // is a no-op if the library failed to open.)
+    static_cast<void>(manager.closeLibrary());
+
+    return (validated);
 }
 
 } // namespace hooks

+ 50 - 9
src/lib/hooks/library_manager.h

@@ -15,6 +15,8 @@
 #ifndef LIBRARY_MANAGER_H
 #define LIBRARY_MANAGER_H
 
+#include <exceptions/exceptions.h>
+
 #include <boost/shared_ptr.hpp>
 
 #include <string>
@@ -22,13 +24,25 @@
 namespace isc {
 namespace hooks {
 
+/// @brief No Callout Manager
+///
+/// Thrown if a library manager is instantiated by an external agency without
+/// specifying a CalloutManager object.
+class NoCalloutManager : public Exception {
+public:
+    NoCalloutManager(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) {}
+};
+
 class CalloutManager;
 class LibraryHandle;
 class LibraryManager;
 
 /// @brief Library manager
 ///
-/// This class handles the loading and unloading of a specific library.
+/// This class handles the loading and unloading of a specific library.  It also
+/// provides a static method for checking that a library is valid (this is used
+/// in configuration parsing).
 ///
 /// On loading, it opens the library using dlopen and checks the version (set
 /// with the "version" method.  If all is OK, it iterates through the list of
@@ -58,22 +72,28 @@ class LibraryManager;
 ///       suspends processing of new requests until all existing ones have
 ///       been serviced and all packet/context structures destroyed before
 ///       reloading the libraries.
+///
+/// When validating a library, only the fact that the library can be opened and
+/// version() exists and returns the correct number is checked.  The library
+/// is closed after the validation.
 
 class LibraryManager {
 public:
     /// @brief Constructor
     ///
-    /// Stores the library name.  The actual loading is done in loadLibrary().
+    /// This constructor is used by external agencies (i.e. the
+    /// LibraryManagerCollection) when instantiating a LibraryManager.  It
+    /// stores the library name - the actual actual loading is done in
+    /// loadLibrary().
     ///
     /// @param name Name of the library to load.  This should be an absolute
     ///        path name.
     /// @param index Index of this library
     /// @param manager CalloutManager object
+    ///
+    /// @throw NoCalloutManager Thrown if the manager argument is NULL.
     LibraryManager(const std::string& name, int index,
-                   const boost::shared_ptr<CalloutManager>& manager)
-        : dl_handle_(NULL), index_(index), manager_(manager),
-          library_name_(name)
-    {}
+                   const boost::shared_ptr<CalloutManager>& manager);
 
     /// @brief Destructor
     ///
@@ -81,9 +101,19 @@ public:
     /// feature to ensure closure in the case of an exception destroying this
     /// object.  However, see the caveat in the class header about when it is
     /// safe to unload libraries.
-    ~LibraryManager() {
-        static_cast<void>(unloadLibrary());
-    }
+    ~LibraryManager();
+
+    /// @brief Validate library
+    ///
+    /// A static method that is used to validate a library.  Validation checks
+    /// that the library can be opened, that "version" exists, and that it
+    /// returns the right number.
+    ///
+    /// @param name Name of the library to validate
+    ///
+    /// @return true if the library validated, false if not.  If the library
+    /// fails to validate, the reason for the failure is logged.
+    static bool validateLibrary(const std::string& name);
 
     /// @brief Loads a library
     ///
@@ -176,6 +206,17 @@ protected:
     bool runUnload();
 
 private:
+    /// @brief Validating constructor
+    ///
+    /// Constructor used when the LibraryManager is instantiated to validate
+    /// a library (i.e. by the "validateLibrary" static method).
+    ///
+    /// @param name Name of the library to load.  This should be an absolute
+    ///        path name.
+    LibraryManager(const std::string& name);
+
+    // Member variables
+
     void*       dl_handle_;     ///< Handle returned by dlopen
     int         index_;         ///< Index associated with this library
     boost::shared_ptr<CalloutManager> manager_;

+ 28 - 13
src/lib/hooks/library_manager_collection.cc

@@ -73,24 +73,18 @@ LibraryManagerCollection::loadLibraries() {
                                    callout_manager_));
 
         // Load the library.  On success, add it to the list of loaded
-        // libraries.  On failure, an error will have been logged and the
-        // library closed.
+        // libraries.  On failure, unload all currently loaded libraries,
+        // leaving the object in the state it was in before loadLibraries was
+        // called.
         if (manager->loadLibrary()) {
             lib_managers_.push_back(manager);
+        } else {
+            static_cast<void>(unloadLibraries());
+            return (false);
         }
     }
 
-    // Update the CalloutManager's idea of the number of libraries it is
-    // handling.
-    callout_manager_->setNumLibraries(lib_managers_.size());
-
-    // Get an indication of whether all libraries loaded successfully.
-    bool status = (library_names_.size() == lib_managers_.size());
-
-    // Don't need the library names any more, so free up the space.
-    library_names_.clear();
-
-    return (status);
+    return (true);
 }
 
 // Unload the libraries.
@@ -110,5 +104,26 @@ LibraryManagerCollection::unloadLibraries() {
     callout_manager_.reset();
 }
 
+// Return number of loaded libraries.
+int
+LibraryManagerCollection::getLoadedLibraryCount() const {
+    return (lib_managers_.size());
+}
+
+// Validate the libraries.
+std::vector<std::string>
+LibraryManagerCollection::validateLibraries(
+                          const std::vector<std::string>& libraries) {
+
+    std::vector<std::string> failures;
+    for (int i = 0; i < libraries.size(); ++i) {
+        if (!LibraryManager::validateLibrary(libraries[i])) {
+            failures.push_back(libraries[i]);
+        }
+    }
+
+    return (failures);
+}
+
 } // namespace hooks
 } // namespace isc

+ 41 - 4
src/lib/hooks/library_manager_collection.h

@@ -69,13 +69,17 @@ class LibraryManager;
 /// code.  However, the link with the CalloutHandle does at least mean that
 /// authors of server code do not need to be so careful about when they destroy
 /// CalloutHandles.
+///
+/// The collection object also provides a utility function to validate a set
+/// of libraries.  The function checks that each library exists, can be opened,
+/// that the "version" function exists and return the right number.
 
 class LibraryManagerCollection {
 public:
     /// @brief Constructor
     ///
-    /// @param List of libraries that this collection will manage.  The order
-    ///        of the libraries is important.
+    /// @param libraries List of libraries that this collection will manage.
+    ///        The order of the libraries is important.
     LibraryManagerCollection(const std::vector<std::string>& libraries)
         : library_names_(libraries)
     {}
@@ -91,8 +95,11 @@ public:
     ///
     /// Loads the libraries.  This creates the LibraryManager associated with
     /// each library and calls its loadLibrary() method.  If a library fails
-    /// to load, the fact is noted but attempts are made to load the remaining
-    /// libraries.
+    /// to load, the loading is abandoned and all libraries loaded so far
+    /// are unloaded.
+    ///
+    /// @return true if all libraries loaded, false if one or more failed t
+    ////        load.
     bool loadLibraries();
 
     /// @brief Get callout manager
@@ -107,6 +114,36 @@ public:
     ///        construction and the time loadLibraries() is called.
     boost::shared_ptr<CalloutManager> getCalloutManager() const;
 
+    /// @brief Get library names
+    ///
+    /// Returns the list of library names.  If called before loadLibraries(),
+    /// the list is the list of names to be loaded; if called afterwards, it
+    /// is the list of libraries that have been loaded.
+    std::vector<std::string> getLibraryNames() const {
+        return (library_names_);
+    }
+
+    /// @brief Get number of loaded libraries
+    ///
+    /// Mainly for testing, this returns the number of libraries that are
+    /// loaded.
+    ///
+    /// @return Number of libraries that are loaded.
+    int getLoadedLibraryCount() const;
+
+    /// @brief Validate libraries
+    ///
+    /// Utility function to validate libraries.  It checks that the libraries
+    /// exist, can be opened, that a "version" function is present in them, and
+    /// that it returns the right number.  All errors are logged.
+    ///
+    /// @param libraries List of libraries to validate
+    ///
+    /// @return Vector of libraries that faled to validate, or an empty vector
+    ///         if all validated.
+    static std::vector<std::string>
+    validateLibraries(const std::vector<std::string>& libraries);
+
 protected:
     /// @brief Unload libraries
     ///

+ 0 - 4
src/lib/hooks/tests/callout_manager_unittest.cc

@@ -199,10 +199,6 @@ TEST_F(CalloutManagerTest, NumberOfLibraries) {
 
     EXPECT_NO_THROW(cm.reset(new CalloutManager(42)));
     EXPECT_EQ(42, cm->getNumLibraries());
-
-    // Check that setting the number of libraries alterns the number reported.
-    EXPECT_NO_THROW(cm->setNumLibraries(27));
-    EXPECT_EQ(27, cm->getNumLibraries());
 }
 
 // Check that we can only set the current library index to the correct values.

+ 97 - 27
src/lib/hooks/tests/hooks_manager_unittest.cc

@@ -157,33 +157,6 @@ TEST_F(HooksManagerTest, LoadLibrariesWithError) {
     // Load the libraries.  We expect a failure return because one of the
     // libraries fails to load.
     EXPECT_FALSE(HooksManager::loadLibraries(library_names));
-
-    // Execute the callouts.  The first library implements the calculation.
-    //
-    // r3 = (7 * d1 - d2) * d3
-    //
-    // The last-loaded library implements the calculation
-    //
-    // r3 = (10 + d1) * d2 - d3
-    //
-    // Putting the processing for each library together in the appropriate
-    // order, we get:
-    //
-    // r3 = ((10 * d1 + d1) - d2) * d2 * d3 - d3
-    {
-        SCOPED_TRACE("Calculation with libraries loaded");
-        executeCallCallouts(10, 3, 33, 2, 62, 3, 183);
-    }
-
-    // Try unloading the libraries.
-    EXPECT_NO_THROW(HooksManager::unloadLibraries());
-
-    // Re-execute the calculation - callouts can be called but as nothing
-    // happens, the result should always be -1.
-    {
-        SCOPED_TRACE("Calculation with libraries not loaded");
-        executeCallCallouts(-1, 3, -1, 22, -1, 83, -1);
-    }
 }
 
 // Test that we can unload a set of libraries while we have a CalloutHandle
@@ -450,5 +423,102 @@ TEST_F(HooksManagerTest, RegisterHooks) {
     EXPECT_EQ(string("gamma"), names[4]);
 }
 
+// Check that we can get the names of the libraries.
+
+TEST_F(HooksManagerTest, LibraryNames) {
+
+    // Set up the list of libraries to be loaded.
+    std::vector<std::string> library_names;
+    library_names.push_back(std::string(FULL_CALLOUT_LIBRARY));
+    library_names.push_back(std::string(BASIC_CALLOUT_LIBRARY));
+
+    // Check the names before the libraries are loaded.
+    std::vector<std::string> loaded_names = HooksManager::getLibraryNames();
+    EXPECT_TRUE(loaded_names.empty());
+
+    // Load the libraries and check the names again.
+    EXPECT_TRUE(HooksManager::loadLibraries(library_names));
+    loaded_names = HooksManager::getLibraryNames();
+    EXPECT_TRUE(library_names == loaded_names);
+
+    // Unload the libraries and check again.
+    EXPECT_NO_THROW(HooksManager::unloadLibraries());
+    loaded_names = HooksManager::getLibraryNames();
+    EXPECT_TRUE(loaded_names.empty());
+}
+
+// Test the library validation function.
+
+TEST_F(HooksManagerTest, validateLibraries) {
+    // Vector of libraries that failed validation
+    std::vector<std::string> failed;
+
+    // Test different vectors of libraries.
+
+    // No libraries should return a success.
+    std::vector<std::string> libraries;
+
+    failed = HooksManager::validateLibraries(libraries);
+    EXPECT_TRUE(failed.empty());
+
+    // Single valid library should validate.
+    libraries.clear();
+    libraries.push_back(BASIC_CALLOUT_LIBRARY);
+
+    failed = HooksManager::validateLibraries(libraries);
+    EXPECT_TRUE(failed.empty());
+
+    // Multiple valid libraries should succeed.
+    libraries.clear();
+    libraries.push_back(BASIC_CALLOUT_LIBRARY);
+    libraries.push_back(FULL_CALLOUT_LIBRARY);
+    libraries.push_back(UNLOAD_CALLOUT_LIBRARY);
+
+    failed = HooksManager::validateLibraries(libraries);
+    EXPECT_TRUE(failed.empty());
+
+    // Single invalid library should fail.
+    libraries.clear();
+    libraries.push_back(NOT_PRESENT_LIBRARY);
+
+    failed = HooksManager::validateLibraries(libraries);
+    EXPECT_TRUE(failed == libraries);
+
+    // Multiple invalid libraries should fail.
+    libraries.clear();
+    libraries.push_back(INCORRECT_VERSION_LIBRARY);
+    libraries.push_back(NO_VERSION_LIBRARY);
+    libraries.push_back(FRAMEWORK_EXCEPTION_LIBRARY);
+
+    failed = HooksManager::validateLibraries(libraries);
+    EXPECT_TRUE(failed == libraries);
+
+    // Combination of valid and invalid (first one valid) should fail.
+    libraries.clear();
+    libraries.push_back(FULL_CALLOUT_LIBRARY);
+    libraries.push_back(INCORRECT_VERSION_LIBRARY);
+    libraries.push_back(NO_VERSION_LIBRARY);
+
+    std::vector<std::string> expected_failures;
+    expected_failures.push_back(INCORRECT_VERSION_LIBRARY);
+    expected_failures.push_back(NO_VERSION_LIBRARY);
+
+    failed = HooksManager::validateLibraries(libraries);
+    EXPECT_TRUE(failed == expected_failures);
+
+    // Combination of valid and invalid (first one invalid) should fail.
+    libraries.clear();
+    libraries.push_back(NO_VERSION_LIBRARY);
+    libraries.push_back(FULL_CALLOUT_LIBRARY);
+    libraries.push_back(INCORRECT_VERSION_LIBRARY);
+
+    expected_failures.clear();
+    expected_failures.push_back(NO_VERSION_LIBRARY);
+    expected_failures.push_back(INCORRECT_VERSION_LIBRARY);
+
+    failed = HooksManager::validateLibraries(libraries);
+    EXPECT_TRUE(failed == expected_failures);
+}
+
 
 } // Anonymous namespace

+ 105 - 38
src/lib/hooks/tests/library_manager_collection_unittest.cc

@@ -76,8 +76,7 @@ TEST_F(LibraryManagerCollectionTest, LoadLibraries) {
 
     // Load the libraries.
     EXPECT_TRUE(lm_collection.loadLibraries());
-    boost::shared_ptr<CalloutManager> manager =
-                                      lm_collection.getCalloutManager();
+    EXPECT_EQ(2, lm_collection.getLoadedLibraryCount());
 
     // Execute the callouts.  The first library implements the calculation.
     //
@@ -91,6 +90,8 @@ TEST_F(LibraryManagerCollectionTest, LoadLibraries) {
     // order, we get:
     //
     // r3 = ((10 * d1 + d1) - d2) * d2 * d3 - d3
+    boost::shared_ptr<CalloutManager> manager =
+                                      lm_collection.getCalloutManager();
     {
         SCOPED_TRACE("Doing calculation with libraries loaded");
         executeCallCallouts(manager, 10, 3, 33, 2, 62, 3, 183);
@@ -98,6 +99,7 @@ TEST_F(LibraryManagerCollectionTest, LoadLibraries) {
 
     // Try unloading the libraries.
     EXPECT_NO_THROW(lm_collection.unloadLibraries());
+    EXPECT_EQ(0, lm_collection.getLoadedLibraryCount());
 
     // Re-execute the calculation - callouts can be called but as nothing
     // happens, the result should always be -1.
@@ -108,8 +110,7 @@ TEST_F(LibraryManagerCollectionTest, LoadLibraries) {
 }
 
 // This is effectively the same test as above, but with a library generating
-// an error when loaded. It is expected that the failing library will not be
-// loaded, but others will be.
+// an error when loaded. It is expected that no libraries will be loaded.
 
 TEST_F(LibraryManagerCollectionTest, LoadLibrariesWithError) {
 
@@ -126,38 +127,9 @@ TEST_F(LibraryManagerCollectionTest, LoadLibrariesWithError) {
     // Load the libraries.  We expect a failure status to be returned as
     // one of the libraries failed to load.
     EXPECT_FALSE(lm_collection.loadLibraries());
-    boost::shared_ptr<CalloutManager> manager =
-                                      lm_collection.getCalloutManager();
-
-    // Expect only two libraries were loaded.
-    EXPECT_EQ(2, manager->getNumLibraries());
-
-    // Execute the callouts.  The first library implements the calculation.
-    //
-    // r3 = (7 * d1 - d2) * d3
-    //
-    // The last-loaded library implements the calculation
-    //
-    // r3 = (10 + d1) * d2 - d3
-    //
-    // Putting the processing for each library together in the appropriate
-    // order, we get:
-    //
-    // r3 = ((10 * d1 + d1) - d2) * d2 * d3 - d3
-    {
-        SCOPED_TRACE("Doing calculation with libraries loaded");
-        executeCallCallouts(manager, 10, 3, 33, 2, 62, 3, 183);
-    }
-
-    // Try unloading the libraries.
-    EXPECT_NO_THROW(lm_collection.unloadLibraries());
 
-    // Re-execute the calculation - callouts can be called but as nothing
-    // happens, the result should always be -1.
-    {
-        SCOPED_TRACE("Doing calculation with libraries not loaded");
-        executeCallCallouts(manager, -1, 3, -1, 22, -1, 83, -1);
-    }
+    // Expect no libraries were loaded.
+    EXPECT_EQ(0, lm_collection.getLoadedLibraryCount());
 }
 
 // Check that everything works even with no libraries loaded.
@@ -170,15 +142,110 @@ TEST_F(LibraryManagerCollectionTest, NoLibrariesLoaded) {
     // be using.
     LibraryManagerCollection lm_collection(library_names);
     EXPECT_TRUE(lm_collection.loadLibraries());
+    EXPECT_EQ(0, lm_collection.getLoadedLibraryCount());
     boost::shared_ptr<CalloutManager> manager =
                                       lm_collection.getCalloutManager();
 
-    // Load the libraries.
-    EXPECT_TRUE(lm_collection.loadLibraries());
-
     // Eecute the calculation - callouts can be called but as nothing
     // happens, the result should always be -1.
     executeCallCallouts(manager, -1, 3, -1, 22, -1, 83, -1);
 }
 
+// Check that we can get the names of the libraries.
+
+TEST_F(LibraryManagerCollectionTest, LibraryNames) {
+
+    // Set up the list of libraries to be loaded.
+    std::vector<std::string> library_names;
+    library_names.push_back(std::string(FULL_CALLOUT_LIBRARY));
+    library_names.push_back(std::string(BASIC_CALLOUT_LIBRARY));
+
+    // Set up the library manager collection and get the callout manager we'll
+    // be using.
+    PublicLibraryManagerCollection lm_collection(library_names);
+
+    // Check the names before the libraries are loaded.
+    std::vector<std::string> collection_names = lm_collection.getLibraryNames();
+    EXPECT_TRUE(library_names == collection_names);
+
+    // Load the libraries and check the names again.
+    EXPECT_TRUE(lm_collection.loadLibraries());
+    EXPECT_EQ(2, lm_collection.getLoadedLibraryCount());
+    collection_names = lm_collection.getLibraryNames();
+    EXPECT_TRUE(library_names == collection_names);
+}
+
+// Test the library validation function.
+
+TEST_F(LibraryManagerCollectionTest, validateLibraries) {
+    // Vector of libraries that failed validation
+    std::vector<std::string> failed;
+
+    // Test different vectors of libraries.
+
+    // No libraries should return a success.
+    std::vector<std::string> libraries;
+
+    failed = LibraryManagerCollection::validateLibraries(libraries);
+    EXPECT_TRUE(failed.empty());
+
+    // Single valid library should validate.
+    libraries.clear();
+    libraries.push_back(BASIC_CALLOUT_LIBRARY);
+
+    failed = LibraryManagerCollection::validateLibraries(libraries);
+    EXPECT_TRUE(failed.empty());
+
+    // Multiple valid libraries should succeed.
+    libraries.clear();
+    libraries.push_back(BASIC_CALLOUT_LIBRARY);
+    libraries.push_back(FULL_CALLOUT_LIBRARY);
+    libraries.push_back(UNLOAD_CALLOUT_LIBRARY);
+
+    failed = LibraryManagerCollection::validateLibraries(libraries);
+    EXPECT_TRUE(failed.empty());
+
+    // Single invalid library should fail.
+    libraries.clear();
+    libraries.push_back(NOT_PRESENT_LIBRARY);
+
+    failed = LibraryManagerCollection::validateLibraries(libraries);
+    EXPECT_TRUE(failed == libraries);
+
+    // Multiple invalid libraries should fail.
+    libraries.clear();
+    libraries.push_back(INCORRECT_VERSION_LIBRARY);
+    libraries.push_back(NO_VERSION_LIBRARY);
+    libraries.push_back(FRAMEWORK_EXCEPTION_LIBRARY);
+
+    failed = LibraryManagerCollection::validateLibraries(libraries);
+    EXPECT_TRUE(failed == libraries);
+
+    // Combination of valid and invalid (first one valid) should fail.
+    libraries.clear();
+    libraries.push_back(FULL_CALLOUT_LIBRARY);
+    libraries.push_back(INCORRECT_VERSION_LIBRARY);
+    libraries.push_back(NO_VERSION_LIBRARY);
+
+    std::vector<std::string> expected_failures;
+    expected_failures.push_back(INCORRECT_VERSION_LIBRARY);
+    expected_failures.push_back(NO_VERSION_LIBRARY);
+
+    failed = LibraryManagerCollection::validateLibraries(libraries);
+    EXPECT_TRUE(failed == expected_failures);
+
+    // Combination of valid and invalid (first one invalid) should fail.
+    libraries.clear();
+    libraries.push_back(NO_VERSION_LIBRARY);
+    libraries.push_back(FULL_CALLOUT_LIBRARY);
+    libraries.push_back(INCORRECT_VERSION_LIBRARY);
+
+    expected_failures.clear();
+    expected_failures.push_back(NO_VERSION_LIBRARY);
+    expected_failures.push_back(INCORRECT_VERSION_LIBRARY);
+
+    failed = LibraryManagerCollection::validateLibraries(libraries);
+    EXPECT_TRUE(failed == expected_failures);
+}
+
 } // Anonymous namespace

+ 14 - 0
src/lib/hooks/tests/library_manager_unittest.cc

@@ -552,4 +552,18 @@ TEST_F(LibraryManagerTest, LoadMultipleLibraries) {
     EXPECT_TRUE(lib_manager_4.unloadLibrary());
 }
 
+// Check that libraries can be validated.
+
+TEST_F(LibraryManagerTest, validateLibraries) {
+    EXPECT_TRUE(LibraryManager::validateLibrary(BASIC_CALLOUT_LIBRARY));
+    EXPECT_TRUE(LibraryManager::validateLibrary(FULL_CALLOUT_LIBRARY));
+    EXPECT_FALSE(LibraryManager::validateLibrary(FRAMEWORK_EXCEPTION_LIBRARY));
+    EXPECT_FALSE(LibraryManager::validateLibrary(INCORRECT_VERSION_LIBRARY));
+    EXPECT_TRUE(LibraryManager::validateLibrary(LOAD_CALLOUT_LIBRARY));
+    EXPECT_TRUE(LibraryManager::validateLibrary(LOAD_ERROR_CALLOUT_LIBRARY));
+    EXPECT_FALSE(LibraryManager::validateLibrary(NOT_PRESENT_LIBRARY));
+    EXPECT_FALSE(LibraryManager::validateLibrary(NO_VERSION_LIBRARY));
+    EXPECT_TRUE(LibraryManager::validateLibrary(UNLOAD_CALLOUT_LIBRARY));
+}
+
 } // Anonymous namespace