Fix few issues in serializer plugin loader

- Refactor plugin loader interface to have explicit sharing of
  plugin data
- Fix crash in serializer plugin test
- Few code improvements
- Add extra environment variable support for protobuf plugins
  location

examples/addressbookserver/main.cpp

@@ -141,7 +141,7 @@ public:
         service->Requestcontacts(&ctx_, &request_, &writer_, cq_, cq_, &tag_);
         service->registerWriter(this);
     }
-    int tag_;
+    unsigned int tag_;
     grpc::ServerContext ctx_;
     ListFrame request_;
     ::grpc::ServerAsyncWriter< ::qtprotobuf::examples::Contacts> writer_;
@@ -172,7 +172,7 @@ void AddressBookService::registerCallStatusHandler(CallHandler *handler) {
     m_callClients.push_back(&(handler->writer_));
 }
 
-int main(int argc, char *argv[])
+int main(int, char *[])
 {
     std::string server_address("localhost:65001");
     AddressBookService service;
@@ -200,7 +200,7 @@ int main(int argc, char *argv[])
     while (true) {
         unsigned int *tag;
         bool ok;
-        cq->Next((void**)&tag, &ok);
+        cq->Next(reinterpret_cast<void**>(&tag), &ok);
         if (tag == nullptr) {
             std::cout << "Some request";
             continue;

examples/simplechatserver/main.cpp

@@ -106,7 +106,7 @@ public:
         service->RequestmessageList(&ctx_, &request_, &writer_, cq_, cq_, &tag_);
         service->loginUser(this);
     }
-    int tag_;
+    unsigned int tag_;
     grpc::ServerContext ctx_;
     None request_;
     ::grpc::ServerAsyncWriter< ::qtprotobuf::examples::ChatMessages> writer_;
@@ -118,7 +118,7 @@ void SimpleChatService::loginUser(MessageListHandler *handler) {
     //TODO: update online if required here
 }
 
-int main(int argc, char *argv[])
+int main(int, char *[])
 {
     std::string server_address("localhost:65002");
     SimpleChatService service;
@@ -145,10 +145,13 @@ int main(int argc, char *argv[])
     while (true) {
         unsigned int *tag;
         bool ok;
-        cq->Next((void**)&tag, &ok);
+        cq->Next(reinterpret_cast<void**>(&tag), &ok);
         if (tag == nullptr) {
             continue;
         }
+        if (!ok) {
+            std::cout << "0xdeadbeef done";
+        }
         if ((*tag) == 0xdeadbeef) {
             std::string name{""};
             std::string password{""};
@@ -162,6 +165,8 @@ int main(int argc, char *argv[])
             std::cout << "Authentication ok update user chat" << std::endl;
             last->writer_.Write(service.m_messages, nullptr);
             last = new MessageListHandler(&service, cq.get());
+        } else {
+            std::cout << "0xdeadbeef done";
         }
     }
 }

src/protobuf/CMakeLists.txt

@@ -62,7 +62,14 @@ protobuf_generate_qt_headers(PUBLIC_HEADERS ${PUBLIC_HEADERS} COMPONENT ${TARGET
 
 add_library(${TARGET} SHARED ${SOURCES})
 
-target_compile_definitions(QtProtobuf PUBLIC QT_PROTOBUF_PLUGIN_PATH=$<TARGET_FILE_DIR:serializationplugin>)
+execute_process(
+    COMMAND ${QT_QMAKE_EXECUTABLE} -query QT_INSTALL_PLUGINS
+    OUTPUT_VARIABLE QT_INSTALL_PLUGINS
+    OUTPUT_STRIP_TRAILING_WHITESPACE
+)
+
+set_target_properties(${TARGET} PROPERTIES QT_PROTOBUF_PLUGIN_PATH "${QT_INSTALL_PLUGINS}/protobuf")
+target_compile_definitions(${TARGET} PUBLIC QT_PROTOBUF_PLUGIN_PATH="${QT_INSTALL_PLUGINS}/protobuf")
 
 target_compile_definitions(${TARGET} PRIVATE QT_BUILD_PROTOBUF_LIB PUBLIC QTPROTOBUF_VERSION_MAJOR=${PROJECT_VERSION_MAJOR}
     QTPROTOBUF_VERSION_MINOR=${PROJECT_VERSION_MINOR})

src/protobuf/qprotobufserializationplugininterface.h

@@ -1,7 +1,7 @@
 /*
  * MIT License
  *
- * Copyright (c) 2019 Tatyana Borisova <tanusshhka@mail.ru>
+ * Copyright (c) 2019 Tatyana Borisova <tanusshhka@mail.ru>, Alexey Edelev <semlanik@gmail.com>
  *
  * This file is part of QtProtobuf project https://git.semlanik.org/semlanik/qtprotobuf
  *
@@ -39,10 +39,10 @@ namespace QtProtobuf {
 class Q_PROTOBUF_EXPORT QProtobufSerializationPluginInterface
 {
 public:
-    explicit QProtobufSerializationPluginInterface() {}
-    virtual ~QProtobufSerializationPluginInterface() {}
+    explicit QProtobufSerializationPluginInterface() = default;
+    virtual ~QProtobufSerializationPluginInterface() = default;
 
-    virtual QtProtobuf::QAbstractProtobufSerializer *serializer(const QString &serializer_name) = 0;
+    virtual std::shared_ptr<QtProtobuf::QAbstractProtobufSerializer> serializer(const QString &serializerName) = 0;
 };
 
 }

src/protobuf/qprotobufserializerregistry.cpp

@@ -23,6 +23,7 @@
  * DEALINGS IN THE SOFTWARE.
  */
 
+#include "qtprotobuflogging.h"
 #include "qprotobufserializerregistry_p.h"
 #include "qprotobufserializer.h"
 #include "qprotobufjsonserializer.h"
@@ -36,7 +37,6 @@
 #include <QJsonArray>
 
 namespace {
-const QLatin1String Serializationplugin("serializationplugin");
 const QLatin1String TypeNames("types");
 const QLatin1String MetaData("MetaData");
 const QLatin1String Version("version");
@@ -46,26 +46,32 @@ const QLatin1String PluginName("name");
 const QLatin1String ProtobufSerializer("protobuf");
 const QLatin1String JsonSerializer("json");
 #ifdef _WIN32
-const QLatin1String libExtension(".dll");
-const QLatin1String libPrefix("");
+const QLatin1String LibExtension(".dll");
+const QLatin1String LibPrefix("");
 #else
-const QLatin1String libExtension(".so");
-const QLatin1String libPrefix("lib");
+const QLatin1String LibExtension(".so");
+const QLatin1String LibPrefix("lib");
 #endif
 
+static const char *QtProtobufPluginPath = QT_PROTOBUF_PLUGIN_PATH;
+const QString DefaultImpl("Default");
 }
 
-#define STRINGIFY2(s) #s
-#define STRINGIFY(s) STRINGIFY2(s)
-static const char *QtProtobufPluginPath = STRINGIFY(QT_PROTOBUF_PLUGIN_PATH)"/";
 
 namespace QtProtobuf {
 
-class QProtobufSerializerRegistryPrivateRecord final
+struct QProtobufSerializerRegistryPrivateRecord final
 {
-public:
     QProtobufSerializerRegistryPrivateRecord() : loader(nullptr) {}
 
+    ~QProtobufSerializerRegistryPrivateRecord() {
+        serializers.clear();
+        if (loader && loader->isLoaded()) {
+            loader->unload();
+        }
+        delete loader;
+    }
+
     void createDefaultImpl()
     {
         if (serializers.find(ProtobufSerializer) == serializers.end()) {
@@ -76,18 +82,10 @@ public:
         }
     }
 
-    void loadPluginMetadata(const QString &path, const QString &pluginName)
+    void loadPluginMetadata(const QString &fullFilePath)
     {
-        // Load default plugin
-        if (path.isEmpty() || pluginName.isEmpty()){
-            libPath = QtProtobufPluginPath + libPrefix + Serializationplugin + libExtension;
-        }
-        // Use custom plugin
-        else {
-            libPath = path + libPrefix + pluginName + libExtension;
-        }
-
-        loader = new QPluginLoader(libPath);
+        libPath = fullFilePath;
+        loader = new QPluginLoader(fullFilePath);
         pluginData = loader->metaData();
 
         QVariantMap fullJson = pluginData.toVariantMap();
@@ -139,27 +137,37 @@ public:
         std::shared_ptr<QProtobufSerializerRegistryPrivateRecord> plugin = std::shared_ptr<QProtobufSerializerRegistryPrivateRecord>(new QProtobufSerializerRegistryPrivateRecord());
         plugin->createDefaultImpl();
         m_plugins[DefaultImpl] = plugin;
+        m_pluginPath = QString::fromUtf8(QtProtobufPluginPath);
+        QString envPluginPath = QString::fromUtf8(qgetenv("QT_PROTOBUF_PLUGIN_PATH"));
+        if (!envPluginPath.isEmpty()) {
+            m_pluginPath = envPluginPath;
+        }
     }
 
-    static const QString &loadPlugin(const QString &path, const QString &name)
+    QString loadPlugin(const QString &name)
     {
+        assert(!name.isEmpty());
+
         std::shared_ptr<QProtobufSerializerRegistryPrivateRecord> plugin = std::shared_ptr<QProtobufSerializerRegistryPrivateRecord>(new QProtobufSerializerRegistryPrivateRecord());
-        plugin->loadPluginMetadata(path, name);
+        QString libPath = m_pluginPath + QDir::separator() + LibPrefix + name + LibExtension;
+        plugin->loadPluginMetadata(libPath);
 
         const QString &pluginName = plugin->pluginLoadedName;
         if (m_plugins.find(pluginName) == m_plugins.end()) {
             plugin->loadPlugin();
             m_plugins[pluginName] = plugin;
+        } else {
+            plugin->loader = nullptr;
+            qProtoInfo() << "Serializer plugin with name" << pluginName << "is already loaded";
         }
         return pluginName;
     }
 
 
-    static std::unordered_map<QString/*pluginName*/, std::shared_ptr<QProtobufSerializerRegistryPrivateRecord>> m_plugins;
+    std::unordered_map<QString/*pluginName*/, std::shared_ptr<QProtobufSerializerRegistryPrivateRecord>> m_plugins;
+    QString m_pluginPath;
 };
 
-std::unordered_map<QString/*pluginName*/, std::shared_ptr<QProtobufSerializerRegistryPrivateRecord>> QProtobufSerializerRegistryPrivate::m_plugins;
-
 }
 
 using namespace QtProtobuf;
@@ -171,9 +179,14 @@ QProtobufSerializerRegistry::QProtobufSerializerRegistry() :
 
 QProtobufSerializerRegistry::~QProtobufSerializerRegistry() = default;
 
-const QString &QProtobufSerializerRegistry::loadPlugin(const QString &path, const QString &name)
+QString QProtobufSerializerRegistry::loadPlugin(const QString &name)
+{
+    return dPtr->loadPlugin(name);
+}
+
+std::shared_ptr<QAbstractProtobufSerializer> QProtobufSerializerRegistry::getSerializer(const QString &id)
 {
-    return dPtr->loadPlugin(path, name);
+    return dPtr->m_plugins[DefaultImpl]->serializers.at(id); //throws
 }
 
 std::shared_ptr<QAbstractProtobufSerializer> QProtobufSerializerRegistry::getSerializer(const QString &id, const QString &plugin)

src/protobuf/qprotobufserializerregistry_p.h

@@ -32,8 +32,6 @@
 #include <type_traits>
 #include <unordered_map>
 
-#include "qtprotobuflogging.h"
-#include "qprotobufselfcheckiterator.h"
 #include "qabstractprotobufserializer.h"
 
 #include "qtprotobufglobal.h"
@@ -41,8 +39,6 @@
 namespace QtProtobuf {
 
 class QProtobufSerializerRegistryPrivate;
-class QProtobufSerializerRegistryPrivateRecord;
-const QString DefaultImpl("Default");
 /*!
  * \ingroup QtProtobuf
  * \private
@@ -53,14 +49,15 @@ const QString DefaultImpl("Default");
 class Q_PROTOBUF_EXPORT QProtobufSerializerRegistry final
 {
 public:
-    std::shared_ptr<QAbstractProtobufSerializer> getSerializer(const QString &id, const QString &plugin = DefaultImpl);
+    std::shared_ptr<QAbstractProtobufSerializer> getSerializer(const QString &id);
+    std::shared_ptr<QAbstractProtobufSerializer> getSerializer(const QString &id, const QString &plugin);
     std::unique_ptr<QAbstractProtobufSerializer> acquireSerializer(const QString &id, const QString &plugin);
     float pluginVersion(const QString &plugin);
     QStringList pluginSerializers(const QString &plugin);
     float pluginProtobufVersion(const QString &plugin);
     int pluginRating(const QString &plugin);
 
-    const QString &loadPlugin(const QString &path = "", const QString &name = "");
+    QString loadPlugin(const QString &name);
 
     static QProtobufSerializerRegistry &instance() {
         static QProtobufSerializerRegistry _instance;

tests/test_qprotobuf_serializer_plugin/CMakeLists.txt

@@ -20,6 +20,9 @@ target_link_libraries(${TARGET} gtest_main gtest ${QtProtobuf_GENERATED} ${QTPRO
 add_target_windeployqt(TARGET ${TARGET}
     QML_DIR ${CMAKE_CURRENT_SOURCE_DIR})
 
+add_subdirectory("serialization")
+
 add_test(NAME ${TARGET} COMMAND ${TARGET})
+set_tests_properties(${TARGET} PROPERTIES
+    ENVIRONMENT QT_PROTOBUF_PLUGIN_PATH=$<TARGET_FILE_DIR:serializationplugin>)
 
-add_subdirectory("serialization")

tests/test_qprotobuf_serializer_plugin/serialization/CMakeLists.txt

@@ -17,14 +17,6 @@ if(NOT DEFINED QT_QMAKE_EXECUTABLE)
     endif()
 endif()
 
-execute_process(
-    COMMAND ${QT_QMAKE_EXECUTABLE} -query QT_INSTALL_QML
-    OUTPUT_VARIABLE TARGET_IMPORTS_DIR
-    OUTPUT_STRIP_TRAILING_WHITESPACE
-)
-
-set(TARGET_IMPORTS_DIR ${TARGET_IMPORTS_DIR}/QtProtobuf)
-
 file(GLOB SOURCES
     qtserializationplugin.cpp
     qprotobufjsonserializerimpl.cpp
@@ -36,17 +28,12 @@ file(GLOB HEADERS
 
 add_library(${TARGET} SHARED ${SOURCES})
 target_link_libraries(${TARGET} PRIVATE Qt5::Core Qt5::Qml ${QTPROTOBUF_COMMON_NAMESPACE}::QtProtobuf)
-set_target_properties(${TARGET} PROPERTIES
-    LIBRARY_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/QtProtobuf"
-    RUNTIME_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/QtProtobuf"
-    RUNTIME_OUTPUT_DIRECTORY_RELEASE "${CMAKE_CURRENT_BINARY_DIR}/QtProtobuf"
-    RUNTIME_OUTPUT_DIRECTORY_DEBUG "${CMAKE_CURRENT_BINARY_DIR}/QtProtobuf")
 target_compile_definitions(${TARGET} PRIVATE SERIALIZATION_LIB)
 
-configure_file("${CMAKE_CURRENT_SOURCE_DIR}/serializeinfo.json" "${CMAKE_CURRENT_BINARY_DIR}/QtProtobuf/serializeinfo.json" COPYONLY)
-install(FILES "${CMAKE_CURRENT_BINARY_DIR}/QtProtobuf/serializeinfo.json" DESTINATION "${TARGET_CMAKE_DIR}")
+configure_file("${CMAKE_CURRENT_SOURCE_DIR}/serializeinfo.json" "${CMAKE_CURRENT_BINARY_DIR}/serializeinfo.json" COPYONLY)
+install(FILES "${CMAKE_CURRENT_BINARY_DIR}/serializeinfo.json" DESTINATION "$<TARGET_PROPERTY:${QTPROTOBUF_COMMON_NAMESPACE}::QtProtobuf,QT_PROTOBUF_PLUGIN_PATH>")
 
 install(TARGETS ${TARGET}
     PUBLIC_HEADER DESTINATION "${TARGET_INCLUDE_DIR}"
-    RUNTIME DESTINATION "${TARGET_IMPORTS_DIR}"
-    LIBRARY DESTINATION "${TARGET_IMPORTS_DIR}")
+    RUNTIME DESTINATION "$<TARGET_PROPERTY:${QTPROTOBUF_COMMON_NAMESPACE}::QtProtobuf,QT_PROTOBUF_PLUGIN_PATH>"
+    LIBRARY DESTINATION "$<TARGET_PROPERTY:${QTPROTOBUF_COMMON_NAMESPACE}::QtProtobuf,QT_PROTOBUF_PLUGIN_PATH>")

tests/test_qprotobuf_serializer_plugin/serialization/qtserializationplugin.cpp

@@ -33,10 +33,10 @@ QtSerializationPlugin::QtSerializationPlugin()
     m_serializers["json"] = std::shared_ptr<QtProtobuf::QAbstractProtobufSerializer>(new QProtobufJsonSerializerImpl());
 }
 
-QtProtobuf::QAbstractProtobufSerializer *QtSerializationPlugin::serializer(const QString &serializer_name)
+std::shared_ptr<QtProtobuf::QAbstractProtobufSerializer> QtSerializationPlugin::serializer(const QString &serializerName)
 {
-    if (m_serializers.find(serializer_name) == m_serializers.end())
+    if (m_serializers.find(serializerName) == m_serializers.end())
         return nullptr;
 
-    return m_serializers[serializer_name].get();
+    return m_serializers[serializerName];
 }

tests/test_qprotobuf_serializer_plugin/serialization/qtserializationplugin.h

@@ -43,7 +43,7 @@ public:
     QtSerializationPlugin();
     ~QtSerializationPlugin() = default;
 
-     virtual QtProtobuf::QAbstractProtobufSerializer *serializer(const QString &serializer_name);
+     virtual std::shared_ptr<QtProtobuf::QAbstractProtobufSerializer> serializer(const QString &serializerName);
 
 protected:
     std::unordered_map<QString/*id*/, std::shared_ptr<QtProtobuf::QAbstractProtobufSerializer>> m_serializers;

tests/test_qprotobuf_serializer_plugin/serializationplugintest.cpp

@@ -32,6 +32,7 @@ using namespace QtProtobuf;
 namespace {
 const QLatin1String ProtobufSerializator("protobuf");
 const QLatin1String JsonSerializator("json");
+const QLatin1String Serializationplugin("serializationplugin");
 }
 
 QString SerializationPluginTest::loadedTestPlugin;
@@ -39,11 +40,11 @@ void SerializationPluginTest::SetUpTestCase()
 {
     //Register all types
     QtProtobuf::qRegisterProtobufTypes();
-    loadedTestPlugin = QProtobufSerializerRegistry::instance().loadPlugin();
 }
 
 void SerializationPluginTest::SetUp()
 {
+    loadedTestPlugin = QProtobufSerializerRegistry::instance().loadPlugin(Serializationplugin);
     serializers[ProtobufSerializator] = QProtobufSerializerRegistry::instance().getSerializer(ProtobufSerializator, loadedTestPlugin);
     serializers[JsonSerializator] = QProtobufSerializerRegistry::instance().getSerializer(JsonSerializator, loadedTestPlugin);
 }