table: StrategyChoice::insert return value includes error description
The error description is used in the response to a strategy-choice/set
management command failure.
refs #3868
Change-Id: I14e0eb4dc311806a90ebfe60fa17177d2809d104
diff --git a/daemon/mgmt/strategy-choice-manager.cpp b/daemon/mgmt/strategy-choice-manager.cpp
index 16ebb0d..9732bd2 100644
--- a/daemon/mgmt/strategy-choice-manager.cpp
+++ b/daemon/mgmt/strategy-choice-manager.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/**
- * Copyright (c) 2014-2016, Regents of the University of California,
+ * Copyright (c) 2014-2017, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -26,6 +26,7 @@
#include "strategy-choice-manager.hpp"
#include "table/strategy-choice.hpp"
#include <ndn-cxx/mgmt/nfd/strategy-choice.hpp>
+#include <boost/lexical_cast.hpp>
namespace nfd {
@@ -53,10 +54,10 @@
const Name& prefix = parameters.getName();
const Name& strategy = parameters.getStrategy();
- if (!m_table.insert(prefix, strategy)) {
- NFD_LOG_DEBUG("strategy-choice/set(" << prefix << "," << strategy << "): cannot-create");
- return done(ControlResponse(404, "Unknown strategy or invalid parameters"));
- ///\todo #3868 pass along the error message from Strategy subclass constructor
+ StrategyChoice::InsertResult res = m_table.insert(prefix, strategy);
+ if (!res) {
+ NFD_LOG_DEBUG("strategy-choice/set(" << prefix << "," << strategy << "): cannot-create " << res);
+ return done(ControlResponse(404, boost::lexical_cast<std::string>(res)));
}
NFD_LOG_DEBUG("strategy-choice/set(" << prefix << "," << strategy << "): OK");
diff --git a/daemon/table/strategy-choice.cpp b/daemon/table/strategy-choice.cpp
index 7e2f5bf..77f0bf5 100644
--- a/daemon/table/strategy-choice.cpp
+++ b/daemon/table/strategy-choice.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/**
- * Copyright (c) 2014-2016, Regents of the University of California,
+ * Copyright (c) 2014-2017, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -66,7 +66,7 @@
++m_nItems;
}
-bool
+StrategyChoice::InsertResult
StrategyChoice::insert(const Name& prefix, const Name& strategyName)
{
unique_ptr<Strategy> strategy;
@@ -75,12 +75,12 @@
}
catch (const std::invalid_argument& e) {
NFD_LOG_ERROR("insert(" << prefix << "," << strategyName << ") cannot create strategy: " << e.what());
- return false;
+ return InsertResult(InsertResult::EXCEPTION, e.what());
}
if (strategy == nullptr) {
NFD_LOG_ERROR("insert(" << prefix << "," << strategyName << ") strategy not registered");
- return false;
+ return InsertResult::NOT_REGISTERED;
}
name_tree::Entry& nte = m_nameTree.lookup(prefix);
@@ -89,7 +89,7 @@
if (entry != nullptr) {
if (entry->getStrategyInstanceName() == strategy->getInstanceName()) {
NFD_LOG_TRACE("insert(" << prefix << ") not changing " << strategy->getInstanceName());
- return true;
+ return InsertResult::OK;
}
oldStrategy = &entry->getStrategy();
NFD_LOG_TRACE("insert(" << prefix << ") changing from " << oldStrategy->getInstanceName() <<
@@ -106,7 +106,27 @@
this->changeStrategy(*entry, *oldStrategy, *strategy);
entry->setStrategy(std::move(strategy));
- return true;
+ return InsertResult::OK;
+}
+
+StrategyChoice::InsertResult::InsertResult(Status status, const std::string& exceptionMessage)
+ : m_status(status)
+ , m_exceptionMessage(exceptionMessage)
+{
+}
+
+std::ostream&
+operator<<(std::ostream& os, const StrategyChoice::InsertResult& res)
+{
+ switch (res.m_status) {
+ case StrategyChoice::InsertResult::OK:
+ return os << "OK";
+ case StrategyChoice::InsertResult::NOT_REGISTERED:
+ return os << "Strategy not registered";
+ case StrategyChoice::InsertResult::EXCEPTION:
+ return os << "Error instantiating strategy: " << res.m_exceptionMessage;
+ }
+ return os;
}
void
diff --git a/daemon/table/strategy-choice.hpp b/daemon/table/strategy-choice.hpp
index e007acf..4e59716 100644
--- a/daemon/table/strategy-choice.hpp
+++ b/daemon/table/strategy-choice.hpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/**
- * Copyright (c) 2014-2016, Regents of the University of California,
+ * Copyright (c) 2014-2017, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -68,17 +68,46 @@
setDefaultStrategy(const Name& strategyName);
public: // Strategy Choice table
- /** \brief set strategy of prefix to be strategyName
- * \param prefix the name prefix for which \p strategyName should be used
- * \param strategyName the strategy to be used
- * \return true on success
- *
- * This method set a strategy onto a Name prefix.
- * The strategy must have been installed.
- * The strategyName can either be exact (contains version component),
- * or omit the version component to pick the latest version.
+ class InsertResult
+ {
+ public:
+ explicit
+ operator bool() const
+ {
+ return m_status == OK;
+ }
+
+ bool
+ isRegistered() const
+ {
+ return m_status != NOT_REGISTERED;
+ }
+
+ private:
+ enum Status {
+ OK,
+ NOT_REGISTERED,
+ EXCEPTION
+ };
+
+ // implicitly constructible from Status
+ InsertResult(Status status, const std::string& exceptionMessage = "");
+
+ private:
+ Status m_status;
+ std::string m_exceptionMessage;
+
+ friend class StrategyChoice;
+ friend std::ostream& operator<<(std::ostream&, const InsertResult&);
+ };
+
+ /** \brief set strategy of \p prefix to be \p strategyName
+ * \param prefix the name prefix to change strategy
+ * \param strategyName strategy instance name, may contain version and parameters;
+ * strategy must have been registered
+ * \return convertible to true on success; convertible to false on failure
*/
- bool
+ InsertResult
insert(const Name& prefix, const Name& strategyName);
/** \brief make prefix to inherit strategy from its parent
@@ -159,6 +188,9 @@
size_t m_nItems;
};
+std::ostream&
+operator<<(std::ostream& os, const StrategyChoice::InsertResult& res);
+
} // namespace strategy_choice
using strategy_choice::StrategyChoice;
diff --git a/tests/daemon/fw/choose-strategy.hpp b/tests/daemon/fw/choose-strategy.hpp
index 9ecc54b..4045540 100644
--- a/tests/daemon/fw/choose-strategy.hpp
+++ b/tests/daemon/fw/choose-strategy.hpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/**
- * Copyright (c) 2014-2016, Regents of the University of California,
+ * Copyright (c) 2014-2017, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -28,6 +28,7 @@
#include "fw/forwarder.hpp"
#include "table/strategy-choice.hpp"
+#include <boost/lexical_cast.hpp>
namespace nfd {
namespace fw {
@@ -41,7 +42,7 @@
* \param forwarder the forwarder
* \param prefix namespace to choose the strategy for
* \param instanceName strategy instance name; the strategy must already be registered
- * \throw std::invalid_argument cannot find strategy by instanceName
+ * \throw std::invalid_argument cannot create strategy by instanceName
* \throw std::bad_cast strategy type is incompatible with S
* \return a reference to the strategy
*/
@@ -51,9 +52,9 @@
const Name& instanceName = S::getStrategyName())
{
StrategyChoice& sc = forwarder.getStrategyChoice();
- bool isInserted = sc.insert(prefix, instanceName);
- if (!isInserted) {
- BOOST_THROW_EXCEPTION(std::invalid_argument("cannot create strategy"));
+ auto insertRes = sc.insert(prefix, instanceName);
+ if (!insertRes) {
+ BOOST_THROW_EXCEPTION(std::invalid_argument(boost::lexical_cast<std::string>(insertRes)));
}
return dynamic_cast<S&>(sc.findEffectiveStrategy(prefix));
}
diff --git a/tests/daemon/table/strategy-choice.t.cpp b/tests/daemon/table/strategy-choice.t.cpp
index dd2fa5e..7dd0e1a 100644
--- a/tests/daemon/table/strategy-choice.t.cpp
+++ b/tests/daemon/table/strategy-choice.t.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/**
- * Copyright (c) 2014-2016, Regents of the University of California,
+ * Copyright (c) 2014-2017, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -121,7 +121,9 @@
BOOST_CHECK(this->isStrategyType<VersionedDummyStrategy<3>>("/F"));
// higher version: failure
- BOOST_CHECK_EQUAL(sc.insert("/G", strategyNameV5), false);
+ StrategyChoice::InsertResult res5 = sc.insert("/G", strategyNameV5);
+ BOOST_CHECK(!res5);
+ BOOST_CHECK(!res5.isRegistered());
}
BOOST_AUTO_TEST_CASE(Parameters)
@@ -139,7 +141,7 @@
// parameter without version is disallowed
Name oneParamUnversioned = strategyNameP.getPrefix(-1).append("param");
- BOOST_CHECK_EQUAL(sc.insert("/D", oneParamUnversioned), false);
+ BOOST_CHECK(!sc.insert("/D", oneParamUnversioned));
}
BOOST_AUTO_TEST_CASE(Get)