drogonframework / drogon

Drogon: A C++14/17/20 based HTTP web application framework running on Linux/macOS/Unix/Windows
MIT License
11.51k stars 1.1k forks source link

Cannot capture returned value of second sql query in transaction #821

Closed shivshankardayal closed 3 years ago

shivshankardayal commented 3 years ago

Consider following code:

auto clientPtr = drogon::app().getFastDbClient("default");
        clientPtr->newTransactionAsync([=, this](const std::shared_ptr<Transaction> &transPtr) mutable {
            assert(transPtr);

            transPtr->execSqlAsync(
                "select t.id, t.visible, t.title, t.created_at , t.posted_by, t.updated_at, t.votes, t.slug, "
                "users.username, users.id as uid, array_agg(topic_tags.tag_id) as tag_id, array_agg(tags.name) as tags from topics t left "
                "join users on t.posted_by=users.id left join topic_tags on topic_tags.topic_id=t.id left join "
                "tags on topic_tags.tag_id = tags.id where t.op_id=0 group by t.id, users.id order by "
                "t.updated_at limit $1 offset $2",
                [=, this](const Result &rows) mutable {
                    if (rows.size() == 0)
                    {
                        return;
                    }
                    else
                    {
                        ret["topics"] = Json::arrayValue;
                        for (auto &r : rows)
                        {
                            Json::Value topic;
                            std::string answers;

                            topic["id"] = r["id"].as<std::string>();
                            topic["visible"] = r["visible"].as<bool>();
                            topic["title"] = r["title"].as<std::string>();
                            topic["created_at"] = r["created_at"].as<std::string>();
                            topic["updated_at"] = r["updated_at"].as<std::string>();
                            topic["votes"] = r["votes"].as<std::string>();
                            topic["slug"] = r["slug"].as<std::string>();
                            topic["username"] = r["username"].as<std::string>();
                            topic["uid"] = r["uid"].as<std::string>();
                            topic["tid"] = r["tag_id"].as<std::string>();
                            topic["tags"] = r["tags"].as<std::string>();
                            transPtr->execSqlAsync(
                                "select count(1) from topics where op_id is not null and op_id=$1",
                                [=, &answers](const Result &rows1) mutable {
                                    for (auto &r1 : rows1)
                                    {
                                        answers = r1["count"].as<std::string>();
                                        // topic["answers"] = r1["count"].as<std::string>();
                                        // ret["topics"].append(topic);
                                    }
                                },
                                [=](const DrogonDbException &e) mutable {
                                    LOG_DEBUG << e.base().what();
                                    ret["error"] = (std::string)e.base().what();
                                    callback(HttpResponse::newHttpJsonResponse(std::move(ret)));
                                },
                                r["id"].as<int64_t>());
                            LOG_DEBUG << answers;
                            ret["topics"].append(topic);
                        }
                    }

                    callback(jsonResponse(std::move(ret)));
                    return;
                },
                [=](const DrogonDbException &e) mutable {
                    LOG_DEBUG << e.base().what();
                    ret["error"] = (std::string)e.base().what();
                    callback(HttpResponse::newHttpJsonResponse(std::move(ret)));
                },
                (long)limit, (long)limit * (page_no - 1));
        });

The string answers is still empty after the second sql query.

an-tao commented 3 years ago

I'm not sure, but maybe it should be:

answers = r1["count(1)"].as<std::string>();

or

size_t count=r1[(Row::SizeType)0].as<size_t>();

The problem is that you print the answer string after calling an async method, which means the answer string is not yet set by the SQL result when you print it. you should print it in the callback of the async method.

shivshankardayal commented 3 years ago

Your second option is fine but not the first one. The problem is that any value inside the second async SQL query is not being returned in the response. I need the count in the response, but it never comes. How can I achieve that?

an-tao commented 3 years ago

move the HTTP Response callback into the callback of the second async SQL query.

shivshankardayal commented 3 years ago

No it cannot be done. The second SQL query is inside a loop. The loop must complete before response can be returned.

an-tao commented 3 years ago

make a counter var to count the result in the callback,or use the coroutine interface

shivshankardayal commented 3 years ago

What is coroutine interface? Do you mean synchronous version of FastDbClient?

an-tao commented 3 years ago

yes,please refer to the coroutines section on wiki

shivshankardayal commented 3 years ago

On the wiki coroutines are supposed to return Task<>. Is that a typo? Should it not be task<>? I get following compilation errors with respect to Task<>

In file included from /home/shiv/Downloads/arth/backend/src/api/v1/index.cc:13:
/home/shiv/Downloads/arth/backend/src/api/v1/index.h:30:94: error: ‘Task’ does not name a type
   30 |         void index(const HttpRequestPtr &req, Callback callback, const std::string &page) -> Task<>;
      |                                                                                              ^~~~
/home/shiv/Downloads/arth/backend/src/api/v1/index.h:30:94: error: expected ‘;’ at end of member declaration
   30 |         void index(const HttpRequestPtr &req, Callback callback, const std::string &page) -> Task<>;
      |                                                                                              ^~~~
      |                                                                                                  ;
/home/shiv/Downloads/arth/backend/src/api/v1/index.h:30:98: error: expected unqualified-id before ‘<’ token
   30 |         void index(const HttpRequestPtr &req, Callback callback, const std::string &page) -> Task<>;
      |                                                                                                  ^
