From 316e5de5fc4eb3cc4e672b15a9e833adb35d6bd6 Mon Sep 17 00:00:00 2001 From: JackLivio Date: Mon, 15 Jun 2020 14:07:01 -0700 Subject: 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 --- .../include/application_manager/rpc_service_impl.h | 3 ++ .../src/commands/request_from_hmi.cc | 14 ++++++++- .../application_manager/src/rpc_handler_impl.cc | 3 +- .../application_manager/src/rpc_service_impl.cc | 7 +++++ .../include/application_manager/rpc_service.h | 8 ++++- .../test/application_manager/mock_rpc_service.h | 5 ++++ .../include/smart_objects/enum_schema_item.h | 34 ++++++++++------------ .../smart_objects/test/CObjectSchemaItem_test.cc | 12 ++++---- 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::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::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::validate( mAllowedElements.find(static_cast(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::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::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)); } -- cgit v1.2.1