From 73b34832829650834ea69b3606ce8127a93ef973 Mon Sep 17 00:00:00 2001 From: "Yaroslav Mamykin (GitHub)" <33784535+YarikMamykin@users.noreply.github.com> Date: Fri, 13 Sep 2019 17:48:27 +0300 Subject: Fix sdl versioning for vehicle data (#3015) * Fix versioning appliance for vehicle data SDL applied SyncMsgVersion=4.5.1 in case mobile app version was < 5.0 With this commit SDL applies SyncMsgVersion respectively to mobile app * Add unit test * Fix existing unit tests * fixup! Fix existing unit tests * fixup! Fix versioning appliance for vehicle data * fixup! Fix versioning appliance for vehicle data * fixup! Add unit test * fixup! Fix existing unit tests * fixup! Add unit test * fixup! Fix existing unit tests * fixup! Fix versioning appliance for vehicle data --- .../rc_get_interior_vehicle_data_consent_test.cc | 8 +- .../mobile/register_app_interface_request.cc | 6 +- .../application_manager/src/rpc_handler_impl.cc | 6 +- .../application_manager/src/rpc_service_impl.cc | 2 +- .../application_manager/test/CMakeLists.txt | 5 + .../test/rpc_handler_impl_test.cc | 117 +++++++++++++++++++++ .../include/smart_objects/object_schema_item.h | 2 +- .../smart_objects/src/object_schema_item.cc | 31 +++--- 8 files changed, 149 insertions(+), 28 deletions(-) create mode 100644 src/components/application_manager/test/rpc_handler_impl_test.cc (limited to 'src') diff --git a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/test/commands/rc_get_interior_vehicle_data_consent_test.cc b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/test/commands/rc_get_interior_vehicle_data_consent_test.cc index d44265a1d5..55be43f37b 100644 --- a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/test/commands/rc_get_interior_vehicle_data_consent_test.cc +++ b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/test/commands/rc_get_interior_vehicle_data_consent_test.cc @@ -265,15 +265,17 @@ TEST_F(RCGetInteriorVehicleDataConsentTest, TEST_F(RCGetInteriorVehicleDataConsentTest, Run_MobileSendButtonPressMessage_HMISendINUSEModeToMobile) { - // Arrange - auto mobile_message = CreateBasicMessage(); - // Expectations EXPECT_CALL(mock_allocation_manager_, AcquireResource(_, _, _)) .WillOnce(Return(rc_rpc_plugin::AcquireResult::IN_USE)); + auto msg_ver = utils::SemanticVersion(); + ON_CALL(*mock_app_, msg_version()).WillByDefault(ReturnRef(msg_ver)); + EXPECT_CALL(*optional_mock_rpc_plugin, GetCommandFactory()) .WillOnce(ReturnRef(mock_command_factory)); + + auto mobile_message = CreateBasicMessage(); auto rc_consent_response = CreateRCCommand( mobile_message); diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/register_app_interface_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/register_app_interface_request.cc index c4bc5cb486..4d41f69b49 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/register_app_interface_request.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/register_app_interface_request.cc @@ -383,11 +383,7 @@ void RegisterAppInterfaceRequest::Run() { // Version negotiation utils::SemanticVersion module_version( major_version, minor_version, patch_version); - if (mobile_version < utils::rpc_version_5) { - // Mobile versioning did not exist for - // versions before 5.0 - application->set_msg_version(utils::base_rpc_version); - } else if (mobile_version < module_version) { + if (mobile_version < module_version) { // Use mobile RPC version as negotiated version application->set_msg_version(mobile_version); } else { diff --git a/src/components/application_manager/src/rpc_handler_impl.cc b/src/components/application_manager/src/rpc_handler_impl.cc index d1ca05e9a1..3fee1da9e4 100644 --- a/src/components/application_manager/src/rpc_handler_impl.cc +++ b/src/components/application_manager/src/rpc_handler_impl.cc @@ -294,11 +294,11 @@ void RPCHandlerImpl::GetMessageVersion( if (sync_msg_version.keyExists(strings::patch_version)) { patch = sync_msg_version[strings::patch_version].asUInt(); } + + message_version = utils::base_rpc_version; utils::SemanticVersion temp_version(major, minor, patch); if (temp_version.isValid()) { - message_version = (temp_version >= utils::rpc_version_5) - ? temp_version - : utils::base_rpc_version; + message_version = temp_version; } } } diff --git a/src/components/application_manager/src/rpc_service_impl.cc b/src/components/application_manager/src/rpc_service_impl.cc index e29edd8f59..1bbc3256c7 100644 --- a/src/components/application_manager/src/rpc_service_impl.cc +++ b/src/components/application_manager/src/rpc_service_impl.cc @@ -181,7 +181,7 @@ bool RPCServiceImpl::ManageMobileCommand( #endif // ENABLE_SECURITY // Message for "CheckPermission" must be with attached schema - mobile_so_factory().attachSchema(*message, false); + mobile_so_factory().attachSchema(*message, true, app_ptr->msg_version()); } auto plugin = diff --git a/src/components/application_manager/test/CMakeLists.txt b/src/components/application_manager/test/CMakeLists.txt index f6146f96f9..230be524c8 100755 --- a/src/components/application_manager/test/CMakeLists.txt +++ b/src/components/application_manager/test/CMakeLists.txt @@ -86,6 +86,9 @@ set (RequestController_SOURCES ${AM_TEST_DIR}/mock_message_helper.cc ) +set (RPCHandlerImplTest_SOURCES ${AM_TEST_DIR}/rpc_handler_impl_test.cc + ${AM_TEST_DIR}/mock_message_helper.cc) + set(LIBRARIES Utils ApplicationManager @@ -144,6 +147,8 @@ create_test("help_prompt_manager_test" "${testSourcesHelpPromptManager}" "${LIBR create_test("request_controller_test" "${RequestController_SOURCES}" "${LIBRARIES}") +create_test("rpc_handler_impl_test" "${RPCHandlerImplTest_SOURCES}" "${LIBRARIES}") + set(ResumptionData_SOURCES ${AM_TEST_DIR}/resumption/resumption_data_test.cc ${AM_TEST_DIR}/resumption/resumption_data_db_test.cc diff --git a/src/components/application_manager/test/rpc_handler_impl_test.cc b/src/components/application_manager/test/rpc_handler_impl_test.cc new file mode 100644 index 0000000000..9db8e7c0d8 --- /dev/null +++ b/src/components/application_manager/test/rpc_handler_impl_test.cc @@ -0,0 +1,117 @@ +/* + * Copyright (c) 2019, Ford Motor Company + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * Redistributions of source code must retain the above copyright notice, this + * list of conditions and the following disclaimer. + * + * Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following + * disclaimer in the documentation and/or other materials provided with the + * distribution. + * + * Neither the name of the Ford Motor Company nor the names of its contributors + * may be used to endorse or promote products derived from this software + * without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + */ + +#include "application_manager/rpc_handler_impl.h" +#include +#include "application_manager/mock_application_manager.h" +#include "application_manager/smart_object_keys.h" +#include "interfaces/HMI_API.h" +#include "interfaces/MOBILE_API.h" + +namespace test { +namespace components { +namespace application_manager_test { + +using ::test::components::application_manager_test::MockApplicationManager; +using ::testing::_; +using ::testing::Mock; +using ::testing::NiceMock; +using namespace ::smart_objects; +using namespace application_manager::rpc_handler; + +class RPCHandlerImplTest : public ::testing::Test { + public: + RPCHandlerImplTest() + : rpc_handler_(new RPCHandlerImpl( + mock_app_mngr_, hmi_so_factory_, mobile_so_factory_)) {} + + protected: + std::unique_ptr rpc_handler_; + hmi_apis::HMI_API hmi_so_factory_; + mobile_apis::MOBILE_API mobile_so_factory_; + NiceMock mock_app_mngr_; +}; + +TEST_F(RPCHandlerImplTest, GetMessageVersion_SUCCESS) { + namespace json_str = ns_smart_device_link::ns_json_handler::strings; + namespace app_str = application_manager::strings; + + std::vector test_versions = { + utils::SemanticVersion("2.5.0"), + utils::SemanticVersion("3.0.0"), + utils::SemanticVersion("5.0.0"), + utils::SemanticVersion("6.1.0")}; + + SmartObject message; + message[json_str::S_MSG_PARAMS] = SmartObject(SmartType_Map); + message[json_str::S_MSG_PARAMS][app_str::sync_msg_version] = + SmartObject(SmartType_Map); + + for (const auto& expected_version : test_versions) { + message[json_str::S_MSG_PARAMS][app_str::sync_msg_version] + [app_str::major_version] = expected_version.major_version_; + message[json_str::S_MSG_PARAMS][app_str::sync_msg_version] + [app_str::minor_version] = expected_version.minor_version_; + message[json_str::S_MSG_PARAMS][app_str::sync_msg_version] + [app_str::patch_version] = expected_version.patch_version_; + + utils::SemanticVersion result_message_version; + rpc_handler_->GetMessageVersion(message, result_message_version); + EXPECT_EQ(expected_version, result_message_version); + } +} + +TEST_F(RPCHandlerImplTest, GetMessageVersion_InvalidVersion) { + namespace json_str = ns_smart_device_link::ns_json_handler::strings; + namespace app_str = application_manager::strings; + + SmartObject message; + message[json_str::S_MSG_PARAMS] = SmartObject(SmartType_Map); + message[json_str::S_MSG_PARAMS][app_str::sync_msg_version] = + SmartObject(SmartType_Map); + message[json_str::S_MSG_PARAMS][app_str::sync_msg_version] + [app_str::major_version] = 0; + message[json_str::S_MSG_PARAMS][app_str::sync_msg_version] + [app_str::minor_version] = 0; + message[json_str::S_MSG_PARAMS][app_str::sync_msg_version] + [app_str::patch_version] = 0; + + utils::SemanticVersion expected_version(utils::base_rpc_version); + + utils::SemanticVersion result_message_version; + rpc_handler_->GetMessageVersion(message, result_message_version); + EXPECT_EQ(expected_version, result_message_version); +} + +} // namespace application_manager_test +} // namespace components +} // namespace test diff --git a/src/components/smart_objects/include/smart_objects/object_schema_item.h b/src/components/smart_objects/include/smart_objects/object_schema_item.h index ee42b36620..5b1eb15078 100644 --- a/src/components/smart_objects/include/smart_objects/object_schema_item.h +++ b/src/components/smart_objects/include/smart_objects/object_schema_item.h @@ -86,7 +86,7 @@ struct SMember { boost::optional mSince; boost::optional mUntil; bool mIsDeprecated; - bool mIsRemoved; + mutable bool mIsRemoved; std::vector mHistoryVector; }; typedef std::map Members; diff --git a/src/components/smart_objects/src/object_schema_item.cc b/src/components/smart_objects/src/object_schema_item.cc index 01e4ec46e0..dee1a7c9c7 100644 --- a/src/components/smart_objects/src/object_schema_item.cc +++ b/src/components/smart_objects/src/object_schema_item.cc @@ -57,7 +57,8 @@ SMember::SMember(const ISchemaItemPtr SchemaItem, const bool IsDeprecated, const bool IsRemoved, const std::vector& history_vector) - : mSchemaItem(SchemaItem), mIsMandatory(IsMandatory) { + : mSchemaItem(SchemaItem) + , mIsMandatory(IsMandatory) { if (Since.size() > 0) { utils::SemanticVersion since_struct(Since); if (since_struct.isValid()) { @@ -251,25 +252,21 @@ CObjectSchemaItem::CObjectSchemaItem(const Members& members) void CObjectSchemaItem::RemoveFakeParams( SmartObject& Object, const utils::SemanticVersion& MessageVersion) { - for (SmartMap::const_iterator it = Object.map_begin(); - it != Object.map_end();) { + for (SmartMap::const_iterator it = Object.map_begin(); it != Object.map_end(); + ++it) { const std::string& key = it->first; std::map::const_iterator members_it = mMembers.find(key); - if (mMembers.end() == members_it - // FIXME(EZamakhov): Remove illegal usage of filed in AM - && key.compare(connection_key) != 0 && key.compare(binary_data) != 0 && - key.compare(app_id) != 0) { - ++it; - Object.erase(key); - } else if (mMembers.end() != members_it && - GetCorrectMember(members_it->second, MessageVersion) - .mIsRemoved) { - ++it; + if (mMembers.end() == members_it && key.compare(connection_key) != 0 && + key.compare(binary_data) != 0 && key.compare(app_id) != 0) { Object.erase(key); - } else { - ++it; + } + + if (mMembers.end() != members_it) { + if (GetCorrectMember(members_it->second, MessageVersion).mIsRemoved) { + Object.erase(key); + } } } } @@ -288,6 +285,10 @@ const SMember& CObjectSchemaItem::GetCorrectMember( } } } + + // If member didn't pass checks above then + // it becomes not valid and must be removed. + member.mIsRemoved = true; // Return member as default return member; } -- cgit v1.2.1