Multiple services and factory services in bundle dependent on same co…

…nfiguration pid (#698)

* Added new test bundle and test

Signed-off-by: The MathWorks, Inc. <alchrist@mathworks.com>

* Configuration Admin multiple notifications

When more than one service that implements the ManagedServiceInterface or ManagedServiceFactoryInterface is dependent on a configuration object, then all services dependent on that object should be updated when the configuration object is updated. Signed-off-by <pelliott@mathworks.com>

* Replaced while and for loops with more efficient constructs

Replaced while loop with for_each and replaced for loops with range based for loops. Signed-off-by The MathWorks, Inc. <pelliott@mathworks.com>

* Update ConfigurationAdminImpl.cpp

Added const to  parameter lists for new for_each statements. Signed-off-by The MathWorks, Inc. <pelliott@mathworks.com>

* Update ConfigurationAdminImpl.cpp

Check that managedServiceWrapper and managedServiceFactoryWrapper shared_ptr are not default constructed before dereferencing them. Signed-off-by The MathWorks, Inc. <pelliott@mathworks.com>

Co-authored-by: Alexander Christoforides <38366659+achristoforides@users.noreply.github.com>

compendium/ConfigurationAdmin/src/ConfigurationAdminImpl.cpp

@@ -544,13 +544,13 @@ void ConfigurationAdminImpl::RemoveConfigurations(
 std::shared_future<void> ConfigurationAdminImpl::NotifyConfigurationUpdated(
   const std::string& pid)
 {
-  // NotifyConfigurationUpdated will only send a notification to the service if 
+  // NotifyConfigurationUpdated will only send a notification to the service if
   // the configuration object has been updated at least once. In order to determine whether or not
   // a configuration object has been updated, it calls the HasBeenUpdatedAtLeastOnce method for
-  // the configuration object. For a remove operation the configuration object 
+  // the configuration object. For a remove operation the configuration object
   // is not available and that method cannot be called. For this reason, NotifyConfigurationUpdated
   // should not be called for Remove operations unless the caller has already confirmed
-  // the configuration object has been updated at least once. 
+  // the configuration object has been updated at least once.
   return PerformAsync([this, pid] {
     AnyMap properties{ AnyMap::UNORDERED_MAP_CASEINSENSITIVE_KEYS };
     std::string fPid;
@@ -563,19 +563,19 @@ std::shared_future<void> ConfigurationAdminImpl::NotifyConfigurationUpdated(
       if (it == std::end(configurations)) {
         removed = true;
         hasBeenUpdated = true;
-       } else {
+      } else {
         try {
-           hasBeenUpdated = it->second->HasBeenUpdatedAtLeastOnce();
-           properties = it->second->GetProperties();
-       } catch (const std::runtime_error&) {
+          hasBeenUpdated = it->second->HasBeenUpdatedAtLeastOnce();
+          properties = it->second->GetProperties();
+        } catch (const std::runtime_error&) {
           // Configuration is being removed
           removed = true;
         }
       }
     }
     // We can only send update notifications for configuration objects that have
     // been updated. Just return without sending the notification for objects
-    // that have not yet been updated. 
+    // that have not yet been updated.
     if (!hasBeenUpdated) {
       return;
     }
@@ -599,51 +599,49 @@ std::shared_future<void> ConfigurationAdminImpl::NotifyConfigurationUpdated(
     }
 
     const auto managedServiceWrappers = managedServiceTracker.GetServices();
-    const auto it = std::find_if(
-      std::begin(managedServiceWrappers),
-      std::end(managedServiceWrappers),
-      [&pid](const auto& managedServiceWrapper) {
-        // The ServiceTracker will return a default constructed shared_ptr for each ManagedService
-        // that we aren't tracking. We must be careful not to dereference these!
-        return (managedServiceWrapper ? (pid == managedServiceWrapper->pid)
-                                      : false);
-      });
-    if (it != std::end(managedServiceWrappers)) {
-      const auto& managedServiceWrapper = *it;
-      notifyServiceUpdated(
-        pid, *(managedServiceWrapper->trackedService), properties, *logger);
-    }
+    std::for_each(managedServiceWrappers.begin(),
+                  managedServiceWrappers.end(),
+                  [&](const auto& managedServiceWrapper) {
+                    // The ServiceTracker will return a default constructed shared_ptr for each ManagedService
+                    // that we aren't tracking. We must be careful not to dereference these!
+                    if ((managedServiceWrapper) && (managedServiceWrapper->pid == pid)) {
+                        notifyServiceUpdated(
+                          pid,
+                          *(managedServiceWrapper->trackedService),
+                          properties,
+                          *logger);
+                    }
+                  });
+
     const auto factoryPid = getFactoryPid(pid);
     if (factoryPid.empty()) {
       return;
     }
+
     const auto managedServiceFactoryWrappers =
       managedServiceFactoryTracker.GetServices();
-    const auto factoryIt = std::find_if(
-      std::begin(managedServiceFactoryWrappers),
-      std::end(managedServiceFactoryWrappers),
-      [&factoryPid](const auto& managedServiceFactoryWrapper) {
-        // The ServiceTracker will return a default constructed shared_ptr for each ManagedServiceFactory
-        // that we aren't tracking. We must be careful not to dereference these!
-        return (managedServiceFactoryWrapper
-                  ? (factoryPid == managedServiceFactoryWrapper->pid)
-                  : false);
-      });
-    if (factoryIt != std::end(managedServiceFactoryWrappers)) {
-      const auto& managedServiceFactoryWrapper = *factoryIt;
-      if (removed) {
-        notifyServiceRemoved(
-          pid, *(managedServiceFactoryWrapper->trackedService), *logger);
-      } else {
-        notifyServiceUpdated(pid,
-                             *(managedServiceFactoryWrapper->trackedService),
-                             properties,
-                             *logger);
-      }
-    }
+      std::for_each(managedServiceFactoryWrappers.begin(),
+                    managedServiceFactoryWrappers.end(),
+                    [&](const auto& managedServiceFactoryWrapper) {
+                    // The ServiceTracker will return a default constructed shared_ptr for each ManagedServiceFactory
+                    // that we aren't tracking. We must be careful not to dereference these!
+                       if ((managedServiceFactoryWrapper) && (managedServiceFactoryWrapper->pid == factoryPid)) {
+                         if (removed) {
+                            notifyServiceRemoved(
+                              pid,
+                              *(managedServiceFactoryWrapper->trackedService),
+                              *logger);
+                         } else {
+                            notifyServiceUpdated(
+                              pid,
+                              *(managedServiceFactoryWrapper->trackedService),
+                              properties,
+                              *logger);
+                         }
+                       }
+                    });
   });
 }
-
 std::shared_future<void> ConfigurationAdminImpl::NotifyConfigurationRemoved(
   const std::string& pid,
   std::uintptr_t configurationId)

compendium/ConfigurationAdmin/test/CMakeLists.txt

@@ -121,6 +121,7 @@ endif()
 set(_test_bundles
   ManagedServiceAndFactoryBundle
   TestBundleManagedServiceFactory
+  TestBundleMultipleManagedServiceFactory
   )
 
 target_link_libraries(${us_configurationadmin_test_exe_name}

compendium/ConfigurationAdmin/test/TestConfigAdmin.cpp

@@ -94,13 +94,37 @@ std::shared_ptr<::test::TestManagedServiceInterface> getManagedService(
   return ctx.GetService<::test::TestManagedServiceInterface>(sr);
 }
 
+std::vector<std::shared_ptr<::test::TestManagedServiceInterface>>
+getManagedServices(cppmicroservices::BundleContext& ctx)
+{
+  auto srs = ctx.GetServiceReferences<::test::TestManagedServiceInterface>();
+  std::vector<std::shared_ptr<::test::TestManagedServiceInterface>> services;
+  for (const auto& sr: srs) {
+    services.push_back( ctx.GetService<::test::TestManagedServiceInterface>(sr));
+  }
+
+  return services;
+}
+
 std::shared_ptr<::test::TestManagedServiceFactory> getManagedServiceFactory(
   cppmicroservices::BundleContext& ctx)
 {
   auto sr = ctx.GetServiceReference<::test::TestManagedServiceFactory>();
   return ctx.GetService<::test::TestManagedServiceFactory>(sr);
 }
 
+std::vector<std::shared_ptr<::test::TestManagedServiceFactory>>
+getManagedServiceFactories(cppmicroservices::BundleContext& ctx)
+{
+  auto srs = ctx.GetServiceReferences<::test::TestManagedServiceFactory>();
+  std::vector<std::shared_ptr<::test::TestManagedServiceFactory>> factories;
+  for (const auto& sr : srs) {
+    factories.push_back(ctx.GetService<::test::TestManagedServiceFactory>(sr));
+  }
+
+  return factories;
+}
+
 enum class PollingCondition
 {
   GT,
@@ -300,7 +324,7 @@ TEST_F(ConfigAdminTests, testServiceRemoved)
 
   auto const numBundles =
     installAndStartTestBundles(ctx, "ManagedServiceAndFactoryBundle");
-    ASSERT_EQ(numBundles, 1ul);
+  ASSERT_EQ(numBundles, 1ul);
 
   auto const service = getManagedService(ctx);
   ASSERT_NE(service, nullptr);
@@ -330,7 +354,7 @@ TEST_F(ConfigAdminTests, testServiceRemoved)
   configuration = m_configAdmin->GetConfiguration("cm.testservice");
   EXPECT_TRUE(configuration->GetProperties().empty());
   EXPECT_EQ(service->getCounter(), expectedCount);
- 
+
   const int newIncrement{ 5 };
   cppmicroservices::AnyMap props(
     cppmicroservices::AnyMap::UNORDERED_MAP_CASEINSENSITIVE_KEYS);
@@ -444,7 +468,7 @@ TEST_F(ConfigAdminTests, testCreateFactoryConfiguration)
 {
   auto f = GetFramework();
   auto ctx = f.GetBundleContext();
- 
+
   auto const numBundles =
     installAndStartTestBundles(ctx, "ManagedServiceAndFactoryBundle");
   ASSERT_EQ(numBundles, 1ul);
@@ -511,28 +535,29 @@ TEST_F(ConfigAdminTests, testRemoveFactoryConfiguration)
   EXPECT_EQ(serviceFactory->getUpdatedCounter("cm.testfactory~config2"),
             expectedCount_config2);
 }
-// This test confirms that if an object exists in the configuration repository 
+// This test confirms that if an object exists in the configuration repository
 // but has not yet been Updated prior to the start of the ManagedServiceFactory
-// then no Updated notification will be sent. 
+// then no Updated notification will be sent.
 TEST_F(ConfigAdminTests, testDuplicateUpdated)
 {
   auto f = GetFramework();
   auto ctx = f.GetBundleContext();
 
   // Add cm.testfactory~0 configuration object to the configuration repository
-  auto configuration = m_configAdmin->GetFactoryConfiguration("cm.testfactory","0");
+  auto configuration =
+    m_configAdmin->GetFactoryConfiguration("cm.testfactory", "0");
 
   auto configurationMap =
-      std::unordered_map<std::string, cppmicroservices::Any>{
-        { "emgrid", std::to_string(0) }
-      };
- 
-  // Start the ManagedServiceFactory for cm.testfactory. Since the 
-  // cm.testfactory~0 instance has not yet been updated, no Update 
+    std::unordered_map<std::string, cppmicroservices::Any>{
+      { "emgrid", std::to_string(0) }
+    };
+
+  // Start the ManagedServiceFactory for cm.testfactory. Since the
+  // cm.testfactory~0 instance has not yet been updated, no Update
   // notification will be sent to the ManagedServiceFactory.
   installAndStartTestBundles(ctx, "TestBundleManagedServiceFactory");
 
-  // Update the cm.testfactory~0 configuration object. An Update 
+  // Update the cm.testfactory~0 configuration object. An Update
   // notification will be sent to the ManagedServiceFactory.
   auto result = configuration->UpdateIfDifferent(configurationMap);
   result.second.get();
@@ -541,4 +566,87 @@ TEST_F(ConfigAdminTests, testDuplicateUpdated)
   ASSERT_NE(serviceFactory, nullptr);
 
   EXPECT_EQ(serviceFactory->getUpdatedCounter("cm.testfactory~0"), 1);
- }
+}
+
+TEST_F(ConfigAdminTests, testMultipleManagedServicesInBundleGetUpdate)
+{
+  auto f = GetFramework();
+  auto ctx = f.GetBundleContext();
+
+  auto const numBundles =
+    installAndStartTestBundles(ctx, "TestBundleMultipleManagedService");
+  ASSERT_EQ(numBundles, 1);
+
+  auto services = getManagedServices(ctx);
+  ASSERT_EQ(services.size(), 2ul);
+  std::for_each(std::begin(services),
+                std::end(services),
+                [](auto& service) { ASSERT_NE(service, nullptr); });
+
+  auto sharedConfiguration =
+    m_configAdmin->GetConfiguration("cm.testservice");
+  ASSERT_EQ(sharedConfiguration->GetPid(), "cm.testservice");
+  ASSERT_TRUE(sharedConfiguration->GetProperties().empty());
+
+  std::for_each(std::begin(services),
+                std::end(services),
+                [](auto& service) {
+                  ASSERT_EQ(service->getCounter(),
+                            0);
+                });
+
+  cppmicroservices::AnyMap props(
+    cppmicroservices::AnyMap::UNORDERED_MAP_CASEINSENSITIVE_KEYS);
+  props["myProp"] = 0;
+
+  auto fut = sharedConfiguration->Update(props);
+  fut.get();
+
+  std::for_each(std::begin(services),
+                std::end(services),
+                [](auto& service) {
+                  ASSERT_EQ(service->getCounter(),
+                            1);
+                });
+}
+TEST_F(ConfigAdminTests, testMultipleManagedFactoriesInBundleGetUpdate)
+{
+  auto f = GetFramework();
+  auto ctx = f.GetBundleContext();
+
+  auto const numBundles =
+    installAndStartTestBundles(ctx, "TestBundleMultipleManagedServiceFactory");
+  ASSERT_EQ(numBundles, 1);
+
+  auto serviceFactories = getManagedServiceFactories(ctx);
+  ASSERT_EQ(serviceFactories.size(), 2ul);
+  std::for_each(std::begin(serviceFactories),
+                std::end(serviceFactories),
+                [](auto& factory) { ASSERT_NE(factory, nullptr); });
+
+  auto sharedConfiguration =
+    m_configAdmin->GetFactoryConfiguration("cm.testfactory", "ver1");
+  ASSERT_EQ(sharedConfiguration->GetFactoryPid(), "cm.testfactory");
+  ASSERT_TRUE(sharedConfiguration->GetProperties().empty());
+
+  std::for_each(std::begin(serviceFactories),
+                std::end(serviceFactories),
+                [](auto& factory) {
+                  ASSERT_EQ(factory->getUpdatedCounter("cm.testfactory~ver1"),
+                            0);
+                });
+
+  cppmicroservices::AnyMap props(
+    cppmicroservices::AnyMap::UNORDERED_MAP_CASEINSENSITIVE_KEYS);
+  props["myProp"] = 0;
+
+  auto fut = sharedConfiguration->Update(props);
+  fut.get();
+
+  std::for_each(std::begin(serviceFactories),
+                std::end(serviceFactories),
+                [](auto& factory) {
+                  ASSERT_EQ(factory->getUpdatedCounter("cm.testfactory~ver1"),
+                            1);
+                });
+}

compendium/test_bundles/CMakeLists.txt

@@ -59,3 +59,5 @@ add_subdirectory(EnglishDictionary)
 
 add_subdirectory(ManagedServiceAndFactoryBundle)
 add_subdirectory(TestBundleManagedServiceFactory)
+add_subdirectory(TestBundleMultipleManagedService)
+add_subdirectory(TestBundleMultipleManagedServiceFactory)

compendium/test_bundles/TestBundleMultipleManagedService/CMakeLists.txt

@@ -0,0 +1,7 @@
+usFunctionCreateDSTestBundle(TestBundleMultipleManagedService)
+
+usFunctionCreateTestBundleWithResources(TestBundleMultipleManagedService
+  SOURCES src/ManagedServiceImpl.cpp ${_glue_file}
+  RESOURCES manifest.json
+  BUNDLE_SYMBOLIC_NAME TestBundleMultipleManagedService
+  OTHER_LIBRARIES usTestInterfaces usServiceComponent cm)

compendium/test_bundles/TestBundleMultipleManagedService/resources/manifest.json

@@ -0,0 +1,34 @@
+{
+  "bundle.symbolic_name": "TestBundleMultipleManagedService",
+  "bundle.name": "TestBundleMultipleManagedService",
+  "bundle.activator": false,
+  "scr": {
+    "version": 1,
+    "components": [
+      {
+        "implementation-class": "cppmicroservices::service::cm::test::TestManagedServiceImpl3",
+        "properties": {
+          "service.pid": "cm.testservice"
+        },
+        "service": {
+          "interfaces": [
+            "test::TestManagedServiceInterface",
+            "cppmicroservices::service::cm::ManagedService"
+          ]
+        }
+      },
+      {
+        "implementation-class": "cppmicroservices::service::cm::test::TestManagedServiceImpl4",
+        "properties": {
+          "service.pid": "cm.testservice"
+        },
+        "service": {
+          "interfaces": [
+            "test::TestManagedServiceInterface",
+            "cppmicroservices::service::cm::ManagedService"
+          ]
+        }
+      }
+    ]
+  }
+}