Enable mock test (#42)

* Add todo for noauth validation

* mock server with token auth

Add tests

* run same test with both modes

* Enable ODBC tests in workflow

* Switch current test cases to use FlightSQLODBCTestBase

So the tests can be skipped when `TEST_CONNECT_STR` is not set.

* Change tests to run on both mock and remote modes

Wrap usage of TEST_CONNECT_STR where possible

* Rename test fixtures and make connection string functions virtual

* Fix lint issue

* Attempt to enable ODBC build on Windows platforms

* Attempt to fix clang64 and MinGW errors

* Attempt to register ODBC

* Address James' comments

Use constant string for token

* use ServerMiddleware to validate token

Author: alinaliBQ

.github/workflows/cpp.yml

@@ -476,6 +476,10 @@ jobs:
           PIPX_BASE_PYTHON: ${{ steps.python-install.outputs.python-path }}
         run: |
           ci/scripts/install_gcs_testbench.sh default
+      - name: Register Flight SQL ODBC Driver
+        shell: cmd
+        run: |
+          call "cpp\src\arrow\flight\sql\odbc\install\install_amd64.cmd" ${{github.workspace}}\build\cpp\%ARROW_BUILD_TYPE%\libarrow_flight_sql_odbc.dll
       - name: Test
         shell: msys2 {0}
         run: |

ci/scripts/cpp_test.sh

@@ -73,8 +73,6 @@ case "$(uname)" in
     exclude_tests="${exclude_tests}|gandiva-precompiled-test"
     exclude_tests="${exclude_tests}|gandiva-projector-test"
     exclude_tests="${exclude_tests}|gandiva-utf8-test"
-    # TODO: Enable ODBC tests
-    exclude_tests="${exclude_tests}|arrow-connection-test"
     ctest_options+=(--exclude-regex "${exclude_tests}")
     ;;
   *)

cpp/cmake_modules/DefineOptions.cmake

@@ -108,7 +108,7 @@ endmacro()
 
 macro(resolve_option_dependencies)
   # Arrow Flight SQL ODBC is available only for Windows for now.
-  if(NOT MSVC_TOOLCHAIN)
+  if(NOT WIN32)
     set(ARROW_FLIGHT_SQL_ODBC OFF)
   endif()
   if(MSVC_TOOLCHAIN)

cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_auth_method.cc

@@ -45,6 +45,10 @@ class NoOpAuthMethod : public FlightSqlAuthMethod {
   void Authenticate(FlightSqlConnection& connection,
                     FlightCallOptions& call_options) override {
     // Do nothing
+
+    // TODO: implement NoOpAuthMethod to validate server address.
+    // Can use NoOpClientAuthHandler.
+    // https://github.com/apache/arrow/issues/46733
   }
 };
 

cpp/src/arrow/flight/sql/odbc/flight_sql/system_dsn.h

@@ -40,7 +40,7 @@ bool DisplayConnectionWindow(void* windowParent, Configuration& config);
  * @param windowParent Parent window handle.
  * @param config Output configuration, presumed to be empty, it will be using values from
  * properties.
