kou commented on code in PR #34817:
URL: https://github.com/apache/arrow/pull/34817#discussion_r1491657360
##########
cpp/src/arrow/flight/integration_tests/test_integration.cc:
##########
@@ -744,6 +746,155 @@ class ExpirationTimeRenewFlightEndpointScenario : public
Scenario {
}
};
+/// \brief The server used for testing Session Options.
+///
+/// setSessionOptions has a blacklisted option name and string option value,
Review Comment:
```suggestion
/// SetSessionOptions has a blacklisted option name and string option value,
```
##########
cpp/src/arrow/flight/integration_tests/test_integration.cc:
##########
@@ -744,6 +746,155 @@ class ExpirationTimeRenewFlightEndpointScenario : public
Scenario {
}
};
+/// \brief The server used for testing Session Options.
+///
+/// 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 {
+ 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:
+ explicit SessionOptionsServer(std::string session_middleware_key)
+ : FlightSqlServerBase(),
+ 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());
+
+ for (const auto& [name, value] : request.session_options) {
+ // Blacklisted value name
+ if (name == invalid_option_name) {
+ res.errors.emplace(name, SetSessionOptionsResult::Error{
+
SetSessionOptionErrorValue::kInvalidName});
+ continue;
+ }
+ // Blacklisted option value
+ if (value == invalid_option_value) {
+ res.errors.emplace(name, SetSessionOptionsResult::Error{
+
SetSessionOptionErrorValue::kInvalidValue});
+ continue;
+ }
+ if (std::holds_alternative<std::monostate>(value)) {
+ session->EraseSessionOption(name);
+ continue;
+ }
+ session->SetSessionOption(name, value);
+ }
+
+ return res;
+ }
+
+ arrow::Result<GetSessionOptionsResult> GetSessionOptions(
+ const ServerCallContext& context,
+ const GetSessionOptionsRequest& request) override {
+ auto* middleware = static_cast<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());
+
+ return GetSessionOptionsResult{session->GetSessionOptions()};
+ }
+
+ arrow::Result<CloseSessionResult> CloseSession(
+ const ServerCallContext& context, const CloseSessionRequest& request)
override {
+ // Broken (does not expire cookie) until C++ middleware SendingHeaders
handling fixed.
Review Comment:
Is this still broken?
##########
cpp/src/arrow/flight/integration_tests/test_integration.cc:
##########
@@ -744,6 +746,155 @@ class ExpirationTimeRenewFlightEndpointScenario : public
Scenario {
}
};
+/// \brief The server used for testing Session Options.
+///
+/// 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 {
+ 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:
+ explicit SessionOptionsServer(std::string session_middleware_key)
+ : FlightSqlServerBase(),
+ 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);
Review Comment:
```suggestion
auto middleware =
static_cast<sql::ServerSessionMiddleware*>(context.GetMiddleware(session_middleware_key));
```
##########
cpp/src/arrow/flight/integration_tests/test_integration.cc:
##########
@@ -744,6 +746,155 @@ class ExpirationTimeRenewFlightEndpointScenario : public
Scenario {
}
};
+/// \brief The server used for testing Session Options.
+///
+/// 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 {
+ 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:
+ explicit SessionOptionsServer(std::string session_middleware_key)
+ : FlightSqlServerBase(),
+ 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());
+
+ for (const auto& [name, value] : request.session_options) {
+ // Blacklisted value name
+ if (name == invalid_option_name) {
+ res.errors.emplace(name, SetSessionOptionsResult::Error{
+
SetSessionOptionErrorValue::kInvalidName});
+ continue;
+ }
+ // Blacklisted option value
+ if (value == invalid_option_value) {
+ res.errors.emplace(name, SetSessionOptionsResult::Error{
+
SetSessionOptionErrorValue::kInvalidValue});
+ continue;
+ }
+ if (std::holds_alternative<std::monostate>(value)) {
+ session->EraseSessionOption(name);
+ continue;
+ }
+ session->SetSessionOption(name, value);
+ }
+
+ return res;
+ }
+
+ arrow::Result<GetSessionOptionsResult> GetSessionOptions(
+ const ServerCallContext& context,
+ const GetSessionOptionsRequest& request) override {
+ auto* middleware = static_cast<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());
+
+ return GetSessionOptionsResult{session->GetSessionOptions()};
+ }
+
+ arrow::Result<CloseSessionResult> CloseSession(
+ 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);
Review Comment:
```suggestion
auto middleware =
static_cast<sql::ServerSessionMiddleware*>(context.GetMiddleware(session_middleware_key));
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
