Merge pull request #741 from apache/feature/87-refactor-use-services

Feature/87 refactor use services

CHANGES.md

@@ -66,6 +66,9 @@ limitations under the License.
  The `celix_bundleContext_trackServicesWithOptions` is still available for more advanced use-cases.
 - Function `celix_bundle_destroyServiceTrackerList` is removed. The returned array list from 
  `celix_bundle_listServiceTrackers` is now configured to destroy the service trackers info entries.
+- It is no longer possible to use the `celix_bundleContext_useService*` functions or `celix::BundleContxt::useService*` 
+ methods on the Celix event thread. The calls will now immediately return and log an error if called on the
+ Celix event thread.
 
 ## New Features
 

bundles/remote_services/remote_service_admin_shm_v2/rsa_shm/gtest/src/RsaShmImplUnitTestSuite.cc

@@ -813,7 +813,6 @@ TEST_F(RsaShmRpcTestSuite, CallRemoteService) {
  EXPECT_EQ(CELIX_SUCCESS, calc->add(calc->handle, 1, 2, &result));
  EXPECT_EQ(3.0, result);
  };
- opts.flags = CELIX_SERVICE_USE_DIRECT | CELIX_SERVICE_USE_SOD;
  auto found = celix_bundleContext_useServiceWithOptions(clientCtx.get(), &opts);
  EXPECT_TRUE(found);
 
@@ -835,7 +834,6 @@ TEST_F(RsaShmRpcTestSuite, CallRemoteService) {
  double result;
  EXPECT_NE(CELIX_SUCCESS, calc->add(calc->handle, 1, 2, &result));
  };
- opts.flags = CELIX_SERVICE_USE_DIRECT | CELIX_SERVICE_USE_SOD;
  found = celix_bundleContext_useServiceWithOptions(clientCtx.get(), &opts);
  EXPECT_TRUE(found);
 

bundles/shell/shell/gtest/CMakeLists.txt

@@ -19,8 +19,10 @@ add_executable(test_shell
  src/ShellTestSuite.cc
 )
 
+add_celix_bundle(celix_shell_empty_resource_test_bundle NO_ACTIVATOR VERSION 0.0.0)
+
 target_link_libraries(test_shell PRIVATE Celix::framework Celix::shell_api GTest::gtest GTest::gtest_main)
-celix_target_bundle_set_definition(test_shell NAME TEST_BUNDLES Celix::shell)
+celix_target_bundle_set_definition(test_shell NAME TEST_BUNDLES Celix::shell celix_shell_empty_resource_test_bundle)
 target_compile_options(test_shell PRIVATE -Wno-deprecated-declarations)
 
 add_test(NAME test_shell COMMAND test_shell)
@@ -31,7 +33,7 @@ if (CELIX_CXX14)
  add_executable(test_cxx_shell src/ShellTestSuite.cc)
 
  target_link_libraries(test_cxx_shell PRIVATE Celix::framework Celix::shell_api GTest::gtest GTest::gtest_main)
- celix_target_bundle_set_definition(test_cxx_shell NAME TEST_BUNDLES Celix::ShellCxx)
+ celix_target_bundle_set_definition(test_cxx_shell NAME TEST_BUNDLES Celix::ShellCxx celix_shell_empty_resource_test_bundle)
  target_compile_definitions(test_cxx_shell PRIVATE -DCXX_SHELL)
 
  add_test(NAME test_cxx_shell COMMAND test_cxx_shell)

bundles/shell/shell/gtest/src/ShellTestSuite.cc

@@ -21,19 +21,20 @@
 #include <gtest/gtest.h>
 #include <thread>
 
-#include "celix_shell_command.h"
-#include "celix_framework_factory.h"
 #include "celix_bundle_context.h"
-#include "celix_shell.h"
-#include "celix_framework_utils.h"
 #include "celix_constants.h"
+#include "celix_framework_factory.h"
+#include "celix_framework_utils.h"
+#include "celix_shell.h"
+#include "celix_shell_command.h"
+#include "celix_stdlib_cleanup.h"
 
 class ShellTestSuite : public ::testing::Test {
 public:
  ShellTestSuite() : ctx{createFrameworkContext()} {
  auto* fw = celix_bundleContext_getFramework(ctx.get());
  size_t nr = celix_framework_utils_installBundleSet(fw, TEST_BUNDLES, true);
- EXPECT_EQ(nr, 1); //shell bundle
+ EXPECT_EQ(nr, 2); //shell and celix_shell_empty_resource_test_bundle bundle
  }
 
  static std::shared_ptr<celix_bundle_context_t> createFrameworkContext() {
@@ -54,12 +55,25 @@ class ShellTestSuite : public ::testing::Test {
  }};
  }
 
+ long getBundleIdForResourceBundle() const {
+ celix_autoptr(celix_array_list_t) bundles = celix_bundleContext_listBundles(ctx.get());
+ for (auto i = 0 ; i < celix_arrayList_size(bundles); ++i) {
+ auto bndId = celix_arrayList_getLong(bundles, i);
+ celix_autofree char* name = celix_bundleContext_getBundleSymbolicName(ctx.get(), bndId);
+ if (strstr(name, "resource") != nullptr) {
+ return bndId;
+ }
+
+ }
+ return -1;
+ }
+
  std::shared_ptr<celix_bundle_context_t> ctx;
 };
 
 TEST_F(ShellTestSuite, shellBundleInstalledTest) {
  auto *bndIds = celix_bundleContext_listBundles(ctx.get());
- EXPECT_EQ(1, celix_arrayList_size(bndIds));
+ EXPECT_EQ(2, celix_arrayList_size(bndIds));
  celix_arrayList_destroy(bndIds);
 }
 
@@ -71,8 +85,6 @@ static void callCommand(std::shared_ptr<celix_bundle_context_t>& ctx, const char
  celix_bundle_context_t* context{};
  long tracker{-1};
  };
- // Note that using celix_bundleContext_useServiceWithOptions to call command quit/stop may result in deadlock.
- // For more on this, see https://github.com/apache/celix/issues/629
  struct callback_data data{};
  data.cmdLine = cmdLine;
  data.cmdShouldSucceed = cmdShouldSucceed;
