From e134f54fb755e31711306c491a988894cc71afd5 Mon Sep 17 00:00:00 2001
From: jacobkeeler <jacob.keeler@livioradio.com>
Date: Fri, 26 Apr 2019 15:48:56 -0400
Subject: Address review comments and fix unit tests

---
 .../application_manager/src/app_service_manager.cc | 37 ++++++++--------------
 .../test/app_service_manager_test.cc               |  9 ++++--
 2 files changed, 20 insertions(+), 26 deletions(-)

diff --git a/src/components/application_manager/src/app_service_manager.cc b/src/components/application_manager/src/app_service_manager.cc
index 397c4ee2fe..bab8147d9f 100644
--- a/src/components/application_manager/src/app_service_manager.cc
+++ b/src/components/application_manager/src/app_service_manager.cc
@@ -72,22 +72,16 @@ smart_objects::SmartObject AppServiceManager::PublishAppService(
 
   std::string service_type = manifest[strings::service_type].asString();
 
-  AppService* existing_service =
-      FindServiceByProvider(connection_key, service_type);
-  if (existing_service) {
-    LOG4CXX_DEBUG(logger_,
-                  "Service already exists for this provider, rejecting");
+  if (FindServiceByProvider(connection_key, service_type)) {
+    LOG4CXX_WARN(logger_,
+                 "Service already exists for this provider, rejecting");
     return smart_objects::SmartObject();
   }
 
-  if (manifest.keyExists(strings::service_name)) {
-    existing_service =
-        FindServiceByName(manifest[strings::service_name].asString());
-    if (existing_service) {
-      LOG4CXX_DEBUG(logger_,
-                    "A service already exists with this name, rejecting");
-      return smart_objects::SmartObject();
-    }
+  if (manifest.keyExists(strings::service_name) &&
+      FindServiceByName(manifest[strings::service_name].asString())) {
+    LOG4CXX_WARN(logger_, "A service already exists with this name, rejecting");
+    return smart_objects::SmartObject();
   }
 
   published_services_lock_.Acquire();
@@ -478,13 +472,11 @@ AppService* AppServiceManager::FindServiceByProvider(
     const uint32_t connection_key, const std::string service_type) {
   LOG4CXX_AUTO_TRACE(logger_);
   sync_primitives::AutoLock lock(published_services_lock_);
-
-  for (auto it = published_services_.begin(); it != published_services_.end();
-       ++it) {
-    if (it->second.connection_key == connection_key &&
-        it->second.record[strings::service_manifest][strings::service_type]
+  for (auto& service : published_services_) {
+    if (service.second.connection_key == connection_key &&
+        service.second.record[strings::service_manifest][strings::service_type]
                 .asString() == service_type) {
-      return &(it->second);
+      return &(service.second);
     }
   }
   return NULL;
@@ -493,11 +485,10 @@ AppService* AppServiceManager::FindServiceByProvider(
 AppService* AppServiceManager::FindServiceByName(std::string name) {
   LOG4CXX_AUTO_TRACE(logger_);
   sync_primitives::AutoLock lock(published_services_lock_);
-  for (auto it = published_services_.begin(); it != published_services_.end();
-       ++it) {
-    if (it->second.record[strings::service_manifest][strings::service_name]
+  for (auto& service : published_services_) {
+    if (service.second.record[strings::service_manifest][strings::service_name]
             .asString() == name) {
-      return &(it->second);
+      return &(service.second);
     }
   }
   return NULL;
diff --git a/src/components/application_manager/test/app_service_manager_test.cc b/src/components/application_manager/test/app_service_manager_test.cc
index 960e8e31f6..526bbbdb7d 100644
--- a/src/components/application_manager/test/app_service_manager_test.cc
+++ b/src/components/application_manager/test/app_service_manager_test.cc
@@ -120,8 +120,11 @@ class AppServiceManagerTest : public testing::Test {
   }
 
   smart_objects::SmartObject PublishService(
-      uint32_t connection_key = kConnectionKey, bool first_run = true) {
-    smart_objects::SmartObject manifest = GenerateMediaManifest(true);
+      uint32_t connection_key = kConnectionKey,
+      std::string service_name = kServiceName,
+      bool first_run = true) {
+    smart_objects::SmartObject manifest =
+        GenerateMediaManifest(true, service_name);
 
     Json::Value empty_json;
     EXPECT_CALL(mock_last_state_, get_dictionary())
@@ -326,7 +329,7 @@ TEST_F(AppServiceManagerTest, ActivateAppService_TwoApps_SUCCESS) {
   // Register two services with the same service type, the first is activated
   // automatically
   auto record = PublishService();
-  auto record2 = PublishService(kConnectionKey + 1, false);
+  auto record2 = PublishService(kConnectionKey + 1, kServiceName + "2", false);
 
   // No capability update
   smart_objects::SmartObject syscap_update_activated;
-- 
cgit v1.2.1