Eliminate all copies of Request

We sometimes need to retry requests after doing things like fetching a new
access token, so the response handlers need access to the request. This
requires either copying the request or heap allocating it, and as the request
body can be quite large heap allocating it is better. Previously we created
them on the stack and then moved them to the heap, but as every request goes
through the path where they're moved to the heap they should just start there.

src/realm/object-store/sync/app.hpp

@@ -491,15 +491,17 @@ class App : public std::enable_shared_from_this<App>,
                               util::UniqueFunction<void(std::optional<AppError>)>&& completion)
         REQUIRES(!m_route_mutex);
 
+    /// The completion type for all intermediate operations which occur before performing the original request
+    using IntermediateCompletion = util::UniqueFunction<void(std::unique_ptr<Request>&&, const Response&)>;
+
     /// Checks if an auth failure has taken place and if so it will attempt to refresh the
     /// access token and then perform the orginal request again with the new access token
     /// @param error The error to check for auth failures
-    /// @param response The original response to pass back should this not be an auth error
     /// @param request The request to perform
     /// @param completion returns the original response in the case it is not an auth error, or if a failure
     /// occurs, if the refresh was a success the newly attempted response will be passed back
-    void handle_auth_failure(const AppError& error, const Response& response, Request&& request,
-                             const std::shared_ptr<User>& user,
+    void handle_auth_failure(const AppError& error, std::unique_ptr<Request>&& request, const Response& response,
+                             const std::shared_ptr<User>& user, RequestTokenType token_type,
                              util::UniqueFunction<void(const Response&)>&& completion) REQUIRES(!m_route_mutex);
 
     std::string url_for_path(const std::string& path) const override REQUIRES(!m_route_mutex);
@@ -531,7 +533,7 @@ class App : public std::enable_shared_from_this<App>,
     /// @param request The original request object that needs to be sent after the update
     /// @param completion The original completion object that will be called with the response to the request
     /// @param new_hostname If provided, the metadata will be requested from this hostname
-    void update_location_and_resend(Request&& request, util::UniqueFunction<void(const Response&)>&& completion,
+    void update_location_and_resend(std::unique_ptr<Request>&& request, IntermediateCompletion&& completion,
                                     std::optional<std::string>&& new_hostname = util::none) REQUIRES(!m_route_mutex);
 
     void post(std::string&& route, util::UniqueFunction<void(std::optional<AppError>)>&& completion,
@@ -541,23 +543,22 @@ class App : public std::enable_shared_from_this<App>,
     /// @param request The request to be performed
     /// @param completion Returns the response from the server
     /// @param update_location Force the location metadata to be updated prior to sending the request
-    void do_request(Request&& request, util::UniqueFunction<void(const Response&)>&& completion,
+    void do_request(std::unique_ptr<Request>&& request, IntermediateCompletion&& completion,
                     bool update_location = false) REQUIRES(!m_route_mutex);
 
+    std::unique_ptr<Request> make_request(HttpMethod method, std::string&& url, const std::shared_ptr<User>& user,
+                                          RequestTokenType, std::string&& body) const;
+
     /// Process the redirect response received from the last request that was sent to the server
     /// @param request The request to be performed (in case it needs to be sent again)
     /// @param response The response from the send_request_to_server operation
     /// @param completion Returns the response from the server if not a redirect
-    void check_for_redirect_response(Request&& request, const Response& response,
-                                     util::UniqueFunction<void(const Response&)>&& completion)
-        REQUIRES(!m_route_mutex);
+    void check_for_redirect_response(std::unique_ptr<Request>&& request, const Response& response,
+                                     IntermediateCompletion&& completion) REQUIRES(!m_route_mutex);
 
-    /// Performs an authenticated request to the Stitch server, using the current authentication state
-    /// @param request The request to be performed
-    /// @param completion Returns the response from the server
-    void do_authenticated_request(Request&& request, const std::shared_ptr<User>& user,
-                                  util::UniqueFunction<void(const Response&)>&& completion) override
-        REQUIRES(!m_route_mutex);
+    void do_authenticated_request(HttpMethod, std::string&& route, std::string&& body,
+                                  const std::shared_ptr<User>& user, RequestTokenType,
+                                  util::UniqueFunction<void(const Response&)>&&) override REQUIRES(!m_route_mutex);
 
 
     /// Gets the social profile for a `User`.

src/realm/object-store/sync/auth_request_client.hpp

@@ -16,17 +16,13 @@
 //
 ////////////////////////////////////////////////////////////////////////////
 
-#ifndef AUTH_REQUEST_CLIENT_HPP
-#define AUTH_REQUEST_CLIENT_HPP
+#ifndef REALM_OS_AUTH_REQUEST_CLIENT_HPP
+#define REALM_OS_AUTH_REQUEST_CLIENT_HPP
 
-#include <realm/util/functional.hpp>
-#include <memory>
-#include <string>
+#include <realm/object-store/sync/generic_network_transport.hpp>
 
-namespace realm {
-namespace app {
+namespace realm::app {
 class User;
-struct Request;
 struct Response;
 
 class AuthRequestClient {
@@ -35,11 +31,11 @@ class AuthRequestClient {
 
     virtual std::string url_for_path(const std::string& path) const = 0;
 
-    virtual void do_authenticated_request(Request&&, const std::shared_ptr<User>& sync_user,
+    virtual void do_authenticated_request(HttpMethod, std::string&& route, std::string&& body,
+                                          const std::shared_ptr<User>& user, RequestTokenType,
                                           util::UniqueFunction<void(const Response&)>&&) = 0;
 };
 
-} // namespace app
-} // namespace realm
+} // namespace realm::app
 
-#endif /* AUTH_REQUEST_CLIENT_HPP */
+#endif /* REALM_OS_AUTH_REQUEST_CLIENT_HPP */

src/realm/object-store/sync/generic_network_transport.cpp

@@ -44,21 +44,21 @@ std::string http_message(const std::string& prefix, int code)
 }
 } // anonymous namespace
 
-const char* httpmethod_to_string(HttpMethod method)
+std::ostream& operator<<(std::ostream& os, HttpMethod method)
 {
     switch (method) {
         case HttpMethod::get:
-            return "GET";
+            return os << "GET";
         case HttpMethod::post:
-            return "POST";
+            return os << "POST";
         case HttpMethod::patch:
-            return "PATCH";
+            return os << "PATCH";
         case HttpMethod::put:
-            return "PUT";
+            return os << "PUT";
         case HttpMethod::del:
-            return "DEL";
+            return os << "DEL";
     }
-    return "UNKNOWN";
+    return os << "UNKNOWN";
 }
 
 AppError::AppError(ErrorCodes::Error ec, std::string message, std::string link,

src/realm/object-store/sync/generic_network_transport.hpp

@@ -78,6 +78,7 @@ std::ostream& operator<<(std::ostream& os, AppError error);
  * An HTTP method type.
  */
 enum class HttpMethod { get, post, patch, put, del };
+std::ostream& operator<<(std::ostream&, HttpMethod);
 
 /**
  * Request/Response headers type
@@ -113,11 +114,11 @@ struct Request {
      * The body of the request.
      */
     std::string body;
-
-    /// Indicates if the request uses the refresh token or the access token
-    bool uses_refresh_token = false;
 };
 
+/// What type of auth token should be used for a HTTP request.
+enum class RequestTokenType { NoAuth, AccessToken, RefreshToken };
+
 /**
  * The contents of an HTTP response.
  */
@@ -155,8 +156,6 @@ struct GenericNetworkTransport {
                                         util::UniqueFunction<void(const Response&)>&& completion) = 0;
 };
 
-const char* httpmethod_to_string(HttpMethod method);
-
 } // namespace realm::app
 
 #endif /* REALM_GENERIC_NETWORK_TRANSPORT_HPP */

src/realm/object-store/sync/push_client.cpp

@@ -44,19 +44,19 @@ void PushClient::register_device(const std::string& registration_token, const st
     std::string route = m_auth_request_client->url_for_path(push_route);
 
     bson::BsonDocument args{{"registrationToken", registration_token}};
-    m_auth_request_client->do_authenticated_request(
-        {HttpMethod::put, std::move(route), m_timeout_ms, {}, bson::Bson(args).to_string(), false}, sync_user,
-        wrap_completion(std::move(completion)));
+    m_auth_request_client->do_authenticated_request(HttpMethod::put, std::move(route), bson::Bson(args).to_string(),
+                                                    sync_user, RequestTokenType::AccessToken,
+                                                    wrap_completion(std::move(completion)));
 }
 
 void PushClient::deregister_device(const std::shared_ptr<User>& sync_user,
                                    util::UniqueFunction<void(util::Optional<AppError>)>&& completion)
 {
     auto push_route = util::format("/app/%1/push/providers/%2/registration", m_app_id, m_service_name);
 
-    m_auth_request_client->do_authenticated_request(
-        {HttpMethod::del, m_auth_request_client->url_for_path(push_route), m_timeout_ms, {}, "", false}, sync_user,
-        wrap_completion(std::move(completion)));
+    m_auth_request_client->do_authenticated_request(HttpMethod::del, m_auth_request_client->url_for_path(push_route),
+                                                    "", sync_user, RequestTokenType::AccessToken,
+                                                    wrap_completion(std::move(completion)));
 }
 
 } // namespace realm::app

src/realm/object-store/sync/push_client.hpp

@@ -20,24 +20,22 @@
 #define PUSH_CLIENT_HPP
 
 #include <realm/util/functional.hpp>
-#include <realm/util/optional.hpp>
 
 #include <memory>
+#include <optional>
 #include <string>
 
-namespace realm {
-namespace app {
+namespace realm::app {
 class AuthRequestClient;
 class User;
 struct AppError;
 
 class PushClient {
 public:
-    PushClient(const std::string& service_name, const std::string& app_id, uint64_t timeout_ms,
+    PushClient(const std::string& service_name, const std::string& app_id,
                std::shared_ptr<AuthRequestClient>&& auth_request_client)
         : m_service_name(service_name)
         , m_app_id(app_id)
-        , m_timeout_ms(timeout_ms)
         , m_auth_request_client(std::move(auth_request_client))
     {
     }
@@ -54,24 +52,22 @@ class PushClient {
     /// @param sync_user The sync user requesting push registration.
     /// @param completion An error will be returned should something go wrong.
     void register_device(const std::string& registration_token, const std::shared_ptr<User>& sync_user,
-                         util::UniqueFunction<void(util::Optional<AppError>)>&& completion);
+                         util::UniqueFunction<void(std::optional<AppError>)>&& completion);
 
 
     /// Deregister a device for push notificatons, no token or device id needs to be passed
     /// as it is linked to the user in MongoDB Realm Cloud.
     /// @param sync_user The sync user requesting push degistration.
     /// @param completion An error will be returned should something go wrong.
     void deregister_device(const std::shared_ptr<User>& sync_user,
-                           util::UniqueFunction<void(util::Optional<AppError>)>&& completion);
+                           util::UniqueFunction<void(std::optional<AppError>)>&& completion);
 
 private:
     std::string m_service_name;
     std::string m_app_id;
-    uint64_t m_timeout_ms;
     std::shared_ptr<AuthRequestClient> m_auth_request_client;
 };
 
-} // namespace app
-} // namespace realm
+} // namespace realm::app
 
 #endif /* PUSH_CLIENT_HPP */

test/object-store/c_api/c_api.cpp

@@ -6114,8 +6114,7 @@ TEST_CASE("C API app: link_user integration w/c_api transport", "[sync][app][c_a
         REQUIRE(request_context != nullptr);
         auto new_request = Request{HttpMethod(request.method), request.url, default_timeout_ms, std::move(headers),
                                    std::string(request.body, request.body_size)};
-        user_data->logger->trace("CAPI: Request URL (%1): %2", httpmethod_to_string(new_request.method),
-                                 new_request.url);
+        user_data->logger->trace("CAPI: Request URL (%1): %2", new_request.method, new_request.url);
         user_data->logger->trace("CAPI: Request body: %1", new_request.body);
         user_data->transport->send_request_to_server(new_request, [&](const Response& response) mutable {
             std::vector<realm_http_header_t> c_headers;

test/object-store/sync/app.cpp

@@ -5429,7 +5429,6 @@ TEST_CASE("app: make_streaming_request", "[sync][app][streaming]") {
         CHECK(req.body == "");
         CHECK(req.headers == Headers{{"Accept", "text/event-stream"}});
         CHECK(req.timeout_ms == timeout_ms);
-        CHECK(req.uses_refresh_token == false);
 
         auto req_args = get_request_args(req);
         CHECK(req_args["name"] == "func");

test/object-store/util/sync/baas_admin_api.cpp

@@ -358,8 +358,8 @@ app::Response do_http_request(const app::Request& request)
             return util::format("BaaS Coid: \"%1\"", coid_header->second);
         }();
 
-        logger->trace("Baas API %1 request to %2 took %3 %4\n", app::httpmethod_to_string(request.method),
-                      request.url, std::chrono::duration_cast<std::chrono::milliseconds>(total_time), coid);
+        logger->trace("Baas API %1 request to %2 took %3 %4\n", request.method, request.url,
+                      std::chrono::duration_cast<std::chrono::milliseconds>(total_time), coid);
     }
 
     int http_code = 0;