dotnet/aspnetcore

Allow the SignalR C++ client to be built without using cpprestsdk

analogrelay opened this issue · 8 comments

analogrelay commented

Epic #5301

We have customers interested in using SignalR with a different backend networking stack. We'll provide abstractions (http_client, websocket_client, etc.) and use cpprestsdk to provide default implementations but we should have a #define that can be used to strip those default implementations so that they aren't in the binary when they aren't being used.

analogrelay commented

Triage decision: C++ tidy up item, not the priority for preview 5.

analogrelay commented

Let's see about getting this done in Preview 8

BrennanConroy commented

FYI, this involves doing something about Json. One thought is to copy the source from cpprestsdk and clean it up a little (namespaces etc.). Then we can delay the refactored public API for adding SignalRDOM and keep Json on the API.

Or we can find a different Json library to use instead.

BrennanConroy commented

microsoft/terminal recently started using jsoncpp. We should look at also using that library.

mmarinchenko commented

Hi guys! Just for the info :)

  1. connection_impl.cpp still uses _XPLATSTR and utility::conversions::to_string_t() in the following string:
headers[_XPLATSTR("Authorization")] = utility::conversions::to_string_t("Bearer " + response.accessToken);
  1. negotiate.cpp still uses utility::conversions::to_utf8string() in the following string:
request.headers.insert(std::make_pair(utility::conversions::to_utf8string(header.first), utility::conversions::to_utf8string(header.second)));

Both of these are not compiled without cpprestsdk even if (and when :)) the alternative http and websocket clients are provided. As I can see some code must be added to copied cpprestsdk/utility.h file.

BrennanConroy commented

if (and when :)) the alternative http and websocket clients are provided

Just to clarify, we are not providing an alternative http and websocket client.
We are providing the option for users to replace the http and websocket client with a custom one that may be platform specific.

mmarinchenko commented

@BrennanConroy

Yes, I understand this, thanks.

The reason why I wrote my comment above is you copied some code from cpprestsdk to have uri_builder, scan_string(), etc. even with alternative http and websocket client implementations.

But you copied neither _XPLATSTR, to_string_t() and to_utf8string() implementations nor wrap the code which use all this by #ifdef USE_CPPRESTSDK ... #endif condition.

BrennanConroy commented

Yep, that's because the work is still in progress and we still need to replace the headers with a normal map of <std::string, std::string>.

  • 👍1