Update to filter unknown enums (#3436)

* Add HMI warnings response for filtered enums

* Only filter enums with valid typing

Invalid types with an enum schema item were being silently filtered, so we need to look for a specific result code from validate() instead of looking for a general failure.

* Fix source for messages from hmis

* Fix unit tests

Co-authored-by: jacobkeeler <jacob.keeler@livioradio.com>

8 files changed, 59 insertions, 27 deletions

diff --git a/src/components/application_manager/include/application_manager/rpc_service_impl.h b/src/components/application_manager/include/application_manager/rpc_service_impl.h
index 289a0bcb66..a535198ca1 100644
--- a/src/components/application_manager/include/application_manager/rpc_service_impl.h
+++ b/src/components/application_manager/include/application_manager/rpc_service_impl.h
@@ -128,6 +128,9 @@ class RPCServiceImpl : public RPCService,
   bool ManageHMICommand(const commands::MessageSharedPtr message,
                         commands::Command::CommandSource source =
                             commands::Command::SOURCE_HMI) OVERRIDE;
+  bool ManageHMICommand(const commands::MessageSharedPtr message,
+                        commands::Command::CommandSource source,
+                        const std::string warning_info) OVERRIDE;
 
   // CALLED ON messages_to_hmi_ thread!
   void Handle(const impl::MessageToHmi message) OVERRIDE;
diff --git a/src/components/application_manager/src/commands/request_from_hmi.cc b/src/components/application_manager/src/commands/request_from_hmi.cc
index 1cf5c50cdd..eb9d914f00 100644
--- a/src/components/application_manager/src/commands/request_from_hmi.cc
+++ b/src/components/application_manager/src/commands/request_from_hmi.cc
@@ -96,7 +96,19 @@ void RequestFromHMI::SendResponse(
   }
 
   (*message)[strings::msg_params][strings::success] = success;
-  (*message)[strings::msg_params][strings::result_code] = result_code;
+  if ((result_code == hmi_apis::Common_Result::SUCCESS ||
+       result_code == hmi_apis::Common_Result::WARNINGS) &&
+      !warning_info().empty()) {
+    bool has_info = (*message)[strings::params].keyExists(strings::error_msg);
+    (*message)[strings::params][strings::error_msg] =
+        has_info ? (*message)[strings::params][strings::error_msg].asString() +
+                       "\n" + warning_info()
+                 : warning_info();
+    (*message)[strings::msg_params][strings::result_code] =
+        mobile_apis::Result::WARNINGS;
+  } else {
+    (*message)[strings::msg_params][strings::result_code] = result_code;
+  }
 
   rpc_service_.ManageHMICommand(message, source);
 }
diff --git a/src/components/application_manager/src/rpc_handler_impl.cc b/src/components/application_manager/src/rpc_handler_impl.cc
index bb8e4fc1fb..44a5af6f41 100644
--- a/src/components/application_manager/src/rpc_handler_impl.cc
+++ b/src/components/application_manager/src/rpc_handler_impl.cc
@@ -202,7 +202,8 @@ void RPCHandlerImpl::ProcessMessageFromHMI(
   }
 
   LOG4CXX_DEBUG(logger_, "Converted message, trying to create hmi command");
-  if (!app_manager_.GetRPCService().ManageHMICommand(smart_object)) {
+  if (!app_manager_.GetRPCService().ManageHMICommand(
+          smart_object, commands::Command::SOURCE_HMI, warning_info)) {
     LOG4CXX_ERROR(logger_, "Received command didn't run successfully");
   }
 }
