Fix ListConfigurations returning empty config objects (#663)

* Fix ListConfigurations returning empty config objects

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

* Fix ListConfigurations

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

* address code review comments

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

Co-authored-by: Jeff <DiClemente>

compendium/ConfigurationAdmin/src/ConfigurationAdminImpl.cpp

@@ -345,7 +345,9 @@ ConfigurationAdminImpl::ListConfigurations(const std::string& filter)
     if (filter.empty()) {
       result.reserve(configurations.size());
       for (const auto& it : configurations) {
-        result.push_back(it.second);
+        if (!it.second->GetProperties().empty()) {
+          result.push_back(it.second);
+        }
       }
       return result;
     }
@@ -357,12 +359,16 @@ ConfigurationAdminImpl::ListConfigurations(const std::string& filter)
     };
 
     for (const auto& it : configurations) {
+      // empty configurations (those with an empty set of properties) cannot be
+      // returned.
+      if (it.second->GetProperties().empty()) {
+        continue;
+      }
       /* Create an AnyMap containing the pid or factoryPid so that the ldap filter 
-	 * functionality can be used to match the pid to the 
-	 * input parameter. Easy way to do the comparison since input parameter could 
-	 * contain a regular expression 
-	 */
-
+	   * functionality can be used to match the pid to the 
+	   * input parameter. Easy way to do the comparison since input parameter could 
+	   * contain a regular expression 
+	   */
       pidMap["pid"] = it.first;
 
       if (ldap.Match(pidMap)) {

compendium/ConfigurationAdmin/test/TestConfigurationAdminImpl.cpp

@@ -290,19 +290,34 @@ TEST_F(TestConfigurationAdminImpl, VerifyListConfigurations)
   const auto conf1 = configAdmin.GetConfiguration(pid1);
   const auto conf2 = configAdmin.GetConfiguration(pid2);
 
+  auto props1 = conf1->GetProperties();
+  props1["foo"] = std::string{ "baz" };
+  EXPECT_NO_THROW(conf1->Update(props1).get());
+
   auto props2 = conf2->GetProperties();
   props2["foo"] = std::string{ "bar" };
-  std::shared_future<void> fut;
-  EXPECT_NO_THROW(fut = conf2->Update(props2));
-  fut.get();
+  EXPECT_NO_THROW(conf2->Update(props2).get());
+
   const auto res1 = configAdmin.ListConfigurations();
   const auto res2 = configAdmin.ListConfigurations("(foo=bar)");
   const auto res3 = configAdmin.ListConfigurations("(foobar=baz)");
 
-  EXPECT_EQ(res1.size(), 2);
-  EXPECT_EQ(res2.size(), 1);
+  EXPECT_EQ(res1.size(), 2ul);
+  EXPECT_EQ(res2.size(), 1ul);
   EXPECT_EQ(res2[0]->GetPid(), pid2);
   EXPECT_TRUE(res3.empty());
+
+  // ListConfigurations should not return empty config objects (i.e. those with no properties)
+  const auto emptyConfig = configAdmin.GetConfiguration("test.pid.emptyconfig");
+  const auto emptyConfigResult =
+    configAdmin.ListConfigurations("(pid=test.pid.emptyconfig)");
+  EXPECT_TRUE(emptyConfigResult.empty());
+
+  // ListConfigurations should not return empty config objects when returning
+  // all available configs
+  const auto allConfigsResult =
+    configAdmin.ListConfigurations();
+  EXPECT_EQ(allConfigsResult.size(), 2ul);
 }
 
 TEST_F(TestConfigurationAdminImpl, VerifyAddConfigurations)

compendium/DeclarativeServices/test/TestUpdateConfiguration.cpp

@@ -77,6 +77,98 @@ TEST_F(tServiceComponent, testUpdateConfigBeforeStartingBundleResolvesService)
   ASSERT_EQ(compConfigs.at(0).state, scr::dto::ComponentState::ACTIVE)
     << "Component state should be ACTIVE";
 }
