management: correct behavior of FaceUpdateCommand::applyDefaultsToRequest and validateResponse

refs #3731

Change-Id: I37feb7a9c4002f723da8e65b40afe0242fd42bfc

AUTHORS.md

@@ -32,7 +32,7 @@ in the library:
 * Chengyu Fan           <chengyu@cs.colostate.edu>
 * Qiuhan Ding           <http://irl.cs.ucla.edu/~qiuhanding/>
 * Spyridon Mastorakis   <http://cs.ucla.edu/~mastorakis/>
-* Eric Newberry         <http://ericnewberry.com>
+* Eric Newberry         <http://cs.arizona.edu/~enewberry/>
 * João Pereira          <http://website.jpereira.co.uk>
 * Mickey Sweatt         <https://www.linkedin.com/in/michaelsweatt>
 * Yanbiao Li            <https://www.linkedin.com/pub/yanbiao-li/24/7a1/4ba>

src/management/nfd-control-command.cpp

@@ -147,16 +147,17 @@ FaceUpdateCommand::FaceUpdateCommand()
     .optional(CONTROL_PARAMETER_FLAGS)
     .optional(CONTROL_PARAMETER_MASK);
   m_responseValidator
-    .optional(CONTROL_PARAMETER_FACE_ID)
-    .optional(CONTROL_PARAMETER_FACE_PERSISTENCY)
-    .optional(CONTROL_PARAMETER_FLAGS)
-    .optional(CONTROL_PARAMETER_MASK);
+    .required(CONTROL_PARAMETER_FACE_ID)
+    .required(CONTROL_PARAMETER_FACE_PERSISTENCY)
+    .required(CONTROL_PARAMETER_FLAGS);
 }
 
 void
 FaceUpdateCommand::applyDefaultsToRequest(ControlParameters& parameters) const
 {
-  parameters.setFaceId(0);
+  if (!parameters.hasFaceId()) {
+    parameters.setFaceId(0);
+  }
 }
 
 void
@@ -172,7 +173,11 @@ FaceUpdateCommand::validateRequest(const ControlParameters& parameters) const
 void
 FaceUpdateCommand::validateResponse(const ControlParameters& parameters) const
 {
-  this->validateRequest(parameters);
+  this->ControlCommand::validateResponse(parameters);
+
+  if (parameters.getFaceId() == 0) {
+    BOOST_THROW_EXCEPTION(ArgumentError("FaceId must not be zero"));
+  }
 }
 
 FaceDestroyCommand::FaceDestroyCommand()

src/management/nfd-control-command.hpp

@@ -171,6 +171,10 @@ public:
   virtual void
   validateRequest(const ControlParameters& parameters) const override;
 
+  /**
+   * \note This can only validate ControlParameters in a success response.
+   *       Failure responses should be validated with validateRequest.
+   */
   virtual void
   validateResponse(const ControlParameters& parameters) const override;
 };

tests/unit-tests/management/nfd-control-command.t.cpp

@@ -51,14 +51,14 @@ BOOST_AUTO_TEST_CASE(FaceCreate)
 
   ControlParameters p4;
   p4.setUri("tcp4://192.0.2.1")
-    .setFacePersistency(ndn::nfd::FacePersistency::FACE_PERSISTENCY_PERSISTENT)
+    .setFacePersistency(FACE_PERSISTENCY_PERSISTENT)
     .setFlags(0x3)
     .setMask(0x1);
   BOOST_CHECK_NO_THROW(command.validateRequest(p4));
 
   ControlParameters p5;
   p5.setUri("tcp4://192.0.2.1")
-    .setFacePersistency(ndn::nfd::FacePersistency::FACE_PERSISTENCY_PERSISTENT)
+    .setFacePersistency(FACE_PERSISTENCY_PERSISTENT)
     .setFlags(0x1);
   BOOST_CHECK_THROW(command.validateRequest(p5), ControlCommand::ArgumentError);
 
@@ -66,12 +66,12 @@ BOOST_AUTO_TEST_CASE(FaceCreate)
   BOOST_CHECK_NO_THROW(command.validateRequest(p4));
   command.applyDefaultsToRequest(p4);
   BOOST_REQUIRE(p4.hasFacePersistency());
