Introduce AbstractClient::error signal ( #108 #111 )

- Mark "lastError" methods as deprecated
- Rename AbstractChannel::StatusCodes enum to StatusCode

CMakeLists.txt

@@ -43,7 +43,7 @@ if(UNIX)
       set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Werror -Wno-pessimizing-move -Wno-mismatched-tags -Wno-unused-private-field")
     elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
       # using GCC
-      set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Werror")
+      set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Werror -Wno-error=deprecated-declarations")
     endif()
 elseif(WIN32)
     if (${CMAKE_CXX_COMPILER_ID} MATCHES "MSVC")

src/grpc/abstractchannel.h

@@ -40,9 +40,9 @@ class QTGRPCSHARED_EXPORT AbstractChannel
 {
 public:
     /*!
-     * \brief The Channel StatusCodes
+     * \brief Channel's status codes
      */
-    enum StatusCodes {
+    enum StatusCode {
         Ok = 0,                 //!< No error
         Cancelled = 1,          //!< The operation was cancelled, typically by the caller
         Unknown = 2,            //!< Unknown error
@@ -70,7 +70,7 @@ public:
      * \param ret
      * \return
      */
-    virtual StatusCodes call(const QString &method, const QString &service, const QByteArray &args, QByteArray &ret) = 0;
+    virtual StatusCode call(const QString &method, const QString &service, const QByteArray &args, QByteArray &ret) = 0;
 
     /*!
      * \brief call

src/grpc/abstractclient.cpp

@@ -31,11 +31,11 @@ namespace qtprotobuf {
 class AbstractClientPrivate final {
 public:
     AbstractClientPrivate(const QString &service) : service(service)
-    , lastError(AbstractChannel::StatusCodes::Ok) {}
+    , lastError(AbstractChannel::StatusCode::Ok) {}
 
     std::shared_ptr<AbstractChannel> channel;
     const QString service;
-    AbstractChannel::StatusCodes lastError;
+    AbstractChannel::StatusCode lastError;
     QString lastErrorString;
 };
 }
@@ -83,7 +83,7 @@ AsyncReply *AbstractClient::call(const QString &method, const QByteArray &arg)
         return reply;
     }
 
-    connect(reply, &AsyncReply::error, this, [this, reply](AbstractChannel::StatusCodes statusCode){
+    connect(reply, &AsyncReply::error, this, [this, reply](AbstractChannel::StatusCode statusCode){
         d->lastError = statusCode;
         reply->deleteLater();
     });
@@ -109,7 +109,7 @@ void AbstractClient::subscribe_p(const QString &method, const QByteArray &arg, c
     d->channel->subscribe(method, d->service, arg, this, handler);
 }
 
-AbstractChannel::StatusCodes AbstractClient::lastError() const
+AbstractChannel::StatusCode AbstractClient::lastError() const
 {
     return d->lastError;
 }

src/grpc/abstractclient.h

@@ -42,16 +42,22 @@
 namespace qtprotobuf {
 
 class AbstractChannel;
-class AbstractClientPrivate;
 
 class QTGRPCSHARED_EXPORT AbstractClient : public QObject
 {
 public:
     void attachChannel(const std::shared_ptr<AbstractChannel> &channel);
 
-    AbstractChannel::StatusCodes lastError() const;
+    [[deprecated]]
+    AbstractChannel::StatusCode lastError() const;
+
+    [[deprecated]]
     QString lastErrorString() const;
 
+signals:
+    // TODO: emit this signal instead of setting lastError
+    void error(AbstractChannel::StatusCode code, QString errorText);
+
 protected:
     AbstractClient(const QString &service, QObject *parent = nullptr);
     virtual ~AbstractClient();
@@ -122,7 +128,8 @@ private:
     AbstractClient(AbstractClient &&) = delete;
     AbstractClient &operator =(AbstractClient &&) = delete;
 
-    AbstractClientPrivate *d;
+    // PIMPL
+    class AbstractClientPrivate *d;
 };
 
 }

src/grpc/asyncreply.h

@@ -56,7 +56,7 @@ public:
 
 signals:
     void finished();
-    void error(AbstractChannel::StatusCodes);
+    void error(AbstractChannel::StatusCode);
 
 protected:
     AsyncReply(const std::shared_ptr<AbstractChannel> &channel, QObject *parent = nullptr) : QObject(parent)

src/grpc/http2channel.cpp

@@ -47,7 +47,13 @@
 using namespace qtprotobuf;
 
 namespace  {
-const static std::unordered_map<QNetworkReply::NetworkError, AbstractChannel::StatusCodes> StatusCodeMap = { { QNetworkReply::ConnectionRefusedError, AbstractChannel::Unavailable },
+
+/**
+ * This QNetworkReply::NetworkError -> AbstractChannel::StatusCode mapping should be kept in sync with original
+ * <a href="https://github.com/grpc/grpc/blob/master/doc/statuscodes.md">gRPC status codes</a>
+ */
+const static std::unordered_map<QNetworkReply::NetworkError, AbstractChannel::StatusCode> StatusCodeMap = {
+                                                                { QNetworkReply::ConnectionRefusedError, AbstractChannel::Unavailable },
                                                                 { QNetworkReply::RemoteHostClosedError, AbstractChannel::Unavailable },
                                                                 { QNetworkReply::HostNotFoundError, AbstractChannel::Unavailable },
                                                                 { QNetworkReply::TimeoutError, AbstractChannel::DeadlineExceeded },
@@ -148,7 +154,7 @@ struct Http2ChannelPrivate {
         }
     }
 
-    static QByteArray processReply(QNetworkReply *networkReply, AbstractChannel::StatusCodes &statusCode) {
+    static QByteArray processReply(QNetworkReply *networkReply, AbstractChannel::StatusCode &statusCode) {
         //Check if no network error occured
         if (networkReply->error() != QNetworkReply::NoError) {
             qProtoWarning() << networkReply->error() << ":" << networkReply->errorString();
@@ -157,8 +163,8 @@ struct Http2ChannelPrivate {
         }
 
         //Check if server answer with error
-        statusCode = static_cast<AbstractChannel::StatusCodes>(networkReply->rawHeader(GrpcStatusHeader).toInt());
-        if (statusCode != AbstractChannel::StatusCodes::Ok) {
+        statusCode = static_cast<AbstractChannel::StatusCode>(networkReply->rawHeader(GrpcStatusHeader).toInt());
+        if (statusCode != AbstractChannel::StatusCode::Ok) {
             qProtoWarning() << "Protobuf server error occured" << networkReply->errorString();
             return {};
         }
@@ -214,7 +220,7 @@ Http2Channel::~Http2Channel()
     delete d;
 }
 
-AbstractChannel::StatusCodes Http2Channel::call(const QString &method, const QString &service, const QByteArray &args, QByteArray &ret)
+AbstractChannel::StatusCode Http2Channel::call(const QString &method, const QString &service, const QByteArray &args, QByteArray &ret)
 {
     QEventLoop loop;
 
@@ -226,7 +232,7 @@ AbstractChannel::StatusCodes Http2Channel::call(const QString &method, const QSt
         loop.exec();
     }
 
-    StatusCodes grpcStatus = StatusCodes::Unknown;
+    StatusCode grpcStatus = StatusCode::Unknown;
     ret = d->processReply(networkReply, grpcStatus);
 
     qProtoDebug() << __func__ << "RECV: " << ret.toHex() << "grpcStatus" << grpcStatus;
@@ -238,10 +244,10 @@ void Http2Channel::call(const QString &method, const QString &service, const QBy
     QNetworkReply *networkReply = d->post(method, service, args);
 
     auto connection = QObject::connect(networkReply, &QNetworkReply::finished, reply, [reply, networkReply]() {
-        StatusCodes grpcStatus = StatusCodes::Unknown;
+        StatusCode grpcStatus = StatusCode::Unknown;
         QByteArray data = Http2ChannelPrivate::processReply(networkReply, grpcStatus);
         qProtoDebug() << "RECV: " << data;
-        if (grpcStatus != StatusCodes::Ok) {
+        if (grpcStatus != StatusCode::Ok) {
             reply->setData({});
             reply->error(grpcStatus);
             reply->finished();
@@ -312,6 +318,6 @@ void Http2Channel::abort(AsyncReply *reply)
 {
     assert(reply != nullptr);
     reply->setData({});
-    reply->error(StatusCodes::Aborted);
+    reply->error(StatusCode::Aborted);
     reply->finished();
 }

src/grpc/http2channel.h

@@ -31,18 +31,17 @@
 
 namespace qtprotobuf {
 
-struct Http2ChannelPrivate;
 class AbstractCredentials;
 
 class QTGRPCSHARED_EXPORT Http2Channel final : public AbstractChannel
 {
 public:
-    // this contructor is obsolete and is going to be removed soon
+    [[deprecated ("This contructor is obsolete and is going to be removed soon. Use Http2Channel(const QUrl&, const AbstractCredentials&)")]]
     Http2Channel(const QString &addr, quint16 port, const AbstractCredentials &credentials);
     Http2Channel(const QUrl &url, const AbstractCredentials &credentials);
     ~Http2Channel();
 
-    StatusCodes call(const QString &method, const QString &service, const QByteArray &args, QByteArray &ret) override;
+    StatusCode call(const QString &method, const QString &service, const QByteArray &args, QByteArray &ret) override;
     void call(const QString &method, const QString &service, const QByteArray &args, qtprotobuf::AsyncReply *reply) override;
     void subscribe(const QString &method, const QString &service, const QByteArray &args, AbstractClient *client, const std::function<void (const QByteArray &)> &handler) override;
 
@@ -51,7 +50,9 @@ protected:
 
 private:
     Q_DISABLE_COPY(Http2Channel)
-    Http2ChannelPrivate *d;
+
+    // PIMPL
+    struct Http2ChannelPrivate *d;
 };
 
 }

tests/test_grpc/clienttest.cpp

@@ -89,7 +89,7 @@ TEST_F(ClientTest, StringEchoAsyncTest)
 
     AsyncReply *reply = testClient.testMethod(request);
     QObject::connect(reply, &AsyncReply::finished, &m_app, [reply, &result, &waiter, &testClient]() {
-        if (testClient.lastError() == AbstractChannel::StatusCodes::Ok) {
+        if (testClient.lastError() == AbstractChannel::StatusCode::Ok) {
             result = reply->read<SimpleStringMessage>();
         }
         waiter.quit();
@@ -108,7 +108,7 @@ TEST_F(ClientTest, StringEchoAsync2Test)
     request.setTestFieldString("Hello beach!");
     QEventLoop waiter;
     testClient.testMethod(request, &m_app, [&result, &waiter, &testClient](AsyncReply *reply) {
-        if (testClient.lastError() == AbstractChannel::StatusCodes::Ok) {
+        if (testClient.lastError() == AbstractChannel::StatusCode::Ok) {
             result = reply->read<SimpleStringMessage>();
         }
         waiter.quit();
@@ -131,13 +131,13 @@ TEST_F(ClientTest, StringEchoAsyncAbortTest)
     AsyncReply *reply = testClient.testMethod(request);
     result.setTestFieldString("Result not changed by echo");
     QObject::connect(reply, &AsyncReply::finished, &m_app, [reply, &result, &waiter, &testClient]() {
-        if (testClient.lastError() == AbstractChannel::StatusCodes::Ok) {
+        if (testClient.lastError() == AbstractChannel::StatusCode::Ok) {
             result = reply->read<SimpleStringMessage>();
         }
         waiter.quit();
     });
 
-    QObject::connect(reply, &AsyncReply::error, reply, [&errorCalled](AbstractChannel::StatusCodes){
+    QObject::connect(reply, &AsyncReply::error, reply, [&errorCalled](AbstractChannel::StatusCode){
         errorCalled = true;
     });
     QTimer::singleShot(5000, &waiter, &QEventLoop::quit);
@@ -145,19 +145,19 @@ TEST_F(ClientTest, StringEchoAsyncAbortTest)
 
     waiter.exec();
     ASSERT_STREQ(result.testFieldString().toStdString().c_str(), "Result not changed by echo");
-    ASSERT_EQ(testClient.lastError(), AbstractChannel::StatusCodes::Aborted);
+    ASSERT_EQ(testClient.lastError(), AbstractChannel::StatusCode::Aborted);
     ASSERT_TRUE(errorCalled);
 
 
     errorCalled = false;
     reply = testClient.testMethod(request);
     QObject::connect(reply, &AsyncReply::finished, &m_app, [reply, &result, &waiter, &testClient]() {
-        if (testClient.lastError() == AbstractChannel::StatusCodes::Ok) {
+        if (testClient.lastError() == AbstractChannel::StatusCode::Ok) {
             result = reply->read<SimpleStringMessage>();
         }
         waiter.quit();
     });
-    QObject::connect(reply, &AsyncReply::error, reply, [&errorCalled](AbstractChannel::StatusCodes){
+    QObject::connect(reply, &AsyncReply::error, reply, [&errorCalled](AbstractChannel::StatusCode){
         errorCalled = true;
     });
 
@@ -167,7 +167,7 @@ TEST_F(ClientTest, StringEchoAsyncAbortTest)
     waiter.exec();
 
     ASSERT_STREQ(result.testFieldString().toStdString().c_str(), "Result not changed by echo");
-    ASSERT_EQ(testClient.lastError(), AbstractChannel::StatusCodes::Aborted);
+    ASSERT_EQ(testClient.lastError(), AbstractChannel::StatusCode::Aborted);
     ASSERT_TRUE(errorCalled);
 }
 
@@ -199,7 +199,7 @@ TEST_F(ClientTest, StringEchoStreamTest)
 
     ASSERT_EQ(i, 4);
     ASSERT_STREQ(result.testFieldString().toStdString().c_str(), "Stream1Stream2Stream3Stream4");
-    ASSERT_EQ(testClient.lastError(), AbstractChannel::StatusCodes::Ok);
+    ASSERT_EQ(testClient.lastError(), AbstractChannel::StatusCode::Ok);
 }
 
 TEST_F(ClientTest, StringEchoStreamTestRetUpdates)
@@ -225,7 +225,7 @@ TEST_F(ClientTest, StringEchoStreamTestRetUpdates)
 
     ASSERT_EQ(i, 4);
     ASSERT_STREQ(result->testFieldString().toStdString().c_str(), "Stream4");
-    ASSERT_EQ(testClient.lastError(), AbstractChannel::StatusCodes::Ok);
+    ASSERT_EQ(testClient.lastError(), AbstractChannel::StatusCode::Ok);
 }
 
 
@@ -254,5 +254,5 @@ TEST_F(ClientTest, HugeBlobEchoStreamTest)
 
     QByteArray returnDataHash = QCryptographicHash::hash(result.testBytes(), QCryptographicHash::Sha256);
     ASSERT_TRUE(returnDataHash == dataHash);
-    ASSERT_EQ(testClient.lastError(), AbstractChannel::StatusCodes::Ok);
+    ASSERT_EQ(testClient.lastError(), AbstractChannel::StatusCode::Ok);
 }