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

8 files changed, 149 insertions, 28 deletions

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<commands::RCGetInteriorVehicleDataConsentResponse>(
           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 <memory>
+#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<RPCHandlerImpl> rpc_handler_;
+  hmi_apis::HMI_API hmi_so_factory_;
+  mobile_apis::MOBILE_API mobile_so_factory_;
+  NiceMock<MockApplicationManager> 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<utils::SemanticVersion> 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<utils::SemanticVersion> mSince;
   boost::optional<utils::SemanticVersion> mUntil;
   bool mIsDeprecated;
-  bool mIsRemoved;
+  mutable bool mIsRemoved;
   std::vector<SMember> mHistoryVector;
 };
 typedef std::map<std::string, SMember> 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<SMember>& 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<std::string, SMember>::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;
 }