Skip to content

Commit

Permalink
[C++ builds & passes] CI linter fixups
Browse files Browse the repository at this point in the history
  • Loading branch information
@stevelorddremio
Paul Nienaber authored and stevelorddremio committed Jan 25, 2024
1 parent 489a29d commit bdd6046
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 43 deletions.
2 changes: 2 additions & 0 deletions cpp/src/arrow/flight/integration_tests/flight_integration_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")); }
}

Expand Down
84 changes: 45 additions & 39 deletions cpp/src/arrow/flight/integration_tests/test_integration.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<FlightClient> client) override {
ARROW_ASSIGN_OR_RAISE(auto info,
Expand Down Expand Up @@ -746,28 +746,27 @@ 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<std::string, SessionOptionValue> 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<SetSessionOptionsResult> SetSessionOptions(
const ServerCallContext& context,
const SetSessionOptionsRequest& request) override {
SetSessionOptionsResult res;

sql::ServerSessionMiddleware* middleware =
(sql::ServerSessionMiddleware*)context.GetMiddleware(session_middleware_key);
ARROW_ASSIGN_OR_RAISE(std::shared_ptr<sql::FlightSession> session, middleware->GetSession());
(sql::ServerSessionMiddleware*)context.GetMiddleware(session_middleware_key);
ARROW_ASSIGN_OR_RAISE(std::shared_ptr<sql::FlightSession> session,
middleware->GetSession());

for (const auto& [name, value] : request.session_options) {
// Blacklisted value name
Expand Down Expand Up @@ -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<sql::FlightSession> session, middleware->GetSession());
ARROW_ASSIGN_OR_RAISE(std::shared_ptr<sql::FlightSession> session,
middleware->GetSession());

return GetSessionOptionsResult{session->GetSessionOptions()};
}
Expand All @@ -809,17 +809,17 @@ 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};
}
};

/// \brief The Session Options scenario.
///
/// 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<FlightServerBase>* server,
FlightServerOptions* options) override {
Expand All @@ -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<std::string>{
"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<std::string>{"a", "b", "sea", "dee", " ",
" ", "geee", "(づ｡◕‿‿◕｡)づ"}}}};
ARROW_ASSIGN_OR_RAISE(auto res1, client.SetSessionOptions({}, req1));
// Some errors
if (!(res1.errors == std::map<std::string, SetSessionOptionsResult::Error>{
{"lol_invalid", SetSessionOptionsResult::Error{
SetSessionOptionErrorValue::kInvalidName}},
{"key_with_invalid_value", SetSessionOptionsResult::Error{
SetSessionOptionErrorValue::kInvalidValue}}})) {
if (!(res1.errors ==
std::map<std::string, SetSessionOptionsResult::Error>{
{"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<std::string, SessionOptionValue>{
{"foolong", 123L},
{"bardouble", 456.0},
{"big_ol_string_list", std::vector<std::string>{
"a", "b", "sea", "dee", " ", " ", "geee", "(づ｡◕‿‿◕｡)づ"}}})) {
if (!(res2.session_options ==
std::map<std::string, SessionOptionValue>{
{"foolong", 123L},
{"bardouble", 456.0},
{"big_ol_string_list",
std::vector<std::string>{"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<std::string, SessionOptionValue>{
{"bardouble", 456.0},
{"big_ol_string_list", "a,b,sea,dee, , ,geee,(づ｡◕‿‿◕｡)づ"}})) {
if (!(res4.session_options ==
std::map<std::string, SessionOptionValue>{
{"bardouble", 456.0},
{"big_ol_string_list", "a,b,sea,dee, , ,geee,(づ｡◕‿‿◕｡)づ"}})) {
return Status::Invalid("res4 incorrect: " + res4.ToString());
}

Expand Down
8 changes: 4 additions & 4 deletions cpp/src/arrow/flight/sql/server_session_middleware.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ class ServerSessionMiddlewareImpl : public ServerSessionMiddleware {
ServerSessionMiddlewareImpl(ServerSessionMiddlewareFactory* factory,
const CallHeaders& headers,
std::shared_ptr<FlightSession> 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)),
Expand All @@ -57,7 +56,8 @@ class ServerSessionMiddlewareImpl : public ServerSessionMiddleware {
}
if (!closed_session_id_.empty()) {
add_call_headers->AddHeader(
"set-cookie", static_cast<std::string>(kSessionCookieName) + "=" + session_id_ + "; Max-Age=0");
"set-cookie", static_cast<std::string>(kSessionCookieName) + "=" + session_id_ +
"; Max-Age=0");
}
}

Expand Down Expand Up @@ -220,7 +220,7 @@ std::map<std::string, SessionOptionValue> FlightSession::GetSessionOptions() {
}

void FlightSession::SetSessionOption(const std::string& name,
const SessionOptionValue value) {
const SessionOptionValue value) {
const std::lock_guard<std::shared_mutex> l(map_lock_);
map_[name] = std::move(value);
}
Expand Down

0 comments on commit bdd6046

Please sign in to comment.