Commit ba91cff4 authored by Stephen Morris's avatar Stephen Morris
Browse files

[2980] Added pre-callout and post-callout function registration

parent d72ba709
......@@ -19,6 +19,7 @@
#include <boost/static_assert.hpp>
#include <algorithm>
#include <climits>
#include <functional>
#include <utility>
......@@ -27,7 +28,24 @@ using namespace std;
namespace isc {
namespace hooks {
// Set the number of libraries handles by the CalloutManager.
// Check that the index of a library is valid. It can range from 1 - n
// (n is the number of libraries) or it can be 0 (pre-user library callouts)
// of INT_MAX (post-user library callouts). It can also be -1 to indicate
// an invalid value.
void
CalloutManager::checkLibraryIndex(int library_index) const {
if (((library_index >= -1) && (library_index <= num_libraries_)) ||
(library_index == INT_MAX)) {
return;
} else {
isc_throw(NoSuchLibrary, "library index " << library_index <<
" is not valid for the number of loaded libraries (" <<
num_libraries_ << ")");
}
}
// Set the number of libraries handled by the CalloutManager.
void
CalloutManager::setNumLibraries(int num_libraries) {
......
......@@ -21,6 +21,7 @@
#include <boost/shared_ptr.hpp>
#include <climits>
#include <map>
#include <string>
......@@ -37,7 +38,6 @@ public:
isc::Exception(file, line, what) {}
};
/// @brief Callout Manager
///
/// This class manages the registration, deregistration and execution of the
......@@ -64,9 +64,47 @@ public:
/// functions use the "current library index" in their processing.
///
/// These two items of data are supplied when an object of this class is
/// constructed.
/// constructed. The latter (number of libraries) can be updated after the
/// class is constructed. (Such an update is used during library loading where
/// the CalloutManager has to be constructed before the libraries are loaded,
/// but one of the libraries subsequently fails to load.)
///
/// The library index is important because it determines in what order callouts
/// on a particular hook are called. For each hook, the CalloutManager
/// maintains a vector of callouts, ordered by library index. When a callout
/// is added to the list, it is added at the end of the callouts associated
/// with the current library. To clarify this further, suppose that three
/// libraries are loaded, A (assigned an index 1), B (assigned an index 2) and
/// C (assigned an index 3). Suppose A registers two callouts on a given hook,
/// A1 and A2 (in that order) B registers B1 and B2 (in that order) and C
/// registers C1 and C2 (in that order). Internally, the callouts are stored
/// in the order A1, A2, B1, B2, C1, and C2: this is also the order in which
/// the are called. If B now registers another callout (B3), it is added to
/// the vector after the list of callouts associated with B: the new order is
/// therefore A1, A2, B1, B2, B3, C1 and C2.
///
/// Indexes range between 1 and n (where n is the number of the libraries
/// loaded) and are assigned to libraries based on the order the libraries
/// presented to the hooks framework for loading (something that occurs in the
/// isc::util::HooksManager) class. However, two other indexes are recognised,
/// 0 and n+1. These are used when the server itself registers callouts - the
/// server is able to register callouts that get called before any user-library
/// callouts, and callouts that get called after user-library callouts. In other
/// words, assuming the callouts on a hook are A1, A2, B1, B2, B3, C2, C2 as
/// before, and that the server registers Spre (to run before the
/// user-registered callouts) and Spost (to run after them), the callouts are
/// stored (and executed) in the order Spre, A1, A2, B1, B2, B3, C2, C2, Spost.
/// In summary, the index values are:
///
/// Note that the callout function do not access the library manager: instead,
/// - < 0: invalid.
/// - 0: used for server-registered callouts that are called before
/// user-registered callouts.
/// - 1 - n: callouts from user-libraries.
/// - INT_MAX: used for server-registered callouts called after
/// user-registered callouts.
/// - > n + 1: invalid
///
/// Note that the callout functions do not access the CalloutManager: instead,
/// they use a LibraryHandle object. This contains an internal pointer to
/// the CalloutManager, but provides a restricted interface. In that way,
/// callouts are unable to affect callouts supplied by other libraries.
......@@ -99,7 +137,8 @@ public:
CalloutManager(int num_libraries = 0)
: current_hook_(-1), current_library_(-1),
hook_vector_(ServerHooks::getServerHooks().getCount()),
library_handle_(this), num_libraries_(num_libraries)
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
......@@ -191,9 +230,14 @@ public:
/// 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
......@@ -224,9 +268,14 @@ public:
/// @brief Set current library index
///
/// Sets the current library index. This must be in the range 0 to
/// (numlib - 1), where "numlib" is the number of libraries loaded in the
/// system, this figure being passed to this object at construction time.
/// Sets the current library index. This has the following valid values:
///
/// - -1: invalidate current index.
/// - 0: pre-user library callout.
/// - 1 - numlib: user-library callout (where "numlib" is the number of
/// libraries loaded in the system, this figure being passed to this
/// object at construction time).
/// - INT_MAX: post-user library callout.
///
/// @param library_index New library index.
///
......@@ -236,6 +285,19 @@ public:
current_library_ = library_index;
}
/// @defgroup calloutManagerLibraryHandles Callout manager library handles
///
/// The CalloutManager offers three library handles:
///
/// - a "standard" one, used to register and deregister callouts for
/// the library index that is marked as current in the CalloutManager.
/// When a callout is called, it is passed this one.
/// - a pre-library callout handle, used by the server to register
// callouts to run prior to user-library callouts.
/// - a post-library callout handle, used by the server to register
/// callouts to run after the user-library callouts.
//@{
/// @brief Return library handle
///
/// The library handle is available to the user callout via the callout
......@@ -244,11 +306,33 @@ public:
/// library of which it is part, whilst denying access to anything that
/// may affect other libraries.
///
/// @return Reference to callout handle for this manager
/// @return Reference to library handle for this manager
LibraryHandle& getLibraryHandle() {
return (library_handle_);
}
/// @brief Return pre-user callouts library handle
///
/// The LibraryHandle to affect callouts that will run before the
/// user-library callouts.
///
/// @return Reference to pre-user library handle for this manager
LibraryHandle& getPreLibraryHandle() {
return (pre_library_handle_);
}
/// @brief Return post-user callouts library handle
///
/// The LibraryHandle to affect callouts that will run before the
/// user-library callouts.
///
/// @return Reference to post-user library handle for this manager
LibraryHandle& getPostLibraryHandle() {
return (post_library_handle_);
}
//@}
private:
/// @brief Check library index
///
......@@ -256,15 +340,11 @@ private:
/// the hook registration functions.
///
/// @param library_index Value to check for validity as a library index.
/// Valid values are 0 - numlib+1 and -1: see @ref setLibraryIndex
/// for the meaning of the various values.
///
/// @throw NoSuchLibrary Library index is not valid.
void checkLibraryIndex(int library_index) const {
if ((library_index < 0) || (library_index >= num_libraries_)) {
isc_throw(NoSuchLibrary, "library index " << library_index <<
" is not valid for the number of loaded libraries (" <<
num_libraries_ << ")");
}
}
void checkLibraryIndex(int library_index) const;
/// @brief Compare two callout entries for library equality
///
......@@ -301,8 +381,18 @@ private:
std::vector<CalloutVector> hook_vector_;
/// LibraryHandle object user by the callout to access the callout
/// registration methods on this CalloutManager object.
/// registration methods on this CalloutManager object. The object is set
/// such that the index of the library associated with any operation is
/// whatever is currently set in the CalloutManager.
LibraryHandle library_handle_;
/// LibraryHandle for callouts to be registered as being called before
/// the user-registered callouts.
LibraryHandle pre_library_handle_;
/// LibraryHandle for callouts to be registered as being called after
/// the user-registered callouts.
LibraryHandle post_library_handle_;
/// Number of libraries.
int num_libraries_;
......
......@@ -145,5 +145,29 @@ HooksManager::registerHook(const std::string& name) {
return (ServerHooks::getServerHooks().registerHook(name));
}
// Return pre- and post- library handles.
isc::hooks::LibraryHandle&
HooksManager::preCalloutsLibraryHandleInternal() {
conditionallyInitialize();
return (callout_manager_->getPreLibraryHandle());
}
isc::hooks::LibraryHandle&
HooksManager::preCalloutsLibraryHandle() {
return (getHooksManager().preCalloutsLibraryHandleInternal());
}
isc::hooks::LibraryHandle&
HooksManager::postCalloutsLibraryHandleInternal() {
conditionallyInitialize();
return (callout_manager_->getPostLibraryHandle());
}
isc::hooks::LibraryHandle&
HooksManager::postCalloutsLibraryHandle() {
return (getHooksManager().postCalloutsLibraryHandleInternal());
}
} // namespace util
} // namespace isc
......@@ -116,16 +116,15 @@ public:
/// callouts on a hook that are called _before_ any callouts belonging
/// to a library.
///
/// @note This library handle is valid only after loadLibraries() is
/// called and before another call to loadLibraries(). Its use
/// at any other time may cause severe problems.
/// @note Both the reference returned and the callouts registered with
/// this handle only remain valid until the next loadLibraries() or
/// unloadLibraries() call. If the callouts are to remain registered
/// after this time, a new handle will need to be obtained and the
/// callouts re-registered.
///
/// TODO: This is also invalidated by a call to obtaining the
/// post-callout function.
///
/// @return Shared pointer to library handle associated with pre-library
/// callout registration.
boost::shared_ptr<LibraryHandle> preCalloutLibraryHandle() const;
/// @return Reference to library handle associated with pre-library callout
/// registration.
static LibraryHandle& preCalloutsLibraryHandle();
/// @brief Return post-callouts library handle
///
......@@ -133,16 +132,15 @@ public:
/// callouts on a hook that are called _after any callouts belonging
/// to a library.
///
/// @note This library handle is valid only after loadLibraries() is
/// called and before another call to loadLibraries(). Its use
/// at any other time may cause severe problems.
///
/// TODO: This is also invalidated by a call to obtaining the
/// pret-callout function.
/// @note Both the reference returned and the callouts registered with
/// this handle only remain valid until the next loadLibraries() or
/// unloadLibraries() call. If the callouts are to remain registered
/// after this time, a new handle will need to be obtained and the
/// callouts re-registered.
///
/// @return Shared pointer to library handle associated with post-library
/// callout registration.
boost::shared_ptr<LibraryHandle> postCalloutLibraryHandle() const;
/// @return Reference to library handle associated with post-library callout
/// registration.
static LibraryHandle& postCalloutsLibraryHandle();
/// @brief Return callout handle
///
......@@ -221,12 +219,21 @@ private:
/// @brief Return callout handle
///
/// @note This handle is valid only after a loadLibraries() call and then
/// only up to the next loadLibraries() call.
///
/// @return Shared pointer to a CalloutHandle object.
boost::shared_ptr<CalloutHandle> createCalloutHandleInternal();
/// @brief Return pre-callouts library handle
///
/// @return Reference to library handle associated with pre-library callout
/// registration.
LibraryHandle& preCalloutsLibraryHandleInternal();
/// @brief Return post-callouts library handle
///
/// @return Reference to library handle associated with post-library callout
/// registration.
LibraryHandle& postCalloutsLibraryHandleInternal();
//@}
/// @brief Initialization to No Libraries
......
......@@ -22,17 +22,54 @@ namespace hooks {
void
LibraryHandle::registerCallout(const std::string& name, CalloutPtr callout) {
// Reset library index if required, saving the current value.
int saved_index = callout_manager_->getLibraryIndex();
if (index_ >= 0) {
callout_manager_->setLibraryIndex(index_);
}
// Register the callout.
callout_manager_->registerCallout(name, callout);
// Restore the library index if required. We know that the saved index
// is valid for the number of libraries (or is -1, which is an internal
// state indicating there is no current library index) as we obtained it
// from the callout manager.
if (index_ >= 0) {
callout_manager_->setLibraryIndex(saved_index);
}
}
bool
LibraryHandle::deregisterCallout(const std::string& name, CalloutPtr callout) {
return (callout_manager_->deregisterCallout(name, callout));
int saved_index = callout_manager_->getLibraryIndex();
if (index_ >= 0) {
callout_manager_->setLibraryIndex(index_);
}
bool status = callout_manager_->deregisterCallout(name, callout);
if (index_ >= 0) {
callout_manager_->setLibraryIndex(saved_index);
}
return (status);
}
bool
LibraryHandle::deregisterAllCallouts(const std::string& name) {
return (callout_manager_->deregisterAllCallouts(name));
int saved_index = callout_manager_->getLibraryIndex();
if (index_ >= 0) {
callout_manager_->setLibraryIndex(index_);
}
bool status = callout_manager_->deregisterAllCallouts(name);
if (index_ >= 0) {
callout_manager_->setLibraryIndex(saved_index);
}
return (status);
}
} // namespace util
......
......@@ -58,7 +58,18 @@ public:
/// object. Note that the "raw" pointer is safe - the only
/// instance of the LibraryHandle in the system is as a member of
/// the CalloutManager to which it points.
LibraryHandle(CalloutManager* manager) : callout_manager_(manager)
///
/// @param index Index of the library to which the LibraryHandle applies.
/// If negative, the library index as set in the CalloutManager is
/// used. Otherwise the current library index is saved, this value
/// is set as the current index, the registration call is made, and
/// the CalloutManager's library index is reset. Note: although -1
/// is a valid argument value for
/// isc::hooks::CalloutManager::setLibraryIndex(), in this class is
/// is used as a sentinel to indicate that the library index in
/// isc::util::CalloutManager should not be set or reset.
LibraryHandle(CalloutManager* manager, int index = -1)
: callout_manager_(manager), index_(index)
{}
/// @brief Register a callout on a hook
......@@ -107,6 +118,10 @@ public:
private:
/// Back pointer to the collection object for the library
CalloutManager* callout_manager_;
/// Library index to which this handle applies. -1 indicates that it
/// applies to whatever index is current in the CalloutManager.
int index_;
};
} // namespace util
......
......@@ -39,7 +39,7 @@ class LibraryManager;
/// On unload, it calls the "unload" method if one was located, clears the
/// callouts from all hooks and closes the library.
///
/// @note Caution needs to be exercised whtn using the unload method. During
/// @note Caution needs to be exercised when using the unload method. During
/// use, data will pass between the server and the library. In this
/// process, the library may allocate memory and pass it back to the
/// server. This could happen by the server setting arguments or context
......
......@@ -22,6 +22,7 @@
#include <gtest/gtest.h>
#include <algorithm>
#include <climits>
#include <string>
#include <vector>
......@@ -207,14 +208,25 @@ TEST_F(CalloutManagerTest, NumberOfLibraries) {
// Check that we can only set the current library index to the correct values.
TEST_F(CalloutManagerTest, CheckLibraryIndex) {
// Check valid indexes
for (int i = 0; i < 4; ++i) {
// Check valid indexes. As the callout manager is sized for 10 libraries,
// we expect:
//
// -1 to be valid as it is the standard "invalid" value.
// 0 to be valid for the pre-user library callouts
// 1-10 to be valid for the user-library callouts
// INT_MAX to be valid for the post-user library callouts
//
// All other values to be invalid.
for (int i = -1; i < 11; ++i) {
EXPECT_NO_THROW(getCalloutManager()->setLibraryIndex(i));
EXPECT_EQ(i, getCalloutManager()->getLibraryIndex());
}
EXPECT_NO_THROW(getCalloutManager()->setLibraryIndex(INT_MAX));
EXPECT_EQ(INT_MAX, getCalloutManager()->getLibraryIndex());
// Check invalid ones
EXPECT_THROW(getCalloutManager()->setLibraryIndex(-1), NoSuchLibrary);
EXPECT_THROW(getCalloutManager()->setLibraryIndex(15), NoSuchLibrary);
EXPECT_THROW(getCalloutManager()->setLibraryIndex(-2), NoSuchLibrary);
EXPECT_THROW(getCalloutManager()->setLibraryIndex(11), NoSuchLibrary);
}
// Check that we can only register callouts on valid hook names.
......@@ -761,6 +773,112 @@ TEST_F(CalloutManagerTest, LibraryHandleRegistration) {
EXPECT_EQ(1, callout_value_);
}
// A repeat of the test above, but using the alternate constructor for the
// LibraryHandle.
TEST_F(CalloutManagerTest, LibraryHandleAlternateConstructor) {
// Ensure that no callouts are attached to any of the hooks.
EXPECT_FALSE(getCalloutManager()->calloutsPresent(alpha_index_));
// Set up so that hooks "alpha" and "beta" have callouts attached from a
// different libraries.
LibraryHandle lh0(getCalloutManager().get(), 0);
lh0.registerCallout("alpha", callout_one);
lh0.registerCallout("alpha", callout_two);
LibraryHandle lh1(getCalloutManager().get(), 1);
lh1.registerCallout("alpha", callout_three);
lh1.registerCallout("alpha", callout_four);
// Check all is as expected.
EXPECT_TRUE(getCalloutManager()->calloutsPresent(alpha_index_));
EXPECT_FALSE(getCalloutManager()->calloutsPresent(beta_index_));
EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
// Check that calling the callouts returns as expected. (This is also a
// test of the callCallouts method.)
callout_value_ = 0;
getCalloutManager()->callCallouts(alpha_index_, getCalloutHandle());
EXPECT_EQ(1234, callout_value_);
// Deregister a callout on library index 0 (after we check we can't
// deregister it through library index 1).
EXPECT_FALSE(lh1.deregisterCallout("alpha", callout_two));
callout_value_ = 0;
getCalloutManager()->callCallouts(alpha_index_, getCalloutHandle());
EXPECT_EQ(1234, callout_value_);
EXPECT_TRUE(lh0.deregisterCallout("alpha", callout_two));
callout_value_ = 0;
getCalloutManager()->callCallouts(alpha_index_, getCalloutHandle());
EXPECT_EQ(134, callout_value_);
// Deregister all callouts on library index 1.
EXPECT_TRUE(lh1.deregisterAllCallouts("alpha"));
callout_value_ = 0;
getCalloutManager()->callCallouts(alpha_index_, getCalloutHandle());
EXPECT_EQ(1, callout_value_);
}
// Check that the pre- and post- user callout library handles work
// appropriately with no user libraries.
TEST_F(CalloutManagerTest, LibraryHandlePrePostNoLibraries) {
// Create a local callout manager and callout handle to reflect no libraries
// being loaded.
boost::shared_ptr<CalloutManager> manager(new CalloutManager(0));
CalloutHandle handle(manager);
// Ensure that no callouts are attached to any of the hooks.
EXPECT_FALSE(manager->calloutsPresent(alpha_index_));
// Setup the pre-and post callouts.
manager->getPostLibraryHandle().registerCallout("alpha", callout_four);
manager->getPreLibraryHandle().registerCallout("alpha", callout_one);
// Check all is as expected.
EXPECT_TRUE(manager->calloutsPresent(alpha_index_));
EXPECT_FALSE(manager->calloutsPresent(beta_index_));
EXPECT_FALSE(manager->calloutsPresent(gamma_index_));
EXPECT_FALSE(manager->calloutsPresent(delta_index_));
// Check that calling the callouts returns as expected.
callout_value_ = 0;
manager->callCallouts(alpha_index_, handle);
EXPECT_EQ(14, callout_value_);
// Deregister the pre- library callout.
EXPECT_TRUE(manager->getPreLibraryHandle().deregisterAllCallouts("alpha"));
callout_value_ = 0;
manager->callCallouts(alpha_index_, handle);
EXPECT_EQ(4, callout_value_);
}
// Repeat the tests with one user library.
TEST_F(CalloutManagerTest, LibraryHandlePrePostUserLibrary) {
// Setup the pre-, library and post callouts.
getCalloutManager()->getPostLibraryHandle().registerCallout("alpha",
callout_four);
getCalloutManager()->getPreLibraryHandle().registerCallout("alpha",
callout_one);
// ... and set up a callout in between, on library number 2.
LibraryHandle lh1(getCalloutManager().get(), 2);
lh1.registerCallout("alpha", callout_five);
// Check all is as expected.
EXPECT_TRUE(getCalloutManager()->calloutsPresent(alpha_index_));
EXPECT_FALSE(getCalloutManager()->calloutsPresent(beta_index_));
EXPECT_FALSE(getCalloutManager()->calloutsPresent(gamma_index_));
EXPECT_FALSE(getCalloutManager()->calloutsPresent(delta_index_));
// Check that calling the callouts returns as expected.
callout_value_ = 0;
getCalloutManager()->callCallouts(alpha_index_, getCalloutHandle());
EXPECT_EQ(154, callout_value_);
}
// The setting of the hook index is checked in the handles_unittest
// set of tests, as access restrictions mean it is not easily tested
// on its own.
......
......@@ -332,6 +332,75 @@ TEST_F(HooksManagerTest, ReloadLibrariesReverseOrder) {
}
}
// Local callouts for the test of server-registered callouts.
namespace {
int
testPreCallout(CalloutHandle& handle) {
handle.setArgument("result", static_cast<int>(1027));
return (0);
}
int
testPostCallout(CalloutHandle& handle) {
int result;
handle.getArgument("result", result);
result *= 2;
handle.setArgument("result", result);
return (0);
}
}
// The next test registers the pre and post- callouts above for hook lm_two,
// and checks they are called.
TEST_F(HooksManagerTest, PrePostCalloutTest) {
// Load a single library.
std::vector<std::string> library_names;
library_names.push_back(std::string(FULL_CALLOUT_LIBRARY));
EXPECT_TRUE(HooksManager::loadLibraries(library_names));