Address Comments

5 files changed, 41 insertions, 54 deletions

diff --git a/src/components/application_manager/src/rpc_handler_impl.cc b/src/components/application_manager/src/rpc_handler_impl.cc
index 0bf62cf64d..8b405cf61d 100644
--- a/src/components/application_manager/src/rpc_handler_impl.cc
+++ b/src/components/application_manager/src/rpc_handler_impl.cc
@@ -220,26 +220,32 @@ bool RPCHandlerImpl::ConvertMessageToSO(
 
       // Attach RPC version to SmartObject if it does not exist yet.
       auto app_ptr = app_manager_.application(message.connection_key());
-      utils::SemanticVersion msg_version(2, 0, 0);
+      utils::SemanticVersion msg_version(0, 0, 0);
       if (app_ptr &&
           (output[NsSmartDeviceLink::NsJSONHandler::strings::S_PARAMS]
                .keyExists(NsSmartDeviceLink::NsJSONHandler::strings::
                               S_RPC_MSG_VERSION) == false)) {
         msg_version = app_ptr->msg_version();
-        output[NsSmartDeviceLink::NsJSONHandler::strings::S_PARAMS]
-              [NsSmartDeviceLink::NsJSONHandler::strings::S_RPC_MSG_VERSION] =
-                  msg_version.toString();
       } else if (mobile_apis::FunctionID::RegisterAppInterfaceID ==
                  static_cast<mobile_apis::FunctionID::eType>(
                      output[strings::params][strings::function_id].asInt())) {
-        // Assume default version 2.0.0 until properly set in RAI
-        output[NsSmartDeviceLink::NsJSONHandler::strings::S_PARAMS]
-              [NsSmartDeviceLink::NsJSONHandler::strings::S_RPC_MSG_VERSION] =
-                  msg_version.toString();
+        if (output.keyExists(
+                NsSmartDeviceLink::NsJSONHandler::strings::S_MSG_PARAMS) &&
+                output[NsSmartDeviceLink::NsJSONHandler::strings::S_MSG_PARAMS]
+                    .keyExists(strings::sync_msg_version)) {
+          // SyncMsgVersion exists, check if it is valid.
+          std::string str_msg_version =
+              output[NsSmartDeviceLink::NsJSONHandler::strings::S_MSG_PARAMS]
+                    [strings::sync_msg_version].asString();
+          utils::SemanticVersion temp_version(str_msg_version);
+          if (temp_version.isValid()) {
+            msg_version = temp_version;
+          }
+        }
       }
 
       if (!conversion_result ||
-          !mobile_so_factory().attachSchema(output, true) ||
+          !mobile_so_factory().attachSchema(output, true, msg_version) ||
           ((output.validate(&report, msg_version) !=
             smart_objects::Errors::OK))) {
         LOG4CXX_WARN(logger_,
diff --git a/src/components/formatters/include/formatters/CSmartFactory.h b/src/components/formatters/include/formatters/CSmartFactory.h
index d66dc56d8b..a459f179c0 100644
--- a/src/components/formatters/include/formatters/CSmartFactory.h
+++ b/src/components/formatters/include/formatters/CSmartFactory.h
@@ -153,8 +153,10 @@ class CSmartFactory {
    *
    * @return True if operation was successful or false otherwise.
    */
-  bool attachSchema(NsSmartDeviceLink::NsSmartObjects::SmartObject& object,
-                    const bool RemoveFakeParameters);
+  bool attachSchema(
+      NsSmartDeviceLink::NsSmartObjects::SmartObject& object,
+      const bool RemoveFakeParameters,
+      const utils::SemanticVersion& MessageVersion = utils::SemanticVersion());
 
   /**
    * @brief Attach schema to the struct SmartObject.
@@ -273,7 +275,8 @@ CSmartFactory<FunctionIdEnum, MessageTypeEnum, StructIdEnum>::CSmartFactory(
 template <class FunctionIdEnum, class MessageTypeEnum, class StructIdEnum>
 bool CSmartFactory<FunctionIdEnum, MessageTypeEnum, StructIdEnum>::attachSchema(
     NsSmartDeviceLink::NsSmartObjects::SmartObject& object,
-    const bool RemoveFakeParameters) {
+    const bool RemoveFakeParameters,
+    const utils::SemanticVersion& MessageVersion) {
   if (false == object.keyExists(strings::S_PARAMS))
     return false;
   if (false == object[strings::S_PARAMS].keyExists(strings::S_MESSAGE_TYPE))
@@ -300,17 +303,8 @@ bool CSmartFactory<FunctionIdEnum, MessageTypeEnum, StructIdEnum>::attachSchema(
 
   object.setSchema(schemaIterator->second);
 
-  // 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 =
-        object[NsSmartDeviceLink::NsJSONHandler::strings::S_PARAMS]
-              [NsSmartDeviceLink::NsJSONHandler::strings::S_RPC_MSG_VERSION]
-                  .asString();
-  }
-
-  schemaIterator->second.applySchema(object, RemoveFakeParameters, msg_version);
+  schemaIterator->second.applySchema(
+      object, RemoveFakeParameters, MessageVersion);
 
   return true;
 }
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 2a4bd5c572..46656d1bd9 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
@@ -47,12 +47,6 @@
 namespace NsSmartDeviceLink {
 namespace NsSmartObjects {
 
-// Element Signature for enums. Fields represent "since", "until", and "removed"
-// typedef boost::optional<std::string> OptionalString;
-// typedef boost::optional<bool> OptionalBool;
-// typedef std::tuple<OptionalString, OptionalString, OptionalBool>
-// ElementSignature;
-
 struct ElementSignature {
   boost::optional<utils::SemanticVersion> mSince;
   boost::optional<utils::SemanticVersion> mUntil;
@@ -76,9 +70,6 @@ struct ElementSignature {
   }
 };
 
-// typedef std::map<OptionalString, OptionalString, OptionalBool>
-// ElementSignature;
-
 template <typename EnumType>
 class EnumConversionHelper;
 /**
@@ -341,30 +332,21 @@ const ElementSignature TEnumSchemaItem<EnumType>::getSignature(
   for (uint i = 0; i < signatures.size(); i++) {
     ElementSignature signature = signatures[i];
     // Check if signature matches message version
-    if (signature.mSince != boost::none) {
-      if (MessageVersion < signature.mSince.get()) {
-        // Msg version predates 'since' field, check next entry
-        continue;
-      }
-
-      if (signature.mUntil != boost::none &&
-          (MessageVersion >= signature.mUntil.get())) {
-        continue;  // Msg version newer than `until` field
-      }
-
-      return signature;
+    if (signature.mSince != boost::none &&
+        MessageVersion < signature.mSince.get()) {
+      // Msg version predates 'since' field, check next entry
+      continue;
     }
-
     if (signature.mUntil != boost::none &&
         (MessageVersion >= signature.mUntil.get())) {
       continue;  // Msg version newer than `until` field, check next entry
     }
-
+    // Found correct signature
     return signature;
   }
 
   // Could not match msg version to element siganture
-  ElementSignature ret("", "", true);
+  ElementSignature ret("", "", false);
   return ret;
 }
 
@@ -404,13 +386,23 @@ Errors::eType TEnumSchemaItem<EnumType>::validate(
     auto signatures_it = mElementSignatures.find(value);
     if (signatures_it != mElementSignatures.end()) {
       if (!signatures_it->second.empty()) {
-        if (getSignature(signatures_it->second, MessageVersion).mRemoved) {
+        ElementSignature signature =
+            getSignature(signatures_it->second, MessageVersion);
+        if (signature.mRemoved) {
           // Element was removed for this version
           std::string validation_info = "Enum value : " + Object.asString() +
                                         " removed for SyncMsgVersion " +
                                         MessageVersion.toString();
           report__->set_validation_info(validation_info);
           return Errors::INVALID_VALUE;
+        } else if (signature.mSince == boost::none &&
+                   signature.mUntil == boost::none) {
+          // Element does not exist for this version
+          std::string validation_info = "Enum value : " + Object.asString() +
+                                        " does not exist for SyncMsgVersion " +
+                                        MessageVersion.toString();
+          report__->set_validation_info(validation_info);
+          return Errors::INVALID_VALUE;
         }
       }
     }
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 aa807e98ab..1d42034317 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
@@ -173,7 +173,7 @@ class CObjectSchemaItem : public ISchemaItem {
                         const utils::SemanticVersion& MessageVersion);
 
   /**
-   * @brief Returns the correct schema item based on messge version.
+   * @brief Returns the correct schema item based on message version.
    * @param member Schema member
    * @param MmessageVersion Semantic Version of mobile message.
    **/
diff --git a/tools/InterfaceGenerator/generator/generators/SmartFactoryBase.py b/tools/InterfaceGenerator/generator/generators/SmartFactoryBase.py
index 81aefcac86..27b5a05f98 100755
--- a/tools/InterfaceGenerator/generator/generators/SmartFactoryBase.py
+++ b/tools/InterfaceGenerator/generator/generators/SmartFactoryBase.py
@@ -919,22 +919,17 @@ class CodeGenerator(object):
             member.removed is None and 
             member.history is None):
             return
-
         if (member.history is not None and member.since is None):
              raise GenerateError("Error: Missing since version parameter for " + member.name)
-
         if (member.until is not None): 
             raise GenerateError("Error: Until should only exist in history tag for " + member.name)
-
         if (member.history is None):
             if(member.until is not None or 
                 member.deprecated is not None or 
                 member.removed is not None):
                 raise GenerateError("Error: No history present for " + member.name)
-
         if (member.deprecated is not None and member.removed is not None):
             raise GenerateError("Error: Deprecated and removed should not be present together for " + member.name)
-
         if(member.history is not None):
             for item in member.history:
                 if item.since is None or item.until is None: