You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by bb...@apache.org on 2021/01/26 15:43:19 UTC

[geode-native] branch develop updated: GEODE-8735: Change factory logic for symbols lookup (#700)

This is an automated email from the ASF dual-hosted git repository.

bbender pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode-native.git


The following commit(s) were added to refs/heads/develop by this push:
     new c9da83a  GEODE-8735: Change factory logic for symbols lookup (#700)
c9da83a is described below

commit c9da83ab5aed349930cf7c4221a84b729e978dd6
Author: Mario Salazar de Torres <ma...@est.tech>
AuthorDate: Tue Jan 26 16:43:11 2021 +0100

    GEODE-8735: Change factory logic for symbols lookup (#700)
    
    * GEODE-8735: Change factory logic for symbols lookup
    
     - In order to allow that factory logic looks into the application space
       for symbols, `library-name` field has been made optional.
     - This removes the need to create shared library for in order to define
       custom factories like cache loaders, cache writers, cache listeners,
       partition resolvers or persistance managers whenever the cache is
       instantiated declaratively.
     - Whenever library-name is not specified a handle to the executable is
       obtained.
     - Take into account that for OS like Linux, Mac... in addition to exporting
       the symbols in the application, you would need to specify `-rdynamic`
       option while compiling.
     - Documentation has been updated.
    
    * GEODE-8735: Revision 2
    
     - Fixed application space factory functions. It seemed that loading factory
       functions from the application was not actually working.
     - Added an IT to verify that application space partition resolver factories
       are working.
     - Consequently an IT for shared library factories have been added to verify
       that the functionality was not working.
     - Added test factories to the testobject shared library.
     - Given GenerateExportHeader CMake module does not support executables and
       we need to set visibility attributes for the test factory functions inside
       cpp-integration-test, a new cmake module 'ExecutableExportHeader' has been
       added to support this.
     - In order to export symbols in a executable ENABLE_EXPORTS property needs to
       be specified for the target in CMake. Therefore, this property has been added
       for cpp-integration-test target.
    
    * GEODE-8735: Revision 3
    
     - Changed client cache XML schema and namespace to the right ones.
---
 cmake/ExecutableExportHeader.cmake                 | 29 +++++++++++
 cppcache/integration/test/CMakeLists.txt           |  5 ++
 cppcache/integration/test/CacheXmlTest.cpp         | 59 ++++++++++++++++++----
 .../test/resources/pr_app_client_cache.xml         | 29 +++++++++++
 .../test/resources/pr_lib_client_cache.xml         | 30 +++++++++++
 cppcache/src/CacheXmlParser.cpp                    |  2 +-
 cppcache/src/RegionAttributes.cpp                  |  3 +-
 cppcache/src/Utils.cpp                             | 13 +++--
 .../client-cache-ref.html.md.erb                   | 21 ++++++--
 tests/cpp/testobject/CMakeLists.txt                |  1 +
 tests/cpp/testobject/TestFactories.cpp             | 43 ++++++++++++++++
 11 files changed, 214 insertions(+), 21 deletions(-)

diff --git a/cmake/ExecutableExportHeader.cmake b/cmake/ExecutableExportHeader.cmake
new file mode 100644
index 0000000..9428d5f
--- /dev/null
+++ b/cmake/ExecutableExportHeader.cmake
@@ -0,0 +1,29 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+function(GENERATE_EXEC_EXPORT_HEADER TARGET)
+  get_property(type TARGET ${TARGET} PROPERTY TYPE)
+  if(NOT ${type} STREQUAL "EXECUTABLE")
+    message(WARNING "This macro can only be used with executables")
+    return()
+  endif()
+
+  include(GenerateExportHeader)
+
+  _test_compiler_hidden_visibility()
+  _test_compiler_has_deprecated()
+  _do_set_macro_values(${TARGET})
+  _do_generate_export_header(${TARGET} ${ARGN})
+endfunction()
\ No newline at end of file
diff --git a/cppcache/integration/test/CMakeLists.txt b/cppcache/integration/test/CMakeLists.txt
index 9aa4109..6ad08b8 100644
--- a/cppcache/integration/test/CMakeLists.txt
+++ b/cppcache/integration/test/CMakeLists.txt
@@ -65,6 +65,10 @@ if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
   )
 endif()
 
+include(ExecutableExportHeader)
+
+generate_exec_export_header(cpp-integration-test)
+
 target_compile_definitions(cpp-integration-test
   PUBLIC
     GTEST_ELLIPSIS_NEEDS_POD_
@@ -115,6 +119,7 @@ configure_file(
 set_target_properties(cpp-integration-test PROPERTIES
   CXX_VISIBILITY_PRESET hidden
   VISIBILITY_INLINES_HIDDEN ON
+  ENABLE_EXPORTS 1
   FOLDER cpp/test/integration
 )
 
diff --git a/cppcache/integration/test/CacheXmlTest.cpp b/cppcache/integration/test/CacheXmlTest.cpp
index 99c95a2..91b74c3 100644
--- a/cppcache/integration/test/CacheXmlTest.cpp
+++ b/cppcache/integration/test/CacheXmlTest.cpp
@@ -17,26 +17,24 @@
 
 #include <framework/Cluster.h>
 #include <framework/Framework.h>
-#include <framework/Gfsh.h>
 #include <framework/TestConfig.h>
-#include <hacks/range.h>
-
-#include <iostream>
-#include <unordered_map>
 
 #include <gtest/gtest.h>
 
 #include <geode/Cache.hpp>
+#include <geode/PartitionResolver.hpp>
 #include <geode/PoolManager.hpp>
-#include <geode/QueryService.hpp>
-#include <geode/RegionFactory.hpp>
-#include <geode/RegionShortcut.hpp>
-#include <geode/Struct.hpp>
+#include <geode/Region.hpp>
+
+#include "cpp-integration-test_export.h"
 
 namespace {
 
 using apache::geode::client::Cache;
+using apache::geode::client::CacheableKey;
 using apache::geode::client::CacheXmlException;
+using apache::geode::client::EntryEvent;
+using apache::geode::client::PartitionResolver;
 
 apache::geode::client::Cache createCacheUsingXmlConfig(
     const std::string& xmlFile) {
@@ -52,6 +50,24 @@ apache::geode::client::Cache createCacheUsingXmlConfig(
   return cache;
 }
 
+class TestAppPartitionResolver : public PartitionResolver {
+ public:
+  const std::string& getName() override {
+    static std::string name = "TestAppPartitionResolver";
+    return name;
+  }
+
+  std::shared_ptr<CacheableKey> getRoutingObject(const EntryEvent&) override {
+    return {};
+  }
+};
+}  // namespace
+
+extern "C" CPP_INTEGRATION_TEST_EXPORT PartitionResolver*
+CacheXmlTest_createAppPartitionResolver() {
+  return new TestAppPartitionResolver{};
+}
+
 /**
  * Example test using 2 servers and waiting for async tasks to synchronize using
  * furtures.
@@ -90,4 +106,27 @@ TEST(CacheXmlTest, loadCacheWithUnnamedPool) {
       .execute();
   EXPECT_THROW(createCacheUsingXmlConfig(cacheXml), CacheXmlException);
 }
-}  // namespace
+
+TEST(CacheXmlTest, testApplicationPartitionResolver) {
+  auto cache_xml = getFrameworkString(FrameworkVariable::NewTestResourcesDir) +
+                   std::string{"/pr_app_client_cache.xml"};
+  auto cache = createCacheUsingXmlConfig(cache_xml);
+
+  auto region = cache.getRegion("region");
+  auto pr = region->getAttributes().getPartitionResolver();
+
+  EXPECT_TRUE(pr);
+  EXPECT_EQ(pr->getName(), "TestAppPartitionResolver");
+}
+
+TEST(CacheXmlTest, testSharedLibPartitionResolver) {
+  auto cache_xml = getFrameworkString(FrameworkVariable::NewTestResourcesDir) +
+                   std::string{"/pr_lib_client_cache.xml"};
+  auto cache = createCacheUsingXmlConfig(cache_xml);
+
+  auto region = cache.getRegion("region");
+  auto pr = region->getAttributes().getPartitionResolver();
+
+  EXPECT_TRUE(pr);
+  EXPECT_EQ(pr->getName(), "TestLibPartitionResolver");
+}
diff --git a/cppcache/integration/test/resources/pr_app_client_cache.xml b/cppcache/integration/test/resources/pr_app_client_cache.xml
new file mode 100644
index 0000000..518d61e
--- /dev/null
+++ b/cppcache/integration/test/resources/pr_app_client_cache.xml
@@ -0,0 +1,29 @@
+<?xml version="1.0"?>
+
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<client-cache xmlns="http://geode.apache.org/schema/cpp-cache"
+			  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+			  xsi:schemaLocation="http://geode.apache.org/schema/cpp-cache http://geode.apache.org/schema/cpp-cache/cpp-cache-1.0.xsd"
+			  version="1.0"
+			  copy-on-read="true">
+	<region name="region">
+		<region-attributes refid="LOCAL">
+			<partition-resolver library-function-name="CacheXmlTest_createAppPartitionResolver"/>
+		</region-attributes>
+	</region>
+</client-cache>
\ No newline at end of file
diff --git a/cppcache/integration/test/resources/pr_lib_client_cache.xml b/cppcache/integration/test/resources/pr_lib_client_cache.xml
new file mode 100644
index 0000000..0e18540
--- /dev/null
+++ b/cppcache/integration/test/resources/pr_lib_client_cache.xml
@@ -0,0 +1,30 @@
+<?xml version="1.0"?>
+
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<client-cache xmlns="http://geode.apache.org/schema/cpp-cache"
+			  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+			  xsi:schemaLocation="http://geode.apache.org/schema/cpp-cache http://geode.apache.org/schema/cpp-cache/cpp-cache-1.0.xsd"
+			  version="1.0"
+			  copy-on-read="true">
+	<region name="region">
+		<region-attributes refid="LOCAL">
+			<partition-resolver library-name="testobject"
+								library-function-name="createTestLibPartitionResolver"/>
+		</region-attributes>
+	</region>
+</client-cache>
\ No newline at end of file
diff --git a/cppcache/src/CacheXmlParser.cpp b/cppcache/src/CacheXmlParser.cpp
index 32c4121..5e8e389 100644
--- a/cppcache/src/CacheXmlParser.cpp
+++ b/cppcache/src/CacheXmlParser.cpp
@@ -1049,7 +1049,7 @@ void CacheXmlParser::startExpirationAttributes(
 }
 
 std::string CacheXmlParser::getLibraryName(const xercesc::Attributes &attrs) {
-  return getRequiredAttribute(attrs, LIBRARY_NAME);
+  return getOptionalAttribute(attrs, LIBRARY_NAME);
 }
 
 std::string CacheXmlParser::getLibraryFunctionName(
diff --git a/cppcache/src/RegionAttributes.cpp b/cppcache/src/RegionAttributes.cpp
index 4276d71..5e5e4e3 100644
--- a/cppcache/src/RegionAttributes.cpp
+++ b/cppcache/src/RegionAttributes.cpp
@@ -109,7 +109,8 @@ std::shared_ptr<CacheListener> RegionAttributes::getCacheListener() const {
 
 std::shared_ptr<PartitionResolver> RegionAttributes::getPartitionResolver()
     const {
-  if (!m_partitionResolver && !m_partitionResolverLibrary.empty()) {
+  if (!m_partitionResolver && (!m_partitionResolverLibrary.empty() ||
+                               !m_partitionResolverFactory.empty())) {
     if (CacheXmlParser::managedPartitionResolverFn_ &&
         m_partitionResolverFactory.find('.') != std::string::npos) {
       // this is a managed library
diff --git a/cppcache/src/Utils.cpp b/cppcache/src/Utils.cpp
index 17227c5..f20d15f 100644
--- a/cppcache/src/Utils.cpp
+++ b/cppcache/src/Utils.cpp
@@ -26,6 +26,7 @@
 #include <ace/OS.h>
 #include <boost/asio.hpp>
 #include <boost/dll/import.hpp>
+#include <boost/dll/runtime_symbol_info.hpp>
 
 namespace apache {
 namespace geode {
@@ -173,20 +174,22 @@ const boost::dll::shared_library& getSharedLibrary(
 
   const auto& find = sharedLibraries.find(libraryName);
   if (find == sharedLibraries.end()) {
+    auto path = libraryName.empty() ? boost::dll::program_location()
+                                    : boost::dll::fs::path{libraryName};
     try {
       return *sharedLibraries
                   .emplace(
                       libraryName,
                       std::make_shared<boost::dll::shared_library>(
-                          libraryName,
+                          path,
                           boost::dll::load_mode::rtld_global |
                               boost::dll::load_mode::rtld_lazy |
                               boost::dll::load_mode::append_decorations |
                               boost::dll::load_mode::search_system_folders))
                   .first->second;
     } catch (const boost::dll::fs::system_error& e) {
-      throw IllegalArgumentException("cannot open library: " + libraryName +
-                                     ": reason: " + e.what());
+      throw IllegalArgumentException("cannot open library: \"" + path.string() +
+                                     "\": reason: " + e.what());
     }
   }
 
@@ -199,8 +202,10 @@ void* Utils::getFactoryFunctionVoid(const std::string& lib,
     const auto& sharedLibrary = getSharedLibrary(lib);
     return reinterpret_cast<void*>(sharedLibrary.get<void*()>(funcName));
   } catch (const boost::dll::fs::system_error&) {
+    std::string location =
+        lib.empty() ? "the application" : "library \"" + lib + "\"";
     throw IllegalArgumentException("cannot find factory function " + funcName +
-                                   " in library " + lib);
+                                   " in " + location);
   }
 }
 
diff --git a/docs/geode-native-docs-cpp/client-cache-ref.html.md.erb b/docs/geode-native-docs-cpp/client-cache-ref.html.md.erb
index a3b25ae..7e398a6 100644
--- a/docs/geode-native-docs-cpp/client-cache-ref.html.md.erb
+++ b/docs/geode-native-docs-cpp/client-cache-ref.html.md.erb
@@ -88,6 +88,12 @@ For example:
     <persistence-manager library-name="SqLiteImpl"
      library-function-name="createSqLiteInstance">
     ```
+Take into account that if a library name is not specified, the function name will be look for in the application.
+For example:
+
+    ```xml
+    <partition-resolver library-function-name="createPartitionResolver"/>
+    ```
 
 # Cache Initialization File Element Descriptions
 
@@ -353,7 +359,8 @@ Use the `<expiration-attributes>` sub-element to specify duration and expiration
 <a id="partition-resolver-ref"></a>
 ## \<partition-resolver\>
 
-\<partition-resolver\> identifies a function by specifying `library-name` and `library-function-name`.
+\<partition-resolver\> identifies a function by specifying `library-function-name` and optionally a `library-name`.
+Take into account that if `library-name` is not specified, the function will be looked for in the application itself.
 
 A partition resolver is used for single-hop access to partitioned region entries on the server side. This resolver
 implementation must match that of the `PartitionResolver` on the server side.
@@ -369,19 +376,22 @@ For example:
 <a id="cache-loader-ref"></a>
 ## \<cache-loader\>
 
-\<cache-loader\> identifies a cache loader function by specifying `library-name` and `library-function-name`.
+\<cache-loader\> identifies a cache loader function by specifying `library-function-name` and optionally a `library-name`.
+Take into account that if `library-name` is not specified, the function will be looked for in the application itself.
 See the [API Class Reference](/<%=vars.cppapiref_version%>/hierarchy.html) for the **CacheLoader** class.
 
 <a id="cache-listener-ref"></a>
 ## \<cache-listener\>
 
-\<cache-listener\> identifies a cache listener function by specifying `library-name` and `library-function-name`.
+\<cache-listener\> identifies a cache listener function by specifying `library-function-name` and optionally a `library-name`.
+Take into account that if `library-name` is not specified, the function will be looked for in the application itself.
 See the [API Class Reference](/<%=vars.cppapiref_version%>/hierarchy.html) for the **CacheListener** class.
 
 <a id="cache-writer-ref"></a>
 ## \<cache-writer\>
 
-\<cache-writer\> identifies a cache writer function by specifying `library-name` and `library-function-name`.
+\<cache-writer\> identifies a cache writer function by specifying `library-function-name` and optionally a `library-name`.
+Take into account that if `library-name` is not specified, the function will be looked for in the application itself.
 See the [API Class Reference](/<%=vars.cppapiref_version%>/hierarchy.html) for the **CacheWriter** class.
 
 <a id="persistence-manager-ref"></a>
@@ -391,7 +401,8 @@ For each region, if the disk-policy attribute is set to `overflows`, a persisten
 must perform cache-to-disk and disk-to-cache operations.
 See the [API Class Reference](/<%=vars.cppapiref_version%>/hierarchy.html) for the **PersistenceManager** class.
 
-\<persistence-manager\> identifies a persistence manager function by specifying `library-name` and `library-function-name`.
+\<persistence-manager\> identifies a persistence manager function by specifying `library-function-name` and optionally a `library-name`.
+Take into account that if `library-name` is not specified, the function will be looked for in the application itself.
 You can also specify a set of properties to be passed to the function as parameters.
 
 The sub-element `<properties>` is a sequence of 0 or more `<property>` elements.
diff --git a/tests/cpp/testobject/CMakeLists.txt b/tests/cpp/testobject/CMakeLists.txt
index c5cfeb5..fee4ee1 100644
--- a/tests/cpp/testobject/CMakeLists.txt
+++ b/tests/cpp/testobject/CMakeLists.txt
@@ -61,6 +61,7 @@ add_library(testobject SHARED
   PositionPdx.hpp
   PSTObject.cpp
   PSTObject.hpp
+  TestFactories.cpp
   TestObject1.cpp
   TestObject1.hpp
   TimestampedObject.hpp
diff --git a/tests/cpp/testobject/TestFactories.cpp b/tests/cpp/testobject/TestFactories.cpp
new file mode 100644
index 0000000..ff501ec
--- /dev/null
+++ b/tests/cpp/testobject/TestFactories.cpp
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <geode/PartitionResolver.hpp>
+
+#include "testobject_export.h"
+
+using apache::geode::client::CacheableKey;
+using apache::geode::client::EntryEvent;
+using apache::geode::client::PartitionResolver;
+
+namespace {
+class TestLibPartitionResolver : public PartitionResolver {
+ public:
+  const std::string& getName() override {
+    static std::string name = "TestLibPartitionResolver";
+    return name;
+  }
+
+  std::shared_ptr<CacheableKey> getRoutingObject(const EntryEvent&) override {
+    return {};
+  }
+};
+}  // namespace
+
+extern "C" TESTOBJECT_EXPORT PartitionResolver*
+createTestLibPartitionResolver() {
+  return new TestLibPartitionResolver{};
+}