Fix usage of the service component name (#437)

The service component name, if it existed, was not being used to generate the two extern "C" functions. If the component name is specified in the SCR meta-data, it should be used. This supports instantiating multiple service implementations with different service dependencies within the same bundle. Signed-off-by: The MathWorks, Inc. Roy.Lurie@mathworks.com
CppMicroServices · Jan 27, 2020 · 48f36a7 · 48f36a7
1 parent ce0d8bf
commit 48f36a7
Show file tree

Hide file tree

Showing 14 changed files with 265 additions and 81 deletions.
diff --git a/compendium/DeclarativeServices/src/manager/BundleLoader.cpp b/compendium/DeclarativeServices/src/manager/BundleLoader.cpp
@@ -23,9 +23,9 @@
 #include "BundleLoader.hpp"
 #include <regex>
 #if defined(_WIN32)
-#include <Windows.h>
+#  include <Windows.h>
 #else
-#include <dlfcn.h>
+#  include <dlfcn.h>
 #endif
 
 namespace cppmicroservices {
@@ -38,44 +38,41 @@ namespace scrimpl {
  */
 std::wstring UTF8StrToWStr(const std::string& inStr)
 {
-  if (inStr.empty())
-  {
+  if (inStr.empty()) {
     return std::wstring();
   }
   int wchar_count = MultiByteToWideChar(CP_UTF8, 0, inStr.c_str(), -1, NULL, 0);
   std::unique_ptr<wchar_t[]> wBuf(new wchar_t[wchar_count]);
-  wchar_count = MultiByteToWideChar(CP_UTF8, 0, inStr.c_str(), -1, wBuf.get(), wchar_count);
-  if (wchar_count == 0)
-  {
+  wchar_count =
+    MultiByteToWideChar(CP_UTF8, 0, inStr.c_str(), -1, wBuf.get(), wchar_count);
+  if (wchar_count == 0) {
     std::invalid_argument("Failed to convert " + inStr + " to UTF16.");
   }
   return wBuf.get();
 }
 #endif
 
-std::tuple<std::function<ComponentInstance*(void)>, std::function<void(ComponentInstance*)>>
-GetComponentCreatorDeletors(const std::string& compClassName,
+std::tuple<std::function<ComponentInstance*(void)>,
+           std::function<void(ComponentInstance*)>>
+GetComponentCreatorDeletors(const std::string& compName,
                             const cppmicroservices::Bundle& fromBundle)
 {
-  // cannot use bundle id as key because id is reused when the framework is restarted. 
-  // strings are not optimal but will work fine as long as a binary is not unloaded 
-  // from the process. 
+  // cannot use bundle id as key because id is reused when the framework is restarted.
+  // strings are not optimal but will work fine as long as a binary is not unloaded
+  // from the process.
   // Note: This code is a temporary hack until the core framework supports Bundle#load API.
-  static Guarded<std::map<std::string, void*>> bundleBinaries; ///< map of bundle location and handle pairs
+  static Guarded<std::map<std::string, void*>>
+    bundleBinaries; ///< map of bundle location and handle pairs
   const auto bundleLoc = fromBundle.GetLocation();
 
   void* handle = nullptr;
-  if(bundleBinaries.lock()->count(bundleLoc) != 0u)
-  {
+  if (bundleBinaries.lock()->count(bundleLoc) != 0u) {
     handle = bundleBinaries.lock()->at(bundleLoc);
-  }
-  else
-  {
+  } else {
 #if defined(_WIN32)
     std::wstring bundlePathWstr = UTF8StrToWStr(fromBundle.GetLocation());
     handle = reinterpret_cast<void*>(LoadLibraryW(bundlePathWstr.c_str()));
-    if(handle == nullptr)
-    {
+    if (handle == nullptr) {
       std::string errMsg("Unable to load bundle binary ");
       errMsg += fromBundle.GetLocation();
       errMsg += ". Error: ";
@@ -84,8 +81,7 @@ GetComponentCreatorDeletors(const std::string& compClassName,
     }
 #else
     handle = dlopen(fromBundle.GetLocation().c_str(), RTLD_LAZY | RTLD_LOCAL);
-    if(handle == nullptr)
-    {
+    if (handle == nullptr) {
       std::string errMsg("Unable to load bundle binary ");
       errMsg += fromBundle.GetLocation();
       errMsg += ". Error: ";
@@ -97,16 +93,16 @@ GetComponentCreatorDeletors(const std::string& compClassName,
     bundleBinaries.lock()->emplace(bundleLoc, handle);
   }
 
-  const std::string symbolName = std::regex_replace(compClassName, std::regex("::"), "_");
+  const std::string symbolName =
+    std::regex_replace(compName, std::regex("::"), "_");
   const std::string newInstanceFuncName("NewInstance_" + symbolName);
   const std::string deleteInstanceFuncName("DeleteInstance_" + symbolName);
 #if defined(_WIN32)
-  void* sym = reinterpret_cast<void*> (GetProcAddress(
+  void* sym = reinterpret_cast<void*>(GetProcAddress(
     reinterpret_cast<HMODULE>(handle), newInstanceFuncName.c_str()));
-  void* delsym = reinterpret_cast<void*> (GetProcAddress(
-    reinterpret_cast<HMODULE> (handle), deleteInstanceFuncName.c_str()));
-  if (sym == nullptr || delsym == nullptr)
-  {
+  void* delsym = reinterpret_cast<void*>(GetProcAddress(
+    reinterpret_cast<HMODULE>(handle), deleteInstanceFuncName.c_str()));
+  if (sym == nullptr || delsym == nullptr) {
     std::string errMsg("Unable to find entry-point functions in bundle ");
     errMsg += fromBundle.GetLocation();
     errMsg += ". Error code: ";
@@ -116,8 +112,7 @@ GetComponentCreatorDeletors(const std::string& compClassName,
 #else
   void* sym = dlsym(handle, newInstanceFuncName.c_str());
   void* delsym = dlsym(handle, deleteInstanceFuncName.c_str());
-  if (sym == nullptr || delsym == nullptr)
-  {
+  if (sym == nullptr || delsym == nullptr) {
     std::string errMsg("Unable to find entry-point functions in bundle ");
     errMsg += fromBundle.GetLocation();
     errMsg += ". Error: ";
@@ -127,9 +122,9 @@ GetComponentCreatorDeletors(const std::string& compClassName,
   }
 #endif
 
-  return std::make_tuple(reinterpret_cast<ComponentInstance*(*)(void)>(sym),      // NOLINT
-                         reinterpret_cast<void(*)(ComponentInstance*)>(delsym));  // NOLINT
-
+  return std::make_tuple(
+    reinterpret_cast<ComponentInstance* (*)(void)>(sym),     // NOLINT
+    reinterpret_cast<void (*)(ComponentInstance*)>(delsym)); // NOLINT
 }
 }
 }
diff --git a/compendium/DeclarativeServices/src/manager/BundleLoader.hpp b/compendium/DeclarativeServices/src/manager/BundleLoader.hpp
@@ -23,9 +23,9 @@
 #ifndef __BUNDLELOADER_HPP__
 #define __BUNDLELOADER_HPP__
 
-#include <map>
 #include "ConcurrencyUtil.hpp"
 #include "cppmicroservices/servicecomponent/detail/ComponentInstance.hpp"
+#include <map>
 
 using cppmicroservices::service::component::detail::ComponentInstance;
 //typedef ComponentInstance*(*NewComponentInstanceFuncPtr)();
@@ -38,7 +38,7 @@ namespace scrimpl {
  * delete {@link ComponentInstance} objects associated with a component from
  * a given {@link Bundle}
  *
- * \param compName is the fully qualified C++ class name of the component
+ * \param compName is a unique identifier for the component
  * \param fromBundle is the bundle where the component is located
  *
  * \throws \c std::runtime_error on failure to load the bundle binary.
@@ -47,8 +47,10 @@ namespace scrimpl {
  *         \c std::invalid_argument if location of \c fromBundle cannot be
  *         converted to UTF16 on the Windows platform
  */
-std::tuple<std::function<ComponentInstance*(void)>, std::function<void(ComponentInstance*)>> GetComponentCreatorDeletors(const std::string& compClassName,
-                                                                                                                         const cppmicroservices::Bundle& fromBundle);
+std::tuple<std::function<ComponentInstance*(void)>,
+           std::function<void(ComponentInstance*)>>
+GetComponentCreatorDeletors(const std::string& compName,
+                            const cppmicroservices::Bundle& fromBundle);
 }
 }
 #endif /* __BUNDLELOADER_HPP__ */
diff --git a/compendium/DeclarativeServices/src/manager/ComponentConfigurationImpl.cpp b/compendium/DeclarativeServices/src/manager/ComponentConfigurationImpl.cpp
@@ -237,7 +237,8 @@ std::shared_ptr<ComponentConfigurationState> ComponentConfigurationImpl::GetStat
 void ComponentConfigurationImpl::LoadComponentCreatorDestructor()
 {
   if(newCompInstanceFunc == nullptr || deleteCompInstanceFunc == nullptr) {
-    std::tie(newCompInstanceFunc, deleteCompInstanceFunc) = GetComponentCreatorDeletors(GetMetadata()->implClassName, GetBundle());
+    const auto compName = GetMetadata()->name.empty() ? GetMetadata()->implClassName : GetMetadata()->name;
+    std::tie(newCompInstanceFunc, deleteCompInstanceFunc) = GetComponentCreatorDeletors(compName, GetBundle());
   }
 }
 

diff --git a/compendium/DeclarativeServices/test/ImportTestBundles.cpp b/compendium/DeclarativeServices/test/ImportTestBundles.cpp
@@ -24,6 +24,8 @@ CPPMICROSERVICES_INITIALIZE_STATIC_BUNDLE(TestBundleDSTOI2)
 CPPMICROSERVICES_INITIALIZE_STATIC_BUNDLE(TestBundleDSTOI3)
 CPPMICROSERVICES_INITIALIZE_STATIC_BUNDLE(TestBundleDSTOI5)
 CPPMICROSERVICES_INITIALIZE_STATIC_BUNDLE(TestBundleDSTOI6)
+CPPMICROSERVICES_INITIALIZE_STATIC_BUNDLE(TestBundleDSTOI7)
+CPPMICROSERVICES_INITIALIZE_STATIC_BUNDLE(TestBundleDSTOI8)
 CPPMICROSERVICES_INITIALIZE_STATIC_BUNDLE(TestBundleDSTOI9)
 CPPMICROSERVICES_IMPORT_BUNDLE(declarative_services)
 #endif
diff --git a/compendium/DeclarativeServices/test/TestComponentConfigurationImpl.cpp b/compendium/DeclarativeServices/test/TestComponentConfigurationImpl.cpp
@@ -33,6 +33,9 @@
 #include "../src/manager/states/CCRegisteredState.hpp"
 #include "../src/manager/states/CCActiveState.hpp"
 
+#include "TestUtils.hpp"
+#include <TestInterfaces/Interfaces.hpp>
+
 using cppmicroservices::service::component::ComponentContext;
 
 namespace cppmicroservices {
@@ -595,5 +598,27 @@ TEST_F(ComponentConfigurationImplTest, TestGetDependencyManagers)
   EXPECT_NE(fakeCompConfig->GetDependencyManager("Foo"), nullptr);
   EXPECT_NE(fakeCompConfig->GetDependencyManager("Bar"), nullptr);
 }
+
+TEST_F(ComponentConfigurationImplTest, TestComponentWithUniqueName)
+{
+#if defined(US_BUILD_SHARED_LIBS)
+  auto dsPluginPath = test::GetDSRuntimePluginFilePath();
+  auto dsbundles = GetFramework().GetBundleContext().InstallBundles(dsPluginPath);
+  ASSERT_EQ(dsbundles.size(), 1);
+  for (auto& bundle : dsbundles) {
+    ASSERT_TRUE(bundle);
+    bundle.Start();
+  }
+#endif
+
+  auto testBundle = test::InstallAndStartBundle(GetFramework().GetBundleContext(), "TestBundleDSTOI8");
+  ASSERT_TRUE(testBundle);
+  ASSERT_EQ(testBundle.GetSymbolicName(), "TestBundleDSTOI8");
+
+  auto svcRef = testBundle.GetBundleContext().GetServiceReference<test::Interface1>();
+  ASSERT_TRUE(svcRef);
+  auto svc = testBundle.GetBundleContext().GetService<test::Interface1>(svcRef);
+  EXPECT_NE(svc, nullptr);
+}
 }
 }
diff --git a/compendium/test_bundles/CMakeLists.txt b/compendium/test_bundles/CMakeLists.txt
@@ -20,6 +20,7 @@ add_subdirectory(TestBundleDSTOI3)
 add_subdirectory(TestBundleDSTOI5)
 add_subdirectory(TestBundleDSTOI6)
 add_subdirectory(TestBundleDSTOI7)
+add_subdirectory(TestBundleDSTOI8)
 add_subdirectory(TestBundleDSTOI9)
 add_subdirectory(ISpellCheckService)
 add_subdirectory(IDictionaryService)

diff --git a/compendium/test_bundles/TestBundleDSTOI8/CMakeLists.txt b/compendium/test_bundles/TestBundleDSTOI8/CMakeLists.txt
@@ -0,0 +1,8 @@
+usFunctionCreateDSTestBundle(TestBundleDSTOI8)
+
+usFunctionCreateTestBundleWithResources(TestBundleDSTOI8
+  SOURCES src/ServiceImpl.cpp ${_glue_file}
+  RESOURCES manifest.json
+  BUNDLE_SYMBOLIC_NAME TestBundleDSTOI8
+  OTHER_LIBRARIES usTestInterfaces usServiceComponent usServiceComponent)
+
diff --git a/compendium/test_bundles/TestBundleDSTOI8/resources/manifest.json b/compendium/test_bundles/TestBundleDSTOI8/resources/manifest.json
@@ -0,0 +1,15 @@
+{
+    "bundle.symbolic_name" : "TestBundleDSTOI8",
+    "scr" : {
+        "version" : 1,
+        "components" : [{
+            "immediate": true,
+            "implementation-class": "sample::ServiceComponent8",
+            "name": "ServiceComponent8",
+            "service": {
+                "interfaces": ["test::Interface1"]
+            },
+            "inject-references": false
+        }]
+    }
+}
diff --git a/compendium/test_bundles/TestBundleDSTOI8/src/ServiceComponents.hpp b/compendium/test_bundles/TestBundleDSTOI8/src/ServiceComponents.hpp
@@ -0,0 +1,6 @@
+#ifndef SERVICECOMPONENTS_HPP
+#define SERVICECOMPONENTS_HPP
+
+#include "ServiceImpl.hpp"
+
+#endif
diff --git a/compendium/test_bundles/TestBundleDSTOI8/src/ServiceImpl.cpp b/compendium/test_bundles/TestBundleDSTOI8/src/ServiceImpl.cpp
@@ -0,0 +1,12 @@
+#include "ServiceImpl.hpp"
+#include <iostream>
+
+namespace sample {
+
+std::string ServiceComponent8::Description()
+{
+  std::string result(STRINGIZE(US_BUNDLE_NAME));
+  return result;
+}
+
+} // namespaces
diff --git a/compendium/test_bundles/TestBundleDSTOI8/src/ServiceImpl.hpp b/compendium/test_bundles/TestBundleDSTOI8/src/ServiceImpl.hpp
@@ -0,0 +1,22 @@
+#ifndef _SERVICE_IMPL_HPP_
+#define _SERVICE_IMPL_HPP_
+
+#include "cppmicroservices/servicecomponent/ComponentContext.hpp"
+#include "TestInterfaces/Interfaces.hpp"
+
+using ComponentContext = cppmicroservices::service::component::ComponentContext;
+
+namespace sample {
+
+class ServiceComponent8
+  : public test::Interface1
+{
+public:
+  ServiceComponent8() = default;
+  std::string Description() override;
+  ~ServiceComponent8() = default;
+};
+
+} // namespaces
+
+#endif // _SERVICE_IMPL_HPP_