[3156] Addressed review comments for b10-dhcp-ddns/NameChangeTransaction

Created new classes, LabeledValue and LabeledValueSet to
provide a cleaner mechanism for defining the set of valid events
and states.  With this commit, events now use these new constructs.
The modifications to use these constructs for states will be done
as separate commit.

Some addtional, minor review comments were also addressed.

src/bin/d2/Makefile.am

@@ -59,6 +59,7 @@ b10_dhcp_ddns_SOURCES += d2_update_message.cc d2_update_message.h
 b10_dhcp_ddns_SOURCES += d2_update_mgr.cc d2_update_mgr.h
 b10_dhcp_ddns_SOURCES += d2_zone.cc d2_zone.h
 b10_dhcp_ddns_SOURCES += dns_client.cc dns_client.h
+b10_dhcp_ddns_SOURCES += labeled_value.cc labeled_value.h
 b10_dhcp_ddns_SOURCES += nc_trans.cc nc_trans.h
 b10_dhcp_ddns_SOURCES += state_model.cc state_model.h
 

src/bin/d2/labeled_value.cc

@@ -0,0 +1,123 @@
+// 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 <d2/labeled_value.h>
+
+namespace isc {
+namespace d2 {
+
+/**************************** LabeledValue ****************************/
+
+LabeledValue::LabeledValue(const int value, const char* label)
+    : value_(value), label_(label) {
+    if (label == NULL || strlen(label) == 0) {
+        isc_throw(LabeledValueError, "labels cannot be empty");
+    }
+}
+
+LabeledValue::~LabeledValue(){
+}
+
+int
+LabeledValue::getValue() const {
+    return (value_);
+}
+
+const char*
+LabeledValue::getLabel() const {
+    return (label_);
+}
+
+bool
+LabeledValue::operator==(const LabeledValue& other) const {
+    return (this->value_ == other.value_);
+}
+
+bool
+LabeledValue::operator!=(const LabeledValue& other) const {
+    return (this->value_ != other.value_);
+}
+
+bool
+LabeledValue::operator<(const LabeledValue& other) const {
+    return (this->value_ < other.value_);
+}
+
+std::ostream& operator<<(std::ostream& os, const LabeledValue& vlp) {
+    os << vlp.getLabel();
+    return (os);
+}
+
+/**************************** LabeledValueSet ****************************/
+
+const char* LabeledValueSet::UNDEFINED_LABEL = "UNDEFINED";
+
+LabeledValueSet::LabeledValueSet(){
+}
+
+LabeledValueSet::~LabeledValueSet() {
+}
+
+void
+LabeledValueSet::add(LabeledValuePtr entry) {
+    if (!entry) {
+        isc_throw(LabeledValueError, "cannot add an null entry to set");
+    }
+
+    const int value = entry->getValue();
+    if (isDefined(value)) {
+        isc_throw(LabeledValueError,
+                  "value: " << value << " is already defined as: "
+                  << getLabel(value));
+        }
+
+    map_[entry->getValue()]=entry;
+}
+
+void
+LabeledValueSet::add(const int value, const char* label) {
+    add (LabeledValuePtr(new LabeledValue(value,label)));
+}
+
+const LabeledValuePtr&
+LabeledValueSet::get(int value) {
+    static LabeledValuePtr undefined;
+    LabeledValueMap::iterator it = map_.find(value);
+    if (it != map_.end()) {
+        return ((*it).second);
+    }
+
+    // Return an empty pointer when not found.
+    return (undefined);
+}
+
+bool
+LabeledValueSet::isDefined(const int value) const {
+    LabeledValueMap::const_iterator it = map_.find(value);
+    return (it != map_.end());
+}
+
+const char*
+LabeledValueSet::getLabel(const int value) const {
+    LabeledValueMap::const_iterator it = map_.find(value);
+    if (it != map_.end()) {
+        const LabeledValuePtr& ptr = (*it).second;
+        return (ptr->getLabel());
+    }
+
+    return (UNDEFINED_LABEL);
+}
+
+} // namespace isc::d2
+} // namespace isc

src/bin/d2/labeled_value.h

@@ -0,0 +1,185 @@
+// 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.
+
+#ifndef LABELED_VALUE_H
+#define LABELED_VALUE_H
+
+#include <exceptions/exceptions.h>
+
+#include <boost/shared_ptr.hpp>
+#include <ostream>
+#include <string>
+#include <map>
+
+/// @file labeled_value.h This file defines classes: LabeledValue and
+/// LabeledValueSet.
+
+namespace isc {
+namespace d2 {
+
+/// @brief Thrown if an error is encountered handling a LabeledValue.
+class LabeledValueError : public isc::Exception {
+public:
+    LabeledValueError(const char* file, size_t line,
+                               const char* what) :
+        isc::Exception(file, line, what) { };
+};
+
+/// @brief Implements the concept of a constant value with a text label.
+///
+/// This class implements an association between an constant integer value
+/// and a text label. It provides a single constructor, accessors for both
+/// the value and label, and boolean operators which treat the value as
+/// the "key" for comparisons.  This allows them to be assembled into
+/// dictionaries of unique values.  Note, that the labels are not required to
+/// be unique but in practice it makes little sense to for them to be
+/// otherwise.
+class LabeledValue {
+public:
+
+    /// @brief Constructor
+    ///
+    /// @param value the numeric constant value to be labeled.
+    /// @param label the text label to associate to this value.
+    ///
+    /// @throw LabeledValueError if label is null or empty.
+    LabeledValue(const int value, const char* label);
+
+    /// @brief Destructor.
+    ///
+    /// Destructor is virtual to permit derivations.
+    virtual ~LabeledValue();
+
+    /// @brief Gets the integer value of this instance.
+    ///
+    /// @return integer value of this instance.
+    int getValue() const;
+
+    /// @brief Gets the text label of this instance.
+    ///
+    /// @return The text label as const char*
+    const char* getLabel() const;
+
+    /// @brief  Equality operator
+    ///
+    /// @return True if a.value_ is equal to b.value_.
+    bool operator==(const LabeledValue& other) const;
+
+    /// @brief  Inequality operator
+    ///
+    /// @return True if a.value_ is not equal to b.value_.
+    bool operator!=(const LabeledValue& other) const;
+
+    /// @brief  Less-than operator
+    ///
+    /// @return True if a.value_ is less than b.value_.
+    bool operator<(const LabeledValue& other) const;
+
+private:
+    /// @brief The numeric value to label.
+    int value_;
+
+    /// @brief The text label for the value.
+    const char* label_;
+};
+
+/// @brief Dumps the label to ostream.
+std::ostream& operator<<(std::ostream& os, const LabeledValue& vlp);
+
+/// @brief Defines a shared pointer to a LabeledValue instance.
+typedef boost::shared_ptr<LabeledValue> LabeledValuePtr;
+
+/// @brief Defines a map of pointers to LabeledValues keyed by value.
+typedef std::map<unsigned int, LabeledValuePtr> LabeledValueMap;
+
+
+/// @brief Implements a set of unique LabeledValues.
+///
+/// This class is intended to function as a dictionary of numeric values
+/// and the labels associated with them.  It is essentially a thin wrapper
+/// around a std::map of LabeledValues, keyed by their values.  This is handy
+/// for defining a set of "valid" constants while conveniently associating a
+/// text label with each value.
+///
+/// This class offers two variants of an add method for adding entries to the
+/// set, and accessors for finding an entry or an entry's label by value.
+/// Note that the add methods may throw but all accessors are exception safe.
+/// It is up to the caller to determine when and if an undefined value is
+/// exception-worthy.
+///
+/// More interestingly, a derivation of this class can be used to "define"
+/// valid instances of derivations of LabeledValue.
+class LabeledValueSet {
+public:
+    /// @brief Defines a text label returned by when value is not found.
+    static const char* UNDEFINED_LABEL;
+
+    /// @brief Constructor
+    ///
+    /// Constructs an empty set.
+    LabeledValueSet();
+
+    /// @brief Destructor
+    ///
+    /// Destructor is virtual to permit derivations.
+    virtual ~LabeledValueSet();
+
+    /// @brief Adds the given entry to the set
+    ///
+    /// @param entry is the entry to add.
+    ///
+    /// @throw LabeledValuePtr if the entry is null or the set already
+    /// contains an entry with the same value.
+    void add(LabeledValuePtr entry);
+
+    /// @brief Adds an entry to the set for the given value and label
+    ///
+    /// @param value the numeric constant value to be labeled.
+    /// @param label the text label to associate to this value.
+    ///
+    /// @throw LabeledValuePtr if the label is null or empty, or if the set
+    /// already contains an entry with the same value.
+    void add(const int value, const char* label);
+
+    /// @brief Fetches a pointer to the entry associated with value
+    ///
+    /// @param value is the value of the entry desired.
+    ///
+    /// @return A pointer to the entry if the entry was found otherwise the
+    /// pointer is empty.
+    const LabeledValuePtr& get(int value);
+
+    /// @brief Tests if the set contains an entry for the given value.
+    ///
+    /// @param value is the value of the entry to test.
+    ///
+    /// @return True if an entry for value exists in the set, false if not.
+    bool isDefined(const int value) const;
+
+    /// @brief Fetches the label for the given value
+    ///
+    /// @param value is the value for which the label is desired.
+    ///
+    /// @return the label of the value if defined, otherwise it returns
+    /// UNDEFINED_LABEL.
+    const char* getLabel(const int value) const;
+
+private:
+    /// @brief The map of labeled values.
+    LabeledValueMap map_;
+};
+
+} // namespace isc::d2
+} // namespace isc
+#endif