-  BOOST_CHECK_EQUAL(p4.getFacePersistency(), ndn::nfd::FacePersistency::FACE_PERSISTENCY_PERSISTENT);
+  BOOST_CHECK_EQUAL(p4.getFacePersistency(), FACE_PERSISTENCY_PERSISTENT);
 
   ControlParameters p6;
   p6.setFaceId(4)
     .setUri("tcp4://192.0.2.1")
-    .setFacePersistency(ndn::nfd::FacePersistency::FACE_PERSISTENCY_PERSISTENT);
+    .setFacePersistency(FACE_PERSISTENCY_PERSISTENT);
   BOOST_CHECK_NO_THROW(command.validateResponse(p6));
 
   ControlParameters p7;
@@ -82,7 +82,7 @@ BOOST_AUTO_TEST_CASE(FaceCreate)
 
   ControlParameters p8;
   p8.setFaceId(5)
-    .setFacePersistency(ndn::nfd::FacePersistency::FACE_PERSISTENCY_PERMANENT)
+    .setFacePersistency(FACE_PERSISTENCY_PERMANENT)
     .setFlags(0x2);
   BOOST_CHECK_NO_THROW(command.validateResponse(p8));
 }
@@ -94,26 +94,30 @@ BOOST_AUTO_TEST_CASE(FaceUpdate)
   ControlParameters p1;
   p1.setFaceId(0);
   BOOST_CHECK_NO_THROW(command.validateRequest(p1));
-  BOOST_CHECK_NO_THROW(command.validateResponse(p1));
+  BOOST_CHECK_THROW(command.validateResponse(p1), ControlCommand::ArgumentError);
 
   p1.setFaceId(1);
   BOOST_CHECK_NO_THROW(command.validateRequest(p1));
-  BOOST_CHECK_NO_THROW(command.validateResponse(p1));
+  BOOST_CHECK_THROW(command.validateResponse(p1), ControlCommand::ArgumentError);
+  command.applyDefaultsToRequest(p1);
+  BOOST_CHECK_EQUAL(p1.getFaceId(), 1);
 
   ControlParameters p2;
   p2.setFaceId(1)
-    .setFacePersistency(ndn::nfd::FacePersistency::FACE_PERSISTENCY_PERSISTENT)
-    .setFlags(0x0)
-    .setMask(0x1);
+    .setFacePersistency(FACE_PERSISTENCY_PERSISTENT)
+    .setFlagBit(BIT_LOCAL_FIELDS_ENABLED, false);
   BOOST_CHECK_NO_THROW(command.validateRequest(p2));
+  BOOST_CHECK_THROW(command.validateResponse(p2), ControlCommand::ArgumentError); // Mask forbidden but present
+
+  // Flags without Mask
+  p2.unsetMask();
+  BOOST_CHECK_THROW(command.validateRequest(p2), ControlCommand::ArgumentError);
   BOOST_CHECK_NO_THROW(command.validateResponse(p2));
 
+  p2.setFlagBit(BIT_LOCAL_FIELDS_ENABLED, false);
   p2.unsetFaceId();
   BOOST_CHECK_NO_THROW(command.validateRequest(p2));
 
-  p2.unsetMask();
-  BOOST_CHECK_THROW(command.validateRequest(p2), ControlCommand::ArgumentError);
-
   ControlParameters p3;
   p3.setFaceId(1)
     .setName("/ndn/name");
@@ -127,7 +131,14 @@ BOOST_AUTO_TEST_CASE(FaceUpdate)
   BOOST_CHECK_THROW(command.validateResponse(p4), ControlCommand::ArgumentError);
 
   ControlParameters p5;
-  BOOST_CHECK_NO_THROW(command.validateResponse(p5));
+  BOOST_CHECK_NO_THROW(command.validateRequest(p5));
+  BOOST_CHECK_THROW(command.validateResponse(p5), ControlCommand::ArgumentError);
+  BOOST_CHECK(!p5.hasFaceId());
+  command.applyDefaultsToRequest(p5);
+  BOOST_REQUIRE(p5.hasFaceId());
+  BOOST_CHECK_NO_THROW(command.validateRequest(p5));
+  BOOST_CHECK_THROW(command.validateResponse(p5), ControlCommand::ArgumentError);
+  BOOST_CHECK_EQUAL(p5.getFaceId(), 0);
 }
 
 BOOST_AUTO_TEST_CASE(FaceDestroy)