Improve Drogon 2.0 API

[rbugajewski]

This is a placeholder ticket for discussions related to public API changes inside the framework.

As we currently already have a user base, and most people would be annoyed by sudden API changes in minor version upgrades, we can use the upcoming release of Drogon 2.0 (whenever that will be) to also improve the public API where it’s currently suboptimal, and introduce breaking changes with a new major version.

[marty1885]

As of now, big, breaking changes includes:

• Replaced JsonCpp with nlohmann/json
• Dropped MD5
• Introduced SHA3 and BLAKE2b
• Replaced all trantor::Date objects with std::chrono::system_clock::time_point

[BertrandD]

Is it relevant to use std::filesystem::path instead of std::string when variable/arguments are a path to a file ?

[marty1885]

That's a good question. But AFAIK most (even C++17 only) libraries uses std::string(_view) anyway. We could add that if it becomes popular.

[0rangeFox]

Isn't it better to create a new branch related to the new version so we can all contribute and test.

[marty1885]

We would like to. But since we replaced a lot of the low level code in drogon2. Almost all of the core functions aren't even compiling. We are in the process of bringing features back. It isn't ready for even pre-alpha. However since we are still bringing up features, most new features added to drogon will also land in drogon2.

[sharm294]

@marty1885 could you elaborate on why nlohmann/json was chosen over other JSON libraries? Is that discussion available somewhere?

[marty1885]

@sharm294 We discussed internally. Some points are illustrated here.

It boils down to a few points:

• 4x faster in parsing compared to JsonCpp (the library that we are using now)
• Best syntax among all libraries
• Graceful type error handling (unlike RapidJSON and SimdJSON)
• Handles both reading and writing to strings/buffer
• Available on most Linux (and BSD) distros (to avoid packing our own submodule, thus potentially conflicting with the user's setup)
  • Otherwise Boost.JSON would also be a candidate

Feel free to suggest other libraries if you think they fit better :)

[sharm294]

I was just curious; nlohmann/json seems like a good balanced choice from what I've read. Still, RapidJSON's performance is enticing but I guess that comes at the cost of type safety and ease of use. I wonder if you can create a clear partition between Drogon and its JSON library backend so the user can choose in Cmake what to build Drogon with.

Btw, you guys have done awesome work on Drogon. I'm currently using it on a project :) The performance, documentation, and your availability and activity on issues here were big factors in choosing it. Look forward to 2.0!

[marty1885]

I think it's unlikely we could do that as there's isn't a universal JSON API in C++. But you don't need it to be performant. Drogon already parses JSON lazily. So you could add an overload to fromRequest then call req->as<rapidjson::Document>() to let RapidJSON to parse the request, instead of letting the default library doing the job.

[omarmohamedkh]

After reading some parts of the code I feel like I started to understand the pattern. Let's take the creation of a db client as an example: config file is parsed -> all the needed parameters are extracted -> passed to a function -> handled by a manger -> specific implementation.

I think that the step where all needed parameters are passed to a function should be handled differently since this is the step at which API consistency breaks unless new parameters are added to the end with a default value.

If only parameters with no logical default value are passed to the functions and other parameters which might have a sensible default (or even all parameters) are passed inside an options class thus allowing the addition of other parameters when needed.

For example a database client must have the rdbms, host, port and dbname, on the other hand timeout, isFast and connNum are not essential and most users might not change the defaults.

drogon::orm::DbClientConfig dbConfig;
dbConfig.isFast = true;
// or
dbConfig.setIsFast(true);
dbConfig.setConnNum(1);
// or
dbConfig.setIsFast(true).setConnNum(1);

drogon::app().createDbClient(rdms, host, port, dbname, dbConfig);

Suppose that MYSQL_OPT_COMPRESSION_ALGORITHMS is to be configurable in the framework the only needed modifications will be in parsing the config file and the specific implementation stages.

After thinking if this is to be implemented why not put all the parameters inside this config class?