Browse Source

Update abort functionality for AsyncReply

- Fix crash when abort
- Make explicit abort handling in Http2 transport
- Implement and update tests
NOTE: Current async test behaivor describes expected
      sequence for any transport
Alexey Edelev 5 years ago
parent
commit
28aa341aca

+ 11 - 2
CMakeLists.txt

@@ -43,17 +43,26 @@ add_subdirectory("src/grpc")
 add_subdirectory("src/generator")
 
 if(DEFINED $ENV{MAKE_TESTS})
-    set(MAKE_TESTS ${MAKE_TESTS})
+    set(MAKE_TESTS $ENV{MAKE_TESTS})
 elseif(NOT DEFINED MAKE_TESTS)
     set(MAKE_TESTS ON)
 endif()
 
 if(DEFINED $ENV{MAKE_EXAMPLES})
-    set(MAKE_EXAMPLES ${MAKE_EXAMPLES})
+    set(MAKE_EXAMPLES $ENV{MAKE_EXAMPLES})
 elseif(NOT DEFINED MAKE_EXAMPLES)
     set(MAKE_EXAMPLES ON)
 endif()
 
+if(DEFINED $ENV{MAKE_COVERAGE})
+    set(MAKE_COVERAGE $ENV{MAKE_COVERAGE})
+elseif(NOT DEFINED MAKE_COVERAGE)
+    set(MAKE_COVERAGE OFF)
+endif()
+
+if(MAKE_COVERAGE)
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fprofile-arcs -ftest-coverage")
+endif()
 
 if(MAKE_TESTS)
     find_package(GTest)

+ 1 - 1
src/grpc/abstractclient.cpp

@@ -87,10 +87,10 @@ AsyncReply *AbstractClient::call(const QString &method, const QByteArray& arg)
     });
 
     connect(reply, &AsyncReply::finished, this, [this, reply](){
-        d->lastError = AbstractChannel::Ok;
         reply->deleteLater();
     });
 
+    d->lastError = AbstractChannel::Ok;//Assume that all is OK until something happened
     d->channel->call(method, d->service, arg, reply);
     return reply;
 }

+ 2 - 1
src/grpc/asyncreply.h

@@ -57,7 +57,8 @@ signals:
     void error(AbstractChannel::StatusCodes);
 
 protected:
-    AsyncReply(const std::shared_ptr<AbstractChannel> &channel, QObject* parent = nullptr) : QObject(parent) {}
+    AsyncReply(const std::shared_ptr<AbstractChannel> &channel, QObject* parent = nullptr) : QObject(parent)
+    , m_channel(channel){}
     ~AsyncReply();
 
 private:

+ 8 - 1
src/grpc/http2channel.cpp