@@ -104,9 +116,10 @@ TEST_F(ShellTestSuite, testAllCommandsAreCallable) {
  callCommand(ctx, "non-existing", false);
  callCommand(ctx, "install a-bundle-loc.zip", true);
  callCommand(ctx, "help", true);
- callCommand(ctx, "help lb", false); //note need namespace
+ callCommand(ctx, "help lb", true);
  callCommand(ctx, "help celix::lb", true);
  callCommand(ctx, "help non-existing-command", false);
+ callCommand(ctx, "help celix::non-existing-command-with-namespace", false);
  callCommand(ctx, "lb", true);
  callCommand(ctx, "lb -l", true);
  callCommand(ctx, "query", true);
@@ -144,7 +157,8 @@ TEST_F(ShellTestSuite, stopFrameworkTest) {
 
 TEST_F(ShellTestSuite, stopSelfTest) {
  auto* list = celix_bundleContext_listBundles(ctx.get());
- EXPECT_EQ(1, celix_arrayList_size(list));
+ ASSERT_GE(celix_arrayList_size(list), 1);
+ auto nrOfBundles = celix_arrayList_size(list);
  long shellBundleId = celix_arrayList_getLong(list, 0);
  celix_arrayList_destroy(list);
 
@@ -156,24 +170,33 @@ TEST_F(ShellTestSuite, stopSelfTest) {
  std::this_thread::sleep_for(std::chrono::milliseconds{100});
 
  list = celix_bundleContext_listBundles(ctx.get());
- EXPECT_EQ(0, celix_arrayList_size(list));
+ EXPECT_EQ(nrOfBundles-1, celix_arrayList_size(list));
  celix_arrayList_destroy(list);
 }
 
 TEST_F(ShellTestSuite, queryTest) {
+ struct data {
+ long resourceBundleId;
+ };
+ data data{};
+ data.resourceBundleId = getBundleIdForResourceBundle();
+
  celix_service_use_options_t opts{};
  opts.filter.serviceName = CELIX_SHELL_COMMAND_SERVICE_NAME;
  opts.filter.filter = "(command.name=celix::query)";
  opts.waitTimeoutInSeconds = 1.0;
- opts.use = [](void */*handle*/, void *svc) {
+ opts.callbackHandle = (void*)&data;
+ opts.use = [](void* handle, void *svc) {
+ auto *d = static_cast<struct data*>(handle);
  auto *command = static_cast<celix_shell_command_t*>(svc);
- EXPECT_TRUE(command != nullptr);
+ ASSERT_TRUE(command != nullptr);
+ ASSERT_TRUE(d != nullptr);
 
  {
  char *buf = nullptr;
  size_t len;
  FILE *sout = open_memstream(&buf, &len);
- command->executeCommand(command->handle, (char *) "query", sout, sout);
+ command->executeCommand(command->handle, "query", sout, sout);
  fclose(sout);
  char* found = strstr(buf, "Provided services found 1"); //note could be 11, 12, etc
  EXPECT_TRUE(found != nullptr);
@@ -183,7 +206,8 @@ TEST_F(ShellTestSuite, queryTest) {
  char *buf = nullptr;
  size_t len;
  FILE *sout = open_memstream(&buf, &len);
- command->executeCommand(command->handle, (char *) "query 0", sout, sout); //note query framework bundle -> no results
+ auto cmd = std::string{"query "} + std::to_string(d->resourceBundleId);
+ command->executeCommand(command->handle, cmd.c_str(), sout, sout); //note query test resource bundle -> no results
  fclose(sout);
  char* found = strstr(buf, "No results"); //note could be 11, 12, etc
  EXPECT_TRUE(found != nullptr);

bundles/shell/shell/src/help_command.c

@@ -22,8 +22,9 @@
 #include <stdint.h>
 
 #include "celix_array_list.h"
-#include "celix_utils.h"
 #include "celix_shell.h"
+#include "celix_stdlib_cleanup.h"
+#include "celix_utils.h"
 #include "std_commands.h"
 
 struct print_handle {
@@ -51,11 +52,9 @@ static void printHelp(void *handle, void *svc) {
  sub = strtok_r(NULL, CELIX_SHELL_COMMAND_SEPARATOR, &save_ptr);
 
  if (sub == NULL) {
- unsigned int i;
  celix_array_list_t *commands = NULL;
-
  shell->getCommands(shell->handle, &commands);
- for (i = 0; i < celix_arrayList_size(commands); i++) {
+ for (int i = 0; i < celix_arrayList_size(commands); i++) {
  char *name = celix_arrayList_get(commands, i);
  fprintf(out, "%s\n", name);
  free(name);
@@ -71,17 +70,19 @@ static void printHelp(void *handle, void *svc) {
  shell->getCommands(shell->handle, &commands);
  bool cmdFound = false;
  for (i = 0; i < celix_arrayList_size(commands); i++) {
- char *name = celix_arrayList_get(commands, i);
- if (strcmp(sub, name) == 0) {
+ char *fqn = celix_arrayList_get(commands, i);
+ bool hasNamespace = strstr(fqn, "::") != NULL;
+ char* localName = hasNamespace ? strstr(fqn, "::") + 2 : fqn;
+ if (strcmp(sub, fqn) == 0 || (hasNamespace && strcmp(sub, localName) == 0)) {
  cmdFound = true;
  char *usage_str = NULL;
  char *desc_str = NULL;
 
- sub_status_desc = shell->getCommandDescription(shell->handle, name, &desc_str);
- sub_status_usage = shell->getCommandUsage(shell->handle, name, &usage_str);
+ sub_status_desc = shell->getCommandDescription(shell->handle, fqn, &desc_str);
+ sub_status_usage = shell->getCommandUsage(shell->handle, fqn, &usage_str);
 
  if (sub_status_usage == CELIX_SUCCESS && sub_status_desc == CELIX_SUCCESS) {
- fprintf(out, "Command : %s\n", name);
+ fprintf(out, "Command : %s\n", fqn);
  fprintf(out, "Usage : %s\n", usage_str == NULL ? "" : usage_str);
  fprintf(out, "Description : %s\n", desc_str == NULL ? "" : desc_str);
  } else {
@@ -91,7 +92,7 @@ static void printHelp(void *handle, void *svc) {
  free(usage_str);
  free(desc_str);
  }
- free(name);
+ free(fqn);
  }
  celix_arrayList_destroy(commands);
 
@@ -103,16 +104,23 @@ static void printHelp(void *handle, void *svc) {
  }
 }
 
-bool helpCommand_execute(void *handle, const char *const_cmdLine, FILE *out, FILE *err) {
- celix_bundle_context_t *ctx = handle;
- struct print_handle printHandle;
- char *cmdLine = celix_utils_strdup(const_cmdLine);
+bool helpCommand_execute(void* handle, const char* const_cmdLine, FILE* out, FILE* err) {
+ celix_bundle_context_t* ctx = handle;
+ celix_autofree char* cmdLine = celix_utils_strdup(const_cmdLine);
+
+ long trkId = celix_bundleContext_trackServices(ctx, CELIX_SHELL_SERVICE_NAME);
+ if (trkId < 0) {
+ fprintf(err, "Error tracking shell services\n");
+ return false;
+ }
 
+ struct print_handle printHandle;
  printHandle.cmdLine = cmdLine;
  printHandle.out = out;
  printHandle.err = err;
- bool called = celix_bundleContext_useService(ctx, CELIX_SHELL_SERVICE_NAME, &printHandle, printHelp);
+ bool called = celix_bundleContext_useTrackedService(ctx, trkId, &printHandle, printHelp);
+
+ celix_bundleContext_stopTracker(ctx, trkId);
 
- free(cmdLine);
  return called & printHandle.callSucceeded;
 }

examples/conan_test_package/test_shell.c

@@ -65,13 +65,10 @@ int main() {
  opts.filter.serviceName = CELIX_SHELL_SERVICE_NAME;
  opts.callbackHandle = NULL;
  opts.waitTimeoutInSeconds = 1.0;
- opts.flags = CELIX_SERVICE_USE_DIRECT | CELIX_SERVICE_USE_SOD;
  opts.use = use;
  bool called = celix_bundleContext_useServiceWithOptions(ctx, &opts);
  assert(called);
  celix_bundleContext_unregisterService(ctx, svcId);
  celix_frameworkFactory_destroyFramework(fw);
  return 0;
 }
-
-