mgmt: change strategy-choice/set response codes

If the specified strategy name cannot be instantiated,
respond with 404 status code.

refs #3868

Change-Id: I8275923f149ec9c2ed82e485e933d16320bd3a03
diff --git a/daemon/mgmt/strategy-choice-manager.cpp b/daemon/mgmt/strategy-choice-manager.cpp
index 6ed7826..16ebb0d 100644
--- a/daemon/mgmt/strategy-choice-manager.cpp
+++ b/daemon/mgmt/strategy-choice-manager.cpp
@@ -38,55 +38,51 @@
   , m_table(strategyChoice)
 {
   registerCommandHandler<ndn::nfd::StrategyChoiceSetCommand>("set",
-    bind(&StrategyChoiceManager::setStrategy, this, _2, _3, _4, _5));
+    bind(&StrategyChoiceManager::setStrategy, this, _4, _5));
   registerCommandHandler<ndn::nfd::StrategyChoiceUnsetCommand>("unset",
-    bind(&StrategyChoiceManager::unsetStrategy, this, _2, _3, _4, _5));
+    bind(&StrategyChoiceManager::unsetStrategy, this, _4, _5));
 
   registerStatusDatasetHandler("list",
-    bind(&StrategyChoiceManager::listChoices, this, _1, _2, _3));
+    bind(&StrategyChoiceManager::listChoices, this, _3));
 }
 
 void
-StrategyChoiceManager::setStrategy(const Name& topPrefix, const Interest& interest,
-                                   ControlParameters parameters,
+StrategyChoiceManager::setStrategy(ControlParameters parameters,
                                    const ndn::mgmt::CommandContinuation& done)
 {
   const Name& prefix = parameters.getName();
-  const Name& selectedStrategy = parameters.getStrategy();
+  const Name& strategy = parameters.getStrategy();
 
-  if (!m_table.hasStrategy(selectedStrategy)) {
-    NFD_LOG_DEBUG("strategy-choice result: FAIL reason: unknown-strategy: "
-                  << parameters.getStrategy());
-    return done(ControlResponse(504, "Unsupported strategy"));
+  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
   }
 
-  if (m_table.insert(prefix, selectedStrategy)) {
-    NFD_LOG_DEBUG("strategy-choice result: SUCCESS");
-    auto currentStrategyChoice = m_table.get(prefix);
-    BOOST_ASSERT(currentStrategyChoice.first);
-    parameters.setStrategy(currentStrategyChoice.second);
-    return done(ControlResponse(200, "OK").setBody(parameters.wireEncode()));
-  }
-  else {
-    NFD_LOG_DEBUG("strategy-choice result: FAIL reason: not-installed");
-    return done(ControlResponse(405, "Strategy not installed"));
-  }
+  NFD_LOG_DEBUG("strategy-choice/set(" << prefix << "," << strategy << "): OK");
+  bool hasEntry = false;
+  Name instanceName;
+  std::tie(hasEntry, instanceName) = m_table.get(prefix);
+  BOOST_ASSERT_MSG(hasEntry, "StrategyChoice entry must exist after StrategyChoice::insert");
+  parameters.setStrategy(instanceName);
+  return done(ControlResponse(200, "OK").setBody(parameters.wireEncode()));
 }
 
 void
-StrategyChoiceManager::unsetStrategy(const Name& topPrefix, const Interest& interest,
-                                     ControlParameters parameters,
+StrategyChoiceManager::unsetStrategy(ControlParameters parameters,
                                      const ndn::mgmt::CommandContinuation& done)
 {
+  const Name& prefix = parameters.getName();
+  // no need to test for ndn:/ , parameter validation takes care of that
+
   m_table.erase(parameters.getName());
 
-  NFD_LOG_DEBUG("strategy-choice result: SUCCESS");
+  NFD_LOG_DEBUG("strategy-choice/unset(" << prefix << "): OK");
   done(ControlResponse(200, "OK").setBody(parameters.wireEncode()));
 }
 
 void
-StrategyChoiceManager::listChoices(const Name& topPrefix, const Interest& interest,
-                                   ndn::mgmt::StatusDatasetContext& context)
+StrategyChoiceManager::listChoices(ndn::mgmt::StatusDatasetContext& context)
 {
   for (const auto& i : m_table) {
     ndn::nfd::StrategyChoice entry;
diff --git a/daemon/mgmt/strategy-choice-manager.hpp b/daemon/mgmt/strategy-choice-manager.hpp
index 5520790..00d4dd9 100644
--- a/daemon/mgmt/strategy-choice-manager.hpp
+++ b/daemon/mgmt/strategy-choice-manager.hpp
@@ -47,18 +47,15 @@
 
 private:
   void
-  setStrategy(const Name& topPrefix, const Interest& interest,
-              ControlParameters parameters,
+  setStrategy(ControlParameters parameters,
               const ndn::mgmt::CommandContinuation& done);
 
   void
-  unsetStrategy(const Name& topPrefix, const Interest& interest,
-                ControlParameters parameters,
+  unsetStrategy(ControlParameters parameters,
                 const ndn::mgmt::CommandContinuation& done);
 
   void
-  listChoices(const Name& topPrefix, const Interest& interest,
-              ndn::mgmt::StatusDatasetContext& context);
+  listChoices(ndn::mgmt::StatusDatasetContext& context);
 
 private:
   strategy_choice::StrategyChoice& m_table;
diff --git a/tests/daemon/mgmt/strategy-choice-manager.t.cpp b/tests/daemon/mgmt/strategy-choice-manager.t.cpp
index f1121ea..93c3080 100644
--- a/tests/daemon/mgmt/strategy-choice-manager.t.cpp
+++ b/tests/daemon/mgmt/strategy-choice-manager.t.cpp
@@ -80,7 +80,7 @@
 BOOST_FIXTURE_TEST_SUITE(TestStrategyChoiceManager, StrategyChoiceManagerFixture)
 
 ///\todo #3868 rewrite test case after changing strategy versioning scheme
-BOOST_AUTO_TEST_CASE_EXPECTED_FAILURES(SetStrategy, 6)
+BOOST_AUTO_TEST_CASE_EXPECTED_FAILURES(SetStrategy, 7)
 BOOST_AUTO_TEST_CASE(SetStrategy)
 {
   auto testSetStrategy = [this] (const ControlParameters& parameters) -> Name {