diff --git a/src/components/application_manager/src/rpc_service_impl.cc b/src/components/application_manager/src/rpc_service_impl.cc
index d544c3b445..c7959c2386 100644
--- a/src/components/application_manager/src/rpc_service_impl.cc
+++ b/src/components/application_manager/src/rpc_service_impl.cc
@@ -332,6 +332,12 @@ bool RPCServiceImpl::ManageMobileCommand(
 
 bool RPCServiceImpl::ManageHMICommand(const commands::MessageSharedPtr message,
                                       commands::Command::CommandSource source) {
+  return ManageHMICommand(message, source, std::string());
+}
+
+bool RPCServiceImpl::ManageHMICommand(const commands::MessageSharedPtr message,
+                                      commands::Command::CommandSource source,
+                                      const std::string warning_info) {
   LOG4CXX_AUTO_TRACE(logger_);
 
   if (!message) {
@@ -381,6 +387,7 @@ bool RPCServiceImpl::ManageHMICommand(const commands::MessageSharedPtr message,
 
   if (kRequest == message_type) {
     LOG4CXX_DEBUG(logger_, "ManageHMICommand");
+    command->set_warning_info(warning_info);
     request_ctrl_.addHMIRequest(command);
   }
 
diff --git a/src/components/include/application_manager/rpc_service.h b/src/components/include/application_manager/rpc_service.h
index f98cd826f9..630070074a 100644
--- a/src/components/include/application_manager/rpc_service.h
+++ b/src/components/include/application_manager/rpc_service.h
@@ -53,7 +53,7 @@ class RPCService {
   /**
    * @brief ManageMobileCommand convert message to mobile command and execute it
    * @param message pointer to received message
-   * @param origin origin of command
+   * @param source source of command
    * @param warning_info warning message to send on a successful response. Only
    * applies to requests from mobile.
    * @return true if command is executed, otherwise return false
@@ -67,11 +67,17 @@ class RPCService {
   /**
    * @brief ManageHMICommand convert message to HMI command and execute it
    * @param message pointer to received message
+   * @param source source of command
+   * @param warning_info warning message to send on a successful response. Only
+   * applies to requests from HMI.
    * @return true if command is executed, otherwise return false
    */
   virtual bool ManageHMICommand(const commands::MessageSharedPtr message,
                                 commands::Command::CommandSource source =
                                     commands::Command::SOURCE_HMI) = 0;
+  virtual bool ManageHMICommand(const commands::MessageSharedPtr message,
+                                commands::Command::CommandSource source,
+                                const std::string warning_info) = 0;
 
   /**
    * @brief SendMessageToMobile Put message to the queue to be sent to mobile.
diff --git a/src/components/include/test/application_manager/mock_rpc_service.h b/src/components/include/test/application_manager/mock_rpc_service.h
index 66de650ab6..e9cf59e670 100644
--- a/src/components/include/test/application_manager/mock_rpc_service.h
+++ b/src/components/include/test/application_manager/mock_rpc_service.h
@@ -17,6 +17,11 @@ class MockRPCService : public application_manager::rpc_service::RPCService {
       ManageHMICommand,
       bool(const application_manager::commands::MessageSharedPtr message,
            application_manager::commands::Command::CommandSource source));
+  MOCK_METHOD3(
+      ManageHMICommand,
+      bool(const application_manager::commands::MessageSharedPtr message,
+           application_manager::commands::Command::CommandSource source,
+           const std::string warning_info));
   MOCK_METHOD2(
       ManageMobileCommand,
       bool(const application_manager::commands::MessageSharedPtr message,
diff --git a/src/components/smart_objects/include/smart_objects/enum_schema_item.h b/src/components/smart_objects/include/smart_objects/enum_schema_item.h
index 836a3faec1..de4c3adcc8 100644
--- a/src/components/smart_objects/include/smart_objects/enum_schema_item.h
+++ b/src/components/smart_objects/include/smart_objects/enum_schema_item.h
@@ -323,7 +323,7 @@ bool TEnumSchemaItem<EnumType>::filterInvalidEnums(
     const utils::SemanticVersion& MessageVersion,
     rpc::ValidationReport* report) {
   rpc::ValidationReport dummy_report("");
-  if (validate(Object, &dummy_report, MessageVersion) != errors::OK) {
+  if (validate(Object, &dummy_report, MessageVersion) == errors::OUT_OF_RANGE) {
     std::string validation_info =
         "Ignored invalid value - " + Object.asString();
     report->set_validation_info(validation_info);
@@ -338,19 +338,18 @@ errors::eType TEnumSchemaItem<EnumType>::validate(
     rpc::ValidationReport* report,
     const utils::SemanticVersion& MessageVersion,
     const bool allow_unknown_enums) {
-  if (SmartType_Integer != Object.getType()) {
-    std::string validation_info;
-    if (SmartType_String == Object.getType()) {
-      if (allow_unknown_enums) {
-        return errors::OK;
-      }
-      validation_info = "Invalid enum value: " + Object.asString();
-    } else {
-      validation_info =
-          "Incorrect type, expected: " +
-          SmartObject::typeToString(SmartType_Integer) +
-          " (enum), got: " + SmartObject::typeToString(Object.getType());
+  if (SmartType_String == Object.getType()) {
+    if (allow_unknown_enums) {
+      return errors::OK;
     }
+    std::string validation_info = "Invalid enum value: " + Object.asString();
+    report->set_validation_info(validation_info);
+    return errors::OUT_OF_RANGE;
+  } else if (SmartType_Integer != Object.getType()) {
+    std::string validation_info =
+        "Incorrect type, expected: " +
+        SmartObject::typeToString(SmartType_Integer) +
+        " (enum), got: " + SmartObject::typeToString(Object.getType());
     report->set_validation_info(validation_info);
     return errors::INVALID_VALUE;
   }
@@ -359,9 +358,8 @@ errors::eType TEnumSchemaItem<EnumType>::validate(
       mAllowedElements.find(static_cast<EnumType>(Object.asInt()));
 
   if (elements_it == mAllowedElements.end()) {
-    std::stringstream stream;
-    stream << "Invalid enum value: " << Object.asInt();
-    std::string validation_info = stream.str();
+    std::string validation_info =
+        "Invalid enum value: " + std::to_string(Object.asInt());
     report->set_validation_info(validation_info);
     return errors::OUT_OF_RANGE;
   }
@@ -380,7 +378,7 @@ errors::eType TEnumSchemaItem<EnumType>::validate(
                                         " removed for SyncMsgVersion " +
                                         MessageVersion.toString();
           report->set_validation_info(validation_info);
-          return errors::INVALID_VALUE;
+          return errors::OUT_OF_RANGE;
         } else if (signature.mSince == boost::none &&
                    signature.mUntil == boost::none) {
           // Element does not exist for this version
@@ -388,7 +386,7 @@ errors::eType TEnumSchemaItem<EnumType>::validate(
                                         " does not exist for SyncMsgVersion " +
                                         MessageVersion.toString();
           report->set_validation_info(validation_info);
-          return errors::INVALID_VALUE;
+          return errors::OUT_OF_RANGE;
         }
       }
     }
diff --git a/src/components/smart_objects/test/CObjectSchemaItem_test.cc b/src/components/smart_objects/test/CObjectSchemaItem_test.cc
index 477d950257..0eee0d1f0d 100644
--- a/src/components/smart_objects/test/CObjectSchemaItem_test.cc
+++ b/src/components/smart_objects/test/CObjectSchemaItem_test.cc
@@ -260,22 +260,22 @@ TEST_F(ObjectSchemaItemTest, validation_invalid_param) {
   obj[S_MSG_PARAMS][Keys::SUCCESS] = 0xABC;
 
   report = rpc::ValidationReport("RPC");
-  EXPECT_EQ(errors::INVALID_VALUE, schema_item->validate(obj, &report));
+  EXPECT_EQ(errors::OUT_OF_RANGE, schema_item->validate(obj, &report));
   EXPECT_NE(std::string(""), rpc::PrettyFormat(report));
 
   obj[S_PARAMS][S_FUNCTION_ID] = 1;
   report = rpc::ValidationReport("RPC");
-  EXPECT_EQ(errors::INVALID_VALUE, schema_item->validate(obj, &report));
+  EXPECT_EQ(errors::OUT_OF_RANGE, schema_item->validate(obj, &report));
   EXPECT_NE(std::string(""), rpc::PrettyFormat(report));
 
   obj[S_PARAMS][S_CORRELATION_ID] = -0xFF1;
   report = rpc::ValidationReport("RPC");
-  EXPECT_EQ(errors::INVALID_VALUE, schema_item->validate(obj, &report));
+  EXPECT_EQ(errors::OUT_OF_RANGE, schema_item->validate(obj, &report));
   EXPECT_NE(std::string(""), rpc::PrettyFormat(report));
 
   obj[S_PARAMS][S_PROTOCOL_VERSION] = 2;
   report = rpc::ValidationReport("RPC");
-  EXPECT_EQ(errors::INVALID_VALUE, schema_item->validate(obj, &report));
+  EXPECT_EQ(errors::OUT_OF_RANGE, schema_item->validate(obj, &report));
   EXPECT_NE(std::string(""), rpc::PrettyFormat(report));
 
   obj[S_MSG_PARAMS][Keys::RESULT_CODE] = 1;
@@ -442,7 +442,7 @@ TEST_F(ObjectSchemaItemTest, validation_unexpected_param_remove) {
   EXPECT_FALSE(obj[S_MSG_PARAMS].keyExists(fake3));
   // Invalide state after enum convertion
   report = rpc::ValidationReport("RPC");
-  EXPECT_EQ(errors::INVALID_VALUE, schema_item->validate(obj, &report));
+  EXPECT_EQ(errors::OUT_OF_RANGE, schema_item->validate(obj, &report));
   EXPECT_NE(std::string(""), rpc::PrettyFormat(report));
 }
 
@@ -469,7 +469,7 @@ TEST_F(ObjectSchemaItemTest, validation_empty_params) {
   schema_item->unapplySchema(obj);
   // Invalide state after enum convertion
   report = rpc::ValidationReport("RPC");
-  EXPECT_EQ(errors::INVALID_VALUE, schema_item->validate(obj, &report));
+  EXPECT_EQ(errors::OUT_OF_RANGE, schema_item->validate(obj, &report));
   EXPECT_NE(std::string(""), rpc::PrettyFormat(report));
 }