Browse Source

Keep serializer ownership inside the grpc channel

- Remove serializer instance from QAbstractGrpcClient to avoid
  creating of the faken instances when the channel is not
  initialize.
- Check if the serializer explicitly before executing any calls
  or streams.
- Adjust tests.
- Minor code improvements.
Alexey Edelev 2 years ago
parent
commit
6ccc0b2bfb

+ 6 - 7
src/grpc/qabstractgrpcclient.cpp

@@ -37,13 +37,10 @@ namespace QtProtobuf {
 //! \private
 class QAbstractGrpcClientPrivate final {
 public:
-    QAbstractGrpcClientPrivate(const QString &service) : service(service) {
-        serializer = QProtobufSerializerRegistry::instance().getSerializer("protobuf");
-    }
+    QAbstractGrpcClientPrivate(const QString &service) : service(service) {}
 
     std::shared_ptr<QAbstractGrpcChannel> channel;
     const QString service;
-    std::shared_ptr<QAbstractProtobufSerializer> serializer;
     std::vector<QGrpcStreamShared> activeStreams;
 };
 }
@@ -70,7 +67,6 @@ void QAbstractGrpcClient::attachChannel(const std::shared_ptr<QAbstractGrpcChann
         stream->cancel();
     }
     dPtr->channel = channel;
-    dPtr->serializer = channel->serializer();
     for (auto stream : dPtr->activeStreams) {
         stream->cancel();
     }
@@ -194,7 +190,10 @@ QGrpcStreamShared QAbstractGrpcClient::stream(const QString &method, const QByte
     return grpcStream;
 }
 
-QAbstractProtobufSerializer *QAbstractGrpcClient::serializer() const
+std::shared_ptr<QAbstractProtobufSerializer> QAbstractGrpcClient::serializer() const
 {
-    return dPtr->serializer.get();
+    if (dPtr->channel == nullptr || dPtr->channel->serializer() == nullptr) {
+        return nullptr;
+    }
+    return dPtr->channel->serializer();
 }

+ 69 - 28
src/grpc/qabstractgrpcclient.h

@@ -83,7 +83,7 @@ signals:
      * \param[out] code gRPC channel StatusCode
      * \param[out] errorText Error description from channel or from QGrpc
      */
-    void error(const QGrpcStatus &status);
+    void error(const QGrpcStatus &status) const;
 
 protected:
     QAbstractGrpcClient(const QString &service, QObject *parent = nullptr);
@@ -108,9 +108,15 @@ protected:
         }
 
         QByteArray retData;
-        status = call(method, arg.serialize(serializer()), retData);
-        if (status == QGrpcStatus::StatusCode::Ok) {
-            return tryDeserialize(*ret, retData);
+        bool ok = false;
+        QByteArray argData = trySerialize(arg, ok);
+        if (ok) {
+            status = call(method, argData, retData);
+            if (status == QGrpcStatus::StatusCode::Ok) {
+                status = tryDeserialize(*ret, retData);
+            }
+        } else {
+            status = QGrpcStatus({QGrpcStatus::Unknown, QLatin1String("Serializing failed. Serializer is not ready")});
         }
         return status;
     }
@@ -123,7 +129,12 @@ protected:
      */
     template<typename A>
     QGrpcCallReplyShared call(const QString &method, const A &arg) {
-        return call(method, arg.serialize(serializer()));
+        bool ok = false;
+        QByteArray argData = trySerialize(arg, ok);
+        if (!ok) {
+            return QGrpcCallReplyShared();
+        }
+        return call(method, argData);
     }
 
     /*!
@@ -136,7 +147,12 @@ protected:
      */
     template<typename A>
     QGrpcStreamShared stream(const QString &method, const A &arg) {
-        return stream(method, arg.serialize(serializer()));
+        bool ok = false;
+        QByteArray argData = trySerialize(arg, ok);
+        if (!ok) {
+            return QGrpcStreamShared();
+        }
+        return stream(method, argData);
     }
 
     /*!
@@ -157,8 +173,12 @@ protected:
             qProtoCritical() << nullPointerError.arg(method);
             return nullptr;
         }
-
-        return stream(method, arg.serialize(serializer()), [ret, this](const QByteArray &data) {
+        bool ok = false;
+        QByteArray argData = trySerialize(arg, ok);
+        if (!ok) {
+            return QGrpcStreamShared();
+        }
+        return stream(method, argData, [ret, this](const QByteArray &data) {
             if (!ret.isNull()) {
                 tryDeserialize(*ret, data);
             } else {
@@ -175,12 +195,6 @@ protected:
      */
     void cancel(const QString &method);
 
-    /*!
-     * \brief serializer provides assigned to client serializer
-     * \return pointer to serializer. Serializer is owned by QtProtobuf::QProtobufSerializerRegistry.
-     */
-    QAbstractProtobufSerializer *serializer() const;
-
     friend class QGrpcAsyncOperationBase;
 private:
     //!\private