src/bin/d2/nc_trans.cc

@@ -81,6 +81,29 @@ NameChangeTransaction::operator()(DNSClient::Status status) {
     runModel(IO_COMPLETED_EVT);
 }
 
+void
+NameChangeTransaction::defineEvents() {
+    StateModel::defineEvents();
+    defineEvent(SELECT_SERVER_EVT, "SELECT_SERVER_EVT");
+    defineEvent(SERVER_SELECTED_EVT, "SERVER_SELECTED_EVT");
+    defineEvent(SERVER_IO_ERROR_EVT, "SERVER_IO_ERROR_EVT");
+    defineEvent(NO_MORE_SERVERS_EVT, "NO_MORE_SERVERS_EVT");
+    defineEvent(IO_COMPLETED_EVT, "IO_COMPLETED_EVT");
+    defineEvent(UPDATE_OK_EVT, "UPDATE_OK_EVT");
+    defineEvent(UPDATE_FAILED_EVT, "UPDATE_FAILED_EVT");
+}
+
+void
+NameChangeTransaction::verifyEvents() {
+    StateModel::verifyEvents();
+    getEvent(SELECT_SERVER_EVT);
+    getEvent(SERVER_SELECTED_EVT);
+    getEvent(SERVER_IO_ERROR_EVT);
+    getEvent(NO_MORE_SERVERS_EVT);
+    getEvent(IO_COMPLETED_EVT);
+    getEvent(UPDATE_OK_EVT);
+    getEvent(UPDATE_FAILED_EVT);
+}
 
 void
 NameChangeTransaction::verifyStateHandlerMap() {
@@ -230,39 +253,5 @@ NameChangeTransaction::getStateLabel(const int state) const {
     return (str);
 }
 
-const char*
-NameChangeTransaction::getEventLabel(const int event) const {
-    const char* str = "Unknown";
-    switch(event) {
-    case SELECT_SERVER_EVT:
-        str = "NameChangeTransaction::SELECT_SERVER_EVT";
-        break;
-    case SERVER_SELECTED_EVT:
-        str = "NameChangeTransaction::SERVER_SELECTED_EVT";
-        break;
-    case SERVER_IO_ERROR_EVT:
-        str = "NameChangeTransaction::SERVER_IO_ERROR_EVT";
-        break;
-    case NO_MORE_SERVERS_EVT:
-        str = "NameChangeTransaction::NO_MORE_SERVERS_EVT";
-        break;
-    case IO_COMPLETED_EVT:
-        str = "NameChangeTransaction::IO_COMPLETED_EVT";
-        break;
-    case UPDATE_OK_EVT:
-        str = "NameChangeTransaction::UPDATE_OK_EVT";
-        break;
-    case UPDATE_FAILED_EVT:
-        str = "NameChangeTransaction::UPDATE_FAILED_EVT";
-        break;
-    default:
-        str = StateModel::getEventLabel(event);
-        break;
-    }
-
-    return (str);
-}
-
-
 } // namespace isc::d2
 } // namespace isc

src/bin/d2/nc_trans.h

@@ -182,6 +182,28 @@ public:
     virtual void operator()(DNSClient::Status status);
 
 protected:
+    /// @brief Adds events defined by NameChangeTransaction to the event set. 
+    ///
+    /// This method adds the events common to NCR transaction processing to
+    /// the set of define events.  It invokes the superclass's implementation
+    /// first to maitain the hierarchical chain of event defintion.
+    /// Derivations of NameChangeTransaction must invoke its implementation 
+    /// in like fashion.
+    ///
+    /// @throw StateModelError if an event definition is invalid or a duplicate.
+    virtual void defineEvents();
+
+    /// @brief Validates the contents of the set of events.
+    ///
+    /// This method verifies that the events defined by both the superclass and
+    /// this class are defined.  As with defineEvents, this method calls the
+    /// superclass's implementation first, to verify events defined by it and
+    /// then this implementation to verify events defined by 
+    /// NameChangeTransaction.
+    ///
+    /// @throw StateModelError if an event value is undefined. 
+    virtual void verifyEvents();
+
     /// @brief Populates the map of state handlers.
     ///
     /// This method is used by derivations to construct a map of states to
@@ -404,43 +426,6 @@ public:
     /// "Unknown" if the value cannot be mapped.
     virtual const char* getStateLabel(const int state) const;
 
