Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
357 changes: 344 additions & 13 deletions google/cloud/odbc/bq_driver/internal/driver_adv_opt_form.cc

Large diffs are not rendered by default.

35 changes: 34 additions & 1 deletion google/cloud/odbc/bq_driver/internal/driver_adv_opt_form.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
#pragma comment(lib, "Comctl32.lib") // Link with Comctl32.lib

namespace google::cloud::odbc_bq_driver_internal {
// NEXTID:153
// NEXTID:155
static int const kIdcUseDefaultCheckbox = 128;
static int const kIdcDatasetNameEdit = 129;
static int const kIdcTempExpirationEdit = 130;
Expand All @@ -45,6 +45,8 @@ static int const kIdcMaxRetriesEdit = 149;
static int const kIdcEnablePscGcdCheckbox = 150;
static int const kIdcPrivateServiceNameEdit = 151;
static int const kIdcUniverseDomainEdit = 152;
static int const kIdcAllowedProjectsListView = 153;
static int const kIdcLoadProjectsButton = 154;

class AdvanceOptions {
public:
Expand Down Expand Up @@ -75,6 +77,9 @@ class AdvanceOptions {
inline std::string const& GetAdditionalProjects() const {
return additional_projects_;
}
inline std::string const& GetAllowedProjects() const {
return allowed_projects_;
}
inline std::string const& GetQueryProperties() const {
return query_properties_;
}
Expand Down Expand Up @@ -124,6 +129,7 @@ class AdvanceOptions {
static std::string default_string_length_;
static std::string session_location_;
static std::string additional_projects_;
static std::string allowed_projects_;
static std::string query_properties_;
static std::string use_wchar_;
static std::string enable_session_;
Expand All @@ -137,6 +143,33 @@ class AdvanceOptions {
static std::string enable_gcd_;
static std::string universe_domain_;

// Current vertical scroll offset, in pixels, of the control area. The dialog
// is taller than the work area on small or scaled displays, so it scrolls.
static int scroll_pos_;

// Leftover wheel movement smaller than one notch. High-resolution wheels and
// precision trackpads report fractions of WHEEL_DELTA, which would otherwise
// be rounded away to nothing.
static int wheel_remainder_;

// Recompute the scrollbar range/page from the current client height. Call
// after the controls are created and whenever the window is resized.
static void UpdateScrollInfo(HWND hwnd);

// Scroll the control area to 'new_pos' pixels, clamped to the scrollable
// range. Moves the child controls with it.
static void ScrollTo(HWND hwnd, int new_pos);

// Fill the allowed-projects pick list with 'project_ids', keeping ticked any
// id that is currently ticked as well as any id already in
// 'allowed_projects_'. Used both when the dialog opens and when the user
// reloads the list from the account.
static void PopulateAllowedProjectsListView(
HWND h_list_view, std::vector<std::string> const& project_ids);

// Comma-join the ticked rows of the allowed-projects pick list.
static std::string CollectCheckedProjects(HWND h_list_view);

static LRESULT CALLBACK AdvanceOptProc(HWND hwnd, UINT uMsg, WPARAM w_param,
LPARAM l_param);
static char const CLASS_NAME[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,31 @@ TEST_F(AdvanceOptionsTest, ShowWindow) {
<< "Window should be visible after calling Show.";
}

TEST_F(AdvanceOptionsTest, AllowedProjectsPickListRoundTripsThroughOk) {
// Opening the dialog with a saved value should tick those ids without
// contacting the account, and OK should save back exactly what is ticked.
advance_options->SetValues({{"AllowedProjects", "project-a,project-b"}});

// Show() runs a modal GetMessage loop. Queue WM_QUIT first so it builds the
// controls and returns immediately instead of blocking this test; the window
// itself stays alive for the assertions below.
PostQuitMessage(0);
advance_options->Show(nullptr);
HWND hwnd = advance_options->GetHwnd();
ASSERT_NE(hwnd, nullptr) << "Window should be created and displayed.";

HWND h_list = GetDlgItem(hwnd, kIdcAllowedProjectsListView);
ASSERT_NE(h_list, nullptr) << "Allowed projects list should be created.";
ASSERT_EQ(ListView_GetItemCount(h_list), 2);
EXPECT_TRUE(ListView_GetCheckState(h_list, 0));
EXPECT_TRUE(ListView_GetCheckState(h_list, 1));

ListView_SetCheckState(h_list, 1, FALSE);
ClickButton(hwnd, kIdcOKButton);

EXPECT_EQ(advance_options->GetAllowedProjects(), "project-a");
}

TEST_F(AdvanceOptionsTest, SetValuesValidinput) {
Section attribute_map = {{"SQLDialect", "1"},
{"LargeResultsDatasetId", "dataset1"},
Expand All @@ -81,6 +106,7 @@ TEST_F(AdvanceOptionsTest, SetValuesValidinput) {
{"LargeResultsTempTableExpirationTime", "3600000"},
{"SessionLocation", "USA"},
{"AdditionalProjects", "projectA,projectB"},
{"AllowedProjects", "projectC,projectD"},
{"QueryProperties", "property1=value1"},
{"MaxThreads", "10"},
{"MaxRetries", "9"}};
Expand All @@ -96,6 +122,7 @@ TEST_F(AdvanceOptionsTest, SetValuesValidinput) {
EXPECT_EQ(options.GetTempTableExpiration(), "3600000");
EXPECT_EQ(options.GetSessionLocation(), "USA");
EXPECT_EQ(options.GetAdditionalProjects(), "projectA,projectB");
EXPECT_EQ(options.GetAllowedProjects(), "projectC,projectD");
EXPECT_EQ(options.GetQueryProperties(), "property1=value1");
EXPECT_EQ(options.GetMaxThreads(), "10");
EXPECT_EQ(options.GetMaxRetries(), "9");
Expand All @@ -118,8 +145,19 @@ TEST_F(AdvanceOptionsTest, SetValuesMissingkeys) {
EXPECT_EQ(options.GetTempTableExpiration(), "3600000");
EXPECT_EQ(options.GetSessionLocation(), "");
EXPECT_EQ(options.GetAdditionalProjects(), "");
EXPECT_EQ(options.GetAllowedProjects(), "");
EXPECT_EQ(options.GetQueryProperties(), "");
EXPECT_EQ(options.GetAllowHtapiForLargeResultsCheckbox(), "");
}

TEST_F(AdvanceOptionsTest, ResetToDefaultsClearsAllowedProjects) {
AdvanceOptions options;
options.SetValues({{"AllowedProjects", "projectC,projectD"}});
ASSERT_EQ(options.GetAllowedProjects(), "projectC,projectD");

options.ResetToDefaults();

EXPECT_EQ(options.GetAllowedProjects(), "");
}

} // namespace google::cloud::odbc_bq_driver_internal
17 changes: 14 additions & 3 deletions google/cloud/odbc/bq_driver/internal/driver_form.cc
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ static Section BuildTestConnectionAttributes(
attributes_map["SessionLocation"] = adv_form.GetSessionLocation();
attributes_map["MaxThreads"] = adv_form.GetMaxThreads();
attributes_map["AdditionalProjects"] = adv_form.GetAdditionalProjects();
attributes_map["AllowedProjects"] = adv_form.GetAllowedProjects();
attributes_map["QueryProperties"] = adv_form.GetQueryProperties();
attributes_map["UseWVarChar"] = adv_form.GetUseWchar();
attributes_map["EnableSession"] = adv_form.GetEnableSession();
Expand Down Expand Up @@ -907,12 +908,22 @@ LRESULT CALLBACK DriverForm::WindowProc(HWND hwnd, UINT u_msg, WPARAM w_param,
return 1; // Indicate we handled the background redraw
}
case WM_LBUTTONDOWN: {
// The documentation hyperlink is currently not created (see the
// commented-out CreateHyperlinkLabel call), so GetDlgItem returns NULL.
// Without these guards GetClientRect fails and leaves 'rect'
// uninitialised, and clicks on empty parts of the dialog can fall inside
// that garbage rectangle and launch a browser.
HWND h_hyperlink = GetDlgItem(hwnd, kIdcHyperlink3);
if (h_hyperlink == NULL) {
break;
}
RECT rect = {};
if (!GetClientRect(h_hyperlink, &rect)) {
break;
}
POINT pt;
GetCursorPos(&pt);
ScreenToClient(hwnd, &pt);
HWND h_hyperlink = GetDlgItem(hwnd, kIdcHyperlink3);
RECT rect;
GetClientRect(h_hyperlink, &rect);
MapWindowPoints(h_hyperlink, hwnd, (LPPOINT)&rect, 2);
if (PtInRect(&rect, pt)) {
ShellExecute(NULL, "open", kBigQueryDocsURL, NULL, NULL, SW_SHOWNORMAL);
Expand Down
1 change: 1 addition & 0 deletions google/cloud/odbc/bq_driver/internal/odbc_conn_handle.cc
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ void ConnectionHandle::SetUp(Section& dsn_section,
dsn_.kms_key_name = dsn_section["KMSKEYNAME"];
dsn_.session_location = dsn_section["SESSIONLOCATION"];
dsn_.additional_projects = dsn_section["ADDITIONALPROJECTS"];
dsn_.allowed_projects = dsn_section["ALLOWEDPROJECTS"];
dsn_.psc = dsn_section["PRIVATESERVICECONNECTURIS"];
dsn_.enable_gcd =
dsn_section["ENABLEGCD"] == "1" || dsn_section["ENABLEGCD"] == "true";
Expand Down
1 change: 1 addition & 0 deletions google/cloud/odbc/bq_driver/internal/odbc_conn_handle.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ struct Dsn {
// Proxy options fields
google::cloud::odbc_bigquery_client_interface::ProxyOptions proxy_options;
std::string additional_projects;
std::string allowed_projects;
std::string psc;
bool enable_gcd;
std::string universe_domain;
Expand Down
23 changes: 23 additions & 0 deletions google/cloud/odbc/bq_driver/internal/odbc_conn_handle_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,29 @@ TEST(ConnectionHandle, DsnSetupListProjectsParentSet) {
EXPECT_EQ(actual.list_projects_parent, kDsnListProjectsParent);
}

TEST(ConnectionHandle, DsnSetupAllowedProjectsNotSet) {
ConnectionHandle conn_handle;
Section dsn_section;

conn_handle.SetUp(dsn_section, kDsnName);

Dsn actual = conn_handle.GetDsn();
EXPECT_TRUE(actual.allowed_projects.empty());
}

TEST(ConnectionHandle, DsnSetupAllowedProjectsSet) {
ConnectionHandle conn_handle;
Section dsn_section;

dsn_section["ALLOWEDPROJECTS"] = "project-1,project-2";
conn_handle.SetUp(dsn_section, kDsnName);

Dsn actual = conn_handle.GetDsn();
EXPECT_EQ(actual.allowed_projects, "project-1,project-2");
// AdditionalProjects and AllowedProjects are independent settings.
EXPECT_TRUE(actual.additional_projects.empty());
}

TEST(ConnectionHandle, SetAttributeSuccessSQLUInt) {
ConnectionHandle conn_handle;

Expand Down
50 changes: 41 additions & 9 deletions google/cloud/odbc/bq_driver/internal/odbc_sql_tables.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,38 @@ StatusRecord ValidateInputParameters(
return StatusRecord::Ok();
}

std::vector<std::string> FilterAllowedProjects(
std::string const& allowed_projects, std::string const& projects_filter,
SQLULEN metadata_id) {
std::vector<std::string> project_ids;
auto filter_regex = BuildRegex(projects_filter, metadata_id);
std::vector<std::string> allowed_ids = Split(allowed_projects, ",");
for (auto& project_id : allowed_ids) {
Trim(project_id);
if (project_id.empty()) {
continue;
}
if ((!metadata_id && projects_filter == kMatchAll) ||
re2::RE2::FullMatch(project_id, *filter_regex)) {
project_ids.push_back(std::move(project_id));
}
}
return project_ids;
}

StatusRecordOr<std::vector<std::string>> GetFilteredProjectIds(
ODBCBQClient& bq_client, std::string const& projects_filter,
SQLULEN metadata_id) {
SQLULEN metadata_id, std::string const& allowed_projects) {
// An explicit allowlist replaces REST-based project discovery entirely.
// projects.list enumerates every project the authenticated principal can
// reach, which is slow and quota-heavy for accounts with access to many
// projects; when the user has named the projects they care about there is
// nothing to discover.
if (!allowed_projects.empty()) {
return FilterAllowedProjects(allowed_projects, projects_filter,
metadata_id);
}

std::vector<std::string> project_ids;
auto filter_regex = BuildRegex(projects_filter, metadata_id);
// For now, we use default options.
Expand Down Expand Up @@ -346,9 +375,10 @@ ResultSet ProcessStringResults(

StatusRecordOr<ResultSet> GetResultSetForProjects(
ODBCBQClient& bq_client, SQLULEN metadata_id,
std::string const& additional_projects) {
auto project_ids_status =
GetFilteredProjectIds(bq_client, kMatchAll, metadata_id);
std::string const& additional_projects,
std::string const& allowed_projects) {
auto project_ids_status = GetFilteredProjectIds(
bq_client, kMatchAll, metadata_id, allowed_projects);
if (!project_ids_status) {
LOG(ERROR) << "GetResultSetForProjects::GetFilteredProjectIds:: "
<< project_ids_status.GetStatusRecord().message;
Expand All @@ -366,9 +396,10 @@ StatusRecordOr<ResultSet> GetResultSetForProjects(

StatusRecordOr<ResultSet> GetResultSetForDatasets(
ODBCBQClient& bq_client, SQLULEN metadata_id,
std::string const& catalog_name, std::string const& additional_projects) {
auto project_ids_status =
GetFilteredProjectIds(bq_client, catalog_name, metadata_id);
std::string const& catalog_name, std::string const& additional_projects,
std::string const& allowed_projects) {
auto project_ids_status = GetFilteredProjectIds(
bq_client, catalog_name, metadata_id, allowed_projects);
if (!project_ids_status) {
LOG(ERROR) << "GetResultSetForDatasets::GetFilteredProjectIds:: "
<< project_ids_status.GetStatusRecord().message;
Expand Down Expand Up @@ -416,6 +447,7 @@ StatusRecordOr<ResultSet> GetResultSetForTables(
std::string const& project_filter, std::string const& dataset_filter,
std::string const& table_filter, std::string const& table_type_filter,
SQLULEN metadata_id) {
ConnectionHandle& conn_handle = *(stmt_handle.GetConnectionHandle());
// When SQL_ATTR_METADATA_ID is true, the catalog argument is an exact project
// identifier (not a pattern). Skip enumerating every accessible project via
// projects.list and use the name directly.
Expand All @@ -424,15 +456,15 @@ StatusRecordOr<ResultSet> GetResultSetForTables(
project_list = {project_filter};
} else {
auto projects_status_record_or =
GetFilteredProjectIds(bq_client, project_filter, metadata_id);
GetFilteredProjectIds(bq_client, project_filter, metadata_id,
conn_handle.GetDsn().allowed_projects);
if (!projects_status_record_or) {
LOG(ERROR) << "GetResultSetForTables::GetFilteredProjectIds:: "
<< projects_status_record_or.GetStatusRecord().message;
return projects_status_record_or.GetStatusRecord();
}
project_list = *projects_status_record_or;
}
ConnectionHandle& conn_handle = *(stmt_handle.GetConnectionHandle());
// Append additional projects if any
project_list = AppendAdditionalProjectsIfMissing(
std::move(project_list), conn_handle.GetDsn().additional_projects);
Expand Down
18 changes: 15 additions & 3 deletions google/cloud/odbc/bq_driver/internal/odbc_sql_tables.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,22 @@ odbc_internal::StatusRecord ValidateInputParameters(
const SQLCHAR* table_name, SQLSMALLINT table_name_len,
SQLSMALLINT table_type_len, SQLULEN metadata_id);

// Return the project ids from the comma-separated 'allowed_projects' allowlist
// that match 'projects_filter', using the same LIKE-pattern /
// SQL_ATTR_METADATA_ID semantics as GetFilteredProjectIds. Blank entries are
// ignored and surrounding whitespace is trimmed. Makes no REST calls.
std::vector<std::string> FilterAllowedProjects(
std::string const& allowed_projects, std::string const& projects_filter,
SQLULEN metadata_id);

// Return a list of project ids depending on SQL_ATTR_METADATA_ID and
// 'projects_filter'. Returns all project ids if SQL_ATTR_METADATA_ID ==
// SQL_FALSE and projects_filter == "%".
// When 'allowed_projects' is non-empty it is used as the set of candidate
// projects and projects.list is not called at all.
odbc_internal::StatusRecordOr<std::vector<std::string>> GetFilteredProjectIds(
ODBCBQClient& bq_client, std::string const& projects_filter,
SQLULEN metadata_id);
SQLULEN metadata_id, std::string const& allowed_projects = "");

// Return a list of dataset ids depending on SQL_ATTR_METADATA_ID and
// 'datasets_filter'. Returns all dataset ids if SQL_ATTR_METADATA_ID ==
Expand Down Expand Up @@ -108,13 +118,15 @@ ResultSet ProcessStringResults(
// Search for all projects and populate ResultSet for it.
odbc_internal::StatusRecordOr<ResultSet> GetResultSetForProjects(
ODBCBQClient& bq_client, SQLULEN metadata_id,
std::string const& additional_projects = "");
std::string const& additional_projects = "",
std::string const& allowed_projects = "");

// Search for all datasets in all projects and populate ResultSet for it.
odbc_internal::StatusRecordOr<ResultSet> GetResultSetForDatasets(
ODBCBQClient& bq_client, SQLULEN metadata_id,
std::string const& catalog_name = kMatchAll,
std::string const& additional_projects = "");
std::string const& additional_projects = "",
std::string const& allowed_projects = "");

// Search for tables and populate ResultSet according to ODBC spec
odbc_internal::StatusRecordOr<ResultSet> GetResultSetForTables(
Expand Down
Loading