diff --git a/cpp/src/arrow/flight/integration_tests/flight_integration_test.cc b/cpp/src/arrow/flight/integration_tests/flight_integration_test.cc index c8cdbd8ecb2f3..4a28827bfd339 100644 --- a/cpp/src/arrow/flight/integration_tests/flight_integration_test.cc +++ b/cpp/src/arrow/flight/integration_tests/flight_integration_test.cc @@ -67,6 +67,8 @@ TEST(FlightIntegration, SessionOptions) { ASSERT_OK(RunScenario("session_options")); } +TEST(FlightIntegration, SessionOptions) { ASSERT_OK(RunScenario("session_options")); } + TEST(FlightIntegration, PollFlightInfo) { ASSERT_OK(RunScenario("poll_flight_info")); } } diff --git a/cpp/src/arrow/flight/integration_tests/test_integration.cc b/cpp/src/arrow/flight/integration_tests/test_integration.cc index 439ffa767059d..babc4a8a447f1 100644 --- a/cpp/src/arrow/flight/integration_tests/test_integration.cc +++ b/cpp/src/arrow/flight/integration_tests/test_integration.cc @@ -713,7 +713,7 @@ class ExpirationTimeRenewFlightEndpointScenario : public Scenario { return Status::OK(); } - Status MakeClient(FlightClientOptions* options) override {return Status::OK(); } + Status MakeClient(FlightClientOptions* options) override { return Status::OK(); } Status RunClient(std::unique_ptr client) override { ARROW_ASSIGN_OR_RAISE(auto info, @@ -746,19 +746,17 @@ class ExpirationTimeRenewFlightEndpointScenario : public Scenario { /// setSessionOptions has a blacklisted option name and string option value, /// both "lol_invalid", which will result in errors attempting to set either. class SessionOptionsServer : public sql::FlightSqlServerBase { - inline const static std::string invalid_option_name = "lol_invalid"; - inline const static SessionOptionValue invalid_option_value = "lol_invalid"; + static inline const std::string invalid_option_name = "lol_invalid"; + static inline const SessionOptionValue invalid_option_value = "lol_invalid"; const std::string session_middleware_key; // These will never be threaded so using a plain map and no lock std::map session_store_; public: - SessionOptionsServer(std::string session_middleware_key) + explicit SessionOptionsServer(std::string session_middleware_key) : FlightSqlServerBase(), - session_middleware_key(std::move(session_middleware_key)) { - - } + session_middleware_key(std::move(session_middleware_key)) {} arrow::Result SetSessionOptions( const ServerCallContext& context, @@ -766,8 +764,9 @@ class SessionOptionsServer : public sql::FlightSqlServerBase { SetSessionOptionsResult res; sql::ServerSessionMiddleware* middleware = - (sql::ServerSessionMiddleware*)context.GetMiddleware(session_middleware_key); - ARROW_ASSIGN_OR_RAISE(std::shared_ptr session, middleware->GetSession()); + (sql::ServerSessionMiddleware*)context.GetMiddleware(session_middleware_key); + ARROW_ASSIGN_OR_RAISE(std::shared_ptr session, + middleware->GetSession()); for (const auto& [name, value] : request.session_options) { // Blacklisted value name @@ -796,11 +795,12 @@ class SessionOptionsServer : public sql::FlightSqlServerBase { const ServerCallContext& context, const GetSessionOptionsRequest& request) override { sql::ServerSessionMiddleware* middleware = - (sql::ServerSessionMiddleware*)context.GetMiddleware(session_middleware_key); + (sql::ServerSessionMiddleware*)context.GetMiddleware(session_middleware_key); if (!middleware->HasSession()) { return Status::Invalid("No existing session to get options from."); } - ARROW_ASSIGN_OR_RAISE(std::shared_ptr session, middleware->GetSession()); + ARROW_ASSIGN_OR_RAISE(std::shared_ptr session, + middleware->GetSession()); return GetSessionOptionsResult{session->GetSessionOptions()}; } @@ -809,9 +809,9 @@ class SessionOptionsServer : public sql::FlightSqlServerBase { const ServerCallContext& context, const CloseSessionRequest& request) override { // Broken (does not expire cookie) until C++ middleware SendingHeaders handling fixed. sql::ServerSessionMiddleware* middleware = - (sql::ServerSessionMiddleware*)context.GetMiddleware(session_middleware_key); + (sql::ServerSessionMiddleware*)context.GetMiddleware(session_middleware_key); ARROW_RETURN_NOT_OK(middleware->CloseSession()); - return CloseSessionResult{ CloseSessionStatus::kClosed }; + return CloseSessionResult{CloseSessionStatus::kClosed}; } }; @@ -819,7 +819,7 @@ class SessionOptionsServer : public sql::FlightSqlServerBase { /// /// This tests Session Options functionality as well as ServerSessionMiddleware. class SessionOptionsScenario : public Scenario { - inline const static std::string server_middleware_key = "sessionmiddleware"; + static inline const std::string server_middleware_key = "sessionmiddleware"; Status MakeServer(std::unique_ptr* server, FlightServerOptions* options) override { @@ -843,41 +843,47 @@ class SessionOptionsScenario : public Scenario { sql::FlightSqlClient client{std::move(flight_client)}; // Set - auto req1 = SetSessionOptionsRequest{{ - {"foolong", 123L}, - {"bardouble", 456.0}, - {"lol_invalid", "this won't get set"}, - {"key_with_invalid_value", "lol_invalid"}, - {"big_ol_string_list", std::vector{ - "a", "b", "sea", "dee", " ", " ", "geee", "(づ。◕‿‿◕。)づ"}} - }}; + auto req1 = SetSessionOptionsRequest{ + {{"foolong", 123L}, + {"bardouble", 456.0}, + {"lol_invalid", "this won't get set"}, + {"key_with_invalid_value", "lol_invalid"}, + {"big_ol_string_list", std::vector{"a", "b", "sea", "dee", " ", + " ", "geee", "(づ。◕‿‿◕。)づ"}}}}; ARROW_ASSIGN_OR_RAISE(auto res1, client.SetSessionOptions({}, req1)); // Some errors - if (!(res1.errors == std::map{ - {"lol_invalid", SetSessionOptionsResult::Error{ - SetSessionOptionErrorValue::kInvalidName}}, - {"key_with_invalid_value", SetSessionOptionsResult::Error{ - SetSessionOptionErrorValue::kInvalidValue}}})) { + if (!(res1.errors == + std::map{ + {"lol_invalid", + SetSessionOptionsResult::Error{SetSessionOptionErrorValue::kInvalidName}}, + {"key_with_invalid_value", + SetSessionOptionsResult::Error{ + SetSessionOptionErrorValue::kInvalidValue}}})) { return Status::Invalid("res1 incorrect: " + res1.ToString()); } // Some set, some omitted due to above errors ARROW_ASSIGN_OR_RAISE(auto res2, client.GetSessionOptions({}, {})); - if (!(res2.session_options == std::map{ - {"foolong", 123L}, - {"bardouble", 456.0}, - {"big_ol_string_list", std::vector{ - "a", "b", "sea", "dee", " ", " ", "geee", "(づ。◕‿‿◕。)づ"}}})) { + if (!(res2.session_options == + std::map{ + {"foolong", 123L}, + {"bardouble", 456.0}, + {"big_ol_string_list", + std::vector{"a", "b", "sea", "dee", " ", " ", "geee", + "(づ。◕‿‿◕。)づ"}}})) { return Status::Invalid("res2 incorrect: " + res2.ToString()); } // Update - ARROW_ASSIGN_OR_RAISE(auto res3, client.SetSessionOptions({}, SetSessionOptionsRequest{{ - {"foolong", std::monostate{}}, - {"big_ol_string_list", "a,b,sea,dee, , ,geee,(づ。◕‿‿◕。)づ"} - }})); + ARROW_ASSIGN_OR_RAISE( + auto res3, + client.SetSessionOptions( + {}, SetSessionOptionsRequest{ + {{"foolong", std::monostate{}}, + {"big_ol_string_list", "a,b,sea,dee, , ,geee,(づ。◕‿‿◕。)づ"}}})); ARROW_ASSIGN_OR_RAISE(auto res4, client.GetSessionOptions({}, {})); - if (!(res4.session_options == std::map{ - {"bardouble", 456.0}, - {"big_ol_string_list", "a,b,sea,dee, , ,geee,(づ。◕‿‿◕。)づ"}})) { + if (!(res4.session_options == + std::map{ + {"bardouble", 456.0}, + {"big_ol_string_list", "a,b,sea,dee, , ,geee,(づ。◕‿‿◕。)づ"}})) { return Status::Invalid("res4 incorrect: " + res4.ToString()); } diff --git a/cpp/src/arrow/flight/sql/server_session_middleware.cc b/cpp/src/arrow/flight/sql/server_session_middleware.cc index 27c4b140ed89c..f3e02de232444 100644 --- a/cpp/src/arrow/flight/sql/server_session_middleware.cc +++ b/cpp/src/arrow/flight/sql/server_session_middleware.cc @@ -42,8 +42,7 @@ class ServerSessionMiddlewareImpl : public ServerSessionMiddleware { ServerSessionMiddlewareImpl(ServerSessionMiddlewareFactory* factory, const CallHeaders& headers, std::shared_ptr session, - std::string session_id, - bool existing_session = true) + std::string session_id, bool existing_session = true) : factory_(factory), headers_(headers), session_(std::move(session)), @@ -57,7 +56,8 @@ class ServerSessionMiddlewareImpl : public ServerSessionMiddleware { } if (!closed_session_id_.empty()) { add_call_headers->AddHeader( - "set-cookie", static_cast(kSessionCookieName) + "=" + session_id_ + "; Max-Age=0"); + "set-cookie", static_cast(kSessionCookieName) + "=" + session_id_ + + "; Max-Age=0"); } } @@ -220,7 +220,7 @@ std::map FlightSession::GetSessionOptions() { } void FlightSession::SetSessionOption(const std::string& name, - const SessionOptionValue value) { + const SessionOptionValue value) { const std::lock_guard l(map_lock_); map_[name] = std::move(value); }