-    /// @brief Converts a event value into a text label.
-    ///
-    /// This method supplies labels for NameChangeTransactions's predefined
-    /// events. It is declared virtual to allow derivations to embed a call to
-    /// this method within their own implementation which would define labels
-    /// for their events.  An example implementation might look like the
-    /// following:
-    /// @code
-    ///
-    /// class DerivedTrans : public NameChangeTransaction {
-    ///     :
-    ///     static const int EXAMPLE_1_EVT = NCT_EVENT_MAX + 1;
-    ///     :
-    ///     const char* getEventLabel(const int event) const {
-    ///         const char* str = "Unknown";
-    ///         switch(event) {
-    ///         case EXAMPLE_1_EVT:
-    ///             str = "DerivedTrans::EXAMPLE_1_EVT";
-    ///             break;
-    ///         :
-    ///         default:
-    ///             // Not a derived event, pass it to NameChangeTransaction's
-    ///             // method.
-    ///             str = StateModel::getEventLabel(event);
-    ///             break;
-    ///         }
-    ///
-    ///         return (str);
-    ///      }
-    ///
-    /// @endcode
-    /// @param event is the event for which a label is desired.
-    ///
-    /// @return Returns a const char* containing the event label or
-    /// "Unknown" if the value cannot be mapped.
-    virtual const char* getEventLabel(const int state) const;
-
 private:
     /// @brief The IOService which should be used to for IO processing.
     isc::asiolink::IOService& io_service_;

src/bin/d2/state_model.cc