+
+TEST_F(tServiceComponent, testConcurrentUpdateConfigAndActivateService)
+{
+  cppmicroservices::BundleContext ctx = framework.GetBundleContext();
+
+  // Get a service reference to ConfigAdmin to create the configuration object.
+  auto configAdminService =
+    GetInstance<cppmicroservices::service::cm::ConfigurationAdmin>();
+  ASSERT_TRUE(configAdminService)
+    << "GetService failed for ConfigurationAdmin.";
+
+  std::promise<void> go;
+  std::shared_future<void> ready(go.get_future());
+  std::vector<std::promise<void>> readies(2);
+
+  auto updateConfigFuture = std::async(std::launch::async, [&ready, &readies, &configAdminService]() {
+      readies[0].set_value();
+      ready.wait();
+    auto configuration =
+      configAdminService->GetConfiguration("sample::ServiceComponentCA02");
+    configuration->UpdateIfDifferent(
+      std::unordered_map<std::string, cppmicroservices::Any>{
+        { "foo", true } });
+  });
+
+  auto activateServiceFuture = std::async(std::launch::async, [this, &ready, &readies, &ctx]() {
+      readies[1].set_value();
+      ready.wait();
+    auto testBundle = ::test::InstallAndStartBundle(ctx, "TestBundleDSCA02");
+    ASSERT_TRUE(testBundle);
+    auto instance = GetInstance<test::CAInterface>();
+    ASSERT_TRUE(instance) << "GetService failed for CAInterface";
+  });
+
+  readies[0].get_future().wait();
+  readies[1].get_future().wait();
+  go.set_value();
+  ASSERT_NO_THROW(updateConfigFuture.get());
+  ASSERT_NO_THROW(activateServiceFuture.get());
+}
+
+/**
+ * Tests that a component configuration which requires a configuration object
+ * is not satisfied by a default constructed, empty configuration object.
+ */
+TEST_F(tServiceComponent, testActivateServiceWithEmptyConfigProps)
+{
+  cppmicroservices::BundleContext ctx = framework.GetBundleContext();
+
+  const std::string svcComponentName{ "sample::ServiceComponentCA02" };
+
+  // Get a service reference to ConfigAdmin to create the configuration object.
+  auto configAdminService =
+    GetInstance<cppmicroservices::service::cm::ConfigurationAdmin>();
+  ASSERT_TRUE(configAdminService)
+    << "GetService failed for ConfigurationAdmin.";
+
+  auto configuration =
+        configAdminService->GetConfiguration("sample::ServiceComponentCA02");
+
+  auto testBundle = ::test::InstallAndStartBundle(ctx, "TestBundleDSCA02");
+  ASSERT_TRUE(testBundle);
+
+  scr::dto::ComponentDescriptionDTO compDescDTO;
+  auto compConfigs = GetComponentConfigs(testBundle, svcComponentName, compDescDTO);
+  EXPECT_EQ(compConfigs.size(), 1ul) << "One default config expected";
+  EXPECT_EQ(compConfigs.at(0).state,
+            scr::dto::ComponentState::UNSATISFIED_REFERENCE)
+    << "UNSATISFIED_REFERENCE is expected because configuration object is not "
+       "created.";
+
+  auto instance = GetInstance<test::CAInterface>();
+  ASSERT_FALSE(instance) << "GetService returned a valid service with an empty config for CAInterface";
+
+  configuration->UpdateIfDifferent(
+        std::unordered_map<std::string, cppmicroservices::Any>{
+          { "foo", true } }).second.get();
+
+  compConfigs = GetComponentConfigs(testBundle, svcComponentName, compDescDTO);
+  EXPECT_EQ(compConfigs.size(), 1ul) << "One default config expected.";
+  EXPECT_EQ(compConfigs.at(0).state, scr::dto::ComponentState::SATISFIED)
+    << "SATISFIED is exepected since configuration object is created.";
+
+  instance = GetInstance<test::CAInterface>();
+  ASSERT_TRUE(instance) << "GetService failed to return a service for CAInterface";
+
+  compConfigs = GetComponentConfigs(testBundle, svcComponentName, compDescDTO);
+  EXPECT_EQ(compConfigs.size(), 1ul) << "One default config expected.";
+  EXPECT_EQ(compConfigs.at(0).state, scr::dto::ComponentState::ACTIVE)
+    << "SATISFIED is exepected since configuration object is created.";
+}
+
 /*
  * Tests that if a configuration object is defined in the manifest.json file
  * the service that is dependent on the configuration object is resolved as soon
@@ -207,13 +299,7 @@ TEST_F(tServiceComponent, testUpdateConfigBeforeStartingBundleAndManifest)
   ASSERT_TRUE(result);
 
   // GetService to make component active
-  std::shared_ptr<test::CAInterface> instance;
-  startTime = std::chrono::steady_clock::now();
-  while (!instance &&
-         std::chrono::duration_cast<std::chrono::milliseconds>(
-           std::chrono::steady_clock::now() - startTime) <= TIMEOUT) {
-    instance = GetInstance<test::CAInterface>();
-  }
+  std::shared_ptr<test::CAInterface> instance = GetInstance<test::CAInterface>();
 
   ASSERT_TRUE(instance) << "GetService failed for CAInterface";