BugFix when creating instance name for factory components (#720)

The auto generation code uses the component name from the manifest.json file when creating the name of the constructor and destructor for the service instance. The factory component was using the name of the implementation class. This coding change fixes this bug so the name of the component is always used. The name of the component is an optional parameter in the manifest.json file. It defaults to the implementation class when not provided. The implementation class will be used for the instance name in those cases. Signed off by The MathWorks, Inc. <pelliott@mathworks.com>

compendium/DeclarativeServices/src/manager/ComponentConfigurationImpl.cpp

@@ -413,20 +413,10 @@ ComponentConfigurationImpl::GetState() const
 void ComponentConfigurationImpl::LoadComponentCreatorDestructor()
 {
   if (newCompInstanceFunc == nullptr || deleteCompInstanceFunc == nullptr) {
-    auto compName = GetMetadata()->name;
-    auto position = compName.find("~");
-
-    if ((position != std::string::npos)) {
-      // this is a factory pid. Use implementation class
-      // instead of name to construct the instance.
-      compName = GetMetadata()->implClassName;
-    } else {
-      compName = GetMetadata()->name.empty() ? GetMetadata()->implClassName
-                                             : GetMetadata()->name;
-    }
-
+    auto instanceName = GetMetadata()->instanceName;
+
     std::tie(newCompInstanceFunc, deleteCompInstanceFunc) =
-      GetComponentCreatorDeletors(compName, GetBundle(), logger);
+      GetComponentCreatorDeletors(instanceName, GetBundle(), logger);
   }
 }
 

compendium/DeclarativeServices/src/metadata/ComponentMetadata.hpp

@@ -48,6 +48,7 @@ struct ComponentMetadata
   {}
 
   std::string name;
+  std::string instanceName;
   bool enabled{ true };
   bool immediate{ false };
   std::string implClassName;

compendium/DeclarativeServices/src/metadata/MetadataParserImpl.cpp

@@ -137,6 +137,7 @@ MetadataParserImplV1::CreateComponentMetadata(const AnyMap& metadata) const
   compMetadata->name = compMetadata->implClassName;
   ObjectValidator(metadata, "name", /*isOptional=*/true)
     .AssignValueTo(compMetadata->name);
+  compMetadata->instanceName = compMetadata->name;
 
   // component.configuration-policy (Optional)
   compMetadata->configurationPolicy = CONFIG_POLICY_IGNORE;

compendium/DeclarativeServices/test/ImportTestBundles.cpp

@@ -39,6 +39,7 @@ CPPMICROSERVICES_INITIALIZE_STATIC_BUNDLE(TestBundleDSCA09)
 CPPMICROSERVICES_INITIALIZE_STATIC_BUNDLE(TestBundleDSCA12)
 CPPMICROSERVICES_INITIALIZE_STATIC_BUNDLE(TestBundleDSCA16)
 CPPMICROSERVICES_INITIALIZE_STATIC_BUNDLE(TestBundleDSCA20)
+CPPMICROSERVICES_INITIALIZE_STATIC_BUNDLE(TestBundleDSCA21)
 CPPMICROSERVICES_INITIALIZE_STATIC_BUNDLE(TestBundleDSCA24)
 CPPMICROSERVICES_INITIALIZE_STATIC_BUNDLE(TestBundleDSCA26)
 CPPMICROSERVICES_INITIALIZE_STATIC_BUNDLE(TestBundleDSCA27)

compendium/DeclarativeServices/test/TestFactoryPid.cpp

@@ -101,4 +101,42 @@ TEST_F(tServiceComponent, testFactoryPidConstruction)
     << "uniqueProp not found in constructed instance";
   EXPECT_EQ(uniqueProp->second, instanceId);
 }
+TEST_F(tServiceComponent, testFactoryPidConstructionNameDifferentThanClass)
+{
+
+  // Start the test bundle containing the factory component.
+  std::string factoryComponentName = "ServiceComponentName";
+  std::string factoryComponentClass = "sample::ServiceComponentCA21";
+  std::string configurationPid = "ServiceComponentPid";
+  cppmicroservices::Bundle testBundle = StartTestBundle("TestBundleDSCA21");
+
+
+  // Get a service reference to ConfigAdmin to create the factory component instance.
+  auto configAdminService =
+    GetInstance<cppmicroservices::service::cm::ConfigurationAdmin>();
+  ASSERT_TRUE(configAdminService) << "GetService failed for ConfigurationAdmin";
+
+  //Create factory configuration object
+  auto factoryConfig =
+    configAdminService->CreateFactoryConfiguration(configurationPid);
+  auto factoryInstance = factoryConfig->GetPid();
+
+  // CreateFactoryConfiguration created the configuration object on
+  // which the component is configured but it created it with no
+  // properties. Update the properties before instantiating the factory
+  // instance.
+  cppmicroservices::AnyMap props(
+    cppmicroservices::AnyMap::UNORDERED_MAP_CASEINSENSITIVE_KEYS);
+  const std::string instanceId{ "instance1" };
+  props["uniqueProp"] = instanceId;
+  auto fut = factoryConfig->Update(props);
+  fut.get();
+
+  //Request a service reference to the new component instance. This will
+  //cause DS to construct the instance with the updated properties.
+  auto instance = GetInstance<test::CAInterface>();
+  ASSERT_TRUE(instance) << "GetService failed for CAInterface";
+
+ }
 }