@@ -45,6 +45,13 @@ StateModel::~StateModel(){
 
 void
 StateModel::startModel(const int start_state) {
+    try {
+        defineEvents();
+        verifyEvents();
+    } catch (const std::exception& ex) {
+        isc_throw(StateModelError, "Event set is invalid: " << ex.what());
+    }
+
     // Initialize the state handler map first.
     initStateHandlerMap();
 
@@ -83,6 +90,32 @@ StateModel::runModel(unsigned int run_event) {
     }
 }
 
+void
+StateModel::defineEvent(unsigned int event_value, const char* label) {
+    if (!isModelNew()) {
+        // Don't allow for self-modifying maps.
+        isc_throw(StateModelError, "Events may only be added to a new model."
+                   << event_value << " - " << label);
+    }
+
+    // add method may throw on duplicate or empty label.  
+    try {
+        events_.add(event_value, label);
+    } catch (const std::exception& ex) {
+        isc_throw(StateModelError, "Error adding event: " << ex.what());
+    }
+}
+
+const EventPtr&
+StateModel::getEvent(unsigned int event_value) {
+    if (!events_.isDefined(event_value)) {
+        isc_throw(StateModelError, 
+                  "Event value is not defined:" << event_value);
+    }
+
+    return (events_.get(event_value));
+}
+
 StateHandler
 StateModel::getStateHandler(unsigned int state) {
     StateHandlerMap::iterator it = state_handlers_.find(state);
@@ -96,7 +129,14 @@ StateModel::getStateHandler(unsigned int state) {
 }
 
 void
-StateModel::addToMap(unsigned int state, StateHandler handler) {
+StateModel::addToStateHandlerMap(unsigned int state, StateHandler handler) {
+    if (!isModelNew()) {
+        // Don't allow for self-modifying maps.
+        isc_throw(StateModelError,
+                  "State handler mappings can only be added to a new model."
+                   << state << " - " << getStateLabel(state));
+    }
+
     StateHandlerMap::iterator it = state_handlers_.find(state);
     // Check for a duplicate attempt.  State's can't have more than one.
     if (it != state_handlers_.end()) {
@@ -114,6 +154,23 @@ StateModel::addToMap(unsigned int state, StateHandler handler) {
     state_handlers_[state] = handler;
 }
 
+void 
+StateModel::defineEvents() {
+    defineEvent(NOP_EVT, "NOP_EVT");
+    defineEvent(START_EVT, "START_EVT");
+    defineEvent(END_EVT, "END_EVT");
+    defineEvent(FAIL_EVT, "FAIL_EVT");
+}
+
+void
+StateModel::verifyEvents() {
+    getEvent(NOP_EVT);
+    getEvent(START_EVT);
+    getEvent(END_EVT);
+    getEvent(FAIL_EVT);
+}
+
+
 void
 StateModel::transition(unsigned int state, unsigned int event) {
     setState(state);
@@ -150,9 +207,14 @@ StateModel::setState(unsigned int state) {
 }
 
 void
-StateModel::postNextEvent(unsigned int event) {
+StateModel::postNextEvent(unsigned int event_value) {
+    if (event_value != FAIL_EVT && !events_.isDefined(event_value)) {
+        isc_throw(StateModelError, 
+                  "Attempt to post an undefined event, value: " << event_value);
+    }
+
     last_event_ = next_event_;
-    next_event_ = event;
+    next_event_ = event_value;
 }
 
 bool
@@ -252,25 +314,7 @@ StateModel::getPrevContextStr() const {
 
 const char*
 StateModel::getEventLabel(const int event) const {
-    const char* str = "Unknown";
-    switch(event) {
-    case NOP_EVT:
-        str = "StateModel::NOP_EVT";
-        break;
-    case START_EVT:
-        str = "StateModel::START_EVT";
-        break;
-    case END_EVT:
-        str = "StateModel::END_EVT";
-        break;
-    case FAIL_EVT:
-        str = "StateModel::FAIL_EVT";
-        break;
-    default:
-        break;
-    }
-
-    return (str);
+    return (events_.getLabel(event));
 }
 
 } // namespace isc::d2

src/bin/d2/state_model.h

@@ -15,12 +15,13 @@
 #ifndef STATE_MODEL_H
 #define STATE_MODEL_H
 
-/// @file nc_trans.h This file defines the class StateModel.
+/// @file state_model.h This file defines the class StateModel.
 
 #include <asiolink/io_service.h>
 #include <exceptions/exceptions.h>
 #include <d2/d2_config.h>
 #include <d2/dns_client.h>
+#include <d2/labeled_value.h>
 #include <dhcp_ddns/ncr_msg.h>
 
 #include <boost/shared_ptr.hpp>
@@ -38,6 +39,12 @@ public:
         isc::Exception(file, line, what) { };
 };
 
+/// @brief Define an Event. 
+typedef LabeledValue Event;
+
+/// @brief Define Event pointer.
+typedef LabeledValuePtr EventPtr;
+
 /// @brief Defines a pointer to an instance method for handling a state.
 typedef boost::function<void()> StateHandler;
 
@@ -50,8 +57,8 @@ typedef std::map<unsigned int, StateHandler> StateHandlerMap;
 /// of a basic finite state machine.
 ///
 /// The state model implementation used is a very basic approach. States
-/// and events are simple integer constants. Each state must have a state
-/// handler. State handlers are void methods which require no parameters.
+/// and events described by simple integer constants. Each state must have a 
+/// state handler. State handlers are void methods which require no parameters.
 /// Each model instance contains a map of states to instance method pointers
 /// to their respective state handlers.  The model tracks the following
 /// context values:
@@ -92,7 +99,7 @@ typedef std::map<unsigned int, StateHandler> StateHandlerMap;
 ///
 /// Derivations add their own states and events appropriate for their state
 /// model.  Note that NEW_ST and END_ST do not support handlers.  No work can
-/// done (events consumed) prior to starting the model nor can work be done
+/// be done (events consumed) prior to starting the model nor can work be done
 /// once the model has ended.
 ///
 /// Model execution consists of iteratively invoking the state handler
@@ -103,11 +110,11 @@ typedef std::map<unsigned int, StateHandler> StateHandlerMap;
 /// the former, the loop may be re-entered upon arrival of the external event.
 ///
 /// This loop is implemented in the runModel method.  This method accepts an
-/// event as argument which is "posts" as the next event.  It then retrieves the
+/// event as argument which it "posts" as the next event.  It then retrieves the
 /// handler for the current state from the handler map and invokes it. runModel
-/// repeats this process until it either NOP_EVT or END_EVT events are posted.
-/// In other words each invocation of runModel causes the model to be traversed
-/// from the current state until it must wait or ends .
+/// repeats this process until it either a NOP_EVT posts or the state changes
+/// to END_ST.  In other words each invocation of runModel causes the model to
+/// be traversed from the current state until it must wait or ends.
 ///
 /// Re-entering the "loop" upon the occurrence of an external event is done by
 /// invoking runModel with the appropriate event.  As before, runModel will
@@ -133,11 +140,11 @@ typedef std::map<unsigned int, StateHandler> StateHandlerMap;
 /// "done" and "failed".  There are several boolean status methods which may
 /// be used to check these conditions.
 ///
-/// To progress from from one state to the another, state handlers invoke use
-/// the method, transition.  This method accepts a state and an a event as
+/// To progress from one state to the another, state handlers invoke use
+/// the method, transition.  This method accepts a state and an event as
 /// parameters.  These values become the current state and the next event
 /// respectively.  This has the effect of entering the given state having posted
-/// the given event.  The postEvent method is may be used to post a new event
+/// the given event.  The postEvent method may be used to post a new event
 /// to the current state.
 ///
 /// Bringing the model to a normal end is done by invoking the endModel method
@@ -218,21 +225,88 @@ public:
     /// @brief Conducts a normal transition to the end of the model.
     ///
     /// This method posts an END_EVT and sets the current state to END_ST.
-    /// It is should be called by any state handler in the model from which
+    /// It should be called by any state handler in the model from which
     /// an exit leads to the model end.  In other words, if the transition
     /// out of a particular state is to the end of the model, that state's
     /// handler should call endModel.
     void endModel();
 
 protected:
+    /// @brief Populates the set of events. 
+    ///
+    /// This method is used to construct the set of valid events. Each class
+    /// within a StateModel derivation heirarchy uses this method to add any
+    /// events it defines to the set.  Each derivation's implementation must
+    /// also call it's superclass's implementation.  This allows each class 
+    /// within the heirarchy to make contributions to the set of defined 
+    /// events. Implementations use the method, defineEvent(), to add event 
+    /// definitions.  An example of the derivation's implementation follows:
+    ///
+    /// @code
+    /// void StateModelDerivation::defineEvents() {
+    ///     // Call the superclass implementation.
+    ///     StateModelDerivation::defineEvents();
+    ///
+    ///     // Add the events defined by the derivation. 
+    ///     defineEvent(SOME_CUSTOM_EVT_1, "CUSTOM_EVT_1");
+    ///     defineEvent(SOME_CUSTOM_EVT_2, "CUSTOM_EVT_2");
+    ///     :
+    /// }
+    /// @endcode
+    virtual void defineEvents();
+
+    /// @brief Adds an event value and associated label to the set of events.
+    ///
+    /// @param value is the numeric value of the event
+    /// @param label is the text label of the event used in log messages and
+    /// exceptions.
+    ///
+    /// @throw StateModelError if the model has already been started, if
+    /// the value is already defined, or if the label is null or empty.
+    void defineEvent(unsigned int value, const char* label);
+
+    /// @brief Fetches the event referred to by value.
+    ///
+    /// @param value is the numeric value of the event desired.
+    /// 
+    /// @return returns a constant pointer reference to the event if found
+    ///
+    /// @throw StateModelError if the event is not defined.
+    const EventPtr& getEvent(unsigned int value);
+
+    /// @brief Validates the contents of the set of events.
+    ///
+    /// This method is invoked immediately after the defineEvents method and 
+    /// is used to verify that all the requred events are defined.  If the 
+    /// event set is determined to be invalid this method should throw a 
+    /// StateModelError.  As with the defineEvents method, each class within
+    /// the a StateModel derivation heirarchy must supply an implementation
+    /// which calls it's superclass's implemenation as well as verifying any
+    /// events added by the derivation.  Validating an event is accomplished
+    /// by simply attempting to fetch en event by its value from the the 
+    /// event set.  An example of the derivation's implementation follows:
+    ///
+    /// @code
+    /// void StateModelDerivation::verifyEvents() {
+    ///     // Call the superclass implementation.
+    ///     StateModelDerivation::verifyEvents();
+    ///
+    ///     // Verify the events defined by the derivation. 
+    ///     events_.get(SOME_CUSTOM_EVT_1, "CUSTOM_EVT_1");
+    ///     events_.get(SOME_CUSTOM_EVT_2, "CUSTOM_EVT_2");
+    ///     :
+    /// }
+    /// @endcode
+    virtual void verifyEvents();
+
     /// @brief Populates the map of state handlers.
     ///
     /// This method is used by derivations to construct a map of states to
     /// their appropriate state handlers (bound method pointers).  It is
     /// invoked at the beginning of startModel().
     ///
-    /// Implementations should use the addToMap() method add entries to
-    /// the map.
+    /// Implementations should use the addToStateHandlerMap() method add
+    /// entries to the map.
     virtual void initStateHandlerMap() = 0;
 
     /// @brief Validates the contents of the state handler map.
@@ -243,7 +317,7 @@ protected:
     /// should throw a StateModelError.
     ///
     /// The simplest implementation would include a call to getStateHandler,
-    /// for each state the derivation supports.  For example, a implementation
+    /// for each state the derivation supports.  For example, an implementation
     /// which included three states, READY_ST, DO_WORK_ST, and DONE_ST could
     /// implement this function as follows:
     ///
@@ -263,7 +337,7 @@ protected:
     /// the given handler to the given state.  The state handler must be
     /// a bound member pointer to a handler method of derivation instance.
     /// The following code snippet shows an example derivation and call to
-    /// addToMap() within its initStateHandlerMap() method.
+    /// addToStateHandlerMap() within its initStateHandlerMap() method.
     ///
     /// @code
     /// class ExampleModel : public StateModel {
@@ -273,7 +347,7 @@ protected:
     /// }
     ///
     /// void initStateHandlerMap() {
-    ///     addToMap(READY_ST,
+    ///     addToStateHandlerMap(READY_ST,
     ///        boost::bind(&ExampleModel::readyHandler, this));
     ///     :
     ///
@@ -283,8 +357,8 @@ protected:
     /// @param handler the bound method pointer to the handler for the state
     ///
     /// @throw StateModelError if the map already contains an entry
-    /// for the given state.
-    void addToMap(unsigned int state, StateHandler handler);
+    /// for the given state, or if the model is beyond the NEW_ST.
+    void addToStateHandlerMap(unsigned int state, StateHandler handler);
 
     /// @brief Handler for fatal model execution errors.
     ///
@@ -349,9 +423,9 @@ protected:
     /// event that will be passed into the current state's handler on the next
     /// iteration of the run loop.
     ///
-    /// @param event the new value to assign to the next event.
+    /// @param the numeric event value to post as the next event.
     ///
-    /// @todo Currently there is no mechanism for validating events.
+    /// @throw StateModelError if the event is undefined 
     void postNextEvent(unsigned int event);
 
     /// @brief Checks if on entry flag is true.
@@ -469,40 +543,13 @@ public:
     /// "Unknown" if the value cannot be mapped.
     virtual const char* getStateLabel(const int state) const;
 
-    /// @brief Converts a event value into a text label.
-    ///
-    /// This method supplies labels for StateModel's predefined events. It is
-    /// declared virtual to allow derivations to embed a call to this method
-    /// within their own implementation which would define labels for its
-    /// events.  An example implementation might look like the following:
-    /// @code
-    ///
-    /// class DerivedModel : public StateModel {
-    ///     :
-    ///     static const int EXAMPLE_1_EVT = SM_EVENT_MAX + 1;
-    ///     :
-    ///     const char* getEventLabel(const int event) const {
-    ///         const char* str = "Unknown";
-    ///         switch(event) {
-    ///         case EXAMPLE_1_EVT:
-    ///             str = "DerivedModel::EXAMPLE_1_EVT";
-    ///             break;
-    ///         :
-    ///         default:
-    ///             // Not a derived event, pass it down to StateModel's method.
-    ///             str = StateModel::getEventLabel(event);
-    ///             break;
-    ///         }
+    /// @brief Fetches the label associated with an event value.
     ///
-    ///         return (str);
-    ///      }
-    ///
-    /// @endcode
-    /// @param event is the event for which a label is desired.
+    /// @param event is the numeric event value for which the label is desired.
     ///
     /// @return Returns a const char* containing the event label or
-    /// "Unknown" if the value cannot be mapped.
-    virtual const char* getEventLabel(const int state) const;
+    /// LabeledValueSet::UNDEFINED_LABEL if the value is undefined.
+    virtual const char* getEventLabel(const int event) const;
 
     /// @brief Convenience method which returns a string rendition of the
     /// current state and next event.
@@ -525,6 +572,9 @@ public:
     std::string getPrevContextStr() const;
 
 private:
+    /// @brief Contains the set of defined Events.
+    LabeledValueSet events_;
+
     /// @brief Contains a map of states to their state handlers.
     StateHandlerMap state_handlers_;
 

src/bin/d2/tests/Makefile.am

@@ -64,6 +64,7 @@ d2_unittests_SOURCES += ../d2_update_message.cc ../d2_update_message.h
 d2_unittests_SOURCES += ../d2_update_mgr.cc ../d2_update_mgr.h
 d2_unittests_SOURCES += ../d2_zone.cc ../d2_zone.h
 d2_unittests_SOURCES += ../dns_client.cc ../dns_client.h
+d2_unittests_SOURCES += ../labeled_value.cc ../labeled_value.h
 d2_unittests_SOURCES += ../nc_trans.cc ../nc_trans.h
 d2_unittests_SOURCES += ../state_model.cc ../state_model.h
 d2_unittests_SOURCES += d_test_stubs.cc d_test_stubs.h
@@ -78,6 +79,7 @@ d2_unittests_SOURCES += d2_update_message_unittests.cc
 d2_unittests_SOURCES += d2_update_mgr_unittests.cc
 d2_unittests_SOURCES += d2_zone_unittests.cc
 d2_unittests_SOURCES += dns_client_unittests.cc
+d2_unittests_SOURCES += labeled_value_unittests.cc
 d2_unittests_SOURCES += nc_trans_unittests.cc
 d2_unittests_SOURCES += state_model_unittests.cc
 nodist_d2_unittests_SOURCES = ../d2_messages.h ../d2_messages.cc

src/bin/d2/tests/labeled_value_unittests.cc

@@ -0,0 +1,110 @@
+// 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 <d2/labeled_value.h>
+
+#include <gtest/gtest.h>
+
+using namespace std;
+using namespace isc;
+using namespace isc::d2;
+
+namespace {
+
+/// @brief Verifies basic construction and accessors for LabeledValue.
+TEST(LabeledValue, construction) {
+    /// Verify that a null label is not allowed.
+    ASSERT_THROW(LabeledValue(1, NULL), LabeledValueError);
+
+    /// Verify that an empty label is not allowed.
+    ASSERT_THROW(LabeledValue(1, ""), LabeledValueError);
+
+    /// Verify that a valid constructor works.
+    LabeledValuePtr lvp;
+    ASSERT_NO_THROW(lvp.reset(new LabeledValue(1, "NotBlank")));
+    ASSERT_TRUE(lvp);
+
+    // Verify that the value can be accessed.
+    EXPECT_EQ(1, lvp->getValue());
+
+    // Verify that the label can be accessed.
+    EXPECT_EQ("NotBlank", lvp->getLabel());
+}
+
+/// @brief Verifies the logical operators defined for LabeledValue.
+TEST(LabeledValue, operators) {
+    LabeledValuePtr lvp1;
+    LabeledValuePtr lvp1Also;
+    LabeledValuePtr lvp2;
+
+    // Create three instances, two of which have the same numeric value.
+    ASSERT_NO_THROW(lvp1.reset(new LabeledValue(1, "One")));
+    ASSERT_NO_THROW(lvp1Also.reset(new LabeledValue(1, "OneAlso")));
+    ASSERT_NO_THROW(lvp2.reset(new LabeledValue(2, "Two")));
+
+    // Verify each of the operators.
+    EXPECT_TRUE(*lvp1 == *lvp1Also);
+    EXPECT_TRUE(*lvp1 != *lvp2);
+    EXPECT_TRUE(*lvp1 < *lvp2);
+    EXPECT_FALSE(*lvp2 < *lvp1);
+}
+
+/// @brief Verifies the default constructor for LabeledValueSet.
+TEST(LabeledValueSet, construction) {
+    ASSERT_NO_THROW (LabeledValueSet());
+}
+
+/// @brief Verifies the basic operations of a LabeledValueSet.
+/// Essentially we verify that we can define a set of valid entries and
+/// look them up without issue.
+TEST(LabeledValueSet, basicOperation) {
+    const char* labels[] = {"Zero", "One", "Two", "Three" };
+    LabeledValueSet lvset;
+    LabeledValuePtr lvp;
+
+    // Verify the we cannot add an empty pointer to the set.
+    EXPECT_THROW(lvset.add(lvp), LabeledValueError);
+
+    // Verify that we can add an entry to the set via pointer.
+    ASSERT_NO_THROW(lvp.reset(new LabeledValue(0, labels[0])));
+    EXPECT_NO_THROW(lvset.add(lvp));
+
+    // Verify that we cannot add a duplicate entry.
+    EXPECT_THROW(lvset.add(lvp), LabeledValueError);
+
+    // Add the remaining entries using add(int,char*) variant.
+    for (int i = 1; i < 3; i++) {
+        EXPECT_NO_THROW(lvset.add(i, labels[i]));
+    }
+
+    // Verify that we can't add a duplicate entry this way either.
+    EXPECT_THROW ((lvset.add(0, labels[0])), LabeledValueError);
+
+    // Verify that we can look up all of the defined entries properly.
+    for (int i = 1; i < 3; i++) {
+        EXPECT_TRUE(lvset.isDefined(i));
+        EXPECT_NO_THROW(lvp = lvset.get(i));
+        EXPECT_EQ(lvp->getValue(), i);
+        EXPECT_EQ(lvp->getLabel(), labels[i]);
+        EXPECT_EQ(lvset.getLabel(i), labels[i]);
+    }
+
+    // Verify behavior for a value that is not defined.
+    EXPECT_FALSE(lvset.isDefined(4));
+    EXPECT_NO_THROW(lvp = lvset.get(4));
+    EXPECT_FALSE(lvp);
+    EXPECT_EQ(lvset.getLabel(4), LabeledValueSet::UNDEFINED_LABEL);
+}
+
+}

src/bin/d2/tests/nc_trans_unittests.cc

@@ -131,25 +131,38 @@ public:
         }
     }
 
+    virtual void defineEvents() {
+        NameChangeTransaction::defineEvents();
+        defineEvent(SEND_UPDATE_EVT, "SEND_UPDATE_EVT");
+    }
+
+    virtual void verifyEvents() {
+        NameChangeTransaction::verifyEvents();
+        getEvent(SEND_UPDATE_EVT);
+    }
+
     /// @brief Initializes the state handler map.
     void initStateHandlerMap() {
-        addToMap(READY_ST,
-            boost::bind(&NameChangeStub::readyHandler, this));
+        addToStateHandlerMap(READY_ST,
+                             boost::bind(&NameChangeStub::readyHandler, this));
 
-        addToMap(SELECTING_FWD_SERVER_ST,
-            boost::bind(&NameChangeStub::dummyHandler, this));
+        addToStateHandlerMap(SELECTING_FWD_SERVER_ST,
+                             boost::bind(&NameChangeStub::dummyHandler, this));
 
-        addToMap(SELECTING_REV_SERVER_ST,
-            boost::bind(&NameChangeStub::dummyHandler, this));
+        addToStateHandlerMap(SELECTING_REV_SERVER_ST,
+                             boost::bind(&NameChangeStub::dummyHandler, this));
 
-        addToMap(DOING_UPDATE_ST,
-            boost::bind(&NameChangeStub::doingUpdateHandler, this));
+        addToStateHandlerMap(DOING_UPDATE_ST,
+                             boost::bind(&NameChangeStub::doingUpdateHandler,
+                                         this));
 
-        addToMap(PROCESS_TRANS_OK_ST,
-            boost::bind(&NameChangeStub::processTransDoneHandler, this));
+        addToStateHandlerMap(PROCESS_TRANS_OK_ST,
+                             boost::bind(&NameChangeStub::
+                                         processTransDoneHandler, this));
 
-        addToMap(PROCESS_TRANS_FAILED_ST,
-            boost::bind(&NameChangeStub::processTransDoneHandler, this));
+        addToStateHandlerMap(PROCESS_TRANS_FAILED_ST,
+                             boost::bind(&NameChangeStub::
+                                         processTransDoneHandler, this));
     }
 
     void verifyStateHandlerMap() {
@@ -173,22 +186,6 @@ public:
         return (str);
     }
 
-    const char* getEventLabel(const int event) const {
-        const char* str = "Unknown";
-        switch(event) {
-        case NameChangeStub::SEND_UPDATE_EVT:
-            str = "NameChangeStub::SEND_UPDATE_EVT";
-            break;
-        default:
-            str = NameChangeTransaction::getEventLabel(event);
-            break;
-        }
-
-        return (str);
-    }
-
-
-
     // Expose the protected methods to be tested.
     using StateModel::runModel;
     using StateModel::getStateHandler;
@@ -554,45 +551,50 @@ TEST_F(NameChangeTransactionTest, eventLabels) {
     NameChangeStubPtr name_change;
     ASSERT_NO_THROW(name_change = makeCannedTransaction());
 
+    // Manually invoke event definition.
+    ASSERT_NO_THROW(name_change->defineEvents());
+    ASSERT_NO_THROW(name_change->verifyEvents());
+
     // Verify StateModel labels.
-    EXPECT_EQ("StateModel::NOP_EVT",
-              name_change->getEventLabel(StateModel::NOP_EVT));
-    EXPECT_EQ("StateModel::START_EVT",
-              name_change->getEventLabel(StateModel::START_EVT));
-    EXPECT_EQ("StateModel::END_EVT",
-              name_change->getEventLabel(StateModel::END_EVT));
-    EXPECT_EQ("StateModel::FAIL_EVT",
-              name_change->getEventLabel(StateModel::FAIL_EVT));
+    EXPECT_EQ("NOP_EVT",
+              std::string(name_change->getEventLabel(StateModel::NOP_EVT)));
+    EXPECT_EQ("START_EVT",
+              std::string(name_change->getEventLabel(StateModel::START_EVT)));
+    EXPECT_EQ("END_EVT",
+              std::string(name_change->getEventLabel(StateModel::END_EVT)));
+    EXPECT_EQ("FAIL_EVT",
+              std::string(name_change->getEventLabel(StateModel::FAIL_EVT)));
 
     // Verify NameChangeTransactionLabels
-    EXPECT_EQ("NameChangeTransaction::SELECT_SERVER_EVT",
-              name_change->getEventLabel(NameChangeTransaction::
-                                         SELECT_SERVER_EVT));
-    EXPECT_EQ("NameChangeTransaction::SERVER_SELECTED_EVT",
-              name_change->getEventLabel(NameChangeTransaction::
-                                         SERVER_SELECTED_EVT));
-    EXPECT_EQ("NameChangeTransaction::SERVER_IO_ERROR_EVT",
-              name_change->getEventLabel(NameChangeTransaction::
-                                         SERVER_IO_ERROR_EVT));
-    EXPECT_EQ("NameChangeTransaction::NO_MORE_SERVERS_EVT",
-              name_change->getEventLabel(NameChangeTransaction::
-                                         NO_MORE_SERVERS_EVT));
-    EXPECT_EQ("NameChangeTransaction::IO_COMPLETED_EVT",
-              name_change->getEventLabel(NameChangeTransaction::
-                                         IO_COMPLETED_EVT));
-    EXPECT_EQ("NameChangeTransaction::UPDATE_OK_EVT",
-              name_change->getEventLabel(NameChangeTransaction::
-                                         UPDATE_OK_EVT));
-    EXPECT_EQ("NameChangeTransaction::UPDATE_FAILED_EVT",
-              name_change->getEventLabel(NameChangeTransaction::
-                                         UPDATE_FAILED_EVT));
+    EXPECT_EQ("SELECT_SERVER_EVT",
+              std::string(name_change->getEventLabel(NameChangeTransaction::
+                                         SELECT_SERVER_EVT)));
+    EXPECT_EQ("SERVER_SELECTED_EVT",
+              std::string(name_change->getEventLabel(NameChangeTransaction::
+                                         SERVER_SELECTED_EVT)));
+    EXPECT_EQ("SERVER_IO_ERROR_EVT",
+              std::string(name_change->getEventLabel(NameChangeTransaction::
+                                         SERVER_IO_ERROR_EVT)));
+    EXPECT_EQ("NO_MORE_SERVERS_EVT",
+              std::string(name_change->getEventLabel(NameChangeTransaction::
+                                         NO_MORE_SERVERS_EVT)));
+    EXPECT_EQ("IO_COMPLETED_EVT",
+              std::string(name_change->getEventLabel(NameChangeTransaction::
+                                         IO_COMPLETED_EVT)));
+    EXPECT_EQ("UPDATE_OK_EVT",
+              std::string(name_change->getEventLabel(NameChangeTransaction::
+                                         UPDATE_OK_EVT)));
+    EXPECT_EQ("UPDATE_FAILED_EVT",
+              std::string(name_change->getEventLabel(NameChangeTransaction::
+                                         UPDATE_FAILED_EVT)));
 
     // Verify stub class labels.
-    EXPECT_EQ("NameChangeStub::SEND_UPDATE_EVT",
-              name_change->getEventLabel(NameChangeStub::SEND_UPDATE_EVT));
+    EXPECT_EQ("SEND_UPDATE_EVT",
+              std::string(name_change->getEventLabel(NameChangeStub::
+                                                     SEND_UPDATE_EVT)));
 
-    // Verify unknown state.
-    EXPECT_EQ("Unknown", name_change->getEventLabel(-1));
+    // Verify undefined event label.
+    EXPECT_EQ(LabeledValueSet::UNDEFINED_LABEL, name_change->getEventLabel(-1));
 }
 
 /// @brief Tests that the transaction will be "failed" upon model errors.

src/bin/d2/tests/state_model_unittests.cc

@@ -154,18 +154,32 @@ public:
         }
     }
 
+    virtual void defineEvents() {
+        StateModel::defineEvents();
+        defineEvent(WORK_START_EVT, "WORK_START_EVT");
+        defineEvent(WORK_DONE_EVT , "WORK_DONE_EVT");
+        defineEvent(ALL_DONE_EVT, "ALL_DONE_EVT");
+    }
+
+    virtual void verifyEvents() {
+        StateModel::verifyEvents();
+        getEvent(WORK_START_EVT);
+        getEvent(WORK_DONE_EVT);
+        getEvent(ALL_DONE_EVT);
+    }
+
     /// @brief Initializes the state handler map.
     virtual void initStateHandlerMap() {
-        addToMap(DUMMY_ST,
+        addToStateHandlerMap(DUMMY_ST,
             boost::bind(&StateModelTest::dummyHandler, this));
 
-        addToMap(READY_ST,
+        addToStateHandlerMap(READY_ST,
             boost::bind(&StateModelTest::readyHandler, this));
 
-        addToMap(DO_WORK_ST,
+        addToStateHandlerMap(DO_WORK_ST,
             boost::bind(&StateModelTest::doWorkHandler, this));
 
-        addToMap(DONE_ST,
+        addToStateHandlerMap(DONE_ST,
             boost::bind(&StateModelTest::doneWorkHandler, this));
     }
 
@@ -205,26 +219,6 @@ public:
         return (str);
     }
 
-    const char* getEventLabel(const int event) const {
-        const char* str = "Unknown";
-        switch(event) {
-        case WORK_START_EVT:
-            str = "StateModelTest::WORK_START_EVT";
-            break;
-        case WORK_DONE_EVT:
-            str = "StateModelTest::WORK_DONE_EVT";
-            break;
-        case ALL_DONE_EVT :
-            str = "StateModelTest::ALL_DONE_EVT";
-            break;
-        default:
-            str = StateModel::getEventLabel(event);
-            break;
-        }
-
-        return (str);
-    }
-
     /// @brief Indicator of whether or not the DUMMY_ST handler has been called.
     bool dummy_called_;
 
@@ -252,8 +246,9 @@ TEST_F(StateModelTest, basicStateMapping) {
     EXPECT_THROW(getStateHandler(READY_ST), StateModelError);
 
     // Verify that we can add a handler to the map.
-    ASSERT_NO_THROW(addToMap(READY_ST, boost::bind(&StateModelTest::
-                                                   dummyHandler, this)));
+    ASSERT_NO_THROW(addToStateHandlerMap(READY_ST,
+                                         boost::bind(&StateModelTest::
+                                                     dummyHandler, this)));
 
     // Verify that we can find the handler by its state.
     StateHandler retreived_handler;
@@ -270,8 +265,9 @@ TEST_F(StateModelTest, basicStateMapping) {
     EXPECT_TRUE(getDummyCalled());
 
     // Verify that we cannot add a duplicate.
-    EXPECT_THROW(addToMap(READY_ST, boost::bind(&StateModelTest::readyHandler,
-                                                this)),
+    EXPECT_THROW(addToStateHandlerMap(READY_ST,
+                                      boost::bind(&StateModelTest::readyHandler,
+                                                  this)),
                  StateModelError);
 
     // Verify that we can still find the handler by its state.
@@ -279,13 +275,15 @@ TEST_F(StateModelTest, basicStateMapping) {
 
 
     // Verify that we cannot add a handler for NEW_ST.
-    EXPECT_THROW(addToMap(NEW_ST, boost::bind(&StateModelTest::dummyHandler,
-                                              this)),
+    EXPECT_THROW(addToStateHandlerMap(NEW_ST,
+                                      boost::bind(&StateModelTest::dummyHandler,
+                                                  this)),
                  StateModelError);
 
     // Verify that we cannot add a handler for END_ST.
-    EXPECT_THROW(addToMap(END_ST, boost::bind(&StateModelTest::dummyHandler,
-                                              this)),
+    EXPECT_THROW(addToStateHandlerMap(END_ST,
+                                      boost::bind(&StateModelTest::dummyHandler,
+                                                  this)),
                  StateModelError);
 }
 
@@ -323,18 +321,21 @@ TEST_F(StateModelTest, stateLabels) {
 /// @brief Tests the ability to decode event values into text labels.
 TEST_F(StateModelTest, eventLabels) {
     // Verify base class labels.
-    EXPECT_EQ("StateModel::NOP_EVT", getEventLabel(NOP_EVT));
-    EXPECT_EQ("StateModel::START_EVT", getEventLabel(START_EVT));
-    EXPECT_EQ("StateModel::END_EVT", getEventLabel(END_EVT));
-    EXPECT_EQ("StateModel::FAIL_EVT", getEventLabel(FAIL_EVT));
+    ASSERT_NO_THROW(defineEvents());
+    ASSERT_NO_THROW(verifyEvents());
+
+    EXPECT_EQ("NOP_EVT", std::string(getEventLabel(NOP_EVT)));
+    EXPECT_EQ("START_EVT", std::string(getEventLabel(START_EVT)));
+    EXPECT_EQ("END_EVT", std::string(getEventLabel(END_EVT)));
+    EXPECT_EQ("FAIL_EVT", std::string(getEventLabel(FAIL_EVT)));
 
     // Verify stub class labels.
-    EXPECT_EQ("StateModelTest::WORK_START_EVT", getEventLabel(WORK_START_EVT));
-    EXPECT_EQ("StateModelTest::WORK_DONE_EVT", getEventLabel(WORK_DONE_EVT));
-    EXPECT_EQ("StateModelTest::ALL_DONE_EVT", getEventLabel(ALL_DONE_EVT));
+    EXPECT_EQ("WORK_START_EVT", std::string(getEventLabel(WORK_START_EVT)));
+    EXPECT_EQ("WORK_DONE_EVT", std::string(getEventLabel(WORK_DONE_EVT)));
+    EXPECT_EQ("ALL_DONE_EVT", std::string(getEventLabel(ALL_DONE_EVT)));
 
     // Verify unknown state.
-    EXPECT_EQ("Unknown", getEventLabel(-1));
+    EXPECT_EQ(LabeledValueSet::UNDEFINED_LABEL, getEventLabel(-1));
 }
 
 /// @brief General testing of member accessors.
@@ -374,7 +375,10 @@ TEST_F(StateModelTest, stateAccessors) {
 }
 
 TEST_F(StateModelTest, eventAccessors) {
-    // Verify post-construction event values.
+    // Construct the event definitions, normally done by startModel.
+    ASSERT_NO_THROW(defineEvents());
+    ASSERT_NO_THROW(verifyEvents());
+
     EXPECT_EQ(NOP_EVT, getNextEvent());
     EXPECT_EQ(NOP_EVT, getLastEvent());
 
@@ -394,6 +398,10 @@ TEST_F(StateModelTest, eventAccessors) {
 }
 
 TEST_F(StateModelTest, transitionWithEnd) {
+    // Construct the event definitions, normally done by startModel.
+    ASSERT_NO_THROW(defineEvents());
+    ASSERT_NO_THROW(verifyEvents());
+
     // Manually init the handlers map.
     ASSERT_NO_THROW(initStateHandlerMap());
     ASSERT_NO_THROW(verifyStateHandlerMap());
@@ -418,6 +426,10 @@ TEST_F(StateModelTest, transitionWithEnd) {
 }
 
 TEST_F(StateModelTest, transitionWithAbort) {
+    // Construct the event definitions, normally done by startModel.
+    ASSERT_NO_THROW(defineEvents());
+    ASSERT_NO_THROW(verifyEvents());
+
     // Manually init the handlers map.
     ASSERT_NO_THROW(initStateHandlerMap());
     ASSERT_NO_THROW(verifyStateHandlerMap());
@@ -442,6 +454,10 @@ TEST_F(StateModelTest, transitionWithAbort) {
 }
 
 TEST_F(StateModelTest, doFlags) {
+    // Construct the event definitions, normally done by startModel.
+    ASSERT_NO_THROW(defineEvents());
+    ASSERT_NO_THROW(verifyEvents());
+
     // Manually init the handlers map.
     ASSERT_NO_THROW(initStateHandlerMap());
     ASSERT_NO_THROW(verifyStateHandlerMap());
@@ -471,6 +487,10 @@ TEST_F(StateModelTest, doFlags) {
 }
 
 TEST_F(StateModelTest, statusMethods) {
+    // Construct the event definitions, normally done by startModel.
+    ASSERT_NO_THROW(defineEvents());
+    ASSERT_NO_THROW(verifyEvents());
+
     // Manually init the handlers map.
     ASSERT_NO_THROW(initStateHandlerMap());
     ASSERT_NO_THROW(verifyStateHandlerMap());
@@ -482,9 +502,21 @@ TEST_F(StateModelTest, statusMethods) {
     EXPECT_FALSE(isModelDone());
     EXPECT_FALSE(didModelFail());
 
+    // verify that you can add to the before the model has started.
+    EXPECT_NO_THROW(addToStateHandlerMap(9998,
+                                         boost::bind(&StateModelTest::
+                                                     readyHandler, this)));
+
     // call transition to move from NEW_ST to DUMMY_ST with START_EVT
     EXPECT_NO_THROW(transition(DUMMY_ST, START_EVT));
 
+
+    // verify that you cannot add to the map once the model has started.
+    EXPECT_THROW(addToStateHandlerMap(9999,
+                                      boost::bind(&StateModelTest::readyHandler,
+                                                  this)),
+                 StateModelError);
+
     // The state and event combos set above, should show the model as
     // "running", all others should be false.
     EXPECT_FALSE(isModelNew());
@@ -516,6 +548,10 @@ TEST_F(StateModelTest, statusMethods) {
 }
 
 TEST_F(StateModelTest, statusMethodsOnFailure) {
+    // Construct the event definitions, normally done by startModel.
+    ASSERT_NO_THROW(defineEvents());
+    ASSERT_NO_THROW(verifyEvents());
+
     // Manually init the handlers map.
     ASSERT_NO_THROW(initStateHandlerMap());
     ASSERT_NO_THROW(verifyStateHandlerMap());
@@ -535,6 +571,10 @@ TEST_F(StateModelTest, statusMethodsOnFailure) {
 }
 
 TEST_F(StateModelTest, contextStrs) {
+    // Construct the event definitions, normally done by startModel.
+    ASSERT_NO_THROW(defineEvents());
+    ASSERT_NO_THROW(verifyEvents());
+
     // Manually init the handlers map.
     ASSERT_NO_THROW(initStateHandlerMap());
     ASSERT_NO_THROW(verifyStateHandlerMap());
@@ -566,7 +606,8 @@ TEST_F(StateModelTest, invalidState) {
 
     // Now call runModel() which should not throw, but should result
     // in a failed model and a call to onModelFailure().
-    EXPECT_NO_THROW(runModel(START_EVT));
+    //EXPECT_NO_THROW(runModel(START_EVT));
+    (runModel(START_EVT));
 
     // Verify that status methods are correct: model is done but failed.
     EXPECT_FALSE(isModelNew());