@@ -175,7 +175,7 @@ void Http2Channel::call(const QString &method, const QString &service, const QBy
 {
     QNetworkReply *networkReply = d->post(method, service, args);
 
-    QObject::connect(networkReply, &QNetworkReply::finished, reply, [this, reply, networkReply]() {
+    auto connection = QObject::connect(networkReply, &QNetworkReply::finished, reply, [reply, networkReply]() {
         StatusCodes grpcStatus = StatusCodes::Unknown;
         QByteArray data = Http2ChannelPrivate::processReply(networkReply, grpcStatus);
         qProtoDebug() << "RECV: " << data;
@@ -188,6 +188,13 @@ void Http2Channel::call(const QString &method, const QString &service, const QBy
         reply->setData(data);
         reply->finished();
     });
+
+    QObject::connect(reply, &AsyncReply::error, networkReply, [reply, networkReply, connection]() {
+        QObject::disconnect(connection);
+        if (networkReply->isRunning()) {
+            networkReply->abort();
+        }
+    });
 }
 
 void Http2Channel::abort(AsyncReply *reply)

+ 55 - 4
tests/test_grpc/clienttest.cpp

@@ -27,6 +27,8 @@
 #include "http2channel.h"
 #include "qtprotobuf.h"
 
+#include <QTimer>
+
 #include <QCoreApplication>
 #include <gtest/gtest.h>
 
@@ -78,8 +80,10 @@ TEST_F(ClientTest, StringEchoAsyncTest)
     QEventLoop waiter;
 
     AsyncReply* reply = testClient.testMethod(request);
-    QObject::connect(reply, &AsyncReply::finished, &app, [reply, &result, &waiter]() {
-        result = reply->read<SimpleStringMessage>();
+    QObject::connect(reply, &AsyncReply::finished, &app, [reply, &result, &waiter, &testClient]() {
+        if (testClient.lastError() == AbstractChannel::StatusCodes::Ok) {
+            result = reply->read<SimpleStringMessage>();
+        }
         waiter.quit();
     });
 
@@ -97,11 +101,58 @@ TEST_F(ClientTest, StringEchoAsync2Test)
     SimpleStringMessage request;
     request.setTestFieldString("Hello beach!");
     QEventLoop waiter;
-    testClient.testMethod(request, &app, [&result, &waiter](AsyncReply *reply) {
-        result = reply->read<SimpleStringMessage>();
+    testClient.testMethod(request, &app, [&result, &waiter, &testClient](AsyncReply *reply) {
+        if (testClient.lastError() == AbstractChannel::StatusCodes::Ok) {
+            result = reply->read<SimpleStringMessage>();
+        }
         waiter.quit();
     });
 
     waiter.exec();
     ASSERT_STREQ(result.testFieldString().toStdString().c_str(), "Hello beach!");
 }
+
+TEST_F(ClientTest, StringEchoAsyncAbortTest)
+{
+    int argc = 0;
+    QCoreApplication app(argc, nullptr);
+    TestServiceClient testClient;
+    testClient.attachChannel(std::make_shared<Http2Channel>("localhost", 50051));
+    SimpleStringMessage result;
+    SimpleStringMessage request;
+    request.setTestFieldString("sleep");
+    QEventLoop waiter;
+
+    AsyncReply* reply = testClient.testMethod(request);
+    result.setTestFieldString("Result not changed by echo");
+    QObject::connect(reply, &AsyncReply::finished, &app, [reply, &result, &waiter, &testClient]() {
+        if (testClient.lastError() == AbstractChannel::StatusCodes::Ok) {
+            result = reply->read<SimpleStringMessage>();
+        }
+        waiter.quit();
+    });
+
+    QTimer::singleShot(5000, &waiter, &QEventLoop::quit);
+    reply->abort();
+
+    waiter.exec();
+    ASSERT_STREQ(result.testFieldString().toStdString().c_str(), "Result not changed by echo");
+    ASSERT_EQ(testClient.lastError(), AbstractChannel::StatusCodes::Aborted);
+
+
+    reply = testClient.testMethod(request);
+    QObject::connect(reply, &AsyncReply::finished, &app, [reply, &result, &waiter, &testClient]() {
+        if (testClient.lastError() == AbstractChannel::StatusCodes::Ok) {
+            result = reply->read<SimpleStringMessage>();
+        }
+        waiter.quit();
+    });
+
+    QTimer::singleShot(2000, reply, &AsyncReply::abort);
+    QTimer::singleShot(5000, &waiter, &QEventLoop::quit);
+
+    waiter.exec();
+
+    ASSERT_STREQ(result.testFieldString().toStdString().c_str(), "Result not changed by echo");
+    ASSERT_EQ(testClient.lastError(), AbstractChannel::StatusCodes::Aborted);
+}

+ 6 - 2
tests/test_grpc/echoserver/testserver/main.cpp

@@ -6,18 +6,22 @@
 #include <testservice.grpc.pb.h>
 #include <simpletest.pb.h>
 #include <simpletest.grpc.pb.h>
+#include <thread>
 
 class SimpleTestImpl final : public qtprotobufnamespace::tests::TestService::Service {
 public:
-    ::grpc::Status testMethod(grpc::ServerContext *context, const qtprotobufnamespace::tests::SimpleStringMessage *request, qtprotobufnamespace::tests::SimpleStringMessage *response)
+    ::grpc::Status testMethod(grpc::ServerContext *, const qtprotobufnamespace::tests::SimpleStringMessage *request, qtprotobufnamespace::tests::SimpleStringMessage *response)
     {
         std::cerr << "testMethod called" << std::endl << request->testfieldstring() << std::endl;
         response->set_testfieldstring(request->testfieldstring());
+        if(request->testfieldstring() == "sleep") {
+            std::this_thread::sleep_for(std::chrono::seconds(3));
+        }
         return ::grpc::Status();
     }
 };
 
-int main(int argc, char *argv[])
+int main(int, char *[])
 {
     std::string server_address("localhost:50051");
     SimpleTestImpl service;