@@ -199,25 +213,52 @@ private:
     template<typename R>
     QGrpcStatus tryDeserialize(R &ret, const QByteArray &retData) {
         QGrpcStatus status{QGrpcStatus::Ok};
-        try {
-            ret.deserialize(serializer(), retData);
-        } catch (std::invalid_argument &) {
-            static const QLatin1String invalidArgumentErrorMessage("Response deserialization failed invalid field found");
-            status = {QGrpcStatus::InvalidArgument, invalidArgumentErrorMessage};
-            error(status);
-            qProtoCritical() << invalidArgumentErrorMessage;
-        } catch (std::out_of_range &) {
-            static const QLatin1String outOfRangeErrorMessage("Invalid size of received buffer");
-            status = {QGrpcStatus::OutOfRange, outOfRangeErrorMessage};
-            error(status);
-            qProtoCritical() << outOfRangeErrorMessage;
-        } catch (...) {
-            status = {QGrpcStatus::Internal, QLatin1String("Unknown exception caught during deserialization")};
+        auto _serializer = serializer();
+        if (_serializer != nullptr) {
+            try {
+                ret.deserialize(_serializer.get(), retData);
+            } catch (std::invalid_argument &) {
+                static const QLatin1String invalidArgumentErrorMessage("Response deserialization failed invalid field found");
+                status = {QGrpcStatus::InvalidArgument, invalidArgumentErrorMessage};
+                error(status);
+                qProtoCritical() << invalidArgumentErrorMessage;
+            } catch (std::out_of_range &) {
+                static const QLatin1String outOfRangeErrorMessage("Invalid size of received buffer");
+                status = {QGrpcStatus::OutOfRange, outOfRangeErrorMessage};
+                error(status);
+                qProtoCritical() << outOfRangeErrorMessage;
+            } catch (...) {
+                status = {QGrpcStatus::Internal, QLatin1String("Unknown exception caught during deserialization")};
+                error(status);
+            }
+        } else {
+            status = {QGrpcStatus::Unknown, QLatin1String("Deserializing failed. Serializer is not ready")};
             error(status);
         }
         return status;
     }
 
+
+    template<typename R>
+    QByteArray trySerialize(const R &arg, bool &ok) {
+        ok = false;
+        QGrpcStatus status{QGrpcStatus::Ok};
+        auto _serializer = serializer();
+        if (_serializer == nullptr) {
+            error({QGrpcStatus::Unknown, QLatin1String("Serializing failed. Serializer is not ready")});
+            return QByteArray();
+        }
+        ok = true;
+        return arg.serialize(_serializer.get());
+    }
+
+    /*!
+     * \private
+     * \brief serializer provides assigned to client serializer
+     * \return pointer to serializer. Serializer is owned by QtProtobuf::QProtobufSerializerRegistry.
+     */
+    std::shared_ptr<QAbstractProtobufSerializer> serializer() const;
+
     Q_DISABLE_COPY_MOVE(QAbstractGrpcClient)
 
     std::unique_ptr<QAbstractGrpcClientPrivate> dPtr;

+ 5 - 11
src/grpc/qgrpcasyncoperationbase_p.h

@@ -41,7 +41,8 @@ namespace QtProtobuf {
 /*!
  * \ingroup QtGrpc
  * \private
- * \brief The QGrpcAsyncOperationBase class implements stream logic
+ * \brief The QGrpcAsyncOperationBase class implements common logic to
+ *        handle communication in Grpc channel.
  */
 class Q_GRPC_EXPORT QGrpcAsyncOperationBase : public QObject
 {
@@ -55,16 +56,9 @@ public:
     T read() {
         QMutexLocker locker(&m_asyncLock);
         T value;
-        try {
-            value.deserialize(static_cast<QAbstractGrpcClient*>(parent())->serializer(), m_data);
-        } catch (std::invalid_argument &) {
-            static const QLatin1String invalidArgumentErrorMessage("Response deserialization failed invalid field found");
-            error({QGrpcStatus::InvalidArgument, invalidArgumentErrorMessage});
-        } catch (std::out_of_range &) {
-            static const QLatin1String outOfRangeErrorMessage("Invalid size of received buffer");
-            error({QGrpcStatus::OutOfRange, outOfRangeErrorMessage});
-        } catch (...) {
-            error({QGrpcStatus::Internal, QLatin1String("Unknown exception caught during deserialization")});
+        auto client = static_cast<QAbstractGrpcClient*>(parent());
+        if (client) {
+            client->tryDeserialize(value, m_data);
         }
         return value;
     }

+ 3 - 3
src/protobuf/qprotobufserializerregistry.cpp

@@ -109,9 +109,9 @@ struct QProtobufSerializerRegistryPrivateRecord final
 
     QObject* loadPluginImpl()
     {
-        if (loader == nullptr || !loader->load())
-        {
-            qProtoWarning() << "Can't load plugin from" << libPath << loader->errorString();
+        if (loader == nullptr || !loader->load()) {
+            qProtoWarning() << "Can't load plugin from" << libPath
+                            << "loader error" << (loader != nullptr ? loader->errorString() : "");
             return nullptr;
         }
         return loader->instance();

+ 2 - 2
tests/test_grpc/clienttest.cpp

@@ -484,7 +484,7 @@ TEST_F(ClientTest, ClientSyncTestUnattachedChannel)
     QGrpcStatus status = testClient.testMethodStatusMessage(request, ret);
 
     ASSERT_EQ(status.code(), QGrpcStatus::Unknown);
-    ASSERT_STREQ("No channel(s) attached.", status.message().toStdString().c_str());
+    ASSERT_STREQ("Serializing failed. Serializer is not ready", status.message().toStdString().c_str());
     delete ret;
 }
 
@@ -506,7 +506,7 @@ TEST_F(ClientTest, ClientSyncTestUnattachedChannelSignal)
     waiter.exec();
 
     ASSERT_EQ(asyncStatus, QGrpcStatus::Unknown);
-    ASSERT_STREQ("No channel(s) attached.", asyncStatus.message().toStdString().c_str());
+    ASSERT_STREQ("Serializing failed. Serializer is not ready", asyncStatus.message().toStdString().c_str());
     delete ret;
 }