+

compendium/DeclarativeServices/test/TestFixture.hpp

@@ -105,6 +105,7 @@ class tServiceComponent : public testing::Test
     test::InstallLib(context, "TestBundleDSCA12");
     test::InstallLib(context, "TestBundleDSCA16");
     test::InstallLib(context, "TestBundleDSCA20");
+    test::InstallLib(context, "TestBundleDSCA21");
     test::InstallLib(context, "TestBundleDSCA24");
     test::InstallLib(context, "TestBundleDSCA26");
     test::InstallLib(context, "TestBundleDSCA27");

compendium/test_bundles/CMakeLists.txt

@@ -43,6 +43,7 @@ add_subdirectory(TestBundleDSCA09)
 add_subdirectory(TestBundleDSCA12)
 add_subdirectory(TestBundleDSCA16)
 add_subdirectory(TestBundleDSCA20)
+add_subdirectory(TestBundleDSCA21)
 add_subdirectory(TestBundleDSCA24)
 add_subdirectory(TestBundleDSCA26)
 add_subdirectory(TestBundleDSCA27)

compendium/test_bundles/TestBundleDSCA21/CMakeLists.txt

@@ -0,0 +1,8 @@
+usFunctionCreateDSTestBundle(TestBundleDSCA21)
+
+usFunctionCreateTestBundleWithResources(TestBundleDSCA21
+  SOURCES src/ServiceImpl.cpp ${_glue_file}
+  RESOURCES manifest.json
+  BUNDLE_SYMBOLIC_NAME TestBundleDSCA21
+  OTHER_LIBRARIES usTestInterfaces usServiceComponent)
+

compendium/test_bundles/TestBundleDSCA21/resources/manifest.json

@@ -0,0 +1,21 @@
+{
+    "bundle.symbolic_name" : "TestBundleDSCA21",
+    "scr" : {
+        "version" : 1,
+         "components" : [{
+            "name": "ServiceComponentName",
+            "enabled" : true,
+            "immediate": false,
+            "implementation-class": "sample::ServiceComponentCA21",
+            "configuration-policy" : "require",
+            "configuration-pid" : ["ServiceComponentPid"],
+            "factory" : "factory id",
+             "factory-properties" : {
+                 "abc" : "123"
+            },
+             "service": {
+                "interfaces": ["test::CAInterface"]
+            }
+        }]
+    }
+}

compendium/test_bundles/TestBundleDSCA21/src/ServiceComponents.hpp

@@ -0,0 +1,6 @@
+#ifndef SERVICECOMPONENTS_HPP
+#define SERVICECOMPONENTS_HPP
+
+#include "ServiceImpl.hpp"
+
+#endif

compendium/test_bundles/TestBundleDSCA21/src/ServiceImpl.cpp

@@ -0,0 +1,19 @@
+#include "ServiceImpl.hpp"
+#include <iostream>
+
+namespace sample {
+
+void ServiceComponentCA21::Modified(
+  const std::shared_ptr<ComponentContext>& /*context*/,
+  const std::shared_ptr<cppmicroservices::AnyMap>& configuration)
+{
+  std::lock_guard<std::mutex> lock(propertiesLock);
+  properties = configuration;
+}
+cppmicroservices::AnyMap ServiceComponentCA21::GetProperties()
+{
+  std::lock_guard<std::mutex> lock(propertiesLock);
+  return *properties;
+}
+
+}

compendium/test_bundles/TestBundleDSCA21/src/ServiceImpl.hpp

@@ -0,0 +1,29 @@
+#ifndef _SERVICE_IMPL_HPP_
+#define _SERVICE_IMPL_HPP_
+
+#include "TestInterfaces/Interfaces.hpp"
+#include "cppmicroservices/servicecomponent/ComponentContext.hpp"
+#include <mutex>
+
+using ComponentContext = cppmicroservices::service::component::ComponentContext;
+
+namespace sample {
+class ServiceComponentCA21 : public test::CAInterface
+{
+public:
+  ServiceComponentCA21(const std::shared_ptr<cppmicroservices::AnyMap>& props)
+    : properties(props)
+  {}
+  void Modified(const std::shared_ptr<ComponentContext>& context,
+                const std::shared_ptr<cppmicroservices::AnyMap>& configuration);
+  cppmicroservices::AnyMap GetProperties();
+
+  ~ServiceComponentCA21() = default;
+
+private:
+  std::mutex propertiesLock;
+  std::shared_ptr<cppmicroservices::AnyMap> properties;
+};
+}
+
+#endif // _SERVICE_IMPL_HPP_