mgmt: refactor management modules to use ControlCommand
Change-Id: Icf5e10f2f2d0b95c85fd871e6484c3ed58ed6c46
diff --git a/daemon/mgmt/face-manager.cpp b/daemon/mgmt/face-manager.cpp
index 039270f..518021a 100644
--- a/daemon/mgmt/face-manager.cpp
+++ b/daemon/mgmt/face-manager.cpp
@@ -587,8 +587,16 @@
ControlParameters& parameters)
{
const Name& requestName = request.getName();
+ ndn::nfd::FaceCreateCommand command;
+
+ if (!validateParameters(command, parameters))
+ {
+ sendResponse(requestName, 400, "Malformed command");
+ return;
+ }
+
FaceUri uri;
- if (!parameters.hasUri() || !uri.parse(parameters.getUri()))
+ if (!uri.parse(parameters.getUri()))
{
sendResponse(requestName, 400, "Malformed command");
return;
@@ -612,7 +620,9 @@
ControlParameters& parameters)
{
const Name& requestName = request.getName();
- if (!parameters.hasFaceId())
+ ndn::nfd::FaceDestroyCommand command;
+
+ if (!validateParameters(command, parameters))
{
sendResponse(requestName, 400, "Malformed command");
return;
@@ -625,6 +635,7 @@
}
sendResponse(requestName, 200, "Success", parameters.wireEncode());
+
}
void
@@ -653,16 +664,14 @@
bool
-FaceManager::validateLocalControlParameters(const Interest& request,
- ControlParameters& parameters,
- shared_ptr<LocalFace>& outFace,
- LocalControlFeature& outFeature)
+FaceManager::extractLocalControlParameters(const Interest& request,
+ ControlParameters& parameters,
+ ControlCommand& command,
+ shared_ptr<LocalFace>& outFace,
+ LocalControlFeature& outFeature)
{
- if (!parameters.hasLocalControlFeature() ||
- (parameters.getLocalControlFeature() != LOCAL_CONTROL_FEATURE_INCOMING_FACE_ID &&
- parameters.getLocalControlFeature() != LOCAL_CONTROL_FEATURE_NEXT_HOP_FACE_ID))
+ if (!validateParameters(command, parameters))
{
- NFD_LOG_INFO("command result: malformed");
sendResponse(request.getName(), 400, "Malformed command");
return false;
}
@@ -671,14 +680,15 @@
if (!static_cast<bool>(face))
{
- NFD_LOG_INFO("command result: faceid " << parameters.getFaceId() << " not found");
- sendResponse(request.getName(), 410, "Requested face not found");
+ NFD_LOG_INFO("command result: faceid " << request.getIncomingFaceId() << " not found");
+ sendResponse(request.getName(), 410, "Face not found");
return false;
}
else if (!face->isLocal())
{
- NFD_LOG_INFO("command result: cannot enable local control on non-local faceid " << parameters.getFaceId());
- sendResponse(request.getName(), 412, "Requested face is non-local");
+ NFD_LOG_INFO("command result: cannot enable local control on non-local faceid " <<
+ face->getId());
+ sendResponse(request.getName(), 412, "Face is non-local");
return false;
}
@@ -692,10 +702,13 @@
FaceManager::enableLocalControl(const Interest& request,
ControlParameters& parameters)
{
+ ndn::nfd::FaceEnableLocalControlCommand command;
+
+
shared_ptr<LocalFace> face;
LocalControlFeature feature;
- if (validateLocalControlParameters(request, parameters, face, feature))
+ if (extractLocalControlParameters(request, parameters, command, face, feature))
{
face->setLocalControlHeaderFeature(feature, true);
sendResponse(request.getName(), 200, "Success", parameters.wireEncode());
@@ -706,10 +719,11 @@
FaceManager::disableLocalControl(const Interest& request,
ControlParameters& parameters)
{
+ ndn::nfd::FaceDisableLocalControlCommand command;
shared_ptr<LocalFace> face;
LocalControlFeature feature;
- if (validateLocalControlParameters(request, parameters, face, feature))
+ if (extractLocalControlParameters(request, parameters, command, face, feature))
{
face->setLocalControlHeaderFeature(feature, false);
sendResponse(request.getName(), 200, "Success", parameters.wireEncode());
diff --git a/daemon/mgmt/face-manager.hpp b/daemon/mgmt/face-manager.hpp
index 7312bba..4033b67 100644
--- a/daemon/mgmt/face-manager.hpp
+++ b/daemon/mgmt/face-manager.hpp
@@ -73,10 +73,11 @@
ControlParameters& parameters);
VIRTUAL_WITH_TESTS bool
- validateLocalControlParameters(const Interest& request,
- ControlParameters& parameters,
- shared_ptr<LocalFace>& outFace,
- LocalControlFeature& outFeature);
+ extractLocalControlParameters(const Interest& request,
+ ControlParameters& parameters,
+ ControlCommand& command,
+ shared_ptr<LocalFace>& outFace,
+ LocalControlFeature& outFeature);
VIRTUAL_WITH_TESTS void
enableLocalControl(const Interest& request,
diff --git a/daemon/mgmt/fib-manager.cpp b/daemon/mgmt/fib-manager.cpp
index b1b869b..0bf4f34 100644
--- a/daemon/mgmt/fib-manager.cpp
+++ b/daemon/mgmt/fib-manager.cpp
@@ -122,9 +122,7 @@
if (signedVerbProcessor != m_signedVerbDispatch.end())
{
ControlParameters parameters;
- if (!extractParameters(parameterComponent, parameters) ||
- !parameters.hasName() ||
- !parameters.hasFaceId())
+ if (!extractParameters(parameterComponent, parameters) || !parameters.hasFaceId())
{
NFD_LOG_INFO("command result: malformed verb: " << verb);
sendResponse(command, 400, "Malformed command");
@@ -148,15 +146,17 @@
}
}
-
-
void
FibManager::addNextHop(ControlParameters& parameters,
ControlResponse& response)
{
- if (!parameters.hasCost())
+ ndn::nfd::FibAddNextHopCommand command;
+
+ if (!validateParameters(command, parameters))
{
- parameters.setCost(0);
+ NFD_LOG_INFO("add-nexthop result: FAIL reason: malformed");
+ setResponse(response, 400, "Malformed command");
+ return;
}
const Name& prefix = parameters.getName();
@@ -179,6 +179,7 @@
<< " faceid: " << faceId
<< " cost: " << cost);
+ NFD_LOG_INFO("add-nexthop result: SUCCESS");
setResponse(response, 200, "Success", parameters.wireEncode());
}
else
@@ -192,6 +193,14 @@
FibManager::removeNextHop(ControlParameters& parameters,
ControlResponse& response)
{
+ ndn::nfd::FibRemoveNextHopCommand command;
+ if (!validateParameters(command, parameters))
+ {
+ NFD_LOG_INFO("remove-nexthop result: FAIL reason: malformed");
+ setResponse(response, 400, "Malformed command");
+ return;
+ }
+
NFD_LOG_DEBUG("remove-nexthop prefix: " << parameters.getName()
<< " faceid: " << parameters.getFaceId());
@@ -211,6 +220,8 @@
}
}
}
+
+ NFD_LOG_INFO("remove-nexthop result: SUCCESS");
setResponse(response, 200, "Success", parameters.wireEncode());
}
diff --git a/daemon/mgmt/manager-base.cpp b/daemon/mgmt/manager-base.cpp
index af5fc37..08d845c 100644
--- a/daemon/mgmt/manager-base.cpp
+++ b/daemon/mgmt/manager-base.cpp
@@ -77,6 +77,24 @@
m_face->put(*responseData);
}
+bool
+ManagerBase::validateParameters(const ControlCommand& command,
+ ControlParameters& parameters)
+{
+ try
+ {
+ command.validateRequest(parameters);
+ }
+ catch (const ControlCommand::ArgumentError& error)
+ {
+ return false;
+ }
+
+ command.applyDefaultsToRequest(parameters);
+
+ return true;
+}
+
void
ManagerBase::onCommandValidationFailed(const shared_ptr<const Interest>& command,
const std::string& error)
diff --git a/daemon/mgmt/manager-base.hpp b/daemon/mgmt/manager-base.hpp
index 46e34b6..ed8224a 100644
--- a/daemon/mgmt/manager-base.hpp
+++ b/daemon/mgmt/manager-base.hpp
@@ -12,11 +12,13 @@
#include "mgmt/command-validator.hpp"
#include "mgmt/internal-face.hpp"
+#include <ndn-cpp-dev/management/nfd-control-command.hpp>
#include <ndn-cpp-dev/management/nfd-control-response.hpp>
#include <ndn-cpp-dev/management/nfd-control-parameters.hpp>
namespace nfd {
+using ndn::nfd::ControlCommand;
using ndn::nfd::ControlResponse;
using ndn::nfd::ControlParameters;
@@ -71,6 +73,10 @@
const std::string& text,
const Block& body);
+ virtual bool
+ validateParameters(const ControlCommand& command,
+ ControlParameters& parameters);
+
PUBLIC_WITH_TESTS_ELSE_PROTECTED:
void
addInterestRule(const std::string& regex,
diff --git a/daemon/mgmt/strategy-choice-manager.cpp b/daemon/mgmt/strategy-choice-manager.cpp
index bbedd34..a4c4e38 100644
--- a/daemon/mgmt/strategy-choice-manager.cpp
+++ b/daemon/mgmt/strategy-choice-manager.cpp
@@ -74,7 +74,7 @@
const Name::Component& parameterComponent = command[COMMAND_PREFIX.size() + 1];
ControlParameters parameters;
- if (!extractParameters(parameterComponent, parameters) || !parameters.hasName())
+ if (!extractParameters(parameterComponent, parameters))
{
sendResponse(command, 400, "Malformed command");
return;
@@ -95,15 +95,19 @@
NFD_LOG_INFO("command result: unsupported verb: " << verb);
setResponse(response, 501, "Unsupported command");
}
+
sendResponse(command, response);
}
void
-StrategyChoiceManager::setStrategy(const ControlParameters& parameters,
+StrategyChoiceManager::setStrategy(ControlParameters& parameters,
ControlResponse& response)
{
- if (!parameters.hasStrategy())
+ ndn::nfd::StrategyChoiceSetCommand command;
+
+ if (!validateParameters(command, parameters))
{
+ NFD_LOG_INFO("strategy-choice result: FAIL reason: malformed");
setResponse(response, 400, "Malformed command");
return;
}
@@ -121,30 +125,41 @@
if (m_strategyChoice.insert(prefix, selectedStrategy))
{
+ NFD_LOG_INFO("strategy-choice result: SUCCESS");
setResponse(response, 200, "Success", parameters.wireEncode());
}
else
{
+ NFD_LOG_INFO("strategy-choice result: FAIL reason: not-installed");
setResponse(response, 405, "Strategy not installed");
}
}
void
-StrategyChoiceManager::unsetStrategy(const ControlParameters& parameters,
+StrategyChoiceManager::unsetStrategy(ControlParameters& parameters,
ControlResponse& response)
{
- static const Name ROOT_PREFIX;
+ ndn::nfd::StrategyChoiceUnsetCommand command;
- const Name& prefix = parameters.getName();
- if (prefix == ROOT_PREFIX)
+ if (!validateParameters(command, parameters))
{
- NFD_LOG_INFO("strategy-choice result: FAIL reason: unknown-prefix: "
- << parameters.getName());
- setResponse(response, 403, "Cannot unset root prefix strategy");
+ static const Name ROOT_PREFIX;
+ if (parameters.hasName() && parameters.getName() == ROOT_PREFIX)
+ {
+ NFD_LOG_INFO("strategy-choice result: FAIL reason: unset-root");
+ setResponse(response, 403, "Cannot unset root prefix strategy");
+ }
+ else
+ {
+ NFD_LOG_INFO("strategy-choice result: FAIL reason: malformed");
+ setResponse(response, 400, "Malformed command");
+ }
return;
}
- m_strategyChoice.erase(prefix);
+ m_strategyChoice.erase(parameters.getName());
+
+ NFD_LOG_INFO("strategy-choice result: SUCCESS");
setResponse(response, 200, "Success", parameters.wireEncode());
}
diff --git a/daemon/mgmt/strategy-choice-manager.hpp b/daemon/mgmt/strategy-choice-manager.hpp
index d884786..19bee33 100644
--- a/daemon/mgmt/strategy-choice-manager.hpp
+++ b/daemon/mgmt/strategy-choice-manager.hpp
@@ -34,12 +34,13 @@
onValidatedStrategyChoiceRequest(const shared_ptr<const Interest>& request);
void
- setStrategy(const ControlParameters& parameters,
+ setStrategy(ControlParameters& parameters,
ControlResponse& response);
void
- unsetStrategy(const ControlParameters& parameters,
+ unsetStrategy(ControlParameters& parameters,
ControlResponse& response);
+
private:
StrategyChoice& m_strategyChoice;
diff --git a/tests/mgmt/face-manager.cpp b/tests/mgmt/face-manager.cpp
index d439ea4..1c3734d 100644
--- a/tests/mgmt/face-manager.cpp
+++ b/tests/mgmt/face-manager.cpp
@@ -899,7 +899,6 @@
FaceTableFixture::m_faceTable.add(dummy);
ControlParameters parameters;
- parameters.setFaceId(dummy->getId());
parameters.setLocalControlFeature(LOCAL_CONTROL_FEATURE_INCOMING_FACE_ID);
Block encodedParameters(parameters.wireEncode());
@@ -966,7 +965,7 @@
TestFaceManagerCommon::m_face->onReceiveData +=
bind(&LocalControlFixture::validateControlResponse, this, _1,
- enableCommand->getName(), 410, "Requested face not found");
+ enableCommand->getName(), 410, "Face not found");
onValidatedFaceRequest(enableCommand);
@@ -987,7 +986,7 @@
TestFaceManagerCommon::m_face->onReceiveData +=
bind(&LocalControlFixture::validateControlResponse, this, _1,
- disableCommand->getName(), 410, "Requested face not found");
+ disableCommand->getName(), 410, "Face not found");
onValidatedFaceRequest(disableCommand);
@@ -1004,7 +1003,6 @@
FaceTableFixture::m_faceTable.add(dummy);
ControlParameters parameters;
- parameters.setFaceId(dummy->getId());
Block encodedParameters(parameters.wireEncode());
@@ -1056,7 +1054,6 @@
FaceTableFixture::m_faceTable.add(dummy);
ControlParameters parameters;
- parameters.setFaceId(dummy->getId());
parameters.setLocalControlFeature(LOCAL_CONTROL_FEATURE_INCOMING_FACE_ID);
Block encodedParameters(parameters.wireEncode());
@@ -1071,7 +1068,7 @@
TestFaceManagerCommon::m_face->onReceiveData +=
bind(&LocalControlFixture::validateControlResponse, this, _1,
- enableCommand->getName(), 412, "Requested face is non-local");
+ enableCommand->getName(), 412, "Face is non-local");
onValidatedFaceRequest(enableCommand);
@@ -1090,7 +1087,7 @@
TestFaceManagerCommon::m_face->onReceiveData +=
bind(&LocalControlFixture::validateControlResponse, this, _1,
- disableCommand->getName(), 412, "Requested face is non-local");
+ disableCommand->getName(), 412, "Face is non-local");
onValidatedFaceRequest(disableCommand);
@@ -1105,7 +1102,6 @@
FaceTableFixture::m_faceTable.add(dummy);
ControlParameters parameters;
- parameters.setFaceId(dummy->getId());
parameters.setLocalControlFeature(LOCAL_CONTROL_FEATURE_NEXT_HOP_FACE_ID);
Block encodedParameters(parameters.wireEncode());
@@ -1173,7 +1169,7 @@
TestFaceManagerCommon::m_face->onReceiveData +=
bind(&LocalControlFixture::validateControlResponse, this, _1,
- enableCommand->getName(), 410, "Requested face not found");
+ enableCommand->getName(), 410, "Face not found");
onValidatedFaceRequest(enableCommand);
@@ -1195,7 +1191,7 @@
TestFaceManagerCommon::m_face->onReceiveData +=
bind(&LocalControlFixture::validateControlResponse, this, _1,
- disableCommand->getName(), 410, "Requested face not found");
+ disableCommand->getName(), 410, "Face not found");
onValidatedFaceRequest(disableCommand);
@@ -1212,7 +1208,6 @@
FaceTableFixture::m_faceTable.add(dummy);
ControlParameters parameters;
- parameters.setFaceId(dummy->getId());
parameters.setLocalControlFeature(LOCAL_CONTROL_FEATURE_NEXT_HOP_FACE_ID);
Block encodedParameters(parameters.wireEncode());
@@ -1227,7 +1222,7 @@
TestFaceManagerCommon::m_face->onReceiveData +=
bind(&LocalControlFixture::validateControlResponse, this, _1,
- enableCommand->getName(), 412, "Requested face is non-local");
+ enableCommand->getName(), 412, "Face is non-local");
onValidatedFaceRequest(enableCommand);
@@ -1246,7 +1241,7 @@
TestFaceManagerCommon::m_face->onReceiveData +=
bind(&LocalControlFixture::validateControlResponse, this, _1,
- disableCommand->getName(), 412, "Requested face is non-local");
+ disableCommand->getName(), 412, "Face is non-local");
onValidatedFaceRequest(disableCommand);
@@ -1482,7 +1477,6 @@
FaceTableFixture::m_faceTable.add(dummy);
ControlParameters parameters;
- parameters.setUri("tcp://127.0.0.1");
parameters.setFaceId(dummy->getId());
Block encodedParameters(parameters.wireEncode());