Update based on comments

10 files changed, 56 insertions, 58 deletions

diff --git a/src/components/application_manager/include/application_manager/application.h b/src/components/application_manager/include/application_manager/application.h
index 0681761b5a..da5860f06b 100644
--- a/src/components/application_manager/include/application_manager/application.h
+++ b/src/components/application_manager/include/application_manager/application.h
@@ -127,9 +127,7 @@ class InitialApplicationData {
   virtual void set_language(const mobile_api::Language::eType& language) = 0;
   virtual void set_ui_language(
       const mobile_api::Language::eType& ui_language) = 0;
-  virtual void set_msg_version(const uint16_t major,
-                               const uint16_t minor,
-                               const uint16_t patch) = 0;
+  virtual void set_msg_version(const utils::SemanticVersion& version) = 0;
 };
 
 /*
diff --git a/src/components/application_manager/include/application_manager/application_data_impl.h b/src/components/application_manager/include/application_manager/application_data_impl.h
index c1056123ea..05091dbfbd 100644
--- a/src/components/application_manager/include/application_manager/application_data_impl.h
+++ b/src/components/application_manager/include/application_manager/application_data_impl.h
@@ -65,9 +65,7 @@ class InitialApplicationDataImpl : public virtual Application {
   void set_ngn_media_screen_name(const smart_objects::SmartObject& ngn_name);
   void set_language(const mobile_api::Language::eType& language);
   void set_ui_language(const mobile_api::Language::eType& ui_language);
-  void set_msg_version(const uint16_t major,
-                       const uint16_t minor,
-                       const uint16_t patch);
+  void set_msg_version(const utils::SemanticVersion& version);
 
   void set_perform_interaction_layout(
       mobile_api::LayoutMode::eType layout) OVERRIDE;
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 9b546a1340..4284b97786 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
@@ -298,14 +298,19 @@ void RegisterAppInterfaceRequest::Run() {
   uint16_t minor =
       msg_params[strings::sync_msg_version][strings::minor_version].asUInt();
   uint16_t patch = 0;
+  // Check if patch exists since it is not mandatory.
   if (msg_params[strings::sync_msg_version].keyExists(strings::patch_version)) {
     patch =
         msg_params[strings::sync_msg_version][strings::patch_version].asUInt();
   }
-  if (major < minimum_major_version ||
-      (major == minimum_major_version && minor < minimum_minor_version) ||
-      (major == minimum_major_version && minor == minimum_minor_version &&
-       patch < minimum_patch_version)) {
+
+  utils::SemanticVersion mobile_version(major, minor, patch);
+  utils::SemanticVersion min_module_version(
+      minimum_major_version, minimum_minor_version, minimum_patch_version);
+
+  if (mobile_version < min_module_version) {
+    LOG4CXX_WARN(logger_,
+                 "Application RPC Version does not meet minimum requirement");
     SendResponse(false, mobile_apis::Result::REJECTED);
   }
 
@@ -317,15 +322,14 @@ void RegisterAppInterfaceRequest::Run() {
   }
 
   // Version negotiation
-  utils::SemanticVersion mobile_version(major, minor, patch);
   utils::SemanticVersion module_version(
       major_version, minor_version, patch_version);
   if (mobile_version < module_version) {
     // Use mobile RPC version as negotiated version
-    application->set_msg_version(major, minor, patch);
+    application->set_msg_version(mobile_version);
   } else {
     // Use module version as negotiated version
-    application->set_msg_version(major_version, minor_version, patch_version);
+    application->set_msg_version(module_version);
   }
 
   // For resuming application need to restore hmi_app_id from resumeCtrl
diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/register_app_interface_request_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/register_app_interface_request_test.cc
index 0fcc1866ae..2518e9c7fd 100644
--- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/register_app_interface_request_test.cc
+++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/register_app_interface_request_test.cc
@@ -135,8 +135,6 @@ class RegisterAppInterfaceRequestTest
     ON_CALL(*mock_app, policy_app_id()).WillByDefault(Return(kAppId));
     ON_CALL(*mock_app, msg_version())
         .WillByDefault(ReturnRef(mock_semantic_version));
-    // EXPECT_CALL(*mock_app,
-    // msg_version()).WillOnce(Return(mock_semantic_version));
     return mock_app;
   }
 
diff --git a/src/components/application_manager/src/application_data_impl.cc b/src/components/application_manager/src/application_data_impl.cc
index 951acb68e9..ea7893ddc5 100644
--- a/src/components/application_manager/src/application_data_impl.cc
+++ b/src/components/application_manager/src/application_data_impl.cc
@@ -157,12 +157,9 @@ void InitialApplicationDataImpl::set_ui_language(
   ui_language_ = ui_language;
 }
 
-void InitialApplicationDataImpl::set_msg_version(const uint16_t major,
-                                                 const uint16_t minor,
-                                                 const uint16_t patch) {
-  msg_version_.major_version = major;
-  msg_version_.minor_version = minor;
-  msg_version_.patch_version = patch;
+void InitialApplicationDataImpl::set_msg_version(
+    const utils::SemanticVersion& version) {
+  msg_version_ = version;
 }
 
 void InitialApplicationDataImpl::set_perform_interaction_layout(
diff --git a/src/components/application_manager/test/include/application_manager/mock_application.h b/src/components/application_manager/test/include/application_manager/mock_application.h
index 79a268530a..602d228147 100644
--- a/src/components/application_manager/test/include/application_manager/mock_application.h
+++ b/src/components/application_manager/test/include/application_manager/mock_application.h
@@ -205,10 +205,7 @@ class MockApplication : public ::application_manager::Application {
                void(const mobile_apis::Language::eType& language));
   MOCK_METHOD1(set_ui_language,
                void(const mobile_apis::Language::eType& ui_language));
-  MOCK_METHOD3(set_msg_version,
-               void(const uint16_t major,
-                    const uint16_t minor,
-                    const uint16_t patch));
+  MOCK_METHOD1(set_msg_version, void(const utils::SemanticVersion& version));
   // DynamicApplicationData methods
   MOCK_CONST_METHOD0(help_prompt, const smart_objects::SmartObject*());
   MOCK_CONST_METHOD0(timeout_prompt, const smart_objects::SmartObject*());
diff --git a/src/components/formatters/include/formatters/CSmartFactory.h b/src/components/formatters/include/formatters/CSmartFactory.h
index e7eb91912f..d66dc56d8b 100644
--- a/src/components/formatters/include/formatters/CSmartFactory.h
+++ b/src/components/formatters/include/formatters/CSmartFactory.h
@@ -300,7 +300,8 @@ bool CSmartFactory<FunctionIdEnum, MessageTypeEnum, StructIdEnum>::attachSchema(
 
   object.setSchema(schemaIterator->second);
 
-  utils::SemanticVersion msg_version;
+  // Initialize msg_version to 0.0.0, an invalid value until properly set.
+  utils::SemanticVersion msg_version(0, 0, 0);
   if (object[NsSmartDeviceLink::NsJSONHandler::strings::S_PARAMS].keyExists(
           NsSmartDeviceLink::NsJSONHandler::strings::S_RPC_MSG_VERSION)) {
     msg_version =
diff --git a/src/components/include/utils/semantic_version.h b/src/components/include/utils/semantic_version.h
index 66cedc2bc5..a67cf6d2d1 100644
--- a/src/components/include/utils/semantic_version.h
+++ b/src/components/include/utils/semantic_version.h
@@ -60,13 +60,9 @@ struct SemanticVersion {
   }
 
   bool operator==(const SemanticVersion& version) const {
-    if (major_version == version.major_version &&
-        minor_version == version.minor_version &&
-        patch_version == version.patch_version) {
-      return true;
-    } else {
-      return false;
-    }
+    return (major_version == version.major_version &&
+            minor_version == version.minor_version &&
+            patch_version == version.patch_version);
   }
 
   bool operator<(const SemanticVersion& version) const {
diff --git a/src/components/smart_objects/src/object_schema_item.cc b/src/components/smart_objects/src/object_schema_item.cc
index 1eea823013..def0c0aceb 100644
--- a/src/components/smart_objects/src/object_schema_item.cc
+++ b/src/components/smart_objects/src/object_schema_item.cc
@@ -33,8 +33,6 @@
 
 #include <algorithm>
 
-#include <boost/algorithm/string.hpp>
-
 #include "smart_objects/always_false_schema_item.h"
 #include "smart_objects/smart_object.h"
 
@@ -59,24 +57,14 @@ CObjectSchemaItem::SMember::SMember(
     const std::vector<CObjectSchemaItem::SMember>& history_vector)
     : mSchemaItem(SchemaItem), mIsMandatory(IsMandatory) {
   if (Since.size() > 0) {
-    utils::SemanticVersion since_struct;
-    std::vector<std::string> since_fields;
-    boost::split(since_fields, Since, boost::is_any_of("."));
-    if (since_fields.size() == 3) {
-      since_struct.major_version = atoi(since_fields[0].c_str());
-      since_struct.minor_version = atoi(since_fields[1].c_str());
-      since_struct.patch_version = atoi(since_fields[2].c_str());
+    utils::SemanticVersion since_struct(Since);
+    if (since_struct.isValid()) {
       mSince = since_struct;
     }
   }
   if (Until.size() > 0) {
-    utils::SemanticVersion until_struct;
-    std::vector<std::string> until_fields;
-    boost::split(until_fields, Until, boost::is_any_of("."));
-    if (until_fields.size() == 3) {
-      until_struct.major_version = atoi(until_fields[0].c_str());
-      until_struct.minor_version = atoi(until_fields[1].c_str());
-      until_struct.patch_version = atoi(until_fields[2].c_str());
+    utils::SemanticVersion until_struct(Until);
+    if (until_struct.isValid()) {
       mUntil = until_struct;
     }
   }
diff --git a/src/components/smart_objects/test/CObjectSchemaItem_test.cc b/src/components/smart_objects/test/CObjectSchemaItem_test.cc
index c49a1bb302..5a0dcefa10 100644
--- a/src/components/smart_objects/test/CObjectSchemaItem_test.cc
+++ b/src/components/smart_objects/test/CObjectSchemaItem_test.cc
@@ -140,12 +140,32 @@ class ObjectSchemaItemTest : public ::testing::Test {
     // Create fake param that has breaking history changes
     std::vector<CObjectSchemaItem::SMember> fake_param_history_vector;
 
-    std::shared_ptr<ISchemaItem> fake_param_SchemaItem = CArraySchemaItem::create(CStringSchemaItem::create(TSchemaItemParameter<size_t>(1), TSchemaItemParameter<size_t>(99), TSchemaItemParameter<std::string>()), TSchemaItemParameter<size_t>(1), TSchemaItemParameter<size_t>(100));
-
-    std::shared_ptr<ISchemaItem> fake_param_history_v1_SchemaItem = CArraySchemaItem::create(CStringSchemaItem::create(TSchemaItemParameter<size_t>(1), TSchemaItemParameter<size_t>(99), TSchemaItemParameter<std::string>()), TSchemaItemParameter<size_t>(1), TSchemaItemParameter<size_t>(100));
-
-    fake_param_history_vector.push_back(CObjectSchemaItem::SMember(fake_param_history_v1_SchemaItem, true, "", "4.5.0", false, false));
-    schemaMembersMap["fakeParam"] = CObjectSchemaItem::SMember(fake_param_SchemaItem, false, "4.5.0", "", false, false, fake_param_history_vector);
+    std::shared_ptr<ISchemaItem> fake_param_SchemaItem =
+        CArraySchemaItem::create(
+            CStringSchemaItem::create(TSchemaItemParameter<size_t>(1),
+                                      TSchemaItemParameter<size_t>(99),
+                                      TSchemaItemParameter<std::string>()),
+            TSchemaItemParameter<size_t>(1),
+            TSchemaItemParameter<size_t>(100));
+
+    std::shared_ptr<ISchemaItem> fake_param_history_v1_SchemaItem =
+        CArraySchemaItem::create(
+            CStringSchemaItem::create(TSchemaItemParameter<size_t>(1),
+                                      TSchemaItemParameter<size_t>(99),
+                                      TSchemaItemParameter<std::string>()),
+            TSchemaItemParameter<size_t>(1),
+            TSchemaItemParameter<size_t>(100));
+
+    fake_param_history_vector.push_back(CObjectSchemaItem::SMember(
+        fake_param_history_v1_SchemaItem, true, "", "4.5.0", false, false));
+    schemaMembersMap["fakeParam"] =
+        CObjectSchemaItem::SMember(fake_param_SchemaItem,
+                                   false,
+                                   "4.5.0",
+                                   "",
+                                   false,
+                                   false,
+                                   fake_param_history_vector);
 
     CObjectSchemaItem::Members rootMembersMap;
     rootMembersMap[S_PARAMS] = CObjectSchemaItem::SMember(
@@ -180,7 +200,7 @@ TEST_F(ObjectSchemaItemTest, validation_correct_with_new_version) {
   obj[S_MSG_PARAMS][Keys::INFO] = "0123456789";
   obj[S_MSG_PARAMS][Keys::SUCCESS] = true;
 
-  utils::SemanticVersion messageVersion(4,5,0);
+  utils::SemanticVersion messageVersion(4, 5, 0);
   rpc::ValidationReport report("RPC");
   EXPECT_EQ(Errors::OK, schema_item->validate(obj, &report, messageVersion));
   EXPECT_EQ(std::string(""), rpc::PrettyFormat(report));
@@ -195,9 +215,10 @@ TEST_F(ObjectSchemaItemTest, validation_invalid_data_with_old_version) {
   obj[S_MSG_PARAMS][Keys::INFO] = "0123456789";
   obj[S_MSG_PARAMS][Keys::SUCCESS] = true;
 
-  utils::SemanticVersion messageVersion(3,0,0);
+  utils::SemanticVersion messageVersion(3, 0, 0);
   rpc::ValidationReport report("RPC");
-  EXPECT_EQ(Errors::MISSING_MANDATORY_PARAMETER, schema_item->validate(obj, &report, messageVersion));
+  EXPECT_EQ(Errors::MISSING_MANDATORY_PARAMETER,
+            schema_item->validate(obj, &report, messageVersion));
 }
 
 TEST_F(ObjectSchemaItemTest, validation_correct_skip_not_mandatory) {