In file included from /home/shiv/Downloads/arth/backend/src/api/v1/index.h:10,
                 from /home/shiv/Downloads/arth/backend/src/api/v1/index.cc:13:
/home/shiv/Downloads/arth/backend/src/api/v1/index.h: In static member function ‘static void api::v1::Index::initPathRouting()’:
/home/shiv/Downloads/arth/backend/src/api/v1/index.h:27:30: error: ‘index’ is not a member of ‘api::v1::Index’
   27 |         ADD_METHOD_TO(Index::index, "/api/v1/topics/{size_t page}", Get);
      |                              ^~~~~
/home/shiv/Downloads/arth/backend/src/api/v1/index.cc: At global scope:
/home/shiv/Downloads/arth/backend/src/api/v1/index.cc:19:99: error: ‘Task’ does not name a type
   19 | void Index::index(const HttpRequestPtr &req, Callback callback, const std::string &page = "1") -> Task<>
      |                                                                                                   ^~~~
/home/shiv/Downloads/arth/backend/src/api/v1/index.cc:19:103: error: expected initializer before ‘<’ token
   19 | void Index::index(const HttpRequestPtr &req, Callback callback, const std::string &page = "1") -> Task<>
      |                                                                                                       ^
make[2]: *** [CMakeFiles/arth.dir/build.make:206: CMakeFiles/arth.dir/src/api/v1/index.cc.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:76: CMakeFiles/arth.dir/all] Error 2
make: *** [Makefile:84: all] Error 2
an-tao commented 3 years ago

Task<>, which is defined in drogon/coroutines.h

shivshankardayal commented 3 years ago

Sorry I just observed that I did not have latest drogon code. Pulled latest master and it is being compiled.

shivshankardayal commented 3 years ago

Converted function to following:

auto Index::index(const HttpRequestPtr &req, Callback callback, const std::string &page = "1") -> Task<>
{
    Json::Value ret;

    auto customConfig = app().getCustomConfig();
    auto page_no = atol(page.c_str());
    int limit = customConfig.get("limit_per_page", 50).asInt64();
    LOG_DEBUG << limit;
    LOG_DEBUG << (page_no - 1) * limit;
    {
        ret["topics"] = Json::arrayValue;
        auto clientPtr = drogon::app().getDbClient();
        try
        {
            auto result = co_await clientPtr->execSqlCoro("select t.id, t.visible, t.title, t.created_at , t.posted_by, t.updated_at, t.votes, t.slug, "
                                                          "users.username, users.id as uid, array_agg(topic_tags.tag_id) as tag_id, array_agg(tags.name) as tags from topics t left "
                                                          "join users on t.posted_by=users.id left join topic_tags on topic_tags.topic_id=t.id left join "
                                                          "tags on topic_tags.tag_id = tags.id where t.op_id=0 group by t.id, users.id order by "
                                                          "t.updated_at limit $1 offset $2",
                                                          limit, (page_no - 1) * limit);
            LOG_DEBUG << result.size();
            if (result.size() == 0)
            {
                co_return;
            }
            else
            {
                ret["topics"] = Json::arrayValue;
                for (auto &r : result)
                {
                    Json::Value topic;
                    std::string counter = 0;

                    topic["id"] = r["id"].as<std::string>();
                    topic["visible"] = r["visible"].as<bool>();
                    topic["title"] = r["title"].as<std::string>();
                    topic["created_at"] = r["created_at"].as<std::string>();
                    topic["updated_at"] = r["updated_at"].as<std::string>();
                    topic["votes"] = r["votes"].as<std::string>();
                    topic["slug"] = r["slug"].as<std::string>();
                    topic["username"] = r["username"].as<std::string>();
                    topic["uid"] = r["uid"].as<std::string>();
                    topic["tid"] = r["tag_id"].as<std::string>();
                    topic["tags"] = r["tags"].as<std::string>();

                    ret["topics"].append(topic);
                }
            }

            callback(jsonResponse(std::move(ret)));
        }
        catch (const DrogonDbException &err)
        {
            // Exception works as sync interfaces.
            auto resp = HttpResponse::newHttpResponse();
            resp->setBody(err.base().what());
            callback(resp);
        }
    }

    co_return;
}

Now it does not even print limit as debug message and never sends response.

an-tao commented 3 years ago

The parameters of coroutine handlers should be value type instead of reference type.

an-tao commented 3 years ago

@marty1885

shivshankardayal commented 3 years ago

Thanks. My bad. Should have been more careful.

marty1885 commented 3 years ago

I should have already banned bad handle signatures from compiling. Maybe that's not working as intended? Hmmmm.... Will look into it later on. Sorry for the inconvenience.

shivshankardayal commented 3 years ago

Can you please add a transaction example for coroutines as well? It is there for dbclients but I am not clear how to use transactions with coroutines. Thanks.

shivshankardayal commented 3 years ago

https://github.com/Nalanda-Labs/arth is where I am using Drogon. It is supposed to be a replacement for Wordpress and Discourse both. Long-long way to go. Perhaps you can add it to who is using Drogon. I will start sending X-Powered-by header set to Drogon.

marty1885 commented 3 years ago

@shivshankardayal

You can create a new transaction through co_await clientPtr->newTransactionCoro(). Then use the transaction as usual ( transactions also supports coroutine APIs)