- * @param config Output properties.
+ * @param properties Output properties.
  * @return True on success and false on fail.
  */
 bool DisplayConnectionWindow(void* windowParent, Configuration& config,

cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt

@@ -21,13 +21,25 @@ include_directories(${ODBC_INCLUDE_DIRS})
 
 add_definitions(-DUNICODE=1)
 
+find_package(SQLite3Alt REQUIRED)
+
+set(ARROW_FLIGHT_SQL_MOCK_SERVER_SRCS
+    ../../example/sqlite_sql_info.cc
+    ../../example/sqlite_type_info.cc
+    ../../example/sqlite_statement.cc
+    ../../example/sqlite_statement_batch_reader.cc
+    ../../example/sqlite_server.cc
+    ../../example/sqlite_tables_schema_batch_reader.cc)
+
 add_arrow_test(connection_test
                SOURCES
                connection_test.cc
                odbc_test_suite.cc
                odbc_test_suite.h
+               ${ARROW_FLIGHT_SQL_MOCK_SERVER_SRCS}
                EXTRA_LINK_LIBS
                ${ODBC_LIBRARIES}
                ${ODBCINST}
+               ${SQLite3_LIBRARIES}
                arrow_odbc_spi_impl
                odbcabstraction)

cpp/src/arrow/flight/sql/odbc/tests/connection_test.cc

@@ -200,49 +200,51 @@ TEST(SQLSetEnvAttr, TestSQLSetEnvAttrODBCVersionInvalid) {
   EXPECT_TRUE(return_set == SQL_ERROR);
 }
 
-TEST_F(FlightSQLODBCTestBase, TestSQLGetEnvAttrOutputNTS) {
-  connect();
+TYPED_TEST(FlightSQLODBCTestBase, TestSQLGetEnvAttrOutputNTS) {
+  this->connect();
 
   SQLINTEGER output_nts;
 
-  SQLRETURN return_get = SQLGetEnvAttr(env, SQL_ATTR_OUTPUT_NTS, &output_nts, 0, 0);
+  SQLRETURN return_get = SQLGetEnvAttr(this->env, SQL_ATTR_OUTPUT_NTS, &output_nts, 0, 0);
 
   EXPECT_TRUE(return_get == SQL_SUCCESS);
 
   EXPECT_EQ(output_nts, SQL_TRUE);
 
-  disconnect();
+  this->disconnect();
 }
 
-TEST_F(FlightSQLODBCTestBase, TestSQLGetEnvAttrGetLength) {
+TYPED_TEST(FlightSQLODBCTestBase, TestSQLGetEnvAttrGetLength) {
   // Test is disabled because call to SQLGetEnvAttr is handled by the driver manager on
-  // Windows. This test case can be potentionally used on macOS/Linux
+  // Windows. This test case can be potentially used on macOS/Linux
   GTEST_SKIP();
 
-  connect();
+  this->connect();
 
   SQLINTEGER length;
 
-  SQLRETURN return_get = SQLGetEnvAttr(env, SQL_ATTR_ODBC_VERSION, nullptr, 0, &length);
+  SQLRETURN return_get =
+      SQLGetEnvAttr(this->env, SQL_ATTR_ODBC_VERSION, nullptr, 0, &length);
 
   EXPECT_TRUE(return_get == SQL_SUCCESS);
 
   EXPECT_EQ(length, sizeof(SQLINTEGER));
 
-  disconnect();
+  this->disconnect();
 }
 
-TEST_F(FlightSQLODBCTestBase, TestSQLGetEnvAttrNullValuePointer) {
+TYPED_TEST(FlightSQLODBCTestBase, TestSQLGetEnvAttrNullValuePointer) {
   // Test is disabled because call to SQLGetEnvAttr is handled by the driver manager on
-  // Windows. This test case can be potentionally used on macOS/Linux
+  // Windows. This test case can be potentially used on macOS/Linux
   GTEST_SKIP();
-  connect();
+  this->connect();
 
-  SQLRETURN return_get = SQLGetEnvAttr(env, SQL_ATTR_ODBC_VERSION, nullptr, 0, nullptr);
+  SQLRETURN return_get =
+      SQLGetEnvAttr(this->env, SQL_ATTR_ODBC_VERSION, nullptr, 0, nullptr);
 
   EXPECT_TRUE(return_get == SQL_ERROR);
 
-  disconnect();
+  this->disconnect();
 }
 
 TEST(SQLSetEnvAttr, TestSQLSetEnvAttrOutputNTSValid) {
@@ -292,7 +294,7 @@ TEST(SQLSetEnvAttr, TestSQLSetEnvAttrNullValuePointer) {
   EXPECT_TRUE(return_set == SQL_ERROR);
 }
 
-TEST(SQLDriverConnect, TestSQLDriverConnect) {
+TYPED_TEST(FlightSQLODBCTestBase, TestSQLDriverConnect) {
   // ODBC Environment
   SQLHENV env;
   SQLHDBC conn;
@@ -312,8 +314,7 @@ TEST(SQLDriverConnect, TestSQLDriverConnect) {
   EXPECT_TRUE(ret == SQL_SUCCESS);
 
   // Connect string
-  ASSERT_OK_AND_ASSIGN(std::string connect_str,
-                       arrow::internal::GetEnvVar(TEST_CONNECT_STR));
+  std::string connect_str = this->getConnectionString();
   ASSERT_OK_AND_ASSIGN(std::wstring wconnect_str,
                        arrow::util::UTF8ToWideString(connect_str));
   std::vector<SQLWCHAR> connect_str0(wconnect_str.begin(), wconnect_str.end());
@@ -361,7 +362,7 @@ TEST(SQLDriverConnect, TestSQLDriverConnect) {
   EXPECT_TRUE(ret == SQL_SUCCESS);
 }
 
-TEST(SQLDriverConnect, TestSQLDriverConnectInvalidUid) {
+TEST_F(FlightSQLODBCRemoteTestBase, TestSQLDriverConnectInvalidUid) {
   // ODBC Environment
   SQLHENV env;
   SQLHDBC conn;
@@ -380,11 +381,8 @@ TEST(SQLDriverConnect, TestSQLDriverConnectInvalidUid) {
 
   EXPECT_TRUE(ret == SQL_SUCCESS);
 
-  // Connect string
-  ASSERT_OK_AND_ASSIGN(std::string connect_str,
-                       arrow::internal::GetEnvVar(TEST_CONNECT_STR));
-  // Append invalid uid to connection string
-  connect_str += std::string("uid=non_existent_id;");
+  // Invalid connect string
+  std::string connect_str = getInvalidConnectionString();
 
   ASSERT_OK_AND_ASSIGN(std::wstring wconnect_str,
                        arrow::util::UTF8ToWideString(connect_str));
@@ -418,7 +416,7 @@ TEST(SQLDriverConnect, TestSQLDriverConnectInvalidUid) {
   EXPECT_TRUE(ret == SQL_SUCCESS);
 }
 
-TEST(SQLConnect, TestSQLConnect) {
+TYPED_TEST(FlightSQLODBCTestBase, TestSQLConnect) {
   // ODBC Environment
   SQLHENV env;
   SQLHDBC conn;
@@ -438,8 +436,7 @@ TEST(SQLConnect, TestSQLConnect) {
   EXPECT_TRUE(ret == SQL_SUCCESS);
 
   // Connect string
-  ASSERT_OK_AND_ASSIGN(std::string connect_str,
-                       arrow::internal::GetEnvVar(TEST_CONNECT_STR));
+  std::string connect_str = this->getConnectionString();
 
   // Write connection string content into a DSN,
   // must succeed before continuing
@@ -454,7 +451,7 @@ TEST(SQLConnect, TestSQLConnect) {
   std::vector<SQLWCHAR> uid0(wuid.begin(), wuid.end());
   std::vector<SQLWCHAR> pwd0(wpwd.begin(), wpwd.end());
 
-  // Connecting to ODBC server.
+  // Connecting to ODBC server. Empty uid and pwd should be ignored.
   ret = SQLConnect(conn, dsn0.data(), static_cast<SQLSMALLINT>(dsn0.size()), uid0.data(),
                    static_cast<SQLSMALLINT>(uid0.size()), pwd0.data(),
                    static_cast<SQLSMALLINT>(pwd0.size()));
@@ -488,7 +485,7 @@ TEST(SQLConnect, TestSQLConnect) {
   EXPECT_TRUE(ret == SQL_SUCCESS);
 }
 
-TEST(SQLConnect, TestSQLConnectInputUidPwd) {
+TEST_F(FlightSQLODBCRemoteTestBase, TestSQLConnectInputUidPwd) {
   // ODBC Environment
   SQLHENV env;
   SQLHDBC conn;
@@ -508,10 +505,9 @@ TEST(SQLConnect, TestSQLConnectInputUidPwd) {
   EXPECT_TRUE(ret == SQL_SUCCESS);
 
   // Connect string
-  ASSERT_OK_AND_ASSIGN(std::string connect_str,
-                       arrow::internal::GetEnvVar(TEST_CONNECT_STR));
+  std::string connect_str = getConnectionString();
 
-  // Retrieve valid uid and pwd
+  // Retrieve valid uid and pwd, assumes TEST_CONNECT_STR contains uid and pwd
   Connection::ConnPropertyMap properties;
   ODBC::ODBCConnection::getPropertiesFromConnString(connect_str, properties);
   std::string uid_key("uid");
@@ -567,7 +563,7 @@ TEST(SQLConnect, TestSQLConnectInputUidPwd) {
   EXPECT_TRUE(ret == SQL_SUCCESS);
 }
 
-TEST(SQLConnect, TestSQLConnectInvalidUid) {
+TEST_F(FlightSQLODBCRemoteTestBase, TestSQLConnectInvalidUid) {
   // ODBC Environment
   SQLHENV env;
   SQLHDBC conn;
@@ -587,10 +583,9 @@ TEST(SQLConnect, TestSQLConnectInvalidUid) {
   EXPECT_TRUE(ret == SQL_SUCCESS);
 
   // Connect string
-  ASSERT_OK_AND_ASSIGN(std::string connect_str,
-                       arrow::internal::GetEnvVar(TEST_CONNECT_STR));
+  std::string connect_str = getConnectionString();
 
-  // Retrieve valid uid and pwd
+  // Retrieve valid uid and pwd, assumes TEST_CONNECT_STR contains uid and pwd
   Connection::ConnPropertyMap properties;
   ODBC::ODBCConnection::getPropertiesFromConnString(connect_str, properties);
   std::string uid = properties[std::string("uid")];
@@ -636,7 +631,7 @@ TEST(SQLConnect, TestSQLConnectInvalidUid) {
   EXPECT_TRUE(ret == SQL_SUCCESS);
 }
 
-TEST(SQLConnect, TestSQLConnectDSNPrecedence) {
+TEST_F(FlightSQLODBCRemoteTestBase, TestSQLConnectDSNPrecedence) {
   // ODBC Environment
   SQLHENV env;
   SQLHDBC conn;
@@ -656,13 +651,13 @@ TEST(SQLConnect, TestSQLConnectDSNPrecedence) {
   EXPECT_TRUE(ret == SQL_SUCCESS);
 
   // Connect string
-  ASSERT_OK_AND_ASSIGN(std::string connect_str,
-                       arrow::internal::GetEnvVar(TEST_CONNECT_STR));
+  std::string connect_str = getConnectionString();
 
   // Write connection string content into a DSN,
   // must succeed before continuing
 
-  // Pass incorrect uid and password to SQLConnect, they will be ignored
+  // Pass incorrect uid and password to SQLConnect, they will be ignored.
+  // Assumes TEST_CONNECT_STR contains uid and pwd
   std::string uid("non_existent_id"), pwd("non_existent_password");
   ASSERT_TRUE(writeDSN(connect_str));
 
@@ -746,7 +741,7 @@ TEST(SQLDisconnect, TestSQLDisconnectWithoutConnection) {
   EXPECT_TRUE(ret == SQL_SUCCESS);
 }
 
-TEST(SQLGetDiagFieldW, TestSQLGetDiagFieldWForConnectFailure) {
+TYPED_TEST(FlightSQLODBCTestBase, TestSQLGetDiagFieldWForConnectFailure) {
   //  ODBC Environment
   SQLHENV env;
   SQLHDBC conn;
@@ -765,11 +760,8 @@ TEST(SQLGetDiagFieldW, TestSQLGetDiagFieldWForConnectFailure) {
 
   EXPECT_TRUE(ret == SQL_SUCCESS);
 
-  // Connect string
-  ASSERT_OK_AND_ASSIGN(std::string connect_str,
-                       arrow::internal::GetEnvVar(TEST_CONNECT_STR));
-  // Append invalid uid to connection string
-  connect_str += std::string("uid=non_existent_id;");
+  // Invalid connect string
+  std::string connect_str = this->getInvalidConnectionString();
 
   ASSERT_OK_AND_ASSIGN(std::wstring wconnect_str,
                        arrow::util::UTF8ToWideString(connect_str));
@@ -859,9 +851,9 @@ TEST(SQLGetDiagFieldW, TestSQLGetDiagFieldWForConnectFailure) {
   EXPECT_TRUE(ret == SQL_SUCCESS);
 }
 
-TEST(SQLGetDiagFieldW, TestSQLGetDiagFieldWForConnectFailureNTS) {
+TYPED_TEST(FlightSQLODBCTestBase, TestSQLGetDiagFieldWForConnectFailureNTS) {
   // Test is disabled because driver manager on Windows does not pass through SQL_NTS
-  // This test case can be potentionally used on macOS/Linux
+  // This test case can be potentially used on macOS/Linux
   GTEST_SKIP();
   //  ODBC Environment
   SQLHENV env;
@@ -881,11 +873,8 @@ TEST(SQLGetDiagFieldW, TestSQLGetDiagFieldWForConnectFailureNTS) {
 
   EXPECT_TRUE(ret == SQL_SUCCESS);
 
-  // Connect string
-  ASSERT_OK_AND_ASSIGN(std::string connect_str,
-                       arrow::internal::GetEnvVar(TEST_CONNECT_STR));
-  // Append invalid uid to connection string
-  connect_str += std::string("uid=non_existent_id;");
+  // Invalid connect string
+  std::string connect_str = this->getInvalidConnectionString();
 
   ASSERT_OK_AND_ASSIGN(std::wstring wconnect_str,
                        arrow::util::UTF8ToWideString(connect_str));
@@ -902,7 +891,6 @@ TEST(SQLGetDiagFieldW, TestSQLGetDiagFieldWForConnectFailureNTS) {
   EXPECT_TRUE(ret == SQL_ERROR);
 
   // Retrieve all supported header level and record level data
-  SQLSMALLINT HEADER_LEVEL = 0;
   SQLSMALLINT RECORD_1 = 1;
 
   // SQL_DIAG_MESSAGE_TEXT SQL_NTS
@@ -929,7 +917,7 @@ TEST(SQLGetDiagFieldW, TestSQLGetDiagFieldWForConnectFailureNTS) {
   EXPECT_TRUE(ret == SQL_SUCCESS);
 }
 
-TEST(SQLGetDiagRec, TestSQLGetDiagRecForConnectFailure) {
+TYPED_TEST(FlightSQLODBCTestBase, TestSQLGetDiagRecForConnectFailure) {
   //  ODBC Environment
   SQLHENV env;
   SQLHDBC conn;
@@ -948,11 +936,8 @@ TEST(SQLGetDiagRec, TestSQLGetDiagRecForConnectFailure) {
 
   EXPECT_TRUE(ret == SQL_SUCCESS);
 
-  // Connect string
-  ASSERT_OK_AND_ASSIGN(std::string connect_str,
-                       arrow::internal::GetEnvVar(TEST_CONNECT_STR));
-  // Append invalid uid to connection string
-  connect_str += std::string("uid=non_existent_id;");
+  // Invalid connect string
+  std::string connect_str = this->getInvalidConnectionString();
 
   ASSERT_OK_AND_ASSIGN(std::wstring wconnect_str,
                        arrow::util::UTF8ToWideString(connect_str));
@@ -978,7 +963,7 @@ TEST(SQLGetDiagRec, TestSQLGetDiagRecForConnectFailure) {
 
   EXPECT_TRUE(ret == SQL_SUCCESS);
 
-  EXPECT_GT(message_length, 200);
+  EXPECT_GT(message_length, 120);
 
   EXPECT_EQ(native_error, 200);
 
@@ -1000,6 +985,12 @@ TEST(SQLGetDiagRec, TestSQLGetDiagRecForConnectFailure) {
   EXPECT_TRUE(ret == SQL_SUCCESS);
 }
 
+TYPED_TEST(FlightSQLODBCTestBase, TestConnect) {
+  // Verifies connect and disconnect works on its own
+  this->connect();
+  this->disconnect();
+}
+
 }  // namespace integration_tests
 }  // namespace odbc
 }  // namespace flight